Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions test/e2e-scenario/framework-tests/e2e-fixture-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
89 changes: 89 additions & 0 deletions test/e2e-scenario/framework-tests/e2e-redaction-entry.test.ts
Original file line number Diff line number Diff line change
@@ -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 <REDACTED>", () => {
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("<REDACTED>");
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=<REDACTED>");
expect(out).toContain("keep=kept-visible");
expect(out).not.toContain(envSecret);
expect(out).not.toContain(extraSecret);
expect(out).not.toContain(canonical);
});
});
26 changes: 7 additions & 19 deletions test/e2e-scenario/framework/secrets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>): 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;
Expand Down Expand Up @@ -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));
}
}
23 changes: 17 additions & 6 deletions test/e2e-scenario/framework/shell-probe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<ShellProbeResult, "artifacts">): Promise<ShellProbeResult["artifacts"]> => ({
Expand Down
18 changes: 17 additions & 1 deletion test/e2e-scenario/scenarios/orchestrators/redaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import type { Readable, Writable } from "node:stream";

const REDACTED = "<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
Expand Down Expand Up @@ -77,13 +78,28 @@ export const CONTEXT_PATTERNS: RegExp[] = [
* Replace every secret-shaped token in `text` with `<REDACTED>`. 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>): 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);
Expand Down
Loading