diff --git a/test/e2e-scenario/framework-tests/e2e-fixture-context.test.ts b/test/e2e-scenario/framework-tests/e2e-fixture-context.test.ts index a18108220d7..636a02c80b7 100644 --- a/test/e2e-scenario/framework-tests/e2e-fixture-context.test.ts +++ b/test/e2e-scenario/framework-tests/e2e-fixture-context.test.ts @@ -122,6 +122,88 @@ describe("E2E fixture primitives", () => { ).toThrow(/argument cannot contain NUL bytes/); }); + it("shell probe enforces options.redactionValues even when the injected redactor ignores extra values", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-e2e-shell-probe-enforce-")); + try { + const artifacts = new ArtifactSink(tmp); + await artifacts.ensureRoot(); + const secret = "redaction-enforced-via-options"; + const controller = new AbortController(); + const probe = new ShellProbe({ + artifacts, + redact: (text) => text, + signal: controller.signal, + }); + + const result = await probe.run( + trustedShellCommand({ + command: process.execPath, + args: ["-e", `console.log(${JSON.stringify(secret)}); console.error(${JSON.stringify(secret)});`], + reason: "verify ShellProbe enforces redactionValues regardless of injected redactor", + }), + { + artifactName: "options-redaction-enforced", + redactionValues: [secret], + timeoutMs: 5_000, + }, + ); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("[REDACTED]"); + expect(result.stderr).toContain("[REDACTED]"); + expect(result.stdout).not.toContain(secret); + expect(result.stderr).not.toContain(secret); + const written = fs.readFileSync(artifacts.pathFor("shell/options-redaction-enforced.result.json"), "utf8"); + expect(written).not.toContain(secret); + expect(fs.readFileSync(artifacts.pathFor("shell/options-redaction-enforced.stdout.txt"), "utf8")).not.toContain(secret); + expect(fs.readFileSync(artifacts.pathFor("shell/options-redaction-enforced.stderr.txt"), "utf8")).not.toContain(secret); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("shell probe scrubs overlapping redactionValues longest-first when the injected redactor ignores extra values", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-e2e-shell-probe-overlap-")); + try { + const artifacts = new ArtifactSink(tmp); + await artifacts.ensureRoot(); + const longer = "alpha-beta-gamma-delta"; + const shorter = "alpha"; + const controller = new AbortController(); + const probe = new ShellProbe({ + artifacts, + redact: (text) => text, + signal: controller.signal, + }); + + const result = await probe.run( + trustedShellCommand({ + command: process.execPath, + args: ["-e", `console.log(${JSON.stringify(longer)}); console.error(${JSON.stringify(longer)});`], + reason: "verify ShellProbe longest-first ordering for overlapping redactionValues", + }), + { + artifactName: "overlap-shorter-first", + redactionValues: [shorter, longer], + timeoutMs: 5_000, + }, + ); + + expect(result.exitCode).toBe(0); + expect(result.stdout).not.toContain(longer); + expect(result.stdout).not.toContain("-beta-gamma-delta"); + expect(result.stderr).not.toContain(longer); + expect(result.stderr).not.toContain("-beta-gamma-delta"); + const written = fs.readFileSync(artifacts.pathFor("shell/overlap-shorter-first.result.json"), "utf8"); + expect(written).not.toContain(longer); + expect(written).not.toContain("-beta-gamma-delta"); + expect(fs.readFileSync(artifacts.pathFor("shell/overlap-shorter-first.stdout.txt"), "utf8")).not.toContain("-beta-gamma-delta"); + expect(fs.readFileSync(artifacts.pathFor("shell/overlap-shorter-first.stderr.txt"), "utf8")).not.toContain("-beta-gamma-delta"); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + it("shell probe cleans up and redacts missing command failures", async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-e2e-shell-probe-")); try { diff --git a/test/e2e-scenario/framework-tests/e2e-redaction-entry.test.ts b/test/e2e-scenario/framework-tests/e2e-redaction-entry.test.ts new file mode 100644 index 00000000000..03d5a766b93 --- /dev/null +++ b/test/e2e-scenario/framework-tests/e2e-redaction-entry.test.ts @@ -0,0 +1,89 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Single-entry contract for the framework redactor. + * + * Both per-test explicit secret values and canonical secret-shape + * matches must flow through `redactString` so the framework has one + * redaction entry point. This file asserts the contract so any future + * helper that wants to add an explicit-value path stays inside the + * canonical entry rather than introducing a parallel one. + * + * Canonical secret-shape coverage (regex parity with the product + * source-of-truth) lives in e2e-redaction-parity.test.ts; this file + * focuses on the entry-point behaviour and SecretStore delegation. + */ + +import { describe, expect, it } from "vitest"; + +import { SecretStore } from "../framework/secrets.ts"; +import { redactString } from "../scenarios/orchestrators/redaction.ts"; + +describe("framework redaction entry point", () => { + it("redacts explicit values with [REDACTED] and canonical shapes with ", () => { + const explicit = "test-secret-aBcD"; + const canonical = `nvapi-${"x".repeat(24)}`; + const text = `explicit=${explicit} canonical=${canonical}`; + + const out = redactString(text, [explicit]); + + expect(out).toContain("[REDACTED]"); + expect(out).toContain(""); + expect(out).not.toContain(explicit); + expect(out).not.toContain(canonical); + }); + + it("applies explicit values longest first so a shorter substring cannot expose a longer one", () => { + const longer = "alpha-beta-gamma"; + const shorter = "alpha"; + const text = `value=${longer}`; + + const out = redactString(text, [shorter, longer]); + + expect(out).toBe("value=[REDACTED]"); + expect(out).not.toContain("-beta-gamma"); + expect(out).not.toContain(shorter); + }); + + it("ignores empty explicit values without throwing", () => { + const out = redactString("plain text", ["", " "]); + expect(out).toBe("plain text"); + }); + + it("returns the input unchanged when no explicit values are supplied and no shape matches", () => { + expect(redactString("nothing sensitive here")).toBe("nothing sensitive here"); + expect(redactString("nothing sensitive here", [])).toBe("nothing sensitive here"); + }); + + it("returns empty input verbatim", () => { + expect(redactString("")).toBe(""); + expect(redactString("", ["anything"])).toBe(""); + }); + + it("SecretStore.redact routes through the same entry and unions env-derived and caller-supplied values", () => { + const envSecret = "env-secret-value"; + const extraSecret = "extra-secret-value"; + const canonical = `ghp_${"y".repeat(36)}`; + const store = new SecretStore( + { + MY_API_KEY: envSecret, + UNRELATED_VAR: "kept-visible", + }, + (note?: string): never => { + throw new Error(note ?? "skipped"); + }, + ); + + const text = `env=${envSecret} extra=${extraSecret} canonical=${canonical} keep=kept-visible`; + const out = store.redact(text, [extraSecret]); + + expect(out).toContain("env=[REDACTED]"); + expect(out).toContain("extra=[REDACTED]"); + expect(out).toContain("canonical="); + expect(out).toContain("keep=kept-visible"); + expect(out).not.toContain(envSecret); + expect(out).not.toContain(extraSecret); + expect(out).not.toContain(canonical); + }); +}); diff --git a/test/e2e-scenario/framework/secrets.ts b/test/e2e-scenario/framework/secrets.ts index e6ab9daf99a..d1d75a0ee41 100644 --- a/test/e2e-scenario/framework/secrets.ts +++ b/test/e2e-scenario/framework/secrets.ts @@ -4,28 +4,16 @@ import { redactString } from "../scenarios/orchestrators/redaction.ts"; const SENSITIVE_NAME_PATTERN = /(api[_-]?key|token|secret|password|credential)/i; -const EXPLICIT_SECRET_REDACTION = "[REDACTED]"; /** - * Bridge-only fixture secret helper. + * Fixture-scoped env-secret store. * - * The Vitest fixture layer still needs a small SecretStore while the scenario - * runner migration is in flight; #4989 tracks consolidating it into shared E2E - * framework infra. Canonical secret-shaped token matching belongs to - * scenarios/orchestrators/redaction.ts. Keep explicit fixture secret-value - * replacement here and always layer the parity-tested framework redactor - * underneath it so this path does not become a second pattern source. + * Holds the per-test view of `process.env` and lets fixtures discover + * sensitive values by name. Redaction itself is owned by the canonical + * entry point in scenarios/orchestrators/redaction.ts; this class only + * supplies the explicit values it knows about and delegates. There is + * no separate fixture redaction pattern source. */ - -export function redactText(text: string, secretValues: Iterable): string { - let redacted = text; - for (const value of secretValues) { - if (!value) continue; - redacted = redacted.split(value).join(EXPLICIT_SECRET_REDACTION); - } - return redactString(redacted); -} - export class SecretStore { private readonly env: NodeJS.ProcessEnv; private readonly skip: (note?: string) => never; @@ -62,6 +50,6 @@ export class SecretStore { } redact(text: string, extraValues: string[] = []): string { - return redactText(text, this.redactionValues(extraValues)); + return redactString(text, this.redactionValues(extraValues)); } } diff --git a/test/e2e-scenario/framework/shell-probe.ts b/test/e2e-scenario/framework/shell-probe.ts index 540f9bd3725..14b3417a86f 100644 --- a/test/e2e-scenario/framework/shell-probe.ts +++ b/test/e2e-scenario/framework/shell-probe.ts @@ -4,16 +4,16 @@ import { spawn } from "node:child_process"; import type { ArtifactSink } from "./artifacts.ts"; -import { redactText } from "./secrets.ts"; /** * Bridge-only host shell probe for the Vitest fixture migration. * * The end state is a shared spawn/evidence helper consumed by both this - * fixture layer and scenarios/orchestrators; #4988 tracks that consolidation. - * Until it lands, this probe mirrors the hardened shell boundary: trusted - * descriptors, NUL-byte rejection, explicit env by default, canonical - * redaction, and detached process-group termination for timeout/abort cleanup. + * fixture layer and scenarios/orchestrators; that consolidation is tracked + * separately. Until it lands, this probe mirrors the hardened shell boundary: + * trusted descriptors, NUL-byte rejection, explicit env by default, canonical + * redaction (routed through the single shared entry point), and detached + * process-group termination for timeout/abort cleanup. */ export interface ShellProbeRunOptions { @@ -137,7 +137,18 @@ export class ShellProbe { const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; const killGraceMs = options.killGraceMs ?? DEFAULT_KILL_GRACE_MS; const redactionValues = options.redactionValues ?? []; - const redactProbeText = (text: string) => this.redact(redactText(text, redactionValues)); + const enforcedValues = [ + ...new Set(redactionValues.filter((value) => value && value.length > 0)), + ].sort((a, b) => b.length - a.length); + const enforceLocalRedaction = (text: string): string => { + let out = text; + for (const value of enforcedValues) { + out = out.split(value).join("[REDACTED]"); + } + return out; + }; + const redactProbeText = (text: string) => + this.redact(enforcedValues.length > 0 ? enforceLocalRedaction(text) : text, redactionValues); const redactedCommand = [command, ...args].map(redactProbeText); const artifactBase = `shell/${safeArtifactBase(redactProbeText(options.artifactName ?? command))}`; const writeArtifacts = async (result: Omit): Promise => ({ diff --git a/test/e2e-scenario/scenarios/orchestrators/redaction.ts b/test/e2e-scenario/scenarios/orchestrators/redaction.ts index 6424964230c..3286010f180 100644 --- a/test/e2e-scenario/scenarios/orchestrators/redaction.ts +++ b/test/e2e-scenario/scenarios/orchestrators/redaction.ts @@ -35,6 +35,7 @@ import type { Readable, Writable } from "node:stream"; const REDACTED = ""; +const EXPLICIT_REDACTED = "[REDACTED]"; // Framework-local mirror of src/lib/security/secret-patterns.ts. The // framework deliberately does not import from src/lib/security/ so it @@ -77,13 +78,28 @@ export const CONTEXT_PATTERNS: RegExp[] = [ * Replace every secret-shaped token in `text` with ``. Uses * the canonical TOKEN_PREFIX_PATTERNS + CONTEXT_PATTERNS sets. * + * When `explicitValues` is supplied, each non-empty value is replaced + * verbatim with `[REDACTED]` before the regex passes run, so per-test + * secret literals (which may not match any canonical shape) are + * scrubbed at the same single entry point. The distinct sentinel keeps + * explicit-value hits visually separable from regex hits in artifacts. + * Values are applied longest first so a value that contains a shorter + * one cannot be exposed by ordering. + * * Best-effort against unknown token shapes. The actual defense is the * env allowlist (buildChildEnv); pattern redaction catches what slips * through (e.g. error messages that echo a secret value). */ -export function redactString(text: string): string { +export function redactString(text: string, explicitValues?: Iterable): string { if (!text) return text; let out = text; + if (explicitValues) { + const values = [...new Set(Array.from(explicitValues).filter((value) => value && value.length > 0))]; + values.sort((a, b) => b.length - a.length); + for (const value of values) { + out = out.split(value).join(EXPLICIT_REDACTED); + } + } for (const p of TOKEN_PREFIX_PATTERNS) { p.lastIndex = 0; out = out.replace(p, REDACTED);