Skip to content

fix(osv): preserve immutable direct-source provenance - #1158

Open
seonghobae wants to merge 54 commits into
mainfrom
fix/osv-direct-source-provenance-1096
Open

fix(osv): preserve immutable direct-source provenance#1158
seonghobae wants to merge 54 commits into
mainfrom
fix/osv-direct-source-provenance-1096

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Closes #1096.

Problem and fail-closed contract

The reusable Security Scan previously passed OSV's npm name/version result to the reporter without preserving direct-tarball URL and integrity evidence. Open-ended npm ranges could therefore attribute already-remediated advisories to an official immutable vendor release.

This repair:

  • retains genuine affected xlsx releases, including 0.18.x, 0.19.2, and 0.20.1;
  • reconciles only exact official immutable xlsx@0.20.3 provenance whose authoritative affected range excludes it;
  • retains unknown, malformed, ambiguous, or conflicting provenance as typed SCANNER_METADATA_CONFLICT evidence;
  • preserves advisory IDs and severities;
  • reads required provenance through descriptor-bound O_NOFOLLOW and regular-file validation;
  • parses only pnpm's authoritative packages map, not provenance-free snapshot duplicates;
  • binds every reconciliation decision to the exact OSV source.path for the governed root lockfile;
  • treats exit-1 plus complete vulnerability JSON as an authoritative completed scan, while missing, malformed, empty-on-failure, symlinked, or stale output remains non-passing;
  • preserves base scan evidence outside the head checkout trust boundary; and
  • binds trusted policy checkout to the governed workflow revision.

Exact identity and evidence

  • Exact head: c45a776f9ec3be8b35ee105e966100c80b95c2cc.
  • Exact base: protected main@fce028b4c3bf8e2e5e4819c1c5622e90cfa6ab39.
  • Merge-base equals the live base; ahead 51 and behind 0.
  • Live diff: nine paths: eight intended OSV provenance/workflow/evidence paths plus the compatible central quick-gate assertion update that matches current protected scheduler semantics.
  • The competing pip-audit lock remains removed; fix(security): repair pip audit and schedule orchestrator review #1198 is its canonical writer.
  • RED: five workflow-evidence regressions failed before the classifier existed; three source-binding regressions failed before exact source.path enforcement; OSV v2.3.8 rejected the stale --output flag before the exact --output-file repair.
  • GREEN: current OSV quality run 32516387088 checked out this exact head, passed 1,333 tests and 69 subtests, and measured the reconciler at 289 statements/98 branches, 100%.
  • The 1,333-test OSV and Strix results cited above belong to predecessor fca5cd1e3d14418b8a5fc5cd3636815582228457 and are now historical. Fresh current-head organization-readiness and exact-artifact SBOM quality runs are terminal success; OSV/Strix and security jobs are running or queued. Pending evidence is non-passing.
  • Unresolved review threads: zero.
  • Exact-head formal verdict: absent.

Consumer acceptance remains a fresh Security Scan on unchanged/descendant Inkspan #318 only after this repair reaches protected main. Queued security/SBOM work and predecessor checks or reviews remain non-passing.


Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d9ed951-eac4-4ccb-b80f-6d2c4a85ccb2

📥 Commits

Reviewing files that changed from the base of the PR and between 4c33442 and 4e1102b.

📒 Files selected for processing (8)
  • .github/workflows/osv-direct-source-quality-ci.yml
  • .github/workflows/security-scan.yml
  • CHANGELOG.md
  • docs/doctoring/osv-direct-source-provenance.md
  • scripts/ci/organization_commercial_readiness_loop.py
  • scripts/ci/osv_direct_source_reconcile.py
  • tests/test_osv_direct_source_reconcile.py
  • tests/test_required_workflow_queue_contract.py
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/osv-direct-source-provenance-1096

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review exact current head f2a94192cba86e661ed3c5fd22a8b6ffa44cd3f0 against protected base 2cce96f8671bfde7e354effef931c14a5928c23b. Verified 6 provenance tests, compileall, interrogate, actionlint, and diff --check. The reconciler retains findings unless exact direct-source identity, SHA-512 integrity, and authoritative GitHub-reviewed affected bounds all prove them disproven; the audit is append-only and the security workflow reconciles before the reporter verdict. Do not approve from queued or status-only evidence.

