diff --git a/packages/adapter-utils/src/command-redaction.test.ts b/packages/adapter-utils/src/command-redaction.test.ts index 73ed4fed09d..a29ce47a1fa 100644 --- a/packages/adapter-utils/src/command-redaction.test.ts +++ b/packages/adapter-utils/src/command-redaction.test.ts @@ -12,6 +12,7 @@ describe("explicit diagnostic credential forms", () => { import { REDACTED_COMMAND_TEXT_VALUE, collectDiagnosticEnvironmentSecrets, + collectHostEnvironmentTokenSecrets, looksLikeCredentialJwt, redactDiagnosticText, redactCommandText, @@ -213,6 +214,29 @@ describe("diagnostic environment secret selection", () => { expect(new Set(selected)).toEqual(new Set([jwt, pem, key, "safe"])); }); + // Host env of a desktop agent session: short ordinary values under names + // the allowlist does not know. Treating "vi" as a credential redacted it + // inside "service" in Sentry reports. + it("does not treat short or bare-number host process values under unknown names as credentials", () => { + expect(collectDiagnosticEnvironmentSecrets({ CHILD_EDITOR: "vi", RUN_PHASE: "RUN", FEATURE_STATE: "off", + SESSION_PID: "67591", XPC_FLAGS: "0x0", ASK_TOOL: "true", BATCH_SIZE: "8", LONG_NUMBER: "123456789012", + CLAUDE_CODE_SDK_HAS_HOST_AUTH_REFRESH: "1" }, [], { hostProcess: true })).toEqual([]); + // Credential names keep short values private, and long opaque values stay private. + expect(collectDiagnosticEnvironmentSecrets({ AWS_ACCESS_KEY_ID: "1", CUSTOM_BINDING: "opaque-host-secret" }, [], + { hostProcess: true }).sort()).toEqual(["1", "opaque-host-secret"]); + // An agent's configured environment keeps the fail-closed rule. + expect(collectDiagnosticEnvironmentSecrets({ CUSTOM_BINDING: "1" })).toEqual(["1"]); + }); + + it("keeps short host values private as whole tokens", () => { + const env = { CHILD_EDITOR: "vi", OPAQUE_SHORT: "k9Q", RUN_PHASE: "RUN", FEATURE_STATE: "off", LEVEL: "2", + SESSION_PID: "67591", LONG_SECRET: "opaque-host-secret", AWS_ACCESS_KEY_ID: "1" }; + // Substring-private values are not repeated; only flag values (booleans, 0, 1) are exempt. + expect(collectHostEnvironmentTokenSecrets(env)).toEqual(["67591", "k9Q", "RUN", "vi", "2"]); + expect(collectHostEnvironmentTokenSecrets({ SHORT_PIN: "482", COUNT_FLAG: "1", OFF_FLAG: "0", MODE_FLAG: "yes" })).toEqual(["482"]); + expect(collectHostEnvironmentTokenSecrets(null)).toEqual([]); + }); + it("keeps unknown opaque values private without declarations", () => { expect(collectDiagnosticEnvironmentSecrets({ CUSTOM_BINDING: "opaque-host-secret" })).toEqual(["opaque-host-secret"]); }); diff --git a/packages/adapter-utils/src/command-redaction.ts b/packages/adapter-utils/src/command-redaction.ts index 754ed834e31..f8322762e50 100644 --- a/packages/adapter-utils/src/command-redaction.ts +++ b/packages/adapter-utils/src/command-redaction.ts @@ -192,10 +192,58 @@ function isOrdinaryDiagnosticEnvSetting(key: string, value: string): boolean { (name === "PAPERCLIP_SENTRY_RUN_FAILURES" && /^(?:off|unexpected|all)$/.test(value)); } +/** In the host process environment, an undeclared value under an + * unrecognized name is redacted as a substring only if it could be a + * credential: at least 8 characters, and not a bare number, hex number or + * boolean-like word. Shorter values ("vi", "RUN") are redacted only as whole + * tokens (`collectHostEnvironmentTokenSecrets`): as substrings they corrupted + * unrelated report text ("service" became "ser***REDACTED***ce"). Declared, + * secret-named and credential-shaped values are always substring-private. An + * agent's configured environment keeps the fail-closed substring rule. */ +function plausibleUndeclaredCredential(key: string, value: string): boolean { + if (credentialNameSegments(key)) return true; + return value.length >= 8 && !/^(?:\d+|0x[0-9a-f]+|true|false|on|off|yes|no)$/i.test(value); +} + +const CREDENTIAL_SEGMENTS = new Set(["TOKEN", "TOKENS", "SECRET", "SECRETS", "PASSWORD", "PASSWD", "PASSPHRASE", + "CREDENTIAL", "CREDENTIALS", "PRIVATE", "BEARER", "AUTHORIZATION", "APIKEY"]); + +/** A name with a credential segment (AWS_SECRET_ACCESS_KEY, AWS_BEARER_TOKEN_BEDROCK), + * or a KEY segment that is last or followed only by ID (KEY, AWS_ACCESS_KEY_ID). + * Such a value stays private whatever its length. */ +function credentialNameSegments(key: string): boolean { + const segments = key.toUpperCase().split(/[^A-Z0-9]+/).filter(Boolean); + return segments.some((segment, index) => CREDENTIAL_SEGMENTS.has(segment) || + (segment === "KEY" && (index === segments.length - 1 || (index === segments.length - 2 && segments[index + 1] === "ID")))); +} + +const BOOLEAN_WORD = /^(?:true|false|on|off|yes|no)$/i; + +/** + * Host process values under unknown names that are too short or too plain to + * redact as substrings (see `plausibleUndeclaredCredential`). They are still + * private: callers redact them where they appear as a whole token, numbers + * included. Only flag values are exempt (boolean words, and `0` or `1`): they + * cannot hold a credential, and as tokens they would erase ordinary words + * ("on", "no") and exit codes from every report. + */ +export function collectHostEnvironmentTokenSecrets(env: unknown): string[] { + if (!env || typeof env !== "object") return []; + const substringValues = new Set(collectDiagnosticEnvironmentSecrets(env, [], { hostProcess: true })); + const values = new Set(); + for (const [key, value] of Object.entries(env)) { + if (typeof value !== "string" || !value.trim() || substringValues.has(value)) continue; + if (isOrdinaryDiagnosticEnvSetting(key, value) || BOOLEAN_WORD.test(value) || /^[01]$/.test(value)) continue; + values.add(value); + } + return [...values].sort((a, b) => b.length - a.length); +} + /** Select private values while preserving explicit ordinary process settings. */ export function collectDiagnosticEnvironmentSecrets( env: unknown, secretKeys: Iterable = [], + options: { hostProcess?: boolean } = {}, ): string[] { if (!env || typeof env !== "object") return []; const declared = new Set(secretKeys); @@ -206,7 +254,7 @@ export function collectDiagnosticEnvironmentSecrets( if (declared.has(key) || secretName.test(key) || redactDiagnosticText(value) !== value || looksLikeCredentialJwt(value) || /-----BEGIN (?:[A-Z]+ )*PRIVATE KEY-----/.test(value) || - !isOrdinaryDiagnosticEnvSetting(key, value)) { + (!isOrdinaryDiagnosticEnvSetting(key, value) && (!options.hostProcess || plausibleUndeclaredCredential(key, value)))) { values.add(value); } try { diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index e8540c3ce3d..df627d1ad52 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -2,7 +2,7 @@ import { captureEnvironmentAcquisitionDiagnostic } from "../environment-acquisit import { describe, expect, it } from "vitest"; import type { heartbeatRuns } from "@paperclipai/db"; import { attachRemoteRunnerRecoveryDiagnostic, RemoteRunnerRecoveryError } from "../native-runtime/remote-runner-recovery-diagnostics.js"; -import { collectRunFailureDiagnostics, collectRunFailureSecretValues, redactRunFailureSecretValues, sanitizeRunFailureDiagnostics, sanitizeRunFailureText } from "../run-failure-diagnostics.js"; +import { collectRunFailureDiagnostics, collectRunFailureHostTokenSecrets, collectRunFailureSecretValues, redactRunFailureSecretValues, sanitizeRunFailureDiagnostics, sanitizeRunFailureText } from "../run-failure-diagnostics.js"; type Run = typeof heartbeatRuns.$inferSelect; const run = (overrides: Partial = {}) => ({ resultJson: null, ...overrides }) as Run; @@ -595,6 +595,20 @@ describe("run failure diagnostics", () => { expect(result.text).not.toContain("***[REDACTED]"); }); + it("redacts short host values as whole tokens without corrupting words that contain them", () => { + const tokens = collectRunFailureHostTokenSecrets({ CHILD_EDITOR: "vi", OPAQUE_SHORT: "k9Q", SHORT_PIN: "482", LEVEL: "1" }); + const result = redactRunFailureSecretValues({ category: "service", text: "editor vi used k9Q and 482; k9Qx and 4821 stay, exit 1", + nested: ["(vi)", "vim"] }, [], tokens); + expect(result).toEqual({ category: "service", + text: "editor ***REDACTED*** used ***REDACTED*** and ***REDACTED***; k9Qx and 4821 stay, exit 1", + nested: ["(***REDACTED***)", "vim"] }); + }); + + it("lets a longer whole-token host value win over a shorter declared secret at the same position", () => { + expect(redactRunFailureSecretValues("upstream rejected 1aB9xQ; code 1", ["1"], ["1aB9xQ"])) + .toBe("upstream rejected ***REDACTED***; code ***REDACTED***"); + }); + it("preserves ordinary settings while retaining declared secrets", () => { const env = { PAPERCLIP_DB_BACKUP_ENABLED: "false", PAPERCLIP_DB_BACKUP_RETENTION_DAYS: "1" }; expect(collectRunFailureSecretValues(env)).toEqual([]); diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index 64ae0cd1977..0ec8fc1170e 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -5,7 +5,11 @@ import { readControllerLeaseLoss, readControllerLeaseLossCause, readRunCancellat import { readProcessLossDiagnostic } from "./process-loss-diagnostics.js"; import { readReviewedChatBindingDenialReason } from "./reviewed-chat-binding-diagnostics.js"; import { WORKSPACE_RESTORE_FAILURE_CODES } from "@paperclipai/shared"; -import { collectDiagnosticEnvironmentSecrets, redactDiagnosticText } from "@paperclipai/adapter-utils/command-redaction"; +import { + collectDiagnosticEnvironmentSecrets, + collectHostEnvironmentTokenSecrets, + redactDiagnosticText, +} from "@paperclipai/adapter-utils/command-redaction"; import { directoryMergeLockTimeoutDiagnosticsFor } from "@paperclipai/adapter-utils/directory-merge-lock-diagnostics"; import { sanitizeWorkspaceRestoreDiagnostic } from "@paperclipai/adapter-utils/workspace-restore-diagnostics"; import { @@ -168,15 +172,31 @@ function scalars(value: unknown, fields: readonly string[]): Context { /** Select credential fields, credential-shaped values, and declared bindings. */ export const collectRunFailureSecretValues = collectDiagnosticEnvironmentSecrets; +/** Short host values that stay private as whole tokens only. */ +export const collectRunFailureHostTokenSecrets = collectHostEnvironmentTokenSecrets; + /** Redact literal, JSON-escaped and URI-encoded forms without rewriting markers. */ -export function redactRunFailureSecretValues(input: T, values: readonly string[]): T { - const forms = [...new Set(values.filter(Boolean).flatMap((value) => { +export function redactRunFailureSecretValues(input: T, values: readonly string[], + tokenValues: readonly string[] = []): T { + const variants = (list: readonly string[]) => [...new Set(list.filter(Boolean).flatMap((value) => { const forms = [value, JSON.stringify(value).slice(1, -1)]; try { forms.push(encodeURIComponent(value)); } catch { /* malformed Unicode */ } return forms; }))].sort((a, b) => b.length - a.length); - if (forms.length === 0) return input; - const pattern = new RegExp(forms.map((value) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")).join("|"), "gu"); + const escape = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const forms = variants(values); + // Short host values are private only as whole tokens: "vi" is redacted in + // "editor vi", never inside "service". One alternation ordered by length + // across both lists, so a longer complete match always wins over a shorter + // prefix that starts at the same position. + const tokenForms = variants(tokenValues).filter((value) => !forms.includes(value)); + const alternatives = [ + ...forms.map((value) => ({ value, pattern: escape(value) })), + ...tokenForms.map((value) => ({ value, + pattern: `(? b.value.length - a.value.length); + if (alternatives.length === 0) return input; + const pattern = new RegExp(alternatives.map((entry) => entry.pattern).join("|"), "gu"); const redact = (value: unknown): unknown => { if (typeof value === "string") return value.replace(pattern, () => REDACTED_EVENT_VALUE); if (Array.isArray(value)) return value.map(redact); diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index b25e97ead98..b993d08a740 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -4,6 +4,7 @@ import { agents, dotRunnerAssignments, grokBotDispatches, heartbeatRuns, nativeR import { captureRunFailure, type RunFailureStatus } from "../sentry.js"; import { collectRunFailureDiagnostics, + collectRunFailureHostTokenSecrets, collectRunFailureSecretValues, redactRunFailureSecretValues, sanitizeRunFailureDiagnostics, @@ -376,9 +377,11 @@ async function captureTerminalRunFailure( errorCode: run.errorCode ?? null, diagnostics: collectRunFailureDiagnostics(run, options), }, [...new Set([ - ...collectRunFailureSecretValues(process.env), + // The server's own environment: short host settings under unknown + // names are private only as whole tokens (see collectHostEnvironmentTokenSecrets). + ...collectRunFailureSecretValues(process.env, [], { hostProcess: true }), ...(options.secretValues ?? []), - ])].sort((a, b) => b.length - a.length)); + ])].sort((a, b) => b.length - a.length), collectRunFailureHostTokenSecrets(process.env)); const agent = await db .select({ adapterType: agents.adapterType }) .from(agents)