diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index ef699ee868c..1574e6a456b 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -297,10 +297,12 @@ After the top-level config binding is frozen, lockdown makes containment monoton It removes unsafe symlinks, special entries, and protected-root names that are not directories through descriptor-relative operations without following their targets. For protected regular files, lockdown publishes a fresh inode, severing hardlinks while preserving file content, read/execute mode, timestamps, and supported extended attributes; this also revokes write authority held through a descriptor opened before `shields up`. The OpenClaw gateway (a member of the `sandbox` group) keeps read access to plugin and agent code; the sandbox user can no longer write them. -The same workflow locks `identity`, `pairing`, and a non-empty `credentials` directory to `root:root 0700`. -An empty `credentials` directory uses `root:sandbox 0710` so OpenClaw can confirm that optional credential files are absent during startup. -The sandbox group cannot list, create, or remove entries, and it cannot read credential files. -Neither the sandbox user nor the gateway can read stored secrets while the lock is active. +The same workflow also locks the secret-bearing `credentials`, `identity`, and `pairing` directories. +The guard sets each confidentiality root to `root:sandbox 0710`, including a non-empty `credentials` directory. +It sets every nested directory and file to `root:root` and removes all group and world permission bits. +The sandbox group cannot list, create, or remove entries in a confidentiality root, and neither the sandbox user nor gateway can read stored secrets. +They can inspect metadata for a direct child only when they already know its name. +Probing a missing direct child, such as the legacy `credentials/oauth.json`, returns `ENOENT` instead of `EACCES`. Restoring the mutable-default posture returns those directories to `sandbox:sandbox 2770`. The list is the union of state directories declared by every shipped agent manifest. The lock helper silently skips dirs that are not present in a given agent's config tree. @@ -347,6 +349,11 @@ Direct edits to these files can be overwritten when NemoClaw regenerates the ima Hermes also stores runtime state such as `state.db`, logs, and platform sessions under the `.hermes` tree. Messaging sessions such as WhatsApp pairing can remain mutable by design so they survive rebuilds. +When Shields locks the tree, the shared state-directory guard treats any present `credentials`, `identity`, or `pairing` directory as a confidentiality root. +For Hermes, this normally applies to `pairing`. +The guard sets the root to `root:sandbox 710`, keeps it traversable but unlistable to the sandbox group, and sets every descendant to `root:root` with no group or world permission bits. +As a result, a known-name probe for a missing direct child returns `ENOENT`, while directory listing, nested traversal, and protected-file reads return `EACCES`. + | Aspect | Detail | |---|---| | Default | The Hermes config tree contains NemoClaw-generated config plus mutable runtime state. | diff --git a/scripts/state-dir-guard.py b/scripts/state-dir-guard.py index 87205a1cf04..1a889cea223 100755 --- a/scripts/state-dir-guard.py +++ b/scripts/state-dir-guard.py @@ -879,19 +879,33 @@ def _preflight( def _expected_ids( - policy: Policy, action: Action, identity: Identity + policy: Policy, + action: Action, + identity: Identity, + is_confidentiality_root: bool = False, ) -> tuple[int, int]: if action == "unlock": return identity.sandbox_uid, identity.sandbox_gid if policy == "confidentiality": + if is_confidentiality_root: + return identity.root_uid, identity.sandbox_gid return identity.root_uid, identity.root_gid return identity.root_uid, identity.sandbox_gid -def _expected_dir_mode(policy: Policy, action: Action) -> int: +def _expected_dir_mode( + policy: Policy, action: Action, is_confidentiality_root: bool = False +) -> int: if action == "unlock": return 0o2770 - return 0o700 if policy == "confidentiality" else 0o755 + if policy == "confidentiality": + # The confidentiality root stays traversable (group execute, no read) + # so a sandbox probe for a missing name directly under the root, such + # as the legacy credentials/oauth.json, resolves as ENOENT instead of + # EACCES. Nested directories and every file keep the sealed posture, + # so contents and the subtree shape stay unreadable. + return 0o710 if is_confidentiality_root else 0o700 + return 0o755 def _expected_file_mode(policy: Policy, action: Action, old_mode: int) -> int: @@ -915,10 +929,11 @@ def _set_dir_metadata( policy: Policy, action: Action, identity: Identity, + is_confidentiality_root: bool = False, ) -> None: - uid, gid = _expected_ids(policy, action, identity) + uid, gid = _expected_ids(policy, action, identity, is_confidentiality_root) os.fchown(dir_fd, uid, gid) - os.fchmod(dir_fd, _expected_dir_mode(policy, action)) + os.fchmod(dir_fd, _expected_dir_mode(policy, action, is_confidentiality_root)) def _set_empty_credentials_startup_metadata( @@ -1354,7 +1369,9 @@ def _mutate_dir( # mutation through directory descriptors opened by the sandbox # before shields-up, and it happens before visiting descendants. _freeze_dir_for_lock(dir_fd) - _set_dir_metadata(dir_fd, policy, action, identity) + _set_dir_metadata( + dir_fd, policy, action, identity, is_root and policy == "confidentiality" + ) elif is_root: # Keep the subtree inaccessible while descendants are restored. os.fchmod(dir_fd, 0o700) @@ -1366,10 +1383,10 @@ def _mutate_dir( if _is_empty_credentials_root( relative_dir, policy, action, is_root, names ): - # OpenClaw probes optional credential paths while starting. An - # empty directory contains no secret metadata to expose, so allow - # the sandbox group to traverse it without granting list, read, or - # write access. Non-empty confidentiality roots remain root-only. + # The confidentiality-root contract already grants the sandbox + # group execute-only access (root:sandbox 0710). Keep this + # explicit empty-root case for compatibility with the startup + # recovery path; nested entries remain root-only. _set_empty_credentials_startup_metadata(dir_fd, identity) except OSError as exc: raise GuardOperationError( @@ -1544,12 +1561,11 @@ def _verify_metadata( policy: Policy, action: Action, identity: Identity, - empty_credentials_root: bool = False, + is_confidentiality_root: bool = False, ) -> Issue | None: - if empty_credentials_root: - expected_uid, expected_gid = identity.root_uid, identity.sandbox_gid - else: - expected_uid, expected_gid = _expected_ids(policy, action, identity) + expected_uid, expected_gid = _expected_ids( + policy, action, identity, is_confidentiality_root + ) if st.st_uid != expected_uid or st.st_gid != expected_gid: return Issue( "verification-owner-mismatch", @@ -1560,7 +1576,7 @@ def _verify_metadata( return None mode = stat.S_IMODE(st.st_mode) if entry_type == "directory": - expected_mode = 0o710 if empty_credentials_root else _expected_dir_mode(policy, action) + expected_mode = _expected_dir_mode(policy, action, is_confidentiality_root) if mode != expected_mode: return Issue( "verification-mode-mismatch", @@ -1607,6 +1623,7 @@ def _verify_dir( replaced_inodes: dict[str, int], issues: list[Issue], depth: int, + is_root: bool = False, ) -> None: if depth > MAX_TRAVERSAL_DEPTH: issues.append( @@ -1635,9 +1652,7 @@ def _verify_dir( policy, action, identity, - _is_empty_credentials_root( - relative_dir, policy, action, relative_dir == "credentials", names - ), + is_root and policy == "confidentiality", ) if dir_issue is not None: issues.append(dir_issue) @@ -1805,14 +1820,16 @@ def _restore_empty_credentials_startup_access( ) ) return result - if names: - if startup_traversable: - _set_dir_metadata( - credentials_fd, "confidentiality", "lock", identity - ) - os.fsync(credentials_fd) - return result - _set_empty_credentials_startup_metadata(credentials_fd, identity) + # All confidentiality roots are execute-only for the sandbox group, + # whether or not they currently contain credentials. This preserves + # known-name ENOENT probes without exposing names or descendants. + _set_dir_metadata( + credentials_fd, + "confidentiality", + "lock", + identity, + is_confidentiality_root=True, + ) after = os.fstat(credentials_fd) if ( after.st_uid != identity.root_uid @@ -2019,6 +2036,7 @@ def _run_guard_unserialized( replaced_inodes, result.issues, 1, + is_root=True, ) finally: os.close(root_fd) diff --git a/test/e2e/live/state-dir-guard-metadata.test.ts b/test/e2e/live/state-dir-guard-metadata.test.ts index 903676f3c77..83f1be7690c 100644 --- a/test/e2e/live/state-dir-guard-metadata.test.ts +++ b/test/e2e/live/state-dir-guard-metadata.test.ts @@ -44,6 +44,18 @@ type GuardAction = "preflight" | "lock" | "unlock"; type GuardTargets = Record<"plugins" | "credentials", string>; type AccessResult = { read: boolean; write: boolean }; +interface ConfidentialityAccessEvidence { + processUid: number; + processGid: number; + rootUid: number; + rootGid: number; + rootMode: number; + missingDirectChildErrno: number; + rootListingErrno: number; + nestedTraversalErrno: number; + secretReadErrno: number; +} + interface GuardLimits { maxEntries: number; maxLogicalBytes: number; @@ -452,6 +464,56 @@ async function expectNamedUserAccessState( expect(actual).toEqual(expected); } +async function probeConfidentialityAccessContract( + host: HostCliClient, + agent: AgentCase, + fixtureRoot: string, + target: string, + identity: { uid: number; gid: number }, +): Promise { + const credentialsRoot = path.join(fixtureRoot, "credentials"); + const containerRoot = containerTargetPath(agent, fixtureRoot, credentialsRoot); + const containerTarget = containerTargetPath(agent, fixtureRoot, target); + const script = [ + "import json, os, stat, sys", + "root_path, secret_path = sys.argv[1:]", + "def errno_of(operation):", + " try:", + " operation()", + " except OSError as exc:", + " return exc.errno", + " return 0", + "def read_secret():", + " with open(secret_path, 'rb') as stream:", + " stream.read(1)", + "root = os.lstat(root_path)", + "print(json.dumps({", + " 'processUid': os.getuid(),", + " 'processGid': os.getgid(),", + " 'rootUid': root.st_uid,", + " 'rootGid': root.st_gid,", + " 'rootMode': stat.S_IMODE(root.st_mode),", + " 'missingDirectChildErrno': errno_of(lambda: os.lstat(os.path.join(root_path, 'oauth.json'))),", + " 'rootListingErrno': errno_of(lambda: os.listdir(root_path)),", + " 'nestedTraversalErrno': errno_of(lambda: os.lstat(os.path.join(root_path, 'providers', 'missing.json'))),", + " 'secretReadErrno': errno_of(read_secret),", + "}))", + ].join("\n"); + const result = await expectCommand( + host, + "docker", + [ + ...mountArgs(agent, fixtureRoot, "python3", `${identity.uid}:${identity.gid}`), + "-c", + script, + containerRoot, + containerTarget, + ], + `${agent.id}-locked-confidentiality-access`, + ); + return JSON.parse(result.stdout.trim()) as ConfidentialityAccessEvidence; +} + function assertBudgetEvidence( tree: TreeMeasurement, limits: GuardLimits, @@ -575,6 +637,25 @@ async function runAgentProbe( plugins: { read: true, write: false }, credentials: { read: false, write: false }, }); + const confidentialityAccess = await probeConfidentialityAccessContract( + host, + agent, + fixtureRoot, + targets.credentials, + identity, + ); + expect(confidentialityAccess).toMatchObject({ + processUid: identity.uid, + processGid: identity.gid, + rootUid: 0, + rootGid: identity.gid, + rootMode: 0o710, + missingDirectChildErrno: os.constants.errno.ENOENT, + rootListingErrno: os.constants.errno.EACCES, + nestedTraversalErrno: os.constants.errno.EACCES, + secretReadErrno: os.constants.errno.EACCES, + }); + expect(confidentialityAccess.processUid).not.toBe(confidentialityAccess.rootUid); const unlock = await runGuard(host, agent, fixtureRoot, "unlock"); expect(unlock.summary).toMatchObject({ @@ -622,6 +703,7 @@ async function runAgentProbe( lock: lock.summary, unlock: unlock.summary, }, + confidentialityAccess, }); } @@ -647,6 +729,7 @@ test( "the installed root-owned guard handles preflight, lock, and unlock for OpenClaw and Hermes", "plugins and credentials preserve content and user xattrs across fresh-inode locking", "numeric ownership, mode, raw ACL, mask, and effective named-user access match each policy", + "a distinct sandbox-group member sees ENOENT for a missing direct child while listing, nested traversal, and secret reads stay denied", "representative-tree entry, byte, depth, copy, and wall-time evidence stays within shipped limits", ], }); @@ -725,6 +808,7 @@ test( contentXattrAclPreserved: true, effectiveAclClamped: true, effectiveAclEnforcedByKernel: true, + confidentialityRootAccessContract: true, productionBudgetsRecorded: true, }, }); diff --git a/test/state-dir-guard.test.ts b/test/state-dir-guard.test.ts index 77faeb01f2b..80a1e6a31f0 100644 --- a/test/state-dir-guard.test.ts +++ b/test/state-dir-guard.test.ts @@ -15,6 +15,19 @@ const PYTHON_HAS_DESCRIPTOR_XATTR = "-c", "import os; assert all(hasattr(os, name) for name in ('listxattr', 'getxattr', 'setxattr'))", ]).status === 0; +// An unprivileged process can chgrp only to a group it belongs to, so a +// supplementary gid distinct from the primary gid stands in for the sandbox +// group; the ownership test skips on runners that have none. +const SUPPLEMENTARY_GID = Number( + spawnSync( + "python3", + [ + "-c", + "import os; groups = [g for g in os.getgroups() if g != os.getgid()]; print(groups[0] if groups else -1)", + ], + { encoding: "utf-8" }, + ).stdout.trim(), +); const RUN_GUARD_AS_CURRENT_USER = String.raw` import importlib.util @@ -136,6 +149,37 @@ finally: os.close(config_fd) `; +const RUN_GUARD_WITH_DISTINCT_SANDBOX_GID = String.raw` +import importlib.util +import json +import os +import sys + +guard_path, config_dir, sandbox_gid = sys.argv[1:4] +spec = importlib.util.spec_from_file_location("nemoclaw_state_dir_guard_gid", guard_path) +module = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = module +spec.loader.exec_module(module) +identity = module.Identity( + root_uid=os.getuid(), root_gid=os.getgid(), + sandbox_uid=os.getuid(), sandbox_gid=int(sandbox_gid), +) +result = module.run_guard("lock", config_dir, identity) + + +def group_of(path): + return os.lstat(os.path.join(config_dir, path)).st_gid + + +print(json.dumps({ + "ok": result.ok, + "rootGid": os.getgid(), + "credentialsGid": group_of("credentials"), + "nestedGid": group_of("credentials/providers"), + "secretGid": group_of("credentials/providers/provider.json"), +})) +`; + const RUN_CARVEOUT_MKDIR_RACE = String.raw` import importlib.util import json @@ -889,6 +933,8 @@ describe("state-dir-guard", () => { const { configDir } = fixture(); const secretDir = path.join(configDir, "credentials"); const secretPath = path.join(secretDir, "token.json"); + const nestedSecretDir = path.join(secretDir, "providers"); + const nestedSecretPath = path.join(nestedSecretDir, "provider.json"); const workspaceDir = path.join(configDir, "workspace-research"); const executablePath = path.join(workspaceDir, "run.sh"); const agentDir = path.join(configDir, "agents", "main"); @@ -897,10 +943,11 @@ describe("state-dir-guard", () => { const sessionBacklink = path.join(sessionsDir, "runtime-link"); const sessionFifo = path.join(sessionsDir, "runtime-events.fifo"); const agentCodePath = path.join(agentDir, "agent.js"); - fs.mkdirSync(secretDir, { recursive: true }); + fs.mkdirSync(nestedSecretDir, { recursive: true }); fs.mkdirSync(workspaceDir); fs.mkdirSync(sessionsDir, { recursive: true }); fs.writeFileSync(secretPath, "secret\n", { mode: 0o640 }); + fs.writeFileSync(nestedSecretPath, "secret\n", { mode: 0o640 }); fs.writeFileSync(executablePath, "#!/bin/sh\n", { mode: 0o775 }); fs.writeFileSync(sessionPath, "runtime\n", { mode: 0o660 }); fs.writeFileSync(agentCodePath, "export {};\n", { mode: 0o664 }); @@ -913,8 +960,10 @@ describe("state-dir-guard", () => { const locked = runGuard("lock", configDir); expect(locked.status).toBe(0); - expect(mode(secretDir)).toBe(0o700); + expect(mode(secretDir)).toBe(0o710); + expect(mode(nestedSecretDir)).toBe(0o700); expect(mode(secretPath)).toBe(0o600); + expect(mode(nestedSecretPath)).toBe(0o600); expect(mode(workspaceDir)).toBe(0o755); expect(mode(executablePath)).toBe(0o755); expect(mode(path.join(configDir, "agents"))).toBe(0o755); @@ -937,7 +986,37 @@ describe("state-dir-guard", () => { expect(mode(agentDir)).toBe(0o2770); }); - it("restores startup traversal only for an empty sealed credentials root (#8112)", () => { + it.skipIf(SUPPLEMENTARY_GID < 0)( + "assigns the sandbox group to the confidentiality root only, while nested entries keep the root group (#7545)", + () => { + const { configDir } = fixture(".openclaw"); + const nested = path.join(configDir, "credentials", "providers"); + fs.mkdirSync(nested, { recursive: true }); + fs.writeFileSync(path.join(nested, "provider.json"), "secret\n", { mode: 0o640 }); + + const result = spawnSync( + "python3", + [ + "-c", + RUN_GUARD_WITH_DISTINCT_SANDBOX_GID, + GUARD_PATH, + configDir, + String(SUPPLEMENTARY_GID), + ], + { encoding: "utf-8", timeout: 15_000 }, + ); + + expect(result.status, result.stderr).toBe(0); + const parsed = JSON.parse(result.stdout.trim()); + expect(parsed.ok).toBe(true); + expect(parsed.rootGid).not.toBe(SUPPLEMENTARY_GID); + expect(parsed.credentialsGid).toBe(SUPPLEMENTARY_GID); + expect(parsed.nestedGid).toBe(parsed.rootGid); + expect(parsed.secretGid).toBe(parsed.rootGid); + }, + ); + + it("restores startup traversal for sealed credentials roots without exposing contents (#8112)", () => { const { configDir } = fixture(); const credentialsDir = path.join(configDir, "credentials"); fs.mkdirSync(credentialsDir); @@ -953,7 +1032,7 @@ describe("state-dir-guard", () => { const nonemptyStartup = runGuard("startup", configDir); expect(nonemptyStartup.status, nonemptyStartup.stderr).toBe(0); - expect(mode(credentialsDir)).toBe(0o700); + expect(mode(credentialsDir)).toBe(0o710); expect(mode(path.join(credentialsDir, "token.json"))).toBe(0o600); });