diff --git a/launchpad/scripts/security_audit_classifier.py b/launchpad/scripts/security_audit_classifier.py index 2ea9a7150cc..9a18c865c97 100644 --- a/launchpad/scripts/security_audit_classifier.py +++ b/launchpad/scripts/security_audit_classifier.py @@ -21,7 +21,7 @@ """ import subprocess -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Optional, Set UPSTREAM_URL = "https://github.com/block/buzz.git" @@ -64,4 +64,13 @@ def classify(path: str, upstream_paths: Optional[Set[str]]) -> str: """'fork-added', 'inherited', or 'indeterminate' when upstream_paths is None.""" if upstream_paths is None: return "indeterminate" - return "inherited" if path.replace("\\", "/") in upstream_paths else "fork-added" + normalized = path.replace("\\", "/") + if normalized.startswith("./"): + normalized = normalized[2:] + # git ls-tree's output (what upstream_paths is built from) is always relative. + # An absolute path can't be safely compared against it without knowing the + # repo root, and guessing fork-added for it is exactly the wrong-direction + # guess this module's docstring warns against — indeterminate is honest. + if PurePosixPath(normalized).is_absolute(): + return "indeterminate" + return "inherited" if normalized in upstream_paths else "fork-added" diff --git a/launchpad/scripts/test_security_audit_classifier.py b/launchpad/scripts/test_security_audit_classifier.py index 63c45fb9427..d76d5fc2ca4 100644 --- a/launchpad/scripts/test_security_audit_classifier.py +++ b/launchpad/scripts/test_security_audit_classifier.py @@ -60,6 +60,29 @@ def test_windows_style_separators_are_normalized(self): "inherited", ) + def test_dot_slash_prefixed_path_is_normalized(self): + self.assertEqual( + classify("./.github/workflows/ci.yml", _SYNTHETIC_UPSTREAM_PATHS), + "inherited", + ) + + def test_absolute_path_is_indeterminate_not_a_guess(self): + self.assertEqual( + classify( + "/home/serina/Launchpad/buzz/.github/workflows/ci.yml", + _SYNTHETIC_UPSTREAM_PATHS, + ), + "indeterminate", + ) + + def test_absolute_path_to_a_fork_only_file_is_still_indeterminate(self): + # A caller bug that computes absolute paths should never resolve to a + # guess in either direction, not just for genuinely-inherited files. + self.assertEqual( + classify("/home/serina/Launchpad/buzz/launchpad/deploy/README.md", _SYNTHETIC_UPSTREAM_PATHS), + "indeterminate", + ) + class FetchUpstreamPathsTest(unittest.TestCase): def test_returns_none_on_called_process_error(self):