Skip to content

fix(ci): pass reviewed audit evidence to protected builds - #11370

Merged
cv merged 3 commits into
mainfrom
fix/protected-audit-controller-11088
Sep 10, 2026
Merged

fix(ci): pass reviewed audit evidence to protected builds#11370
cv merged 3 commits into
mainfrom
fix/protected-audit-controller-11088

Conversation

@sandl99

@sandl99 sandl99 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Outcome

Supply verified mcporter audit evidence to the offline protected image build. This prerequisite lets trusted-main E2E exercise #11156 without executing its candidate workflow as trusted code.

Reason

The protected rebuild uses --network none and --no-cache, but its controller does not supply audit evidence. OpenClaw therefore attempts a live npm audit without network access.

The failed protected job returned exit 1, EAI_AGAIN, 206 bytes, and metadata:missing on both attempts. Both responses had SHA-256 2ca8dc3553861955190f44adaa5ed386851b1c217d7e5084d6bc150ddb8623b4. Runtime qualification did not run. This is separate from the receipt-identity failure fixed by #11069.

Related issues

Refs #11088. Prerequisite for #11156, which owns the complete fix and protected-runtime acceptance.

Changes

The seven-file diff is +234/-7. The build path changes only the workflow handoff and its controller. The remaining changes protect that boundary. Main's merged CI memory fix (#11356) is consumed without duplication.

Verification

  • Existing affected audit, controller, and workflow tests: 313 passed across 12 files.
  • npm run build:cli and npm --prefix nemoclaw run build: passed.
  • npm exec -- vitest run --project package-contract test/package-contract/managed-image-registry-transport.test.ts: one test passed.
  • Normal pre-commit and commit-message hooks: passed, including repository checks, ShellCheck, secret scanning, and source-shape/growth checks.
  • NODE_OPTIONS=--max-old-space-size=5120 npm run validate:pr: passed at d2c864d4accfacdfe6825ded68d12e9b3fc7b462 against canonical main 270275f2a2b31a70fa72692d2ec304b5dffe8ee3. No tracked file changed.
Focused test command
npm exec -- vitest run --project integration \
  test/platform/images/protected-managed-image-build-script.test.ts \
  test/automation/releases/reviewed-npm-audit.test.ts \
  test/automation/releases/npm-audit-receipt.test.ts \
  test/automation/releases/reviewed-npm-audit-handoff.test.ts \
  test/automation/releases/reviewed-npm-audit-workflow.test.ts \
  test/automation/releases/reviewed-npm-audit-cache-key.test.ts \
  --project e2e-support \
  test/e2e/support/e2e-operations-workflow-boundary.test.ts \
  test/e2e/support/managed-image-protected-runtime-workflow.test.ts \
  test/e2e/support/hermes-workflow-boundary.test.ts \
  test/e2e/support/jetson-workflow-boundary.test.ts \
  test/e2e/support/staging-brev-launchable-identity-workflow-boundary.test.ts \
  test/e2e/support/shared-e2e-workflow-boundary.test.ts --reporter=default

Validation used pinned Node.js 22.23.2 and verified npm 10.9.4 in an unprivileged Linux container without host mounts, credentials, or a Docker socket. Signed candidate d2c864d4accfacdfe6825ded68d12e9b3fc7b462 has the same source tree as the hook-tested isolated commit. The CI memory setting comes from merged #11356.

No secrets, API keys, or credentials are in the diff. Incomplete, invalid, or unaccepted high-severity audit results still fail closed. Completeness checks, exception policy, raw-report and receipt integrity, registry selection, and retry bounds are unchanged. No broad-suite or protected-runtime pass is claimed.

Review notes

Self-review covered all seven paths at d2c864d4accfacdfe6825ded68d12e9b3fc7b462 in NVIDIA/NemoClaw. Sensitive paths are .github/workflows/e2e.yaml, scripts/checks/build-protected-managed-images.sh, and the two changed tools/e2e/ validators. Their independent review remains pending. Changed validators ran only in the approved credential-free environment; this is not independent security approval.

Protected GPU/local-inference acceptance remains pending. This prerequisite must enter trusted main through normal review and merge gates before dispatching jobs=managed-image-protected-runtime for #11156. MCP or image-startup results do not substitute for that job. No CI waiver is claimed.

The controller's private policy result follows its existing exit/signal cleanup. No protected registry, builder, container, image, or credential file was created by the local tests; Docker commands in controller tests are stubbed.


Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Security Improvements
    • Protected managed-image builds now require verified, reviewed npm audit evidence before importing caches or building images.
    • Builds reject incomplete or unverifiable audit evidence before Docker execution.
    • Audit results are bound to the candidate runtime and policy configuration, with integrity hashes passed into the protected build.
    • Workflow validation ensures audits and builds run through trusted controls rather than candidate-provided scripts.

