fix(state): heal ~/.nemoclaw directory and file permissions on read path#4628
Merged
cv merged 12 commits intoJun 4, 2026
Merged
Conversation
…VIDIA#4546) readConfigFile() did not call ensureConfigDir(), so read-only CLI commands (e.g. nemoclaw list) never triggered permission drift healing. If a user manually changed ~/.nemoclaw to 755, only a write operation would repair it. - Call ensureConfigDir() in readConfigFile() to heal parent directory permissions (755 → 700) on every config access - After a successful read, check file-level permissions and tighten group/world bits (e.g. 644 → 600) - Add tests for both directory and file permission healing on read Fixes NVIDIA#4546 Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughScoped predicates and a new healer tighten host config directory (700) and root-level files (600); readConfigFile now ensures parent dir, parses JSON, and best-effort tightens the file. Tests add sibling-heal and symlink-safety coverage and verify scope boundaries using a temporary HOME. ChangesConfig permission healing
Sequence DiagramsequenceDiagram
participant Caller
participant readConfigFile
participant ensureConfigDir
participant healRootLevelFiles
participant FS
Caller->>readConfigFile: request read
readConfigFile->>ensureConfigDir: ensure/validate parent dir
ensureConfigDir->>healRootLevelFiles: scan root entries
healRootLevelFiles->>FS: lstat / chmod on regular files (skip symlinks)
readConfigFile->>FS: read file (may ENOENT)
readConfigFile->>FS: lstat + chmod on file (best-effort, skip symlinks)
FS-->>readConfigFile: file contents / status
readConfigFile-->>Caller: parsed JSON (or fallback)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
✨ Thanks for submitting this detailed PR about fixing the permission drift issue in the ~/.nemoclaw directory and file permissions. This proposes a way to heal the directory and file permissions on every config access, not just writes. Related open issues: |
…e being read Extends the read-path permission healing to cover every root-level file in the config dir, addressing the full acceptance criteria of NVIDIA#4546. The original PR fixed the directory perm drift (755 → 700) and tightened sandboxes.json (the only file that flows through readConfigFile), but left the other root-level files documented in the issue (ollama-auth-proxy.pid, onboard-session.json, ollama-proxy-token, usage-notice.json) unhealed because they are written by code paths that do not go through config-io. Add a healRootLevelFiles helper called from ensureConfigDir. The walk uses lstat so a planted symlink inside ~/.nemoclaw does not get chmodded through to a target outside the config dir. Subdirectories are skipped — heal stays root-level only, matching the issue's scope. Also harden the existing per-file heal in readConfigFile with the same lstat guard so reading through a symlink does not mutate the link target. Tests: - ensureConfigDir heals every root-level file in the dir (multi-sibling case mirroring the issue's reproducer file list) - ensureConfigDir skips symlinks during the root-level heal (lstat guard) - readConfigFile does not chmod through a symlink even via the per-file heal (defensive duplicate) 16/16 tests pass. Co-authored-by: kagura-agent <kagura.agent.ai@gmail.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cjagwani
approved these changes
Jun 3, 2026
Contributor
There was a problem hiding this comment.
Approving. Pushed one follow-up commit (2e56702) to widen the heal to all root-level files in ~/.nemoclaw/ and close the symlink hole. full context:
- File coverage: #4546's acceptance criteria call out 5 root-level files (
sandboxes.json,onboard-session.json,ollama-proxy-token,ollama-auth-proxy.pid,usage-notice.json). The original PR healedsandboxes.jsonviareadConfigFile's per-file tighten — the other four are written by code paths that don't flow throughreadConfigFile/writeConfigFile(e.g.inference/ollama/proxy.ts,state/onboard-session.ts,onboard/usage-notice.ts). Added ahealRootLevelFileswalk inensureConfigDirso every nemoclaw invocation that touches the config dir restores the entire root level to 600. - Symlink hole: both heal sites (
healRootLevelFilesandreadConfigFile's per-file tighten) now uselstatbeforechmodso a symlinked entry inside~/.nemoclaw/doesn't get chmodded through to a target outside the config dir.
Tests: 16/16 pass. Added 3 new — multi-sibling heal mirroring the issue's reproducer file list, plus two symlink-guard cases (one for the dir walk, one for the per-file heal).
Original directory healing logic + per-file heal kept intact — additive change only.
… regression guard The previous version passed even with the dir-walker reverted, because without the walker nothing chmods anything and the symlink target's mode trivially stays at 0o644 — a vacuous pass. Add a regular sibling at 0o644 in the same dir and assert it gets tightened to 0o600. The sibling proves the walker actually ran; the symlink-target assertion then proves the lstat guard skipped the link. Verified by reverting src/lib/state/config-io.ts to the parent of 2e56702 and confirming the test now fails with "positive control: walker tightened the regular sibling: expected 420 to be 384" (0o644 vs 0o600). Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
…IDIA#734/NVIDIA#735) CodeQL flagged the symlink-target paths in the two symlink-skip tests as insecure temp file creation (predictable `os.tmpdir() + pid` paths let a coresident attacker pre-create the target). Switch both to the existing `makeTempDir()` helper, which uses `mkdtempSync` for an unguessable directory name. Cleanup also simplifies to the afterEach hook since both temp dirs are now tracked in `tmpDirs`. Tests still pass (16/16). Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cv
reviewed
Jun 3, 2026
| * acceptance criteria (#4546). Best-effort: a single file's chmod failure | ||
| * does not abort the walk. | ||
| */ | ||
| function healRootLevelFiles(dirPath: string): void { |
Collaborator
There was a problem hiding this comment.
Scope the host-state permission contract explicitly before the helper. This gives us a cheap guardrail so the 700/600 healer does not become a generic permission normalizer for mutable sandbox config paths.
Suggested change
| function healRootLevelFiles(dirPath: string): void { | |
| function hostNemoclawDir(): string { | |
| const home = process.env.HOME ?? os.homedir(); | |
| return path.resolve(home, ".nemoclaw"); | |
| } | |
| function isHostNemoclawRoot(dirPath: string): boolean { | |
| return path.resolve(dirPath) === hostNemoclawDir(); | |
| } | |
| function isMutableSandboxConfigPath(targetPath: string): boolean { | |
| const resolved = path.resolve(targetPath); | |
| return resolved === "/sandbox/.openclaw" || resolved.startsWith("/sandbox/.openclaw/"); | |
| } | |
| function healRootLevelFiles(dirPath: string): void { |
cv
reviewed
Jun 3, 2026
|
|
||
| export function ensureConfigDir(dirPath: string): void { | ||
| // SECURITY: Block symlink attacks before creating or writing to the directory. | ||
| rejectSymlinksOnPath(dirPath); |
Collaborator
There was a problem hiding this comment.
Track whether this call is touching the mutable OpenClaw sandbox tree. That path has its own permission contract and should not be normalized to host-state defaults.
Suggested change
| rejectSymlinksOnPath(dirPath); | |
| rejectSymlinksOnPath(dirPath); | |
| const mutableSandboxPath = isMutableSandboxConfigPath(dirPath); |
cv
reviewed
Jun 3, 2026
Comment on lines
+218
to
+225
| // Heal every root-level file in the dir, not just the one being read. | ||
| // Issue #4546 expects auto-repair across all root-level files | ||
| // (sandboxes.json, onboard-session.json, ollama-auth-proxy.pid, | ||
| // ollama-proxy-token, usage-notice.json, etc.) — most of which are written | ||
| // by code paths that don't flow through readConfigFile/writeConfigFile. | ||
| // Doing the walk here means every nemoclaw invocation that touches the | ||
| // config dir restores the entire root level to mode 600. | ||
| healRootLevelFiles(dirPath); |
Collaborator
There was a problem hiding this comment.
Please scope the root-level sibling sweep to the exact host NemoClaw state root. This preserves #4546 while avoiding another #4538-style regression if a future caller routes a mutable sandbox config directory through ensureConfigDir(). When applying this, also update the directory-mode chmod above to skip mutableSandboxPath.
Suggested change
| // Heal every root-level file in the dir, not just the one being read. | |
| // Issue #4546 expects auto-repair across all root-level files | |
| // (sandboxes.json, onboard-session.json, ollama-auth-proxy.pid, | |
| // ollama-proxy-token, usage-notice.json, etc.) — most of which are written | |
| // by code paths that don't flow through readConfigFile/writeConfigFile. | |
| // Doing the walk here means every nemoclaw invocation that touches the | |
| // config dir restores the entire root level to mode 600. | |
| healRootLevelFiles(dirPath); | |
| if (isHostNemoclawRoot(dirPath)) { | |
| // Heal every root-level file in the host NemoClaw state dir, not just the | |
| // one being read. Issue #4546 expects auto-repair across root-level files | |
| // such as sandboxes.json, onboard-session.json, and ollama-proxy-token. | |
| // | |
| // Keep this scoped to ~/.nemoclaw. Mutable sandbox config paths such as | |
| // /sandbox/.openclaw deliberately use group-writable permissions. | |
| healRootLevelFiles(dirPath); | |
| } |
cv
reviewed
Jun 3, 2026
| // for read paths whose dirname differs from what ensureConfigDir saw. | ||
| try { | ||
| const st = fs.lstatSync(filePath); | ||
| if (st.isFile() && (st.mode & 0o077) !== 0) { |
Collaborator
There was a problem hiding this comment.
Same boundary on the per-file heal: do not chmod through the generic read path when the file is a mutable sandbox OpenClaw config file.
Suggested change
| if (st.isFile() && (st.mode & 0o077) !== 0) { | |
| if (!isMutableSandboxConfigPath(filePath) && st.isFile() && (st.mode & 0o077) !== 0) { |
cv
reviewed
Jun 3, 2026
Comment on lines
+165
to
+249
| it("ensureConfigDir heals every root-level file in the dir, not just the one being read (#4546)", () => { | ||
| // #4546 expects auto-repair across all root-level files. Most of those | ||
| // files (onboard-session.json, ollama-proxy-token, etc.) are written by | ||
| // code paths that don't flow through readConfigFile, so the read-time | ||
| // per-file heal alone misses them. The dir walk in ensureConfigDir is | ||
| // what covers them — verify by writing several siblings at 644 and | ||
| // confirming a single read tightens all of them to 600. | ||
| const dir = makeTempDir(); | ||
| fs.chmodSync(dir, 0o700); | ||
| const target = path.join(dir, "config.json"); | ||
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | ||
|
|
||
| const siblings = [ | ||
| "onboard-session.json", | ||
| "ollama-proxy-token", | ||
| "ollama-auth-proxy.pid", | ||
| "usage-notice.json", | ||
| ]; | ||
| for (const name of siblings) { | ||
| fs.writeFileSync(path.join(dir, name), "stale", { mode: 0o644 }); | ||
| } | ||
|
|
||
| readConfigFile(target, null); | ||
|
|
||
| for (const name of siblings) { | ||
| const mode = fs.statSync(path.join(dir, name)).mode & 0o777; | ||
| expect(mode, `${name} should be tightened to 600`).toBe(0o600); | ||
| } | ||
| }); | ||
|
|
||
| it("ensureConfigDir skips symlinks during the root-level heal", () => { | ||
| // A chmod on a symlink follows to the target — if ~/.nemoclaw/X is a | ||
| // symlink to /etc/passwd, healing must NOT chmod /etc/passwd. lstat | ||
| // before chmod keeps the heal scoped to real files inside the dir. | ||
| // | ||
| // Positive control: a regular sibling at 0o644 proves the walker | ||
| // actually ran (it should be tightened to 0o600). Without the | ||
| // control, this test would pass vacuously if the walker were a no-op. | ||
| const dir = makeTempDir(); | ||
| fs.chmodSync(dir, 0o700); | ||
| const target = path.join(dir, "config.json"); | ||
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | ||
|
|
||
| const sibling = path.join(dir, "should-be-healed.json"); | ||
| fs.writeFileSync(sibling, "stale", { mode: 0o644 }); | ||
|
|
||
| // Use mkdtempSync (via makeTempDir) for an unguessable outside path — | ||
| // a predictable os.tmpdir()+pid path is a CodeQL "insecure temporary | ||
| // file" pattern and lets a coresident attacker pre-create the target. | ||
| const outsideDir = makeTempDir(); | ||
| const outside = path.join(outsideDir, "target"); | ||
| fs.writeFileSync(outside, "outside", { mode: 0o644 }); | ||
| const linkPath = path.join(dir, "rogue-link"); | ||
| fs.symlinkSync(outside, linkPath); | ||
|
|
||
| readConfigFile(target, null); | ||
| expect( | ||
| fs.statSync(sibling).mode & 0o777, | ||
| "positive control: walker tightened the regular sibling", | ||
| ).toBe(0o600); | ||
| expect( | ||
| fs.statSync(outside).mode & 0o777, | ||
| "symlink target must not be chmodded through the link", | ||
| ).toBe(0o644); | ||
| // Cleanup of linkPath and outside happens via afterEach (both live | ||
| // inside dirs in tmpDirs). | ||
| }); | ||
|
|
||
| it("readConfigFile does not chmod through a symlink even via the per-file heal", () => { | ||
| // Defensive duplicate of the symlink check, this time for the per-file | ||
| // heal in readConfigFile itself (not the dir walk in ensureConfigDir). | ||
| const dir = makeTempDir(); | ||
| fs.chmodSync(dir, 0o700); | ||
|
|
||
| const outsideDir = makeTempDir(); | ||
| const outside = path.join(outsideDir, "target.json"); | ||
| fs.writeFileSync(outside, JSON.stringify({ outside: true }), { mode: 0o644 }); | ||
| const symlinkPath = path.join(dir, "config.json"); | ||
| fs.symlinkSync(outside, symlinkPath); | ||
|
|
||
| // Reading through the symlink should not chmod the target file. | ||
| readConfigFile(symlinkPath, null); | ||
| expect(fs.statSync(outside).mode & 0o777).toBe(0o644); | ||
| // Cleanup via afterEach (both dirs are tracked in tmpDirs). | ||
| }); |
Collaborator
There was a problem hiding this comment.
Please add regression coverage for the scope boundary. The important guarantee is: root-level sibling healing happens under the host ~/.nemoclaw root, but not in arbitrary config directories that may have a different permission contract.
Suggested change
| it("ensureConfigDir heals every root-level file in the dir, not just the one being read (#4546)", () => { | |
| // #4546 expects auto-repair across all root-level files. Most of those | |
| // files (onboard-session.json, ollama-proxy-token, etc.) are written by | |
| // code paths that don't flow through readConfigFile, so the read-time | |
| // per-file heal alone misses them. The dir walk in ensureConfigDir is | |
| // what covers them — verify by writing several siblings at 644 and | |
| // confirming a single read tightens all of them to 600. | |
| const dir = makeTempDir(); | |
| fs.chmodSync(dir, 0o700); | |
| const target = path.join(dir, "config.json"); | |
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | |
| const siblings = [ | |
| "onboard-session.json", | |
| "ollama-proxy-token", | |
| "ollama-auth-proxy.pid", | |
| "usage-notice.json", | |
| ]; | |
| for (const name of siblings) { | |
| fs.writeFileSync(path.join(dir, name), "stale", { mode: 0o644 }); | |
| } | |
| readConfigFile(target, null); | |
| for (const name of siblings) { | |
| const mode = fs.statSync(path.join(dir, name)).mode & 0o777; | |
| expect(mode, `${name} should be tightened to 600`).toBe(0o600); | |
| } | |
| }); | |
| it("ensureConfigDir skips symlinks during the root-level heal", () => { | |
| // A chmod on a symlink follows to the target — if ~/.nemoclaw/X is a | |
| // symlink to /etc/passwd, healing must NOT chmod /etc/passwd. lstat | |
| // before chmod keeps the heal scoped to real files inside the dir. | |
| // | |
| // Positive control: a regular sibling at 0o644 proves the walker | |
| // actually ran (it should be tightened to 0o600). Without the | |
| // control, this test would pass vacuously if the walker were a no-op. | |
| const dir = makeTempDir(); | |
| fs.chmodSync(dir, 0o700); | |
| const target = path.join(dir, "config.json"); | |
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | |
| const sibling = path.join(dir, "should-be-healed.json"); | |
| fs.writeFileSync(sibling, "stale", { mode: 0o644 }); | |
| // Use mkdtempSync (via makeTempDir) for an unguessable outside path — | |
| // a predictable os.tmpdir()+pid path is a CodeQL "insecure temporary | |
| // file" pattern and lets a coresident attacker pre-create the target. | |
| const outsideDir = makeTempDir(); | |
| const outside = path.join(outsideDir, "target"); | |
| fs.writeFileSync(outside, "outside", { mode: 0o644 }); | |
| const linkPath = path.join(dir, "rogue-link"); | |
| fs.symlinkSync(outside, linkPath); | |
| readConfigFile(target, null); | |
| expect( | |
| fs.statSync(sibling).mode & 0o777, | |
| "positive control: walker tightened the regular sibling", | |
| ).toBe(0o600); | |
| expect( | |
| fs.statSync(outside).mode & 0o777, | |
| "symlink target must not be chmodded through the link", | |
| ).toBe(0o644); | |
| // Cleanup of linkPath and outside happens via afterEach (both live | |
| // inside dirs in tmpDirs). | |
| }); | |
| it("readConfigFile does not chmod through a symlink even via the per-file heal", () => { | |
| // Defensive duplicate of the symlink check, this time for the per-file | |
| // heal in readConfigFile itself (not the dir walk in ensureConfigDir). | |
| const dir = makeTempDir(); | |
| fs.chmodSync(dir, 0o700); | |
| const outsideDir = makeTempDir(); | |
| const outside = path.join(outsideDir, "target.json"); | |
| fs.writeFileSync(outside, JSON.stringify({ outside: true }), { mode: 0o644 }); | |
| const symlinkPath = path.join(dir, "config.json"); | |
| fs.symlinkSync(outside, symlinkPath); | |
| // Reading through the symlink should not chmod the target file. | |
| readConfigFile(symlinkPath, null); | |
| expect(fs.statSync(outside).mode & 0o777).toBe(0o644); | |
| // Cleanup via afterEach (both dirs are tracked in tmpDirs). | |
| }); | |
| function withHome<T>(home: string, fn: () => T): T { | |
| const previous = process.env.HOME; | |
| process.env.HOME = home; | |
| try { | |
| return fn(); | |
| } finally { | |
| if (previous === undefined) { | |
| delete process.env.HOME; | |
| } else { | |
| process.env.HOME = previous; | |
| } | |
| } | |
| } | |
| it("ensureConfigDir heals every root-level file in the host state root, not just the one being read (#4546)", () => { | |
| // #4546 expects auto-repair across all root-level files. Most of those | |
| // files (onboard-session.json, ollama-proxy-token, etc.) are written by | |
| // code paths that don't flow through readConfigFile, so the read-time | |
| // per-file heal alone misses them. The dir walk in ensureConfigDir is | |
| // what covers them - verify by writing several siblings at 644 and | |
| // confirming a single read tightens all of them to 600. | |
| const home = makeTempDir(); | |
| const dir = path.join(home, ".nemoclaw"); | |
| fs.mkdirSync(dir, { mode: 0o700 }); | |
| const target = path.join(dir, "config.json"); | |
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | |
| const siblings = [ | |
| "onboard-session.json", | |
| "ollama-proxy-token", | |
| "ollama-auth-proxy.pid", | |
| "usage-notice.json", | |
| ]; | |
| for (const name of siblings) { | |
| fs.writeFileSync(path.join(dir, name), "stale", { mode: 0o644 }); | |
| } | |
| withHome(home, () => readConfigFile(target, null)); | |
| for (const name of siblings) { | |
| const mode = fs.statSync(path.join(dir, name)).mode & 0o777; | |
| expect(mode, `${name} should be tightened to 600`).toBe(0o600); | |
| } | |
| }); | |
| it("does not heal sibling files outside the host state root", () => { | |
| const home = makeTempDir(); | |
| const dir = path.join(home, "other-config"); | |
| fs.mkdirSync(dir, { mode: 0o700 }); | |
| const target = path.join(dir, "config.json"); | |
| const sibling = path.join(dir, "group-writable.json"); | |
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | |
| fs.writeFileSync(sibling, "keep-mutable", { mode: 0o660 }); | |
| fs.chmodSync(sibling, 0o660); | |
| withHome(home, () => readConfigFile(target, null)); | |
| expect(fs.statSync(sibling).mode & 0o777).toBe(0o660); | |
| }); | |
| it("ensureConfigDir skips symlinks during the root-level heal", () => { | |
| // A chmod on a symlink follows to the target - if ~/.nemoclaw/X is a | |
| // symlink to /etc/passwd, healing must NOT chmod /etc/passwd. lstat | |
| // before chmod keeps the heal scoped to real files inside the dir. | |
| // | |
| // Positive control: a regular sibling at 0o644 proves the walker | |
| // actually ran (it should be tightened to 0o600). Without the | |
| // control, this test would pass vacuously if the walker were a no-op. | |
| const home = makeTempDir(); | |
| const dir = path.join(home, ".nemoclaw"); | |
| fs.mkdirSync(dir, { mode: 0o700 }); | |
| const target = path.join(dir, "config.json"); | |
| fs.writeFileSync(target, JSON.stringify({ ok: true }), { mode: 0o600 }); | |
| const sibling = path.join(dir, "should-be-healed.json"); | |
| fs.writeFileSync(sibling, "stale", { mode: 0o644 }); | |
| const outsideDir = makeTempDir(); | |
| const outside = path.join(outsideDir, "target"); | |
| fs.writeFileSync(outside, "outside", { mode: 0o644 }); | |
| fs.chmodSync(outside, 0o644); | |
| const linkPath = path.join(dir, "rogue-link"); | |
| fs.symlinkSync(outside, linkPath); | |
| withHome(home, () => readConfigFile(target, null)); | |
| expect( | |
| fs.statSync(sibling).mode & 0o777, | |
| "positive control: walker tightened the regular sibling", | |
| ).toBe(0o600); | |
| expect( | |
| fs.statSync(outside).mode & 0o777, | |
| "symlink target must not be chmodded through the link", | |
| ).toBe(0o644); | |
| }); | |
| it("readConfigFile does not chmod through a symlink even via the per-file heal", () => { | |
| // Defensive duplicate of the symlink check, this time for the per-file | |
| // heal in readConfigFile itself (not the dir walk in ensureConfigDir). | |
| const dir = makeTempDir(); | |
| fs.chmodSync(dir, 0o700); | |
| const outsideDir = makeTempDir(); | |
| const outside = path.join(outsideDir, "target.json"); | |
| fs.writeFileSync(outside, JSON.stringify({ outside: true }), { mode: 0o644 }); | |
| fs.chmodSync(outside, 0o644); | |
| const symlinkPath = path.join(dir, "config.json"); | |
| fs.symlinkSync(outside, symlinkPath); | |
| // Reading through the symlink should not chmod the target file. | |
| readConfigFile(symlinkPath, null); | |
| expect(fs.statSync(outside).mode & 0o777).toBe(0o644); | |
| }); |
…4628 review) Address @cv's PR NVIDIA#4628 review feedback: the 700/600 heal was too generic. If a future caller routes a mutable-sandbox OpenClaw config dir (which uses the 2770/660 contract per NVIDIA#4538 / PR NVIDIA#4610) through `ensureConfigDir`, the walker would silently normalize it to 700/600 — exactly the NVIDIA#4538 EACCES regression. Add three predicates: - `hostNemoclawDir()` — canonical `${HOME}/.nemoclaw` - `isHostNemoclawRoot(dirPath)` — gate for the sibling sweep - `isMutableSandboxConfigPath(targetPath)` — gate for the file-level heal Wire them through: - `ensureConfigDir`: skip dir-mode chmod when path is mutable-sandbox. Run `healRootLevelFiles` only when path IS the host nemoclaw root. - `readConfigFile`: skip per-file chmod when path is mutable-sandbox. Tests: - 2 new scope-boundary tests (negative: arbitrary dir, no heal; positive: host nemoclaw root, heal fires). - Existing heal tests (multi-sibling + symlink positive control) updated to use the new `withHome` helper so they place files under `<HOME>/.nemoclaw` instead of an arbitrary tmp dir — preserves the NVIDIA#4546 acceptance while respecting the new scope boundary. 18/18 tests pass. Co-authored-by: kagura-agent <kagura.agent.ai@gmail.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Contributor
|
@cv addressed in
18/18 tests pass. The previous APPROVED review still stands; rebased on latest main while I was here. |
cv
pushed a commit
that referenced
this pull request
Jun 5, 2026
## Summary
- Add the v0.0.59 release notes from the GitHub announcement discussion.
- Refresh local inference and credential-storage guidance for the
current release behavior.
- Regenerate the user skills from the updated Fern docs.
- Tighten release-prep and docs review guidance for generated skills, PR
labels, and shared `$$nemoclaw` command placeholders.
## Verification
- `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix
nemoclaw-user --doc-platform fern-mdx`
- `rg "permissive mode|shields down|shields up|shields status|config
rotate-token|rotate-token" --glob '*.{md,mdx}'`
- `git diff --check`
- `npm run docs` (rerun outside sandbox after sandbox-only `tsx` IPC
permission failure)
- `npm run typecheck:cli`
- Pre-commit hooks during commit passed, including markdownlint,
docs-to-skills verification, gitleaks, commitlint, and skills YAML
tests.
## Source Summary
- #3679, #4437, #4681, #4766, #4772, #4775, #4786 ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`,
`docs/reference/troubleshooting.mdx`: Summarize OpenClaw 2026.5.27
compatibility, runtime path pinning, plugin registry recovery, live
gateway reconciliation, and clearer host-alias/startup diagnostics.
- #4332, #4402, #4769, #4776, #4779 -> `docs/about/release-notes.mdx`,
`docs/inference/inference-options.mdx`,
`docs/inference/use-local-inference.mdx`,
`docs/inference/switch-inference-providers.mdx`: Document the release
inference changes covering Local NIM waits, Hermes Anthropic routing,
Nemotron 3 Ultra, the current Ollama starter fallback, and Spark
managed-vLLM context length.
- #4628, #4652, #4733, #4745 -> `docs/about/release-notes.mdx`,
`docs/security/credential-storage.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/troubleshooting.mdx`: Capture permission healing,
gateway-stored credential reuse, cross-sandbox messaging credential
conflict checks, and CDI preflight diagnostics.
- #4728, #4737, #4743, #4744, #4782 -> `.agents/skills/nemoclaw-user-*`:
Regenerate the user skill references from the updated source docs.
- Follow-up maintenance ->
`.agents/skills/nemoclaw-contributor-update-docs/SKILL.md`,
`.coderabbit.yaml`: Add release-prep area labels for docs and skills
PRs, and teach docs review guidance that `$$nemoclaw` is the correct
shared command placeholder for examples that work across agent aliases.
Note: the `documentation` label was not present in the repository, so
this PR is labeled with `v0.0.59` only.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Updated default model for local Ollama inference setup to qwen3.5:9b
* Added Nemotron 3 Ultra 550B as an NVIDIA Endpoints model option
* Clarified credential storage and reuse behavior for post-deployment
(day-two) operations
* Added v0.0.59 release notes covering OpenClaw compatibility, inference
options, Hermes messaging sync, and troubleshooting
* Clarified CLI selection guidance and updated OpenClaw version example
in status output
* Revised release-prep instructions and docs review guidance for CLI
alias usage
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
6 tasks
This was referenced Jun 7, 2026
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
5 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
readConfigFile()did not callensureConfigDir(), so read-only CLI commands (e.g.nemoclaw list) never triggered permission drift healing. If a user manually changed~/.nemoclawto755, only a write operation would repair it back to700.Changes
ensureConfigDir()inreadConfigFile()before reading the file, ensuring parent directory permissions are healed (755 → 700) on every config access, not just writesconfig-io.test.ts:readConfigFile repairs a 755 parent directory to 700readConfigFile repairs a 644 file to 600Root Cause
readConfigFile()was a straight-through read with no security posture enforcement.ensureConfigDir()— which already handles permission drift healing — was only called fromwriteConfigFile(). This meant CLI commands that only read config (listing, status, etc.) would silently operate with weakened directory/file permissions.Testing
All 13 tests pass in
config-io.test.ts(11 existing + 2 new). Typecheck clean.Fixes #4546
Signed-off-by: kagura-agent kagura.agent.ai@gmail.com
Summary by CodeRabbit
Tests
Bug Fixes