@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 05:03

Copy link
Copy Markdown
Contributor Author

@opencode-agent Keep this PR Draft and repair the current exact head 29af209ed2e0ecdf57e54a04d04969fb544061a6 test-first. Two fail-closed defects remain in the bounded provenance contract:

  1. reconcile_payload() selects candidates[0]. Two same-package/version direct-source records can therefore disagree while the first valid record suppresses the finding. Add a RED regression with conflicting duplicate xlsx records, then require exactly one unambiguous matching source; zero or multiple/conflicting candidates must retain the vulnerability and emit SCANNER_METADATA_CONFLICT.
  2. The production reconciler accepts every canonical SheetJS version and removes any finding whenever version >= exclusive upper bound. The declared contract is intentionally limited to the exact official immutable xlsx@0.20.3 exception. Add RED controls for 0.20.2/0.20.4 and other versions outside an advisory bound, then prevent those versions from being reconciled without a separately reviewed policy expansion.

Also remove the self-trust exception in security-scan.yml: the required gate must not execute scripts/ci/osv_direct_source_reconcile.py from the pull-request head merely because the target repository is .github. Bind runtime policy to the governed workflow/default-branch source, and use the dedicated quality workflow—not PR-controlled policy execution—to test candidate changes. Preserve advisory IDs/severities in append-only audit evidence, exact URL/tarball/integrity checks, affected-version positive controls, and existing 100% statement/branch/docstring gates. Reacquire all exact-head checks and formal review after the repair; do not mark Ready or enable merge before then.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Revalidated PR #1158 after repairing the current-head provenance contract.
Current head: c573b35

Root-cause fixes:

  • Only one direct-source record may match a package/version; zero or multiple matches retain every finding and emit SCANNER_METADATA_CONFLICT.
  • The reconciliation exception is restricted to immutable SheetJS xlsx@0.20.3. Other official versions remain findings and cannot be silently reconciled.
  • The central security policy checkout now always uses github.workflow_sha; the .github PR cannot execute its own head policy as trusted runtime policy.

Exact-head proof:

  • 1249 tests passed with 63 subtests.
  • scripts/ci/osv_direct_source_reconcile.py: 226 statements / 84 branches at 100% coverage.
  • interrogate 100%, ruff clean, actionlint passed for security-scan.yml and the provenance quality workflow, compileall and git diff --check passed.

Please rerun independent review and all protected Checks for this exact head. The PR remains subject to branch protection; no merge is claimed.

Copy link
Copy Markdown
Contributor Author

@opencode-agent review exact current head c573b35648fd424337ca2897095673a52c0be1d4 against independently resolved protected main@2cce96f8671bfde7e354effef931c14a5928c23b. This supersedes the predecessor-head request for f2a94192cba86e661ed3c5fd22a8b6ffa44cd3f0. Required acceptance is a substantive formal Reviews API verdict bound to this SHA. Preserve the fail-closed provenance contract: exact official direct-source identity/version/resolved URL/integrity and authoritative affected bounds may disprove a finding; affected or unverifiable provenance remains non-passing. Queued, status-only, and predecessor evidence are not acceptance.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Integrated current main normally at c3636cc. Focused provenance suite passed (15 tests, 47 subtests), actionlint, ruff, and diff checks passed. Re-run the exact-head security/OSV checks and review this current HEAD.

@OpenCode review this exact HEAD and report only current-head findings.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review this exact current HEAD. Use the current commit SHA, current Checks, and current diff; do not reuse prior approvals or prior-head evidence.

Copy link
Copy Markdown
Contributor Author

Inkspan downstream canary is ready for this existing owner lane; no consumer-side suppression/workaround has been introduced.

Fresh bindings before this note:

  • central protected base: .github/main@aa8503f4383e8328d89104796bc3e9f7da810376;
  • this PR is Ready/open/mergeable at exact head c3636cc2962a24e50e0f1b545f22fed824df26f8 against that base;
  • exact-head owner evidence currently includes OSV Direct Source Quality CI 32347266926 success, Security Scan 32347266784 success, OSV-Scanner PR 32347267550 success, SAST Semgrep 32347267061 success, Secret Scan 32347266807 success, Scorecard 32347266777 success, and SBOM Generation 32347266875 success; Python Security 32347266788 and CodeQL PR 32347266808 are still queued/non-passing, with no formal reviews or unresolved review threads yet. Do not promote predecessor evidence or merge while those live gates/then-current governance remain incomplete.
  • consumer protected base: ContextualWisdomLab/inkspan main@3b38ead2d00f44eb578d0689087b9293b3dabe1e;
  • consumer canary: Inkspan Draft fix(review): put contract-reliable reasoning models first, flagships last #318 exact unchanged head fce8fe72ebf4d438c17a7c889dd37c92118c6204;
  • current consumer Security Scan 32144783558 fails only at OSV job 95735816267 after exact base/head checkout and successful base/head scans; debug artifact 9328396543, digest sha256:594d8a8551017a78c98a33b6574bfbb9ce945f8b202988788ab96b224c3cfd3d, is bound to that run/head. CI 32144783586 and SAST 32144783644 are success.

After this exact owner repair integrates into protected .github/main under live governance, rerun required Security Scan on the then-unchanged Inkspan #318 head rather than transferring this PR's or predecessor consumer evidence. GREEN means the official integrity-bound direct xlsx@0.20.3 artifact no longer inherits the two npm-registry false positives while the npm-registry vulnerable controls, malformed/unknown provenance, and unrelated advisories remain fail-closed. If either owner or consumer head moves, refresh the binding before canary acceptance.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Security root-cause fix at current head e2c0031: malformed UTF-8 is now rejected at the shared text-input boundary with an explicit fail-closed ValueError, covering lockfile and JSON audit/results inputs. Added regression coverage; local 15 tests, Ruff, compileall, and diff checks passed. Please run the independent current-head review and required checks for this exact SHA.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent please review current HEAD e2c0031 for PR #1158; focus on malformed input handling, fail-closed provenance, and security regressions.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head repair pushed as eb7efe5 from fca5cd1.

The live branch had already incorporated the scheduler assertion correction; its remaining hashed audit lock was still pip 26.1.2. Updated only that lock to pip 26.2.1 with exact hashes.

