fix(entrypoint): relax config permissions before write after CAP_DAC_OVERRIDE drop

[jyaunches]

Summary

Fix the test/e2e/test-runtime-overrides.sh E2E crash at baseline config capture. After PR #917 drops CAP_DAC_OVERRIDE via capsh, root can no longer write to 444-mode config files. The three runtime override functions (apply_model_override, apply_cors_override, apply_slack_token_override) now temporarily relax permissions to 644 before writing and re-lock to 444 afterward, using CAP_FOWNER which is retained by design.

Related Issue

Fixes #2653

Changes

• scripts/lib/sandbox-init.sh: Add relax_config_for_write() / lock_config_after_write() shared helpers that chmod 644/chmod 444 with symlink guards
• scripts/nemoclaw-start.sh: Wrap python3 write + hash recomputation in all three override functions with the new helpers
• scripts/nemoclaw-start.sh: Tighten apply_model_override() trigger guard — only fire when NEMOCLAW_MODEL_OVERRIDE or NEMOCLAW_INFERENCE_API_OVERRIDE is explicitly set (Dockerfile ENV defaults no longer trigger spurious override runs)
• test/e2e/test-runtime-overrides.sh: Create timestamped log file matching CI artifact glob test-runtime-overrides-*.log, replace all 2>/dev/null with 2>>"$LOG_FILE"
• test/e2e/test-runtime-overrides.sh: Update tests 3-5, 9-11, 14 to pair standalone parameters with NEMOCLAW_MODEL_OVERRIDE (matching the tightened trigger guard)

Type of Change

• 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)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• make 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)

AI Disclosure

• AI-assisted — tool: Claude Code (pi agent)

---

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Overrides now only apply when explicit override flags are set, preventing unintended changes.
  • Safer config writes: permissions are temporarily relaxed with strict safety checks (rejecting unsafe targets, skipping missing files), clear security errors on failure, recompute verification only on successful writes, and permissions are always restored; write/hash failures are surfaced.

• Tests

  • Improved test logging with timestamped artifacts and preserved stderr; runtime override tests updated to cover model-override scenarios.

[coderabbitai]

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: c82375ce-1ca9-45f1-84b4-90134d03e70e

📥 Commits

Reviewing files that changed from the base of the PR and between 5965c0d and b8f4032.

📒 Files selected for processing (2)

• scripts/nemoclaw-start.sh
• test/nemoclaw-start.test.ts

---

📝 Walkthrough

Walkthrough

Adds two sandbox permission helper functions, uses them to safely relax and re-lock config file modes around runtime override writers, narrows model-override gating, and routes Docker/validation stderr into timestamped CI test logs.

Changes

Cohort / File(s)	Summary
Config Permission Management scripts/lib/sandbox-init.sh	Adds relax_config_for_write() and lock_config_after_write() to validate targets (reject symlinks), ignore missing regular files, emit [SECURITY] warnings on violations, and set modes to 644 before writes and 444 after.
Runtime Override Logic scripts/nemoclaw-start.sh	Restricts apply_model_override to run only when NEMOCLAW_MODEL_OVERRIDE or NEMOCLAW_INFERENCE_API_OVERRIDE are set. Wraps override writers (apply_model_override, apply_cors_override, apply_slack_token_override) with the permission helpers, captures writer and hash exit codes, recomputes SHA256 only on successful writes, and always re-locks; functions return non-zero when write/hash fail.
E2E Test Logging & Env Requirements test/e2e/test-runtime-overrides.sh	Creates timestamped test-runtime-overrides-*.log and appends Docker stderr and validation stderr to it for CI visibility. Adjusts tests to include NEMOCLAW_MODEL_OVERRIDE=test in validation/rejection scenarios and updates related descriptions.
Unit Test Guard Adjustment test/nemoclaw-start.test.ts	Narrows apply_model_override() guard tests to assert only NEMOCLAW_MODEL_OVERRIDE or NEMOCLAW_INFERENCE_API_OVERRIDE trigger the patch; adds negative checks that NEMOCLAW_CONTEXT_WINDOW, NEMOCLAW_MAX_TOKENS, and NEMOCLAW_REASONING are not referenced in the early-return guard.

