427d85ab1fec
refuse restore paths outside the repo and keep sibling folders out of pruning (3.2.1)
- time
- 2026-09-28 15:02 UTC · 2 weeks ago
- machine
- Junes-MacBook-Pro.local
- agent
- gpt-5
- session
- harden shared path handling
- parent
- 87658431342d
↓ koh-427d85ab.tar.gzatlas at this save
files changed7 files
VERSION+1 −1src/exchange/exchange.v+6 −1src/snapshot/restore.v+16 −1src/snapshot/snapshot_test.v+34 −0tests/cli_testlargetests/cli_test.v+40 −0v.mod+1 −1src/exchange/exchange.v+6 −1
@@ -264,7 +264,12 @@264264 }265265 decode_save(record_object.body)!266266 }267- fetched.unsafe_paths << drop_unsafe_paths(mut record)267+ for file in record.files {268+ if !snapshot.legal_path(file.path) {269+ fetched.unsafe_paths << snapshot.escape_path(file.path)270+ }271+ }272+ drop_unsafe_paths(mut record)268273 for file in record.files {269274 if !store.has_object(file.hash)! && file.hash !in fetched.repo_objects {270275 blob := backend.get(object_key(repo, file.hash))!src/snapshot/restore.v+16 −1
@@ -60,6 +60,7 @@6060 }6161 6262 fn write_tree(store &storage.Store, root string, ig Ignore, target []FileEntry, remove []string) !RestoreReport {63+ check_containment(root, target, remove)!6364 mut wanted := map[string]bool{}6465 for entry in target {6566 wanted[entry.path] = true@@ -106,6 +107,19 @@106107 }107108 }108109 110+// check_containment proves the whole restore stays below root before its first removal or write.111+fn check_containment(root string, target []FileEntry, remove []string) ! {112+ base := os.abs_path(root).trim_right(os.path_separator) + os.path_separator113+ mut paths := target.map(it.path)114+ paths << remove115+ for path in paths {116+ full := os.abs_path(os.join_path(root, path))117+ if !legal_path(path) || !full.starts_with(base) {118+ return error('unsafe path ${escape_path(path)} is outside the repo')119+ }120+ }121+}122+109123 // already_there reports whether the file on disk has the target's content and110124 // executable bit. The file cache answers without reading when the file is as111125 // it was when last hashed.@@ -153,7 +167,8 @@153167 // prune_empty_dirs clears directories a restore emptied, never the repo root.154168 fn prune_empty_dirs(root string, dir string) {155169 mut current := dir156- for current.len > root.len && current.starts_with(root) {170+ inside := root.trim_right(os.path_separator) + os.path_separator171+ for current.starts_with(inside) {157172 if (os.ls(current) or { return }).len > 0 {158173 return159174 }src/snapshot/snapshot_test.v+34 −0
@@ -156,6 +156,40 @@156156 assert walked.unsafe_paths == ['Icon\\x0d', 'bad\\\\name.txt']157157 }158158 159+fn test_write_tree_refuses_every_path_outside_the_root_before_writing() {160+ base := temp_root('restore_containment')!161+ defer {162+ os.rmdir_all(base) or {}163+ }164+ root := os.join_path(base, 'repo')165+ os.mkdir(root)!166+ store := storage.create(root)!167+ defer {168+ store.close()169+ }170+ for path in ['../outside.txt', '/etc/koh-test', 'a/../../outside.txt', 'bad\\name', 'control\rname'] {171+ if _ := write_tree(store, root, Ignore{}, [FileEntry{172+ path: path173+ }], []) {174+ assert false, '${path} must be refused'175+ }176+ }177+ assert !os.exists(os.join_path(base, 'outside.txt'))178+}179+180+fn test_prune_empty_dirs_never_touches_a_sibling_with_the_roots_prefix() {181+ base := temp_root('prune_prefix')!182+ defer {183+ os.rmdir_all(base) or {}184+ }185+ root := os.join_path(base, 'repo')186+ sibling := os.join_path(base, 'repo2')187+ os.mkdir(root)!188+ os.mkdir(sibling)!189+ prune_empty_dirs(root, sibling)190+ assert os.is_dir(sibling)191+}192+159193 // The point of nanosecond timestamps: a file rewritten inside the same second160194 // with the same size must not reuse the cached hash, or a save would record161195 // content that is no longer on disk.tests/cli_test.v+40 −0
@@ -1027,6 +1027,46 @@10271027 assert !os.exists(os.join_path(b_parent, 'proj'))10281028 }10291029 1030+fn test_steal_drops_crafted_unsafe_paths_and_lands_the_rest() {1031+ syncroot := temp_repo('unsafe-remote')1032+ home := temp_repo('unsafe-remote-home')1033+ defer {1034+ os.rmdir_all(syncroot) or {}1035+ os.rmdir_all(home) or {}1036+ }1037+ remote := os.join_path(syncroot, 'remote')1038+ a := os.join_path(syncroot, 'A', 'proj')1039+ b_parent := os.join_path(syncroot, 'B')1040+ os.mkdir_all(a) or { panic(err) }1041+ os.mkdir_all(b_parent) or { panic(err) }1042+ init_repo(a)1043+ write(a, 'safe.txt', 'safe\n')1044+ assert run_home(a, home, 'remote', 'folder', remote).code == 01045+ id := run_home(a, home, 'save', 'crafted', '--json').out.all_after('"id":"')[..12]1046+ assert run_home(a, home, 'offer').code == 01047+ record_path := os.join_path(remote, 'proj', 'saves', id)1048+ record := os.read_file(record_path) or { panic(err) }1049+ path_at := record.index('"path":"safe.txt"') or { panic('safe path missing from ${record}') }1050+ file_start := record[..path_at].last_index('{') or { panic('safe file start missing') }1051+ file_end := path_at + (record[path_at..].index('}') or { panic('safe file end missing') }) + 11052+ file := record[file_start..file_end]1053+ mut files := [file]1054+ for path in ['../outside.txt', '/etc/koh-test', 'a/../../outside.txt', 'bad\\\\name',1055+ 'control\\rname'] {1056+ files << file.replace('safe.txt', path)1057+ }1058+ os.write_file(record_path, record[..file_start] + files.join(',') + record[file_end..]) or {1059+ panic(err)1060+ }1061+1062+ stolen := run_home(b_parent, home, 'steal', 'proj')1063+ assert stolen.code == 0, stolen.err1064+ assert os.read_file(os.join_path(b_parent, 'proj', 'safe.txt'))! == 'safe\n'1065+ assert !os.exists(os.join_path(b_parent, 'outside.txt'))1066+ assert !os.exists(os.join_path(b_parent, 'proj', 'bad', 'name'))1067+ assert !os.exists('${b_parent}/proj/control\rname')1068+}1069+10301070 fn test_offer_steal_folder_remote() {10311071 syncroot := temp_repo('sync')10321072 home := temp_repo('sync-home')v.mod+1 −1
@@ -1,6 +1,6 @@11 Module {22 name: 'koh3'3- version: '3.2.0'3+ version: '3.2.1'44 description: 'One SQLite file per repo, one line of history.'55 license: 'MIT'66 }