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", ], ], }, diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index 4421943d2..2ace4b9b0 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -544,6 +544,162 @@ 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. + */ +// keyRef is metadata (points to env var source), not a secret value — excluded intentionally. +const CREDENTIAL_FIELDS = new Set([ + "apiKey", + "api_key", + "token", + "secret", + "password", + "resolvedKey", +]); + +/** + * 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 or CREDENTIAL_FIELD_PATTERN. + */ +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 (isCredentialField(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 = JSON5.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"); + + // 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); + // 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 = JSON5.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}`); + } + } +} + +/** + * 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"); +} + +/** Recursively walk dirPath and remove any files matching targetName. */ +function walkAndRemoveFile(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); + // Skip symlinks — only operate on real files within the snapshot + if (stat.isSymbolicLink()) continue; + if (stat.isDirectory()) { + walkAndRemoveFile(fullPath, targetName); + } else if (entry === targetName) { + rmSync(fullPath, { force: true }); + } + } catch (err) { + console.warn(`[credential-sanitize] Unable to process ${fullPath}: ${err}`); + } + } +} + function prepareSandboxState(snapshotDir: string, manifest: SnapshotManifest): string { const preparedStateDir = path.join(snapshotDir, "sandbox-bundle", "openclaw"); rmSync(preparedStateDir, { recursive: true, force: true }); @@ -560,6 +716,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 +760,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..f040aa28f --- /dev/null +++ b/test/security-credential-exposure.test.js @@ -0,0 +1,234 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Security tests for migration credential sanitization. +// 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"); +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"; + +// 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); +} + +/** Local reimplementation of stripCredentials for test isolation. */ +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 +// ═══════════════════════════════════════════════════════════════════ +/** 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 }); + + 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. Sanitization 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 copy + const bundleDir = path.join(tmpDir, "bundle", "openclaw"); + fs.cpSync(mock.stateDir, bundleDir, { recursive: true }); + + // Apply sanitization (mirrors production sanitizeCredentialsInBundle) + const agentsDir = path.join(bundleDir, "agents"); + if (fs.existsSync(agentsDir)) { + walkAndRemoveFile(agentsDir, "auth-profiles.json"); + } + + 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: 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]"); + + // 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 }); + } + }); + + 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"); + }); +});