From 9c768bb72c82e7e9ab874106f1c98d6e20b78892 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Wed, 29 Jul 2026 02:56:10 +0530 Subject: [PATCH 1/3] fix(security): scrub migration and backup credentials consistently Align host-to-sandbox migration and rebuild backups with the shared credential filter so bot tokens, env secrets, Authorization headers, Hermes YAML, and .env PASS fields cannot survive snapshot sanitization. Signed-off-by: Ayush7614 --- nemoclaw/src/commands/migration-state.ts | 63 +------ .../src/security/credential-filter.test.ts | 81 ++++++++ nemoclaw/src/security/credential-filter.ts | 177 ++++++++++++++++++ src/lib/security/credential-filter.test.ts | 83 +++++++- src/lib/security/credential-filter.ts | 106 ++++++++++- src/lib/state/sandbox.ts | 27 ++- 6 files changed, 453 insertions(+), 84 deletions(-) create mode 100644 nemoclaw/src/security/credential-filter.test.ts create mode 100644 nemoclaw/src/security/credential-filter.ts diff --git a/nemoclaw/src/commands/migration-state.ts b/nemoclaw/src/commands/migration-state.ts index 43aab3d3d31..5f8b494402d 100644 --- a/nemoclaw/src/commands/migration-state.ts +++ b/nemoclaw/src/commands/migration-state.ts @@ -20,6 +20,7 @@ import { create as createTar } from "tar"; import { createHash } from "node:crypto"; import JSON5 from "json5"; import type { PluginLogger } from "../index.js"; +import { isSensitiveFile, stripCredentials } from "../security/credential-filter.js"; import { isObjectRecord, type UnknownRecord } from "../shared/object-record.js"; const SANDBOX_MIGRATION_DIR = "/sandbox/.nemoclaw/migration"; @@ -504,65 +505,9 @@ export function detectHostOpenClaw(env: NodeJS.ProcessEnv = process.env): HostOp } // --------------------------------------------------------------------------- -// Credential sanitization +// Credential sanitization (shared with nemoclaw/src/security/credential-filter) // --------------------------------------------------------------------------- -/** - * Basenames that MUST NOT be copied into snapshot bundles. - * These files contain credential references or session tokens - * that should never cross the sandbox boundary. - */ -const CREDENTIAL_SENSITIVE_BASENAMES = new Set(["auth-profiles.json"]); - -/** - * Credential field names that MUST be stripped from config files - * before they enter the sandbox. Credentials should be injected - * at runtime via OpenShell's provider credential mechanism. - */ -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)$/; - -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. - */ -function stripCredentials(obj: unknown): unknown { - if (Array.isArray(obj)) return obj.map(stripCredentials); - if (!isObjectRecord(obj)) return obj; - - return stripCredentialsFromRecord(obj); -} - -function stripCredentialsFromRecord(obj: UnknownRecord): UnknownRecord { - const result: UnknownRecord = {}; - for (const [key, value] of Object.entries(obj)) { - if (isCredentialField(key)) { - result[key] = "[STRIPPED_BY_MIGRATION]"; - } else { - result[key] = stripCredentials(value); - } - } - return result; -} - /** * Strip credential fields from openclaw.json and remove the gateway * config section (contains auth tokens — regenerated by sandbox entrypoint). @@ -571,7 +516,7 @@ function sanitizeConfigFile(configPath: string): void { const config = loadConfigDocument(configPath); if (!config) return; delete config.gateway; - const sanitized = stripCredentialsFromRecord(config); + const sanitized = stripCredentials(config) as UnknownRecord; writeFileSync(configPath, JSON.stringify(sanitized, null, 2)); chmodSync(configPath, 0o600); } @@ -593,7 +538,7 @@ function copyDirectory( cpSync(sourcePath, destinationPath, { recursive: true, filter: options?.stripCredentials - ? (source: string) => !CREDENTIAL_SENSITIVE_BASENAMES.has(path.basename(source).toLowerCase()) + ? (source: string) => !isSensitiveFile(path.basename(source)) : undefined, }); } diff --git a/nemoclaw/src/security/credential-filter.test.ts b/nemoclaw/src/security/credential-filter.test.ts new file mode 100644 index 00000000000..65e21f117b1 --- /dev/null +++ b/nemoclaw/src/security/credential-filter.test.ts @@ -0,0 +1,81 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + CREDENTIAL_PLACEHOLDER, + isCredentialField, + isSafeCredentialPlaceholder, + isSensitiveFile, + stripCredentials, + valueLooksLikeSecret, +} from "./credential-filter.js"; + +describe("plugin credential-filter", () => { + it("treats Slack botToken, Authorization, and GITHUB_TOKEN as credential fields", () => { + expect(isCredentialField("botToken")).toBe(true); + expect(isCredentialField("appToken")).toBe(true); + expect(isCredentialField("Authorization")).toBe(true); + expect(isCredentialField("GITHUB_TOKEN")).toBe(true); + expect(isCredentialField("DB_PASS")).toBe(true); + expect(isCredentialField("publicKey")).toBe(false); + expect(isCredentialField("NODE_ENV")).toBe(false); + }); + + it("strips channel tokens, headers, env secrets, and CLI flag args", () => { + const result = stripCredentials({ + channels: { + slack: { + accounts: { + default: { + botToken: "xoxb-raw-slack-token", + appToken: "xapp-raw-app-token", + }, + }, + }, + }, + mcp: { + headers: { Authorization: "Bearer sk-abcdefghijklmnopqrstuvwxyz" }, + env: { GITHUB_TOKEN: "ghp_abcdefghijklmnopqrstuvwxyz0123456789", NODE_ENV: "test" }, + args: ["--api-key", "opaque-secret-value", "--verbose"], + }, + model: "keep-me", + publicKey: "verify-me", + apiKey: "openshell:resolve:env:NVIDIA_API_KEY", + }) as Record; + + const channels = result.channels as { + slack: { accounts: { default: { botToken: string; appToken: string } } }; + }; + expect(channels.slack.accounts.default.botToken).toBe(CREDENTIAL_PLACEHOLDER); + expect(channels.slack.accounts.default.appToken).toBe(CREDENTIAL_PLACEHOLDER); + + const mcp = result.mcp as { + headers: { Authorization: string }; + env: { GITHUB_TOKEN: string; NODE_ENV: string }; + args: string[]; + }; + expect(mcp.headers.Authorization).toBe(CREDENTIAL_PLACEHOLDER); + expect(mcp.env.GITHUB_TOKEN).toBe(CREDENTIAL_PLACEHOLDER); + expect(mcp.env.NODE_ENV).toBe("test"); + expect(mcp.args).toEqual(["--api-key", CREDENTIAL_PLACEHOLDER, "--verbose"]); + expect(result.model).toBe("keep-me"); + expect(result.publicKey).toBe("verify-me"); + expect(result.apiKey).toBe("openshell:resolve:env:NVIDIA_API_KEY"); + }); + + it("preserves safe placeholders and detects secret-shaped values", () => { + expect(isSafeCredentialPlaceholder("unused")).toBe(true); + expect(isSafeCredentialPlaceholder("openshell:resolve:env:TOKEN")).toBe(true); + expect(valueLooksLikeSecret("sk-abcdefghijklmnopqrstuvwxyz")).toBe(true); + expect(valueLooksLikeSecret("not-a-secret")).toBe(false); + }); + + it("excludes auth state basenames from migration copies", () => { + expect(isSensitiveFile("auth-profiles.json")).toBe(true); + expect(isSensitiveFile("auth.json")).toBe(true); + expect(isSensitiveFile("chatgpt-auth.json")).toBe(true); + expect(isSensitiveFile("openclaw.json")).toBe(false); + }); +}); diff --git a/nemoclaw/src/security/credential-filter.ts b/nemoclaw/src/security/credential-filter.ts new file mode 100644 index 00000000000..dce343eda68 --- /dev/null +++ b/nemoclaw/src/security/credential-filter.ts @@ -0,0 +1,177 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Credential stripping for host→sandbox migration snapshots. +// Kept in parity with src/lib/security/credential-filter.ts so migration +// cannot leave channel tokens, env secrets, or auth headers in the sandbox. + +import { isObjectRecord, type UnknownRecord } from "../shared/object-record.js"; + +export const CREDENTIAL_PLACEHOLDER = "[STRIPPED_BY_MIGRATION]"; + +/** + * Basenames that MUST NOT be copied into snapshot bundles. + */ +export const CREDENTIAL_SENSITIVE_BASENAMES = new Set([ + "auth-profiles.json", + "auth.json", + "chatgpt-auth.json", +]); + +const CREDENTIAL_FIELDS = new Set([ + "apiKey", + "api_key", + "token", + "secret", + "password", + "pass", + "passwd", + "resolvedKey", +]); + +const CREDENTIAL_FIELD_PATTERN = + /(?:access|refresh|client|bearer|auth|api|private|public|signing|session|bot|app)(?:Token|Key|Secret|Password)$/; + +const ENV_SECRET_FIELD_PATTERN = + /^(?:[A-Z0-9]+_)*(?:TOKEN|KEY|SECRET|PASSWORD|PASSWD|PASS|PASSPHRASE|CREDENTIAL)S?$/; + +const CREDENTIAL_HEADER_NAMES: ReadonlySet = new Set([ + "authorization", + "proxy-authorization", + "cookie", + "set-cookie", +]); + +const HEADER_CREDENTIAL_PATTERN = /-(?:key|token|secret|password|passphrase|credential|auth)s?$/i; + +const PUBLIC_KEY_FIELD_PATTERN = /(?:^|[-_])public[-_]?keys?$/i; + +const SAFE_CREDENTIAL_PLACEHOLDER_PATTERNS: readonly RegExp[] = [ + /^openshell:resolve:env:[A-Za-z0-9_]+$/, + /^Bearer\s+openshell:resolve:env:[A-Za-z0-9_]+$/i, + /^xoxb-OPENSHELL-RESOLVE-ENV-[A-Za-z0-9_]+$/, + /^xapp-OPENSHELL-RESOLVE-ENV-[A-Za-z0-9_]+$/, +]; + +const SAFE_CREDENTIAL_PLACEHOLDER_LITERALS: ReadonlySet = new Set([ + "unused", + CREDENTIAL_PLACEHOLDER, +]); + +/** High-confidence raw secret shapes used as a value-level backstop. */ +const VALUE_SECRET_PATTERNS: readonly RegExp[] = [ + /nvapi-[A-Za-z0-9_-]{10,}/, + /ghp_[A-Za-z0-9_-]{10,}/, + /sk-proj-[A-Za-z0-9_-]{10,}/, + /sk-ant-[A-Za-z0-9_-]{10,}/, + /sk-[A-Za-z0-9_-]{20,}/, + /(?:xox[bpas]|xapp)-[A-Za-z0-9-]{10,}/, + /A(?:K|S)IA[A-Z0-9]{16}/, + /hf_[A-Za-z0-9]{10,}/, + /tvly-[A-Za-z0-9_-]{10,}/, +]; + +function hasPassCredentialSegment(key: string): boolean { + const normalized = key + .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2") + .replace(/([a-z0-9])([A-Z])/g, "$1_$2") + .replace(/[^A-Za-z0-9]+/g, "_") + .replace(/^_+|_+$/g, "") + .toLowerCase(); + return ( + normalized === "pass" || + normalized === "passwd" || + normalized.endsWith("_pass") || + normalized.endsWith("_passwd") + ); +} + +export function isCredentialField(key: string): boolean { + if (PUBLIC_KEY_FIELD_PATTERN.test(key)) return false; + return ( + CREDENTIAL_FIELDS.has(key) || + CREDENTIAL_FIELD_PATTERN.test(key) || + hasPassCredentialSegment(key) || + ENV_SECRET_FIELD_PATTERN.test(key) || + HEADER_CREDENTIAL_PATTERN.test(key) || + CREDENTIAL_HEADER_NAMES.has(key.toLowerCase()) + ); +} + +export function valueLooksLikeSecret(value: string): boolean { + return VALUE_SECRET_PATTERNS.some((pattern) => pattern.test(value)); +} + +export function isSafeCredentialPlaceholder(value: unknown): boolean { + if (typeof value !== "string") return false; + const withoutScheme = value.replace(/^Bearer\s+/i, ""); + if ( + SAFE_CREDENTIAL_PLACEHOLDER_LITERALS.has(value) || + SAFE_CREDENTIAL_PLACEHOLDER_LITERALS.has(withoutScheme) + ) { + return true; + } + return SAFE_CREDENTIAL_PLACEHOLDER_PATTERNS.some((pattern) => pattern.test(value)); +} + +function scrubConfigValue(value: unknown): unknown { + if (typeof value === "string") { + if (isSafeCredentialPlaceholder(value)) return value; + return valueLooksLikeSecret(value) ? CREDENTIAL_PLACEHOLDER : value; + } + return stripCredentials(value); +} + +function cliFlagName(token: string): string | null { + const match = /^--?([A-Za-z0-9][A-Za-z0-9._-]*)$/.exec(token); + return match ? match[1] : null; +} + +function scrubArrayElement(value: unknown, previous: unknown): unknown { + if (typeof value !== "string") return stripCredentials(value); + if (isSafeCredentialPlaceholder(value)) return value; + + const eq = value.indexOf("="); + if (eq > 0 && value.startsWith("-")) { + const flagName = cliFlagName(value.slice(0, eq)); + if (flagName && isCredentialField(flagName)) { + const inlineValue = value.slice(eq + 1); + return isSafeCredentialPlaceholder(inlineValue) + ? value + : `${value.slice(0, eq)}=${CREDENTIAL_PLACEHOLDER}`; + } + } + + if (!value.startsWith("-") && typeof previous === "string") { + const prevFlag = cliFlagName(previous); + if (prevFlag && isCredentialField(prevFlag)) return CREDENTIAL_PLACEHOLDER; + } + + return valueLooksLikeSecret(value) ? CREDENTIAL_PLACEHOLDER : value; +} + +/** + * Recursively strip credential fields from a JSON-like object. + */ +export 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((value, index) => scrubArrayElement(value, obj[index - 1])); + } + if (!isObjectRecord(obj)) return obj; + + const result: UnknownRecord = {}; + for (const [key, value] of Object.entries(obj)) { + if (isCredentialField(key)) { + result[key] = isSafeCredentialPlaceholder(value) ? value : CREDENTIAL_PLACEHOLDER; + } else { + result[key] = scrubConfigValue(value); + } + } + return result; +} + +export function isSensitiveFile(filename: string): boolean { + return CREDENTIAL_SENSITIVE_BASENAMES.has(filename.toLowerCase()); +} diff --git a/src/lib/security/credential-filter.test.ts b/src/lib/security/credential-filter.test.ts index d7a8f4005ba..60e96a5b459 100644 --- a/src/lib/security/credential-filter.test.ts +++ b/src/lib/security/credential-filter.test.ts @@ -11,6 +11,8 @@ import { isSafeCredentialPlaceholder, isSensitiveFile, sanitizeConfigFile, + sanitizeEnvFile, + sanitizeEnvFileContent, shouldScanSnapshotFileForCredentials, stripCredentials, } from "./credential-filter.js"; @@ -209,6 +211,82 @@ describe("sanitizeConfigFile", () => { expect(JSON.parse(readFileSync(targetPath, "utf-8"))).toEqual({ apiKey: "sk-secret" }); }); + + it("strips Hermes YAML credentials and removes gateway", () => { + const configPath = join(tmpDir, "config.yaml"); + writeFileSync( + configPath, + [ + "model: hermes", + "api_key: sk-hermes-secret-key-value", + "botToken: xoxb-slack-bot-token-value", + "publicKey: keep-me", + "gateway:", + " authToken: gw-token", + "env:", + " GITHUB_TOKEN: ghp_abcdefghijklmnopqrstuvwxyz0123456789", + " NODE_ENV: production", + "", + ].join("\n"), + ); + + sanitizeConfigFile(configPath); + + const result = readFileSync(configPath, "utf-8"); + expect(result).toContain("model: hermes"); + expect(result).toContain("publicKey: keep-me"); + expect(result).toContain("NODE_ENV: production"); + expect(result).toContain("[STRIPPED_BY_MIGRATION]"); + expect(result).not.toContain("sk-hermes-secret-key-value"); + expect(result).not.toContain("xoxb-slack-bot-token-value"); + expect(result).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz0123456789"); + expect(result).not.toContain("gateway:"); + }); +}); + +describe("sanitizeEnvFileContent", () => { + it("strips PASS/TOKEN secrets without over-matching KEYBOARD_LAYOUT", () => { + const input = [ + "# comment", + "NODE_ENV=production", + "KEYBOARD_LAYOUT=us", + "DB_PASS=super-secret", + "GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz0123456789", + "API_KEY=openshell:resolve:env:API_KEY", + "PASSPHRASE=raw-passphrase", + "", + ].join("\n"); + + const result = sanitizeEnvFileContent(input); + expect(result).toContain("NODE_ENV=production"); + expect(result).toContain("KEYBOARD_LAYOUT=us"); + expect(result).toContain("DB_PASS=[STRIPPED_BY_MIGRATION]"); + expect(result).toContain("GITHUB_TOKEN=[STRIPPED_BY_MIGRATION]"); + expect(result).toContain("API_KEY=openshell:resolve:env:API_KEY"); + expect(result).toContain("PASSPHRASE=[STRIPPED_BY_MIGRATION]"); + expect(result).toContain("# comment"); + }); +}); + +describe("sanitizeEnvFile", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "cred-env-test-")); + }); + + afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("rewrites .env credentials in place", () => { + const envPath = join(tmpDir, ".env"); + writeFileSync(envPath, "DB_PASS=secret\nLOG_LEVEL=info\n"); + sanitizeEnvFile(envPath); + expect(readFileSync(envPath, "utf-8")).toBe( + "DB_PASS=[STRIPPED_BY_MIGRATION]\nLOG_LEVEL=info\n", + ); + }); }); describe("isSensitiveFile", () => { @@ -229,11 +307,13 @@ describe("isSensitiveFile", () => { }); describe("shouldScanSnapshotFileForCredentials", () => { - it("scans runtime config and env files", () => { + it("scans runtime config, env, and Hermes YAML files", () => { expect(shouldScanSnapshotFileForCredentials("openclaw.json")).toBe(true); expect(shouldScanSnapshotFileForCredentials("config.json")).toBe(true); expect(shouldScanSnapshotFileForCredentials(".env")).toBe(true); expect(shouldScanSnapshotFileForCredentials("service.env")).toBe(true); + expect(shouldScanSnapshotFileForCredentials("config.yaml")).toBe(true); + expect(shouldScanSnapshotFileForCredentials("config.yml")).toBe(true); }); it("skips dependency lockfiles that can contain non-secret package metadata matches", () => { @@ -246,5 +326,6 @@ describe("shouldScanSnapshotFileForCredentials", () => { it("applies lockfile exclusions to paths by basename", () => { expect(shouldScanSnapshotFileForCredentials("/tmp/snapshot/package-lock.json")).toBe(false); expect(shouldScanSnapshotFileForCredentials("/tmp/snapshot/config.json")).toBe(true); + expect(shouldScanSnapshotFileForCredentials("/tmp/snapshot/config.yaml")).toBe(true); }); }); diff --git a/src/lib/security/credential-filter.ts b/src/lib/security/credential-filter.ts index d430e62bf16..fd715cd10a3 100644 --- a/src/lib/security/credential-filter.ts +++ b/src/lib/security/credential-filter.ts @@ -22,6 +22,7 @@ import { writeFileSync, } from "node:fs"; import { basename, dirname, join } from "node:path"; +import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; import { isObjectRecord } from "../core/json-types"; import { hasPassCredentialSegment, SECRET_PATTERNS } from "./secret-patterns"; @@ -358,23 +359,108 @@ function scrubArrayElement(value: ConfigValue, previous: ConfigValue): ConfigVal } /** - * Strip credential fields from a JSON config file in-place. + * Strip credential fields from a KEY=value env file body. + * Uses the same field-name rules as JSON scrubbing so `DB_PASS` and + * `PASSPHRASE` are stripped while benign names like `KEYBOARD_LAYOUT` + * and `NODE_ENV` are preserved. + */ +export function sanitizeEnvFileContent(content: string): string { + return content + .split("\n") + .map((line) => { + const trimmed = line.trim(); + if (trimmed === "" || trimmed.startsWith("#")) return line; + const eq = line.indexOf("="); + if (eq <= 0) return line; + const key = line.slice(0, eq).trim(); + if (!key || !isCredentialField(key)) return line; + const value = line.slice(eq + 1); + if (isSafeCredentialPlaceholder(value)) return line; + return `${line.slice(0, eq)}=${CREDENTIAL_PLACEHOLDER}`; + }) + .join("\n"); +} + +/** + * Strip credential lines from a `.env` file in-place. + */ +export function sanitizeEnvFile(filePath: string): void { + const raw = readRegularFileNoFollow(filePath); + if (raw === null) return; + writeFileAtomically(filePath, sanitizeEnvFileContent(raw)); +} + +function toConfigValue(value: unknown): ConfigValue | undefined { + if (value === null || value === undefined) return value; + if (typeof value === "boolean" || typeof value === "number" || typeof value === "string") { + return value; + } + if (Array.isArray(value)) { + const items: ConfigValue[] = []; + for (const entry of value) { + const converted = toConfigValue(entry); + if (converted === undefined && entry !== undefined && entry !== null) return undefined; + items.push(converted as ConfigValue); + } + return items; + } + if (!isObjectRecord(value)) return undefined; + const result: ConfigObject = {}; + for (const [key, entry] of Object.entries(value)) { + const converted = toConfigValue(entry); + if (converted === undefined && entry !== undefined && entry !== null) return undefined; + result[key] = converted as ConfigValue; + } + return result; +} + +/** + * Strip credential fields from a Hermes YAML config file in-place. + * Removes the "gateway" section when present (auth tokens — regenerated + * at startup), matching JSON sanitization. + */ +export function sanitizeYamlConfigFile(configPath: string): void { + const rawConfig = readRegularFileNoFollow(configPath); + if (rawConfig === null) return; + let parsed: unknown; + try { + parsed = parseYaml(rawConfig); + } catch { + return; + } + const configValue = toConfigValue(parsed); + if (!isConfigObject(configValue)) return; + + const { gateway: _gateway, ...config } = configValue; + const sanitized = stripCredentials(config); + writeFileAtomically(configPath, stringifyYaml(sanitized)); +} + +/** + * Strip credential fields from a JSON or YAML config file in-place. * Removes the "gateway" section (contains auth tokens — regenerated at startup). + * JSON is preferred when the file parses as JSON; otherwise YAML is tried + * so Hermes `config.yaml` secrets are scrubbed from rebuild backups. */ export function sanitizeConfigFile(configPath: string): void { const rawConfig = readRegularFileNoFollow(configPath); if (rawConfig === null) return; - let parsed: ConfigValue; + try { - parsed = parseJson(rawConfig); + const parsed = parseJson(rawConfig); + if (!isConfigObject(parsed)) return; + const { gateway: _gateway, ...config } = parsed; + const sanitized = stripCredentials(config); + writeFileAtomically(configPath, JSON.stringify(sanitized, null, 2)); + return; } catch { - return; // Not valid JSON — skip (may be YAML for Hermes) + // Fall through to YAML for Hermes and other non-JSON configs. } - if (!isConfigObject(parsed)) return; - const { gateway: _gateway, ...config } = parsed; - const sanitized = stripCredentials(config); - writeFileAtomically(configPath, JSON.stringify(sanitized, null, 2)); + const normalized = basename(configPath).toLowerCase(); + if (normalized.endsWith(".yaml") || normalized.endsWith(".yml")) { + sanitizeYamlConfigFile(configPath); + } } /** @@ -394,6 +480,8 @@ export function shouldScanSnapshotFileForCredentials(filename: string): boolean return ( normalizedBasename === ".env" || normalizedBasename.endsWith(".env") || - normalizedBasename.endsWith(".json") + normalizedBasename.endsWith(".json") || + normalizedBasename.endsWith(".yaml") || + normalizedBasename.endsWith(".yml") ); } diff --git a/src/lib/state/sandbox.ts b/src/lib/state/sandbox.ts index af6040273a7..8584c72be1a 100644 --- a/src/lib/state/sandbox.ts +++ b/src/lib/state/sandbox.ts @@ -42,7 +42,11 @@ import { } from "../domain/backup-failure.js"; import { shellQuote } from "../runner.js"; import { createTempSshConfig } from "../sandbox/temp-ssh-config.js"; -import { isSensitiveFile, sanitizeConfigFile } from "../security/credential-filter.js"; +import { + isSensitiveFile, + sanitizeConfigFile, + sanitizeEnvFile, +} from "../security/credential-filter.js"; import { buildRestoreCleanupCommand, buildRestoreTarArgs, @@ -639,24 +643,17 @@ function sanitizeBackupDirectory(dirPath: string): void { } catch { /* best effort */ } - } else if (entry.name.endsWith(".json")) { + } else if ( + entry.name.endsWith(".json") || + entry.name.endsWith(".yaml") || + entry.name.endsWith(".yml") + ) { + // JSON (OpenClaw) and YAML (Hermes config.yaml) both carry secrets. sanitizeConfigFile(fullPath); } else if (entry.name === ".env" || entry.name.endsWith(".env")) { - // Strip credential lines from .env files (KEY=value format). // Hermes stores API keys in .env alongside config.yaml. try { - const envContent = readFileSync(fullPath, "utf-8"); - const filtered = envContent - .split("\n") - .map((line) => { - const key = line.split("=")[0]?.trim().toUpperCase() || ""; - if (/KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL/.test(key)) { - return `${line.split("=")[0]}=[STRIPPED_BY_MIGRATION]`; - } - return line; - }) - .join("\n"); - writeFileSync(fullPath, filtered); + sanitizeEnvFile(fullPath); chmodSync(fullPath, 0o600); } catch { /* best effort */ From 48037bad26adc9bc374854363b2e1d9076ce0c8e Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Wed, 29 Jul 2026 03:23:40 +0530 Subject: [PATCH 2/3] fix(security): strip export-prefixed secrets in .env scrubbing Shell-sourced env files often use `export KEY=value`, which bypassed key detection. Strip the prefix before credential-field matching. Signed-off-by: Ayush7614 --- src/lib/security/credential-filter.test.ts | 16 ++++++++++++++++ src/lib/security/credential-filter.ts | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/lib/security/credential-filter.test.ts b/src/lib/security/credential-filter.test.ts index 60e96a5b459..5ee31b2a251 100644 --- a/src/lib/security/credential-filter.test.ts +++ b/src/lib/security/credential-filter.test.ts @@ -266,6 +266,22 @@ describe("sanitizeEnvFileContent", () => { expect(result).toContain("PASSPHRASE=[STRIPPED_BY_MIGRATION]"); expect(result).toContain("# comment"); }); + + it("strips credential keys that use a leading export prefix", () => { + const input = [ + "export DB_PASS=super-secret", + "export NODE_ENV=production", + " export GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz0123456789", + "", + ].join("\n"); + + const result = sanitizeEnvFileContent(input); + expect(result).toContain("export DB_PASS=[STRIPPED_BY_MIGRATION]"); + expect(result).toContain("export NODE_ENV=production"); + expect(result).toContain("export GITHUB_TOKEN=[STRIPPED_BY_MIGRATION]"); + expect(result).not.toContain("super-secret"); + expect(result).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz0123456789"); + }); }); describe("sanitizeEnvFile", () => { diff --git a/src/lib/security/credential-filter.ts b/src/lib/security/credential-filter.ts index fd715cd10a3..e55e4d8614a 100644 --- a/src/lib/security/credential-filter.ts +++ b/src/lib/security/credential-filter.ts @@ -372,7 +372,9 @@ export function sanitizeEnvFileContent(content: string): string { if (trimmed === "" || trimmed.startsWith("#")) return line; const eq = line.indexOf("="); if (eq <= 0) return line; - const key = line.slice(0, eq).trim(); + const rawKey = line.slice(0, eq).trim(); + // Shell-sourced .env files often use `export KEY=value`. + const key = rawKey.replace(/^export\s+/i, "").trim(); if (!key || !isCredentialField(key)) return line; const value = line.slice(eq + 1); if (isSafeCredentialPlaceholder(value)) return line; From 2f9ccb1888c07ee19b9f9564c66b96c6a11081a1 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Wed, 29 Jul 2026 03:42:15 +0530 Subject: [PATCH 3/3] fix(security): harden credential scrubbing fail-closed paths Preserve unset credential fields, omit unsanitizable Hermes YAML from backups, normalize backup file extensions, and align migration secret shape detection with the canonical token patterns. Signed-off-by: Ayush7614 --- .../src/security/credential-filter.test.ts | 12 +++++++ nemoclaw/src/security/credential-filter.ts | 22 +++++++++++-- src/lib/security/credential-filter.test.ts | 18 ++++++++++- src/lib/security/credential-filter.ts | 31 ++++++++++++------- src/lib/state/sandbox.ts | 19 +++++++----- 5 files changed, 80 insertions(+), 22 deletions(-) diff --git a/nemoclaw/src/security/credential-filter.test.ts b/nemoclaw/src/security/credential-filter.test.ts index 65e21f117b1..02423a14027 100644 --- a/nemoclaw/src/security/credential-filter.test.ts +++ b/nemoclaw/src/security/credential-filter.test.ts @@ -69,9 +69,21 @@ describe("plugin credential-filter", () => { expect(isSafeCredentialPlaceholder("unused")).toBe(true); expect(isSafeCredentialPlaceholder("openshell:resolve:env:TOKEN")).toBe(true); expect(valueLooksLikeSecret("sk-abcdefghijklmnopqrstuvwxyz")).toBe(true); + expect(valueLooksLikeSecret("glpat-abcdefghijklmnopqrst")).toBe(true); + expect(valueLooksLikeSecret("nvcf-abcdefghij")).toBe(true); expect(valueLooksLikeSecret("not-a-secret")).toBe(false); }); + it("preserves null and undefined under credential field names", () => { + const result = stripCredentials({ apiKey: null, token: undefined, model: "keep" }) as Record< + string, + unknown + >; + expect(result.apiKey).toBeNull(); + expect(result.token).toBeUndefined(); + expect(result.model).toBe("keep"); + }); + it("excludes auth state basenames from migration copies", () => { expect(isSensitiveFile("auth-profiles.json")).toBe(true); expect(isSensitiveFile("auth.json")).toBe(true); diff --git a/nemoclaw/src/security/credential-filter.ts b/nemoclaw/src/security/credential-filter.ts index dce343eda68..72d45a7fd90 100644 --- a/nemoclaw/src/security/credential-filter.ts +++ b/nemoclaw/src/security/credential-filter.ts @@ -58,17 +58,32 @@ const SAFE_CREDENTIAL_PLACEHOLDER_LITERALS: ReadonlySet = new Set([ CREDENTIAL_PLACEHOLDER, ]); -/** High-confidence raw secret shapes used as a value-level backstop. */ +/** + * High-confidence raw secret shapes used as a value-level backstop. + * Kept aligned with TOKEN_PREFIX / STRUCTURED / SECRET_BLOCK patterns from + * src/lib/security/secret-patterns.ts (plugin package cannot import src/lib). + */ const VALUE_SECRET_PATTERNS: readonly RegExp[] = [ /nvapi-[A-Za-z0-9_-]{10,}/, + /nvcf-[A-Za-z0-9_-]{10,}/, /ghp_[A-Za-z0-9_-]{10,}/, + /(?:github_pat_)[A-Za-z0-9_]{30,}/, /sk-proj-[A-Za-z0-9_-]{10,}/, /sk-ant-[A-Za-z0-9_-]{10,}/, /sk-[A-Za-z0-9_-]{20,}/, /(?:xox[bpas]|xapp)-[A-Za-z0-9-]{10,}/, /A(?:K|S)IA[A-Z0-9]{16}/, /hf_[A-Za-z0-9]{10,}/, + /glpat-[A-Za-z0-9_-]{10,}/, + /gsk_[A-Za-z0-9]{10,}/, + /pypi-[A-Za-z0-9_-]{10,}/, + /\bbot\d{8,10}:[A-Za-z0-9_-]{35}\b/, + /\b\d{8,10}:[A-Za-z0-9_-]{35}\b/, + /\b[A-Za-z0-9]{24}\.[A-Za-z0-9_-]{6}\.[A-Za-z0-9_-]{27,}\b/, /tvly-[A-Za-z0-9_-]{10,}/, + /lsv2_(?:pt|sk)_[A-Za-z0-9]{10,}(?:_[A-Za-z0-9]+)*/, + /\beyJ[A-Za-z0-9_-]{5,}\.[A-Za-z0-9_-]{2,}\.[A-Za-z0-9_-]{10,}\b/, + /-----BEGIN (?:[A-Z0-9]+ )?PRIVATE KEY-----[\s\S]*?-----END (?:[A-Z0-9]+ )?PRIVATE KEY-----/, ]; function hasPassCredentialSegment(key: string): boolean { @@ -164,7 +179,10 @@ export function stripCredentials(obj: unknown): unknown { const result: UnknownRecord = {}; for (const [key, value] of Object.entries(obj)) { if (isCredentialField(key)) { - result[key] = isSafeCredentialPlaceholder(value) ? value : CREDENTIAL_PLACEHOLDER; + result[key] = + value === null || value === undefined || isSafeCredentialPlaceholder(value) + ? value + : CREDENTIAL_PLACEHOLDER; } else { result[key] = scrubConfigValue(value); } diff --git a/src/lib/security/credential-filter.test.ts b/src/lib/security/credential-filter.test.ts index 5ee31b2a251..d85820e2c29 100644 --- a/src/lib/security/credential-filter.test.ts +++ b/src/lib/security/credential-filter.test.ts @@ -13,6 +13,7 @@ import { sanitizeConfigFile, sanitizeEnvFile, sanitizeEnvFileContent, + sanitizeYamlConfigFile, shouldScanSnapshotFileForCredentials, stripCredentials, } from "./credential-filter.js"; @@ -85,6 +86,13 @@ describe("stripCredentials", () => { expect(stripCredentials(42)).toBe(42); }); + it("preserves null and undefined under credential field names", () => { + const result = stripCredentials({ apiKey: null, token: undefined, model: "keep" }); + expect(result.apiKey).toBeNull(); + expect(result.token).toBeUndefined(); + expect(result.model).toBe("keep"); + }); + it("preserves OpenShell resolve placeholders under credential fields (#5027)", () => { const input = { models: { providers: { nvidia: { apiKey: "unused", baseUrl: "https://x/v1" } } }, @@ -230,7 +238,7 @@ describe("sanitizeConfigFile", () => { ].join("\n"), ); - sanitizeConfigFile(configPath); + expect(sanitizeConfigFile(configPath)).toBe(true); const result = readFileSync(configPath, "utf-8"); expect(result).toContain("model: hermes"); @@ -242,6 +250,14 @@ describe("sanitizeConfigFile", () => { expect(result).not.toContain("ghp_abcdefghijklmnopqrstuvwxyz0123456789"); expect(result).not.toContain("gateway:"); }); + + it("fails closed for malformed Hermes YAML", () => { + const configPath = join(tmpDir, "broken.yaml"); + writeFileSync(configPath, "api_key: [unclosed\n"); + expect(sanitizeYamlConfigFile(configPath)).toBe(false); + expect(sanitizeConfigFile(configPath)).toBe(false); + expect(readFileSync(configPath, "utf-8")).toContain("api_key:"); + }); }); describe("sanitizeEnvFileContent", () => { diff --git a/src/lib/security/credential-filter.ts b/src/lib/security/credential-filter.ts index e55e4d8614a..f81feaa4af6 100644 --- a/src/lib/security/credential-filter.ts +++ b/src/lib/security/credential-filter.ts @@ -295,9 +295,12 @@ export function stripCredentials(obj: ConfigValue): ConfigValue { const result: ConfigObject = {}; for (const [key, value] of Object.entries(obj)) { if (isCredentialField(key)) { - // Preserve non-secret references (OpenShell resolve placeholders, the - // `unused` sentinel); scrub anything else that looks like a raw secret. - result[key] = isSafeCredentialPlaceholder(value) ? value : CREDENTIAL_PLACEHOLDER; + // Preserve unset credential fields and non-secret references (OpenShell + // resolve placeholders, the `unused` sentinel); scrub raw secrets. + result[key] = + value === null || value === undefined || isSafeCredentialPlaceholder(value) + ? value + : CREDENTIAL_PLACEHOLDER; } else { result[key] = scrubConfigValue(value); } @@ -421,21 +424,22 @@ function toConfigValue(value: unknown): ConfigValue | undefined { * Removes the "gateway" section when present (auth tokens — regenerated * at startup), matching JSON sanitization. */ -export function sanitizeYamlConfigFile(configPath: string): void { +export function sanitizeYamlConfigFile(configPath: string): boolean { const rawConfig = readRegularFileNoFollow(configPath); - if (rawConfig === null) return; + if (rawConfig === null) return false; let parsed: unknown; try { parsed = parseYaml(rawConfig); } catch { - return; + return false; } const configValue = toConfigValue(parsed); - if (!isConfigObject(configValue)) return; + if (!isConfigObject(configValue)) return false; const { gateway: _gateway, ...config } = configValue; const sanitized = stripCredentials(config); writeFileAtomically(configPath, stringifyYaml(sanitized)); + return true; } /** @@ -443,26 +447,29 @@ export function sanitizeYamlConfigFile(configPath: string): void { * Removes the "gateway" section (contains auth tokens — regenerated at startup). * JSON is preferred when the file parses as JSON; otherwise YAML is tried * so Hermes `config.yaml` secrets are scrubbed from rebuild backups. + * Returns false when a YAML/YML target cannot be sanitized so callers can + * fail closed instead of retaining the raw file. */ -export function sanitizeConfigFile(configPath: string): void { +export function sanitizeConfigFile(configPath: string): boolean { const rawConfig = readRegularFileNoFollow(configPath); - if (rawConfig === null) return; + if (rawConfig === null) return false; try { const parsed = parseJson(rawConfig); - if (!isConfigObject(parsed)) return; + if (!isConfigObject(parsed)) return false; const { gateway: _gateway, ...config } = parsed; const sanitized = stripCredentials(config); writeFileAtomically(configPath, JSON.stringify(sanitized, null, 2)); - return; + return true; } catch { // Fall through to YAML for Hermes and other non-JSON configs. } const normalized = basename(configPath).toLowerCase(); if (normalized.endsWith(".yaml") || normalized.endsWith(".yml")) { - sanitizeYamlConfigFile(configPath); + return sanitizeYamlConfigFile(configPath); } + return false; } /** diff --git a/src/lib/state/sandbox.ts b/src/lib/state/sandbox.ts index 8584c72be1a..ab1d1059e73 100644 --- a/src/lib/state/sandbox.ts +++ b/src/lib/state/sandbox.ts @@ -637,20 +637,25 @@ function sanitizeBackupDirectory(dirPath: string): void { if (entry.isDirectory()) { walk(fullPath); } else if (entry.isFile()) { + const name = entry.name.toLowerCase(); if (isSensitiveFile(entry.name)) { try { require("node:fs").unlinkSync(fullPath); } catch { /* best effort */ } - } else if ( - entry.name.endsWith(".json") || - entry.name.endsWith(".yaml") || - entry.name.endsWith(".yml") - ) { + } else if (name.endsWith(".json") || name.endsWith(".yaml") || name.endsWith(".yml")) { // JSON (OpenClaw) and YAML (Hermes config.yaml) both carry secrets. - sanitizeConfigFile(fullPath); - } else if (entry.name === ".env" || entry.name.endsWith(".env")) { + // Fail closed for YAML: omit the artifact when sanitization cannot run. + const sanitized = sanitizeConfigFile(fullPath); + if (!sanitized && (name.endsWith(".yaml") || name.endsWith(".yml"))) { + try { + require("node:fs").unlinkSync(fullPath); + } catch { + /* best effort */ + } + } + } else if (name === ".env" || name.endsWith(".env")) { // Hermes stores API keys in .env alongside config.yaml. try { sanitizeEnvFile(fullPath);