Verification: 77 tests and 53 subtests for direct-source reconciliation and required workflow contracts passed; pip-audit reported no known vulnerabilities; git diff check passed. Hosted checks and exact-head independent approval must be re-evaluated for eb7efe5. No bypass or force push was used.

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head c45a776f9ec3be8b35ee105e966100c80b95c2cc against protected main@fce028b4c3bf8e2e5e4819c1c5622e90cfa6ab39. The pip lock was restored again to canonical owner #1198; the diff is nine paths and current exact OSV quality is terminal success. Verify immutable direct-source provenance, tamper-safe base/head outputs, current checks, and leave a formal verdict only for this SHA.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Please run the formal current-head review at exact SHA ; this central OSV provenance repair is required before dependent LineageWeave security Checks can be trusted.\n\nThe required merge gate remains protected; no self-approval or bypass is requested.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Please run the formal current-head review at exact SHA c45a776; this central OSV provenance repair is required before dependent LineageWeave security Checks can be trusted. The required merge gate remains protected; no self-approval or bypass is requested.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head integration note for c45a776: contextual-orchestrator #820 run 32514293613 confirmed scanner exit 0 followed by failure of the deprecated output-file contract. This PR is the normal central repair path for LineageWeave #383, contextual-orchestrator #818/#820, and GRC #51. Please preserve the exact-head and independent approval gates; rerun downstream security checks only after this repair is protected.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact evidence from LineageWeave #383 and contextual-orchestrator #820/#818 confirms the shared OSV failure was not a package finding: osv-scanner v2.3.8 returned exit 0, warned that --output is deprecated, and produced no output files, so the reusable workflow failed its file-presence gate. Current head c45a776f9ec3be8b35ee105e966100c80b95c2cc uses --output-file; dependent PRs need a fresh exact-head run after this protected PR receives independent approval and merges.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current head f61a879 has passed the direct-source provenance contract and exact-path policy; remaining checks are still running. Please perform an independent formal review of this shared-workflow change before protected merge, since the OSV correction gates downstream consumer checks #383 and #820.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Please review exact current head f61a879. Confirm the OSV --output-file migration preserves immutable direct-source provenance, fork checkout behavior, and fail-closed vulnerability reporting. Publish a current-head review only; do not modify, self-approve, or merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head maintainer audit

  • Current exact head: 4748ced
  • Current exact base: 0156282
  • Review repair: malformed but valid-JSON OSV container shapes now return the same explicit fail-closed error path as other scanner evidence errors; regression coverage was added.
  • Local exact-head verification: 1344 tests, 69 subtests, 100% statement/branch coverage across 8457/3260, Interrogate 100%, compileall, Bash syntax, actionlint for the affected workflows, and git diff check passed.
  • Hosted state after push: required security and review lanes are queued or in progress; no independent formal approval is present.
  • Decision: WAIT_AND_REMEDIATE. The normal protected path remains required; no bypass, direct protected push, or force-push was used.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Consumer verification: contextual-orchestrator#818 exact head 133729cfbab2ac74f3309cad8fbc7e20c2e33627 and LineageWeave#383 exact head 4eaa07172fde827f4ad89580326a0d2db5ceb0e4 both reproduced the inherited osv-scan failure. Logs show OSV scanner exit code 0 with no findings, followed by test -s old-results.json failing because only the new result file was produced. The current #1158 head 4748ced3e09c8c684d34b1b6f763c450f6d4e218 has all 49 exact-head checks terminal and passing; these consumer failures should clear once this protected central fix is approved and merged.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Review exact current head 4748ced3e09c8c684d34b1b6f763c450f6d4e218 for the OSV result-path repair. Confirm the scanner output-file contract, cross-fork workspace isolation, and current Checks before any protected merge.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +60 to +69
def valid_sha512_integrity(value: str) -> bool:
"""Return whether an integrity string contains one exact SHA-512 digest."""

if not value.startswith("sha512-"):
return False
try:
decoded = base64.b64decode(value.removeprefix("sha512-"), validate=True)
except (ValueError, binascii.Error):
return False
return len(decoded) == 64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: SHA-512 integrity gate is well-formedness only

valid_sha512_integrity (osv_direct_source_reconcile.py) only checks that the integrity string decodes to 64 bytes. It does not verify the digest against a real artifact. Reconciliation trusts the lockfile-declared URL/integrity shape and OSV's reported version rather than re-verifying the tarball.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 21:24
@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Review exact current head against base . Confirm coverage, security, data-loss, and workflow behavior from this head only. Do not self-approve, bypass, force-push, or merge.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Review exact current head 4748ced against base 0156282. Confirm coverage, security, data-loss, and workflow behavior from this head only. Do not self-approve, bypass, force-push, or merge.

@seonghobae
seonghobae enabled auto-merge (squash) August 21, 2026 21:29
@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Review exact current head 4e1102b against base 4c33442 after normal branch update. Verify the OSV direct-source output contract, workspace isolation, and conflict resolution from this head only. Do not self-approve, bypass, force-push, or merge.

@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 21:41
@seonghobae
seonghobae enabled auto-merge (squash) August 21, 2026 21:42
@seonghobae

