diff --git a/src/osforge/merge.py b/src/osforge/merge.py index 308e1e9..4fd6fae 100644 --- a/src/osforge/merge.py +++ b/src/osforge/merge.py @@ -6 +6 @@ import fnmatch -from functools import cmp_to_key +from functools import cmp_to_key, lru_cache @@ -887,0 +888,8 @@ def remove_path(path: Path) -> None: +@lru_cache(maxsize=128) +def _compiled_filters(filters: tuple[str, ...]): + patterns = frozenset(pattern.strip("/") for pattern in filters if pattern.strip("/")) + globs = [fnmatch.translate(pattern) for pattern in sorted(patterns) + if any(char in pattern for char in "*?[")] + return patterns, re.compile("|".join(globs)) if globs else None + + @@ -890 +898 @@ def rel_filtered(relpath: str, filters: list[str]) -> bool: - if relpath in ("", "."): + if relpath in ("", ".") or not filters: @@ -892,7 +900,6 @@ def rel_filtered(relpath: str, filters: list[str]) -> bool: - for raw_pattern in filters: - pattern = raw_pattern.strip("/") - if not pattern: - continue - if relpath == pattern or relpath.startswith(f"{pattern}/"): - return True - if fnmatch.fnmatchcase(relpath, pattern): + patterns, globs = _compiled_filters(tuple(filters)) + # Exact directory rules also exclude descendants. This includes literal + # names containing glob characters, as the original matcher did. + ancestor = relpath + while ancestor: + if ancestor in patterns: @@ -900 +907,2 @@ def rel_filtered(relpath: str, filters: list[str]) -> bool: - return False + ancestor = ancestor.rpartition("/")[0] + return globs is not None and globs.match(relpath) is not None diff --git a/src/osforge/repositories.py b/src/osforge/repositories.py index f80c013..b18ca77 100644 --- a/src/osforge/repositories.py +++ b/src/osforge/repositories.py @@ -21 +21 @@ from .jobs import load_run_details -from .transition import Runtime, Operation, atomic_json, atomic_text, lock, name, read_toml, sync_directory, tree_digest +from .transition import Runtime, Operation, atomic_json, atomic_text, lock, name, read_toml, sync_directory, tree_digest, validate_tree @@ -135 +135 @@ def overlay(source: Path, destination: Path, *, selection: dict | None = None) - - tree_digest(source) # reject external links and special files before copying + validate_tree(source) # content hashes are checked at capture/freeze/publication boundaries diff --git a/src/osforge/repository_activation.py b/src/osforge/repository_activation.py index e6d0682..ea84673 100644 --- a/src/osforge/repository_activation.py +++ b/src/osforge/repository_activation.py @@ -85,0 +86,5 @@ def prepare_release_access(data: dict, home: Path, release: Path) -> None: + if release.is_symlink(): + raise ValueError("release root must be a real directory") + # The tree's containing directory is outside tree_digest, just like its + # release parent. A private publisher umask must not hide valid contents. + os.chmod(release, stat.S_IMODE(release.stat().st_mode) | stat.S_IRGRP | stat.S_IXGRP) diff --git a/src/osforge/transition.py b/src/osforge/transition.py index db47908..7e79634 100644 --- a/src/osforge/transition.py +++ b/src/osforge/transition.py @@ -97,2 +97,2 @@ def lock(path: Path) -> Iterator[None]: -def tree_digest(root: Path) -> str: - """Hash names, modes, link targets and bytes, excluding Git administration.""" +def _tree_entries(root: Path): + """Visit validated entries in the existing manifest's deterministic order.""" @@ -101 +101 @@ def tree_digest(root: Path) -> str: - digest = hashlib.sha256() + resolved = root.resolve() @@ -104 +104,2 @@ def tree_digest(root: Path) -> str: - base = Path(directory) + relative_dir = os.path.relpath(directory, root) + prefix = "" if relative_dir == "." else relative_dir + "/" @@ -108,5 +109,5 @@ def tree_digest(root: Path) -> str: - path = base / item - info = path.lstat() - relative = path.relative_to(root).as_posix() - digest.update(json.dumps([relative, stat.S_IMODE(info.st_mode)], separators=(",", ":")).encode()) - if path.is_symlink(): + path = os.path.join(directory, item) + info = os.lstat(path) + relative = prefix + item + target = None + if stat.S_ISLNK(info.st_mode): @@ -114 +115 @@ def tree_digest(root: Path) -> str: - if not path.resolve(strict=False).is_relative_to(root.resolve()): + if not Path(path).resolve(strict=False).is_relative_to(resolved): @@ -116,9 +117 @@ def tree_digest(root: Path) -> str: - digest.update(b"L" + os.fsencode(target)) - elif path.is_file(): - digest.update(b"F") - with path.open("rb") as handle: - for block in iter(lambda: handle.read(1024 * 1024), b""): - digest.update(block) - elif path.is_dir(): - digest.update(b"D") - else: + elif not (stat.S_ISREG(info.st_mode) or stat.S_ISDIR(info.st_mode)): @@ -125,0 +119,23 @@ def tree_digest(root: Path) -> str: + yield relative, path, info.st_mode, target + + +def validate_tree(root: Path) -> None: + """Reject escaping links and special files without reading package bytes.""" + for _entry in _tree_entries(root): + pass + + +def tree_digest(root: Path) -> str: + """Hash names, modes, link targets and bytes, excluding Git administration.""" + digest = hashlib.sha256() + for relative, path, mode, target in _tree_entries(root): + digest.update(json.dumps([relative, stat.S_IMODE(mode)], separators=(",", ":")).encode()) + if target is not None: + digest.update(b"L" + os.fsencode(target)) + elif stat.S_ISREG(mode): + digest.update(b"F") + with open(path, "rb") as handle: + for block in iter(lambda: handle.read(65536), b""): + digest.update(block) + else: + digest.update(b"D") diff --git a/tests/test_repository_activation.py b/tests/test_repository_activation.py index 0979c78..e10064d 100644 --- a/tests/test_repository_activation.py +++ b/tests/test_repository_activation.py @@ -112,0 +113,35 @@ class RepositoryActivationTests(unittest.TestCase): + def test_publication_under_private_umask_keeps_release_readable(self): + previous = os.umask(0o077) + try: + candidate = self.update() + digest = tree_digest(self.home / 'candidates' / candidate / 'tree') + self.publish(candidate) + finally: + os.umask(previous) + self.assertEqual(self.location().stat().st_mode & 0o050, 0o050) + self.assertEqual(tree_digest(self.location()), digest) + + def test_retry_repairs_only_the_retained_tree_container(self): + candidate = self.update() + with patch('osforge.repositories.prepare_release_access', side_effect=OSError('interrupted before activation')): + with self.assertRaisesRegex(OSError, 'interrupted'): + self.publish(candidate) + release = self.home / 'releases' / candidate / 'tree' + release.chmod(0o700) + digest = tree_digest(release) + self.publish(candidate) + self.assertEqual(tree_digest(release), digest) + self.assertEqual((release / 'payload').stat().st_mode & 0o777, 0o644) + self.assertEqual(git('--git-dir', str(self.home / 'repository.git'), 'rev-list', '--count', 'develop'), '1') + + def test_release_root_symlink_cannot_change_outside_permissions(self): + from osforge.repository_activation import prepare_release_access + outside = self.root / 'private' + outside.mkdir(mode=0o700) + release = self.home / 'releases/linked-tree' + release.symlink_to(outside, target_is_directory=True) + data = read_toml(self.runtime.repositories_dir / 'example.toml') + with self.assertRaisesRegex(ValueError, 'real directory'): + prepare_release_access(data, self.home, release) + self.assertEqual(outside.stat().st_mode & 0o777, 0o700) + diff --git a/tests/test_repository_tree.py b/tests/test_repository_tree.py new file mode 100644 index 0000000..ca70e3b --- /dev/null +++ b/tests/test_repository_tree.py @@ -0,0 +1,116 @@ +from pathlib import Path +from tempfile import TemporaryDirectory +import fnmatch +import os +import unittest + +from osforge.merge import rel_filtered +from osforge.repositories import overlay +from osforge.transition import tree_digest, validate_tree + + +class FilterMatchingTests(unittest.TestCase): + def test_exact_glob_and_literal_prefix_contract(self): + cases = [ + ('cat/pkg/file', ['cat/pkg'], True), + ('cat/pkg-other/file', ['cat/pkg'], False), + ('cat/pkg/file', ['/cat/pkg/'], True), + ('cat/pkg/files/patch', ['cat/*/files'], False), + ('cat/pkg/files/patch', ['cat/*/files/*'], True), + ('cat/*/files/patch', ['cat/*/files'], True), + ('cat/pkg/version-2.ebuild', ['cat/pkg/version-?.ebuild'], True), + ('cat/pkg/version-12.ebuild', ['cat/pkg/version-?.ebuild'], False), + ('cat/Pkg', ['cat/pkg'], False), + ('cat/[broken', ['cat/[broken'], True), + ('cat/pkg2', ['cat/pkg[!1]'], True), + ('cat/pkg1', ['cat/pkg[!1]'], False), + ('cat/a+b', ['cat/a+b'], True), + ('cat/ab', ['cat/a+b'], False), + ('cat/pkg', ['', '/'], False), + ('/', ['*'], False), + ('.', ['*'], False), + ] + for path, patterns, expected in cases: + with self.subTest(path=path, patterns=patterns): + self.assertEqual(rel_filtered(path, patterns), expected) + + def test_mutated_filter_list_does_not_reuse_obsolete_rules(self): + rules = ['cat/old'] + self.assertTrue(rel_filtered('cat/old/file', rules)) + rules[:] = ['cat/new'] + self.assertFalse(rel_filtered('cat/old/file', rules)) + self.assertTrue(rel_filtered('cat/new/file', rules)) + + def test_compiled_rules_agree_with_previous_matcher(self): + rules = ['cat/pkg', 'cat/a*', 'cat/[ab]?', 'cat/name[', 'cat/é', + 'cat/line\nbreak', 'cat/*/files', '/other/', 'pkg-?.ebuild'] + names = ['pkg', 'pkg-other', 'abc', 'a1', 'b2', 'name[', 'é', + 'line\nbreak', '*', 'pkg-2.ebuild', 'pkg-12.ebuild'] + for name in names: + for prefix in ['', 'cat/', 'other/']: + for suffix in ['', '/files', '/files/patch']: + path = prefix + name + suffix + expected = any(path == rule.strip('/') or + path.startswith(rule.strip('/') + '/') or + fnmatch.fnmatchcase(path, rule.strip('/')) for rule in rules) + self.assertEqual(rel_filtered(path, rules), expected, path) + + +class RepositoryTreeTests(unittest.TestCase): + def setUp(self): + self.temp = TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) / 'tree' + self.root.mkdir() + + def fixture(self): + p = self.root + (p / 'a').mkdir(); (p / 'a').chmod(0o755) + (p / 'a/nested').mkdir(); (p / 'a/nested').chmod(0o700) + (p / 'a/value').write_bytes(b'A\r\n'); (p / 'a/value').chmod(0o750) + (p / 'z').write_bytes(b'\x00\xff'); (p / 'z').chmod(0o644) + (p / 'é').write_bytes(b'unicode'); (p / 'é').chmod(0o444) + (p / 'link').symlink_to('a/value') + (p / '.git').mkdir(); (p / '.git/config').write_text('ignored') + + def test_existing_digest_format_is_preserved(self): + self.fixture() + self.assertEqual(tree_digest(self.root), 'f6285874533e15e8f7099022a9f6d4ec3212e0b882eb117fe0767e86bbee10f6') + (self.root / '.git/config').write_text('still ignored') + self.root.chmod(0o700) # the containing directory is outside the digest + self.assertEqual(tree_digest(self.root), 'f6285874533e15e8f7099022a9f6d4ec3212e0b882eb117fe0767e86bbee10f6') + + def test_bytes_modes_and_link_targets_remain_integrity_inputs(self): + self.fixture() + original = tree_digest(self.root) + file = self.root / 'a/value' + file.write_bytes(b'B\r\n') + self.assertNotEqual(tree_digest(self.root), original) + file.write_bytes(b'A\r\n') + file.chmod(0o700) + self.assertNotEqual(tree_digest(self.root), original) + file.chmod(0o750) + (self.root / 'link').unlink() + (self.root / 'link').symlink_to('z') + self.assertNotEqual(tree_digest(self.root), original) + + def test_escaping_links_are_rejected_before_any_copy(self): + (self.root / 'first').write_text('package') + (self.root / 'outside').symlink_to(self.root.parent) + destination = self.root.parent / 'copy' + for check in [tree_digest, validate_tree]: + with self.assertRaisesRegex(ValueError, 'escapes'): + check(self.root) + with self.assertRaisesRegex(ValueError, 'escapes'): + overlay(self.root, destination) + self.assertFalse(destination.exists()) + + def test_special_files_are_rejected_before_any_copy(self): + os.mkfifo(self.root / 'pipe') + destination = self.root.parent / 'copy' + for check in [tree_digest, validate_tree]: + with self.assertRaisesRegex(ValueError, 'unsupported'): + check(self.root) + with self.assertRaisesRegex(ValueError, 'unsupported'): + overlay(self.root, destination) + self.assertFalse(destination.exists())