Skip to content

fix(security): surface immutable symlink hardening status - #1499

Merged
cv merged 2 commits into
NVIDIA:mainfrom
13ernkastel:codex/pr-1137-hardening-observability-mainline
Apr 6, 2026
Merged

fix(security): surface immutable symlink hardening status#1499
cv merged 2 commits into
NVIDIA:mainfrom
13ernkastel:codex/pr-1137-hardening-observability-mainline

Conversation

@13ernkastel

@13ernkastel 13ernkastel commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

Summary

This follow-up builds on #1137 and improves the observability around immutable symlink hardening without changing the underlying defense-in-depth approach.

What Changed

  • factors .openclaw symlink validation into a reusable helper so both startup paths use the same validation logic
  • adds explicit security logging when immutable hardening succeeds, is partial, or is skipped because chattr is unavailable
  • extends the gateway-isolation E2E to fail if chattr is missing from the image, so the mitigation cannot silently disappear

Why

The original immutable-hardening fix is directionally strong, but the chattr path is intentionally best-effort and currently silent. That makes the mitigation harder to trust and harder to debug because:

  • a missing chattr binary looks the same as successful hardening
  • partial chattr +i failures are suppressed with no visibility
  • the image can regress and stop shipping chattr without CI catching it

These changes make the mitigation easier to audit while staying compatible with the current layered hardening model.

Validation

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

Relationship To #1137

This is a repost of the follow-up originally opened as latenighthackathon/NemoClaw#1, now targeted at NVIDIA/NemoClaw as requested.

Note

This replaces #1467, which GitHub auto-closed because the repository's contributor open-PR limit was hit at the time.

Signed-off-by: 13ernkastel LennonCMJ@live.com

Summary by CodeRabbit

  • Chores

    • Enhanced startup process validation to ensure system integrity and correct configuration
    • Improved security hardening mechanisms with comprehensive logging and graceful fallback handling when system features are unavailable
  • Tests

    • Updated end-to-end integration tests to verify system hardening capabilities and feature availability

@coderabbitai

coderabbitai Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

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: ebe299c6-fd83-4e71-b381-167f1efd5fdb

📥 Commits

Reviewing files that changed from the base of the PR and between 2842c2f and d1c4baa.

📒 Files selected for processing (2)
  • scripts/nemoclaw-start.sh
  • test/e2e-gateway-isolation.sh
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/e2e-gateway-isolation.sh
  • scripts/nemoclaw-start.sh

📝 Walkthrough

Walkthrough

Added symlink validation and conditional hardening to the startup script (scripts/nemoclaw-start.sh) and adjusted e2e test ordering to check for chattr availability before running the sandbox-vs-gateway kill/isolation test (test/e2e-gateway-isolation.sh).

Changes

