fix(security): strip gateway token from descendants - #8872
Merged
Conversation
Refs #8693 Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe gateway launch paths now use a shared helper. The helper controls log mode, removes ChangesGateway launch behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@test/nemoclaw-start-gateway-token-env.test.ts`:
- Around line 16-42: Update the test around launch_openclaw_gateway_process to
seed gatewayLog with existing content before launching, then assert
mode-specific final contents: truncate must replace the seed with the command
output, while append must preserve the seed and add the output. Keep the
existing token-removal assertion and cleanup behavior unchanged.
🪄 Autofix
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: 73d44a78-11a0-4509-ab71-d6814e827448
📒 Files selected for processing (2)
scripts/nemoclaw-start.shtest/nemoclaw-start-gateway-token-env.test.ts
Comment on lines
+16
to
+42
| it.each([ | ||
| "truncate", | ||
| "append", | ||
| ])("removes the dashboard token from a %s gateway launch (#8693)", (logMode) => { | ||
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-token-env-")); | ||
| const gatewayLog = path.join(tmpDir, "gateway.log"); | ||
| const source = fs.readFileSync(START_SCRIPT, "utf8"); | ||
| const launch = extractShellFunctionFromSource( | ||
| source, | ||
| "launch_openclaw_gateway_process", | ||
| ).replaceAll("/tmp/gateway.log", gatewayLog); | ||
| const script = [ | ||
| "set -euo pipefail", | ||
| launch, | ||
| "export OPENCLAW_GATEWAY_TOKEN=dashboard-secret", | ||
| `launch_openclaw_gateway_process ${logMode} sh -c 'printf "%s\\n" "\${OPENCLAW_GATEWAY_TOKEN-unset}"'`, | ||
| 'wait "$GATEWAY_PID"', | ||
| ].join("\n"); | ||
|
|
||
| try { | ||
| const result = spawnSync("bash", ["-c", script], { encoding: "utf8", timeout: 5000 }); | ||
| expect(result.status, result.stderr).toBe(0); | ||
| expect(fs.readFileSync(gatewayLog, "utf8")).toBe("unset\n"); | ||
| } finally { | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test the log-mode effect.
The test uses an empty gateway log for both modes. It passes if append truncates the log or if truncate appends to it. Seed the log and assert the distinct final contents.
Proposed test update
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-token-env-"));
const gatewayLog = path.join(tmpDir, "gateway.log");
+ fs.writeFileSync(gatewayLog, "existing\n");
const source = fs.readFileSync(START_SCRIPT, "utf8");
@@
- expect(fs.readFileSync(gatewayLog, "utf8")).toBe("unset\n");
+ expect(fs.readFileSync(gatewayLog, "utf8")).toBe(
+ logMode === "append" ? "existing\nunset\n" : "unset\n",
+ );As per path instructions, “Review tests for behavioral confidence rather than implementation lock-in.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it.each([ | |
| "truncate", | |
| "append", | |
| ])("removes the dashboard token from a %s gateway launch (#8693)", (logMode) => { | |
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-token-env-")); | |
| const gatewayLog = path.join(tmpDir, "gateway.log"); | |
| const source = fs.readFileSync(START_SCRIPT, "utf8"); | |
| const launch = extractShellFunctionFromSource( | |
| source, | |
| "launch_openclaw_gateway_process", | |
| ).replaceAll("/tmp/gateway.log", gatewayLog); | |
| const script = [ | |
| "set -euo pipefail", | |
| launch, | |
| "export OPENCLAW_GATEWAY_TOKEN=dashboard-secret", | |
| `launch_openclaw_gateway_process ${logMode} sh -c 'printf "%s\\n" "\${OPENCLAW_GATEWAY_TOKEN-unset}"'`, | |
| 'wait "$GATEWAY_PID"', | |
| ].join("\n"); | |
| try { | |
| const result = spawnSync("bash", ["-c", script], { encoding: "utf8", timeout: 5000 }); | |
| expect(result.status, result.stderr).toBe(0); | |
| expect(fs.readFileSync(gatewayLog, "utf8")).toBe("unset\n"); | |
| } finally { | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| } | |
| }); | |
| it.each([ | |
| "truncate", | |
| "append", | |
| ])("removes the dashboard token from a %s gateway launch (#8693)", (logMode) => { | |
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-gateway-token-env-")); | |
| const gatewayLog = path.join(tmpDir, "gateway.log"); | |
| fs.writeFileSync(gatewayLog, "existing\n"); | |
| const source = fs.readFileSync(START_SCRIPT, "utf8"); | |
| const launch = extractShellFunctionFromSource( | |
| source, | |
| "launch_openclaw_gateway_process", | |
| ).replaceAll("/tmp/gateway.log", gatewayLog); | |
| const script = [ | |
| "set -euo pipefail", | |
| launch, | |
| "export OPENCLAW_GATEWAY_TOKEN=dashboard-secret", | |
| `launch_openclaw_gateway_process ${logMode} sh -c 'printf "%s\\n" "\${OPENCLAW_GATEWAY_TOKEN-unset}"'`, | |
| 'wait "$GATEWAY_PID"', | |
| ].join("\n"); | |
| try { | |
| const result = spawnSync("bash", ["-c", script], { encoding: "utf8", timeout: 5000 }); | |
| expect(result.status, result.stderr).toBe(0); | |
| expect(fs.readFileSync(gatewayLog, "utf8")).toBe( | |
| logMode === "append" ? "existing\nunset\n" : "unset\n", | |
| ); | |
| } finally { | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| } | |
| }); |
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 21-21: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(START_SCRIPT, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 37-37: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(gatewayLog, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 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/nemoclaw-start-gateway-token-env.test.ts` around lines 16 - 42, Update
the test around launch_openclaw_gateway_process to seed gatewayLog with existing
content before launching, then assert mode-specific final contents: truncate
must replace the seed with the command output, while append must preserve the
seed and add the output. Keep the existing token-removal assertion and cleanup
behavior unchanged.
Source: Path instructions
cv
approved these changes
Aug 12, 2026
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology 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: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
This was referenced Aug 12, 2026
23 tasks
cv
pushed a commit
that referenced
this pull request
Aug 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary OpenClaw managed-image activation stopped before binding the gateway port after #8872 correctly removed `OPENCLAW_GATEWAY_TOKEN` from the gateway process environment. The direct cause was OpenClaw 2026.7 classifying NemoClaw's freshly authenticated config as clobbered because the atomic token rotation did not stamp OpenClaw's `meta.lastTouchedAt` field. OpenClaw then restored its tokenless build-time backup before gateway authentication resolved. This change stamps that metadata in the existing atomic config write, so OpenClaw keeps the rotated authenticated config while the bearer remains absent from both gateway argv and the gateway/descendant environment. Gateway logging now also opens the fixed log path through a descriptor-pinned, no-follow boundary under the final gateway identity. ## Related Issue Refs #8693 ## Changes - Stamp `meta.lastTouchedAt` when rotating the gateway token through the existing pinned-directory, no-symlink, owner-only temporary file, `fsync`, and atomic-rename path. - Keep the gateway launch command credential-free and continue removing `OPENCLAW_GATEWAY_TOKEN` at the process boundary for initial launches and automatic respawns. - Open `/tmp/gateway.log` with `O_NOFOLLOW`, pin and verify the opened inode before redirecting output, prevent gateway launch when safe initial log replacement fails, and fail closed if a respawn encounters a symlink or replaced path. - In root mode, step down to the `gateway` identity before opening that log descriptor while preserving `HOME=/sandbox` and umask `0007`. - Add Linux process-level coverage that reads `/proc/<pid>/cmdline` and `/proc/<pid>/environ` for both launch paths and rejects token exposure. - Preserve and test truncate-on-initial-launch and append-on-respawn log behavior, including replacement of a planted symlink at initial launch and refusal of one at respawn. ## 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 - [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: The final diff removes the interim argv credential path entirely. The token remains in the existing OpenClaw config and is rotated through the existing symlink-rejecting, exclusive temporary file, `fsync`, atomic rename, and hash-refresh flow. The gateway launcher removes `OPENCLAW_GATEWAY_TOKEN`, steps down before the descriptor-safe opener in root mode, opens the log with `O_NOFOLLOW`, verifies the opened inode, and only then redirects and execs the gateway. Safe initial-log replacement failure prevents launch. Linux process tests inspect both argv and environment for initial launch and respawn; filesystem regressions prove initial launch replaces a planted gateway-log symlink without changing its target, refuses to launch on safe-replacement failure, and refuses a respawn symlink. No input parsing, shell interpolation, network bind, dependency, or cryptographic boundary is expanded. - [ ] 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: `docs/security/filesystem-controls.mdx` accurately documents that the entrypoint retains the token path for interactive sandbox shells while the OpenClaw launch boundary removes `OPENCLAW_GATEWAY_TOKEN` from the gateway/descendant environment and does not add the bearer to process arguments; the gateway reads it from `openclaw.json`. The privilege-drop change moves the descriptor-safe gateway-log opener under the `gateway` identity in root mode and requires no additional public documentation. - Agent: Codex Desktop <!-- docs-review-head-sha: 495a433 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Result: Not applicable - Supporting evidence: Not applicable ## 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 - [x] Targeted behavior tests pass for the current change set — the focused changed-file review passed 149 tests with 3 platform skips; the fail-closed safe-log suite passes 4/4, including refusal to launch after initial log replacement failure. The exact published OpenClaw 2026.7.1 managed image passed all 32 affected gateway lifecycle, credential, and log-safety tests, including live initial-launch and respawn `/proc` assertions. The earlier exact-image activation reached HTTP 200, preserved the rotated config token, and exposed the token in neither `/proc/<pid>/cmdline` nor `/proc/<pid>/environ`. - [x] Applicable broad gate passed — `npm run typecheck`, repository checks, ShellCheck, secret scanning, source-shape budget, and test-file-size budget passed through local validation and normal hooks. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `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) `npm run docs` passes with 0 errors and the repository's 2 known pre-existing Fern warnings. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Managed OpenClaw gateway launches and automatic respawns no longer inherit
OPENCLAW_GATEWAY_TOKEN, so descendants of those gateway processes do not receive that ambient environment value.This is a bounded defense-in-depth improvement for #8693. It does not remove the token from OpenClaw configuration, the generated runtime environment file, or other same-UID sandbox-readable surfaces. The issue's host-side/injection-only credential design remains blocked on the OpenShell/OpenClaw runtime architecture and threat-model decision.
Related Issue
Refs #8693
Changes
OPENCLAW_GATEWAY_TOKENwith the absolute/usr/bin/envboundary before privilege step-down or OpenClaw execution.Type of Change
Quality Gates
/usr/bin/envis absolute to avoid sandbox-userPATHsubstitution;"$@"preserves argv boundaries; only the ambient token is removed; launch PID identity, privilege step-down, redirection modes, restart accounting, and fail-closed invalid-mode handling are preserved. Real-shell tests prove the credential is absent in both launch modes and no process starts for an invalid mode. No new secret, auth bypass, dependency, configuration weakening, or cryptographic behavior is introduced.Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and the commit appears as Verified in GitHubnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests