fix(state): create the missing sessions carveout during state-dir lock - #7602
Merged
apurvvkumaria merged 4 commits intoJul 27, 2026
Merged
Conversation
The state-dir guard honored the writable agents/<id>/sessions carveout only when the directory already existed at lock time. An agent booting for the first time under an active lock found its parent directory root-owned and read-only, so its own mkdir of the sessions directory failed with EACCES and session writes stayed impossible while shields were up. Lock now converges each agent directory on a usable carveout. The check runs after the entry loop so an agent whose unsafe entry named sessions was just removed is also covered. A missing sessions entry is created descriptor-relative with O_NOFOLLOW verification and receives the existing carveout posture (sandbox:sandbox 2770). A name that appears between the existence check and the mkdir fails the lock closed with a carveout-create-failed issue, matching the guard's raced-entry policy. A surviving non-directory entry named sessions keeps its locked posture. Refs #7545 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Advisory documentation-writer findings on the carveout change: split the long lockdown sentence and put the condition first, name _mutate_dir as the caller in the helper docstring, make the lock the subject of the fail-closed sentence, and drop the ownership verb from the predicate docstring where it collided with filesystem-ownership vocabulary. Refs #7545 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Dongni-Yang
added
the
area: security
Contributor
📝 WalkthroughWalkthroughLock transitions now create missing agent ChangesRuntime sessions carveout
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant _mutate_dir
participant _ensure_runtime_carveout
participant Filesystem
participant _set_dir_metadata
_mutate_dir->>_ensure_runtime_carveout: Process agent carveout during lock
_ensure_runtime_carveout->>Filesystem: Check or create sessions directory
_ensure_runtime_carveout->>_set_dir_metadata: Apply unlock metadata
_set_dir_metadata-->>_mutate_dir: Return configured carveout
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 36d1657 in the TypeScript / code-coverage/cliThe overall coverage in commit 36d1657 in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7602.docs.buildwithfern.com/nemoclaw |
![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/state-dir-guard.test.ts`:
- Around line 915-916: Update the assertions for stagedSessions to inspect the
symlink target with statSync rather than lstatSync, so isDirectory() and the
expected 0o2770 mode validate the target directory. Preserve the test’s coverage
that the lock retains an existing non-directory sessions entry.
🪄 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: ec135b0e-4315-4005-817b-02a9a461c52e
📒 Files selected for processing (3)
docs/security/best-practices.mdxscripts/state-dir-guard.pytest/state-dir-guard.test.ts
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
23 tasks
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
…essions entry Advisor warning PRA-1 on this PR: _ensure_runtime_carveout returns when any sessions entry survives the lock. Its contract says a surviving non-directory entry keeps its locked posture, but no test covered that. This test creates agents/main/sessions as a regular file and runs lock. It asserts the entry survives as a locked regular file on a fresh inode instead of being replaced by a writable carveout. On relock it asserts the entry stays a locked regular file. The guard already satisfies the contract, so this test-only commit protects the contract against regressions rather than reproducing a defect. Refs #7545 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
Author
|
Addressed advisor warning PRA-1 in 36d1657: Signed-off-by: Dongni Yang dongniy@nvidia.com |
apurvvkumaria
self-requested a review
July 27, 2026 04:44
apurvvkumaria
approved these changes
Jul 27, 2026
23 tasks
prekshivyas
pushed a commit
that referenced
this pull request
Jul 27, 2026
<!-- markdownlint-disable MD041 --> ## Summary `docs/changelog/2026-07-25.mdx` now updates the `v0.0.96` release entry for the state-lock fix that merged after #7564. The entry identifies the first-boot sessions carveout behavior and links to Security Best Practices without claiming that the separate legacy credentials symptom is fixed. ## Changes - [#7602](#7602) -> `docs/changelog/2026-07-25.mdx`: Document the first-boot `agents/<id>/sessions/` carveout fix in the `v0.0.96` release entry and link to the deeper security guidance. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This PR changes release-entry prose only. The changelog contract test and Fern validation cover the dated entry, route, and rendering requirements. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] 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: At exact PR head `f445009d2`, a Codex Desktop documentation writer reviewed `docs/changelog/2026-07-25.mdx` against `WRITING.md` and `docs/CONTRIBUTING.md`. The review confirmed the source claim matches #7602, limits the release meaning to the first-boot sessions-directory `EACCES` path, uses the specific published Security Best Practices route, and changes no code samples or skip terms. The changelog test passed 6/6, and the docs build completed with 0 errors and 2 unrelated repository warnings. - Agent: Codex Desktop <!-- docs-review-head-sha: f445009 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) — exited 0 with 0 errors and 2 unrelated repository warnings. - [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) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded the v0.0.96 changelog with details on first-boot sandbox handling and prevention of permission errors. * Clarified CLI error behavior, exit statuses, configuration guidance, and messaging safeguards. * Added a Security Best Practices reference to the supporting documentation links. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
apurvvkumaria
added a commit
that referenced
this pull request
Aug 5, 2026
…group (#7603) <!-- markdownlint-disable MD041 --> ## Summary With shields up, the state-dir guard locked the confidentiality directories (`credentials`, `identity`, `pairing`) to `root:root 700`, so OpenClaw's startup lstat of the optional legacy `credentials/oauth.json` failed with `EACCES` instead of `ENOENT` — the gateway restart then failed with `GATEWAY_HEALTH_TIMEOUT` and Telegram dispatch kept retrying (#7545). Lock now sets only the confidentiality root to `root:sandbox 710` (group execute, no read): a probe for a missing top-level name resolves as missing, while the directory stays unlistable and nested directories and every file keep the sealed `root:root` posture with no group or world bits. ## Related Issue Closes #7545 — together with #7602 (the sessions-carveout half, `Refs`), this resolves both filed symptoms. If this PR merges first, #7602 still tracks the remaining `EACCES: mkdir agents/main/sessions` symptom on the issue's thread. ## Changes - `scripts/state-dir-guard.py`: an `is_confidentiality_root` flag threads through `_expected_ids`, `_expected_dir_mode`, `_set_dir_metadata`, `_verify_metadata`, and `_verify_dir`. During lock, the mutation pass and the independent verification pass both expect `root:sandbox 710` on the confidentiality root and the unchanged `root:root 700` on nested confidentiality directories. Confidentiality file modes and ownership are untouched (`old_mode & 0o700`, `root:root`), so the live E2E contract for locked credential files (`uid 0, gid 0, 0600` in `test/e2e/live/state-dir-guard-metadata.test.ts`) holds without modification. Scoping the group-execute bit to the root keeps the widened surface to one directory level of known-name metadata — resolving the `oauth.json` probe needs search permission on `credentials/` alone. - `test/state-dir-guard.test.ts`: the distinct-modes test now pins the root at `0o710` and a nested `credentials/providers/` directory at `0o700`; a new driver runs the guard with a distinct sandbox gid (a supplementary group of the current user, skipped when none exists) and asserts the group ownership lands only on the confidentiality root, not on nested directories or files. Both fail on the pre-change guard. - `test/e2e/live/state-dir-guard-metadata.test.ts`: the real container probe asserts a distinct sandbox-group member gets `ENOENT` for a missing direct child while root listing, nested traversal, and secret reads all remain `EACCES`. - `docs/security/best-practices.mdx`: the secret-bearing-directories paragraph now states the root/nested split, the known-name metadata visibility, and the missing-probe behavior. ## 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: nine-category security review PASS for code revision `c2553b7d1`; the subsequent `3f8bba830` change is documentation-only. No secrets, dependency, authentication, cryptography, network, or logging surface was added; the root-only `0710` scope is mirrored by independent verification and negative traversal/read tests. - [ ] 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: OpenClaw and Hermes confidentiality-root behavior reviewed; `npm run docs` completed with 0 errors and 2 existing Fern warnings; terminology and style scans passed - Agent: Codex Desktop documentation writer subagent <!-- docs-review-head-sha: 3f8bba8 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project integration test/state-dir-guard.test.ts` → 31 passed / 1 host-capability skip; disposable Linux DAC probe confirmed root `root:sandbox 0710`, nested directory `root:root 0700`, secret `root:root 0600`, direct-child `ENOENT`, and listing/nested/read `EACCES`; `npm run typecheck:cli`, `npm run docs`, and `npx prek run --from-ref origin/main --to-ref HEAD` passed - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) --- Signed-off-by: Dongni Yang <dongniy@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Updated lockdown behavior for secret-bearing confidentiality directories: the confidentiality root is now set to `root:sandbox` with restricted directory permissions, while nested directories and files are locked to `root:root` with no group/world access. * Probes now report missing immediate entries as “missing” rather than permission failures. * **Bug Fixes** * Improved metadata verification to treat the confidentiality root differently from nested confidentiality entries. * **Tests** * Expanded unit and e2e coverage, including nested permission checks and new confidentiality-root access evidence. * **Documentation** * Refreshed security best practices to match the updated lockdown rules. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@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
2 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
With shields up, the state-dir guard honored the writable
agents/<id>/sessionscarveout only when the directory already existed at lock time, so an agent booting for the first time under an active lock could not create it — itsmkdirunder the root-owned, read-only agent directory failed withEACCES(themkdir '/sandbox/.openclaw/agents/main/sessions'failure in #7545). Lock now creates the carveout when the agent directory has nosessionsentry left after containment, using the same descriptor-relative, fail-closed discipline as the rest of the guard.Related Issue
Refs #7545 — this PR resolves the sessions-carveout
EACCESsymptom. The other symptom (EACCESonlstatof a missing legacycredentials/oauth.jsonbecause the confidentiality lock makes the directory non-traversable) is a separate decision: either OpenClaw treatsEACCESon that optional file as absent, or the confidentiality-dir posture changes, and both need maintainer direction.Changes
scripts/state-dir-guard.py: new_is_runtime_carveout_parentpredicate (exactlyagents/<name>) and_ensure_runtime_carveouthelper. During lock,_mutate_dircalls it after the entry loop, so an agent whose unsafe entry namedsessionswas just removed also converges on a created carveout. The created directory ismkdired descriptor-relative, re-opened withO_NOFOLLOWand a dev/ino race check, given the existing carveout posture (sandbox:sandbox2770), and charged to the mutationWorkBudget. A name that appears between the existence check and themkdirfails the lock closed with a newcarveout-create-failedissue. A surviving non-directory entry namedsessionskeeps its locked posture. Unlock is unchanged.test/state-dir-guard.test.ts: a lock test covering creation for a first-boot agent, convergence after an unsafe staged symlink namedsessionsis removed, no creation under nested agent subdirectories, non-agent directories, or theagents/root, relock idempotence, and the posture surviving unlock; plus a deterministic race test (monkeypatchedos.mkdir) asserting thecarveout-create-failedfail-closed path. Both tests fail on the pre-fix guard.docs/security/best-practices.mdx: one sentence in the shields lockdown section documenting the creation behavior.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/_buildis gitignored)DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/state-dir-guard.test.ts→ 29/29 pass; both new tests fail with the pre-fix guard (red→green verified);npx vitest run --project cli src/lib/shields/state-dir-lock.test.ts→ 4/4 passnpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only) — not doc-only; build ran with 0 errors and 2 warnings not attributable to this diffSigned-off-by: Dongni Yang dongniy@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
sessionscarve-out for agents missing asessionsentry, enabling first boot under an active lock.Bug Fixes
sessionspaths, including clearer validation when directory creation fails.sessionscarve-out setup, covering race scenarios and cases wheresessionsexists as a file (not a directory).