Run the existing trusted audit action before the offline protected build.
Verify the candidate graph with trusted policy and pass the receipt, raw
report, and policy result through the existing BuildKit secret interface.

Refs #11088. Prerequisite for protected qualification of #11156.

Signed-off-by: San Dang <sdang@nvidia.com>
Consume #11356 from main; retain the protected audit handoff and resolve
the type-stripping fixture overlap.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this Sep 10, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 248a235a-c54b-4ace-8d40-164ff52587be

📥 Commits

Reviewing files that changed from the base of the PR and between edfa1e7 and 800053e.

📒 Files selected for processing (7)
  • .github/workflows/e2e.yaml
  • scripts/checks/build-protected-managed-images.sh
  • test/automation/releases/reviewed-npm-audit.test.ts
  • test/e2e/support/managed-image-protected-runtime-workflow.test.ts
  • test/platform/images/protected-managed-image-build-script.test.ts
  • tools/e2e/managed-image-protected-runtime-workflow-boundary.mts
  • tools/e2e/workflow-boundary.mts

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The protected runtime workflow now runs a reviewed npm audit against the candidate checkout. The build script validates the resulting evidence and passes it to OpenClaw builds as secrets and hashes. Boundary and build tests cover the new enforcement.

Changes

Protected image audit enforcement

Layer / File(s) Summary
Workflow audit and boundary enforcement
.github/workflows/e2e.yaml, tools/e2e/managed-image-protected-runtime-workflow-boundary.mts, tools/e2e/workflow-boundary.mts
The protected workflow runs the approved reviewed audit action against .candidate-runtime, passes its evidence to the trusted build, and enforces the required ordering and action validation.
Audit evidence validation and build propagation
scripts/checks/build-protected-managed-images.sh
The build script validates trusted audit inputs, verifies receipt and report evidence, requires evidence for cache imports, records hashes, and passes evidence to OpenClaw through BuildKit secrets and arguments.
Workflow and build validation coverage
test/automation/releases/reviewed-npm-audit.test.ts, test/e2e/support/managed-image-protected-runtime-workflow.test.ts, test/platform/images/protected-managed-image-build-script.test.ts
Tests cover workflow boundaries, verifier failures, incomplete evidence, imported-cache builds, and evidence propagation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProtectedRuntime as Protected runtime workflow
  participant NpmAudit as Reviewed npm audit action
  participant BuildScript as Protected image build script
  participant OpenClaw as OpenClaw build
  ProtectedRuntime->>NpmAudit: Audit .candidate-runtime
  NpmAudit-->>ProtectedRuntime: Write reviewed audit evidence
  ProtectedRuntime->>BuildScript: Pass audit evidence directory
  BuildScript->>BuildScript: Verify receipt, report, policy, and hashes
  BuildScript->>OpenClaw: Pass evidence secrets and digest arguments
Loading

Suggested reviewers: cv, ericksoa, prekshivyas

Merge Risk: ⚪ Minimal · up to 80005

Protected image builds now carry validated reviewed npm-audit evidence into offline builds, with failure paths covered before image construction. No concrete current-head merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: passing reviewed audit evidence into protected builds.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 6 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/protected-audit-controller-11088

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

@github-code-quality

github-code-quality Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 800053e in the fix/protected-audit-... branch remains at 96%, unchanged from commit edfa1e7 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 800053e in the fix/protected-audit-... branch remains at 83%, unchanged from commit edfa1e7 in the main branch.


Updated September 10, 2026 05:06 UTC

@github-actions

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit d2c864d. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

@sandl99

sandl99 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Validation and Advisor disposition for d2c864d4accfacdfe6825ded68d12e9b3fc7b462:

Architecture, migration, reduction, and verification report one shared concern: main's Dockerfile does not consume the policy-result secret and hash. That observation is correct for main's Dockerfile, but the proposed removal does not apply to this prerequisite's declared consumer.

The trusted controller builds --source-root, not necessarily its own checkout. This PR exists so trusted main can qualify #11156. At that PR's existing commit d59be959222801b076141569d898b178fc6aa2d5, the Dockerfile declares the policy-result hash, mounts the policy-result secret, and calls the existing helper. That helper requires all three evidence files and both hashes. Removing the policy input would make this candidate fail closed on an incomplete handoff.

Disposition: retain the policy-result input for this existing companion consumer. Do not copy its Dockerfile migration into the prerequisite, remove a required input, or add another test scenario. The trusted controller still verifies the receipt, raw report, candidate graph, and current policy before Docker. The five other specialists found no actionable issue. No production repair is accepted from this Advisor run.

