fix(security): keep Sigstore audit out of image builds - #8927
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
added
area: sandbox
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a pull-request-only reviewed npm audit gate for managed images. It removes duplicate ChangesReviewed npm audit boundary
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The change removes Sigstore access from restricted image builds while retaining fail-closed verification in trusted CI and adding regression coverage. No actionable merge-blocking risk remains beyond normal required checks. Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant CIWorkflow
participant ReviewedAudit
participant ManagedImageBuilder
participant Provenance
PullRequest->>CIWorkflow: open or update PR
CIWorkflow->>ReviewedAudit: verify exact lock and signatures
ReviewedAudit-->>CIWorkflow: pass or fail audit result
CIWorkflow->>ManagedImageBuilder: build after audit passes
ManagedImageBuilder->>Provenance: record schema 4 and reviewed-audit recipe
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 9435084 in the TypeScript / code-coverage/cliThe overall coverage in commit 9435084 in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-8927.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
7 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: 1 optional E2E recommendation
1 warning · 0 suggestionsWarningsWarnings do not block.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
877-890: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSeparate trusted-CI attestation from the base provenance marker.
Dockerfile.basewrites the marker without runningnpm audit signatures, butDockerfileaccepts a matching marker fromnemoclaw-sandbox-base-localas sufficient to reuse the reviewed base. Remove local references from this reuse path, or require an independently verified CI attestation. Treatschema=4andmcporter-recipe=locked-ci+reviewed-audit-v3as package and audit metadata, not signature evidence.🤖 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 `@Dockerfile` around lines 877 - 890, Separate trusted CI signature attestation from the provenance fields emitted by the Dockerfile marker. In the base-image reuse validation, do not accept a matching marker from nemoclaw-sandbox-base-local unless it includes an independently verified CI attestation; remove local-marker acceptance otherwise. Treat schema=4 and mcporter-recipe=locked-ci+reviewed-audit-v3 only as package and audit metadata, not signature evidence.
🤖 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.
Outside diff comments:
In `@Dockerfile`:
- Around line 877-890: Separate trusted CI signature attestation from the
provenance fields emitted by the Dockerfile marker. In the base-image reuse
validation, do not accept a matching marker from nemoclaw-sandbox-base-local
unless it includes an independently verified CI attestation; remove local-marker
acceptance otherwise. Treat schema=4 and
mcporter-recipe=locked-ci+reviewed-audit-v3 only as package and audit metadata,
not signature evidence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7769461e-b3e2-4011-8214-937464385f84
📒 Files selected for processing (13)
.github/workflows/managed-images.yamlDockerfileDockerfile.baseagents/openclaw/dependency-review.mdci/source-shape-test-budget.jsondocs/security/configure-corporate-ca-trust.mdxtest/corporate-ca-build-tls-anchor.test.tstest/fetch-guard-patch-regression.test.tstest/managed-image-publication-workflow.test.tstest/mcporter-supply-chain.test.tstest/openclaw-dependency-review.test.tstest/openclaw-integrity-pin-suite.tstest/openclaw-locked-install.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the supply-chain review finding.\n\n- The base provenance marker now records package and advisory metadata only. It does not authorize runtime reuse by itself.\n- The final image reuses an installed OpenClaw and mcporter runtime only from a digest-addressed base in the official GHCR namespace.\n- Local bases, custom bases, and mutable official tags reinstall the locked runtimes. A newer incompatible base still fails closed.\n- Regression coverage exercises local, bare-local, mutable-tag, custom, malformed-marker, version-mismatch, and digest-addressed official-base paths.\n\nSecurity review covered all nine repository categories. The change adds no secret, credential, input, authorization, cryptographic primitive, privilege, or network-policy behavior. It narrows an existing trust decision and preserves the required CI signature gate.\n\nValidation:\n\n- affected supply-chain and workflow suites: 8 files, 124 tests passed\n- CLI type-check and repository checks passed\n- all pre-commit hooks passed, including Dockerfile lint, secret scan, source-shape, and test-size checks\n- documentation build passed with zero errors and two existing Fern warnings\n- the latest PR commit is Verified and includes the DCO sign-off\n\nFresh repository checks and human review remain required. |
2 participants
Summary
Restricted OpenClaw sandbox image builds no longer contact Sigstore while installing the committed mcporter graph, so denied TUF egress cannot abort onboarding or rebuild.
Registry-signature verification remains fail closed in trusted CI for the committed lock, including every merge and image-publication path.
Related Issue
Fixes #8925
Changes
npm audit signaturesrequest fromDockerfileandDockerfile.basewhile retaining exact lock, registry origin, integrity, installed-graph, lifecycle, runtime, and reviewed-advisory checks.schema=4andmcporter-recipe=locked-ci+reviewed-audit-v3so older base markers cannot cross the updated control boundary.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/security/configure-corporate-ca-trust.mdxandagents/openclaw/dependency-review.mdto separate package provenance from CI signature evidence and explain the digest-pinned base-reuse boundary.npm run docspassed at the final commit with zero errors and two existing Fern warnings.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable27c375e. Documentation, project-boundary, source-shape, title, size, Dockerfile lint, type-check, formatting, and secret-scan checks passed.npm testencountered unrelated host-specific WSL, Homebrew trust, process, and filesystem failures. Required GitHub CI is the authoritative broad gate and must pass before merge.npm run docsbuilds without warnings (doc changes only) — the build passed with zero errors and two existing Fern warnings.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Security
Documentation
Tests