test(e2e): harden image reuse workflow validator - #7551
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
📝 WalkthroughWalkthroughThe workflow boundary validator now detects action-based authentication, multiline Buildx commands, and expanded secret usage. Hermes producer and consumer checks enforce artifact ordering, canonical action pins, and authentication restrictions with updated E2E cases. ChangesHermes workflow boundary validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/e2e/sandbox-images-workflow-boundary.mts`:
- Around line 388-393: Update the Hermes consumer check around the raw step.run
inspection at line 868 to pass the command through normalizeShellContinuations
before evaluating Docker Buildx commands, so backslash-continued docker buildx
build commands trigger the consumer rebuild guard. Add a regression test
covering this continued-command form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b3994195-5ac9-49d6-b60e-fa0466aca9b2
📒 Files selected for processing (2)
test/e2e/support/sandbox-images-workflow-boundary.test.tstools/e2e/sandbox-images-workflow-boundary.mts
| function dockerBuildLines(job: SandboxImagesWorkflowJob): string[] { | ||
| return steps(job).flatMap((step) => | ||
| (step.run ?? "") | ||
| normalizeShellContinuations(step.run ?? "") | ||
| .split("\n") | ||
| .map((line) => line.trim()) | ||
| .filter((line) => /^docker\s+build(?:\s|$)/u.test(line)), | ||
| .filter((line) => /^docker\s+(?:build|buildx\s+build)(?:\s|$)/u.test(line)), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Normalize continued Buildx commands in the Hermes consumer check.
Line 868 still tests raw step.run, so docker \ followed by buildx build --load ... bypasses the consumer rebuild guard. Apply normalizeShellContinuations there and add the corresponding consumer regression test.
Suggested fix
- /\bdocker\s+(?:build|buildx\s+build)\b/u.test(step.run ?? ""),
+ /\bdocker\s+(?:build|buildx\s+build)\b/u.test(
+ normalizeShellContinuations(step.run ?? ""),
+ ),🤖 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/e2e/sandbox-images-workflow-boundary.mts` around lines 388 - 393,
Update the Hermes consumer check around the raw step.run inspection at line 868
to pass the command through normalizeShellContinuations before evaluating Docker
Buildx commands, so backslash-continued docker buildx build commands trigger the
consumer rebuild guard. Add a regression test covering this continued-command
form.
<!-- markdownlint-disable MD041 --> ## Summary Close the remaining Hermes image-consumer validator bypasses identified after #7551 merged. Shell continuations are normalized before the no-rebuild check, and assigned Buildx push values are evaluated case-insensitively. Literal `false` remains non-writing; true, empty, mixed-case, and dynamic or otherwise nonliteral values are rejected so the consumer cannot hide a rebuild or registry write. ## Related Issue Part of #7451. Follow-up to #7508 and #7551. ## Changes - Normalize Hermes consumer shell continuations before checking for Docker and Buildx builds. - Detect assigned, mixed-case, and dynamic Buildx registry-push values in the consumer guard. - Preserve literal `--push=false` as non-writing. - Add adversarial regressions for multiline rebuilds and assigned push values. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: the production workflow and documented no-rebuild contract are unchanged; this makes the internal validator enforce that existing contract for continued commands. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: CodeRabbit reproduced the bypass on #7551; the exact mutation is now an adversarial test and passes only with normalization at the consumer guard. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: At PR SHA `2cef897b4fcba8033d115d94f2dd4c2ae12d86ed`, the change only strengthens the repository-internal Hermes image-consumer validator for multiline rebuilds and assigned, mixed-case, or dynamic Buildx push values. Literal `--push=false` remains non-writing. `test/e2e/README.md` already documents the no-rebuild/no-registry-write contract. The focused E2E-support suite passed 29/29, the PR workflow-contract suite passed 22/22, and `npm run build:cli` plus `npm run check:diff` passed. - Agent: Codex Desktop <!-- docs-review-head-sha: 2cef897 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed; `npm run build:cli` and `npm run check:diff` also passed on exact head `2cef897b4fcba8033d115d94f2dd4c2ae12d86ed`. - [x] Targeted behavior tests pass for the current change set — E2E-support passed 29/29 and the PR workflow-contract suite passed 22/22. - [x] Applicable broad gate passed — not applicable to this validator/test-only delta; the exact affected suites, CLI build, and complete diff gate passed, and GitHub CI remains required before approval. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox image workflow validation to more precisely detect `docker buildx build ... --push` commands, including shell line continuations and `--push` casing/formatting. * Strengthened Hermes prebuilt-image reuse checks by normalizing command formatting before validating consumer steps. * **Tests** * Added new end-to-end negative cases covering consumer attempts to rebuild the prebuilt Hermes production image. * Added coverage to ensure consumers attempting registry writes via `--push` are rejected, while explicitly using `--push=false` is allowed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the exact `## v0.0.96` heading. The entry reconciles all 90 first-parent commits since v0.0.95 with all 92 merged PRs in the live `v0.0.96` label ledger and groups the user-visible changes by operator journey. ## Changes - Add the parser-safe dated MDX changelog entry for v0.0.96 with root-absolute links to the focused user guides. - Source summary: - [#7194](#7194) -> `docs/changelog/2026-07-25.mdx`: Document persistent baseline network policy exclusions and their inspection, rebuild, and snapshot behavior. - [#7188](#7188), [#7427](#7427), and [#7546](#7546) -> `docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference routing, keyless loopback endpoints, and provider-marker isolation. - [#7238](#7238) -> `docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider identifier validation before state writes or OpenShell calls, with bounded terminal-safe rejection previews. - [#7319](#7319), [#7274](#7274), [#7528](#7528), [#7353](#7353), and [#7560](#7560) -> `docs/changelog/2026-07-25.mdx`: Document the managed default gateway service, onboarding readiness, and container-runtime identity safeguards. - [#7349](#7349), [#7498](#7498), [#7406](#7406), [#7196](#7196), [#7559](#7559), [#7421](#7421), [#7510](#7510), [#7295](#7295), and [#7565](#7565) -> `docs/changelog/2026-07-25.mdx`: Document gateway-scoped status, lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and fail-closed CLI prompt and command output. - [#7591](#7591) -> `docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP tool-name discovery, its bounded and names-only contract, probe interaction, and rebuild requirement. - [#7305](#7305), [#7480](#7480), [#7471](#7471), [#7365](#7365), and [#7541](#7541) -> `docs/changelog/2026-07-25.mdx`: Document installer version checks, version-tag reporting, license guidance, WSL Ollama selection, and DGX Station vLLM detection. - [#7482](#7482), [#7466](#7466), [#7208](#7208), [#7434](#7434), and [#7586](#7586) -> `docs/changelog/2026-07-25.mdx`: Document Ollama resource details, reasoning precedence, Hermes onboarding behavior, and preserved managed Hermes BuildKit failures. - [#6830](#6830), [#7492](#7492), [#7563](#7563), and [#7582](#7582) -> `docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw production lock, fixed managed-image dependencies, immutable Hermes base adoption, and Hermes image-size reduction. - [#7505](#7505), [#7530](#7530), [#7547](#7547), [#7508](#7508), [#7548](#7548), [#7549](#7549), [#7537](#7537), [#7534](#7534), [#7515](#7515), [#7511](#7511), [#7551](#7551), [#7562](#7562), [#7575](#7575), [#7496](#7496), [#7594](#7594), [#7595](#7595), and [#7599](#7599) -> `docs/changelog/2026-07-25.mdx`: Summarize release validation, transient and bounded dispatch reconciliation, exact pre-tag qualification, identity revalidation, npm-audit retry, sharding, image reuse, timeout, telemetry, and workflow-hardening changes. - Reconciled without separate changelog prose: - [#7539](#7539), [#7526](#7526), [#7507](#7507), [#7506](#7506), [#7519](#7519), [#7516](#7516), [#7396](#7396), [#7254](#7254), [#7583](#7583), [#7596](#7596), and [#7598](#7598): Test-harness or fixture-only changes. - [#7403](#7403), [#7161](#7161), [#6877](#6877), [#7531](#7531), [#7525](#7525), [#7522](#7522), [#7536](#7536), [#7552](#7552), [#7566](#7566), [#7553](#7553), [#7561](#7561), [#7577](#7577), [#7569](#7569), [#7585](#7585), [#7584](#7584), [#7592](#7592), [#7580](#7580), [#7571](#7571), [#7517](#7517), [#7589](#7589), [#7402](#7402), [#7558](#7558), [#7544](#7544), and [#7601](#7601): Dependency, internal recovery, validation, contributor-workflow, E2E optimization, telemetry, or CI trust changes with no separate user-facing release claim. - [#7556](#7556), [#7573](#7573), [#7576](#7576), and [#7578](#7578): Experimental repository-maintainer conflict automation with no canonical user documentation surface. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, version headings, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head `0f5dedb47` against 90 first-parent release commits and 92 merged PRs labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version heading, literal CLI names, writing style, skip terms, all 20 root-absolute published links, and the accepted #7591 opt-in authenticated discovery bounds. #7544, #7599, and #7601 remain internal or CI-only release-ledger entries. Changelog tests passed 6/6, the docs build passed with 0 errors and two pre-existing Fern warnings, and `npm run check:diff` plus the final diff check passed. - Agent: Codex Desktop documentation-writer subagent <!-- docs-review-head-sha: 0f5dedb --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts`: 6/6 passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this prose-only changelog entry. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — the build passed with 0 errors and 2 existing Fern warnings; the published-route check passed. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — native changelog files use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Persistent network policy exclusions with consistent restore/exclusion reporting across rebuilds/snapshots. * Opt-in MCP tool discovery via `mcp status --tools` with bounded, redacted authenticated traffic. * Improved HTTPS inference switching for custom endpoints and refreshed onboarding/model menu details. * Refined OpenShell gateway defaults for port `8080`, including more reliable readiness checks. * **Bug Fixes** * Prevent incorrect provider/model restoration after compatible-provider update failures. * Preserve managed MCP state after exec loss and tighten gateway/doctor status scoping. * **Tests** * Stronger, fail-closed release validation with hardened evidence/artifact handoff and bounded timeouts/retries. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
Harden the merged Hermes image-reuse workflow validator so action inputs and shell continuations cannot bypass credential, registry-write, build-cardinality, or artifact-handoff safeguards. Production workflow behavior is unchanged.
Related Issue
Part of #7451. Follow-up to #7508.
Changes
withinputs for protected secrets and rejectdocker/login-actionoutside the canonical producer authentication step.docker buildx buildso multiline pushes and duplicate CLI builds fail closed.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededa6e864323, the only changes are internal E2E workflow-boundary validator hardening and adversarial support tests. The production workflow remains unchanged. Existingtest/e2e/README.mdalready documents the Hermes producer/consumer artifact flow, exact action pins, local image loading, and disabled registry writes. The explicit affected suite passed 26/26 andnpm run check:diffpassed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed;npm run check:diffalso passed on exact refreshed heada6e864323.npm run test:changed -- --coverage=falsealso passed 26/26; after the merge-topology refresh it selected no tests, so the affected files were run explicitly.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit