From 0a0b339af096d460e1ddcad65ff1f1648dd9f099 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Sun, 9 Aug 2026 23:45:57 -0700 Subject: [PATCH 1/2] test(e2e): capture Hermes MCP rebuild failures Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 18 +- test/e2e/fixtures/shell-probe.ts | 42 +- test/e2e/live/mcp-bridge-hermes-http.ts | 142 ++++++ test/e2e/live/mcp-bridge.test.ts | 31 +- test/e2e/support/e2e-redaction-entry.test.ts | 43 ++ .../support/mcp-bridge-hermes-http.test.ts | 460 ++++++++++++++++++ 6 files changed, 724 insertions(+), 12 deletions(-) create mode 100644 test/e2e/live/mcp-bridge-hermes-http.ts create mode 100644 test/e2e/support/mcp-bridge-hermes-http.test.ts diff --git a/test/e2e/README.md b/test/e2e/README.md index c1d8b40b352..b02f21ea3f2 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -739,11 +739,19 @@ Live modules import `fixtures/e2e-test.ts`; selected integration modules import It also follows shared E2E runtime helpers. Run child processes through `ShellProbe` or an existing audited progress-aware boundary; new direct async process boundaries fail the check. Synchronous calls require both a positive -timeout shorter than the first heartbeat and `killSignal: "SIGKILL"`. Keep child -contents in redacted artifacts and report only timestamp-based output activity -to the console. Pass the fixture-provided frozen, canonical `progress` -capability unchanged to an audited subprocess boundary; do not replace it with -a custom, copied, or no-op adapter. +timeout shorter than the first heartbeat and `killSignal: "SIGKILL"`. +Keep child contents in redacted artifacts. +During command execution, report only timestamp-based output activity. +Before a `ShellProbe` command runs, register every known scenario-specific +sensitive value in `redactionValues`. +A terminal failure assertion may include a bounded preview only after the +fixture redacts the output. +Pass the same explicit values to the assertion before it formats the preview. +Cover the preview limit and redaction behavior with deterministic tests. +Success paths must not emit response contents. +Pass the fixture-provided frozen, canonical `progress` capability unchanged to +an audited subprocess boundary. +Do not replace it with a custom, copied, or no-op adapter. ## Push and Manual PR E2E diff --git a/test/e2e/fixtures/shell-probe.ts b/test/e2e/fixtures/shell-probe.ts index cb704e7e770..0013f08b9c7 100644 --- a/test/e2e/fixtures/shell-probe.ts +++ b/test/e2e/fixtures/shell-probe.ts @@ -28,6 +28,8 @@ export interface ShellProbeRunOptions { redactionValues?: string[]; /** Retain at most the last N bytes from each output stream. */ captureLimitBytes?: number; + /** After redaction, retain at most the first N UTF-8 bytes from each output stream. */ + postRedactionCaptureLimitBytes?: number; /** Timestamp-only output observer; chunk contents never cross this boundary. */ onOutput?: (event: ShellProbeOutputEvent) => void; } @@ -98,6 +100,12 @@ interface TextCapture { }; } +function validateCaptureLimitBytes(value: number | undefined, optionName: string): void { + if (value !== undefined && (!Number.isSafeInteger(value) || value <= 0)) { + throw new Error(`${optionName} must be a positive safe integer`); + } +} + function createTextCapture(limitBytes: number | undefined): TextCapture { if (limitBytes === undefined) { let text = ""; @@ -110,9 +118,7 @@ function createTextCapture(limitBytes: number | undefined): TextCapture { }, }; } - if (!Number.isSafeInteger(limitBytes) || limitBytes <= 0) { - throw new Error("captureLimitBytes must be a positive safe integer"); - } + validateCaptureLimitBytes(limitBytes, "captureLimitBytes"); let droppedBytes = 0; let tail = Buffer.alloc(0); @@ -152,6 +158,24 @@ function redactTruncatedSecretPrefix(text: string, redactionValues: string[]): s return fragmentLength > 0 ? `[REDACTED]${text.slice(fragmentLength)}` : text; } +function truncateRedactedUtf8Prefix(text: string, limitBytes: number | undefined): string { + if (limitBytes === undefined) return text; + const bytes = Buffer.from(text.replaceAll("\uFFFD", "?"), "utf8"); + if (bytes.length <= limitBytes) return bytes.toString("utf8"); + let end = limitBytes; + let sequenceStart = end - 1; + while (sequenceStart >= 0 && (bytes[sequenceStart]! & 0xc0) === 0x80) { + sequenceStart -= 1; + } + if (sequenceStart >= 0) { + const first = bytes[sequenceStart]!; + const sequenceLength = + (first & 0x80) === 0 ? 1 : (first & 0xe0) === 0xc0 ? 2 : (first & 0xf0) === 0xe0 ? 3 : 4; + if (sequenceStart + sequenceLength > end) end = sequenceStart; + } + return bytes.subarray(0, end).toString("utf8"); +} + export class ShellProbe { private readonly artifacts: ArtifactSink; private readonly progress: ChildProcessProgress; @@ -174,6 +198,10 @@ export class ShellProbe { const args = [...trustedCommand.args]; const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; const killGraceMs = options.killGraceMs ?? DEFAULT_KILL_GRACE_MS; + validateCaptureLimitBytes( + options.postRedactionCaptureLimitBytes, + "postRedactionCaptureLimitBytes", + ); const redactionValues = options.redactionValues ?? []; const enforcedValues = [ ...new Set(redactionValues.filter((value) => value && value.length > 0)), @@ -192,9 +220,11 @@ export class ShellProbe { const boundarySafeText = droppedBytes > 0 ? redactTruncatedSecretPrefix(text, enforcedValues) : text; const redacted = redactProbeText(boundarySafeText); - return droppedBytes > 0 - ? `[shell-probe omitted ${droppedBytes} earlier bytes; showing up to the last ${limitBytes} bytes]\n${redacted}` - : redacted; + const rendered = + droppedBytes > 0 + ? `[shell-probe omitted ${droppedBytes} earlier bytes; showing up to the last ${limitBytes} bytes]\n${redacted}` + : redacted; + return truncateRedactedUtf8Prefix(rendered, options.postRedactionCaptureLimitBytes); }; const redactedCommand = [command, ...args].map(redactProbeText); const activityName = safeArtifactBase(redactProbeText(options.artifactName ?? command)); diff --git a/test/e2e/live/mcp-bridge-hermes-http.ts b/test/e2e/live/mcp-bridge-hermes-http.ts new file mode 100644 index 00000000000..b11f0740145 --- /dev/null +++ b/test/e2e/live/mcp-bridge-hermes-http.ts @@ -0,0 +1,142 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { shellQuote } from "../../../src/lib/core/shell-quote"; +import { redactString } from "../fixtures/redaction.ts"; + +export const HERMES_MCP_HTTP_STATUS_MARKER = "NEMOCLAW_HERMES_MCP_HTTP_STATUS="; +export const HERMES_MCP_RESULT_TOKEN_MARKER = "NEMOCLAW_HERMES_MCP_RESULT_TOKEN="; +export const HERMES_MCP_FAILURE_CAPTURE_BYTES = 4_096; +export const HERMES_MCP_FAILURE_PREVIEW_CHARS = 1_024; +export const HERMES_MCP_RESPONSE_FILE_BYTES = 65_536; +export const HERMES_MCP_OVERSIZE_BODY_MARKER = + ""; + +interface HermesMcpCommandResult { + exitCode: number | null; + signal?: NodeJS.Signals | null; + stdout: string; + stderr: string; +} + +interface McpRequestObservation { + rpcMethod?: string; + auth?: string; + path?: string; +} + +const FAILURE_BODY_EMITTER = [ + "import os, pathlib, sys", + "path = pathlib.Path(sys.argv[1])", + "limit = int(sys.argv[2])", + `oversize = ${JSON.stringify(HERMES_MCP_OVERSIZE_BODY_MARKER)}.encode('utf-8')`, + "secret = os.environ.get('API_SERVER_KEY', '').encode('utf-8')", + "with path.open('rb') as stream:", + " raw = stream.read(limit + 1)", + "if len(raw) > limit or len(secret) > limit:", + " sys.stdout.buffer.write(oversize)", + "else:", + " replacement = b'[REDACTED]' if len(secret) >= 10 else b'*' * len(secret)", + " sys.stdout.buffer.write(raw.replace(secret, replacement) if secret else raw)", +].join("\n"); + +export function buildHermesMcpChatProbeScript(payload: string, resultToken: string): string { + return [ + "set -eu", + "umask 077", + 'response_file="$(mktemp /tmp/nemoclaw-hermes-mcp-chat.XXXXXX)"', + "trap 'rm -f \"$response_file\"' EXIT", + `set -- -sS --max-time 180 --max-filesize ${HERMES_MCP_RESPONSE_FILE_BYTES} -o "$response_file" -w '%{http_code}' http://localhost:8642/v1/chat/completions -H 'Content-Type: application/json'`, + 'if [ -n "${API_SERVER_KEY:-}" ]; then set -- "$@" -H "Authorization: Bearer ${API_SERVER_KEY}"; fi', + `emit_failure_body() { /usr/bin/python3 -I -S -c ${shellQuote(FAILURE_BODY_EMITTER)} "$response_file" ${HERMES_MCP_RESPONSE_FILE_BYTES}; }`, + "set +e", + `status="$(curl "$@" --data-binary ${shellQuote(payload)})"`, + "curl_rc=$?", + "set -e", + `printf '\\n${HERMES_MCP_HTTP_STATUS_MARKER}%s\\n' "$status" >&2`, + 'if [ "$curl_rc" -ne 0 ]; then exit "$curl_rc"; fi', + `case "$status" in 2??) if grep -Fq -- ${shellQuote(resultToken)} "$response_file"; then printf '${HERMES_MCP_RESULT_TOKEN_MARKER}present\\n' >&2; else printf '${HERMES_MCP_RESULT_TOKEN_MARKER}missing\\n' >&2; fi ;; *) emit_failure_body ;; esac`, + ].join("\n"); +} + +function sanitizedPreview(text: string, explicitValues: Iterable): string { + const sanitized = redactString(text, explicitValues) + .replace(/\u001b\[[0-?]*[ -/]*[@-~]/gu, "") + .replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/gu, " ") + .trim(); + if (!sanitized) return ""; + const characters = [...sanitized]; + if (characters.length <= HERMES_MCP_FAILURE_PREVIEW_CHARS) return sanitized; + const suffix = "… [truncated]"; + return `${characters + .slice(0, HERMES_MCP_FAILURE_PREVIEW_CHARS - [...suffix].length) + .join("")}${suffix}`; +} + +function parseHttpStatus(stderr: string): number | null { + const escapedMarker = HERMES_MCP_HTTP_STATUS_MARKER.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const matches = [...stderr.matchAll(new RegExp(`^${escapedMarker}([0-9]{3})$`, "gmu"))]; + if (matches.length !== 1) return null; + return Number(matches[0]?.[1]); +} + +function hasResultToken(stderr: string): boolean { + const marker = `${HERMES_MCP_RESULT_TOKEN_MARKER}present`; + return stderr.split(/\r?\n/u).filter((line) => line === marker).length === 1; +} + +export function assertHermesMcpHttpResponse( + result: HermesMcpCommandResult, + explicitRedactionValues: Iterable, +): void { + const status = parseHttpStatus(result.stderr); + if (result.exitCode !== 0) { + const transport = result.signal + ? `signal=${result.signal}` + : `exit=${result.exitCode ?? "unknown"}`; + const detail = sanitizedPreview([result.stdout, result.stderr].filter(Boolean).join("\n"), [ + ...explicitRedactionValues, + ]); + throw new Error(`Hermes real MCP tool call transport failed (${transport}): ${detail}`); + } + if (status === null) { + throw new Error("Hermes real MCP tool call did not report exactly one HTTP status marker"); + } + if (status < 200 || status >= 300) { + const body = sanitizedPreview(result.stdout, explicitRedactionValues); + throw new Error( + `Hermes real MCP tool call failed: HTTP ${status}; redacted response body: ${body}`, + ); + } + if (!hasResultToken(result.stderr)) { + throw new Error("Hermes real MCP tool call response did not contain the fixture result token"); + } + if (result.stdout !== "") { + throw new Error("Hermes real MCP tool call success path emitted response contents"); + } +} + +export function assertAuthenticatedMcpToolCallOutcome(options: { + requests: readonly McpRequestObservation[]; + callsBefore: number; + expectedSecret: string; +}): void { + const calls = options.requests.filter((request) => request.rpcMethod === "tools/call"); + if (calls.length !== options.callsBefore + 1) { + throw new Error( + `Hermes real MCP tool call must issue exactly one tools/call request (observed delta=${calls.length - options.callsBefore})`, + ); + } + const call = calls.at(-1); + if (call?.path !== "/mcp") { + throw new Error("Hermes real MCP tool call did not reach the managed /mcp endpoint"); + } + if (call.auth !== `Bearer ${options.expectedSecret}`) { + throw new Error( + "Hermes real MCP tool call did not use the expected resolved bearer credential", + ); + } + if (call.auth.includes("openshell:resolve:env")) { + throw new Error("Hermes real MCP tool call forwarded an unresolved credential placeholder"); + } +} diff --git a/test/e2e/live/mcp-bridge.test.ts b/test/e2e/live/mcp-bridge.test.ts index a40359ef8c3..eab7d1daf1d 100644 --- a/test/e2e/live/mcp-bridge.test.ts +++ b/test/e2e/live/mcp-bridge.test.ts @@ -26,6 +26,12 @@ import { type McpAdapter, removeMcpBridgeWithOneConcurrencyRetry, } from "./mcp-bridge-cleanup.ts"; +import { + assertAuthenticatedMcpToolCallOutcome, + assertHermesMcpHttpResponse, + buildHermesMcpChatProbeScript, + HERMES_MCP_FAILURE_CAPTURE_BYTES, +} from "./mcp-bridge-hermes-http.ts"; import { assertHermesConfig, assertHermesInspectionRejectsUnmanagedFields, @@ -537,6 +543,17 @@ async function assertRealAdapterToolCall( messages: [{ role: "user", content: prompt }], max_tokens: 256, }); + // ShellProbe redacts these values before it returns command output or writes + // artifacts. The HTTP assertion redacts them again before Vitest formats a + // bounded failure preview. + const hermesRedactionValues = [ + HOST_SECRET, + ROTATED_HOST_SECRET, + COMPATIBLE_KEY, + TOOL_CHALLENGE, + prompt, + hermesPayload, + ]; const command = options.agent === "openclaw" ? `nemoclaw-start mcporter call fake.fake_echo --args ${JSON.stringify(JSON.stringify({ challenge: TOOL_CHALLENGE }))} --output json` @@ -545,7 +562,7 @@ async function assertRealAdapterToolCall( "set -a", "[ ! -f /sandbox/.hermes/.env ] || . /sandbox/.hermes/.env", "set +a", - `if [ -n "\${API_SERVER_KEY:-}" ]; then curl -fsS --max-time 180 http://localhost:8642/v1/chat/completions -H 'Content-Type: application/json' -H "Authorization: Bearer \${API_SERVER_KEY}" --data-binary ${shellQuote(hermesPayload)}; else curl -fsS --max-time 180 http://localhost:8642/v1/chat/completions -H 'Content-Type: application/json' --data-binary ${shellQuote(hermesPayload)}; fi`, + buildHermesMcpChatProbeScript(hermesPayload, options.resultToken), ].join("\n") : `nemoclaw-start dcode -n ${JSON.stringify(prompt)}`; const result = await sandbox.execShell( @@ -554,9 +571,21 @@ async function assertRealAdapterToolCall( { artifactName: options.artifactName, env: buildAvailabilityProbeEnv(), + postRedactionCaptureLimitBytes: + options.agent === "hermes" ? HERMES_MCP_FAILURE_CAPTURE_BYTES : undefined, + redactionValues: options.agent === "hermes" ? hermesRedactionValues : [], timeoutMs: 5 * 60_000, }, ); + if (options.agent === "hermes") { + assertHermesMcpHttpResponse(result, hermesRedactionValues); + assertAuthenticatedMcpToolCallOutcome({ + requests: fakeMcp.requests, + callsBefore: before, + expectedSecret: options.expectedSecret ?? HOST_SECRET, + }); + return; + } expectExitZero(result, `${options.agent} real MCP tool call`); expect(resultText(result)).toContain(options.resultToken); const calls = fakeMcp.requests.filter((request) => request.rpcMethod === "tools/call"); diff --git a/test/e2e/support/e2e-redaction-entry.test.ts b/test/e2e/support/e2e-redaction-entry.test.ts index 71de84a2771..b2b4e65dfb4 100644 --- a/test/e2e/support/e2e-redaction-entry.test.ts +++ b/test/e2e/support/e2e-redaction-entry.test.ts @@ -435,4 +435,47 @@ describe("fixture redaction entry point", () => { await fs.rm(rootDir, { recursive: true, force: true }); } }); + + it.each([ + 0, + -1, + 1.5, + Number.POSITIVE_INFINITY, + Number.MAX_SAFE_INTEGER + 1, + ])("rejects invalid post-redaction capture limit %s before spawning a child or writing artifacts (#8697)", async (postRedactionCaptureLimitBytes) => { + const rootDir = await fs.mkdtemp( + path.join(os.tmpdir(), "nemoclaw-e2e-invalid-post-redaction-capture-"), + ); + try { + const artifactRoot = path.join(rootDir, "e2e-artifacts/live/invalid-post-redaction-capture"); + const spawnMarker = path.join(rootDir, "spawned.txt"); + const artifacts = new ArtifactSink(artifactRoot); + await artifacts.ensureRoot(); + const probe = new ShellProbe({ + artifacts, + progress: supportProgress(), + redact: (text, extra) => redactString(text, extra), + signal: new AbortController().signal, + }); + + await expect( + probe.run( + trustedShellCommand({ + command: "bash", + args: ["-lc", 'printf spawned >"$SPAWN_MARKER"'], + reason: + "verify that ShellProbe rejects invalid post-redaction limits before child execution", + }), + { + env: { SPAWN_MARKER: spawnMarker }, + postRedactionCaptureLimitBytes, + }, + ), + ).rejects.toThrow("postRedactionCaptureLimitBytes must be a positive safe integer"); + await expect(fs.access(spawnMarker)).rejects.toThrow(); + await expect(fs.readdir(artifactRoot)).resolves.toEqual([]); + } finally { + await fs.rm(rootDir, { recursive: true, force: true }); + } + }); }); diff --git a/test/e2e/support/mcp-bridge-hermes-http.test.ts b/test/e2e/support/mcp-bridge-hermes-http.test.ts new file mode 100644 index 00000000000..683e5e12654 --- /dev/null +++ b/test/e2e/support/mcp-bridge-hermes-http.test.ts @@ -0,0 +1,460 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { ArtifactSink } from "../fixtures/artifacts.ts"; +import { startTestProgress } from "../fixtures/progress.ts"; +import { redactString } from "../fixtures/redaction.ts"; +import { ShellProbe, trustedShellCommand } from "../fixtures/shell-probe.ts"; +import { + assertAuthenticatedMcpToolCallOutcome, + assertHermesMcpHttpResponse, + buildHermesMcpChatProbeScript, + HERMES_MCP_FAILURE_CAPTURE_BYTES, + HERMES_MCP_FAILURE_PREVIEW_CHARS, + HERMES_MCP_HTTP_STATUS_MARKER, + HERMES_MCP_OVERSIZE_BODY_MARKER, + HERMES_MCP_RESPONSE_FILE_BYTES, + HERMES_MCP_RESULT_TOKEN_MARKER, +} from "../live/mcp-bridge-hermes-http.ts"; + +const SPAWN_TIMEOUT_MS = 5_000; +const SYSTEM_PATH = "/usr/bin:/bin"; + +function deterministicEnv(overrides: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + return { + PATH: overrides.PATH ?? SYSTEM_PATH, + ...overrides, + }; +} + +function httpResult(status: number, body: string, resultTokenState?: "present" | "missing") { + return { + exitCode: 0, + signal: null, + stdout: body, + stderr: + `${HERMES_MCP_HTTP_STATUS_MARKER}${status}\n` + + (resultTokenState ? `${HERMES_MCP_RESULT_TOKEN_MARKER}${resultTokenState}\n` : ""), + }; +} + +describe("Hermes MCP HTTP failure diagnostics", () => { + it("sends one authenticated request per chat probe execution without retrying (#8697)", () => { + const resultToken = "fixture-result-token"; + const script = buildHermesMcpChatProbeScript('{"messages":[]}', resultToken); + const syntax = spawnSync("sh", ["-n"], { + encoding: "utf8", + input: script, + killSignal: "SIGKILL", + timeout: SPAWN_TIMEOUT_MS, + }); + + expect(syntax.status, syntax.stderr).toBe(0); + expect(script.match(/\bcurl "\$@"/gu)).toHaveLength(1); + expect(script).toContain("Authorization: Bearer ${API_SERVER_KEY}"); + expect(script).toContain(`--max-filesize ${HERMES_MCP_RESPONSE_FILE_BYTES}`); + expect(script).toContain("/usr/bin/python3 -I -S -c"); + expect(script).not.toMatch(/\bretry\b|\bfor\b|\bwhile\b/iu); + expect(script).not.toContain('cat "$response_file"'); + + const directory = mkdtempSync(path.join(tmpdir(), "nemoclaw-hermes-mcp-http-")); + const curl = path.join(directory, "curl"); + const count = path.join(directory, "count"); + writeFileSync( + curl, + [ + "#!/bin/sh", + "set -eu", + "output=", + "authenticated=0", + 'while [ "$#" -gt 0 ]; do', + ' if [ "$1" = "-o" ]; then output="$2"; shift 2; continue; fi', + ' if [ "$1" = "-H" ] && [ "$2" = "Authorization: Bearer ${FAKE_EXPECTED_API_KEY}" ]; then authenticated=1; fi', + " shift", + "done", + '[ "$authenticated" -eq 1 ]', + 'printf "1\\n" >> "$FAKE_CURL_COUNT"', + 'if [ -n "${FAKE_CURL_BODY_FILE:-}" ]; then cp "$FAKE_CURL_BODY_FILE" "$output"; else printf "%s" "$FAKE_CURL_BODY" > "$output"; fi', + 'printf "%s" "${FAKE_CURL_STATUS:-200}"', + ].join("\n"), + "utf8", + ); + chmodSync(curl, 0o755); + try { + const executed = spawnSync("sh", ["-c", script], { + encoding: "utf8", + env: deterministicEnv({ + API_SERVER_KEY: "fixture-api-key", + FAKE_CURL_BODY: `private response contents ${resultToken}`, + FAKE_CURL_BODY_FILE: "", + FAKE_CURL_COUNT: count, + FAKE_CURL_STATUS: "200", + FAKE_EXPECTED_API_KEY: "fixture-api-key", + PATH: `${directory}:${SYSTEM_PATH}`, + }), + killSignal: "SIGKILL", + timeout: SPAWN_TIMEOUT_MS, + }); + expect(executed.status, executed.stderr).toBe(0); + expect(readFileSync(count, "utf8")).toBe("1\n"); + expect(executed.stdout).toBe(""); + expect(executed.stderr).toContain(`${HERMES_MCP_HTTP_STATUS_MARKER}200`); + expect(executed.stderr).toContain(`${HERMES_MCP_RESULT_TOKEN_MARKER}present`); + expect(executed.stderr).not.toContain("private response contents"); + + writeFileSync( + path.join(directory, "sitecustomize.py"), + [ + "import os, sys", + "secret = os.environ.get('API_SERVER_KEY', '')", + "sys.stdout.write(secret)", + "sys.stderr.write(secret)", + ].join("\n"), + "utf8", + ); + const hostileKey = "fixture-api-key"; + const failed = spawnSync("sh", ["-c", script], { + encoding: "utf8", + env: deterministicEnv({ + API_SERVER_KEY: hostileKey, + FAKE_CURL_BODY: "failure body without credential", + FAKE_CURL_BODY_FILE: "", + FAKE_CURL_COUNT: count, + FAKE_CURL_STATUS: "500", + FAKE_EXPECTED_API_KEY: hostileKey, + PATH: `${directory}:${SYSTEM_PATH}`, + PYTHONPATH: directory, + PYTHONWARNINGS: `${hostileKey}::Warning`, + }), + killSignal: "SIGKILL", + timeout: SPAWN_TIMEOUT_MS, + }); + expect(failed.status, failed.stderr).toBe(0); + expect(readFileSync(count, "utf8")).toBe("1\n1\n"); + expect(failed.stdout).toBe("failure body without credential"); + expect(failed.stdout).not.toContain(hostileKey); + expect(failed.stderr).toContain(`${HERMES_MCP_HTTP_STATUS_MARKER}500`); + expect(failed.stderr).not.toContain(HERMES_MCP_RESULT_TOKEN_MARKER); + expect(failed.stderr).not.toContain(hostileKey); + + const unknownKey = "synthetic-credential-segment-".repeat(4); + const boundaryStart = + HERMES_MCP_FAILURE_CAPTURE_BYTES - Math.floor(Buffer.byteLength(unknownKey) / 2); + const repeatedBody = `${unknownKey}${"x".repeat( + boundaryStart - Buffer.byteLength(unknownKey), + )}${unknownKey}tail`; + const repeated = spawnSync("sh", ["-c", script], { + encoding: "utf8", + env: deterministicEnv({ + API_SERVER_KEY: unknownKey, + FAKE_CURL_BODY: repeatedBody, + FAKE_CURL_BODY_FILE: "", + FAKE_CURL_COUNT: count, + FAKE_CURL_STATUS: "500", + FAKE_EXPECTED_API_KEY: unknownKey, + PATH: `${directory}:${SYSTEM_PATH}`, + }), + killSignal: "SIGKILL", + timeout: SPAWN_TIMEOUT_MS, + }); + expect(repeated.status, repeated.stderr).toBe(0); + expect(readFileSync(count, "utf8")).toBe("1\n1\n1\n"); + expect(repeated.stdout.match(/\[REDACTED\]/gu)).toHaveLength(2); + expect(repeated.stdout).not.toContain(unknownKey); + expect(repeated.stdout).not.toContain(unknownKey.slice(0, 32)); + expect(repeated.stdout).not.toContain(unknownKey.slice(-32)); + } finally { + rmSync(directory, { force: true, recursive: true }); + } + }); + + it("redacts sensitive values before limiting text and binary response evidence (#8697)", async () => { + const resultToken = "fixture-result-token"; + const script = buildHermesMcpChatProbeScript('{"messages":[]}', resultToken); + const directory = mkdtempSync(path.join(tmpdir(), "nemoclaw-hermes-mcp-bytes-")); + const curl = path.join(directory, "curl"); + const bodyFile = path.join(directory, "body.bin"); + const apiKey = `boundary-api-key-${"k".repeat(64)}`; + const explicitValue = `private-prompt-${"p".repeat(64)}`; + const oversizeSecret = `oversize-private-${"s".repeat(80)}`; + writeFileSync( + curl, + [ + "#!/bin/sh", + "set -eu", + "max_size=", + "output=", + 'while [ "$#" -gt 0 ]; do', + ' if [ "$1" = "--max-filesize" ]; then max_size="$2"; shift 2; continue; fi', + ' if [ "$1" = "-o" ]; then output="$2"; shift 2; continue; fi', + " shift", + "done", + '[ -n "$max_size" ]', + 'if [ -n "${FAKE_CURL_IGNORE_MAX_SIZE:-}" ]; then', + ' cp "$FAKE_CURL_BODY_FILE" "$output"', + "else", + ' body_size="$(wc -c < "$FAKE_CURL_BODY_FILE")"', + ' if [ "$body_size" -gt "$max_size" ]; then', + ' dd if="$FAKE_CURL_BODY_FILE" of="$output" bs=1 count="$max_size" 2>/dev/null', + ' if [ -n "${FAKE_CURL_RESPONSE_SIZE_FILE:-}" ]; then wc -c < "$output" > "$FAKE_CURL_RESPONSE_SIZE_FILE"; fi', + ' printf "%s" "$FAKE_CURL_STATUS"', + " exit 63", + " fi", + ' cp "$FAKE_CURL_BODY_FILE" "$output"', + "fi", + 'if [ -n "${FAKE_CURL_RESPONSE_SIZE_FILE:-}" ]; then wc -c < "$output" > "$FAKE_CURL_RESPONSE_SIZE_FILE"; fi', + 'printf "%s" "$FAKE_CURL_STATUS"', + ].join("\n"), + "utf8", + ); + chmodSync(curl, 0o755); + const artifacts = new ArtifactSink(path.join(directory, "artifacts")); + const progress = startTestProgress( + "Hermes MCP HTTP diagnostic support", + ["run probe", "verify result"], + { logLine: () => undefined }, + ); + try { + await artifacts.ensureRoot(); + const probe = new ShellProbe({ + artifacts, + progress, + redact: (text, values) => redactString(text, values), + signal: new AbortController().signal, + }); + const runBody = async ( + body: Buffer, + artifactName: string, + envOverrides: NodeJS.ProcessEnv = {}, + ) => { + writeFileSync(bodyFile, body); + return probe.run( + trustedShellCommand({ + command: "sh", + args: ["-c", script], + reason: + "verify that ShellProbe redacts sensitive values before limiting response evidence", + }), + { + artifactName, + env: deterministicEnv({ + API_SERVER_KEY: apiKey, + FAKE_CURL_BODY: "", + FAKE_CURL_BODY_FILE: bodyFile, + FAKE_CURL_STATUS: "500", + PATH: `${directory}:${SYSTEM_PATH}`, + ...envOverrides, + }), + postRedactionCaptureLimitBytes: HERMES_MCP_FAILURE_CAPTURE_BYTES, + redactionValues: [apiKey, explicitValue, oversizeSecret], + timeoutMs: SPAWN_TIMEOUT_MS, + }, + ); + }; + + const secondKeyStart = + HERMES_MCP_FAILURE_CAPTURE_BYTES + Math.floor(Buffer.byteLength(apiKey) / 2); + const repeatedKeyBody = Buffer.from( + `${apiKey}${"x".repeat(secondKeyStart - Buffer.byteLength(apiKey))}${apiKey}tail`, + "utf8", + ); + const repeated = await runBody(repeatedKeyBody, "repeated-key-boundary"); + expect(Buffer.byteLength(repeated.stdout, "utf8")).toBeLessThanOrEqual( + HERMES_MCP_FAILURE_CAPTURE_BYTES, + ); + expect(repeated.stdout.match(/\[REDACTED\]/gu)).toHaveLength(2); + expect(repeated.stdout).not.toContain(apiKey.slice(0, 32)); + expect(repeated.stdout).not.toContain(apiKey.slice(-32)); + const repeatedArtifact = readFileSync( + artifacts.pathFor("shell/repeated-key-boundary.stdout.txt"), + ); + expect(repeatedArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); + expect(repeatedArtifact.includes(Buffer.from(apiKey.slice(0, 32), "utf8"))).toBe(false); + + const explicitStart = + HERMES_MCP_FAILURE_CAPTURE_BYTES - Math.floor(Buffer.byteLength(explicitValue) / 2); + const explicitBody = Buffer.from(`${"y".repeat(explicitStart)}${explicitValue}tail`, "utf8"); + const explicit = await runBody(explicitBody, "explicit-value-boundary"); + expect(Buffer.byteLength(explicit.stdout, "utf8")).toBeLessThanOrEqual( + HERMES_MCP_FAILURE_CAPTURE_BYTES, + ); + expect(explicit.stdout).toContain("[REDACTED]"); + expect(explicit.stdout).not.toContain(explicitValue.slice(0, 32)); + expect(explicit.stdout).not.toContain(explicitValue.slice(-32)); + const explicitArtifact = readFileSync( + artifacts.pathFor("shell/explicit-value-boundary.stdout.txt"), + ); + expect(explicitArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); + expect(explicitArtifact.includes(Buffer.from(explicitValue.slice(0, 32), "utf8"))).toBe( + false, + ); + + const binaryBody = Buffer.concat([ + Buffer.alloc(1_000, 0xff), + Buffer.from("🙂".repeat(1_000), "utf8"), + Buffer.alloc(1_000, 0x7a), + ]); + const binary = await runBody(binaryBody, "invalid-utf8-multibyte-boundary"); + expect(Buffer.byteLength(binary.stdout, "utf8")).toBeLessThanOrEqual( + HERMES_MCP_FAILURE_CAPTURE_BYTES, + ); + expect(binary.stdout.startsWith("?".repeat(1_000))).toBe(true); + expect(binary.stdout).toContain("🙂"); + expect(binary.stdout).not.toContain("�"); + expect(binary.stderr).not.toContain(apiKey); + expect( + readFileSync(artifacts.pathFor("shell/invalid-utf8-multibyte-boundary.stdout.txt")).length, + ).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); + + const oversizeSecretStart = + HERMES_MCP_RESPONSE_FILE_BYTES - Math.floor(Buffer.byteLength(oversizeSecret) / 2); + const oversizeBody = Buffer.from( + `${"q".repeat(oversizeSecretStart)}${oversizeSecret}${"r".repeat(1_024)}`, + "utf8", + ); + const responseSizeFile = path.join(directory, "oversize-response-size.txt"); + const curlLimited = await runBody(oversizeBody, "curl-limited-oversize-body", { + FAKE_CURL_RESPONSE_SIZE_FILE: responseSizeFile, + }); + expect(curlLimited.exitCode).toBe(63); + expect(Number(readFileSync(responseSizeFile, "utf8").trim())).toBe( + HERMES_MCP_RESPONSE_FILE_BYTES, + ); + expect(curlLimited.stdout).toBe(""); + expect(curlLimited.stdout).not.toContain(oversizeSecret.slice(0, 32)); + expect(curlLimited.stderr).not.toContain(oversizeSecret.slice(0, 32)); + const curlLimitedArtifact = readFileSync( + artifacts.pathFor("shell/curl-limited-oversize-body.stdout.txt"), + ); + expect(curlLimitedArtifact.length).toBe(0); + expect(curlLimitedArtifact.includes(Buffer.from(oversizeSecret.slice(0, 32), "utf8"))).toBe( + false, + ); + + const emitterLimited = await runBody(oversizeBody, "emitter-limited-oversize-body", { + FAKE_CURL_IGNORE_MAX_SIZE: "1", + }); + expect(emitterLimited.exitCode).toBe(0); + expect(emitterLimited.stdout).toBe(HERMES_MCP_OVERSIZE_BODY_MARKER); + expect(emitterLimited.stdout).not.toContain(oversizeSecret.slice(0, 32)); + const emitterLimitedArtifact = readFileSync( + artifacts.pathFor("shell/emitter-limited-oversize-body.stdout.txt"), + ); + expect(emitterLimitedArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); + expect(emitterLimitedArtifact.toString("utf8")).toBe(HERMES_MCP_OVERSIZE_BODY_MARKER); + } finally { + progress.stop(); + rmSync(directory, { force: true, recursive: true }); + } + }); + + it("rejects HTTP 500 with bounded, redacted response evidence (#8697)", () => { + const dynamicSecret = "fixture-dynamic-secret-value"; + const requestPayload = '{"messages":[{"content":"private diagnostic prompt"}]}'; + const longTail = "x".repeat(HERMES_MCP_FAILURE_PREVIEW_CHARS * 2); + const body = `${requestPayload}\nAuthorization: Bearer api-server-secret-value\n${dynamicSecret}\n${longTail}`; + + expect(() => + assertHermesMcpHttpResponse(httpResult(500, body), [dynamicSecret, requestPayload]), + ).toThrowError(/HTTP 500; redacted response body:/u); + + try { + assertHermesMcpHttpResponse(httpResult(500, body), [dynamicSecret, requestPayload]); + throw new Error("expected HTTP 500 to fail"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + expect(message).not.toContain(dynamicSecret); + expect(message).not.toContain(requestPayload); + expect(message).not.toContain("api-server-secret-value"); + expect(message).toContain("[REDACTED]"); + expect(message).toContain("[truncated]"); + expect([...message.split("redacted response body: ")[1]!].length).toBeLessThanOrEqual( + HERMES_MCP_FAILURE_PREVIEW_CHARS, + ); + } + }); + + it("requires a result token and exactly one authenticated tools/call after HTTP 2xx (#8697)", () => { + const request = { + rpcMethod: "tools/call", + auth: "Bearer rotated-fixture-secret", + path: "/mcp", + }; + + expect(() => assertHermesMcpHttpResponse(httpResult(200, "", "present"), [])).not.toThrow(); + expect(() => assertHermesMcpHttpResponse(httpResult(200, "", "missing"), [])).toThrowError( + /fixture result token/u, + ); + expect(() => + assertHermesMcpHttpResponse(httpResult(200, "raw response", "present"), []), + ).toThrowError(/success path emitted response contents/u); + expect(() => + assertAuthenticatedMcpToolCallOutcome({ + requests: [request], + callsBefore: 0, + expectedSecret: "rotated-fixture-secret", + }), + ).not.toThrow(); + expect(() => + assertAuthenticatedMcpToolCallOutcome({ + requests: [request, request], + callsBefore: 0, + expectedSecret: "rotated-fixture-secret", + }), + ).toThrowError(/exactly one tools\/call/u); + expect(() => + assertAuthenticatedMcpToolCallOutcome({ + requests: [{ ...request, path: "/other" }], + callsBefore: 0, + expectedSecret: "rotated-fixture-secret", + }), + ).toThrowError(/managed \/mcp endpoint/u); + expect(() => + assertAuthenticatedMcpToolCallOutcome({ + requests: [request], + callsBefore: 0, + expectedSecret: "unexpected-secret", + }), + ).toThrowError(/expected resolved bearer credential/u); + }); + + it("redacts transport evidence and rejects missing or duplicate HTTP status markers (#8697)", () => { + const secret = "fixture-transport-secret"; + for (const result of [ + { exitCode: 7, signal: null, stdout: secret, stderr: "transport detail" }, + { exitCode: null, signal: "SIGKILL" as const, stdout: secret, stderr: "" }, + ]) { + try { + assertHermesMcpHttpResponse(result, [secret]); + throw new Error("expected transport failure"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + expect(message).toMatch(/transport failed \((?:exit=7|signal=SIGKILL)\)/u); + expect(message).toContain("[REDACTED]"); + expect(message).not.toContain(secret); + } + } + + expect(() => + assertHermesMcpHttpResponse({ exitCode: 0, signal: null, stdout: "", stderr: "" }, []), + ).toThrowError(/exactly one HTTP status marker/u); + expect(() => + assertHermesMcpHttpResponse( + { + exitCode: 0, + signal: null, + stdout: "", + stderr: `${HERMES_MCP_HTTP_STATUS_MARKER}200\n${HERMES_MCP_HTTP_STATUS_MARKER}500\n`, + }, + [], + ), + ).toThrowError(/exactly one HTTP status marker/u); + }); +}); From f8228a6a398268045f47bf6570988531fd7b0796 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Mon, 10 Aug 2026 00:02:25 -0700 Subject: [PATCH 2/2] test(e2e): narrow Hermes rebuild diagnostics Signed-off-by: Apurv Kumaria --- test/e2e/README.md | 18 +- test/e2e/fixtures/shell-probe.ts | 42 +- test/e2e/live/mcp-bridge-hermes-http.ts | 48 +- test/e2e/live/mcp-bridge.test.ts | 23 +- test/e2e/support/e2e-redaction-entry.test.ts | 43 -- .../support/mcp-bridge-hermes-http.test.ts | 430 ++---------------- 6 files changed, 70 insertions(+), 534 deletions(-) diff --git a/test/e2e/README.md b/test/e2e/README.md index b02f21ea3f2..c1d8b40b352 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -739,19 +739,11 @@ Live modules import `fixtures/e2e-test.ts`; selected integration modules import It also follows shared E2E runtime helpers. Run child processes through `ShellProbe` or an existing audited progress-aware boundary; new direct async process boundaries fail the check. Synchronous calls require both a positive -timeout shorter than the first heartbeat and `killSignal: "SIGKILL"`. -Keep child contents in redacted artifacts. -During command execution, report only timestamp-based output activity. -Before a `ShellProbe` command runs, register every known scenario-specific -sensitive value in `redactionValues`. -A terminal failure assertion may include a bounded preview only after the -fixture redacts the output. -Pass the same explicit values to the assertion before it formats the preview. -Cover the preview limit and redaction behavior with deterministic tests. -Success paths must not emit response contents. -Pass the fixture-provided frozen, canonical `progress` capability unchanged to -an audited subprocess boundary. -Do not replace it with a custom, copied, or no-op adapter. +timeout shorter than the first heartbeat and `killSignal: "SIGKILL"`. Keep child +contents in redacted artifacts and report only timestamp-based output activity +to the console. Pass the fixture-provided frozen, canonical `progress` +capability unchanged to an audited subprocess boundary; do not replace it with +a custom, copied, or no-op adapter. ## Push and Manual PR E2E diff --git a/test/e2e/fixtures/shell-probe.ts b/test/e2e/fixtures/shell-probe.ts index 0013f08b9c7..cb704e7e770 100644 --- a/test/e2e/fixtures/shell-probe.ts +++ b/test/e2e/fixtures/shell-probe.ts @@ -28,8 +28,6 @@ export interface ShellProbeRunOptions { redactionValues?: string[]; /** Retain at most the last N bytes from each output stream. */ captureLimitBytes?: number; - /** After redaction, retain at most the first N UTF-8 bytes from each output stream. */ - postRedactionCaptureLimitBytes?: number; /** Timestamp-only output observer; chunk contents never cross this boundary. */ onOutput?: (event: ShellProbeOutputEvent) => void; } @@ -100,12 +98,6 @@ interface TextCapture { }; } -function validateCaptureLimitBytes(value: number | undefined, optionName: string): void { - if (value !== undefined && (!Number.isSafeInteger(value) || value <= 0)) { - throw new Error(`${optionName} must be a positive safe integer`); - } -} - function createTextCapture(limitBytes: number | undefined): TextCapture { if (limitBytes === undefined) { let text = ""; @@ -118,7 +110,9 @@ function createTextCapture(limitBytes: number | undefined): TextCapture { }, }; } - validateCaptureLimitBytes(limitBytes, "captureLimitBytes"); + if (!Number.isSafeInteger(limitBytes) || limitBytes <= 0) { + throw new Error("captureLimitBytes must be a positive safe integer"); + } let droppedBytes = 0; let tail = Buffer.alloc(0); @@ -158,24 +152,6 @@ function redactTruncatedSecretPrefix(text: string, redactionValues: string[]): s return fragmentLength > 0 ? `[REDACTED]${text.slice(fragmentLength)}` : text; } -function truncateRedactedUtf8Prefix(text: string, limitBytes: number | undefined): string { - if (limitBytes === undefined) return text; - const bytes = Buffer.from(text.replaceAll("\uFFFD", "?"), "utf8"); - if (bytes.length <= limitBytes) return bytes.toString("utf8"); - let end = limitBytes; - let sequenceStart = end - 1; - while (sequenceStart >= 0 && (bytes[sequenceStart]! & 0xc0) === 0x80) { - sequenceStart -= 1; - } - if (sequenceStart >= 0) { - const first = bytes[sequenceStart]!; - const sequenceLength = - (first & 0x80) === 0 ? 1 : (first & 0xe0) === 0xc0 ? 2 : (first & 0xf0) === 0xe0 ? 3 : 4; - if (sequenceStart + sequenceLength > end) end = sequenceStart; - } - return bytes.subarray(0, end).toString("utf8"); -} - export class ShellProbe { private readonly artifacts: ArtifactSink; private readonly progress: ChildProcessProgress; @@ -198,10 +174,6 @@ export class ShellProbe { const args = [...trustedCommand.args]; const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; const killGraceMs = options.killGraceMs ?? DEFAULT_KILL_GRACE_MS; - validateCaptureLimitBytes( - options.postRedactionCaptureLimitBytes, - "postRedactionCaptureLimitBytes", - ); const redactionValues = options.redactionValues ?? []; const enforcedValues = [ ...new Set(redactionValues.filter((value) => value && value.length > 0)), @@ -220,11 +192,9 @@ export class ShellProbe { const boundarySafeText = droppedBytes > 0 ? redactTruncatedSecretPrefix(text, enforcedValues) : text; const redacted = redactProbeText(boundarySafeText); - const rendered = - droppedBytes > 0 - ? `[shell-probe omitted ${droppedBytes} earlier bytes; showing up to the last ${limitBytes} bytes]\n${redacted}` - : redacted; - return truncateRedactedUtf8Prefix(rendered, options.postRedactionCaptureLimitBytes); + return droppedBytes > 0 + ? `[shell-probe omitted ${droppedBytes} earlier bytes; showing up to the last ${limitBytes} bytes]\n${redacted}` + : redacted; }; const redactedCommand = [command, ...args].map(redactProbeText); const activityName = safeArtifactBase(redactProbeText(options.artifactName ?? command)); diff --git a/test/e2e/live/mcp-bridge-hermes-http.ts b/test/e2e/live/mcp-bridge-hermes-http.ts index b11f0740145..ec0c76e18fc 100644 --- a/test/e2e/live/mcp-bridge-hermes-http.ts +++ b/test/e2e/live/mcp-bridge-hermes-http.ts @@ -9,8 +9,6 @@ export const HERMES_MCP_RESULT_TOKEN_MARKER = "NEMOCLAW_HERMES_MCP_RESULT_TOKEN= export const HERMES_MCP_FAILURE_CAPTURE_BYTES = 4_096; export const HERMES_MCP_FAILURE_PREVIEW_CHARS = 1_024; export const HERMES_MCP_RESPONSE_FILE_BYTES = 65_536; -export const HERMES_MCP_OVERSIZE_BODY_MARKER = - ""; interface HermesMcpCommandResult { exitCode: number | null; @@ -19,25 +17,12 @@ interface HermesMcpCommandResult { stderr: string; } -interface McpRequestObservation { - rpcMethod?: string; - auth?: string; - path?: string; -} - const FAILURE_BODY_EMITTER = [ "import os, pathlib, sys", - "path = pathlib.Path(sys.argv[1])", - "limit = int(sys.argv[2])", - `oversize = ${JSON.stringify(HERMES_MCP_OVERSIZE_BODY_MARKER)}.encode('utf-8')`, + "raw = pathlib.Path(sys.argv[1]).read_bytes()", "secret = os.environ.get('API_SERVER_KEY', '').encode('utf-8')", - "with path.open('rb') as stream:", - " raw = stream.read(limit + 1)", - "if len(raw) > limit or len(secret) > limit:", - " sys.stdout.buffer.write(oversize)", - "else:", - " replacement = b'[REDACTED]' if len(secret) >= 10 else b'*' * len(secret)", - " sys.stdout.buffer.write(raw.replace(secret, replacement) if secret else raw)", + "replacement = b'[REDACTED]' if len(secret) >= 10 else b'*' * len(secret)", + "sys.stdout.buffer.write(raw.replace(secret, replacement) if secret else raw)", ].join("\n"); export function buildHermesMcpChatProbeScript(payload: string, resultToken: string): string { @@ -48,7 +33,7 @@ export function buildHermesMcpChatProbeScript(payload: string, resultToken: stri "trap 'rm -f \"$response_file\"' EXIT", `set -- -sS --max-time 180 --max-filesize ${HERMES_MCP_RESPONSE_FILE_BYTES} -o "$response_file" -w '%{http_code}' http://localhost:8642/v1/chat/completions -H 'Content-Type: application/json'`, 'if [ -n "${API_SERVER_KEY:-}" ]; then set -- "$@" -H "Authorization: Bearer ${API_SERVER_KEY}"; fi', - `emit_failure_body() { /usr/bin/python3 -I -S -c ${shellQuote(FAILURE_BODY_EMITTER)} "$response_file" ${HERMES_MCP_RESPONSE_FILE_BYTES}; }`, + `emit_failure_body() { /usr/bin/python3 -I -S -c ${shellQuote(FAILURE_BODY_EMITTER)} "$response_file"; }`, "set +e", `status="$(curl "$@" --data-binary ${shellQuote(payload)})"`, "curl_rc=$?", @@ -115,28 +100,3 @@ export function assertHermesMcpHttpResponse( throw new Error("Hermes real MCP tool call success path emitted response contents"); } } - -export function assertAuthenticatedMcpToolCallOutcome(options: { - requests: readonly McpRequestObservation[]; - callsBefore: number; - expectedSecret: string; -}): void { - const calls = options.requests.filter((request) => request.rpcMethod === "tools/call"); - if (calls.length !== options.callsBefore + 1) { - throw new Error( - `Hermes real MCP tool call must issue exactly one tools/call request (observed delta=${calls.length - options.callsBefore})`, - ); - } - const call = calls.at(-1); - if (call?.path !== "/mcp") { - throw new Error("Hermes real MCP tool call did not reach the managed /mcp endpoint"); - } - if (call.auth !== `Bearer ${options.expectedSecret}`) { - throw new Error( - "Hermes real MCP tool call did not use the expected resolved bearer credential", - ); - } - if (call.auth.includes("openshell:resolve:env")) { - throw new Error("Hermes real MCP tool call forwarded an unresolved credential placeholder"); - } -} diff --git a/test/e2e/live/mcp-bridge.test.ts b/test/e2e/live/mcp-bridge.test.ts index eab7d1daf1d..26466b0f737 100644 --- a/test/e2e/live/mcp-bridge.test.ts +++ b/test/e2e/live/mcp-bridge.test.ts @@ -27,7 +27,6 @@ import { removeMcpBridgeWithOneConcurrencyRetry, } from "./mcp-bridge-cleanup.ts"; import { - assertAuthenticatedMcpToolCallOutcome, assertHermesMcpHttpResponse, buildHermesMcpChatProbeScript, HERMES_MCP_FAILURE_CAPTURE_BYTES, @@ -571,23 +570,19 @@ async function assertRealAdapterToolCall( { artifactName: options.artifactName, env: buildAvailabilityProbeEnv(), - postRedactionCaptureLimitBytes: - options.agent === "hermes" ? HERMES_MCP_FAILURE_CAPTURE_BYTES : undefined, + captureLimitBytes: options.agent === "hermes" ? HERMES_MCP_FAILURE_CAPTURE_BYTES : undefined, redactionValues: options.agent === "hermes" ? hermesRedactionValues : [], timeoutMs: 5 * 60_000, }, ); - if (options.agent === "hermes") { - assertHermesMcpHttpResponse(result, hermesRedactionValues); - assertAuthenticatedMcpToolCallOutcome({ - requests: fakeMcp.requests, - callsBefore: before, - expectedSecret: options.expectedSecret ?? HOST_SECRET, - }); - return; - } - expectExitZero(result, `${options.agent} real MCP tool call`); - expect(resultText(result)).toContain(options.resultToken); + const assertResponse = + options.agent === "hermes" + ? () => assertHermesMcpHttpResponse(result, hermesRedactionValues) + : () => { + expectExitZero(result, `${options.agent} real MCP tool call`); + expect(resultText(result)).toContain(options.resultToken); + }; + assertResponse(); const calls = fakeMcp.requests.filter((request) => request.rpcMethod === "tools/call"); expect(calls).toHaveLength(before + 1); expect(calls.at(-1)).toMatchObject({ diff --git a/test/e2e/support/e2e-redaction-entry.test.ts b/test/e2e/support/e2e-redaction-entry.test.ts index b2b4e65dfb4..71de84a2771 100644 --- a/test/e2e/support/e2e-redaction-entry.test.ts +++ b/test/e2e/support/e2e-redaction-entry.test.ts @@ -435,47 +435,4 @@ describe("fixture redaction entry point", () => { await fs.rm(rootDir, { recursive: true, force: true }); } }); - - it.each([ - 0, - -1, - 1.5, - Number.POSITIVE_INFINITY, - Number.MAX_SAFE_INTEGER + 1, - ])("rejects invalid post-redaction capture limit %s before spawning a child or writing artifacts (#8697)", async (postRedactionCaptureLimitBytes) => { - const rootDir = await fs.mkdtemp( - path.join(os.tmpdir(), "nemoclaw-e2e-invalid-post-redaction-capture-"), - ); - try { - const artifactRoot = path.join(rootDir, "e2e-artifacts/live/invalid-post-redaction-capture"); - const spawnMarker = path.join(rootDir, "spawned.txt"); - const artifacts = new ArtifactSink(artifactRoot); - await artifacts.ensureRoot(); - const probe = new ShellProbe({ - artifacts, - progress: supportProgress(), - redact: (text, extra) => redactString(text, extra), - signal: new AbortController().signal, - }); - - await expect( - probe.run( - trustedShellCommand({ - command: "bash", - args: ["-lc", 'printf spawned >"$SPAWN_MARKER"'], - reason: - "verify that ShellProbe rejects invalid post-redaction limits before child execution", - }), - { - env: { SPAWN_MARKER: spawnMarker }, - postRedactionCaptureLimitBytes, - }, - ), - ).rejects.toThrow("postRedactionCaptureLimitBytes must be a positive safe integer"); - await expect(fs.access(spawnMarker)).rejects.toThrow(); - await expect(fs.readdir(artifactRoot)).resolves.toEqual([]); - } finally { - await fs.rm(rootDir, { recursive: true, force: true }); - } - }); }); diff --git a/test/e2e/support/mcp-bridge-hermes-http.test.ts b/test/e2e/support/mcp-bridge-hermes-http.test.ts index 683e5e12654..7814b71725c 100644 --- a/test/e2e/support/mcp-bridge-hermes-http.test.ts +++ b/test/e2e/support/mcp-bridge-hermes-http.test.ts @@ -8,453 +8,115 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { ArtifactSink } from "../fixtures/artifacts.ts"; -import { startTestProgress } from "../fixtures/progress.ts"; -import { redactString } from "../fixtures/redaction.ts"; -import { ShellProbe, trustedShellCommand } from "../fixtures/shell-probe.ts"; import { - assertAuthenticatedMcpToolCallOutcome, assertHermesMcpHttpResponse, buildHermesMcpChatProbeScript, - HERMES_MCP_FAILURE_CAPTURE_BYTES, HERMES_MCP_FAILURE_PREVIEW_CHARS, HERMES_MCP_HTTP_STATUS_MARKER, - HERMES_MCP_OVERSIZE_BODY_MARKER, - HERMES_MCP_RESPONSE_FILE_BYTES, HERMES_MCP_RESULT_TOKEN_MARKER, } from "../live/mcp-bridge-hermes-http.ts"; -const SPAWN_TIMEOUT_MS = 5_000; +const TIMEOUT_MS = 5_000; const SYSTEM_PATH = "/usr/bin:/bin"; -function deterministicEnv(overrides: NodeJS.ProcessEnv): NodeJS.ProcessEnv { - return { - PATH: overrides.PATH ?? SYSTEM_PATH, - ...overrides, - }; -} - -function httpResult(status: number, body: string, resultTokenState?: "present" | "missing") { +function httpResult(status: number, body = "", result = "") { return { exitCode: 0, signal: null, stdout: body, - stderr: - `${HERMES_MCP_HTTP_STATUS_MARKER}${status}\n` + - (resultTokenState ? `${HERMES_MCP_RESULT_TOKEN_MARKER}${resultTokenState}\n` : ""), + stderr: `${HERMES_MCP_HTTP_STATUS_MARKER}${status}\n${result}`, }; } describe("Hermes MCP HTTP failure diagnostics", () => { - it("sends one authenticated request per chat probe execution without retrying (#8697)", () => { - const resultToken = "fixture-result-token"; - const script = buildHermesMcpChatProbeScript('{"messages":[]}', resultToken); - const syntax = spawnSync("sh", ["-n"], { - encoding: "utf8", - input: script, - killSignal: "SIGKILL", - timeout: SPAWN_TIMEOUT_MS, - }); - - expect(syntax.status, syntax.stderr).toBe(0); + it("sends one authenticated request without retrying and redacts its API key from failure output (#8697)", () => { + const token = "fixture-result-token"; + const script = buildHermesMcpChatProbeScript('{"messages":[]}', token); expect(script.match(/\bcurl "\$@"/gu)).toHaveLength(1); + expect(script).not.toMatch(/\bretry\b/iu); expect(script).toContain("Authorization: Bearer ${API_SERVER_KEY}"); - expect(script).toContain(`--max-filesize ${HERMES_MCP_RESPONSE_FILE_BYTES}`); - expect(script).toContain("/usr/bin/python3 -I -S -c"); - expect(script).not.toMatch(/\bretry\b|\bfor\b|\bwhile\b/iu); - expect(script).not.toContain('cat "$response_file"'); const directory = mkdtempSync(path.join(tmpdir(), "nemoclaw-hermes-mcp-http-")); + const bodyFile = path.join(directory, "body"); + const countFile = path.join(directory, "count"); const curl = path.join(directory, "curl"); - const count = path.join(directory, "count"); writeFileSync( curl, [ "#!/bin/sh", "set -eu", + 'all_args="$*"', "output=", - "authenticated=0", 'while [ "$#" -gt 0 ]; do', ' if [ "$1" = "-o" ]; then output="$2"; shift 2; continue; fi', - ' if [ "$1" = "-H" ] && [ "$2" = "Authorization: Bearer ${FAKE_EXPECTED_API_KEY}" ]; then authenticated=1; fi', " shift", "done", - '[ "$authenticated" -eq 1 ]', - 'printf "1\\n" >> "$FAKE_CURL_COUNT"', - 'if [ -n "${FAKE_CURL_BODY_FILE:-}" ]; then cp "$FAKE_CURL_BODY_FILE" "$output"; else printf "%s" "$FAKE_CURL_BODY" > "$output"; fi', - 'printf "%s" "${FAKE_CURL_STATUS:-200}"', + 'case "$all_args" in *"Authorization: Bearer $FAKE_API_KEY"*) ;; *) exit 67 ;; esac', + 'cp "$FAKE_BODY_FILE" "$output"', + 'printf "1\\n" >> "$FAKE_COUNT_FILE"', + 'printf "%s" "$FAKE_STATUS"', ].join("\n"), - "utf8", ); chmodSync(curl, 0o755); - try { - const executed = spawnSync("sh", ["-c", script], { - encoding: "utf8", - env: deterministicEnv({ - API_SERVER_KEY: "fixture-api-key", - FAKE_CURL_BODY: `private response contents ${resultToken}`, - FAKE_CURL_BODY_FILE: "", - FAKE_CURL_COUNT: count, - FAKE_CURL_STATUS: "200", - FAKE_EXPECTED_API_KEY: "fixture-api-key", - PATH: `${directory}:${SYSTEM_PATH}`, - }), - killSignal: "SIGKILL", - timeout: SPAWN_TIMEOUT_MS, - }); - expect(executed.status, executed.stderr).toBe(0); - expect(readFileSync(count, "utf8")).toBe("1\n"); - expect(executed.stdout).toBe(""); - expect(executed.stderr).toContain(`${HERMES_MCP_HTTP_STATUS_MARKER}200`); - expect(executed.stderr).toContain(`${HERMES_MCP_RESULT_TOKEN_MARKER}present`); - expect(executed.stderr).not.toContain("private response contents"); - - writeFileSync( - path.join(directory, "sitecustomize.py"), - [ - "import os, sys", - "secret = os.environ.get('API_SERVER_KEY', '')", - "sys.stdout.write(secret)", - "sys.stderr.write(secret)", - ].join("\n"), - "utf8", - ); - const hostileKey = "fixture-api-key"; - const failed = spawnSync("sh", ["-c", script], { - encoding: "utf8", - env: deterministicEnv({ - API_SERVER_KEY: hostileKey, - FAKE_CURL_BODY: "failure body without credential", - FAKE_CURL_BODY_FILE: "", - FAKE_CURL_COUNT: count, - FAKE_CURL_STATUS: "500", - FAKE_EXPECTED_API_KEY: hostileKey, - PATH: `${directory}:${SYSTEM_PATH}`, - PYTHONPATH: directory, - PYTHONWARNINGS: `${hostileKey}::Warning`, - }), - killSignal: "SIGKILL", - timeout: SPAWN_TIMEOUT_MS, - }); - expect(failed.status, failed.stderr).toBe(0); - expect(readFileSync(count, "utf8")).toBe("1\n1\n"); - expect(failed.stdout).toBe("failure body without credential"); - expect(failed.stdout).not.toContain(hostileKey); - expect(failed.stderr).toContain(`${HERMES_MCP_HTTP_STATUS_MARKER}500`); - expect(failed.stderr).not.toContain(HERMES_MCP_RESULT_TOKEN_MARKER); - expect(failed.stderr).not.toContain(hostileKey); - const unknownKey = "synthetic-credential-segment-".repeat(4); - const boundaryStart = - HERMES_MCP_FAILURE_CAPTURE_BYTES - Math.floor(Buffer.byteLength(unknownKey) / 2); - const repeatedBody = `${unknownKey}${"x".repeat( - boundaryStart - Buffer.byteLength(unknownKey), - )}${unknownKey}tail`; - const repeated = spawnSync("sh", ["-c", script], { + const apiKey = "fixture-api-key-value"; + const run = (body: string, status: string) => { + writeFileSync(bodyFile, body); + return spawnSync("sh", ["-c", script], { encoding: "utf8", - env: deterministicEnv({ - API_SERVER_KEY: unknownKey, - FAKE_CURL_BODY: repeatedBody, - FAKE_CURL_BODY_FILE: "", - FAKE_CURL_COUNT: count, - FAKE_CURL_STATUS: "500", - FAKE_EXPECTED_API_KEY: unknownKey, + env: { + API_SERVER_KEY: apiKey, + FAKE_API_KEY: apiKey, + FAKE_BODY_FILE: bodyFile, + FAKE_COUNT_FILE: countFile, + FAKE_STATUS: status, PATH: `${directory}:${SYSTEM_PATH}`, - }), + }, killSignal: "SIGKILL", - timeout: SPAWN_TIMEOUT_MS, + timeout: TIMEOUT_MS, }); - expect(repeated.status, repeated.stderr).toBe(0); - expect(readFileSync(count, "utf8")).toBe("1\n1\n1\n"); - expect(repeated.stdout.match(/\[REDACTED\]/gu)).toHaveLength(2); - expect(repeated.stdout).not.toContain(unknownKey); - expect(repeated.stdout).not.toContain(unknownKey.slice(0, 32)); - expect(repeated.stdout).not.toContain(unknownKey.slice(-32)); - } finally { - rmSync(directory, { force: true, recursive: true }); - } - }); + }; - it("redacts sensitive values before limiting text and binary response evidence (#8697)", async () => { - const resultToken = "fixture-result-token"; - const script = buildHermesMcpChatProbeScript('{"messages":[]}', resultToken); - const directory = mkdtempSync(path.join(tmpdir(), "nemoclaw-hermes-mcp-bytes-")); - const curl = path.join(directory, "curl"); - const bodyFile = path.join(directory, "body.bin"); - const apiKey = `boundary-api-key-${"k".repeat(64)}`; - const explicitValue = `private-prompt-${"p".repeat(64)}`; - const oversizeSecret = `oversize-private-${"s".repeat(80)}`; - writeFileSync( - curl, - [ - "#!/bin/sh", - "set -eu", - "max_size=", - "output=", - 'while [ "$#" -gt 0 ]; do', - ' if [ "$1" = "--max-filesize" ]; then max_size="$2"; shift 2; continue; fi', - ' if [ "$1" = "-o" ]; then output="$2"; shift 2; continue; fi', - " shift", - "done", - '[ -n "$max_size" ]', - 'if [ -n "${FAKE_CURL_IGNORE_MAX_SIZE:-}" ]; then', - ' cp "$FAKE_CURL_BODY_FILE" "$output"', - "else", - ' body_size="$(wc -c < "$FAKE_CURL_BODY_FILE")"', - ' if [ "$body_size" -gt "$max_size" ]; then', - ' dd if="$FAKE_CURL_BODY_FILE" of="$output" bs=1 count="$max_size" 2>/dev/null', - ' if [ -n "${FAKE_CURL_RESPONSE_SIZE_FILE:-}" ]; then wc -c < "$output" > "$FAKE_CURL_RESPONSE_SIZE_FILE"; fi', - ' printf "%s" "$FAKE_CURL_STATUS"', - " exit 63", - " fi", - ' cp "$FAKE_CURL_BODY_FILE" "$output"', - "fi", - 'if [ -n "${FAKE_CURL_RESPONSE_SIZE_FILE:-}" ]; then wc -c < "$output" > "$FAKE_CURL_RESPONSE_SIZE_FILE"; fi', - 'printf "%s" "$FAKE_CURL_STATUS"', - ].join("\n"), - "utf8", - ); - chmodSync(curl, 0o755); - const artifacts = new ArtifactSink(path.join(directory, "artifacts")); - const progress = startTestProgress( - "Hermes MCP HTTP diagnostic support", - ["run probe", "verify result"], - { logLine: () => undefined }, - ); try { - await artifacts.ensureRoot(); - const probe = new ShellProbe({ - artifacts, - progress, - redact: (text, values) => redactString(text, values), - signal: new AbortController().signal, - }); - const runBody = async ( - body: Buffer, - artifactName: string, - envOverrides: NodeJS.ProcessEnv = {}, - ) => { - writeFileSync(bodyFile, body); - return probe.run( - trustedShellCommand({ - command: "sh", - args: ["-c", script], - reason: - "verify that ShellProbe redacts sensitive values before limiting response evidence", - }), - { - artifactName, - env: deterministicEnv({ - API_SERVER_KEY: apiKey, - FAKE_CURL_BODY: "", - FAKE_CURL_BODY_FILE: bodyFile, - FAKE_CURL_STATUS: "500", - PATH: `${directory}:${SYSTEM_PATH}`, - ...envOverrides, - }), - postRedactionCaptureLimitBytes: HERMES_MCP_FAILURE_CAPTURE_BYTES, - redactionValues: [apiKey, explicitValue, oversizeSecret], - timeoutMs: SPAWN_TIMEOUT_MS, - }, - ); - }; - - const secondKeyStart = - HERMES_MCP_FAILURE_CAPTURE_BYTES + Math.floor(Buffer.byteLength(apiKey) / 2); - const repeatedKeyBody = Buffer.from( - `${apiKey}${"x".repeat(secondKeyStart - Buffer.byteLength(apiKey))}${apiKey}tail`, - "utf8", - ); - const repeated = await runBody(repeatedKeyBody, "repeated-key-boundary"); - expect(Buffer.byteLength(repeated.stdout, "utf8")).toBeLessThanOrEqual( - HERMES_MCP_FAILURE_CAPTURE_BYTES, - ); - expect(repeated.stdout.match(/\[REDACTED\]/gu)).toHaveLength(2); - expect(repeated.stdout).not.toContain(apiKey.slice(0, 32)); - expect(repeated.stdout).not.toContain(apiKey.slice(-32)); - const repeatedArtifact = readFileSync( - artifacts.pathFor("shell/repeated-key-boundary.stdout.txt"), - ); - expect(repeatedArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); - expect(repeatedArtifact.includes(Buffer.from(apiKey.slice(0, 32), "utf8"))).toBe(false); - - const explicitStart = - HERMES_MCP_FAILURE_CAPTURE_BYTES - Math.floor(Buffer.byteLength(explicitValue) / 2); - const explicitBody = Buffer.from(`${"y".repeat(explicitStart)}${explicitValue}tail`, "utf8"); - const explicit = await runBody(explicitBody, "explicit-value-boundary"); - expect(Buffer.byteLength(explicit.stdout, "utf8")).toBeLessThanOrEqual( - HERMES_MCP_FAILURE_CAPTURE_BYTES, - ); - expect(explicit.stdout).toContain("[REDACTED]"); - expect(explicit.stdout).not.toContain(explicitValue.slice(0, 32)); - expect(explicit.stdout).not.toContain(explicitValue.slice(-32)); - const explicitArtifact = readFileSync( - artifacts.pathFor("shell/explicit-value-boundary.stdout.txt"), - ); - expect(explicitArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); - expect(explicitArtifact.includes(Buffer.from(explicitValue.slice(0, 32), "utf8"))).toBe( - false, - ); - - const binaryBody = Buffer.concat([ - Buffer.alloc(1_000, 0xff), - Buffer.from("🙂".repeat(1_000), "utf8"), - Buffer.alloc(1_000, 0x7a), - ]); - const binary = await runBody(binaryBody, "invalid-utf8-multibyte-boundary"); - expect(Buffer.byteLength(binary.stdout, "utf8")).toBeLessThanOrEqual( - HERMES_MCP_FAILURE_CAPTURE_BYTES, - ); - expect(binary.stdout.startsWith("?".repeat(1_000))).toBe(true); - expect(binary.stdout).toContain("🙂"); - expect(binary.stdout).not.toContain("�"); - expect(binary.stderr).not.toContain(apiKey); - expect( - readFileSync(artifacts.pathFor("shell/invalid-utf8-multibyte-boundary.stdout.txt")).length, - ).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); - - const oversizeSecretStart = - HERMES_MCP_RESPONSE_FILE_BYTES - Math.floor(Buffer.byteLength(oversizeSecret) / 2); - const oversizeBody = Buffer.from( - `${"q".repeat(oversizeSecretStart)}${oversizeSecret}${"r".repeat(1_024)}`, - "utf8", - ); - const responseSizeFile = path.join(directory, "oversize-response-size.txt"); - const curlLimited = await runBody(oversizeBody, "curl-limited-oversize-body", { - FAKE_CURL_RESPONSE_SIZE_FILE: responseSizeFile, - }); - expect(curlLimited.exitCode).toBe(63); - expect(Number(readFileSync(responseSizeFile, "utf8").trim())).toBe( - HERMES_MCP_RESPONSE_FILE_BYTES, - ); - expect(curlLimited.stdout).toBe(""); - expect(curlLimited.stdout).not.toContain(oversizeSecret.slice(0, 32)); - expect(curlLimited.stderr).not.toContain(oversizeSecret.slice(0, 32)); - const curlLimitedArtifact = readFileSync( - artifacts.pathFor("shell/curl-limited-oversize-body.stdout.txt"), - ); - expect(curlLimitedArtifact.length).toBe(0); - expect(curlLimitedArtifact.includes(Buffer.from(oversizeSecret.slice(0, 32), "utf8"))).toBe( - false, - ); + const failed = run(`failed with ${apiKey}`, "500"); + expect(failed.status, failed.stderr).toBe(0); + expect(failed.stdout).toContain("[REDACTED]"); + expect(failed.stdout).not.toContain(apiKey); + expect(failed.stderr).toContain(`${HERMES_MCP_HTTP_STATUS_MARKER}500`); - const emitterLimited = await runBody(oversizeBody, "emitter-limited-oversize-body", { - FAKE_CURL_IGNORE_MAX_SIZE: "1", - }); - expect(emitterLimited.exitCode).toBe(0); - expect(emitterLimited.stdout).toBe(HERMES_MCP_OVERSIZE_BODY_MARKER); - expect(emitterLimited.stdout).not.toContain(oversizeSecret.slice(0, 32)); - const emitterLimitedArtifact = readFileSync( - artifacts.pathFor("shell/emitter-limited-oversize-body.stdout.txt"), - ); - expect(emitterLimitedArtifact.length).toBeLessThanOrEqual(HERMES_MCP_FAILURE_CAPTURE_BYTES); - expect(emitterLimitedArtifact.toString("utf8")).toBe(HERMES_MCP_OVERSIZE_BODY_MARKER); + expect(readFileSync(countFile, "utf8")).toBe("1\n"); } finally { - progress.stop(); rmSync(directory, { force: true, recursive: true }); } }); - it("rejects HTTP 500 with bounded, redacted response evidence (#8697)", () => { - const dynamicSecret = "fixture-dynamic-secret-value"; - const requestPayload = '{"messages":[{"content":"private diagnostic prompt"}]}'; - const longTail = "x".repeat(HERMES_MCP_FAILURE_PREVIEW_CHARS * 2); - const body = `${requestPayload}\nAuthorization: Bearer api-server-secret-value\n${dynamicSecret}\n${longTail}`; - - expect(() => - assertHermesMcpHttpResponse(httpResult(500, body), [dynamicSecret, requestPayload]), - ).toThrowError(/HTTP 500; redacted response body:/u); - + it("rejects duplicate status markers, HTTP 500, and missing result tokens (#8697)", () => { + const secret = "fixture-diagnostic-secret"; + const longBody = `${secret}\nAuthorization: Bearer another-secret\n${"x".repeat( + HERMES_MCP_FAILURE_PREVIEW_CHARS * 2, + )}`; try { - assertHermesMcpHttpResponse(httpResult(500, body), [dynamicSecret, requestPayload]); - throw new Error("expected HTTP 500 to fail"); + assertHermesMcpHttpResponse(httpResult(500, longBody), [secret]); + throw new Error("expected the HTTP 500 response assertion to throw"); } catch (error) { const message = error instanceof Error ? error.message : String(error); - expect(message).not.toContain(dynamicSecret); - expect(message).not.toContain(requestPayload); - expect(message).not.toContain("api-server-secret-value"); + expect(message).toContain("HTTP 500"); expect(message).toContain("[REDACTED]"); expect(message).toContain("[truncated]"); - expect([...message.split("redacted response body: ")[1]!].length).toBeLessThanOrEqual( - HERMES_MCP_FAILURE_PREVIEW_CHARS, - ); + expect(message).not.toContain(secret); } - }); - - it("requires a result token and exactly one authenticated tools/call after HTTP 2xx (#8697)", () => { - const request = { - rpcMethod: "tools/call", - auth: "Bearer rotated-fixture-secret", - path: "/mcp", - }; - expect(() => assertHermesMcpHttpResponse(httpResult(200, "", "present"), [])).not.toThrow(); - expect(() => assertHermesMcpHttpResponse(httpResult(200, "", "missing"), [])).toThrowError( + expect(() => assertHermesMcpHttpResponse(httpResult(200), [])).toThrowError( /fixture result token/u, ); expect(() => - assertHermesMcpHttpResponse(httpResult(200, "raw response", "present"), []), - ).toThrowError(/success path emitted response contents/u); - expect(() => - assertAuthenticatedMcpToolCallOutcome({ - requests: [request], - callsBefore: 0, - expectedSecret: "rotated-fixture-secret", - }), - ).not.toThrow(); - expect(() => - assertAuthenticatedMcpToolCallOutcome({ - requests: [request, request], - callsBefore: 0, - expectedSecret: "rotated-fixture-secret", - }), - ).toThrowError(/exactly one tools\/call/u); - expect(() => - assertAuthenticatedMcpToolCallOutcome({ - requests: [{ ...request, path: "/other" }], - callsBefore: 0, - expectedSecret: "rotated-fixture-secret", - }), - ).toThrowError(/managed \/mcp endpoint/u); - expect(() => - assertAuthenticatedMcpToolCallOutcome({ - requests: [request], - callsBefore: 0, - expectedSecret: "unexpected-secret", - }), - ).toThrowError(/expected resolved bearer credential/u); - }); - - it("redacts transport evidence and rejects missing or duplicate HTTP status markers (#8697)", () => { - const secret = "fixture-transport-secret"; - for (const result of [ - { exitCode: 7, signal: null, stdout: secret, stderr: "transport detail" }, - { exitCode: null, signal: "SIGKILL" as const, stdout: secret, stderr: "" }, - ]) { - try { - assertHermesMcpHttpResponse(result, [secret]); - throw new Error("expected transport failure"); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - expect(message).toMatch(/transport failed \((?:exit=7|signal=SIGKILL)\)/u); - expect(message).toContain("[REDACTED]"); - expect(message).not.toContain(secret); - } - } - - expect(() => - assertHermesMcpHttpResponse({ exitCode: 0, signal: null, stdout: "", stderr: "" }, []), + assertHermesMcpHttpResponse(httpResult(200, "", `${HERMES_MCP_HTTP_STATUS_MARKER}500\n`), []), ).toThrowError(/exactly one HTTP status marker/u); expect(() => assertHermesMcpHttpResponse( - { - exitCode: 0, - signal: null, - stdout: "", - stderr: `${HERMES_MCP_HTTP_STATUS_MARKER}200\n${HERMES_MCP_HTTP_STATUS_MARKER}500\n`, - }, + httpResult(200, "", `${HERMES_MCP_RESULT_TOKEN_MARKER}present\n`), [], ), - ).toThrowError(/exactly one HTTP status marker/u); + ).not.toThrow(); }); });