From 6ec4180ea1b98208d81f8be204788eb885b20ce5 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:01:55 -0600 Subject: [PATCH] fix: preserve ordinary diagnostic text during environment secret redaction (#19461) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Run diagnostics help operators find the cause of a failed agent run. > - The failure reporter treated unknown environment settings as credentials. > - It replaced every use of those values in diagnostic text. > - A flag set to `1`, `2`, or `safe` removed parts of dates, model names, versions, and normal words. > - This pull request preserves explicit ordinary settings and keeps unknown values private. > - Operators can read the failure details while real credentials stay masked. ## Linked Issues or Issue Description **What happened?** Run-failure contexts contained redaction markers inside timestamps, model names, driver versions, and the word "safely". Provider failure diagnostics used the same broad environment-value rule. **Expected behavior** Ordinary flags, counts, paths, regions, and mode labels leave diagnostic text readable. API keys, URL passwords, and declared secret bindings remain masked. **Steps to reproduce** 1. Set ordinary environment settings to `1`, `2`, and `safe`. 2. Report a failed native run with a timestamp, `gpt-6.1-sol`, `phase6-v2`, and "safely" in its details. 3. Inspect the run failure report. Before this fix, unrelated fragments are replaced. Related work: #13668 addresses a separate model-ID false positive. #11311 and #15524 address credential capture at other boundaries. This change keeps the existing command scanner and registered-secret replacement. ## What Changed - Add a shared environment selector in adapter-utils. Keep unknown values private. Exempt explicit ordinary process settings, typed flags and counts, mode labels, and key selectors. - Use the same selector for run-failure reports and inherited provider failure environments. Keep one policy for both consumers. - Keep configured provider bindings private because that API does not carry separate secret-reference metadata. - Preserve short declared secrets, unknown opaque values, encoded credentials, and inherited Bedrock tokens. Add regressions for ordinary `SORT_KEY` values and both Bedrock consumers. - Add screenshot-shaped regressions, real database reporter coverage, and credential tests. - Document the policy in `doc/observability.md`. The optional monitoring gate stays in place. ## Verification Linux, Node v25.9.0, pnpm 9.15.4. The published tree matches the tested local tree `cb236f072cca195e4c2cfb4034c7cc06ea7adbc5`. - Before the fix, four diagnostic regressions failed. The output reproduced the damaged timestamp, model, version, and word. - Review regressions: six tests failed on the first revision. They proved the missed Bedrock token, opaque inherited-value exposure, and key-selector false positive. All pass after the correction. - 555 focused tests pass on the final source tree. - `cd server && pnpm exec vitest run src/services/__tests__/run-failure-diagnostics.test.ts src/__tests__/redaction.test.ts src/__tests__/redact-sensitive.test.ts src/__tests__/run-secret-redaction.test.ts`: 206 passed. - `cd server && pnpm exec vitest run src/services/__tests__/run-failure-report.test.ts`: 289 passed, with real embedded PostgreSQL. - `pnpm exec vitest run packages/adapter-utils/src/command-redaction.test.ts packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts`: 60 passed. - `pnpm --filter @paperclipai/adapter-utils typecheck`: passed. - `git diff --check`: passed. - The first reporter attempt could not load the unbuilt plugin SDK. It passed after `pnpm --filter @paperclipai/plugin-sdk ensure-build-deps`. - The filtered install omitted the CLI tsx loader, so `pnpm verify --explain` could not start. The equivalent verifier entry points ran with the installed server loader and returned exit 2: broader CI coverage is required for the observability document. - Full local build/typecheck and browser suites were not duplicated. [Final-revision CI](https://github.com/paperclipai/paperclip/actions/runs/42231412547) passed on `013960a4c6fa6279e1f28b0f6af5f036d7cad5a7`, attempt 1. This includes typecheck, build, general/server/chat tests, serialized suites, Runner checks, eight E2E shards, and the canary dry run. - [Greptile review](https://github.com/paperclipai/paperclip/pull/19461#issuecomment-6686592783) scored this head 5/5 with zero new findings. All four inline threads are resolved. The security rescan and Snyk status passed. The PR has no merge conflicts. ## Risks - Ordinary-setting exemptions need a known name or a name/value type rule. Unknown opaque values remain private. Declared secret bindings and recognizable credentials override these exemptions. - A real short secret still masks matching fragments. Removing that protection would expose the secret. - Existing stored redaction markers cannot restore their original text. This fix applies when new diagnostics are produced. - No migration, dependency, workflow, authorization, or default monitoring changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. This change is a focused bug fix. ## Model Used OpenAI GPT-6, through Codex, with shell execution and file tools. The host does not expose the exact runtime variant or context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/observability.md | 11 ++- .../terminal-session-failure.test.ts | 15 +++- .../acpx-engine/terminal-session-failure.ts | 25 +------ .../src/command-redaction.test.ts | 44 +++++++++++ .../adapter-utils/src/command-redaction.ts | 73 +++++++++++++++++++ .../__tests__/run-failure-diagnostics.test.ts | 34 +++++++-- .../__tests__/run-failure-report.test.ts | 44 +++++++++-- .../src/services/run-failure-diagnostics.ts | 62 +--------------- server/src/services/run-failure-report.ts | 2 +- 9 files changed, 209 insertions(+), 101 deletions(-) diff --git a/doc/observability.md b/doc/observability.md index 0014bacddb1..b4bbd0d60c8 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -1548,9 +1548,14 @@ without one, the report has no synthetic reporting stack. Credential patterns, the current user's home path, registered run-secret values, and known host/runtime environment credentials are removed before truncation. -Declared environment secret bindings are included even when their key has no -credential-like name. Runtime values are used only for redaction and are never -copied into an event. Stack fields are limited to 8,192 characters, +Environment values stay private unless their names and values identify an ordinary +process setting. Flags and counts require both a setting-name match and a boolean +or numeric value. Mode labels use a closed value list. Paths, regions, and key +selectors such as `SORT_KEY` use explicit ordinary-setting names. Unknown opaque +values remain private, including credentials under arbitrary names. Declared +secret bindings and recognizable credential contents override ordinary-setting +exemptions, including short values. Runtime values are used only for redaction +and are never copied into an event. Stack fields are limited to 8,192 characters, exception messages to 2,048, provider titles to 4,096, and provider details to 12,288. Other diagnostic strings are limited to 200 characters (adapter cause messages: 2,048). Truncation is marked in the text and in diff --git a/packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts b/packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts index e1fdad45b7d..15c9885a876 100644 --- a/packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts +++ b/packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts @@ -87,7 +87,7 @@ describe("terminal session failure diagnostics", () => { ); }); - it("redacts unknown inherited launch values while preserving public process context", () => { + it("keeps opaque inherited values private while preserving public process settings", () => { const diagnostic = sanitizeTerminalSessionFailure({ category: "service", details: "request_id=req_123 inherited-canary A /workspace", @@ -95,6 +95,19 @@ describe("terminal session failure diagnostics", () => { expect(diagnostic.details).toBe("request_id=req_123 ***REDACTED*** ***REDACTED*** /workspace"); }); + it("masks an inherited Bedrock token without a credential label", () => { + const token = "opaque-bedrock-credential"; + const diagnostic = sanitizeTerminalSessionFailure({ category: "access", details: `upstream rejected ${token}` }, { AWS_BEARER_TOKEN_BEDROCK: token }); + expect(diagnostic.details).toBe("upstream rejected ***REDACTED***"); + }); + + it("preserves ordinary key selectors and short launch flags", () => { + const details = "column created_at does not exist; model gpt-6.1-sol; safely retry"; + expect(sanitizeTerminalSessionFailure({ category: "service", details }, { + SORT_KEY: "created_at", FEATURE_FLAG: "1", WORKER_COUNT: "2", MODE: "safe", + }).details).toBe(details); + }); + it("preserves known boolean settings, HTTP codes and request IDs", () => { const details = "HTTP 401 request_id=req_123 req-1 /1/path retries=1"; const diagnostic = sanitizeTerminalSessionFailure({ category: "access", details }, { diff --git a/packages/adapter-utils/src/acpx-engine/terminal-session-failure.ts b/packages/adapter-utils/src/acpx-engine/terminal-session-failure.ts index c83e23e31ca..8d7c2758adb 100644 --- a/packages/adapter-utils/src/acpx-engine/terminal-session-failure.ts +++ b/packages/adapter-utils/src/acpx-engine/terminal-session-failure.ts @@ -1,4 +1,4 @@ -import { redactDiagnosticText, REDACTED_COMMAND_TEXT_VALUE } from "../command-redaction.js"; +import { collectDiagnosticEnvironmentSecrets, redactDiagnosticText, REDACTED_COMMAND_TEXT_VALUE } from "../command-redaction.js"; import type { AdapterExecutionResult } from "../types.js"; export interface AcpxTerminalSessionFailure { @@ -32,16 +32,6 @@ export function classifyToolDefinitionFailure( // Leave room under the server's 64 KiB run-log chunk limit even when every // retained character needs JSON escaping. The transcript stores the text once. const FIELD_LIMITS = { title: 4096, details: 24576 } as const; -// Only conventional public process settings may survive by default. Provider, -// proxy, bridge, and future launch contributions may carry secrets under any -// name. Explicitly configured values are still redacted even for these keys. -const PUBLIC_ENV_KEYS = new Set([ - "PATH", "PATHEXT", "SYSTEMROOT", "WINDIR", "COMSPEC", "HOME", "USERPROFILE", - "HOMEDRIVE", "HOMEPATH", "USER", "USERNAME", "LOGNAME", "SHELL", "LANG", - "LANGUAGE", "LC_ALL", "LC_CTYPE", "TZ", "TMPDIR", "TEMP", "TMP", "NODE_ENV", - "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", - "PAPERCLIP_AGENT_ID", "PAPERCLIP_COMPANY_ID", "PAPERCLIP_RUN_ID", "PAPERCLIP_TASK_ID", -]); const PUBLIC_BOOLEAN_ENV_KEYS = new Set([ "OPENCODE_ALLOW_ALL_MODELS", "CLAUDE_CODE_USE_BEDROCK", "GOOGLE_GENAI_USE_GCA", "CI", "NO_COLOR", "FORCE_COLOR", @@ -58,23 +48,12 @@ export function sanitizeTerminalSessionFailure( authToken?: string, configuredEnv: Record = {}, ): AcpxTerminalSessionFailureDiagnostic { - const secrets = Object.entries(env) - .filter(([key, value]) => value && !PUBLIC_ENV_KEYS.has(key.toUpperCase()) && !isPublicBooleanSetting(key, value)) - .map(([, value]) => value); + const secrets = collectDiagnosticEnvironmentSecrets(env); // Configured values can be resolved secret_refs under arbitrary names (for // example DATABASE_URL). Key-name heuristics cannot establish they are public. for (const [key, value] of Object.entries(configuredEnv)) { if (typeof value === "string" && value && !isPublicBooleanSetting(key, value)) secrets.push(value); } - // A provider may echo just the password from a configured connection URL. - for (const value of Object.values(env)) { - try { - const url = new URL(value); - if (url.password) { - secrets.push(value, url.password, decodeURIComponent(url.password)); - } - } catch { /* ordinary environment values are not URLs */ } - } if (authToken) secrets.push(authToken); const secretForms = [...new Set(secrets.flatMap((value) => { const forms = [value, JSON.stringify(value).slice(1, -1)]; diff --git a/packages/adapter-utils/src/command-redaction.test.ts b/packages/adapter-utils/src/command-redaction.test.ts index 2afd383a6d9..73ed4fed09d 100644 --- a/packages/adapter-utils/src/command-redaction.test.ts +++ b/packages/adapter-utils/src/command-redaction.test.ts @@ -11,6 +11,7 @@ describe("explicit diagnostic credential forms", () => { }); import { REDACTED_COMMAND_TEXT_VALUE, + collectDiagnosticEnvironmentSecrets, looksLikeCredentialJwt, redactDiagnosticText, redactCommandText, @@ -184,3 +185,46 @@ describe("redactCommandText on long runs", () => { } }); }); + + +describe("diagnostic environment secret selection", () => { + it("does not infer secrets from ordinary configuration values", () => { + expect(collectDiagnosticEnvironmentSecrets({ + FEATURE_FLAG: "1", WORKER_COUNT: "2", MODE: "safe", REGION: "us-east-1", + NODE_ENV: "production", HOME: "/home/operator", PORT: "3100", MAX_TOKENS: "4096", + PUBLIC_KEY_PATH: "/workspace/key.pem", KEYBOARD_LAYOUT: "us", + SORT_KEY: "created_at", CACHE_KEY: "cache-v1", ROUTING_KEY: "jobs", + PARTITION_KEY: "tenant_id", IDEMPOTENCY_KEY: "operation-123", PUBLIC_KEY: "public-material", + })).toEqual([]); + }); + + it.each(["API_KEY", "KEY", "ACCESS_TOKEN", "AWS_SECRET_ACCESS_KEY", "AWS_ACCESS_KEY_ID", + "CLIENT_SECRET", "PASSWORD", "AUTHORIZATION", "PRIVATE_KEY", "API_KEY_PRODUCTION", "AWS_BEARER_TOKEN_BEDROCK"])( + "keeps short credentials private under %s", key => { + expect(collectDiagnosticEnvironmentSecrets({ [key]: "1" })).toEqual(["1"]); + }, + ); + + it("recognizes credential content and declared bindings under arbitrary names", () => { + const jwt = `${Buffer.from('{"alg":"HS256"}').toString("base64url")}.e30.c2ln`; + const pem = "-----BEGIN PRIVATE KEY-----\nprivate-canary\n-----END PRIVATE KEY-----"; + const key = "sk-fixturecredential12345"; + const selected = collectDiagnosticEnvironmentSecrets({ A: jwt, B: pem, C: key, D: "safe", FEATURE_FLAG: "false" }, ["D"]); + expect(new Set(selected)).toEqual(new Set([jwt, pem, key, "safe"])); + }); + + it("keeps unknown opaque values private without declarations", () => { + expect(collectDiagnosticEnvironmentSecrets({ CUSTOM_BINDING: "opaque-host-secret" })).toEqual(["opaque-host-secret"]); + }); + + it("does not exempt credentials on ordinary-setting names", () => { + expect(collectDiagnosticEnvironmentSecrets({ SORT_KEY: "opaque-secret", FEATURE_FLAG: "1" }, ["SORT_KEY", "FEATURE_FLAG"])) + .toEqual(["opaque-secret", "1"]); + }); + + it("extracts encoded URL passwords and deduplicates selected values", () => { + const url = "postgres://user:private%20password@db.test/database"; + expect(new Set(collectDiagnosticEnvironmentSecrets({ DATABASE_URL: url, PASSWORD: "private password" }))) + .toEqual(new Set([url, "private%20password", "private password"])); + }); +}); diff --git a/packages/adapter-utils/src/command-redaction.ts b/packages/adapter-utils/src/command-redaction.ts index 451f0ccf58e..754ed834e31 100644 --- a/packages/adapter-utils/src/command-redaction.ts +++ b/packages/adapter-utils/src/command-redaction.ts @@ -147,3 +147,76 @@ export function redactDiagnosticText( .replace(JSON_ESCAPED_SECRET_FIELD_RE, `$1${redactedValue}$2`) .replace(JSON_SECRET_FIELD_RE, `$1${redactedValue}$2`); } + +const PUBLIC_ENV_KEYS = new Set([ + "PATH", "PATHEXT", "SYSTEMROOT", "WINDIR", "COMSPEC", "HOME", "USERPROFILE", + "HOMEDRIVE", "HOMEPATH", "USER", "USERNAME", "LOGNAME", "SHELL", "LANG", + "LANGUAGE", "LC_ALL", "LC_CTYPE", "TZ", "TMPDIR", "TEMP", "TMP", "NODE_ENV", + "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", + "PWD", "OLDPWD", "INIT_CWD", "SHLVL", "_", "TERM", "TERM_PROGRAM", "TERM_PROGRAM_VERSION", "COLORTERM", + "NPM_LIFECYCLE_EVENT", "NPM_COMMAND", "NPM_EXEC_PATH", "NPM_EXECPATH", "NPM_NODE_EXECPATH", + "NPM_PACKAGE_NAME", "NPM_PACKAGE_VERSION", "NPM_PACKAGE_JSON", "NPM_CONFIG_USER_AGENT", + "XPC_SERVICE_NAME", "VITEST_POOL_ID", "VITEST_WORKER_ID", + "REGION", "AWS_REGION", "AWS_DEFAULT_REGION", "KEYBOARD_LAYOUT", "PUBLIC_KEY_PATH", "PUBLIC_KEY", + "SORT_KEY", "CACHE_KEY", "ROUTING_KEY", "PARTITION_KEY", "IDEMPOTENCY_KEY", + "PAPERCLIP_AGENT_ID", "PAPERCLIP_COMPANY_ID", "PAPERCLIP_RUN_ID", "PAPERCLIP_TASK_ID", +]); +const PUBLIC_BOOLEAN_ENV_KEYS = new Set([ + "OPENCODE_ALLOW_ALL_MODELS", "CLAUDE_CODE_USE_BEDROCK", "GOOGLE_GENAI_USE_GCA", + "CI", "NO_COLOR", "FORCE_COLOR", + "VITEST", "MALLOCNANOZONE", "CODEX_CI", "CODEX_SHELL", "CODEX_SAGE_BACKFILL_TRACKER_TAB_REUSE", + "DEV", "PROD", "SSR", "PAPERCLIP_REQUIRE_SENTRY_TEST_SDK", + "PAPERCLIP_DB_BACKUP_ENABLED", "PAPERCLIP_WORKSPACE_COMPACTION_ENABLED", "PAPERCLIP_AUTH_DISABLE_SIGN_UP", "PAPERCLIP_ENABLE_COMPANY_DELETION", + "PAPERCLIP_STORAGE_S3_FORCE_PATH_STYLE", "PAPERCLIP_SECRETS_STRICT_MODE", + "SERVE_UI", "PAPERCLIP_UI_DEV_MIDDLEWARE", "HEARTBEAT_SCHEDULER_ENABLED", "PAPERCLIP_ANNOUNCEMENTS_ENABLED", + "GITHUB_ACTIONS", "GITHUB_REF_PROTECTED", "RUNNER_DEBUG", "ACTIONS_RUNNER_DEBUG", "ACTIONS_STEP_DEBUG", +]); +const PUBLIC_NUMBER_ENV_KEYS = new Set([ + "PORT", "PAPERCLIP_DB_BACKUP_INTERVAL_MINUTES", "PAPERCLIP_DB_BACKUP_RETENTION_DAYS", + "PAPERCLIP_WORKSPACE_REAPER_COOLDOWN_DAYS", "PAPERCLIP_WORKSPACE_COMPACTION_DELAY_HOURS", + "HEARTBEAT_SCHEDULER_INTERVAL_MS", + "GITHUB_RUN_ATTEMPT", "GITHUB_RUN_NUMBER", "GITHUB_RUN_ID", "GITHUB_RETENTION_DAYS", +]); + +// A setting name and a bounded value must agree before it is public. Unknown +// opaque values remain private; declared bindings always override these rules. +function isOrdinaryDiagnosticEnvSetting(key: string, value: string): boolean { + const name = key.toUpperCase(); + if (PUBLIC_ENV_KEYS.has(name)) return true; + if ((PUBLIC_BOOLEAN_ENV_KEYS.has(name) || /(?:^|_)(?:FLAG|ENABLED|DISABLED|DEBUG)$/.test(name)) && + /^(?:0|1|true|false)$/i.test(value)) return true; + if ((PUBLIC_NUMBER_ENV_KEYS.has(name) || /(?:^|_)(?:COUNT|WORKERS|PORT|MAX_TOKENS)$/.test(name)) && + /^\d+$/.test(value)) return true; + return (name === "BASE_URL" && value === "/") || + (/(?:^|_)MODE$/.test(name) && /^(?:test|development|production|safe)$/.test(value)) || + (name === "PAPERCLIP_SENTRY_RUN_FAILURES" && /^(?:off|unexpected|all)$/.test(value)); +} + +/** Select private values while preserving explicit ordinary process settings. */ +export function collectDiagnosticEnvironmentSecrets( + env: unknown, + secretKeys: Iterable = [], +): string[] { + if (!env || typeof env !== "object") return []; + const declared = new Set(secretKeys); + const secretName = new RegExp(`^(?:${SECRET_NAME_PATTERN})$`, "i"); + const values = new Set(); + for (const [key, value] of Object.entries(env)) { + if (typeof value !== "string" || !value) continue; + if (declared.has(key) || secretName.test(key) || + redactDiagnosticText(value) !== value || looksLikeCredentialJwt(value) || + /-----BEGIN (?:[A-Z]+ )*PRIVATE KEY-----/.test(value) || + !isOrdinaryDiagnosticEnvSetting(key, value)) { + values.add(value); + } + try { + const url = new URL(value); + if (url.password) { + values.add(value); + values.add(url.password); + values.add(decodeURIComponent(url.password)); + } + } catch { /* ordinary settings and malformed URLs need no URL projection */ } + } + return [...values].sort((a, b) => b.length - a.length); +} diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index a11b3f6dfd2..4a7d20b9e99 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -551,9 +551,27 @@ describe("run failure diagnostics", () => { it("selects declared environment secrets under opaque names and common credential keys", () => { expect(collectRunFailureSecretValues({ CUSTOM_BINDING: "bound-opaque-value", ACCESS_TOKEN: "plain-opaque-value", REGION: "us-east-1", EMPTY_KEY: "", - }, ["CUSTOM_BINDING"])).toEqual(["bound-opaque-value", "plain-opaque-value", "us-east-1"]); - expect(collectRunFailureSecretValues({ PATH: "/usr/bin", CUSTOM_BINDING: "opaque" }, [], true)).toEqual(["opaque"]); - expect(collectRunFailureSecretValues({ PATH: "secret-override" }, ["PATH"], true)).toEqual(["secret-override"]); + }, ["CUSTOM_BINDING"])).toEqual(["bound-opaque-value", "plain-opaque-value"]); + expect(collectRunFailureSecretValues({ PATH: "/usr/bin", CUSTOM_BINDING: "opaque" })).toEqual(["opaque"]); + expect(collectRunFailureSecretValues({ PATH: "secret-override" }, ["PATH"])).toEqual(["secret-override"]); + }); + + it("keeps screenshot diagnostics intact when ordinary flags overlap their text", () => { + const diagnostic = collectRunFailureDiagnostics(run({ + runtimeMode: "native", driverKind: "codex_app_server", driverVersion: "phase6-v2", + startedAt: new Date("2026-10-19T19:18:20.617Z"), finishedAt: new Date("2026-10-19T19:21:20.385Z"), + runnerProfileJson: { nativeExecutionInput: { provider: { kind: "codex", model: "gpt-6.1-sol" } } }, + }), {}); + const input = { diagnostics: diagnostic, errorMessage: "Reconcile any uncertain external action before safely continuing this task." }; + const values = collectRunFailureSecretValues({ FEATURE_FLAG: "1", WORKER_COUNT: "2", MODE: "safe", REGION: "us-east-1" }); + expect(redactRunFailureSecretValues(input, values)).toEqual(input); + }); + + it("finds credentials by content and keeps short declared secrets private", () => { + const key = "sk-fixturecredential12345"; + const values = collectRunFailureSecretValues({ CUSTOM_SETTING: key, FEATURE_FLAG: "1", CUSTOM_BINDING: "2" }, ["CUSTOM_BINDING"]); + expect(values).toEqual([key, "2"]); + expect(redactRunFailureSecretValues(`key ${key} code 2`, values)).toBe("key ***REDACTED*** code ***REDACTED***"); }); it("redacts encoded credentials and URL passwords without expanding replacement markers", () => { @@ -565,12 +583,12 @@ describe("run failure diagnostics", () => { expect(result.text).not.toContain("***[REDACTED]"); }); - it("preserves known boolean and numeric settings without exempting unknown or declared secrets", () => { + it("preserves ordinary settings while retaining declared secrets", () => { const env = { PAPERCLIP_DB_BACKUP_ENABLED: "false", PAPERCLIP_DB_BACKUP_RETENTION_DAYS: "1" }; - expect(collectRunFailureSecretValues(env, [], true)).toEqual([]); - expect(collectRunFailureSecretValues(env, ["PAPERCLIP_DB_BACKUP_ENABLED"], true)).toEqual(["false"]); - expect(collectRunFailureSecretValues({ CUSTOM_BINDING: "1" }, [], true)).toEqual(["1"]); - expect(collectRunFailureSecretValues({ PAPERCLIP_DB_BACKUP_ENABLED: "opaque" }, [], true)).toEqual(["opaque"]); + expect(collectRunFailureSecretValues(env)).toEqual([]); + expect(collectRunFailureSecretValues(env, ["PAPERCLIP_DB_BACKUP_ENABLED"])).toEqual(["false"]); + expect(collectRunFailureSecretValues({ CUSTOM_BINDING: "1" })).toEqual(["1"]); + expect(collectRunFailureSecretValues({ PAPERCLIP_DB_BACKUP_ENABLED: "opaque" })).toEqual(["opaque"]); }); it("preserves generic exceptions, numeric codes and nested network causes", () => { const root = Object.assign(new Error("network failed"), { code: "ECONNRESET", statusCode: 502, request_id: "req-123" }); diff --git a/server/src/services/__tests__/run-failure-report.test.ts b/server/src/services/__tests__/run-failure-report.test.ts index 54bdd588786..f308bec5443 100644 --- a/server/src/services/__tests__/run-failure-report.test.ts +++ b/server/src/services/__tests__/run-failure-report.test.ts @@ -55,9 +55,8 @@ describeEmbeddedPostgres("reportRunFailure", () => { }, 20_000); beforeEach(() => { - // Unknown host values intentionally count as secrets, including short values - // such as "1". Keep these fixtures independent of the developer/CI environment - // and add secret values explicitly in the tests that exercise redaction. + // Keep fixtures independent of host credentials. Add credential values + // explicitly in the tests that exercise redaction. inheritedEnv = process.env; process.env = Object.fromEntries( ["PATH", "HOME", "USER", "USERNAME", "LOGNAME", "USERPROFILE", "TMPDIR", "TEMP", "TMP"] @@ -1119,12 +1118,45 @@ describeEmbeddedPostgres("reportRunFailure", () => { expect(captured).not.toHaveProperty("secretValues"); }); - it("still redacts short values from unknown host settings", async () => { + it("masks an inherited Bedrock credential from every failure projection", async () => { await seedCompanyAndAgent(); - vi.stubEnv("FIXTURE_CUSTOM_VALUE", "1"); + const token = "opaque-bedrock-credential"; + vi.stubEnv("AWS_BEARER_TOKEN_BEDROCK", token); + const text = `upstream rejected ${token}`; + await reportRunFailure(db, buildRun({ error: text, resultJson: { terminalSessionFailure: { details: text } } }), { + error: new Error(text), adapterErrorMeta: { causeMessage: text }, + }); + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + const captured = mockCaptureRunFailure.mock.calls[0][0]; + expect(captured.errorMessage).toBe("upstream rejected ***REDACTED***"); + expect(captured.diagnostics.provider.details).toBe("upstream rejected ***REDACTED***"); + expect(JSON.stringify(captured)).not.toContain(token); + }); - await reportRunFailure(db, buildRun()); + it("keeps run metadata and prose readable with short non-secret host settings", async () => { + await seedCompanyAndAgent(); + vi.stubEnv("FIXTURE_FLAG", "1"); + vi.stubEnv("FIXTURE_WORKERS", "2"); + vi.stubEnv("FIXTURE_MODE", "safe"); + vi.stubEnv("SORT_KEY", "created_at"); + const error = "Column created_at is missing. Reconcile any uncertain external action before safely continuing this task."; + const run = buildRun({ error, runtimeMode: "native", driverKind: "codex_app_server", driverVersion: "phase6-v2", + startedAt: new Date("2026-10-19T19:18:20.617Z"), finishedAt: new Date("2026-10-19T19:21:20.385Z"), + runnerProfileJson: { nativeExecutionInput: { provider: { kind: "codex", model: "gpt-6.1-sol" } } }, + }); + await reportRunFailure(db, run); + + expect(mockCaptureRunFailure).toHaveBeenCalledOnce(); + const captured = mockCaptureRunFailure.mock.calls[0][0]; + expect(captured.errorMessage).toBe(error); + expect(captured.diagnostics.execution).toMatchObject({ driverVersion: "phase6-v2", model: "gpt-6.1-sol", + startedAt: "2026-10-19T19:18:20.617Z", finishedAt: "2026-10-19T19:21:20.385Z" }); + }); + + it("still masks a short declared runtime secret", async () => { + await seedCompanyAndAgent(); + await reportRunFailure(db, buildRun(), { secretValues: ["1"] }); expect(mockCaptureRunFailure.mock.calls[0][0].errorMessage) .toBe(`the provider process exited with code ${REDACTED_EVENT_VALUE}`); }); diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index b1acdb40b2f..5d264a6549a 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -5,7 +5,7 @@ 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 { redactDiagnosticText } from "@paperclipai/adapter-utils/command-redaction"; +import { collectDiagnosticEnvironmentSecrets, 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 { @@ -165,64 +165,8 @@ function scalars(value: unknown, fields: readonly string[]): Context { return result; } -// Unknown inherited values may contain credentials under arbitrary names. -// Explicitly configured values remain private, including overrides of these keys. -const PUBLIC_ENV_KEYS = new Set([ - "PATH", "PATHEXT", "SYSTEMROOT", "WINDIR", "COMSPEC", "HOME", "USERPROFILE", - "HOMEDRIVE", "HOMEPATH", "USER", "USERNAME", "LOGNAME", "SHELL", "LANG", - "LANGUAGE", "LC_ALL", "LC_CTYPE", "TZ", "TMPDIR", "TEMP", "TMP", "NODE_ENV", - "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", - "PWD", "OLDPWD", "INIT_CWD", "SHLVL", "_", "TERM", "TERM_PROGRAM", "TERM_PROGRAM_VERSION", "COLORTERM", - "NPM_LIFECYCLE_EVENT", "NPM_COMMAND", "NPM_EXEC_PATH", "NPM_EXECPATH", "NPM_NODE_EXECPATH", - "NPM_PACKAGE_NAME", "NPM_PACKAGE_VERSION", "NPM_PACKAGE_JSON", "NPM_CONFIG_USER_AGENT", - "XPC_SERVICE_NAME", "VITEST_POOL_ID", "VITEST_WORKER_ID", - "PAPERCLIP_AGENT_ID", "PAPERCLIP_COMPANY_ID", "PAPERCLIP_RUN_ID", "PAPERCLIP_TASK_ID", -]); -const PUBLIC_BOOLEAN_ENV_KEYS = new Set([ - "OPENCODE_ALLOW_ALL_MODELS", "CLAUDE_CODE_USE_BEDROCK", "GOOGLE_GENAI_USE_GCA", - "CI", "NO_COLOR", "FORCE_COLOR", - "VITEST", "MALLOCNANOZONE", "CODEX_CI", "CODEX_SHELL", "CODEX_SAGE_BACKFILL_TRACKER_TAB_REUSE", - "DEV", "PROD", "SSR", "PAPERCLIP_REQUIRE_SENTRY_TEST_SDK", - "PAPERCLIP_DB_BACKUP_ENABLED", "PAPERCLIP_WORKSPACE_COMPACTION_ENABLED", "PAPERCLIP_AUTH_DISABLE_SIGN_UP", "PAPERCLIP_ENABLE_COMPANY_DELETION", - "PAPERCLIP_STORAGE_S3_FORCE_PATH_STYLE", "PAPERCLIP_SECRETS_STRICT_MODE", - "SERVE_UI", "PAPERCLIP_UI_DEV_MIDDLEWARE", "HEARTBEAT_SCHEDULER_ENABLED", "PAPERCLIP_ANNOUNCEMENTS_ENABLED", - "GITHUB_ACTIONS", "GITHUB_REF_PROTECTED", "RUNNER_DEBUG", "ACTIONS_RUNNER_DEBUG", "ACTIONS_STEP_DEBUG", -]); -const PUBLIC_NUMBER_ENV_KEYS = new Set([ - "PORT", "PAPERCLIP_DB_BACKUP_INTERVAL_MINUTES", "PAPERCLIP_DB_BACKUP_RETENTION_DAYS", - "PAPERCLIP_WORKSPACE_REAPER_COOLDOWN_DAYS", "PAPERCLIP_WORKSPACE_COMPACTION_DELAY_HOURS", - "HEARTBEAT_SCHEDULER_INTERVAL_MS", - "GITHUB_RUN_ATTEMPT", "GITHUB_RUN_NUMBER", "GITHUB_RUN_ID", "GITHUB_RETENTION_DAYS", -]); - -/** Include declared bindings and unknown values, not just credential-like keys. */ -export function collectRunFailureSecretValues( - env: unknown, secretKeys: Iterable = [], inherited = false, -): string[] { - if (!env || typeof env !== "object") return []; - const strings = Object.fromEntries(Object.entries(env).filter( - (entry): entry is [string, string] => typeof entry[1] === "string", - )); - const declared = new Set(secretKeys); - const values: string[] = []; - for (const [key, value] of Object.entries(strings)) { - if (!value) continue; - const publicSetting = (inherited && ( - PUBLIC_ENV_KEYS.has(key.toUpperCase()) || - (PUBLIC_NUMBER_ENV_KEYS.has(key.toUpperCase()) && /^\d+$/.test(value)) || - (key === "BASE_URL" && value === "/") || - (key === "MODE" && /^(?:test|development|production)$/.test(value)) || - (key === "PAPERCLIP_SENTRY_RUN_FAILURES" && /^(?:off|unexpected|all)$/.test(value)) - )) || - (PUBLIC_BOOLEAN_ENV_KEYS.has(key.toUpperCase()) && /^(?:0|1|true|false)$/i.test(value)); - if (declared.has(key) || !publicSetting) values.push(value); - try { - const password = new URL(value).password; - if (password) values.push(value, password, decodeURIComponent(password)); - } catch { /* ordinary environment values are not URLs */ } - } - return [...new Set(values)].sort((a, b) => b.length - a.length); -} +/** Select credential fields, credential-shaped values, and declared bindings. */ +export const collectRunFailureSecretValues = collectDiagnosticEnvironmentSecrets; /** Redact literal, JSON-escaped and URI-encoded forms without rewriting markers. */ export function redactRunFailureSecretValues(input: T, values: readonly string[]): T { diff --git a/server/src/services/run-failure-report.ts b/server/src/services/run-failure-report.ts index c553ad3db47..dc210833b55 100644 --- a/server/src/services/run-failure-report.ts +++ b/server/src/services/run-failure-report.ts @@ -359,7 +359,7 @@ async function captureTerminalRunFailure( errorCode: run.errorCode ?? null, diagnostics: collectRunFailureDiagnostics(run, options), }, [...new Set([ - ...collectRunFailureSecretValues(process.env, [], true), + ...collectRunFailureSecretValues(process.env), ...(options.secretValues ?? []), ])].sort((a, b) => b.length - a.length)); const agent = await db