feat(supply-chain): verify locked hashes against PyPI releases - #1370
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR governance metadata gate is not ready for
|
|
Fixed current-head SSRF risk in |
|
Current head |
23a548b to
f6eeb69
Compare
Stale review: cited a coverage-evidence/required-check failure on an earlier commit; current head has been verified (gh pr checks) to pass coverage-evidence and all other non-metadata-gate required checks, with no current-head review from this reviewer. Dismissing as superseded per AGENTS.md stale-review guidance.
opencode-agent
Bot
added
area: ci-cd
* fix(http): reject explicit zero loopback ports * test(security): lock OIDC hostname boundary * test(security): preserve subdomain validation contracts * docs(security): record local HTTP port validation boundary * style: format local HTTP validation tests --------- Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
…ovenance attestation (#1370)
…egistry-hash verification Reconcile feat/python-lock-registry-provenance with the advanced base branch feat/dependency-lock-provenance-receipt (#1369 side): - scripts/ci/python_lock_provenance.py + tests: take base evolution (inline source-comment pin binding, non-file lock candidate skip) - .github/workflows/app-ci.yml: union keeping both offline provenance and new PyPI release hash provenance steps before dependency install
cd72417
into
feat/dependency-lock-provenance-receipt
Merge reconciliation: base-branch conflicts resolved, develop mergedNew head: Why DIRTY persistedThe PR head had already reconciled with parent #1369 at Conflicts resolved (merge commit
|
| File | Type | Side taken | Rationale |
|---|---|---|---|
scripts/ci/python_lock_provenance.py |
add/add | base | Strict superset evolution of the provenance attestation contract: inline source-comment pin binding (re.split(r"\s+#", ...)) and skip of non-file lock candidates in discovery. Nothing unique on the PR side was lost (diff ours→theirs = +3/−1). |
backend/tests/test_python_lock_provenance.py |
add/add | base | Adds exactly two tests for the above behaviors (inline-comment version binding; directory/broken-symlink discovery skip). Pure superset (+30). |
.github/workflows/app-ci.yml |
content | union | Keeps this PR's Validate PyPI release hash provenance step between the offline provenance step and dependency install; base added nothing in that region. Ordering preserved: offline lock validation → PyPI registry verification → install. |
Separately, origin/develop was merged first (4535698d, clean merge) so unrelated base-side changes (calendar-conflict API, NetworkGraph Map lookups, embedding chunking/local-http work, text_safety) are carried at develop parity — verified zero diff vs origin/develop on all those paths.
CodeRabbit warning disposition
The gate-cited coderabbitai comment (2026-08-16) reports zero unresolved comments and a skipped review bound to stale head 1a6ac604. There are no actionable code findings to address; the evidence is superseded by this push — current-head review/security evidence will re-materialize against a1f89ebb.
Verification (resolved tree, byte-identical 502263a1)
uv run pytest tests/test_python_lock_{provenance,provenance_includes,registry_non_vacuous,registry_provenance,registry_provenance_edges,registry_redirect_policy}.py tests/test_release_governance.py -q→ 86 passed- Develop-side merge-touched regressions (
test_local_http.py test_url_validation.py test_embedding.py test_batch_embedding_service.py test_email_import_service.py) → 126 passed python scripts/ci/python_lock_provenance.py --json(repo root) →status: passed, 5 locks, 0 violationspython scripts/ci/python_lock_registry_provenance.py --json(repo root, live PyPI) →status: passed, 5 locks, 0 violationsgit diff --checkclean; no conflict markers in tree
Mergeability now reports MERGEABLE; remaining states are fresh-head check/review wait states.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - name: Validate PyPI release hash provenance | ||
| run: | | ||
| status=0 | ||
| receipt="$(python scripts/ci/python_lock_registry_provenance.py --json)" || status=$? | ||
| printf '%s\n' "$receipt" | ||
| { | ||
| echo '### PyPI release hash provenance' | ||
| echo '```json' | ||
| printf '%s\n' "$receipt" | ||
| echo '```' | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
| exit "$status" |
There was a problem hiding this comment.
🔍 New CI gate makes backend job depend on live PyPI reachability for ~120 pinned packages with no retry
The new step (\.github/workflows/app-ci.yml:62-73) runs python_lock_registry_provenance.py, which discovers every requirements*.txt hash lock in the repo (backend/requirements-hashes.txt ~105 pins, backend/requirements-agent.txt, connector/requirements-hashes.txt, requirements-strix-ci-hashes.txt, requirements-bandit-ci-hashes.txt) and issues one live GET pypi.org/pypi/<name>/<ver>/json per unique (project,version). fetch_pypi_release (python_lock_registry_provenance.py) makes a single attempt with a 15s timeout and no retry; cached_fetch (python_lock_registry_provenance.py) caches failures so a single transient 5xx/timeout on any one of ~120 sequential requests permanently emits registry-metadata-fetch-failed and fails the whole backend job (no continue-on-error). The docs describe the gate as intentionally fail-closed, but conflating a transient network error with a provenance failure is a real CI-flakiness source that runs on every PR to develop/master and every push. Consider bounded retries/backoff for transport errors distinct from genuine provenance mismatches.
Was this helpful? React with 👍 or 👎 to provide feedback.
| f"trusted metadata identity does not match {project}", | ||
| ) | ||
| ) | ||
| if not isinstance(metadata_version, str) or metadata_version != version: |
There was a problem hiding this comment.
📝 Info: Exact string version comparison could false-fail on non-canonical version forms
_validate_requirement_metadata compares the locked version to PyPI info.version with strict string equality (metadata_version != version at python_lock_registry_provenance.py) rather than PEP 440 normalized comparison. This is fail-closed (a false negative, not a false pass), and in practice both uv-generated locks and PyPI info.version are canonical so they match. Flagging only because if any lock ever records a non-canonical version string (e.g. 1.0 vs a stored 1.0.0, or post/local forms), it would emit registry-version-mismatch and block CI. Low risk given uv canonical output.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for requirement in parsed_requirements: | ||
| project = str(requirement["project"]) | ||
| version = str(requirement["version"]) | ||
| raw_hashes = requirement["hashes"] | ||
| assert isinstance(raw_hashes, list) | ||
| locked_hashes = {str(value).lower() for value in raw_hashes} | ||
| try: | ||
| metadata = fetch_release(project, version) | ||
| except Exception: | ||
| requirement_receipts.append( | ||
| { | ||
| "project": project, | ||
| "version": version, | ||
| "status": "failed", | ||
| "matched_artifact_count": 0, | ||
| } | ||
| ) | ||
| violations.append( | ||
| _violation( | ||
| "registry-metadata-fetch-failed", | ||
| relative_path, | ||
| f"trusted PyPI metadata could not be resolved for {project}=={version}", | ||
| ) | ||
| ) | ||
| continue | ||
| requirement_receipt, metadata_violations = _validate_requirement_metadata( | ||
| project=project, | ||
| version=version, | ||
| locked_hashes=locked_hashes, | ||
| metadata=metadata, | ||
| relative_path=relative_path, | ||
| ) | ||
| requirement_receipts.append(requirement_receipt) | ||
| violations.extend(metadata_violations) |
There was a problem hiding this comment.
📝 Info: Requirement with no SHA-256 still triggers a redundant network fetch and second violation
When the lock parser records lock-requirement-has-no-sha256 for a pin with no attached hash, validate_lock_against_registry still proceeds to call fetch_release for that pin and compares an empty locked_hashes against PyPI, adding a second registry-hash-mismatch violation and an unnecessary network request (python_lock_registry_provenance.py). Behavior remains fail-closed and correct, so not a bug, but the extra fetch and duplicated violation code are mild inefficiency/noise for an already-invalid requirement.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def fetch_pypi_release( | ||
| project: str, | ||
| version: str, | ||
| *, | ||
| timeout_seconds: float = 15.0, | ||
| max_metadata_bytes: int = MAX_METADATA_BYTES, | ||
| ) -> Mapping[str, object]: | ||
| """Fetch one exact PyPI release document with a bounded credential-free GET.""" | ||
| if timeout_seconds <= 0: | ||
| raise ValueError("timeout_seconds must be positive") | ||
| if max_metadata_bytes <= 0: | ||
| raise ValueError("max_metadata_bytes must be positive") | ||
|
|
||
| release_url = build_pypi_release_url(project, version) | ||
| request = urllib.request.Request( | ||
| release_url, | ||
| headers={ | ||
| "Accept": "application/json", | ||
| "User-Agent": "naruon-lock-provenance/1", | ||
| }, | ||
| method="GET", | ||
| ) | ||
| with _open_pypi_request(request, timeout_seconds=timeout_seconds) as response: | ||
| final_url_getter = getattr(response, "geturl", None) | ||
| final_url = final_url_getter() if callable(final_url_getter) else release_url | ||
| if final_url != release_url: | ||
| raise ValueError("PyPI metadata response left the trusted PyPI origin") | ||
| content_type = response.headers.get("Content-Type", "") | ||
| if not content_type.lower().startswith("application/json"): | ||
| raise ValueError("PyPI release metadata must be JSON") | ||
| payload = response.read(max_metadata_bytes + 1) | ||
| if len(payload) > max_metadata_bytes: | ||
| raise ValueError("PyPI release metadata exceeds the configured byte limit") | ||
| decoded = json.loads(payload.decode("utf-8")) | ||
| if not isinstance(decoded, dict): | ||
| raise ValueError("PyPI release metadata must be a JSON object") | ||
| return decoded |
There was a problem hiding this comment.
🔍 Private/internal package pins (e.g. rankweave) would hard-fail the gate if not published on public PyPI
backend/requirements-hashes.txt:1264 pins rankweave==0.1.0 (described in the PR as an "OSMU spin-off"). The validator only accepts evidence from https://pypi.org/pypi/<project>/<version>/json (python_lock_registry_provenance.py); any package resolved from a private index, mirror, or not published to public PyPI will yield registry-metadata-fetch-failed and fail the gate closed. If rankweave (or any other pin such as ecosystem spin-offs) is not on public PyPI, merging this PR would immediately block all backend CI. Verify every discovered hash-lock pin across backend/connector/strix-ci/bandit-ci locks resolves on public PyPI before merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def build_pypi_release_url( | ||
| project: str, | ||
| version: str, | ||
| *, | ||
| pypi_origin: str = DEFAULT_PYPI_ORIGIN, | ||
| ) -> str: | ||
| """Build an exact PyPI release JSON URL from a credential-free HTTPS origin.""" | ||
| try: | ||
| parsed = urllib.parse.urlsplit(pypi_origin) | ||
| port = parsed.port | ||
| except ValueError as exc: | ||
| raise ValueError("pypi_origin must be the trusted PyPI origin") from exc | ||
| if ( | ||
| parsed.scheme != "https" | ||
| or (parsed.hostname or "").lower() != "pypi.org" | ||
| or parsed.username is not None | ||
| or parsed.password is not None | ||
| or port is not None | ||
| or parsed.path not in {"", "/"} | ||
| or parsed.query | ||
| or parsed.fragment | ||
| ): | ||
| raise ValueError("pypi_origin must be the trusted PyPI origin") | ||
|
|
||
| normalized_project = _normalized_name(project) | ||
| project_segment = urllib.parse.quote(normalized_project, safe="-._") | ||
| version_segment = urllib.parse.quote(version, safe="-._") | ||
| return f"{DEFAULT_PYPI_ORIGIN}/pypi/{project_segment}/{version_segment}/json" |
There was a problem hiding this comment.
📝 Info: build_pypi_release_url ignores the validated pypi_origin value when constructing the URL
build_pypi_release_url accepts a pypi_origin parameter and validates it strictly, but then constructs the final URL using the module constant DEFAULT_PYPI_ORIGIN rather than pypi_origin (python_lock_registry_provenance.py). This is currently harmless because validation guarantees pypi_origin can only be exactly https://pypi.org (no port/path/query/userinfo), so the constant and the argument are always identical. It is dead flexibility: the parameter can never point the client at a different origin, which is fine for the security intent but means the argument's apparent configurability is illusory.
Was this helpful? React with 👍 or 👎 to provide feedback.
| matched_count = 0 | ||
| if not violations: | ||
| registry_hashes = _eligible_registry_hashes(metadata) | ||
| if not registry_hashes: | ||
| violations.append( | ||
| _violation( | ||
| "registry-release-has-no-allowed-artifacts", | ||
| relative_path, | ||
| f"{project}=={version} has no eligible non-yanked wheel or sdist SHA-256", | ||
| ) | ||
| ) | ||
| else: | ||
| matched_count = len(locked_hashes & registry_hashes) | ||
| if matched_count == 0: | ||
| violations.append( | ||
| _violation( | ||
| "registry-hash-mismatch", | ||
| relative_path, | ||
| f"{project}=={version} lock hashes do not match eligible PyPI artifacts", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
📝 Info: Single-hash platform-specific connector lock still passes via intersection semantics
connector/requirements-hashes.txt records only one manylinux wheel hash for websockets==16.1 (generated with --only-binary=:all: --platform manylinux_2_28_x86_64). _eligible_registry_hashes returns all non-yanked wheel/sdist sha256s from the PyPI release and the gate requires only a non-empty intersection (python_lock_registry_provenance.py), so this single-hash lock passes correctly. Noting this to confirm the intersection (not subset/superset) semantics were considered and are compatible with platform-narrowed locks.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Stack dependency
This PR is stacked on #1369 (
feat/dependency-lock-provenance-receipt) and implements the next bounded issue #1229 supply-chain slice. Parent #1369 advanced after this branch was created, so the child was reconstructed non-destructively with a two-parent merge commit80454eecb04ae8455a52828f1e5efe6cd9577bcb. The exact live parent is nowbb8e34988af133bf28b0c1b657be7f757b66b2c8; compare evidence shows the child is 11 commits ahead, 0 behind, with merge base exactly that parent head.This PR may run current-head review/security evidence while stacked, but it must not merge into the feature branch. After #1369 reaches protected
develop, retarget this PR todevelop, recompute ancestry/live-base mergeability, and obtain fresh exact-head/base-sensitive checks and review before merge.Buyer-visible outcome
The parent slice proves repository-controlled lock declarations are internally consistent, including recursive requirements-file includes. This slice adds independent network-derived evidence that each exact locked project/version has at least one eligible non-yanked PyPI wheel or source distribution whose published SHA-256 digest is actually present in the lock. A stale-but-well-formed hash can therefore no longer look like valid provenance evidence.
TDD lineage
dcd0e3543f6027fc928305b7eb27e17bc7fa6a2dimported the not-yet-implemented registry validator and required CI ordering before dependency installation.f816166cb611f9ccf8787c4f0be92bf7a4b7246aadded the bounded PyPI release-hash validator.d9dfedf03f578bb3479cc476dfbdc678bca80a70placed registry verification after offline lock validation and before dependency installation.cfcf064ead52ea3040798a47f946d857cb390606records the exact evidence boundary and APA 7 references.fa6063b6a33944f6be89e3f1e85db00b8d66a59cadds fail-closed path, malformed metadata, bounded transport, cache-failure, and CLI tests.ecb492ce15aa9b86a9077f56e915b1f0ef05301b.80454eecb04ae8455a52828f1e5efe6cd9577bcbpreserves the child tree while incorporating feat(supply-chain): attest Python lock provenance before install #1369's three newer include-provenance commits as the second parent; no force update or rebase was used.The contract specifies exact release/hash match acceptance, stale-hash rejection, yanked/unsupported artifact exclusion, project/version identity binding, provider-error redaction, deduplicated release fetches, credential-free HTTPS
pypi.orgorigin validation, deterministic receipts, and CI publication before installation.Trust and scope boundary
This slice is deliberately PyPI-specific and consumes exact-version release metadata from the official PyPI JSON API. It does not claim generic private-index support. It records only project/version, match counts, stable reason codes, and path-relative repository evidence; artifact URLs and provider exception strings are not receipt data.
This slice proves membership of a recorded SHA-256 in an eligible exact PyPI release. It does not yet prove platform/Python tag compatibility, complete dependency closure, clean
pip install --require-hashesrehearsal, private-index parity, or PEP 740 attestation identity. Those remain follow-on #1229 work and must not be inferred from a passing receipt.Current exact candidate
bb8e34988af133bf28b0c1b657be7f757b66b2c8.80454eecb04ae8455a52828f1e5efe6cd9577bcb.Merge boundary
Ready-for-review is not merge-ready. While #1369 remains open, this head is dependency-blocked. After parent integration and retargeting, merge only if the unchanged exact head satisfies every live CI/security/coverage/docstring/dependency/package/provenance rule, zero valid review findings remain, and a qualifying independent non-author approval applies to the current/last-push head. Pending, skipped, stale, predecessor-head, author-only, model-only, or infrastructure-only evidence is non-passing.
Refs #1229.