Cohort / File(s) Summary
Symlink Validation & Hardening
scripts/nemoclaw-start.sh
Introduced validate_openclaw_symlinks() to verify resolved symlink targets under /sandbox/.openclaw/* and fail startup if mismatched; added harden_openclaw_symlinks() to attempt chattr +i hardening with detailed logging and tolerant failure behavior; replaced inline validation/hardening with these function calls in both root and non-root startup paths.
Test Reordering & Precondition
test/e2e-gateway-isolation.sh
Replaced previous Test 11 with a new precondition that checks for chattr availability as root; moved the sandbox-vs-gateway kill/isolation check to Test 12; renumbered the capabilities/bounding-set validation test label to Test 13 (logic unchanged).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Init as Startup Script
  participant Validator as validate_openclaw_symlinks()
  participant FS as /sandbox/.openclaw (filesystem)
  participant Hardener as harden_openclaw_symlinks()
  participant Chattr as `chattr` binary

  Init->>Validator: invoke (non-root & root paths)
  Validator->>FS: read symlinks (readlink -f)
  FS-->>Validator: resolved targets
  Validator-->>Init: success/fail (return 0/1)

  alt hardening available
    Init->>Hardener: invoke (root path)
    Hardener->>Chattr: check availability (command -v)
    Chattr-->>Hardener: exists
    Hardener->>FS: apply chattr +i to files/dirs
    FS-->>Hardener: success/failure per path
    Hardener-->>Init: log counts, return 0 (tolerant)
  else chattr missing
    Hardener->>Chattr: not found
    Hardener-->>Init: log and return 0 (skip hardening)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through links both old and new,

I checked each path to see what's true,
I asked the chattr to stand and stay,
If it won't, I still let startup play,
A little hop for safer day.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring symlink validation into reusable functions and adding explicit logging for immutable symlink hardening status to improve observability.

✏️ 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

Copy link
Copy Markdown
Contributor Author

@cv this is open upstream now: #1499.

#1467 would not reopen cleanly after the auto-close, so I recreated the PR from the same branch once a contributor PR slot was free.

@13ernkastel
13ernkastel force-pushed the codex/pr-1137-hardening-observability-mainline branch from 2842c2f to d1c4baa Compare April 5, 2026 23:39
@13ernkastel

Copy link
Copy Markdown
Contributor Author

Maintainers: this PR has been rebased onto current main and is now mergeable, but the refreshed GitHub Actions runs are blocked in action_required and do not appear to have started jobs yet. It looks like this fork PR may need workflow approval before pr, commit-lint, and dco-check can run. Could someone please approve the runs so the checks can execute?

@cv
cv merged commit 2c40b46 into NVIDIA:main Apr 6, 2026
8 checks passed
tranzmatt pushed a commit to tranzmatt/NemoClaw that referenced this pull request Apr 6, 2026
## Summary

This follow-up builds on NVIDIA#1137 and improves the observability around
immutable symlink hardening without changing the underlying
defense-in-depth approach.

## What Changed

- factors `.openclaw` symlink validation into a reusable helper so both
startup paths use the same validation logic
- adds explicit security logging when immutable hardening succeeds, is
partial, or is skipped because `chattr` is unavailable
- extends the gateway-isolation E2E to fail if `chattr` is missing from
the image, so the mitigation cannot silently disappear

## Why

The original immutable-hardening fix is directionally strong, but the
`chattr` path is intentionally best-effort and currently silent. That
makes the mitigation harder to trust and harder to debug because:

- a missing `chattr` binary looks the same as successful hardening
- partial `chattr +i` failures are suppressed with no visibility
- the image can regress and stop shipping `chattr` without CI catching
it

These changes make the mitigation easier to audit while staying
compatible with the current layered hardening model.

## Validation

- `bash -n scripts/nemoclaw-start.sh`
- `bash -n test/e2e-gateway-isolation.sh`
- `git diff --check`
- not run: `test/e2e-gateway-isolation.sh` (`docker` is not installed in
this environment)

## Relationship To NVIDIA#1137

This is a repost of the follow-up originally opened as
`latenighthackathon#1`, now targeted at `NVIDIA/NemoClaw` as
requested.

## Note

This replaces `NVIDIA#1467`, which GitHub auto-closed because the repository's
contributor open-PR limit was hit at the time.

Signed-off-by: 13ernkastel <LennonCMJ@live.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Enhanced startup process validation to ensure system integrity and
correct configuration
* Improved security hardening mechanisms with comprehensive logging and
graceful fallback handling when system features are unavailable

* **Tests**
* Updated end-to-end integration tests to verify system hardening
capabilities and feature availability

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Carlos Villela <cvillela@nvidia.com>
gemini2026 pushed a commit to gemini2026/NemoClaw that referenced this pull request Apr 14, 2026
## Summary

This follow-up builds on NVIDIA#1137 and improves the observability around
immutable symlink hardening without changing the underlying
defense-in-depth approach.

## What Changed

- factors `.openclaw` symlink validation into a reusable helper so both
startup paths use the same validation logic
- adds explicit security logging when immutable hardening succeeds, is
partial, or is skipped because `chattr` is unavailable
- extends the gateway-isolation E2E to fail if `chattr` is missing from
the image, so the mitigation cannot silently disappear

## Why

The original immutable-hardening fix is directionally strong, but the
`chattr` path is intentionally best-effort and currently silent. That
makes the mitigation harder to trust and harder to debug because:

- a missing `chattr` binary looks the same as successful hardening
- partial `chattr +i` failures are suppressed with no visibility
- the image can regress and stop shipping `chattr` without CI catching
it

These changes make the mitigation easier to audit while staying
compatible with the current layered hardening model.

## Validation

- `bash -n scripts/nemoclaw-start.sh`
- `bash -n test/e2e-gateway-isolation.sh`
- `git diff --check`
- not run: `test/e2e-gateway-isolation.sh` (`docker` is not installed in
this environment)

## Relationship To NVIDIA#1137

This is a repost of the follow-up originally opened as
`latenighthackathon#1`, now targeted at `NVIDIA/NemoClaw` as
requested.

## Note

This replaces `NVIDIA#1467`, which GitHub auto-closed because the repository's
contributor open-PR limit was hit at the time.

Signed-off-by: 13ernkastel <LennonCMJ@live.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Enhanced startup process validation to ensure system integrity and
correct configuration
* Improved security hardening mechanisms with comprehensive logging and
graceful fallback handling when system features are unavailable

* **Tests**
* Updated end-to-end integration tests to verify system hardening
capabilities and feature availability

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: Carlos Villela <cvillela@nvidia.com>
cv added a commit that referenced this pull request Apr 17, 2026
…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>
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants