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
7 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 −1
VERSION+1 −1
@@ -1 +1 @@1-3.2.01+3.2.1

view file

src/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))!

view file

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 		}

view file

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.

view file

tests/cli_testlarge

over 1 MB, not shown

view file

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')

view file

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 }

view file