Sequence Diagram(s)

sequenceDiagram
    participant Starter as "nemoclaw-start.sh"
    participant SandboxLib as "sandbox-init.sh helpers"
    participant Writer as "inline Python writer"
    participant FS as "filesystem (/sandbox/.openclaw/openclaw.json)"
    participant Hasher as "SHA256 recompute"

    Starter->>SandboxLib: relax_config_for_write(target file)
    SandboxLib-->>Starter: success / failure (stderr [SECURITY] on violations)
    alt relax succeeded
        Starter->>Writer: invoke writer (apply_*_override)
        Writer-->>Starter: exit status
        alt writer succeeded
            Starter->>Hasher: recompute SHA256 of FS file
            Hasher-->>Starter: hash value / status
        else writer failed
            Starter-->>Starter: record non-zero write status
        end
    end
    Starter->>SandboxLib: lock_config_after_write(target file)
    SandboxLib-->>Starter: success / failure (stderr [SECURITY] on violations)
    Starter-->>Caller: return captured status (0 or non-zero)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I nudged the locks and peeked inside,

I set them loose where writers bide,
No symlinks slipped, no secrets flown,
I sealed them back when work was done.
Logs purr softly — tidy trail, well-done!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding permission-relaxation helpers to fix a crash caused by dropping CAP_DAC_OVERRIDE capability, which is the core objective of the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
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.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-2653-runtime-overrides-e2e-crash

---

_{Review rate limit: 9/10 reviews remaining, refill in 6 minutes.}

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

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

scripts/lib/sandbox-init.sh (1)

170-170: Consider propagating chmod failures.

The chmod commands don't check their return status. If chmod fails (e.g., filesystem error, unexpected ownership), the function silently continues. While rare, this could lead to confusing behavior where the caller believes permissions were changed but they weren't.

♻️ Optional: propagate chmod failures

 relax_config_for_write() {
   local f
   for f in "$@"; do
     if [ -L "$f" ]; then
       printf '[SECURITY] Refusing to relax permissions — %s is a symlink\n' "$f" >&2
       return 1
     fi
     [ -f "$f" ] || continue
-    chmod 644 "$f"
+    if ! chmod 644 "$f"; then
+      printf '[SECURITY] Failed to relax permissions on %s\n' "$f" >&2
+      return 1
+    fi
   done
 }

 lock_config_after_write() {
   local f
   for f in "$@"; do
     if [ -L "$f" ]; then
       printf '[SECURITY] Refusing to lock permissions — %s is a symlink\n' "$f" >&2
       return 1
     fi
     [ -f "$f" ] || continue
-    chmod 444 "$f"
+    if ! chmod 444 "$f"; then
+      printf '[SECURITY] Failed to lock permissions on %s\n' "$f" >&2
+      return 1
+    fi
   done
 }