Copy link
Copy Markdown
Contributor Author

@claude Review exact current head 4e1102b against base 4c33442. Re-check only this current diff and report actionable correctness, security, data-loss, and buyer-facing findings. Do not self-approve, bypass, force-push, or merge.

@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 21:55

seonghobae commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Current-head RCA: integrity must be identity-bound, not only well-formed

Fresh live-state resolution after central-main movement:

  • GitHub PR metadata still resolves the actual fix(osv): preserve immutable direct-source provenance #1158 head as 4e1102ba720ab5ef43296d17ca49b2a227fa58c0, base branch main, mergeable false.
  • Protected .github/main has advanced to signed/protected aba287b541237c688f9db516165c1e4331d0ca29 (parent da12d10130189a8b0c40fd6752b3b30da54dbc0e). compare(aba287b..., 4e1102b...) is diverged: head ahead 54 / behind 2, merge-base 4c33442021d63b09f35a874c5e7a779dd46ef8f2.
  • The PR body currently claims “Exact head c45a776f... / exact base fce028b4... / behind 0”. That prose is stale and must not be used as current-head evidence: compare(4e1102b..., c45a776f...) proves c45a776f... is five commits behind the live PR head.
  • The scripts/ci/osv_direct_source_reconcile.py:60-69 review thread remains unresolved and actionable.
  • Exact live-head 4e1102b... workflows: Organization Commercial Readiness, OpenCode Rust Coverage, OSV Direct Source Quality, and Exact Artifact SBOM Attestation are success; Secret Scan, Security Scan, Python Security, Strix, CodeQL PR, SAST, OSV-Scanner PR, SBOM Generation, and Scorecard PR are cancelled and therefore non-passing. No formal approving verdict exists.

First causal boundary / conflicting propositions. The PR contract says reconciliation is limited to an “exact official immutable” SheetJS release, but current valid_sha512_integrity() only proves that a consumer-controlled lockfile value decodes to one 64-byte SHA-512 digest. validate_sheetjs_source() then accepts that syntactically valid digest together with canonical URL/version shape; it does not bind the digest to independently trusted artifact identity. Because a consumer PR controls its head lockfile, the lockfile digest cannot itself be the trust root for suppressing scanner evidence.

Exact consumer reproduction. Inkspan #318 remains fce8fe72ebf4d438c17a7c889dd37c92118c6204. Its lock records xlsx@https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz with integrity sha512-oLDq3jw7AcLqKWH2AhCpVTZl8mf6X2YReP+Neh0SJUzV/BdZYjth94tG5toiMB1PPrYtxOCfaoUCkvtuH+3AJA==; Security run 32144783558, OSV job 95735816267, reports the two known xlsx@0.20.3 advisories. Inkspan CI 32144783586 and SAST 32144783644 pass, but Security is failure and remains non-passing.

Falsifiable RED acceptance. Keep canonical URL/version and replace integrity with any other valid base64-encoded 64-byte SHA-512 value. The reconciler must retain the advisories as SCANNER_METADATA_CONFLICT; it must not classify that record as official immutable provenance.

Smallest safe remedy. Bind the governed xlsx@0.20.3 exception to trusted, reviewed artifact identity (for example an exact allowlisted digest sourced from authoritative vendor release evidence) rather than network-time trust or a looser shape check. Known exact URL + version + trusted digest may reconcile; unknown, mismatched, malformed, or merely well-formed alternative digests remain fail-closed. Add the realistic mutant regression first, then require full OSV quality GREEN.

After the source repair, update/restack the existing owner branch onto then-current protected main without force-push/destructive history, regenerate exact-head required evidence, and correct the PR body to the independently resolved head/base. Only after that repair reaches protected central main should Inkspan #318 receive a fresh Security Scan on unchanged/descendant source. Do not resolve the integrity thread, merge, suppress locally, or transfer predecessor evidence before those conditions are met.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security(osv): preserve direct-tarball provenance instead of npm false positives

1 participant