Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 89 additions & 1 deletion launchpad/scripts/security_audit_classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import re
import subprocess
from pathlib import Path, PurePosixPath
from typing import Optional, Set
from typing import Dict, Optional, Set

#: Matches a Windows drive letter root ("C:/", "d:/") after backslash-to-slash
#: normalization. PurePosixPath.is_absolute() only recognizes a leading "/" --
Expand Down Expand Up @@ -85,3 +85,91 @@ def classify(path: str, upstream_paths: Optional[Set[str]]) -> str:
if PurePosixPath(normalized).is_absolute() or _WINDOWS_DRIVE_ROOT.match(normalized):
return "indeterminate"
return "inherited" if normalized in upstream_paths else "fork-added"


def fetch_upstream_blobs(repo_root: Path, timeout: int = 30) -> Optional[Dict[str, str]]:
"""Map every upstream path to its blob OID, or None if upstream is unreachable.

Same single fetch as `fetch_upstream_paths`, one column wider: `git ls-tree`
already carries the OID, so identity costs no extra network. Callers that
only need origin should keep using `fetch_upstream_paths`.

Origin is not enough for a check that asks "is the cohort accountable for
this file". A path present upstream may still have been modified here, and a
modification is exactly where cohort-owned content could hide inside an
inherited filename. Comparing OIDs answers "is this byte-for-byte upstream's
file", which is the question that licenses skipping it.
"""
try:
subprocess.run(
["git", "fetch", "--depth=1", UPSTREAM_URL, UPSTREAM_REF],
cwd=repo_root,
check=True,
capture_output=True,
timeout=timeout,
)
result = subprocess.run(
# -z for the same quoting reason `fetch_upstream_paths` documents.
# Default (non---name-only) output is "<mode> <type> <oid>\t<path>".
["git", "ls-tree", "-r", "-z", "FETCH_HEAD"],
cwd=repo_root,
check=True,
capture_output=True,
text=True,
timeout=timeout,
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError):
return None
blobs: Dict[str, str] = {}
for entry in filter(None, result.stdout.split("\0")):
meta, _, path = entry.partition("\t")
parts = meta.split()
if not path or len(parts) < 3:
continue
blobs[path] = parts[2]
return blobs


def local_blob(repo_root: Path, path: str, timeout: int = 30) -> Optional[str]:
"""The blob OID of `path` at HEAD, or None when it cannot be resolved."""
try:
result = subprocess.run(
["git", "rev-parse", f"HEAD:{path}"],
cwd=repo_root,
check=True,
capture_output=True,
text=True,
timeout=timeout,
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError):
return None
return result.stdout.strip() or None


def divergence(path: str, repo_root: Path, upstream_blobs: Optional[Dict[str, str]]) -> str:
"""Ownership of one path, as one of four answers.

- `fork-added` — absent upstream. The cohort wrote it.
- `inherited-modified` — present upstream, different content here.
- `inherited-identical`— present upstream, byte-for-byte identical.
- `indeterminate` — upstream unreachable, or the local OID would not
resolve. Never guessed in either direction, for the
asymmetry this module's docstring sets out.

Only `inherited-identical` licenses a caller to treat a file as none of the
cohort's business. The other three are all cohort-accountable or unknown.
"""
if upstream_blobs is None:
return "indeterminate"
normalized = path.replace("\\", "/")
if normalized.startswith("./"):
normalized = normalized[2:]
if PurePosixPath(normalized).is_absolute() or _WINDOWS_DRIVE_ROOT.match(normalized):
return "indeterminate"
upstream_oid = upstream_blobs.get(normalized)
if upstream_oid is None:
return "fork-added"
here = local_blob(repo_root, normalized)
if here is None:
return "indeterminate"
return "inherited-identical" if here == upstream_oid else "inherited-modified"
77 changes: 73 additions & 4 deletions launchpad/scripts/security_audit_tracked_files_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from typing import List, Optional

from security_audit_core import CheckResult, Status
from security_audit_classifier import divergence, fetch_upstream_blobs
from security_audit_ignore_coverage_check import REQUIRED_COVERAGE

NAME = "tracked-sensitive-files"
Expand Down Expand Up @@ -149,6 +150,46 @@ def _check_newly_hidden_tracked_files(repo_root: Path, tracked: List[str]) -> Li
return findings