Also applies to: 182-182

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/lib/sandbox-init.sh` at line 170, The chmod invocations (e.g., the
line containing chmod 644 "$f" and the other chmod calls around the same area)
currently ignore failures; update the surrounding code to check chmod's exit
status and propagate errors instead of continuing silently — after each chmod,
if it fails emit an explanatory error to stderr via the function's logger and
return a non‑zero status (or exit non‑zero if the script is intended to
terminate), so callers can detect and handle permission-change failures.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 366-368: The relax_config_for_write call currently isn't paired
with a guaranteed re-lock; wrap the section that runs the Python override and
the subsequent hash recomputation so lock_config_after_write is always invoked
even on error by installing a trap immediately after relax_config_for_write
(similar to the pattern used around apply_model_override): set a trap to call
lock_config_after_write with the same arguments on EXIT (or ERR), run the Python
script and recompute_hash, then clear the trap once successful; apply the same
trap/clear pattern to the later relax_config_for_write block that surrounds the
388-390 operations so both blocks use lock_config_after_write as an error-safe
finally.
- Around line 276-278: After calling relax_config_for_write, ensure
lock_config_after_write is always invoked on failure or exit by installing a
trap (or explicit error handling) before running the Python script and hash
recomputation so that lock_config_after_write "$config_file" "$hash_file" runs
on ERR/EXIT; remove or disable the trap only after successful completion of the
Python script and the hash recomputation (and before the later chattr
+i/Landlock steps) to avoid leaving files at 644 if the Python script or hash
step fails; apply the same pattern around the second relax/lock pair (the
321–323 code path) so both relax_config_for_write/lock_config_after_write pairs
are guarded.
- Around line 447-449: The relax_config_for_write call that temporarily loosens
permissions for the Slack token override should be protected with a trap-based
cleanup so permissions are always re-locked even if the subsequent Python script
or hash recomputation fails: before calling relax_config_for_write (used for the
slack token override) register a trap that will call the inverse/relock routine
(undoing relax_config_for_write’s changes) on EXIT/ERR, then run the Python
script and hash recompute, and finally remove or replace the trap on successful
completion; apply the exact same trap-wrapping pattern to the other occurrence
of relax_config_for_write mentioned in the review.

---

Nitpick comments:
In `@scripts/lib/sandbox-init.sh`:
- Line 170: The chmod invocations (e.g., the line containing chmod 644 "$f" and
the other chmod calls around the same area) currently ignore failures; update
the surrounding code to check chmod's exit status and propagate errors instead
of continuing silently — after each chmod, if it fails emit an explanatory error
to stderr via the function's logger and return a non‑zero status (or exit
non‑zero if the script is intended to terminate), so callers can detect and
handle permission-change failures.

🪄 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: 0684cdf1-e5dc-4f13-93b8-28f83dc966fc

📥 Commits

Reviewing files that changed from the base of the PR and between 9113dca and 956f773.

📒 Files selected for processing (3)

• scripts/lib/sandbox-init.sh
• scripts/nemoclaw-start.sh
• test/e2e/test-runtime-overrides.sh

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Same concern: Slack token override lacks error-safe re-locking.

The pattern repeats here. For defense-in-depth, consider the same trap-based approach to ensure permissions are re-locked even if the Python script or hash recomputation fails.

Also applies to: 484-486

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/nemoclaw-start.sh` around lines 447 - 449, The relax_config_for_write
call that temporarily loosens permissions for the Slack token override should be
protected with a trap-based cleanup so permissions are always re-locked even if
the subsequent Python script or hash recomputation fails: before calling
relax_config_for_write (used for the slack token override) register a trap that
will call the inverse/relock routine (undoing relax_config_for_write’s changes)
on EXIT/ERR, then run the Python script and hash recompute, and finally remove
or replace the trap on successful completion; apply the exact same trap-wrapping
pattern to the other occurrence of relax_config_for_write mentioned in the
review.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 320-324: The success message is printed even when sha256sum fails
because failure is swallowed by "|| _write_rc=$?" but printf is unconditional;
change the pattern so the printf only runs on successful sha256sum. Replace the
current construct (cd ... && sha256sum ... >"$hash_file") || _write_rc=$?;
printf ... with an if/then: if (cd /sandbox/.openclaw && sha256sum openclaw.json
>"$hash_file"); then printf '[SECURITY] Config hash recomputed after model
override\n' >&2; else _write_rc=$?; fi, and apply the same fix to the analogous
sites in apply_cors_override and apply_slack_token_override so the success log
is emitted only on true success.
- Around line 196-203: Update the test titled "triggers on any override env
var..." to match the new guard semantics: assert that setting either
NEMOCLAW_MODEL_OVERRIDE or NEMOCLAW_INFERENCE_API_OVERRIDE causes the config
patch to run, and remove the expectation that NEMOCLAW_CONTEXT_WINDOW,
NEMOCLAW_MAX_TOKENS, or NEMOCLAW_REASONING alone should trigger it; add/adjust
cases to explicitly verify (a) model override triggers, (b) inference API
override triggers, and (c) the other three vars by themselves do not trigger the
guard.

🪄 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: c60aebfb-2bd2-45af-bcd7-6ddc091dbba9

📥 Commits

Reviewing files that changed from the base of the PR and between 956f773 and 5965c0d.

📒 Files selected for processing (2)

• scripts/lib/sandbox-init.sh
• scripts/nemoclaw-start.sh

🚧 Files skipped from review as they are similar to previous changes (1)

• scripts/lib/sandbox-init.sh