close
Skip to content

ci: gate vendored _vendor tree behind a sha256 manifest (#492) - #2392

Merged
iamwhatever merged 1 commit into
mainfrom
fix/vendor-checksum-492
Aug 10, 2026
Merged

ci: gate vendored _vendor tree behind a sha256 manifest (#492)#2392
iamwhatever merged 1 commit into
mainfrom
fix/vendor-checksum-492

Conversation

@bolichen97

Copy link
Copy Markdown
Collaborator

Problem

src/kiro_crew/_vendor (vendored llama-cpp-python 0.3.34: ~616K of executable Python plus ~26MB of native libs, 46 files) is deliberately excluded from every source-level content review — .semgrepignore, the AI reviewers' reviewable-diff computation in code-review.yml/claude-review.yml, and the black/isort/flake8/mypy configs. The existing scripts/verify_vendored_payload.py only checks that built artifacts contain the declared native libs; nothing verifies the content of any vendored file.

Why it matters

A malicious or accidental modification to vendored Python source, a swapped native .so/.dylib/.dll, or a rogue .py added to the vendored sys.path root passes every current gate unnoticed. The vendored code runs in-process (ctypes-loaded native libs included), so this is a supply-chain-grade blind spot in an otherwise heavily gated repo.

Fix (symptoms → root cause → change)

The root cause is that the review exclusion covers the whole tree with nothing standing in for it. The change adds a content gate whose review surface lives outside the excluded tree:

  • scripts/verify_vendor_manifest.py (new, stdlib-only): walks _vendor deterministically, SHA-256 per file. Default --check compares against the committed manifest and exits non-zero naming every MODIFIED, MISSING, and UNEXPECTED (added) file — an added importable file is as dangerous as a modified one. --write regenerates the manifest (the documented path for legitimate vendored bumps). Symlinks anywhere in the tree are refused in both modes: a broken symlink is invisible to is_file() on the Linux runner but can resolve on another OS, and --write regenerating over one would bake the blind spot into the manifest. __pycache__ artifacts are excluded — they are machine-local import residue, and hashing them would make the documented --write procedure emit a manifest that fails on CI's clean checkout.
  • scripts/vendor_manifest.sha256 (new, generated): 46 sha256sum-compatible lines (<hex> src/kiro_crew/_vendor/<relpath>, sorted, trailing newline) — independently verifiable with sha256sum -c from the repo root. Deliberately outside _vendor/, so a manifest change is in the reviewable diff; scripts/** is also in ci.yml's meta paths-filter bucket, so a manifest-touching PR runs the full CI matrix.
  • .github/workflows/ci.yml: new always-on vendor-manifest job (modeled on scrub-lint: ubuntu-latest, pinned checkout SHA, workflow-level contents: read, 10-min timeout). Deliberately not gated behind the changes job — hashing 26MB takes seconds, and an always-on job cannot be dodged by a path-filter edge case.
  • src/kiro_crew/_vendor/README.md: "Updating the vendored tree" section — verify upstream sha256s against the existing source table, then --write + commit the manifest diff alongside the vendored changes.
  • docs/ci/ci-and-reviews.md: vendor-manifest row in the blocking-jobs table.

This is a separate concern from scripts/verify_vendored_payload.py (artifact completeness — untouched); this gate covers source-tree content integrity.

Tests

test/test_vendor_manifest.py (new, fixture-tree only — never hashes the real 26MB tree; that is the CI job's role):

  • --write--check round-trip green; byte-identical deterministic output in the exact sha256sum -c shape
  • Detection of each divergence class — modified file (swapped lib bytes), missing file, unexpected added file — and all classes named in one run
  • Symlink refused in --check even when broken; --write over a symlinked tree refuses without mutating the manifest
  • __pycache__ ignored by both modes and never baked into the manifest
  • Failure output names the documented --write regeneration path
  • Distinct exit-2 setup errors (absent manifest / absent tree / malformed manifest line fails loudly rather than silently narrowing the check)
  • Pins that the committed manifest exists and parses, and that ci.yml wires the gate

Manual verification

  • Bootstrapped the manifest against the clean checkout (main @ 8b502bc lineage), confirmed a green --check round-trip and sha256sum -c scripts/vendor_manifest.sha256 --quiet passing from the repo root.
  • Full local gates: isort / flake8 / mypy / black green; full backend suite run — all failures verified pre-existing host-environment issues (identical on a stashed pure-main run; most are in ci.yml's own BACKEND_DESELECTS).
  • No UI change — screenshots N/A.

Closes #492

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a — this comment is updated in place on each push.

Review details

This PR adds a CI checksum-gate script (scripts/verify_vendor_manifest.py), its manifest, docs, and tests.

Key observations for scope:

  • scripts/verify_vendor_manifest.py is not under src/kiro_crew/**, so the backend AUTOSDE rules (backend-security-controls, no-blocking-call-on-event-loop) don't file-pattern match it. It's a stdlib-only CI/dev tool that runs before any event loop.
  • test/test_vendor_manifest.py matches no-test-side-effects — but all writes/symlinks are confined to tmp_path; real-repo reads (ci.yml, committed manifest) are read-only. Clean.
  • Script logic (symlink/pycache refusal, modified/missing/unexpected diff, --write/--check mutual exclusion, malformed-line loud fail) has no reachable crash/corruption path; docstring __doc__.splitlines()[0] is safe (module docstring present).
  • No frontend files, so website/AUTOSDE.yaml is inert here.

No AUTOSDE violation and no residual-class defect on the changed lines survives falsification.

No findings.

[OPUS-REVIEWED] b10b5c6

Verdict parsed from the review's SHA-scoped output markers for commit b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a.

False positive or not applicable? A repository writer can comment:
/ai-review override fable b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a: <one-sentence reason>

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] b10b5c6

False positive or not applicable? A repository writer can comment:
/ai-review override gpt b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of b10b5c6f28ac29fa15ed0fc29da05e0d6f7cfd6a — updated in place on each push; does not block merge.

Design-Verdict: PASS

A real, well-scoped supply-chain blind spot closed with the minimal proportionate mechanism: a reviewable out-of-tree hash manifest plus an always-on, undodgeable CI job.

The design gets the hard parts right: the manifest lives outside the excluded tree so tampering becomes a visible diff; added files are a first-class failure like modified ones; symlinks and __pycache__ are refused in both modes so --write can't bake a bypass into the manifest; and the residual risk (a manifest regenerated by a malicious "bump" still needs a human to check content against upstream) is explicitly acknowledged and mitigated by the documented upstream-sha256 verification step — an enforced upstream comparison would be disproportionate given the tree is a curated multi-wheel subset. Fully reversible, no new public surface, and the exclusion claims in the problem statement check out against .semgrepignore and the CI config.

[DESIGN-REVIEWED] b10b5c6

@bolichen97
bolichen97 force-pushed the fix/vendor-checksum-492 branch from 0b338e1 to b10b5c6 Compare August 9, 2026 20:43
@bolichen97

Copy link
Copy Markdown
Collaborator Author

Disposition for GPT 5.6 findings on 0b338e129468f0538a26ab66bf8f5285022114c8:

  • BLOCKING — scripts/verify_vendor_manifest.py:82 — tracked bytecode bypasses integrity verification: FIXED in b10b5c6. The finding is legitimate: the previous walk skipped __pycache__ entries, so a force-added hash-based .pyc (PEP 552 unchecked variant) under _vendor/llama_cpp/__pycache__/ would execute on import in place of its source without validation, while every .py hash still matched the manifest. The walk now classifies __pycache__ files as a PYCACHE violation class refused in both --check and --write (same fail-closed treatment as symlinks; --write refuses without touching the manifest, so machine-local caches still can't poison a regeneration — the original reason for the exclusion). The refusal output includes a deletion hint for the legitimate locally-generated-cache case. Locked in by test_pycache_is_refused_never_silently_skipped, which asserts exit 1 + the PYCACHE: report in --check, and exit 1 + byte-identical manifest in --write.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 9, 2026
@iamwhatever
iamwhatever merged commit 584bbb0 into main Aug 10, 2026
52 checks passed
@iamwhatever
iamwhatever deleted the fix/vendor-checksum-492 branch August 10, 2026 04:55
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 10, 2026
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
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.

Add provenance/checksum verification for vendored _vendor source (not covered by content review)

2 participants