def _partition_by_ownership(
repo_root: Path, hits: List[str]
) -> tuple[List[str], List[str], List[str]]:
"""Split sensitive-shaped hits into (cohort, upstream_identical, unknown).

WHY OWNERSHIP MATTERS HERE. This is a fork that operates the upstream
product rather than developing it. Upstream ships APNs test fixtures that
are genuinely private-key-shaped and genuinely private keys
(`crates/buzz-push-gateway/tests/fixtures/apns-test-*.pem`, from
`c432a111c`, an ancestor of `block/buzz@main`, byte-for-byte identical
here). This check flagged all six on every branch cut after 2026-08-28, so
`audit` was red on trunk and on every pull request regardless of content
(launchpad-26/buzz#1965). A deterministic check that fails for everyone
stops being read.

The cohort cannot fix those files — it does not own them, and it does not
move or rename upstream paths. What it can do is be precise about what it is
accountable for. So this narrows by OWNERSHIP, not by adding an exemption
list: an allowlist would be a standing hole, and this check deliberately has
none (see `_matches_sensitive_shape`, where a suffix exemption was removed
after one let a tracked `seed/authorized_keys.example` through).

Identity, not origin. A file present upstream but MODIFIED here still fails:
an inherited filename is exactly where cohort-added key material could hide.
"""
upstream_blobs = fetch_upstream_blobs(repo_root)
cohort: List[str] = []
upstream_identical: List[str] = []
unknown: List[str] = []
for path in hits:
verdict = divergence(path, repo_root, upstream_blobs)
if verdict == "inherited-identical":
upstream_identical.append(path)
elif verdict == "indeterminate":
unknown.append(path)
else: # fork-added or inherited-modified — the cohort is accountable
cohort.append(path)
return cohort, upstream_identical, unknown


def run(repo_root: Path) -> CheckResult:
tracked = _tracked_paths(repo_root)
if tracked is None:
Expand All @@ -157,13 +198,31 @@ def run(repo_root: Path) -> CheckResult:
sensitive_hits = sorted(p for p in tracked if _matches_sensitive_shape(p))
newly_hidden = _check_newly_hidden_tracked_files(repo_root, tracked)

cohort_hits: List[str] = []
upstream_hits: List[str] = []
unknown_hits: List[str] = []
if sensitive_hits:
cohort_hits, upstream_hits, unknown_hits = _partition_by_ownership(
repo_root, sensitive_hits
)

if cohort_hits:
return CheckResult(
NAME,
Status.FAIL,
f"{len(sensitive_hits)} tracked file(s) match a sensitive shape: "
+ "; ".join(sensitive_hits[:10])
+ (f", and {len(sensitive_hits) - 10} more" if len(sensitive_hits) > 10 else ""),
f"{len(cohort_hits)} cohort-owned tracked file(s) match a sensitive shape: "
+ "; ".join(cohort_hits[:10])
+ (f", and {len(cohort_hits) - 10} more" if len(cohort_hits) > 10 else ""),
)
if unknown_hits:
# Upstream unreachable: ownership is unknown, so neither failing nor
# passing is honest. INDETERMINATE is visibly not-green and cannot be
# mistaken for a clean result.
return CheckResult(
NAME,
Status.INDETERMINATE,
f"could not establish upstream ownership for {len(unknown_hits)} "
f"sensitive-shaped tracked file(s): " + "; ".join(unknown_hits[:10]),
)
if newly_hidden:
return CheckResult(
Expand All @@ -172,8 +231,18 @@ def run(repo_root: Path) -> CheckResult:
f"{len(newly_hidden)} newly-added ignore pattern(s) cover already-tracked content: "
+ "; ".join(newly_hidden),
)
# The upstream-identical hits are named, not silently dropped. A skip nobody
# can see is indistinguishable from a check that stopped looking.
suffix = ""
if upstream_hits:
suffix = (
f"; {len(upstream_hits)} sensitive-shaped file(s) are byte-for-byte "
f"upstream's and not cohort-owned: " + "; ".join(upstream_hits[:10])
+ (f", and {len(upstream_hits) - 10} more" if len(upstream_hits) > 10 else "")
)
return CheckResult(
NAME,
Status.PASS,
f"no tracked file matches a sensitive shape ({len(tracked)} tracked files checked)",
f"no cohort-owned tracked file matches a sensitive shape "
f"({len(tracked)} tracked files checked){suffix}",
)
125 changes: 123 additions & 2 deletions launchpad/scripts/test_security_audit_tracked_files_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,42 @@ def _init_repo_with_files(root: Path, files: dict[str, str]) -> None:
subprocess.run(["git", "commit", "-q", "-m", "test fixture"], cwd=root, check=True)


class TrackedSensitiveFilesTest(unittest.TestCase):
# FIXTURE CONTENT IS DELIBERATELY NOT KEY-SHAPED. The check under test matches on
# path shape, never on content, so a fixture never needs a real PEM header — and
# an earlier revision of this file that used one made `gitleaks-secret-scan` fail
# on two of these very lines. A control for a secret-detection suite must not
# itself be the thing it detects.

def _oid(root: Path, rel_path: str) -> str:
"""The committed blob OID of `rel_path`, for building a fake upstream tree."""
result = subprocess.run(
["git", "rev-parse", f"HEAD:{rel_path}"],
cwd=root, check=True, capture_output=True, text=True,
)
return result.stdout.strip()