Protected GPU/local-inference acceptance is still pending. These CI/MCP results do not substitute for jobs=managed-image-protected-runtime. This prerequisite needs normal review and merge into trusted main before that target runs on #11156. No merge approval or CI waiver is claimed.

Local validation's temporary container and downloaded image were removed and their absence confirmed. Temporary SDK archives and the validator download were removed. The source worktree and bounded evidence are retained; unrelated local changes are untouched.

@sandl99
sandl99 marked this pull request as ready for review September 10, 2026 04:50
@hunglp6d

Copy link
Copy Markdown
Collaborator

Verdict

No actionable correctness or security findings in PR #11370 at commit 800053e436d833d209f45bdf6502daa64100de32, compared with base edfa1e78870106a14aac8e3f550a758165ff8219.

The change fits its role as a prerequisite for #11156: trusted workflow code audits the candidate, the trusted controller verifies the evidence, and BuildKit receives it for the offline build. Merge readiness remains pending while CI completes.

Findings Table

No findings requiring changes within the reviewed diff.

Detailed Analysis

These verdicts reflect static review of the implementation and test coverage. No tests or E2E runs were executed during this review.

Category Verdict Assessment
Secrets and Credentials PASS No credentials added. Audit evidence uses BuildKit secrets; build arguments contain hashes.
Input Validation and Data Sanitization PASS Paths, symlinks, and missing evidence are checked. The verifier binds the receipt to the manifest, lockfile, raw report, and policy.
Authentication and Authorization PASS The action, verifier, and exception policy come from the trusted checkout. Candidate-provided audit actions are not executed.
Dependencies and Third-Party Libraries PASS Reuses the existing audit action without adding dependencies or registry fallbacks.
Error Handling and Logging PASS Verifier failure stops the controller before Docker. The temporary policy result follows existing cleanup handling.
Cryptography and Data Protection PASS Receipt and policy-result transport use SHA-256. The verifier checks the raw-report, graph, and policy hashes.
Configuration and Security Headers PASS Preserves --network none, OpenClaw’s --no-cache, and the high audit threshold.
Security Testing PASS Added tests cover incomplete evidence, verifier failure, and secret/hash handoff. Existing verifier tests cover identity, expiry, and policy mismatches.
System Security PASS Audit precedes the build. The change does not expand retries or weaken audit rejection conditions.

The policy-result input has a concrete consumer in #11156’s helper, which requires all three evidence files and both hashes. Its absence from main’s current Dockerfile is therefore not a reason to remove this prerequisite’s handoff.

At the review snapshot, no checks had failed, but some CLI shards and managed-image jobs were still running. Protected GPU/local-inference acceptance for #11156 remains pending after this prerequisite reaches trusted main. Ordinary image-build or startup results do not replace that acceptance.

@cv
cv merged commit dd88c3e into main Sep 10, 2026
82 checks passed
@cv
cv deleted the fix/protected-audit-controller-11088 branch September 10, 2026 08:52
sandl99 added a commit that referenced this pull request Sep 11, 2026
## Outcome

Protected managed-image builds can verify supplied mcporter audit
evidence before the offline rebuild.

## Reason

The trusted controller passes `--legacy-npmjs`, but the verifier accepts
only `--legacy-audit`. It rejects valid evidence with `verifier
arguments has unexpected or missing keys` before Docker starts. Manual
PR E2E uses this controller from `main`, so repairing the candidate copy
cannot unblock GPU qualification.

### Related issues

Refs #11088. Prerequisite for #11156, following #11370.

## Changes

Replace the obsolete verifier flag with the supported name. One line
changes; no new mechanism or test scenario.

## Verification

- Executed the controller's evidence-validation function with a valid
receipt and the real verifier: main exited 1; the one-line fix exited 0
and produced a clean policy result.
- Existing controller and receipt tests: 46 passed.
- `npm run validate:pr`: passed on
`615eb8afdc9906d22d8eb360b33380122780aad4` against canonical main
`189043e740fdab61c8c3fdf6fe9407f438e4d955`.
- No secrets, API keys, or credentials are in the diff.

## Review notes

Self-review covered `scripts/checks/build-protected-managed-images.sh`
at `615eb8afdc` in NVIDIA/NemoClaw. Validation used an unprivileged Node
22.23.2 Linux container without host mounts, credentials, or a Docker
socket. Independent review is pending. The GPU target remains owned by
#11156.

---
Signed-off-by: San Dang <sdang@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Updated audit receipt verification to use the correct legacy audit
option, improving compatibility with protected managed image checks.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: San Dang <sdang@nvidia.com>
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.

3 participants