diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 2b8e1a90ae..e4c73f63f1 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -406,3 +406,12 @@ export { type MaintainerNoiseReport, type PullRequestReviewability, } from "./reward-risk.js"; + +// Shared subprocess env-allowlist + secret-redaction helpers (#4284) — one source of truth for every driver that +// spawns a locally-authenticated CLI subprocess (src/selfhost/ai.ts and the coming gittensory-miner drivers). +export { + SUBPROCESS_CLI_ENV_ALLOWLIST, + buildAllowlistedEnv, + SECRET_PATTERNS, + redactSecrets, +} from "./subprocess-env.js"; diff --git a/packages/gittensory-engine/src/subprocess-env.ts b/packages/gittensory-engine/src/subprocess-env.ts new file mode 100644 index 0000000000..455fb7d7ca --- /dev/null +++ b/packages/gittensory-engine/src/subprocess-env.ts @@ -0,0 +1,78 @@ +// Shared subprocess env-allowlist + secret-redaction helpers (#4284). Any driver that spawns a locally-authenticated +// CLI (the review `claude`/`codex` subprocess in src/selfhost/ai.ts, and the coding-agent drivers coming in +// gittensory-miner) needs the SAME two safety primitives: hand the child a STRICT allowlisted env (never the full +// worker/host env, which can carry runtime credentials into a prompt-injectable subprocess), and redact well-known +// secret shapes out of the child's untrusted stderr before it reaches logs. This module is the single engine-hosted +// source of truth for both, so those callers depend on one implementation instead of copy-pasting the pattern. + +/** + * The standard env-var allowlist for a locally-authenticated CLI subprocess: home + proxy + TLS-cert + locale + + * XDG config paths, so the CLI keeps its own auth/proxy/cert settings, but nothing else (no runtime secrets) leaks + * in. A caller that needs a different/larger set (e.g. a coding-agent driver) passes its own list to + * {@link buildAllowlistedEnv} rather than editing this one. + */ +export const SUBPROCESS_CLI_ENV_ALLOWLIST = [ + "HOME", + "HTTPS_PROXY", + "HTTP_PROXY", + "LANG", + "LC_ALL", + "NODE_EXTRA_CA_CERTS", + "NO_PROXY", + "PATH", + "SSL_CERT_DIR", + "SSL_CERT_FILE", + "TERM", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + "XDG_STATE_HOME", + "https_proxy", + "http_proxy", + "no_proxy", +] as const; + +/** + * Build a child-process env by copying ONLY `allowlist` keys from `parent`, then overlaying `extra`. Parameterized + * (the allowlist is a caller argument, not hardcoded) so different subprocess kinds can use different allowlists. + * `undefined` values are dropped from both sources; `extra` wins over an allowlisted parent value for the same key. + * Pure — never reads the ambient process env itself. + */ +export function buildAllowlistedEnv( + parent: Record, + allowlist: readonly string[], + extra: Record = {}, +): Record { + const child: Record = {}; + for (const key of allowlist) { + const value = parent[key]; + if (value !== undefined) child[key] = value; + } + for (const [key, value] of Object.entries(extra)) { + if (value !== undefined) child[key] = value; + } + return child; +} + +/** Well-known secret token shapes to strip from untrusted subprocess output. Ported verbatim from + * src/selfhost/ai.ts (`SECRET_PATTERNS`) — keep the two in sync (or shim ai.ts onto this) rather than weakening. */ +export const SECRET_PATTERNS: readonly RegExp[] = [ + /\bsk-[A-Za-z0-9_-]{16,}/g, // OpenAI / Anthropic keys (sk-..., sk-ant-..., sk-proj-...) + /\bgh[oprsu]_[A-Za-z0-9]{20,}/g, // GitHub PAT / OAuth / server / refresh tokens + /\bgithub_pat_[A-Za-z0-9_]{20,}/g, // GitHub fine-grained PAT + /\beyJ[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, // JWT (header.payload.signature) + /\bAKIA[0-9A-Z]{16}/g, // AWS access key id +]; + +/** + * Redact secrets from untrusted subprocess output before it flows to logs/Sentry: strip each caller-supplied known + * secret value exactly (length-guarded so a short/empty token can't blank unrelated text), then well-known token + * shapes ({@link SECRET_PATTERNS}). Ported from src/selfhost/ai.ts's `redactSecrets`. Pure. + */ +export function redactSecrets(text: string, knownSecrets: readonly string[] = []): string { + let out = text; + for (const secret of knownSecrets) { + if (secret.length >= 8) out = out.split(secret).join("[redacted]"); + } + for (const pattern of SECRET_PATTERNS) out = out.replace(pattern, "[redacted]"); + return out; +} diff --git a/packages/gittensory-engine/test/subprocess-env.test.ts b/packages/gittensory-engine/test/subprocess-env.test.ts new file mode 100644 index 0000000000..50f51de0c2 --- /dev/null +++ b/packages/gittensory-engine/test/subprocess-env.test.ts @@ -0,0 +1,33 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { SUBPROCESS_CLI_ENV_ALLOWLIST, buildAllowlistedEnv, SECRET_PATTERNS, redactSecrets } from "../dist/index.js"; + +test("buildAllowlistedEnv: copies only allowlisted keys; a caller-supplied allowlist is honored; extra overlays", () => { + const parent = { HOME: "/home/node", SECRET_TOKEN: "sk-should-not-copy", PATH: "/usr/bin", CUSTOM: "keep" }; + // the standard allowlist copies HOME + PATH, drops SECRET_TOKEN + CUSTOM + assert.deepEqual(buildAllowlistedEnv(parent, SUBPROCESS_CLI_ENV_ALLOWLIST), { HOME: "/home/node", PATH: "/usr/bin" }); + // a DIFFERENT caller-supplied allowlist is honored (CUSTOM now allowed), and `extra` overlays a parent value + assert.deepEqual(buildAllowlistedEnv(parent, ["HOME", "CUSTOM"], { EXTRA: "v", HOME: "/override" }), { + HOME: "/override", + CUSTOM: "keep", + EXTRA: "v", + }); + // undefined values are dropped from both the parent and `extra` + assert.deepEqual(buildAllowlistedEnv({ A: undefined }, ["A"], { B: undefined }), {}); +}); + +test("redactSecrets: strips every SECRET_PATTERNS family, plus caller-supplied known secrets", () => { + assert.equal(redactSecrets("key sk-abcdefghijklmnop123"), "key [redacted]"); // OpenAI/Anthropic + assert.equal(redactSecrets("tok ghp_ABCDEFGHIJKLMNOPQRSTUV"), "tok [redacted]"); // GitHub token + assert.equal(redactSecrets("pat github_pat_ABCDEFGHIJKLMNOPQRST"), "pat [redacted]"); // GitHub fine-grained PAT + assert.equal(redactSecrets("jwt eyJhbGciOi.eyJzdWIiO.SflKxwRJSM"), "jwt [redacted]"); // JWT + assert.equal(redactSecrets("aws AKIAIOSFODNN7EXAMPLE"), "aws [redacted]"); // AWS access key id + // a known secret (length >= 8) is stripped exactly; a short one is NOT (guards unrelated diagnostic text) + assert.equal(redactSecrets("value=supersecretvalue", ["supersecretvalue"]), "value=[redacted]"); + assert.equal(redactSecrets("t and t again", ["t"]), "t and t again"); +}); + +test("SECRET_PATTERNS carries the full ported regex family", () => { + assert.equal(SECRET_PATTERNS.length, 5); +}); diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index 8e3b6aea76..699ffc8a90 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -353,6 +353,13 @@ export function createAnthropicAi(opts: { apiKey: string; model?: string | undef // SECURITY: subscription CLIs get a strict allowlisted env, not the worker env. This keeps runtime // credentials out of prompt-injectable subprocesses while preserving CLI auth/home/proxy/cert settings. The CLI // runs read-only / no extra tools, and non-zero exit / empty output / error-envelope THROWS so the caller degrades. +// +// NOTE (#4284): the reusable half of this pattern — a parameterized allowlist builder + secret redaction — now also +// lives in `@jsonbored/gittensory-engine` (`SUBPROCESS_CLI_ENV_ALLOWLIST`, `buildAllowlistedEnv`, `SECRET_PATTERNS`, +// `redactSecrets`) so the coming gittensory-miner coding-agent drivers can depend on one source of truth. This copy +// is deliberately kept parallel for now (the review path's `subscriptionCliEnv` also folds in CLI-specific PATH +// resolution); keep the two in sync, or shim this onto the engine copy (like `src/rules/predicted-gate.ts` does) in +// a follow-up if it drifts. const SUBSCRIPTION_CLI_ENV_ALLOWLIST = [ "HOME", "HTTPS_PROXY", diff --git a/test/unit/engine-subprocess-env.test.ts b/test/unit/engine-subprocess-env.test.ts new file mode 100644 index 0000000000..2c19f4a453 --- /dev/null +++ b/test/unit/engine-subprocess-env.test.ts @@ -0,0 +1,37 @@ +// App-vitest coverage for the engine subprocess-env helper (#4284). The engine also has its own node:test suite, +// but codecov/patch is computed from this app vitest run (vitest.config coverage includes +// packages/gittensory-engine/src/**), so the changed engine lines need a vitest test that imports the SRC directly. +import { describe, expect, it } from "vitest"; +import { + SUBPROCESS_CLI_ENV_ALLOWLIST, + buildAllowlistedEnv, + SECRET_PATTERNS, + redactSecrets, +} from "../../packages/gittensory-engine/src/subprocess-env"; + +describe("engine subprocess-env helper (#4284)", () => { + it("buildAllowlistedEnv copies only allowlisted keys; a caller allowlist is honored; extra overlays; undefined dropped", () => { + const parent = { HOME: "/home/node", SECRET_TOKEN: "sk-should-not-copy", PATH: "/usr/bin", CUSTOM: "keep" }; + expect(buildAllowlistedEnv(parent, SUBPROCESS_CLI_ENV_ALLOWLIST)).toEqual({ HOME: "/home/node", PATH: "/usr/bin" }); + expect(buildAllowlistedEnv(parent, ["HOME", "CUSTOM"], { EXTRA: "v", HOME: "/override" })).toEqual({ + HOME: "/override", + CUSTOM: "keep", + EXTRA: "v", + }); + expect(buildAllowlistedEnv({ A: undefined }, ["A"], { B: undefined })).toEqual({}); + }); + + it("redactSecrets strips every SECRET_PATTERNS family + caller-supplied known secrets (length-guarded)", () => { + expect(redactSecrets("key sk-abcdefghijklmnop123")).toBe("key [redacted]"); + expect(redactSecrets("tok ghp_ABCDEFGHIJKLMNOPQRSTUV")).toBe("tok [redacted]"); + expect(redactSecrets("pat github_pat_ABCDEFGHIJKLMNOPQRST")).toBe("pat [redacted]"); + expect(redactSecrets("jwt eyJhbGciOi.eyJzdWIiO.SflKxwRJSM")).toBe("jwt [redacted]"); + expect(redactSecrets("aws AKIAIOSFODNN7EXAMPLE")).toBe("aws [redacted]"); + expect(redactSecrets("value=supersecretvalue", ["supersecretvalue"])).toBe("value=[redacted]"); + expect(redactSecrets("t and t again", ["t"])).toBe("t and t again"); // short known secret NOT stripped + }); + + it("SECRET_PATTERNS carries the full ported regex family", () => { + expect(SECRET_PATTERNS).toHaveLength(5); + }); +});