class _NoUpstreamMixin:
"""Keep these tests off the network.

`run()` now asks `security_audit_classifier.fetch_upstream_blobs` who owns a
sensitive-shaped hit, and that shells out to `git fetch` against block/buzz.
Unpatched, every case below would clone a real remote to decide something
the fixture already knows. An empty upstream tree is the honest stand-in for
"this fixture repo inherited nothing", which is what makes every
pre-existing expectation here still the right one.
"""

def setUp(self):
patcher = mock.patch(
"security_audit_tracked_files_check.fetch_upstream_blobs",
return_value={},
)
self.fetch_upstream = patcher.start()
self.addCleanup(patcher.stop)


class TrackedSensitiveFilesTest(_NoUpstreamMixin, unittest.TestCase):
def test_clean_repo_passes(self):
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
Expand Down Expand Up @@ -103,7 +138,93 @@ def test_git_failure_is_indeterminate(self):
self.assertEqual(result.status, Status.INDETERMINATE)


class NewlyHiddenTrackedFileTest(unittest.TestCase):
class UpstreamOwnershipTest(unittest.TestCase):
"""Ownership scoping for #1965.

Upstream ships genuinely private-key-shaped APNs test fixtures. This fork
operates the upstream product rather than developing it, cannot fix those
files, and was reporting FAIL on every branch for all six — so `audit` was
red on trunk and unreadable on every pull request. Scoping by ownership must
silence exactly that case and nothing else.
"""

def test_upstream_identical_fixture_does_not_fail_and_is_named(self):
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
_init_repo_with_files(
root, {"crates/gw/tests/fixtures/apns-test-identity.pem": "fixture bytes, deliberately not key-shaped\n"}
)
upstream = {"crates/gw/tests/fixtures/apns-test-identity.pem":
_oid(root, "crates/gw/tests/fixtures/apns-test-identity.pem")}
with mock.patch(
"security_audit_tracked_files_check.fetch_upstream_blobs",
return_value=upstream,
):
result = run(root)
self.assertEqual(result.status, Status.PASS)
# Named, not silently dropped: a skip nobody can see is a check that stopped looking.
self.assertIn("apns-test-identity.pem", result.detail)
self.assertIn("upstream", result.detail)

def test_a_cohort_added_key_still_fails_alongside_upstream_fixtures(self):
"""The acceptance criterion: narrowing must not blind the check.

An upstream-identical fixture and a cohort-added key in the same tree —
the second one must still fail the run.
"""
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
_init_repo_with_files(root, {
"crates/gw/tests/fixtures/apns-test-identity.pem": "fixture bytes, deliberately not key-shaped\n",
"launchpad/deploy/relay.pem": "fixture bytes, deliberately not key-shaped\n",
})
upstream = {"crates/gw/tests/fixtures/apns-test-identity.pem":
_oid(root, "crates/gw/tests/fixtures/apns-test-identity.pem")}
with mock.patch(
"security_audit_tracked_files_check.fetch_upstream_blobs",
return_value=upstream,
):
result = run(root)
self.assertEqual(result.status, Status.FAIL)
self.assertIn("relay.pem", result.detail)
# The upstream fixture must not be counted against the cohort.
self.assertNotIn("apns-test-identity.pem", result.detail)

def test_an_inherited_path_modified_here_still_fails(self):
"""Identity, not origin.

An inherited filename is exactly where cohort-added key material could
hide, so presence upstream is not enough — the bytes must match.
"""
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
_init_repo_with_files(
root, {"crates/gw/tests/fixtures/apns-test-identity.pem": "fixture bytes, deliberately not key-shaped (ours)\n"}
)
upstream = {"crates/gw/tests/fixtures/apns-test-identity.pem": "0" * 40}
with mock.patch(
"security_audit_tracked_files_check.fetch_upstream_blobs",
return_value=upstream,
):
result = run(root)
self.assertEqual(result.status, Status.FAIL)
self.assertIn("apns-test-identity.pem", result.detail)

def test_unreachable_upstream_is_indeterminate_not_pass(self):
"""Ownership unknown is neither clean nor damning, and must not read as clean."""
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
_init_repo_with_files(root, {"deploy/host.pem": "fixture bytes, deliberately not key-shaped\n"})
with mock.patch(
"security_audit_tracked_files_check.fetch_upstream_blobs",
return_value=None,
):
result = run(root)
self.assertEqual(result.status, Status.INDETERMINATE)
self.assertIn("host.pem", result.detail)


class NewlyHiddenTrackedFileTest(_NoUpstreamMixin, unittest.TestCase):
"""PR-mode only: a real two-commit history against a real 'origin' remote
(a second bare repo, not a mock) -- proves the diff-against-base-ref path
actually reads real git history, not a mocked stand-in for it.
Expand Down
Loading