Skip to content

[codex] disable unsupported OpenClaw self-update hints in sandbox#1215

Closed
13ernkastel wants to merge 4 commits intoNVIDIA:mainfrom
13ernkastel:codex/disable-openclaw-update-hints
Closed

[codex] disable unsupported OpenClaw self-update hints in sandbox#1215
13ernkastel wants to merge 4 commits intoNVIDIA:mainfrom
13ernkastel:codex/disable-openclaw-update-hints

Conversation

@13ernkastel
Copy link
Copy Markdown
Contributor

@13ernkastel 13ernkastel commented Apr 1, 2026 •
edited by coderabbitai bot
Loading

Summary

This PR stops NemoClaw sandbox images from advertising openclaw update as the supported upgrade path.

Root cause

NemoClaw installs openclaw into the sandbox image at build time and pins that version in Dockerfile.base, but the generated sandbox config still allows OpenClaw's startup update hint. Inside the sandbox that hint tells users to run openclaw update, which leads them into an in-container self-update flow that is not how NemoClaw images are upgraded.

What changed

  • disable startup update hints in the generated sandbox openclaw.json
  • document the supported upgrade path in troubleshooting docs
  • add an image-level regression assertion that the sandbox config sets update.checkOnStart to false

User impact

Users inside a NemoClaw sandbox will stop seeing a misleading recommendation to run openclaw update. The supported path remains shipping a newer pinned OpenClaw version in the image and recreating the sandbox from that image.

Addresses #1029.

Validation

  • bash -n test/e2e-gateway-isolation.sh
  • git diff --check
  • not run: ./test/e2e-gateway-isolation.sh (docker is not installed in the publishing environment)

Summary by CodeRabbit

  • Documentation

    • Added troubleshooting guidance for update operations that hang or time out when run inside sandboxes, and recommended resolution steps.

  • Configuration

    • Startup update checks are disabled by default in the generated runtime configuration.

  • Tests

    • End-to-end gateway isolation tests updated to verify the startup update hint is disabled and test sequence renumbered accordingly.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff2e564a-d6df-4672-b272-a18b019e900d

📥 Commits

Reviewing files that changed from the base of the PR and between e1b790a and cc4177c.

📒 Files selected for processing (1)
  • Dockerfile
✅ Files skipped from review due to trivial changes (1)
  • Dockerfile
📝 Walkthrough

Walkthrough

Build-time config now sets update.checkOnStart: False in the generated openclaw.json; documentation was added explaining why openclaw update must not be run inside sandboxes; e2e tests were updated to assert the new config value and renumbered accordingly.

Changes

Cohort / File(s) Summary
Configuration Generation
Dockerfile		 Added update.checkOnStart: False to the build-time generated openclaw.json to disable startup update checks in sandbox images.
Documentation
docs/reference/troubleshooting.md		 Added a troubleshooting subsection describing that openclaw update may hang inside sandboxes (expected when openclaw is pinned) and instructions to upgrade NemoClaw or rebuild the sandbox base image.
End-to-End Tests
test/e2e-gateway-isolation.sh		 Inserted a new test asserting /sandbox/.openclaw/openclaw.json contains update.checkOnStart: False; previous test about config hash writability was renumbered and subsequent tests updated accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I toggled a flag in the build-time dew,
No startup checks hopping into view,
Docs whisper why updates stall,
Tests peek in the sandbox hall,
Rebuild the image, and all will be new. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: disabling OpenClaw self-update hints in the sandbox environment, which aligns with the core objective of preventing unsupported update suggestions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@13ernkastel 13ernkastel marked this pull request as ready for review April 1, 2026 00:46
This was referenced Apr 1, 2026
Closed
Open
@wscurran wscurran added status: triage For new items that haven't been reviewed yet. bug Something isn't working NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). and removed status: triage For new items that haven't been reviewed yet. labels Apr 1, 2026
@wscurran
Copy link
Copy Markdown
Contributor

wscurran commented Apr 1, 2026

✨ Thanks for submitting this pull request, which proposes a way to disable unsupported OpenClaw self-update hints in sandbox.

Possibly related open issues:

@13ernkastel
Copy link
Copy Markdown
Contributor Author

13ernkastel commented Apr 5, 2026

Temporarily closing this bundled security PR to free one contributor PR slot for the grouped cleanup PR. I’ll link the replacement PR here as soon as it is open.

@13ernkastel 13ernkastel closed this Apr 5, 2026
@13ernkastel
Copy link
Copy Markdown
Contributor Author

13ernkastel commented Apr 5, 2026

Follow-up: the grouped cleanup PR is now open at #1500. I’m grouping the remaining Telegram/self-update security cleanup there while keeping #1416 and #1499 separate on purpose.

@13ernkastel
Copy link
Copy Markdown
Contributor Author

13ernkastel commented Apr 5, 2026

Consolidated into the open security PR #1416: #1416

cv added a commit that referenced this pull request Apr 17, 2026
@13ernkastel @latenighthackathon @cv
fix(security): harden sandbox command execution and validate sandbox …
800195b 
…names (#1416)

## Summary

Bundles the remaining sandbox command-hardening work with the Telegram
fail-closed cleanup and the unsupported self-update-hint fix.

This now includes the original `#1416` scope plus the changes that had
temporarily been split into `#1500`.
`#1499` remains separate on purpose.

## Linked Issues

- Fixes #1029

## Related PRs / Issues

- follow-up to `#1392`
- folds in `#1218`
- folds in `#1215`
- replaces `#1500`
- keeps `#1499` separate
- addresses `#896`

## Changes

- re-validates sandbox names at the `createSandbox()` boundary and
removes the remaining shell-string dependency from follow-on sandbox
command paths
- adds `runFile()` and uses argv-style execution for
`setup-dns-proxy.sh`
- replaces the dashboard readiness probe with the structured OpenShell
helper path
- requires an explicit Telegram chat allowlist before the bridge
forwards prompts
- adds `nemoclaw telegram` subcommands and `nemoclaw start
--discover-chat-id`
- preserves the reserved-sandbox-name guard added during the Telegram
review follow-up
- disables unsupported OpenClaw self-update hints in the generated
sandbox config
- propagates saved Telegram allowlists into the remote deploy env so
deployed bridges stay fail-closed too
- updates focused CLI/deploy tests to match the current services-based
startup path on `main`

## Why

These changes all tighten the default security posture around
operator-managed sandboxes:
- sandbox creation and follow-on helper execution rely less on
shell-string construction
- Telegram bridge access now fails closed unless the operator explicitly
allowlists chat IDs
- sandbox images stop advertising an unsupported in-container
self-update path

Keeping them together in `#1416` makes the remaining security review
surface smaller while still leaving the separate immutable-hardening
follow-up in `#1499` alone.

## Validation

- `npm run build:cli`
- `npx vitest run src/lib/deploy.test.ts src/lib/onboard-session.test.ts
test/onboard.test.js test/cli.test.js test/runner.test.js
test/service-env.test.js test/registry.test.js
test/shellquote-sandbox.test.js`

## Risks / Notes

- `npm run typecheck:cli` still hits the repo's existing
`src/lib/*.test.ts -> ../../dist/lib/*` type-resolution issue in this
environment, so validation here relies on the targeted build plus Vitest
coverage above
- `#1499` remains separate on purpose


Signed-off-by: Chia Min Jun Lennon <LennonCMJ@live.com>

---------

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
Signed-off-by: 13ernkastel <LennonCMJ@live.com>
Co-authored-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
Co-authored-by: Test User <test@example.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@13ernkastel @wscurran