From e03c9a9d2e11d1b02f5219b54af38695ca07c613 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Tue, 17 Mar 2026 16:30:55 +0800 Subject: [PATCH 1/6] security: strip credentials from migration snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes a critical vulnerability where createSnapshotBundle() copies the entire ~/.openclaw directory — including auth-profiles.json with live API keys, GitHub PATs, and npm tokens — into the sandbox filesystem. A compromised agent can read these credentials and exfiltrate them. Fix: Two sanitization layers in migration-state.ts: - sanitizeCredentialsInBundle(): deletes auth-profiles.json from agents/ subtree, strips credential fields from openclaw.json - sanitizeExternalRootSnapshot(): strips auth-profiles.json from external root snapshots before archiving Note: Blueprint digest bypass fix (verify.ts/resolve.ts) was dropped from this PR — those modules were deleted upstream in PR #492 as dead code when the openclaw nemoclaw CLI commands were removed. Signed-off-by: Lucas Wang --- nemoclaw/src/commands/migration-state.ts | 106 +++++++++++++ test/security-credential-exposure.test.js | 176 ++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 test/security-credential-exposure.test.js diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index 4421943d2..bc8ecf802 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -544,6 +544,102 @@ function setConfigValue( (current as Record)[finalToken] = value; } +/** + * Credential field names that MUST be stripped from config and auth files + * before they are sent into the sandbox. Credentials should be injected + * at runtime via OpenShell's provider credential mechanism, not baked + * into the sandbox filesystem. + */ +const CREDENTIAL_FIELDS = new Set([ + "apiKey", + "api_key", + "token", + "secret", + "password", + "resolvedKey", + "keyRef", +]); + +/** + * Recursively strip credential fields from a JSON-like object. + * Returns a new object with sensitive values replaced by a placeholder. + * Any value type (string, object, boolean, number, null) is stripped if + * the key matches CREDENTIAL_FIELDS. + */ +function stripCredentials(obj: unknown): unknown { + if (obj === null || obj === undefined) return obj; + if (typeof obj !== "object") return obj; + if (Array.isArray(obj)) return obj.map(stripCredentials); + + const result: Record = {}; + for (const [key, value] of Object.entries(obj as Record)) { + if (CREDENTIAL_FIELDS.has(key)) { + result[key] = "[STRIPPED_BY_MIGRATION]"; + } else { + result[key] = stripCredentials(value); + } + } + return result; +} + +/** + * Remove auth-profiles.json from the agents/ subtree and strip credential + * fields from openclaw.json inside the prepared sandbox state directory. + * + * Note: when hasExternalConfig is true, prepareSandboxState has already + * merged the external config into openclaw.json — so stripping that file + * covers both the inline and external config cases. + */ +function sanitizeCredentialsInBundle(preparedStateDir: string): void { + // Remove auth-profiles.json files from agents/ subtree + removeAuthProfileFiles(preparedStateDir); + + // Strip credential fields from openclaw.json + const configPath = path.join(preparedStateDir, "openclaw.json"); + if (existsSync(configPath)) { + const raw = readFileSync(configPath, "utf-8"); + const config = JSON.parse(raw) as Record; + const sanitized = stripCredentials(config) as Record; + writeFileSync(configPath, JSON.stringify(sanitized, null, 2)); + } +} + +/** + * Sanitize a snapshot directory for an external root (agentDir, workspace, + * skills) before it is archived and sent into the sandbox. External roots + * may contain their own auth-profiles.json or credential files. + */ +function sanitizeExternalRootSnapshot(rootSnapshotDir: string): void { + // Remove auth-profiles.json anywhere in the external root + walkAndRemoveFile(rootSnapshotDir, "auth-profiles.json"); +} + +/** + * Remove auth-profiles.json files from known OpenClaw credential locations. + * Scoped to the agents/ subtree to avoid traversing large workspace directories. + */ +function removeAuthProfileFiles(preparedStateDir: string): void { + const agentsDir = path.join(preparedStateDir, "agents"); + if (!existsSync(agentsDir)) return; + walkAndRemoveFile(agentsDir, "auth-profiles.json"); +} + +function walkAndRemoveFile(dirPath: string, targetName: string): void { + for (const entry of readdirSync(dirPath)) { + const fullPath = path.join(dirPath, entry); + try { + const stat = lstatSync(fullPath); + if (stat.isDirectory()) { + walkAndRemoveFile(fullPath, targetName); + } else if (entry === targetName) { + rmSync(fullPath, { force: true }); + } + } catch { + // Non-fatal: skip files that disappeared or lack permissions + } + } +} + function prepareSandboxState(snapshotDir: string, manifest: SnapshotManifest): string { const preparedStateDir = path.join(snapshotDir, "sandbox-bundle", "openclaw"); rmSync(preparedStateDir, { recursive: true, force: true }); @@ -560,6 +656,13 @@ function prepareSandboxState(snapshotDir: string, manifest: SnapshotManifest): s } writeFileSync(path.join(preparedStateDir, "openclaw.json"), JSON.stringify(config, null, 2)); + + // SECURITY: Strip all credentials from the bundle before it enters the sandbox. + // Credentials must be injected at runtime via OpenShell's provider credential + // mechanism, not baked into the sandbox filesystem where a compromised agent + // can read them. + sanitizeCredentialsInBundle(preparedStateDir); + return preparedStateDir; } @@ -597,6 +700,9 @@ export function createSnapshotBundle( const destination = path.join(parentDir, root.snapshotRelativePath); mkdirSync(path.dirname(destination), { recursive: true }); copyDirectory(root.sourcePath, destination); + // SECURITY: strip credential files from external root snapshots + // before they are archived and sent into the sandbox. + sanitizeExternalRootSnapshot(destination); externalRoots.push({ ...root, symlinkPaths: collectSymlinkPaths(root.sourcePath), diff --git a/test/security-credential-exposure.test.js b/test/security-credential-exposure.test.js new file mode 100644 index 000000000..458e8e807 --- /dev/null +++ b/test/security-credential-exposure.test.js @@ -0,0 +1,176 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Security tests for migration credential sanitization. +// Demonstrates the credential exposure vulnerability and verifies the fix. +// +// Note: Blueprint digest tests were removed — the verify.ts and resolve.ts +// modules were deleted upstream in PR #492 (CLI commands removed). + +const { describe, it } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const os = require("node:os"); + +// Deliberately non-matching fake tokens that will NOT trigger secret scanners. +// These do NOT follow real token formats (no "ghp_", "nvapi-", "npm_" prefixes). +const FAKE_NVIDIA_KEY = "test-fake-nvidia-key-0000000000000000"; +const FAKE_GITHUB_TOKEN = "test-fake-github-token-1111111111111111"; +const FAKE_NPM_TOKEN = "test-fake-npm-token-2222222222222222"; +const FAKE_GATEWAY_TOKEN = "test-fake-gateway-token-333333333333"; + +// ═══════════════════════════════════════════════════════════════════ +// Helper: create a mock ~/.openclaw directory with credential files +// ═══════════════════════════════════════════════════════════════════ +function createMockOpenClawHome(tmpDir) { + const stateDir = path.join(tmpDir, ".openclaw"); + fs.mkdirSync(stateDir, { recursive: true }); + + const config = { + agents: { + defaults: { + model: { primary: "nvidia/nemotron-3-super-120b-a12b" }, + workspace: path.join(stateDir, "workspace"), + }, + }, + gateway: { + mode: "local", + auth: { token: FAKE_GATEWAY_TOKEN }, + }, + nvidia: { apiKey: FAKE_NVIDIA_KEY }, + }; + fs.writeFileSync( + path.join(stateDir, "openclaw.json"), + JSON.stringify(config, null, 2), + ); + + const authDir = path.join(stateDir, "agents", "main", "agent"); + fs.mkdirSync(authDir, { recursive: true }); + const authProfiles = { + "nvidia:manual": { + type: "api_key", + provider: "nvidia", + keyRef: { source: "env", id: "NVIDIA_API_KEY" }, + resolvedKey: FAKE_NVIDIA_KEY, + profileId: "nvidia:manual", + }, + "github:pat": { + type: "api_key", + provider: "github", + token: FAKE_GITHUB_TOKEN, + profileId: "github:pat", + }, + "npm:publish": { + type: "api_key", + provider: "npm", + token: FAKE_NPM_TOKEN, + profileId: "npm:publish", + }, + }; + fs.writeFileSync( + path.join(authDir, "auth-profiles.json"), + JSON.stringify(authProfiles, null, 2), + ); + + const workspace = path.join(stateDir, "workspace"); + fs.mkdirSync(workspace, { recursive: true }); + fs.writeFileSync(path.join(workspace, "project.md"), "# My Project\n"); + + return { stateDir, config, authProfiles }; +} + +// ═══════════════════════════════════════════════════════════════════ +// 1. Migration copies ALL credentials into sandbox (demonstrates vuln) +// ═══════════════════════════════════════════════════════════════════ +describe("Migration credential exposure (pre-fix behavior)", () => { + it("raw cpSync copies auth-profiles.json with all tokens into sandbox", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-poc-")); + try { + const mock = createMockOpenClawHome(tmpDir); + + // Simulate the vulnerable codepath: cpSync(stateDir, snapshotDir) + const snapshotDir = path.join(tmpDir, "snapshot", "openclaw"); + fs.cpSync(mock.stateDir, snapshotDir, { recursive: true }); + + const authPath = path.join(snapshotDir, "agents", "main", "agent", "auth-profiles.json"); + const configPath = path.join(snapshotDir, "openclaw.json"); + + assert.ok(fs.existsSync(authPath), "auth-profiles.json copied into sandbox"); + assert.ok(fs.existsSync(configPath), "openclaw.json copied into sandbox"); + + const stolenAuth = JSON.parse(fs.readFileSync(authPath, "utf-8")); + const stolenConfig = JSON.parse(fs.readFileSync(configPath, "utf-8")); + + // All tokens are fully readable — this is the vulnerability + assert.strictEqual(stolenAuth["nvidia:manual"].resolvedKey, FAKE_NVIDIA_KEY); + assert.strictEqual(stolenAuth["github:pat"].token, FAKE_GITHUB_TOKEN); + assert.strictEqual(stolenAuth["npm:publish"].token, FAKE_NPM_TOKEN); + assert.strictEqual(stolenConfig.gateway.auth.token, FAKE_GATEWAY_TOKEN); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + +// ═══════════════════════════════════════════════════════════════════ +// 2. Fix verification — sanitized migration blocks the attack chain +// ═══════════════════════════════════════════════════════════════════ +describe("Fix verification: sanitized migration blocks credential theft", () => { + it("sanitizeCredentialsInBundle deletes auth-profiles.json and strips config tokens", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fix-")); + try { + const mock = createMockOpenClawHome(tmpDir); + + // Simulate snapshot + prepare + const bundleDir = path.join(tmpDir, "bundle", "openclaw"); + fs.cpSync(mock.stateDir, bundleDir, { recursive: true }); + + // Apply the same sanitization logic from the fix in migration-state.ts + const CREDENTIAL_FIELDS = new Set([ + "apiKey", "api_key", "token", "secret", "password", "resolvedKey", "keyRef", + ]); + + function stripCredentials(obj) { + if (obj === null || obj === undefined) return obj; + if (typeof obj !== "object") return obj; + if (Array.isArray(obj)) return obj.map(stripCredentials); + const result = {}; + for (const [key, value] of Object.entries(obj)) { + if (CREDENTIAL_FIELDS.has(key) && (typeof value === "string" || typeof value === "object")) { + result[key] = "[STRIPPED_BY_MIGRATION]"; + } else { + result[key] = stripCredentials(value); + } + } + return result; + } + + // Delete auth-profiles.json + const authPath = path.join(bundleDir, "agents", "main", "agent", "auth-profiles.json"); + if (fs.existsSync(authPath)) fs.rmSync(authPath, { force: true }); + + // Strip config credentials + const configPath = path.join(bundleDir, "openclaw.json"); + const config = JSON.parse(fs.readFileSync(configPath, "utf-8")); + fs.writeFileSync(configPath, JSON.stringify(stripCredentials(config), null, 2)); + + // Verify: auth-profiles.json deleted + assert.ok(!fs.existsSync(authPath), "auth-profiles.json must be deleted"); + + // Verify: config credentials stripped + const sanitized = JSON.parse(fs.readFileSync(configPath, "utf-8")); + assert.strictEqual(sanitized.nvidia.apiKey, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.gateway.auth.token, "[STRIPPED_BY_MIGRATION]"); + + // Verify: non-credential fields preserved + assert.strictEqual(sanitized.agents.defaults.model.primary, "nvidia/nemotron-3-super-120b-a12b"); + assert.strictEqual(sanitized.gateway.mode, "local"); + + // Verify: workspace files untouched + assert.ok(fs.existsSync(path.join(bundleDir, "workspace", "project.md"))); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); From e24b6ce7dcb1a8a385ecc2ae0b8fb05a0ccef2e9 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Sun, 22 Mar 2026 08:44:54 +0800 Subject: [PATCH 2/6] chore: add security to commitlint allowed types Signed-off-by: Lucas Wang --- commitlint.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/commitlint.config.js b/commitlint.config.js index b4ee2777d..7f44906a6 100644 --- a/commitlint.config.js +++ b/commitlint.config.js @@ -16,6 +16,7 @@ module.exports = { "test", "ci", "perf", + "security", ], ], }, From c29c2aec0877b9b39775a3109d8d7cb07b97f848 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Sun, 22 Mar 2026 08:48:07 +0800 Subject: [PATCH 3/6] fix: harden walkAndRemoveFile error handling in credential sanitization - Move readdirSync inside try/catch to handle permission errors - Add { force: true } to rmSync for race-condition safety - Log warnings instead of silently swallowing removal failures - Remove unused node:crypto import from test Signed-off-by: Lucas Wang --- nemoclaw/src/commands/migration-state.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index bc8ecf802..b3143c094 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -625,7 +625,14 @@ function removeAuthProfileFiles(preparedStateDir: string): void { } function walkAndRemoveFile(dirPath: string, targetName: string): void { - for (const entry of readdirSync(dirPath)) { + let entries: string[]; + try { + entries = readdirSync(dirPath); + } catch (err) { + console.warn(`[credential-sanitize] Unable to read directory ${dirPath}: ${err}`); + return; + } + for (const entry of entries) { const fullPath = path.join(dirPath, entry); try { const stat = lstatSync(fullPath); @@ -634,8 +641,8 @@ function walkAndRemoveFile(dirPath: string, targetName: string): void { } else if (entry === targetName) { rmSync(fullPath, { force: true }); } - } catch { - // Non-fatal: skip files that disappeared or lack permissions + } catch (err) { + console.warn(`[credential-sanitize] Unable to process ${fullPath}: ${err}`); } } } From 612a8579f8db73df549759473e315728c7d8c468 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Sun, 22 Mar 2026 09:01:39 +0800 Subject: [PATCH 4/6] fix: strip credential fields from config files in external root snapshots sanitizeExternalRootSnapshot only removed auth-profiles.json but did not strip credential fields from openclaw.json files that may exist in external roots (agentDir, workspace, skills). Add walkAndStripCredentials to apply the same field-level sanitization used for the main bundle. Signed-off-by: Lucas Wang --- nemoclaw/src/commands/migration-state.ts | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index b3143c094..cdbe80a47 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -612,6 +612,39 @@ function sanitizeCredentialsInBundle(preparedStateDir: string): void { function sanitizeExternalRootSnapshot(rootSnapshotDir: string): void { // Remove auth-profiles.json anywhere in the external root walkAndRemoveFile(rootSnapshotDir, "auth-profiles.json"); + + // Strip credential fields from any openclaw.json found in the external root + walkAndStripCredentials(rootSnapshotDir, "openclaw.json"); +} + +/** + * Recursively find files matching targetName and strip credential fields + * from their JSON content. + */ +function walkAndStripCredentials(dirPath: string, targetName: string): void { + let entries: string[]; + try { + entries = readdirSync(dirPath); + } catch (err) { + console.warn(`[credential-sanitize] Unable to read directory ${dirPath}: ${err}`); + return; + } + for (const entry of entries) { + const fullPath = path.join(dirPath, entry); + try { + const stat = lstatSync(fullPath); + if (stat.isDirectory()) { + walkAndStripCredentials(fullPath, targetName); + } else if (entry === targetName) { + const raw = readFileSync(fullPath, "utf-8"); + const config = JSON.parse(raw) as Record; + const sanitized = stripCredentials(config) as Record; + writeFileSync(fullPath, JSON.stringify(sanitized, null, 2)); + } + } catch (err) { + console.warn(`[credential-sanitize] Unable to process ${fullPath}: ${err}`); + } + } } /** From 41b182fdbe6b0fcfd1c185a0a9d738273e1616f9 Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Sun, 22 Mar 2026 09:56:33 +0800 Subject: [PATCH 5/6] fix: harden credential sanitization with pattern matching, JSON5 and symlink guards - Remove keyRef from CREDENTIAL_FIELDS (metadata, not a secret) - Add CREDENTIAL_FIELD_PATTERN for broader credential detection (accessToken, refreshToken, privateKey, clientSecret, etc.) - Use JSON5.parse instead of JSON.parse for openclaw.json consistency - Add symlink guards in walkAndStripCredentials and walkAndRemoveFile to prevent modifying/deleting files outside the snapshot boundary - Rewrite tests to align with production logic, add pattern matching, symlink safety and value-type coverage tests --- nemoclaw/src/commands/migration-state.ts | 29 +++- test/security-credential-exposure.test.js | 186 ++++++++++++++-------- 2 files changed, 145 insertions(+), 70 deletions(-) diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index cdbe80a47..8033dfde7 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -550,6 +550,7 @@ function setConfigValue( * at runtime via OpenShell's provider credential mechanism, not baked * into the sandbox filesystem. */ +// keyRef is metadata (points to env var source), not a secret value — excluded intentionally. const CREDENTIAL_FIELDS = new Set([ "apiKey", "api_key", @@ -557,14 +558,28 @@ const CREDENTIAL_FIELDS = new Set([ "secret", "password", "resolvedKey", - "keyRef", ]); +/** + * Pattern-based detection for credential field names not covered by the + * explicit set above. Matches common suffixes like accessToken, privateKey, + * clientSecret, etc. + */ +const CREDENTIAL_FIELD_PATTERN = + /(?:access|refresh|client|bearer|auth|api|private|public|signing|session)(?:Token|Key|Secret|Password)$/; + +/** + * Check whether a JSON key is a credential field that must be stripped. + */ +function isCredentialField(key: string): boolean { + return CREDENTIAL_FIELDS.has(key) || CREDENTIAL_FIELD_PATTERN.test(key); +} + /** * Recursively strip credential fields from a JSON-like object. * Returns a new object with sensitive values replaced by a placeholder. * Any value type (string, object, boolean, number, null) is stripped if - * the key matches CREDENTIAL_FIELDS. + * the key matches CREDENTIAL_FIELDS or CREDENTIAL_FIELD_PATTERN. */ function stripCredentials(obj: unknown): unknown { if (obj === null || obj === undefined) return obj; @@ -573,7 +588,7 @@ function stripCredentials(obj: unknown): unknown { const result: Record = {}; for (const [key, value] of Object.entries(obj as Record)) { - if (CREDENTIAL_FIELDS.has(key)) { + if (isCredentialField(key)) { result[key] = "[STRIPPED_BY_MIGRATION]"; } else { result[key] = stripCredentials(value); @@ -598,7 +613,7 @@ function sanitizeCredentialsInBundle(preparedStateDir: string): void { const configPath = path.join(preparedStateDir, "openclaw.json"); if (existsSync(configPath)) { const raw = readFileSync(configPath, "utf-8"); - const config = JSON.parse(raw) as Record; + const config = JSON5.parse(raw) as Record; const sanitized = stripCredentials(config) as Record; writeFileSync(configPath, JSON.stringify(sanitized, null, 2)); } @@ -633,11 +648,13 @@ function walkAndStripCredentials(dirPath: string, targetName: string): void { const fullPath = path.join(dirPath, entry); try { const stat = lstatSync(fullPath); + // Skip symlinks — only sanitize real files within the snapshot + if (stat.isSymbolicLink()) continue; if (stat.isDirectory()) { walkAndStripCredentials(fullPath, targetName); } else if (entry === targetName) { const raw = readFileSync(fullPath, "utf-8"); - const config = JSON.parse(raw) as Record; + const config = JSON5.parse(raw) as Record; const sanitized = stripCredentials(config) as Record; writeFileSync(fullPath, JSON.stringify(sanitized, null, 2)); } @@ -669,6 +686,8 @@ function walkAndRemoveFile(dirPath: string, targetName: string): void { const fullPath = path.join(dirPath, entry); try { const stat = lstatSync(fullPath); + // Skip symlinks — only operate on real files within the snapshot + if (stat.isSymbolicLink()) continue; if (stat.isDirectory()) { walkAndRemoveFile(fullPath, targetName); } else if (entry === targetName) { diff --git a/test/security-credential-exposure.test.js b/test/security-credential-exposure.test.js index 458e8e807..8824e5b25 100644 --- a/test/security-credential-exposure.test.js +++ b/test/security-credential-exposure.test.js @@ -2,10 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 // // Security tests for migration credential sanitization. -// Demonstrates the credential exposure vulnerability and verifies the fix. -// -// Note: Blueprint digest tests were removed — the verify.ts and resolve.ts -// modules were deleted upstream in PR #492 (CLI commands removed). +// Verifies that the sanitization logic correctly strips credentials from +// migration bundles before they enter the sandbox. const { describe, it } = require("node:test"); const assert = require("node:assert/strict"); @@ -20,6 +18,51 @@ const FAKE_GITHUB_TOKEN = "test-fake-github-token-1111111111111111"; const FAKE_NPM_TOKEN = "test-fake-npm-token-2222222222222222"; const FAKE_GATEWAY_TOKEN = "test-fake-gateway-token-333333333333"; +// SYNC REQUIRED: if CREDENTIAL_FIELDS, CREDENTIAL_FIELD_PATTERN, stripCredentials, +// or walkAndRemoveFile change in migration-state.ts, update the copies here. +// Divergence is a silent false-pass bug. +const CREDENTIAL_FIELDS = new Set([ + "apiKey", "api_key", "token", "secret", "password", "resolvedKey", +]); +const CREDENTIAL_FIELD_PATTERN = + /(?:access|refresh|client|bearer|auth|api|private|public|signing|session)(?:Token|Key|Secret|Password)$/; + +function isCredentialField(key) { + return CREDENTIAL_FIELDS.has(key) || CREDENTIAL_FIELD_PATTERN.test(key); +} + +function stripCredentials(obj) { + if (obj === null || obj === undefined) return obj; + if (typeof obj !== "object") return obj; + if (Array.isArray(obj)) return obj.map(stripCredentials); + const result = {}; + for (const [key, value] of Object.entries(obj)) { + if (isCredentialField(key)) { + result[key] = "[STRIPPED_BY_MIGRATION]"; + } else { + result[key] = stripCredentials(value); + } + } + return result; +} + +function walkAndRemoveFile(dirPath, targetName) { + let entries; + try { entries = fs.readdirSync(dirPath); } catch { return; } + for (const entry of entries) { + const fullPath = path.join(dirPath, entry); + try { + const stat = fs.lstatSync(fullPath); + if (stat.isSymbolicLink()) continue; + if (stat.isDirectory()) { + walkAndRemoveFile(fullPath, targetName); + } else if (entry === targetName) { + fs.rmSync(fullPath, { force: true }); + } + } catch { /* non-fatal */ } + } +} + // ═══════════════════════════════════════════════════════════════════ // Helper: create a mock ~/.openclaw directory with credential files // ═══════════════════════════════════════════════════════════════════ @@ -81,84 +124,33 @@ function createMockOpenClawHome(tmpDir) { } // ═══════════════════════════════════════════════════════════════════ -// 1. Migration copies ALL credentials into sandbox (demonstrates vuln) +// 1. Sanitization deletes auth-profiles.json and strips config tokens // ═══════════════════════════════════════════════════════════════════ -describe("Migration credential exposure (pre-fix behavior)", () => { - it("raw cpSync copies auth-profiles.json with all tokens into sandbox", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-poc-")); - try { - const mock = createMockOpenClawHome(tmpDir); - - // Simulate the vulnerable codepath: cpSync(stateDir, snapshotDir) - const snapshotDir = path.join(tmpDir, "snapshot", "openclaw"); - fs.cpSync(mock.stateDir, snapshotDir, { recursive: true }); - - const authPath = path.join(snapshotDir, "agents", "main", "agent", "auth-profiles.json"); - const configPath = path.join(snapshotDir, "openclaw.json"); - - assert.ok(fs.existsSync(authPath), "auth-profiles.json copied into sandbox"); - assert.ok(fs.existsSync(configPath), "openclaw.json copied into sandbox"); - - const stolenAuth = JSON.parse(fs.readFileSync(authPath, "utf-8")); - const stolenConfig = JSON.parse(fs.readFileSync(configPath, "utf-8")); - - // All tokens are fully readable — this is the vulnerability - assert.strictEqual(stolenAuth["nvidia:manual"].resolvedKey, FAKE_NVIDIA_KEY); - assert.strictEqual(stolenAuth["github:pat"].token, FAKE_GITHUB_TOKEN); - assert.strictEqual(stolenAuth["npm:publish"].token, FAKE_NPM_TOKEN); - assert.strictEqual(stolenConfig.gateway.auth.token, FAKE_GATEWAY_TOKEN); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - }); -}); - -// ═══════════════════════════════════════════════════════════════════ -// 2. Fix verification — sanitized migration blocks the attack chain -// ═══════════════════════════════════════════════════════════════════ -describe("Fix verification: sanitized migration blocks credential theft", () => { - it("sanitizeCredentialsInBundle deletes auth-profiles.json and strips config tokens", () => { +describe("Migration credential sanitization", () => { + it("deletes auth-profiles.json and strips credential fields from openclaw.json", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fix-")); try { const mock = createMockOpenClawHome(tmpDir); - // Simulate snapshot + prepare + // Simulate snapshot copy const bundleDir = path.join(tmpDir, "bundle", "openclaw"); fs.cpSync(mock.stateDir, bundleDir, { recursive: true }); - // Apply the same sanitization logic from the fix in migration-state.ts - const CREDENTIAL_FIELDS = new Set([ - "apiKey", "api_key", "token", "secret", "password", "resolvedKey", "keyRef", - ]); - - function stripCredentials(obj) { - if (obj === null || obj === undefined) return obj; - if (typeof obj !== "object") return obj; - if (Array.isArray(obj)) return obj.map(stripCredentials); - const result = {}; - for (const [key, value] of Object.entries(obj)) { - if (CREDENTIAL_FIELDS.has(key) && (typeof value === "string" || typeof value === "object")) { - result[key] = "[STRIPPED_BY_MIGRATION]"; - } else { - result[key] = stripCredentials(value); - } - } - return result; + // Apply sanitization (mirrors production sanitizeCredentialsInBundle) + const agentsDir = path.join(bundleDir, "agents"); + if (fs.existsSync(agentsDir)) { + walkAndRemoveFile(agentsDir, "auth-profiles.json"); } - // Delete auth-profiles.json - const authPath = path.join(bundleDir, "agents", "main", "agent", "auth-profiles.json"); - if (fs.existsSync(authPath)) fs.rmSync(authPath, { force: true }); - - // Strip config credentials const configPath = path.join(bundleDir, "openclaw.json"); const config = JSON.parse(fs.readFileSync(configPath, "utf-8")); fs.writeFileSync(configPath, JSON.stringify(stripCredentials(config), null, 2)); // Verify: auth-profiles.json deleted + const authPath = path.join(bundleDir, "agents", "main", "agent", "auth-profiles.json"); assert.ok(!fs.existsSync(authPath), "auth-profiles.json must be deleted"); - // Verify: config credentials stripped + // Verify: credential fields stripped const sanitized = JSON.parse(fs.readFileSync(configPath, "utf-8")); assert.strictEqual(sanitized.nvidia.apiKey, "[STRIPPED_BY_MIGRATION]"); assert.strictEqual(sanitized.gateway.auth.token, "[STRIPPED_BY_MIGRATION]"); @@ -173,4 +165,68 @@ describe("Fix verification: sanitized migration blocks credential theft", () => fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + it("strips credential fields matched by pattern (e.g. accessToken, privateKey)", () => { + const config = { + provider: { + accessToken: "test-access-token-value", + refreshToken: "test-refresh-token-value", + privateKey: "test-private-key-value", + clientSecret: "test-client-secret-value", + displayName: "should-be-preserved", + sortKey: "should-also-be-preserved", + }, + }; + + const sanitized = stripCredentials(config); + + assert.strictEqual(sanitized.provider.accessToken, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.provider.refreshToken, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.provider.privateKey, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.provider.clientSecret, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.provider.displayName, "should-be-preserved"); + assert.strictEqual(sanitized.provider.sortKey, "should-also-be-preserved"); + }); + + it("skips symlinks during credential sanitization walk", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-symlink-")); + try { + // Create a real auth-profiles.json outside the bundle + const outsideDir = path.join(tmpDir, "outside"); + fs.mkdirSync(outsideDir, { recursive: true }); + const outsideAuth = path.join(outsideDir, "auth-profiles.json"); + fs.writeFileSync(outsideAuth, JSON.stringify({ shouldNotBeDeleted: true })); + + // Create bundle with symlink pointing to outside + const bundleDir = path.join(tmpDir, "bundle"); + const agentsDir = path.join(bundleDir, "agents"); + fs.mkdirSync(agentsDir, { recursive: true }); + fs.symlinkSync(outsideAuth, path.join(agentsDir, "auth-profiles.json")); + + // Walk should skip the symlink + walkAndRemoveFile(agentsDir, "auth-profiles.json"); + + // The outside file should still exist (symlink was skipped) + assert.ok(fs.existsSync(outsideAuth), "file outside bundle must not be deleted via symlink"); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("strips all value types when key matches (not just strings/objects)", () => { + const config = { + token: 12345, // number — isCredentialField fires before recursive call + secret: true, // boolean — same + password: null, // null — isCredentialField fires first; early-return is NOT reached + apiKey: "string-val", // string + normalField: "keep", + }; + + const sanitized = stripCredentials(config); + assert.strictEqual(sanitized.token, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.secret, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.password, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.apiKey, "[STRIPPED_BY_MIGRATION]"); + assert.strictEqual(sanitized.normalField, "keep"); + }); }); From 4b1dc9c303ca8445c336b3a404f0b76dbc54860c Mon Sep 17 00:00:00 2001 From: Lucas Wang Date: Sun, 22 Mar 2026 23:46:04 +0800 Subject: [PATCH 6/6] fix: add missing docstrings to resolve lint coverage failure Address PR review feedback from cv: add JSDoc comments to walkAndRemoveFile, createMockOpenClawHome, and stripCredentials. --- nemoclaw/src/commands/migration-state.ts | 1 + test/security-credential-exposure.test.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index 8033dfde7..2ace4b9b0 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -674,6 +674,7 @@ function removeAuthProfileFiles(preparedStateDir: string): void { walkAndRemoveFile(agentsDir, "auth-profiles.json"); } +/** Recursively walk dirPath and remove any files matching targetName. */ function walkAndRemoveFile(dirPath: string, targetName: string): void { let entries: string[]; try { diff --git a/test/security-credential-exposure.test.js b/test/security-credential-exposure.test.js index 8824e5b25..f040aa28f 100644 --- a/test/security-credential-exposure.test.js +++ b/test/security-credential-exposure.test.js @@ -31,6 +31,7 @@ function isCredentialField(key) { return CREDENTIAL_FIELDS.has(key) || CREDENTIAL_FIELD_PATTERN.test(key); } +/** Local reimplementation of stripCredentials for test isolation. */ function stripCredentials(obj) { if (obj === null || obj === undefined) return obj; if (typeof obj !== "object") return obj; @@ -66,6 +67,7 @@ function walkAndRemoveFile(dirPath, targetName) { // ═══════════════════════════════════════════════════════════════════ // Helper: create a mock ~/.openclaw directory with credential files // ═══════════════════════════════════════════════════════════════════ +/** Create a mock ~/.openclaw directory tree populated with fake credential files. */ function createMockOpenClawHome(tmpDir) { const stateDir = path.join(tmpDir, ".openclaw"); fs.mkdirSync(stateDir, { recursive: true });