[codex] Add supply chain rails menagerie exhibit#496
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 161 files, which is 11 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (161)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughProtocol bumped to v1.6.3 with catalog/spec CIDs and evolution artifacts; self-contracts/lift rules updated; CLI gains package inspect, admission, and version check; Supply Chain Rails runner, fixtures, tools, and walkthrough added; docs and pins updated. ChangesProtocol v1.6.3, CLI, and Rails
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4e2d59afa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+599
to
+603
| if let Some(policy_path) = &args.policy { | ||
| let policy = read_json_value(policy_path)?; | ||
| let pinned = policy | ||
| .get("policyCid") | ||
| .and_then(Value::as_str) |
There was a problem hiding this comment.
In verify_artifact_or_policy, passing --policy causes an early return based only on policyCid equality, so an invocation like provekit verify --artifact tampered.tgz --proof release.json --policy policy.json can be reported as accepted without ever comparing binaryCid. This creates a false-green admission path whenever callers provide all three flags. Please either make --policy and --artifact mutually exclusive at argument parsing time or run both checks when both are provided.
Useful? React with 👍 / 👎.
Comment on lines
+103
to
+104
| let previous_link_ok = candidate_previous == previous_contract_set_cid; | ||
| let ok = missing.is_empty() && previous_link_ok; |
There was a problem hiding this comment.
check_extension computes acceptance from contract-set inclusion plus previousContractSetCid linkage, but never verifies that previous and candidate describe the same package/ecosystem. As written, a receipt from a different package can be marked as a compatible extension if it reuses the same contract-set lineage, which weakens this command as a release-gate for package updates.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (11)
menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/slsa/vsa.key-1-3 (1)
1-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDemo-only Ed25519 private key checked into the repository.
This is a real PKCS#8 Ed25519 private key committed alongside the menagerie fixture (and the same pattern repeats for
safe-json-1.4.2-weakened/attestations/in-toto/functionary.key,safe-json-1.4.2-lie/attestations/slsa/vsa.key, andsafe-json-1.4.2-substituted/attestations/slsa/vsa.key). Even though the exhibit deliberately ships signing material so the walkthrough can regenerate receipts viarefresh-native-receipts.mjs, please:
- Add a sibling
README(or a header comment in the exhibit's top-level docs) explicitly labeling every checked-in*.keyundermenagerie/supply-chain-rails/authenticated-betrayal/packages/**/attestations/**as demo material — do not reuse for any real signing.- Confirm secret-scanning allowlists (GitHub push protection, gitleaks/trufflehog if used in CI) are configured to ignore this fixture path, otherwise every contributor PR will trip the scanner.
- Consider whether the refresh tooling can regenerate fresh keys on first run instead of committing them, so the only checked-in material is the public verifier input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/slsa/vsa.key` around lines 1 - 3, Remove hardcoded/demo private keys from the repository and add explicit README/header docs and CI exceptions: for each checked-in key file (e.g., vsa.key, functionary.key and other *.key files in the attestations directories) add a sibling README or a prominent header in the exhibit docs clearly labeling them as "DEMO — do not reuse for real signing"; update secret-scanning/allowlist rules in CI (GitHub push protection, gitleaks/trufflehog) to ignore these fixture paths so PRs won’t fail; and modify the refresh-native-receipts tooling so it can optionally generate fresh keypairs on first run (leaving only public verifier material committed) or document the manual key regeneration steps in the new README.menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/in-toto/layout.key-1-3 (1)
1-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd documentation and scanner allowlist for committed demo keys.
Both
layout.keyandfunctionary.keyare Ed25519 PKCS8 private keys committed as fixtures for the in-toto betrayal demo. While they are scoped strictly to the menagerie and public counterparts (.pubfiles) exist, the lack of inline documentation or scanner allowlist creates friction:
- Downstream consumers' secret scanners (gitleaks, trufflehoc, GitHub push protection) will flag them as real leaks and potentially block forks/mirrors.
- Future contributors may misunderstand their purpose without context.
Add a
README.mdor inline comment inmenagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/in-toto/explaining these are deterministic demo keys, and configure a repo-level scanner allowlist (e.g.,.gitleaksignoreor push-protection bypass in.github/secret_scanning.yml) to prevent downstream noise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/in-toto/layout.key` around lines 1 - 3, The committed Ed25519 PKCS8 demo private keys (layout.key and functionary.key) need explicit documentation and scanner allowlisting: add a README.md (or a short inline comment next to the files) in the attestations/in-toto directory stating these are deterministic demo fixtures with public counterparts and safe for the menagerie demo, and create/update repository scanner allowlists by adding entries to .gitleaksignore to ignore these filenames and/or adding a rule to .github/secret_scanning.yml to exempt these specific paths so downstream secret scanners (gitleaks, GitHub push protection) won’t block forks or flag noise.menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs-9-11 (1)
9-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winArgument parsing accepts extraneous arguments.
The check on line 9 only verifies that
--rpcappears somewhere in the argument list, but doesn't validate that it's the only argument or in the correct position. This allows invocations likesupply-chain-js-lowerer foo --rpc barto proceed.🔧 Proposed fix to enforce strict argument validation
- if !std::env::args().any(|arg| arg == "--rpc") { + let args: Vec<String> = std::env::args().collect(); + if args.len() != 2 || args[1] != "--rpc" { eprintln!("Usage: supply-chain-js-lowerer --rpc"); std::process::exit(1); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs` around lines 9 - 11, The current check using std::env::args().any(|arg| arg == "--rpc") allows extra arguments; replace it with a strict validation that the argument list equals exactly ["<prog>", "--rpc"] (i.e. collect args into a Vec and assert args.len() == 2 && args[1] == "--rpc") before proceeding, otherwise call eprintln! and std::process::exit(1); update the condition surrounding std::env::args(), the "--rpc" literal check, and the error/exit branch to enforce exact-position/length validation (or alternatively replace with a small parser like clap if preferred).menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs-160-160 (1)
160-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnhandled serialization error with
.unwrap().Line 160 uses
.unwrap()onserde_json::to_string(&evidence), which could panic if serialization fails. While unlikely for this simple structure, proper error handling would improve robustness.🛡️ Proposed fix to handle serialization error
- "evidenceCid": blake3_512(serde_json::to_string(&evidence).unwrap().as_bytes()), + "evidenceCid": blake3_512( + serde_json::to_string(&evidence) + .map_err(|e| format!("serialize evidence: {}", e))? + .as_bytes() + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs` at line 160, The current call uses serde_json::to_string(&evidence).unwrap() which can panic; replace the unwrap by handling the serialization error and propagating or converting it into the function's error type before computing the CID. Specifically, change the code that builds "evidenceCid" to call serde_json::to_string(&evidence)? (or .map_err(|e| <convert e to your error>) as appropriate), then pass the resulting String.as_bytes() into blake3_512; adjust the surrounding function (return type) to return Result if needed so errors from serde_json::to_string are properly propagated instead of panicking. Ensure you update any callers to handle the propagated error.docs/reference/per-language-status.md-131-131 (1)
131-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the new
package inspectsubcommand to the CLI list.Line 131 lists canonical subcommands but omits the new
package inspectsurface introduced by this PR, so the reference doc is already stale.📌 Suggested doc update
-**CLI:** `provekit` is the canonical Rust CLI for protocol v1.6.3. Subcommands include `prove`, `proof`, `protocol`, `ci`, `verify`, `verify-protocol`, `version`, `init`, `mint`, `lift`, `dump`, `hash`, `ask`, `search`, and `implicate`. Bug Zoo is repo-owned machinery under `menagerie/bug-zoo/`, not a public `provekit` subcommand. Distributed from source today with `cargo install --path implementations/rust/provekit-cli`; crates.io publishing remains future work. +**CLI:** `provekit` is the canonical Rust CLI for protocol v1.6.3. Subcommands include `prove`, `proof`, `protocol`, `ci`, `verify`, `verify-protocol`, `version`, `init`, `mint`, `lift`, `dump`, `hash`, `ask`, `search`, `implicate`, and `package inspect`. Bug Zoo is repo-owned machinery under `menagerie/bug-zoo/`, not a public `provekit` subcommand. Distributed from source today with `cargo install --path implementations/rust/provekit-cli`; crates.io publishing remains future work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/per-language-status.md` at line 131, Update the CLI subcommand list for `provekit` to include the new `package inspect` subcommand; locate the sentence that currently enumerates subcommands (the line containing "Subcommands include `prove`, `proof`, `protocol`, ...") and add `package inspect` to that comma-separated list so the documentation reflects the new surface introduced by this PR.menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py-39-43 (1)
39-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the product-name typo in the generated layout text.
ProvekItis misspelled here. Because the walkthroughs show raw receipts, this typo becomes user-visible in the exhibit output.✏️ Proposed fix
- "and ProvekIt contract source. This layout does not assert that " + "and Provekit contract source. This layout does not assert that "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py` around lines 39 - 43, The generated layout README string stored in the variable readme contains a product-name typo "ProvekIt"; update that literal in write-in-toto-layout.py so the README reads "ProveKit" (i.e., change "ProvekIt contract source" to "ProveKit contract source") to ensure the exhibit output shows the correct product name.docs/reference/cids.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix product name casing typo in the intro line.
Line 3 uses
ProvekIt; it should beProvekitfor consistency across docs.✏️ Suggested patch
-Every spec in ProvekIt is content-addressed by BLAKE3-512. This page lists the canonical CIDs at HEAD (protocol v1.6.3). Verify the local install conforms via `provekit verify-protocol`. +Every spec in Provekit is content-addressed by BLAKE3-512. This page lists the canonical CIDs at HEAD (protocol v1.6.3). Verify the local install conforms via `provekit verify-protocol`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/cids.md` at line 3, Replace the incorrect product name casing "ProvekIt" with the correct "Provekit" in the intro sentence of docs/reference/cids.md (the text containing "Every spec in ProvekIt is content-addressed..."); ensure the new spelling matches other docs and keep the rest of the sentence intact, including the reference to `provekit verify-protocol`.tools/foundation-keygen/src/bin/sign_catalog_v1_6_3.rs-43-43 (1)
43-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCorrect the CLI banner product spelling.
Line 43 prints
ProvekIt; useProvekitfor consistency with the rest of the toolchain output.✏️ Suggested patch
- println!("# ProvekIt v1.6.3 catalog attestation"); + println!("# Provekit v1.6.3 catalog attestation");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/foundation-keygen/src/bin/sign_catalog_v1_6_3.rs` at line 43, The CLI banner string printed by the sign_catalog_v1_6_3 binary is misspelled: replace the println! invocation that currently outputs "# ProvekIt v1.6.3 catalog attestation" with the correctly spelled "# Provekit v1.6.3 catalog attestation" so the banner in sign_catalog_v1_6_3.rs (the println! call) matches the rest of the toolchain.implementations/rust/provekit-cli/src/cmd_prove.rs-605-606 (1)
605-606:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast when proof
policyCidis missing in policy mode.Line 605–Line 606 silently coerces a missing proof
policyCidto"", which turns malformed input into a normal mismatch result. Treat this as an input error instead.🛠️ Suggested patch
- let candidate = proof.get("policyCid").and_then(Value::as_str).unwrap_or(""); + let candidate = proof + .get("policyCid") + .and_then(Value::as_str) + .ok_or_else(|| "proof receipt missing policyCid".to_string())?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@implementations/rust/provekit-cli/src/cmd_prove.rs` around lines 605 - 606, The code currently sets candidate = proof.get("policyCid").and_then(Value::as_str).unwrap_or("") and compares pinned == candidate, which masks a missing policyCid; instead, in policy mode detect a missing or non-string policyCid and fail fast (return an Err/exit) with a clear input error rather than treating it as a mismatch. Replace the unwrap_or("") usage with an explicit check of proof.get("policyCid").and_then(Value::as_str) and, if None, return an error from the surrounding function (or propagate via Result) referencing the variables/locations candidate, proof.get("policyCid"), and the pinned/ok comparison so callers can distinguish malformed input from a genuine mismatch.menagerie/README.md-25-25 (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the runnable command list below as well.
This row now marks Supply Chain Rails as runnable, but the quick-start block still only shows Bug Zoo and Bridgeworks. The README stays internally inconsistent until the new runnable entry gets a matching command example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/README.md` at line 25, The README table was updated to mark "Supply Chain Rails" as Runnable but the quick-start runnable command list wasn't updated; add a matching runnable command example for "Supply Chain Rails" in the quick-start block so the README is consistent. Locate the quick-start section and add an entry referencing "Supply Chain Rails" (same phrasing as in the table) with the runnable command or launch example used for other entries (mirror the format used for "Bug Zoo" and "Bridgeworks") and ensure the new command appears in the runnable commands list.menagerie/supply-chain-rails/README.md-142-149 (1)
142-149:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify that this
cargo runexample is repo-root-relative.
--manifest-path menagerie/supply-chain-rails/Cargo.tomlonly works when the reader runs it from the repository root, but this README sits inside that directory. Please either say that explicitly or switch the example to a path that works from here so the copy-pasted command does not fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/README.md` around lines 142 - 149, The README's cargo run example uses a repo-root-relative manifest path (--manifest-path menagerie/supply-chain-rails/Cargo.toml) which will fail when run from the current directory; either update the example to use the local manifest path (--manifest-path Cargo.toml) or add a short clarifying sentence stating that the shown command must be executed from the repository root, and keep the interactive walkthrough reference unchanged.
🧹 Nitpick comments (7)
tools/foundation-keygen/src/lib.rs (1)
101-104: ⚡ Quick winAdd
V1_6_3_DECLARED_ATto thedeclared_at_constants_are_pinned_iso8601guard.The pinned-format test at lines 719-730 still tops out at
V1_6_0_DECLARED_AT, so the newV1_6_3_DECLARED_AT(and the existingV1_6_1_DECLARED_AT/V1_6_2_DECLARED_AT) are not covered by the determinism guardrail. Adding it keeps the "no live clock" invariant enforced for every new pinned version.♻️ Proposed fix
let pinned = [ V1_1_0_DECLARED_AT, V1_2_0_DECLARED_AT, V1_3_0_DECLARED_AT, V1_3_1_DECLARED_AT, V1_4_0_DECLARED_AT, V1_4_1_DECLARED_AT, V1_5_0_DECLARED_AT, V1_6_0_DECLARED_AT, + V1_6_1_DECLARED_AT, + V1_6_2_DECLARED_AT, + V1_6_3_DECLARED_AT, SELF_CONTRACTS_DECLARED_AT_V1_3_1, SELF_CONTRACTS_DECLARED_AT_V1_6_0, ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/foundation-keygen/src/lib.rs` around lines 101 - 104, The test guard function declared_at_constants_are_pinned_iso8601 currently only checks up through V1_6_0_DECLARED_AT; update that test to include V1_6_1_DECLARED_AT, V1_6_2_DECLARED_AT and the new V1_6_3_DECLARED_AT so the determinism/no-live-clock invariant covers the newer pinned constants. Locate the assertions or list inside declared_at_constants_are_pinned_iso8601 and add those three symbol names to the expected set/sequence used for validation.menagerie/supply-chain-rails/authenticated-betrayal/tools/install-native-receipt-tools.sh (1)
16-20: 💤 Low valueConsider detecting the actual Go binary installation path.
The script assumes
slsa-verifieris installed to${HOME}/go/bin/, which is the default but can be customized viaGOBINorGOPATH. For improved portability, consider detecting the actual installation location:♻️ Optional improvement to detect actual Go paths
go install github.com/slsa-framework/slsa-verifier/v2/cli/slsa-verifier@v2.7.1 uv tool install in-toto -printf 'installed slsa-verifier: %s\n' "${HOME}/go/bin/slsa-verifier" +GOBIN="$(go env GOBIN)" +if [ -z "$GOBIN" ]; then + GOPATH="$(go env GOPATH)" + GOBIN="${GOPATH}/bin" +fi +printf 'installed slsa-verifier: %s\n' "${GOBIN}/slsa-verifier" printf 'installed in-toto tools under: %s\n' "$(uv tool dir --bin)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/tools/install-native-receipt-tools.sh` around lines 16 - 20, The script currently prints a hardcoded ${HOME}/go/bin/slsa-verifier; change it to detect the real install path by resolving the binary after installation (e.g., use command -v slsa-verifier or check GOBIN then ${GOPATH:-$HOME/go}/bin if command -v fails) and use that resolved path in the printf; update the printf that references "${HOME}/go/bin/slsa-verifier" to print the detected path variable instead so the message is correct regardless of GOBIN/GOPATH customization.menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-substituted/attestations/in-toto/layout.key (1)
1-3: ⚡ Quick winMark committed fixture keys so scanners treat them as test data, not live secrets.
This PEM blob looks intentional for the exhibit, but it is indistinguishable from an accidental secret to leak detectors. Add an explicit secret-scan allowlist/annotation for this fixture path so future scans do not keep failing on expected test material.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-substituted/attestations/in-toto/layout.key` around lines 1 - 3, The PEM blob between "-----BEGIN PRIVATE KEY-----" and "-----END PRIVATE KEY-----" in layout.key is a committed test fixture that scanners will flag as a secret; add an explicit secret-scan allowlist/annotation for this file so scanners treat it as test data (e.g., add a repository-specific ignore entry such as gitleaks/.gitlab-ci/etc. allowlist entry or an approved-fixtures config that references layout.key and the PEM marker), include a short justification/comment in the allowlist entry referencing the PEM marker, and commit the allowlist change so future scans skip this fixture.menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py (1)
6-9: ⚡ Quick winAvoid depending on
in_toto.models._signer.Line 6 imports from a private module, which is not part of in-toto's supported public API. A routine upgrade can break this code without any local change. Use
securesystemslibinstead:CryptoSigner.from_file()for PKCS#8 signers andSSlibKeyfor public key loading, which are the officially documented approach.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py` around lines 6 - 9, The code imports private symbols load_crypto_signer_from_pkcs8_file and load_public_key_from_file from in_toto.models._signer; replace these usages with the public securesystemslib APIs instead: use securesystemslib.signer.CryptoSigner.from_file (or CryptoSigner.from_pkcs8_file if available) to load PKCS#8 signers and use securesystemslib.keys.SSlibKey (or the documented SSlib key-loading helper) to load public keys, updating any callsites that reference load_crypto_signer_from_pkcs8_file and load_public_key_from_file to the new constructor/factory names and adjust arguments accordingly so the script uses the supported securesystemslib public API.menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/04-preserve-contracts-fail-witness.sh (1)
26-26: ⚡ Quick winEmpty
jqresult silently produces an emptyplanfile.If
lift.jsondoesn't contain a witness withattachTo == "runtime.no-env-secret-read"(e.g., the lifter changes its naming),jqexits 0 with empty output and$planbecomes a zero-byte file. The downstreamprovekit lower --plan "$plan"will then fail for a different reason than "preserved contract has no witness," weakening the walkthrough's pedagogical signal and confusing CI diagnosis. Consider asserting the plan is non-empty before proceeding.jq '.witnesses[] | select(.attachTo=="runtime.no-env-secret-read")' "$lift_json" > "$plan" +if [ ! -s "$plan" ]; then + say "Lift produced no witness for runtime.no-env-secret-read; cannot demonstrate witness rejection." + show_json_file "$lift_json" + exit 1 +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/04-preserve-contracts-fail-witness.sh` at line 26, The jq filter that writes to "$plan" can produce a zero-byte file if no witness matches, so after running jq '.witnesses[] | select(.attachTo=="runtime.no-env-secret-read")' "$lift_json" > "$plan" add a check that "$plan" is non-empty (or use jq -e to detect no-match) and fail early with a clear error message referencing the expected witness name; specifically, guard the output of the jq invocation that produces "$plan" and exit non‑zero with a message like "no witness found for attachTo=runtime.no-env-secret-read; aborting before provekit lower" so that provekit lower --plan "$plan" never runs on an empty plan.implementations/rust/provekit-cli/src/main.rs (1)
146-154: 💤 Low valueConsider validating
--artifact/--proof/--policycombinations.These three flags are accepted independently with no
requires/conflicts_withconstraints, but the admission semantics described in the supply-chain walkthrough (e.g.,provekit verify --artifact … --proof …) only make sense for specific pairings (e.g.,--artifactwithout--proofhas nothing to compare itsbinaryCidagainst;--policylikely only applies alongside--proof). Wiring this viarequires/requires_alllets clap reject malformed invocations up-front instead of pushing the validation deep intocmd_prove::run./// Artifact bytes to verify against a package release proof/receipt. - #[arg(long)] + #[arg(long, requires = "proof")] pub artifact: Option<PathBuf>, /// Package release proof/receipt naming the expected binaryCid. #[arg(long)] pub proof: Option<PathBuf>, /// Consumer policy proof/receipt used for policy admission checks. - #[arg(long)] + #[arg(long, requires = "proof")] pub policy: Option<PathBuf>,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@implementations/rust/provekit-cli/src/main.rs` around lines 146 - 154, The three CLI options artifact, proof, and policy can be misused independently; update the clap arg attributes so --artifact and --policy require --proof (e.g., add #[arg(long, requires = "proof")] to the artifact and policy fields) so clap rejects invocations like --artifact without --proof (leave proof optional so it can be used alone when appropriate). This uses the existing field names artifact, proof, and policy to wire the requires constraint rather than deferring validation to cmd_prove::run.menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/02-admit-baseline.sh (1)
18-22: ⚡ Quick winHarden the perl rewrite against unsafe
EXHIBIT_ROOTcharacters.
$EXHIBIT_ROOTis shell-interpolated directly into the perl source with#as thes///delimiter and no escaping. If the value ever contains#,$,@,\, or other perl metachars (think CI runners with cache paths,$HOME-style mounts, or someone running from a path containing#), the substitution silently corrupts the manifest or fails the script. Also note../../on the LHS is treated as a regex, so each.matches any character.Pass the value through the environment and quote the literal LHS with
\Q…\E:♻️ Suggested refactor
-perl -0pi -e "s#../../kit-rpc/run-supply-chain-npm-lifter.sh#$EXHIBIT_ROOT/kit-rpc/run-supply-chain-npm-lifter.sh#g" \ - "$work_pkg/.provekit/lift/supply-chain-npm/manifest.toml" -perl -0pi -e "s#../../kit-rpc/run-supply-chain-js-lowerer.sh#$EXHIBIT_ROOT/kit-rpc/run-supply-chain-js-lowerer.sh#g" \ - "$work_pkg/.provekit/lower/javascript/manifest.toml" \ - "$work_pkg/.provekit/lower/package-manifest/manifest.toml" +EXHIBIT_ROOT="$EXHIBIT_ROOT" perl -0pi -e \ + 's{\Q../../kit-rpc/run-supply-chain-npm-lifter.sh\E}{$ENV{EXHIBIT_ROOT}."/kit-rpc/run-supply-chain-npm-lifter.sh"}ge' \ + "$work_pkg/.provekit/lift/supply-chain-npm/manifest.toml" +EXHIBIT_ROOT="$EXHIBIT_ROOT" perl -0pi -e \ + 's{\Q../../kit-rpc/run-supply-chain-js-lowerer.sh\E}{$ENV{EXHIBIT_ROOT}."/kit-rpc/run-supply-chain-js-lowerer.sh"}ge' \ + "$work_pkg/.provekit/lower/javascript/manifest.toml" \ + "$work_pkg/.provekit/lower/package-manifest/manifest.toml"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/02-admit-baseline.sh` around lines 18 - 22, The perl one-liners interpolate $EXHIBIT_ROOT directly into the Perl program and treat the LHS as a regex, which breaks on metacharacters; change each perl invocation (the two perl -0pi -e lines that modify manifest.toml) to read EXHIBIT_ROOT from the environment inside Perl (ENV{'EXHIBIT_ROOT'}) and use \Q...\E around the literal left-hand path (e.g. ../../kit-rpc/run-supply-chain-npm-lifter.sh and ../../kit-rpc/run-supply-chain-js-lowerer.sh) so the LHS is quoted as a literal, and build the replacement using the safely quoted ENV value; keep the same output files (the manifest.toml targets) but stop shell-interpolating $EXHIBIT_ROOT into the Perl source.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/tutorials/java.md`:
- Line 3: Update all remaining documentation references of the protocol catalog
version string from "v1.6.2" to "v1.6.3" to match the change made in
docs/tutorials/java.md; specifically search for and replace the literal "v1.6.2"
occurrences mentioned in README.md, docs/how-to/ide-integration/overview.md,
docs/explanation/product.md, docs/explanation/architecture.md,
docs/explanation/landing.md, docs/papers/README.md,
docs/lsp/callsite-resolution-v1.md, docs/lsp/diagnostic-shape-v1.md, and
docs/lsp/forward-propagation-floor-v1.md while leaving intentional historical
references under protocol/evolution/v1.6.2/ and any conformance/spec artifacts
untouched.
In `@implementations/rust/provekit-cli/src/cmd_version.rs`:
- Around line 98-113: The code currently treats a missing candidate
"contractSetCid" as "" and can accept malformed receipts; compute a
candidate_contract_set_cid variable from
candidate.get("contractSetCid").and_then(Value::as_str) and ensure it is
Some/non-empty before accepting. Update the acceptance logic (the ok variable)
to include a check that candidate_contract_set_cid is present and not empty (in
addition to missing.is_empty() and previous_link_ok), and use that variable when
emitting "candidateContractSetCid" and determining "previousLinkOk".
In `@menagerie/manifest.yaml`:
- Around line 13-14: The manifest entry in menagerie/manifest.yaml sets
runnable: true but lacks the required command key; add a command field alongside
runnable to specify the launch invocation for this destination (matching the
style used by other runnable entries), e.g. set command: "<shell invocation or
binary path that starts this service>" so whatever orchestration reads
menagerie/manifest.yaml can execute it; ensure the command is a string value
under the same mapping as runnable: true.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/expected/release-1.4.1.json`:
- Line 9: The JSON field previousContractSetCid currently holds an empty string
(""), which is invalid for a CID; change it to null (or remove the key entirely)
so it matches contracts.json and strict validators — update the
previousContractSetCid value to null (or delete the previousContractSetCid
property) in the release-1.4.1.json payload to represent “no previous contract
set.”
In `@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/Cargo.toml`:
- Line 16: The Cargo.toml currently declares an empty [workspace] section which
makes this crate a workspace root and can break dependency resolution with the
parent workspace; remove the empty [workspace] stanza from this Cargo.toml so
the crate becomes a normal member of the repository workspace (or alternatively
if you intended this to be a standalone workspace, replace the empty section
with an explicit members = [...] list), updating Cargo.toml to either omit
[workspace] entirely or to define members explicitly.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/run-supply-chain-npm-lifter.sh`:
- Around line 11-13: The timestamp checks using BIN, SCRIPT_DIR and -nt are a
partial gating mechanism that can allow a stale supply-chain-npm-lifter binary
to be executed; remove the conditional that checks [ ! -x "$BIN" ] || ... and
instead always invoke cargo build --quiet --manifest-path
"$SCRIPT_DIR/Cargo.toml" --target-dir "$TARGET_DIR" --bin
supply-chain-npm-lifter before exec "$BIN" so Cargo performs correct freshness
checks; update the run-supply-chain-npm-lifter.sh snippet to call cargo build
unconditionally (retain BIN, SCRIPT_DIR, TARGET_DIR names) and then exec the
freshly-built "$BIN".
In
`@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs`:
- Around line 100-136: The evidenceCid is currently computed from just the
message in the refusal branch (blake3_512(message.as_bytes())), which is
inconsistent with the success branch that hashes the full serialized evidence;
change the refusal branch to build the same evidence Value used in the JSON
response (e.g., assign the json!({...}) object to a local variable like
evidence), use serde_json::to_string(&evidence).unwrap().as_bytes() when calling
blake3_512 to compute evidenceCid, and then include that evidence variable in
the response; apply the same pattern to the other path referenced (lines
138-174) so both success and refusal compute evidenceCid from the serialized
evidence object instead of the message string.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-npm-lifter.rs`:
- Around line 133-149: The current logic in the block that sets
tarball_path/package_path silently treats package.json as the artifact when
package.tgz is missing; update the logic in the function using tarball_path,
package_path, artifact_path, artifact_bytes, artifact_cid and artifact_report to
fail fast instead of falling back: if package.tgz does not exist return an error
(or call map_err to produce a clear failure) rather than reading and reporting
package.json as the admitted artifact; alternatively, if you must preserve both,
add an explicit separate manifest field (e.g., manifest_bytes/manifest_report)
and keep artifact_* for the binary tarball only so artifact_cid always
represents package.tgz.
- Around line 829-858: The current package_input_closure_cid builds the input by
concatenating rel + NUL + file_bytes + NUL which is ambiguous when file contents
contain 0x00; change package_input_closure_cid so each path and file blob is
length-prefixed before appending to bytes (or alternatively hash each (path,
bytes) pair and append the fixed-size hash) to make the encoding unambiguous;
specifically, inside the loop over rels in package_input_closure_cid, replace
the rel.as_bytes() + NUL + file_bytes + NUL appending with a deterministic
length-prefix scheme (e.g., append a fixed-size integer for rel.len(), then rel
bytes, then a fixed-size integer for file_bytes.len(), then file bytes) and then
call blake3_512(&bytes) as before.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-lie/attestations/in-toto/functionary.key`:
- Around line 1-3: The committed PEM in functionary.key contains a live private
key; remove the private key content from the fixture and keep only the public
verification material (or replace functionary.key with a public-key file), and
update the fixture/attestation generation code that reads functionary.key to
instead generate or import the private key at runtime (e.g., generate an
ephemeral keypair in the test/fixture setup or load the private key from a
secure env/secret store) so signing uses the runtime private key while the repo
only contains the public verifier; ensure any attestation signing code that
referenced functionary.key now accepts the runtime-provided private key and stop
committing private keys to source control.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-weakened/attestations/slsa/vsa.key`:
- Around line 1-3: Delete the private key file vsa.key from the repo and from
version history: remove the file from the working tree and commits (e.g., git rm
--cached vsa.key and commit, then purge historical occurrences with git
filter-repo or BFG), keep only the signed attestation (vsa.jsonl) and any public
verification material, and add vsa.key to .gitignore to prevent re-commit;
ensure any build/CI scripts referencing vsa.key are updated to use external
secret storage or the public key instead.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/tools/refresh-native-receipts.mjs`:
- Around line 182-184: The code builds a non-existent sibling interpreter path
(inTotoPython) next to inTotoRun causing failures; change run invocation to
either call the write-in-toto-layout.py script directly (run(join(toolsDir,
"write-in-toto-layout.py"), [...])) or resolve a PATH python executable instead
(use commandPath("python3", ["python"]) to set inTotoPython) and then call
run(inTotoPython, [...]); update references to inTotoRun/inTotoPython and ensure
toolsDir/write-in-toto-layout.py are used as described.
In `@menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/common.sh`:
- Around line 152-167: The rebuild heuristic in build_cargo_binary_if_needed
misses transitive workspace deps so changes in shared crates under
implementations/rust won't trigger rebuilds; update the calls to
build_cargo_binary_if_needed (or change that function) so it either always runs
cargo build for the workspace binaries (provekit and
provekit-supply-chain-rails) or expands the watched paths to include the shared
workspace crates (e.g. add "$REPO_ROOT/implementations/rust/src" and any other
dependent crate directories) when checking PROVEKIT_BIN and RUNNER_BIN to ensure
changes in transitive crates force a rebuild.
In `@menagerie/supply-chain-rails/src/lib.rs`:
- Around line 118-135: The current branch in run() that handles args.all
collects matching specimen directories into out but silently returns Ok(out)
even when out.is_empty(), masking a bad invocation; after building and sorting
out in the args.all block (where specimen.clone()/PathBuf and out Vec are used),
check if out.is_empty() and if so return an Err with a user-facing error (e.g.,
formatted String like "no specimens found in {root}" or similar) so the function
fails fast instead of returning success when no specimen.yaml targets were
discovered.
- Around line 85-106: The current branch that checks args.out.json /
args.out.quiet suppresses error output when quiet is true; change the logic in
the reporting block (the code that checks args.out.json, args.out.quiet and
iterates over reports, setup_errors, verify_errors) so that printing the PASS
lines for each report remains gated by args.out.quiet but the eprintln! loop for
setup_errors and verify_errors runs unconditionally; keep the JSON path
(serde_json::to_string_pretty on reports, setup_errors, verify_errors) unchanged
and only alter the non-JSON branch to print PASS lines conditionally while
always emitting errors from setup_errors.iter().chain(verify_errors.iter()).
---
Minor comments:
In `@docs/reference/cids.md`:
- Line 3: Replace the incorrect product name casing "ProvekIt" with the correct
"Provekit" in the intro sentence of docs/reference/cids.md (the text containing
"Every spec in ProvekIt is content-addressed..."); ensure the new spelling
matches other docs and keep the rest of the sentence intact, including the
reference to `provekit verify-protocol`.
In `@docs/reference/per-language-status.md`:
- Line 131: Update the CLI subcommand list for `provekit` to include the new
`package inspect` subcommand; locate the sentence that currently enumerates
subcommands (the line containing "Subcommands include `prove`, `proof`,
`protocol`, ...") and add `package inspect` to that comma-separated list so the
documentation reflects the new surface introduced by this PR.
In `@implementations/rust/provekit-cli/src/cmd_prove.rs`:
- Around line 605-606: The code currently sets candidate =
proof.get("policyCid").and_then(Value::as_str).unwrap_or("") and compares pinned
== candidate, which masks a missing policyCid; instead, in policy mode detect a
missing or non-string policyCid and fail fast (return an Err/exit) with a clear
input error rather than treating it as a mismatch. Replace the unwrap_or("")
usage with an explicit check of proof.get("policyCid").and_then(Value::as_str)
and, if None, return an error from the surrounding function (or propagate via
Result) referencing the variables/locations candidate, proof.get("policyCid"),
and the pinned/ok comparison so callers can distinguish malformed input from a
genuine mismatch.
In `@menagerie/README.md`:
- Line 25: The README table was updated to mark "Supply Chain Rails" as Runnable
but the quick-start runnable command list wasn't updated; add a matching
runnable command example for "Supply Chain Rails" in the quick-start block so
the README is consistent. Locate the quick-start section and add an entry
referencing "Supply Chain Rails" (same phrasing as in the table) with the
runnable command or launch example used for other entries (mirror the format
used for "Bug Zoo" and "Bridgeworks") and ensure the new command appears in the
runnable commands list.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/supply-chain-js-lowerer.rs`:
- Around line 9-11: The current check using std::env::args().any(|arg| arg ==
"--rpc") allows extra arguments; replace it with a strict validation that the
argument list equals exactly ["<prog>", "--rpc"] (i.e. collect args into a Vec
and assert args.len() == 2 && args[1] == "--rpc") before proceeding, otherwise
call eprintln! and std::process::exit(1); update the condition surrounding
std::env::args(), the "--rpc" literal check, and the error/exit branch to
enforce exact-position/length validation (or alternatively replace with a small
parser like clap if preferred).
- Line 160: The current call uses serde_json::to_string(&evidence).unwrap()
which can panic; replace the unwrap by handling the serialization error and
propagating or converting it into the function's error type before computing the
CID. Specifically, change the code that builds "evidenceCid" to call
serde_json::to_string(&evidence)? (or .map_err(|e| <convert e to your error>) as
appropriate), then pass the resulting String.as_bytes() into blake3_512; adjust
the surrounding function (return type) to return Result if needed so errors from
serde_json::to_string are properly propagated instead of panicking. Ensure you
update any callers to handle the propagated error.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/in-toto/layout.key`:
- Around line 1-3: The committed Ed25519 PKCS8 demo private keys (layout.key and
functionary.key) need explicit documentation and scanner allowlisting: add a
README.md (or a short inline comment next to the files) in the
attestations/in-toto directory stating these are deterministic demo fixtures
with public counterparts and safe for the menagerie demo, and create/update
repository scanner allowlists by adding entries to .gitleaksignore to ignore
these filenames and/or adding a rule to .github/secret_scanning.yml to exempt
these specific paths so downstream secret scanners (gitleaks, GitHub push
protection) won’t block forks or flag noise.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.1/attestations/slsa/vsa.key`:
- Around line 1-3: Remove hardcoded/demo private keys from the repository and
add explicit README/header docs and CI exceptions: for each checked-in key file
(e.g., vsa.key, functionary.key and other *.key files in the attestations
directories) add a sibling README or a prominent header in the exhibit docs
clearly labeling them as "DEMO — do not reuse for real signing"; update
secret-scanning/allowlist rules in CI (GitHub push protection,
gitleaks/trufflehog) to ignore these fixture paths so PRs won’t fail; and modify
the refresh-native-receipts tooling so it can optionally generate fresh keypairs
on first run (leaving only public verifier material committed) or document the
manual key regeneration steps in the new README.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py`:
- Around line 39-43: The generated layout README string stored in the variable
readme contains a product-name typo "ProvekIt"; update that literal in
write-in-toto-layout.py so the README reads "ProveKit" (i.e., change "ProvekIt
contract source" to "ProveKit contract source") to ensure the exhibit output
shows the correct product name.
In `@menagerie/supply-chain-rails/README.md`:
- Around line 142-149: The README's cargo run example uses a repo-root-relative
manifest path (--manifest-path menagerie/supply-chain-rails/Cargo.toml) which
will fail when run from the current directory; either update the example to use
the local manifest path (--manifest-path Cargo.toml) or add a short clarifying
sentence stating that the shown command must be executed from the repository
root, and keep the interactive walkthrough reference unchanged.
In `@tools/foundation-keygen/src/bin/sign_catalog_v1_6_3.rs`:
- Line 43: The CLI banner string printed by the sign_catalog_v1_6_3 binary is
misspelled: replace the println! invocation that currently outputs "# ProvekIt
v1.6.3 catalog attestation" with the correctly spelled "# Provekit v1.6.3
catalog attestation" so the banner in sign_catalog_v1_6_3.rs (the println! call)
matches the rest of the toolchain.
---
Nitpick comments:
In `@implementations/rust/provekit-cli/src/main.rs`:
- Around line 146-154: The three CLI options artifact, proof, and policy can be
misused independently; update the clap arg attributes so --artifact and --policy
require --proof (e.g., add #[arg(long, requires = "proof")] to the artifact and
policy fields) so clap rejects invocations like --artifact without --proof
(leave proof optional so it can be used alone when appropriate). This uses the
existing field names artifact, proof, and policy to wire the requires constraint
rather than deferring validation to cmd_prove::run.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-substituted/attestations/in-toto/layout.key`:
- Around line 1-3: The PEM blob between "-----BEGIN PRIVATE KEY-----" and
"-----END PRIVATE KEY-----" in layout.key is a committed test fixture that
scanners will flag as a secret; add an explicit secret-scan allowlist/annotation
for this file so scanners treat it as test data (e.g., add a repository-specific
ignore entry such as gitleaks/.gitlab-ci/etc. allowlist entry or an
approved-fixtures config that references layout.key and the PEM marker), include
a short justification/comment in the allowlist entry referencing the PEM marker,
and commit the allowlist change so future scans skip this fixture.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/tools/install-native-receipt-tools.sh`:
- Around line 16-20: The script currently prints a hardcoded
${HOME}/go/bin/slsa-verifier; change it to detect the real install path by
resolving the binary after installation (e.g., use command -v slsa-verifier or
check GOBIN then ${GOPATH:-$HOME/go}/bin if command -v fails) and use that
resolved path in the printf; update the printf that references
"${HOME}/go/bin/slsa-verifier" to print the detected path variable instead so
the message is correct regardless of GOBIN/GOPATH customization.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/tools/write-in-toto-layout.py`:
- Around line 6-9: The code imports private symbols
load_crypto_signer_from_pkcs8_file and load_public_key_from_file from
in_toto.models._signer; replace these usages with the public securesystemslib
APIs instead: use securesystemslib.signer.CryptoSigner.from_file (or
CryptoSigner.from_pkcs8_file if available) to load PKCS#8 signers and use
securesystemslib.keys.SSlibKey (or the documented SSlib key-loading helper) to
load public keys, updating any callsites that reference
load_crypto_signer_from_pkcs8_file and load_public_key_from_file to the new
constructor/factory names and adjust arguments accordingly so the script uses
the supported securesystemslib public API.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/02-admit-baseline.sh`:
- Around line 18-22: The perl one-liners interpolate $EXHIBIT_ROOT directly into
the Perl program and treat the LHS as a regex, which breaks on metacharacters;
change each perl invocation (the two perl -0pi -e lines that modify
manifest.toml) to read EXHIBIT_ROOT from the environment inside Perl
(ENV{'EXHIBIT_ROOT'}) and use \Q...\E around the literal left-hand path (e.g.
../../kit-rpc/run-supply-chain-npm-lifter.sh and
../../kit-rpc/run-supply-chain-js-lowerer.sh) so the LHS is quoted as a literal,
and build the replacement using the safely quoted ENV value; keep the same
output files (the manifest.toml targets) but stop shell-interpolating
$EXHIBIT_ROOT into the Perl source.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/04-preserve-contracts-fail-witness.sh`:
- Line 26: The jq filter that writes to "$plan" can produce a zero-byte file if
no witness matches, so after running jq '.witnesses[] |
select(.attachTo=="runtime.no-env-secret-read")' "$lift_json" > "$plan" add a
check that "$plan" is non-empty (or use jq -e to detect no-match) and fail early
with a clear error message referencing the expected witness name; specifically,
guard the output of the jq invocation that produces "$plan" and exit non‑zero
with a message like "no witness found for attachTo=runtime.no-env-secret-read;
aborting before provekit lower" so that provekit lower --plan "$plan" never runs
on an empty plan.
In `@tools/foundation-keygen/src/lib.rs`:
- Around line 101-104: The test guard function
declared_at_constants_are_pinned_iso8601 currently only checks up through
V1_6_0_DECLARED_AT; update that test to include V1_6_1_DECLARED_AT,
V1_6_2_DECLARED_AT and the new V1_6_3_DECLARED_AT so the
determinism/no-live-clock invariant covers the newer pinned constants. Locate
the assertions or list inside declared_at_constants_are_pinned_iso8601 and add
those three symbol names to the expected set/sequence used for validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Tutorial: Java / JVM | ||
|
|
||
| > **Status:** kit shipping in the current v1.6.2 tree. Lift adapters shipping: Bean Validation, JUnit Jupiter, JML, Spring Web, Cofoja, plus bindings for Spring Security, Swagger, Jackson, JPA, Hibernate. Java realizer work is active in Bug Zoo. Embedded verifier and LSP plugin planned. Verification via the Rust CLI. | ||
| > **Status:** kit shipping in the current v1.6.3 tree. Lift adapters shipping: Bean Validation, JUnit Jupiter, JML, Spring Web, Cofoja, plus bindings for Spring Security, Swagger, Jackson, JPA, Hibernate. Java realizer work is active in Bug Zoo. Embedded verifier and LSP plugin planned. Verification via the Rust CLI. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for remaining v1.6.2 references that may need updating
# Search for v1.6.2 in documentation and configuration files
rg -n 'v1\.6\.2|1\.6\.2' --type=md --type=toml --type=json --type=yaml -C2
# Also check Java POM files
fd -e xml -e pom | xargs rg -n '1\.6\.2' -C2Repository: TSavo/provekit
Length of output: 45939
Update remaining documentation references from v1.6.2 to v1.6.3 for consistency.
The version bump in docs/tutorials/java.md line 3 aligns with the PR's protocol catalog v1.6.3 update. However, other documentation files still reference v1.6.2 as the "current" or "shipping" version and should be updated to v1.6.3 to maintain consistency:
README.md(lines 115-116): Protocol catalog status linedocs/how-to/ide-integration/overview.md(lines 7-11): IDE integration status tabledocs/explanation/product.md(lines 13, 23, 126): Explanation of product and CID referencesdocs/explanation/architecture.md(line 3): Architecture document introdocs/explanation/landing.md(line 39): Landing page protocol shorthanddocs/papers/README.md(line 13): Bluepaper reference to current catalogdocs/lsp/callsite-resolution-v1.md(line 3): LSP baseline model descriptiondocs/lsp/diagnostic-shape-v1.md(lines 33, 37): Diagnostic CID modeldocs/lsp/forward-propagation-floor-v1.md(line 35, 58, 66): Forward propagation baseline
(Retain intentional v1.6.2 references in protocol/evolution/v1.6.2/, protocol specs, conformance artifacts, and historical CID documentation.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/tutorials/java.md` at line 3, Update all remaining documentation
references of the protocol catalog version string from "v1.6.2" to "v1.6.3" to
match the change made in docs/tutorials/java.md; specifically search for and
replace the literal "v1.6.2" occurrences mentioned in README.md,
docs/how-to/ide-integration/overview.md, docs/explanation/product.md,
docs/explanation/architecture.md, docs/explanation/landing.md,
docs/papers/README.md, docs/lsp/callsite-resolution-v1.md,
docs/lsp/diagnostic-shape-v1.md, and docs/lsp/forward-propagation-floor-v1.md
while leaving intentional historical references under protocol/evolution/v1.6.2/
and any conformance/spec artifacts untouched.
| sha2 = "0.10" | ||
| tar = "0.4" | ||
|
|
||
| [workspace] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Empty [workspace] section may cause dependency resolution issues.
Line 16 declares an empty [workspace] section, which makes this crate its own workspace root. This is unusual given the path dependencies to other ProvekIt crates (lines 10-11). Typically, you would either:
- Omit the
[workspace]section if this is a workspace member, or - Remove this section and rely on the parent workspace at the repository root
An empty workspace can cause dependency resolution conflicts and prevents sharing of Cargo.lock with the parent workspace.
♻️ Proposed fix
-[workspace]
-
[[bin]]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/Cargo.toml` at
line 16, The Cargo.toml currently declares an empty [workspace] section which
makes this crate a workspace root and can break dependency resolution with the
parent workspace; remove the empty [workspace] stanza from this Cargo.toml so
the crate becomes a normal member of the repository workspace (or alternatively
if you intended this to be a standalone workspace, replace the empty section
with an explicit members = [...] list), updating Cargo.toml to either omit
[workspace] entirely or to define members explicitly.
Comment on lines
+1
to
+3
| -----BEGIN PRIVATE KEY----- | ||
| MC4CAQAwBQYDK2VwBCIEIFKT8jmdFrku2kkdiBkuCTx9vfmjamoqiuz/DdNom5Mv | ||
| -----END PRIVATE KEY----- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether this private key is actually consumed anywhere in the repo.
set -euo pipefail
fd -a 'vsa\.key$'
echo
echo '--- exact path references ---'
rg -n -C2 --fixed-strings \
'menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-weakened/attestations/slsa/vsa.key'
echo
echo '--- generic vsa.key references ---'
rg -n -C2 --fixed-strings 'vsa.key'
echo
echo '--- nearby fixture references ---'
rg -n -C2 --fixed-strings 'safe-json-1.4.2-weakened/attestations/slsa'Repository: TSavo/provekit
Length of output: 602
Remove the uncommitted private signing key.
The vsa.key file is a usable Ed25519 private key that should never be committed to version control. This file poses a security risk: anyone with repository access (including in historical versions) can mint valid replacement VSAs for this package. Because no code in the repository references this key file, it is safe to delete immediately. Keep only the signed attestation (vsa.jsonl) and any public verification material in-tree.
🧰 Tools
🪛 Betterleaks (1.1.2)
[high] 1-3: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@menagerie/supply-chain-rails/authenticated-betrayal/packages/safe-json-1.4.2-weakened/attestations/slsa/vsa.key`
around lines 1 - 3, Delete the private key file vsa.key from the repo and from
version history: remove the file from the working tree and commits (e.g., git rm
--cached vsa.key and commit, then purge historical occurrences with git
filter-repo or BFG), keep only the signed attestation (vsa.jsonl) and any public
verification material, and add vsa.key to .gitignore to prevent re-commit;
ensure any build/CI scripts referencing vsa.key are updated to use external
secret storage or the public key instead.
TSavo
force-pushed
the
codex/supply-chain-rails-spec
branch
2 times, most recently
from
00442e1 to
155d6c8
Compare
TSavo
force-pushed
the
codex/supply-chain-rails-spec
branch
from
155d6c8 to
ff74868
Compare
This was referenced May 9, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the Supply Chain Rails menagerie exhibit around an npm authenticated-betrayal scenario: conventional supply-chain receipts stay green while ProvekIt rejects preserved-contract betrayal, weakened contract sets, binary substitution, and stale CI input closure.
This also adds the
provekit package inspectsurface, supply-chain npm lifter/lowerer wiring, real npm pack fixture artifacts, real SLSA VSA and in-toto receipt fixtures, protocol catalog v1.6.3 dogfood material, and walkthrough scripts that show raw line-numbered receipts with human analysis.Validation
./menagerie/supply-chain-rails/authenticated-betrayal/tools/install-native-receipt-tools.shCARGO_TARGET_DIR=/tmp/provekit-supply-chain-kit-rpc-target cargo test --manifest-path menagerie/supply-chain-rails/authenticated-betrayal/kit-rpc/Cargo.toml -- --nocapturecargo test --manifest-path implementations/rust/provekit-cli/Cargo.toml --test supply_chain_rails_cli -- --nocapturecargo test --manifest-path menagerie/supply-chain-rails/Cargo.toml -- --nocapturePROVEKIT_SUPPLY_CHAIN_WALKTHROUGH_NO_PAUSE=1 PROVEKIT_SUPPLY_CHAIN_WALKTHROUGH_TARGET_DIR=/tmp/provekit-supply-chain-walkthrough-pr ./menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/03-show-conventional-green.shPROVEKIT_SUPPLY_CHAIN_WALKTHROUGH_NO_PAUSE=1 PROVEKIT_SUPPLY_CHAIN_WALKTHROUGH_TARGET_DIR=/tmp/provekit-supply-chain-walkthrough-pr ./menagerie/supply-chain-rails/authenticated-betrayal/walkthrough/08-run-whole-exhibit.shgit diff --checkSummary by CodeRabbit
Release Notes - Version v1.6.3
New Features
provekit package inspectcommand for analyzing package identity and contents.provekit version check-extensionsubcommand for verifying contract set compatibility.provekit provefor validating artifacts against proofs and policies.Documentation
Tests