fix(openclaw): disable EC2 metadata discovery - #6096
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
📝 WalkthroughWalkthroughThis PR sets ChangesEC2 metadata disable enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-6096.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Findings index
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 7 in-scope improvements
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/runner.test.ts (1)
949-956: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrdering check relies on source-text/position lock-in rather than observed behavior.
This check verifies relative string positions in
nemoclaw-start.shrather than executing the script and observing the resulting environment value.test/service-env.test.ts(lines 69-96) already proves the override behavior end-to-end by executing an extracted snippet and asserting on stdout. Consider whether the positional check here adds enough value over that behavioral test to justify the source-text coupling, since any future reformatting of the script (even if behavior is unchanged) could break this assertion.As per path instructions for test files, prefer observable outcomes through the public boundary over source-text assertions.
🤖 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 `@test/runner.test.ts` around lines 949 - 956, The assertion in runner.test.ts is coupled to the literal ordering of strings in the start script instead of verifying behavior. Update the test around the AWS_EC2_METADATA_DISABLED checks to validate the observed environment outcome through the same executable/public boundary used in test/service-env.test.ts, and remove the index-based source-position assertions tied to NEMOCLAW_CMD and OPENCLAW.Source: Path instructions
🤖 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 `@test/runner.test.ts`:
- Around line 939-948: The test in runner.test.ts currently uses
runtimeSrc.indexOf("# Stage 3: Runtime image") as the stage anchor, which can
silently fall back to the start of the file if the marker is missing. Add an
explicit assertion that the runtime stage marker exists before computing
fromIndex, and use the same marker in the loop that checks the ordering of FROM,
RUN, and ENV AWS_EC2_METADATA_DISABLED=true so the assertion fails loudly
instead of validating the wrong stage.
---
Nitpick comments:
In `@test/runner.test.ts`:
- Around line 949-956: The assertion in runner.test.ts is coupled to the literal
ordering of strings in the start script instead of verifying behavior. Update
the test around the AWS_EC2_METADATA_DISABLED checks to validate the observed
environment outcome through the same executable/public boundary used in
test/service-env.test.ts, and remove the index-based source-position assertions
tied to NEMOCLAW_CMD and OPENCLAW.
🪄 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: 7de3317d-2e27-4dad-a039-7a007c1dd37d
📒 Files selected for processing (8)
DockerfileDockerfile.basedocs/reference/troubleshooting.mdxdocs/security/best-practices.mdxscripts/nemoclaw-start.shtest/hermes-start.test.tstest/runner.test.tstest/service-env.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28496607030
|
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28550876803
|
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- markdownlint-disable MD041 --> ## Summary Force `AWS_EC2_METADATA_DISABLED=true` throughout OpenClaw image builds, gateway startup, child processes, and connected shells. OpenShell denies EC2 Instance Metadata Service access, so this durable sandbox invariant prevents an impossible credential-discovery path without disabling other AWS credential providers. ## Changes <!-- Bullet list of key changes. --> - Set the metadata-disable environment invariant before the first runtime-stage `RUN` in both OpenClaw Dockerfiles. The early stage-wide placement is intentional so every image-build command inherits it. - Override inherited or wrapper-supplied `false` values during OpenClaw startup and persist the fixed export in the runtime shell environment. - Add behavior and contract coverage for startup override, connected shells, idempotent root-mode generation, image ordering, and the unchanged Hermes boundary. - Define cross-agent credential-discovery semantics: host-brokered providers own host credentials, while sandbox-direct providers admit only deliberate, reachable sources and disable impossible metadata probes. - Document the OpenClaw provider scope and rebuild steps, plus why managed Hermes remains unaffected and how future direct providers must be reviewed and tested. Hermes code stays unchanged because the shipped image omits native Bedrock and its supported provider is host-brokered through `inference.local`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: Independent implementation, security, and docs reviews passed. Live OpenShell acceptance covered fresh startup, restart, gateway and connected-shell `AWS_EC2_METADATA_DISABLED=true`, no logged `169.254.169.254` probes, explicit IMDS denial, shared-profile resolution, and a successful inference turn. Rebuild uses the same tested image and entrypoint contract; the exact-head `ubuntu-repo-cloud-openclaw` target also [passed](https://github.com/NVIDIA/NemoClaw/actions/runs/28496607030). - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Force
AWS_EC2_METADATA_DISABLED=truethroughout OpenClaw image builds, gateway startup, child processes, and connected shells. OpenShell denies EC2 Instance Metadata Service access, so this durable sandbox invariant prevents an impossible credential-discovery path without disabling other AWS credential providers.Changes
RUNin both OpenClaw Dockerfiles. The early stage-wide placement is intentional so every image-build command inherits it.falsevalues during OpenClaw startup and persist the fixed export in the runtime shell environment.inference.local.Type of Change
Quality Gates
AWS_EC2_METADATA_DISABLED=true, no logged169.254.169.254probes, explicit IMDS denial, shared-profile resolution, and a successful inference turn. Rebuild uses the same tested image and entrypoint contract; the exact-headubuntu-repo-cloud-openclawtarget also passed.Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Aaron Erickson aerickson@nvidia.com