From 04d32bca5116b9d0465d3be905f34955ecda9721 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Sat, 11 Jul 2026 09:08:38 +0400 Subject: [PATCH 1/5] feat(audit): secure audit file permissions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/audit/audit-logger.ts | 7 +++++-- tests/audit.test.ts | 21 ++++++++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/audit/audit-logger.ts b/src/audit/audit-logger.ts index d947daae..b2a0498e 100644 --- a/src/audit/audit-logger.ts +++ b/src/audit/audit-logger.ts @@ -17,13 +17,16 @@ export class AuditLogger { } async log(event: AuditEvent): Promise { - await mkdir(dirname(this.path), { recursive: true }); + await mkdir(dirname(this.path), { recursive: true, mode: 0o700 }); const safeEvent = redactSecrets( !this.options.includeArguments ? { ...event, arguments: undefined } : event, this.options.secretValues ?? [] ); - await appendFile(this.path, `${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`, "utf8"); + await appendFile(this.path, `${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`, { + encoding: "utf8", + mode: 0o600 + }); } } diff --git a/tests/audit.test.ts b/tests/audit.test.ts index 667e3898..04b71c61 100644 --- a/tests/audit.test.ts +++ b/tests/audit.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile } from "node:fs/promises"; +import { mkdtemp, readFile, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -29,4 +29,23 @@ describe("audit logger", () => { status: "success" }); }); + + it.skipIf(process.platform === "win32")("creates audit directories and files with owner-only permissions", async () => { + const root = await mkdtemp(join(tmpdir(), "miftah-audit-permissions-")); + const directory = join(root, "private"); + const path = join(directory, "audit.jsonl"); + const logger = new AuditLogger(path); + + await logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4 + }); + + expect((await stat(directory)).mode & 0o077).toBe(0); + expect((await stat(path)).mode & 0o077).toBe(0); + }); }); From 3bf6a9488d36d3eb924acca2fc8e293c722fdf44 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Sat, 11 Jul 2026 09:12:56 +0400 Subject: [PATCH 2/5] feat(audit): add shared redaction and resilient writes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/audit/audit-logger.ts | 97 +++++++++++++++---- src/audit/audit-types.ts | 13 +++ src/cli/create-runtime.ts | 6 +- src/secrets/redact.ts | 75 +++++++++++++- .../multi-upstream-process-manager.ts | 18 +++- src/upstream/upstream-process-manager.ts | 27 ++++-- src/utils/errors.ts | 1 + tests/audit.test.ts | 68 ++++++++++++- tests/fixtures/fake-upstream.mjs | 9 +- tests/secrets.test.ts | 18 +++- tests/upstream-manager.test.ts | 38 ++++++++ 11 files changed, 338 insertions(+), 32 deletions(-) diff --git a/src/audit/audit-logger.ts b/src/audit/audit-logger.ts index b2a0498e..427ba57a 100644 --- a/src/audit/audit-logger.ts +++ b/src/audit/audit-logger.ts @@ -1,32 +1,95 @@ -import { appendFile, mkdir } from "node:fs/promises"; +import { chmod, mkdir, open } from "node:fs/promises"; import { dirname } from "node:path"; -import type { AuditEvent } from "./audit-types.js"; -import { redactSecrets } from "../secrets/redact.js"; +import type { AuditEvent, AuditFailureMode, AuditHealth } from "./audit-types.js"; +import { SecretRedactor } from "../secrets/redact.js"; +import { MiftahError } from "../utils/errors.js"; + +export interface AuditLoggerOptions { + secretValues?: readonly string[]; + redactor?: SecretRedactor; + includeArguments?: boolean; + failureMode?: AuditFailureMode; +} export class AuditLogger { - private readonly options: { secretValues: readonly string[]; includeArguments: boolean }; + private static readonly writesByPath = new Map>(); + private readonly options: { includeArguments: boolean; failureMode: AuditFailureMode }; + private readonly redactor: SecretRedactor; + private lastFailure?: AuditHealth["lastFailure"]; - constructor( - private readonly path: string, - options: { secretValues?: readonly string[]; includeArguments?: boolean } = {} - ) { + constructor(private readonly path: string, options: AuditLoggerOptions = {}) { this.options = { - secretValues: options.secretValues ?? [], - includeArguments: options.includeArguments ?? false + includeArguments: options.includeArguments ?? false, + failureMode: options.failureMode ?? "fail-closed" }; + this.redactor = options.redactor ?? new SecretRedactor(); + this.redactor.addAll(options.secretValues ?? []); } async log(event: AuditEvent): Promise { - await mkdir(dirname(this.path), { recursive: true, mode: 0o700 }); - const safeEvent = redactSecrets( + const safeEvent = this.redactor.redact( !this.options.includeArguments ? { ...event, arguments: undefined } - : event, - this.options.secretValues ?? [] + : event ); - await appendFile(this.path, `${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`, { - encoding: "utf8", - mode: 0o600 + try { + await this.enqueue(`${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`); + this.lastFailure = undefined; + } catch (error) { + const failure = this.asWriteFailure(error); + this.lastFailure = { + timestamp: new Date().toISOString(), + errorCode: "AUDIT_WRITE_FAILED", + message: failure.message + }; + if (this.options.failureMode === "fail-closed") throw failure; + } + } + + health(): AuditHealth { + return this.lastFailure ? { state: "failed", lastFailure: structuredClone(this.lastFailure) } : { state: "healthy" }; + } + + private enqueue(line: string): Promise { + const prior = AuditLogger.writesByPath.get(this.path) ?? Promise.resolve(); + const write = prior.catch(() => undefined).then(() => this.writeLine(line)); + const tail = write.catch(() => undefined); + AuditLogger.writesByPath.set(this.path, tail); + void tail.then(() => { + if (AuditLogger.writesByPath.get(this.path) === tail) AuditLogger.writesByPath.delete(this.path); }); + return write; + } + + private async writeLine(line: string): Promise { + const directory = dirname(this.path); + await mkdir(directory, { recursive: true, mode: 0o700 }); + await setRestrictiveMode(directory, 0o700); + const file = await open(this.path, "a", 0o600); + try { + await setRestrictiveMode(this.path, 0o600); + await file.writeFile(line, "utf8"); + } finally { + await file.close(); + } + } + + private asWriteFailure(error: unknown): MiftahError { + const message = this.redactor.redactText(error instanceof Error ? error.message : String(error)); + return new MiftahError("AUDIT_WRITE_FAILED", `AUDIT_WRITE_FAILED: unable to write audit record: ${message}`); + } +} + +async function setRestrictiveMode(path: string, mode: number): Promise { + try { + await chmod(path, mode); + } catch (error) { + if ( + !(error instanceof Error) || + !("code" in error) || + (error.code !== "ENOSYS" && error.code !== "ENOTSUP" && error.code !== "EOPNOTSUPP") + ) { + throw error; + } } } diff --git a/src/audit/audit-types.ts b/src/audit/audit-types.ts index 31f5885a..d136ffa7 100644 --- a/src/audit/audit-types.ts +++ b/src/audit/audit-types.ts @@ -1,5 +1,18 @@ import type { PolicyAction, RiskLevel } from "../policy/policy-types.js"; +export type AuditFailureMode = "fail-open" | "fail-closed"; + +export interface AuditWriteFailure { + timestamp: string; + errorCode: "AUDIT_WRITE_FAILED"; + message: string; +} + +export interface AuditHealth { + state: "healthy" | "failed"; + lastFailure?: AuditWriteFailure; +} + export interface AuditEvent { wrapper: string; profile: string; diff --git a/src/cli/create-runtime.ts b/src/cli/create-runtime.ts index ff98c6b5..3cdfa557 100644 --- a/src/cli/create-runtime.ts +++ b/src/cli/create-runtime.ts @@ -1,6 +1,7 @@ import { loadConfig } from "../config/load-config.js"; import { ProfileManager } from "../profiles/profile-manager.js"; import { SecretResolver } from "../secrets/secret-resolver.js"; +import { SecretRedactor } from "../secrets/redact.js"; import { MultiUpstreamProcessManager } from "../upstream/multi-upstream-process-manager.js"; import { UpstreamProcessManager } from "../upstream/upstream-process-manager.js"; @@ -66,10 +67,11 @@ export async function createRuntime(configPath: string) { headers: resolveMap(resolvedConfig.upstream.headers) } : undefined; - const managerOptions = { ...config.process, secretValues: [...secretValues] }; + const redactor = new SecretRedactor([...secretValues]); + const managerOptions = { ...config.process, secretValues: [...secretValues], redactor }; const manager = resolvedConfig.upstreams ? new MultiUpstreamProcessManager(resolvedConfig, managerOptions) : new UpstreamProcessManager(upstream!, profiles, managerOptions); const profileManager = new ProfileManager(resolvedConfig, resolvedConfig.security); - return { config: resolvedConfig, manager, profileManager }; + return { config: resolvedConfig, manager, profileManager, redactor }; } diff --git a/src/secrets/redact.ts b/src/secrets/redact.ts index 9b373715..47244f82 100644 --- a/src/secrets/redact.ts +++ b/src/secrets/redact.ts @@ -71,9 +71,82 @@ function redactValue(value: unknown, secretValues: readonly string[], key?: stri return value; } +/** Shares a mutable set of known secret values across runtime output boundaries. */ +export class SecretRedactor { + private readonly secretValues = new Set(); + + constructor(secretValues: readonly string[] = []) { + this.addAll(secretValues); + } + + add(value: string): void { + if (value.length > 0) this.secretValues.add(value); + } + + addAll(values: readonly string[]): void { + for (const value of values) this.add(value); + } + + values(): string[] { + return [...this.secretValues]; + } + + redact(value: T): T { + return redactValue(value, this.values()) as T; + } + + redactText(value: string): string { + return this.redact(redactUrisInText(value)); + } + + redactUri(uri: string): string { + return this.redact(redactUri(uri)); + } + + createTextStream(): { write(value: string): string; flush(): string } { + let pending = ""; + return { + write: (value) => { + pending += value; + const lastLineBreak = pending.lastIndexOf("\n"); + if (lastLineBreak >= 0) { + const completeLines = pending.slice(0, lastLineBreak + 1); + pending = pending.slice(lastLineBreak + 1); + return this.redactText(completeLines); + } + const retainedLength = Math.max(1_024, ...[...this.secretValues].map((secret) => secret.length)); + if (pending.length <= retainedLength) return ""; + const requestedBoundary = pending.length - retainedLength; + const boundary = this.safeTextBoundary(pending, requestedBoundary); + const completeText = pending.slice(0, boundary); + pending = pending.slice(boundary); + return this.redactText(completeText); + }, + flush: () => { + const completeText = this.redactText(pending); + pending = ""; + return completeText; + } + }; + } + + private safeTextBoundary(value: string, requestedBoundary: number): number { + let boundary = requestedBoundary; + for (const secret of this.secretValues) { + let index = value.indexOf(secret); + while (index >= 0) { + if (index < boundary && index + secret.length > boundary) boundary = index; + index = value.indexOf(secret, index + 1); + } + } + return boundary; + } +} + /** Creates a reusable deep redactor for a fixed collection of secret values. */ export function createRedactor(secretValues: readonly string[] = []): (value: T) => T { - return (value: T) => redactValue(value, secretValues) as T; + const redactor = new SecretRedactor(secretValues); + return (value: T) => redactor.redact(value); } /** Produces a safe public representation of a URI while retaining only its non-sensitive identity. */ diff --git a/src/upstream/multi-upstream-process-manager.ts b/src/upstream/multi-upstream-process-manager.ts index 38fdf92a..bb2dd341 100644 --- a/src/upstream/multi-upstream-process-manager.ts +++ b/src/upstream/multi-upstream-process-manager.ts @@ -9,18 +9,28 @@ import { import { ProfileSessionLimiter } from "./profile-session-limiter.js"; import { UpstreamSession } from "./upstream-session.js"; import { MiftahError } from "../utils/errors.js"; +import { SecretRedactor } from "../secrets/redact.js"; /** Coordinates named upstream managers while sharing profile-capacity accounting across the bundle. */ export class MultiUpstreamProcessManager { private readonly managers: Record; private readonly healthListeners = new Set<(health: UpstreamHealth) => void>(); private readonly limiter: ProfileSessionLimiter; + private readonly redactor: SecretRedactor; constructor(config: MiftahConfig, options: UpstreamManagerOptions = {}) { this.limiter = new ProfileSessionLimiter(options.maxConcurrentProfiles); + this.redactor = options.redactor ?? new SecretRedactor(); + this.redactor.addAll(options.secretValues ?? []); this.managers = Object.fromEntries( Object.entries(config.upstreams ?? {}).map(([name, upstream]) => { - const manager = new UpstreamProcessManager(upstream, scopedProfiles(config.profiles, name), options, name, this.limiter); + const manager = new UpstreamProcessManager( + upstream, + scopedProfiles(config.profiles, name), + { ...options, redactor: this.redactor }, + name, + this.limiter + ); manager.addHealthListener((health) => this.publishHealth(health)); return [name, manager]; }) @@ -75,7 +85,11 @@ export class MultiUpstreamProcessManager { } getSecretValues(): string[] { - return [...new Set(Object.values(this.managers).flatMap((manager) => manager.getSecretValues()))]; + return this.redactor.values(); + } + + getRedactor(): SecretRedactor { + return this.redactor; } async close(): Promise { diff --git a/src/upstream/upstream-process-manager.ts b/src/upstream/upstream-process-manager.ts index 8ea12b06..ce691336 100644 --- a/src/upstream/upstream-process-manager.ts +++ b/src/upstream/upstream-process-manager.ts @@ -7,7 +7,7 @@ import type { Tool } from "@modelcontextprotocol/sdk/types.js"; import type { Stream } from "node:stream"; import type { ProfileConfig, UpstreamConfig } from "../config/types.js"; import { expandEnvironmentReferencesWithSecretValues } from "../config/env-expand.js"; -import { redactSecrets } from "../secrets/redact.js"; +import { SecretRedactor } from "../secrets/redact.js"; import { MiftahError } from "../utils/errors.js"; import { ProfileSessionLimiter } from "./profile-session-limiter.js"; import { UpstreamSession } from "./upstream-session.js"; @@ -30,6 +30,7 @@ export interface UpstreamManagerOptions { maxRestarts?: number; maxConcurrentProfiles?: number; secretValues?: readonly string[]; + redactor?: SecretRedactor; onStderr?: (profile: string, message: string) => void; } @@ -110,7 +111,7 @@ export class UpstreamProcessManager { private readonly stabilityTimers = new Map(); private readonly generations = new Map(); private readonly startEpochs = new Map(); - private readonly secretValuesSet = new Set(); + private readonly redactor: SecretRedactor; private readonly automaticRestartCounts = new Map(); private readonly consecutiveRestartAttempts = new Map(); private readonly restartExhausted = new Set(); @@ -135,6 +136,8 @@ export class UpstreamProcessManager { restartOnCrash: options.restartOnCrash ?? false, maxRestarts: options.maxRestarts ?? defaultMaxRestarts }; + this.redactor = options.redactor ?? new SecretRedactor(); + this.redactor.addAll(options.secretValues ?? []); this.limiter = limiter ?? new ProfileSessionLimiter(options.maxConcurrentProfiles); } @@ -161,7 +164,11 @@ export class UpstreamProcessManager { /** Returns every configured or dynamically resolved value that must be redacted from upstream output. */ getSecretValues(): string[] { - return [...new Set([...this.secretValuesSet, ...(this.options.secretValues ?? [])])]; + return this.redactor.values(); + } + + getRedactor(): SecretRedactor { + return this.redactor; } addHealthListener(listener: (health: UpstreamHealth) => void): () => void { @@ -404,11 +411,11 @@ export class UpstreamProcessManager { ...(upstreamHeaders?.secretValues ?? []), ...(profileHeaders?.secretValues ?? []) ]) { - this.secretValuesSet.add(value); + this.redactor.add(value); } for (const [key, value] of Object.entries({ ...environment, ...headers })) { if (credentialKeyPattern.test(key) && value.length > 0) { - this.secretValuesSet.add(value); + this.redactor.add(value); } } return { environment, headers }; @@ -416,14 +423,20 @@ export class UpstreamProcessManager { /** Emits process stderr only after applying static and dynamically resolved secret redaction. */ private attachStderr(profile: string, stderr: Stream | null): void { + const streamRedactor = this.redactor.createTextStream(); + const emit = (value: string): void => { + if (value.length > 0) this.options.onStderr?.(profile, value); + }; stderr?.on("data", (chunk: Buffer) => { - this.options.onStderr?.(profile, this.redactProcessOutput(chunk.toString("utf8"))); + emit(streamRedactor.write(chunk.toString("utf8"))); }); + stderr?.once("end", () => emit(streamRedactor.flush())); + stderr?.once("close", () => emit(streamRedactor.flush())); } /** Redacts process-originated text using static and dynamically resolved secret values. */ private redactProcessOutput(value: string): string { - return redactSecrets(value, [...this.secretValuesSet, ...(this.options.secretValues ?? [])]); + return this.redactor.redactText(value); } /** Handles an unexpected close only when it belongs to the current live session generation. */ diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 18c56041..0ccbff30 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -25,6 +25,7 @@ export type MiftahErrorCode = | "UPSTREAM_TOOL_LIST_FAILED" | "UPSTREAM_DISCOVERY_FAILED" | "UPSTREAM_CALL_FAILED" + | "AUDIT_WRITE_FAILED" | "UPSTREAM_SELECTION_AMBIGUOUS" | "ROUTING_AMBIGUOUS" | "ROUTING_BLOCKED" diff --git a/tests/audit.test.ts b/tests/audit.test.ts index 04b71c61..5b2b6b29 100644 --- a/tests/audit.test.ts +++ b/tests/audit.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, stat } from "node:fs/promises"; +import { mkdtemp, readFile, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -48,4 +48,70 @@ describe("audit logger", () => { expect((await stat(directory)).mode & 0o077).toBe(0); expect((await stat(path)).mode & 0o077).toBe(0); }); + + it("keeps the operation result available when fail-open audit writing fails", async () => { + const root = await mkdtemp(join(tmpdir(), "miftah-audit-fail-open-")); + const blockingPath = join(root, "not-a-directory"); + await writeFile(blockingPath, "file"); + const logger = new AuditLogger(join(blockingPath, "audit.jsonl"), { failureMode: "fail-open" }); + + await expect( + logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4 + }) + ).resolves.toBeUndefined(); + + expect(logger.health()).toMatchObject({ + state: "failed", + lastFailure: { errorCode: "AUDIT_WRITE_FAILED" } + }); + }); + + it("fails closed with a stable error code when audit writing fails", async () => { + const root = await mkdtemp(join(tmpdir(), "miftah-audit-fail-closed-")); + const blockingPath = join(root, "not-a-directory"); + await writeFile(blockingPath, "file"); + const logger = new AuditLogger(join(blockingPath, "audit.jsonl")); + + await expect( + logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4 + }) + ).rejects.toMatchObject({ code: "AUDIT_WRITE_FAILED" }); + }); + + it("serializes concurrent writes into complete JSONL records", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-concurrent-")); + const path = join(directory, "audit.jsonl"); + const logger = new AuditLogger(path); + + await Promise.all( + Array.from({ length: 64 }, (_, index) => + logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: `operation-${index}`, + status: "success", + durationMs: index + }) + ) + ); + + const lines = (await readFile(path, "utf8")).trim().split("\n"); + expect(lines).toHaveLength(64); + expect(lines.map((line) => JSON.parse(line).name).sort()).toEqual( + Array.from({ length: 64 }, (_, index) => `operation-${index}`).sort() + ); + }); }); diff --git a/tests/fixtures/fake-upstream.mjs b/tests/fixtures/fake-upstream.mjs index 32a1805e..20f0ab82 100644 --- a/tests/fixtures/fake-upstream.mjs +++ b/tests/fixtures/fake-upstream.mjs @@ -40,6 +40,7 @@ const crashAfterConnectDelayMs = Number(process.env.TEST_CRASH_AFTER_CONNECT_DEL const startCountPath = process.env.TEST_START_COUNT_PATH; const failInitialize = process.env.TEST_FAIL_INITIALIZE === "true"; const stderrMessage = process.env.TEST_STDERR_MESSAGE; +const stderrSplitAt = Number(process.env.TEST_STDERR_SPLIT_AT ?? "0"); const hangOnStartPath = process.env.TEST_HANG_ON_START_PATH; const hangOnStartReadyPath = process.env.TEST_HANG_ON_START_READY_PATH; const shutdownDelayMs = Number(process.env.TEST_SHUTDOWN_DELAY_MS ?? "0"); @@ -76,7 +77,13 @@ if (process.env.TEST_IGNORE_SIGTERM === "true") { process.on("SIGTERM", () => undefined); } if (stderrMessage) { - process.stderr.write(`${stderrMessage}\n`); + if (stderrSplitAt > 0 && stderrSplitAt < stderrMessage.length) { + process.stderr.write(stderrMessage.slice(0, stderrSplitAt)); + await delay(0); + process.stderr.write(`${stderrMessage.slice(stderrSplitAt)}\n`); + } else { + process.stderr.write(`${stderrMessage}\n`); + } } if (crashOnCallToolPath && existsSync(crashOnCallToolPath)) { throw new Error("test upstream configured to stay unavailable after an abrupt exit"); diff --git a/tests/secrets.test.ts b/tests/secrets.test.ts index 69960cf6..525d2237 100644 --- a/tests/secrets.test.ts +++ b/tests/secrets.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { createRedactor, redactSecrets, redactUri } from "../src/secrets/redact.js"; +import { SecretRedactor, createRedactor, redactSecrets, redactUri } from "../src/secrets/redact.js"; const opaqueInvalidUriPattern = /^miftah-invalid-uri:[a-f0-9]{64}$/; @@ -19,6 +19,22 @@ describe("secret redaction", () => { }); }); + it("shares newly resolved secret values with every later redaction", () => { + const redactor = new SecretRedactor(["initial-secret"]); + + redactor.add("later-secret"); + + expect( + redactor.redact({ + initial: "initial-secret", + nested: { later: "prefix later-secret suffix" } + }) + ).toEqual({ + initial: "[REDACTED]", + nested: { later: "prefix [REDACTED] suffix" } + }); + }); + it("redacts secret-looking environment keys", () => { expect(redactSecrets({ API_TOKEN: "hidden", ACCOUNT: "work" })).toEqual({ API_TOKEN: "[REDACTED]", diff --git a/tests/upstream-manager.test.ts b/tests/upstream-manager.test.ts index 1c26a746..c06d3777 100644 --- a/tests/upstream-manager.test.ts +++ b/tests/upstream-manager.test.ts @@ -7,6 +7,7 @@ import { setTimeout as delay } from "node:timers/promises"; import { describe, expect, it, vi } from "vitest"; import { MultiUpstreamProcessManager } from "../src/upstream/multi-upstream-process-manager.js"; import { UpstreamProcessManager } from "../src/upstream/upstream-process-manager.js"; +import { SecretRedactor } from "../src/secrets/redact.js"; import { MiftahError } from "../src/utils/errors.js"; const fixture = join(dirname(fileURLToPath(import.meta.url)), "fixtures", "fake-upstream.mjs"); @@ -127,6 +128,43 @@ describe("upstream process manager", () => { } }); + it("shares dynamically resolved values with split stderr redaction", async () => { + const secret = "split-stderr-secret"; + const message = `upstream stderr: ${secret}`; + const stderr: string[] = []; + const redactor = new SecretRedactor(); + const manager = new UpstreamProcessManager( + { + transport: "stdio", + command: process.execPath, + args: [fixture] + }, + { + work: { + env: { + API_TOKEN: secret, + TEST_STDERR_MESSAGE: message, + TEST_STDERR_SPLIT_AT: String(message.indexOf(secret) + 5) + } + } + }, + { + startupTimeoutMs: 1_000, + redactor, + onStderr: (_profile, output) => stderr.push(output) + } + ); + + try { + await manager.get("work"); + await waitFor(() => stderr.join(""), (output) => output.includes("[REDACTED]")); + expect(stderr.join("")).not.toContain(secret); + expect(redactor.redact({ secret })).toEqual({ secret: "[REDACTED]" }); + } finally { + await manager.close(); + } + }); + it("redacts dynamically resolved secrets from startup diagnostics", async () => { const secret = "dynamic-startup-secret"; const manager = new UpstreamProcessManager( From 14655e3f723a8fdb68eeac959c18177df6135584 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Sat, 11 Jul 2026 10:18:03 +0400 Subject: [PATCH 3/5] feat(audit): add terminal audit outcomes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 6 + docs/architecture.md | 4 +- docs/config.md | 4 +- docs/security.md | 5 +- examples/github.miftah.json | 3 +- examples/multi-upstream.miftah.json | 3 +- src/audit/audit-logger.ts | 62 +- src/audit/audit-trail.ts | 141 +++++ src/audit/audit-types.ts | 15 +- src/config/presets.ts | 3 +- src/config/schema.ts | 6 +- src/config/types.ts | 1 + src/mcp/server/miftah-server.ts | 575 +++++++++++------ src/mcp/server/operation-pipeline.ts | 115 +--- src/secrets/redact.ts | 13 +- .../multi-upstream-process-manager.ts | 30 +- src/upstream/upstream-process-manager.ts | 86 ++- tests/audit-outcomes.test.ts | 583 ++++++++++++++++++ tests/audit.test.ts | 46 +- tests/config-runtime-parity.test.ts | 10 +- tests/config-schema-contract.test.ts | 6 +- tests/fixtures/fake-upstream.mjs | 8 +- tests/mcp-wrapper.test.ts | 12 +- tests/operation-pipeline.test.ts | 36 +- tests/upstream-manager.test.ts | 118 ++++ 25 files changed, 1563 insertions(+), 328 deletions(-) create mode 100644 src/audit/audit-trail.ts create mode 100644 tests/audit-outcomes.test.ts diff --git a/README.md b/README.md index ca65d264..916d742f 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,12 @@ Supported local references include environment variables (`${NAME}` and `secretr Use `miftah doctor` to inspect config and upstream readiness without printing process environment values. +## Audit logging + +Set `audit.path` to record one terminal JSONL event for every supported MCP operation, including discovery, management, tool, resource, and prompt requests. Events include a per-process session ID, request/event ID, source and selected profiles, upstream, routing and policy metadata where applicable, terminal outcome, stable error code, and duration. Wrapper and upstream lifecycle transitions are recorded separately. Arguments are omitted unless `audit.includeArguments` is `true`. + +New audit directories and files use owner-only permissions where the platform supports them. `audit.failureMode` defaults to `"fail-closed"`, which verifies the audit sink before dispatch and refuses the request if it cannot be prepared. Set it to `"fail-open"` only when availability outweighs that guarantee; the original operation remains available and `miftah_health` reports a redacted `AUDIT_WRITE_FAILED` audit-health entry. + ## CLI | Command | Purpose | diff --git a/docs/architecture.md b/docs/architecture.md index ba38827a..560199c5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -16,7 +16,7 @@ The public server is built with the official `@modelcontextprotocol/sdk` `Server The server advertises management tools plus tools discovered from the active profile. It advertises `tools.listChanged` and emits `notifications/tools/list_changed` after profile changes, restarts, and recovery that changes the public tool snapshot. Unknown names are rejected from the capability snapshot rather than guessed or forwarded. If a routing rule targets another profile, Miftah forwards only when that profile exposes the same name with an identical client-visible schema; otherwise it returns `TOOL_SCHEMA_MISMATCH`. In strict discovery mode, Miftah preflights every configured profile and rejects unavailable upstreams or any mismatched exposed tool contract before publishing a snapshot. -Every proxied tool call, resource read, and prompt retrieval enters `OperationPipeline`. It captures the source profile state before awaiting work, resolves routing against that fixed active-profile fallback, evaluates the selected profile policy, resolves the exact target upstream route, executes, redacts the result or error, and emits one terminal operation audit record when audit logging is configured. Tools retain their original upstream names for routing and policy compatibility; resource reads and prompt retrieval use the stable policy names `resources/read` and `prompts/get`. Denied, confirmation-required, blocked, and ambiguous operations never resolve or execute an upstream read/get route. +Every supported MCP request enters one outer audit scope. The scope records one terminal operation event on success or safe failure and also covers discovery/list failures, unknown names, and management tools that do not enter the proxy pipeline. `OperationPipeline` enriches proxied tool calls, resource reads, and prompt retrieval with captured source/target profile, upstream, routing, policy, and risk metadata; it does not emit its own record. It captures the source profile state before awaiting work, resolves routing against that fixed active-profile fallback, evaluates the selected profile policy, resolves the exact target upstream route, executes, and redacts the result or error. Tools retain their original upstream names for routing and policy compatibility; resource reads and prompt retrieval use the stable policy names `resources/read` and `prompts/get`. Denied, confirmation-required, blocked, and ambiguous operations never resolve or execute an upstream read/get route. Upstream managers publish typed lifecycle transitions, which the server records as separate audit events without letting audit I/O interrupt cleanup or recovery. For a multi-entry `upstreams` map, `ResourcePromptRegistry` discovers resources and prompts from every configured upstream and publishes namespaced public values only after collision checks succeed. It names resources and prompts `__`, and exposes resources as `miftah://resource/?uri=`. The registry retains the original upstream URI privately with the exact profile and upstream route. Prompt resource links and read-result sub-resources are registered as exact Miftah routes to their originating upstream. Before resource/prompt URI metadata crosses the boundary, structural redaction strips userinfo/fragments and redacts query values, including URI metadata returned by reads and prompt content. After the operation pipeline authorizes its selected profile, a read or prompt get resolves that exact route and forwards only to its originating upstream; unknown identifiers are rejected rather than forwarded. Aggregate pagination stores the individual upstream cursors behind opaque, bounded in-memory LRU state scoped to the profile and capability kind. @@ -30,7 +30,7 @@ Configuration and runtime concerns are intentionally separate: - `routing/` resolves explicit rules and safe fallbacks. - `policy/` classifies proxied-operation risk and returns allow/deny/confirm decisions. - `upstream/` owns child processes, MCP initialization, caching, health, and cleanup. -- `audit/` writes local JSONL metadata only. +- `audit/` owns request scopes, redacted local JSONL metadata, restrictive file permissions, serialized same-process writes, and audit health. - `mcp/server/` adapts those services to MCP operations and management tools. The design leaves transport and multi-upstream seams in the config and interfaces. Remote transports should be added as separate upstream session implementations rather than weakening the STDIO security defaults. diff --git a/docs/config.md b/docs/config.md index bcc18d4e..832a3646 100644 --- a/docs/config.md +++ b/docs/config.md @@ -46,7 +46,9 @@ Routing rules receive a tool's original arguments unchanged. Resource reads expo Policies classify these operation names as `read`, `write`, or `destructive` using configurable overrides and conservative name heuristics. `denyRisk` takes precedence over `allowRisk`; `requireConfirmation` returns a structured error instead of forwarding the operation. -Audit logging defaults to local JSONL when a path is configured. Arguments are excluded unless `includeArguments` is true, and all configured secret values are redacted before writing. +Audit logging writes local JSONL when a path is configured. Every supported MCP request emits one terminal operation event with a request ID, per-process session ID, source/selected profiles, stable outcome/error code, duration, and any available upstream, routing, policy, and risk metadata; wrapper and upstream lifecycle transitions emit separate event records. Arguments are excluded unless `includeArguments` is true, and all configured secret values are redacted before writing. Audit directories and files are created with owner-only permissions where the platform supports them. + +`audit.failureMode` accepts `"fail-closed"` (the default) or `"fail-open"`. Fail-closed verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared; a terminal write error also surfaces as `AUDIT_WRITE_FAILED`. Fail-open preserves the request result and exposes a redacted `AUDIT_WRITE_FAILED` entry through `miftah_health`; it should be used only when availability is more important than complete auditability. ## Runtime-supported controls diff --git a/docs/security.md b/docs/security.md index ed060526..3b538c54 100644 --- a/docs/security.md +++ b/docs/security.md @@ -8,9 +8,12 @@ Miftah is a credential broker, so safe defaults are part of the product contract - upstream stderr, errors, diagnostics, audit entries, and tool results pass through redaction; - profile switching can be disabled or locked to a single profile; - destructive and ambiguous requests are not silently routed; -- audit records contain metadata, not sensitive payloads, by default; +- audit records contain metadata, not sensitive payloads or arguments, by default; +- audit files and directories are owner-only where platform support permits it, and audit-write failures are explicit; - provider tokens should be separate, least-privilege tokens per account and risk level. +Audit writes default to fail-closed: Miftah verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared. An operator can set `audit.failureMode` to `"fail-open"` for availability-sensitive deployments; Miftah then preserves the request outcome but exposes a redacted `AUDIT_WRITE_FAILED` health entry. This mode trades complete auditability for availability. + Miftah cannot reduce privileges granted by a provider token. A read-only Miftah policy is a local blocklist, not a replacement for provider-side scopes. Avoid putting real credentials in examples, commits, or support logs. The STDIO transport is the default because it avoids a network listener. Any future HTTP server must bind localhost by default and require explicit authentication before non-local binding. diff --git a/examples/github.miftah.json b/examples/github.miftah.json index 4043685e..5ad612a7 100644 --- a/examples/github.miftah.json +++ b/examples/github.miftah.json @@ -81,7 +81,8 @@ "path": "~/.local/state/miftah/github.audit.jsonl", "format": "jsonl", "includeArguments": false, - "redact": true + "redact": true, + "failureMode": "fail-closed" }, "tooling": { "collisionStrategy": "prefix-upstream" diff --git a/examples/multi-upstream.miftah.json b/examples/multi-upstream.miftah.json index 427fae43..bd5f45fa 100644 --- a/examples/multi-upstream.miftah.json +++ b/examples/multi-upstream.miftah.json @@ -57,7 +57,8 @@ "path": "~/.local/state/miftah/dev-tools.audit.jsonl", "format": "jsonl", "includeArguments": false, - "redact": true + "redact": true, + "failureMode": "fail-closed" }, "tooling": { "collisionStrategy": "prefix-upstream" diff --git a/src/audit/audit-logger.ts b/src/audit/audit-logger.ts index 427ba57a..4426a02a 100644 --- a/src/audit/audit-logger.ts +++ b/src/audit/audit-logger.ts @@ -1,4 +1,4 @@ -import { chmod, mkdir, open } from "node:fs/promises"; +import { chmod, mkdir, open, type FileHandle } from "node:fs/promises"; import { dirname } from "node:path"; import type { AuditEvent, AuditFailureMode, AuditHealth } from "./audit-types.js"; import { SecretRedactor } from "../secrets/redact.js"; @@ -27,33 +27,41 @@ export class AuditLogger { } async log(event: AuditEvent): Promise { - const safeEvent = this.redactor.redact( + const safeEvent = this.redactor.redactForAudit( !this.options.includeArguments ? { ...event, arguments: undefined } : event ); try { - await this.enqueue(`${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`); + await this.enqueue(() => this.writeLine(`${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`)); this.lastFailure = undefined; } catch (error) { - const failure = this.asWriteFailure(error); - this.lastFailure = { - timestamp: new Date().toISOString(), - errorCode: "AUDIT_WRITE_FAILED", - message: failure.message - }; + const failure = this.recordFailure(error); if (this.options.failureMode === "fail-closed") throw failure; } } + /** Verifies that a fail-closed sink is writable before an operation can make a side effect. */ + async ensureWritable(): Promise { + if (this.options.failureMode !== "fail-closed") return; + try { + await this.enqueue(() => this.prepareFile()); + } catch (error) { + throw this.recordFailure(error); + } + } + health(): AuditHealth { return this.lastFailure ? { state: "failed", lastFailure: structuredClone(this.lastFailure) } : { state: "healthy" }; } - private enqueue(line: string): Promise { + private enqueue(operation: () => Promise): Promise { const prior = AuditLogger.writesByPath.get(this.path) ?? Promise.resolve(); - const write = prior.catch(() => undefined).then(() => this.writeLine(line)); - const tail = write.catch(() => undefined); + const write = prior.catch(() => undefined).then(operation); + const tail = write.then( + () => undefined, + () => undefined + ); AuditLogger.writesByPath.set(this.path, tail); void tail.then(() => { if (AuditLogger.writesByPath.get(this.path) === tail) AuditLogger.writesByPath.delete(this.path); @@ -62,18 +70,42 @@ export class AuditLogger { } private async writeLine(line: string): Promise { + const file = await this.openAuditFile(); + try { + await file.writeFile(line, "utf8"); + } finally { + await file.close(); + } + } + + private async prepareFile(): Promise { + const file = await this.openAuditFile(); + await file.close(); + } + + private async openAuditFile(): Promise { const directory = dirname(this.path); await mkdir(directory, { recursive: true, mode: 0o700 }); - await setRestrictiveMode(directory, 0o700); const file = await open(this.path, "a", 0o600); try { await setRestrictiveMode(this.path, 0o600); - await file.writeFile(line, "utf8"); - } finally { + return file; + } catch (error) { await file.close(); + throw error; } } + private recordFailure(error: unknown): MiftahError { + const failure = this.asWriteFailure(error); + this.lastFailure = { + timestamp: new Date().toISOString(), + errorCode: "AUDIT_WRITE_FAILED", + message: failure.message + }; + return failure; + } + private asWriteFailure(error: unknown): MiftahError { const message = this.redactor.redactText(error instanceof Error ? error.message : String(error)); return new MiftahError("AUDIT_WRITE_FAILED", `AUDIT_WRITE_FAILED: unable to write audit record: ${message}`); diff --git a/src/audit/audit-trail.ts b/src/audit/audit-trail.ts new file mode 100644 index 00000000..fca8aba3 --- /dev/null +++ b/src/audit/audit-trail.ts @@ -0,0 +1,141 @@ +import { randomUUID } from "node:crypto"; +import { AuditLogger } from "./audit-logger.js"; +import type { AuditEvent, AuditHealth, AuditRoutingSource, AuditStatus } from "./audit-types.js"; + +export interface AuditOperationInput { + operation: string; + name: string; + sourceProfile: string; + profile?: string; + arguments?: Record; +} + +export interface AuditScopeUpdate { + name?: string; + profile?: string; + upstream?: string; + routingReason?: string; + routingSource?: AuditRoutingSource; + policyName?: string; + policyDecision?: AuditEvent["policyDecision"]; + risk?: AuditEvent["risk"]; +} + +export interface AuditScopeResult { + status: AuditStatus; + errorCode?: string; +} + +export interface AuditLifecycleInput { + operation: string; + name: string; + profile: string; + upstream?: string; + lockToProfile?: string; + status: AuditStatus; + errorCode?: string; +} + +/** Creates one final audit record per MCP request when audit logging is configured. */ +export class AuditTrail { + readonly sessionId = randomUUID(); + + constructor( + private readonly wrapperName: string, + private readonly logger?: AuditLogger + ) {} + + beginOperation(input: AuditOperationInput): AuditScope { + return new AuditScope(this, input); + } + + health(): { enabled: boolean; state?: AuditHealth["state"]; lastFailure?: AuditHealth["lastFailure"] } { + if (!this.logger) return { enabled: false }; + return { enabled: true, ...this.logger.health() }; + } + + async ensureWritable(): Promise { + await this.logger?.ensureWritable(); + } + + async write(event: AuditEvent): Promise { + await this.logger?.log(event); + } + + async writeLifecycle(input: AuditLifecycleInput): Promise { + await this.write({ + wrapper: this.wrapperName, + kind: "lifecycle", + eventId: randomUUID(), + sessionId: this.sessionId, + sourceProfile: input.profile, + profile: input.profile, + operation: input.operation, + name: input.name, + status: input.status, + durationMs: 0, + ...(input.upstream === undefined ? {} : { upstream: input.upstream }), + ...(input.lockToProfile === undefined ? {} : { lockToProfile: input.lockToProfile }), + ...(input.errorCode === undefined ? {} : { errorCode: input.errorCode }) + }); + } + + /** Records a background lifecycle event without letting audit I/O disrupt process management. */ + recordLifecycle(input: AuditLifecycleInput): void { + void this.writeLifecycle(input).catch(() => undefined); + } + + wrapper(): string { + return this.wrapperName; + } +} + +/** Accumulates request context and prevents duplicate terminal records. */ +export class AuditScope { + private readonly requestId = randomUUID(); + private readonly startedAt = Date.now(); + private readonly event: AuditOperationInput & AuditScopeUpdate; + private finalized = false; + + constructor( + private readonly trail: AuditTrail, + input: AuditOperationInput + ) { + this.event = { ...input, profile: input.profile ?? input.sourceProfile }; + } + + get isFinalized(): boolean { + return this.finalized; + } + + update(update: AuditScopeUpdate): void { + Object.assign(this.event, update); + } + + async finish(result: AuditScopeResult): Promise { + if (this.finalized) throw new Error("Audit scope already has a terminal event"); + this.finalized = true; + await this.trail.write({ + wrapper: this.trail.wrapper(), + kind: "operation", + eventId: this.requestId, + requestId: this.requestId, + sessionId: this.trail.sessionId, + sourceProfile: this.event.sourceProfile, + profile: this.event.profile ?? this.event.sourceProfile, + operation: this.event.operation, + name: this.event.name, + status: result.status, + durationMs: Date.now() - this.startedAt, + ...(this.event.upstream === undefined ? {} : { upstream: this.event.upstream }), + ...(this.event.routingReason === undefined ? {} : { routingReason: this.event.routingReason }), + ...(this.event.routingSource === undefined ? {} : { routingSource: this.event.routingSource }), + ...(this.event.policyName === undefined ? {} : { policyName: this.event.policyName }), + ...(this.event.policyDecision === undefined ? {} : { policyDecision: this.event.policyDecision }), + ...(this.event.risk === undefined ? {} : { risk: this.event.risk }), + ...(this.event.arguments === undefined ? {} : { arguments: this.event.arguments }), + ...(result.errorCode === undefined ? {} : { errorCode: result.errorCode }) + }); + } + +} diff --git a/src/audit/audit-types.ts b/src/audit/audit-types.ts index d136ffa7..0fcbac4e 100644 --- a/src/audit/audit-types.ts +++ b/src/audit/audit-types.ts @@ -13,14 +13,27 @@ export interface AuditHealth { lastFailure?: AuditWriteFailure; } +export type AuditEventKind = "operation" | "lifecycle"; +export type AuditStatus = "success" | "failure" | "blocked" | "denied" | "confirmation-required" | "ambiguous"; +export type AuditRoutingSource = "rule" | "active-profile" | "default-profile"; + export interface AuditEvent { wrapper: string; profile: string; + kind?: AuditEventKind; + eventId?: string; + requestId?: string; + sessionId?: string; + sourceProfile?: string; + upstream?: string; + lockToProfile?: string; operation: "tools/call" | "resources/read" | "prompts/get" | string; name: string; - status: "success" | "failure" | "blocked"; + status: AuditStatus; durationMs: number; routingReason?: string; + routingSource?: AuditRoutingSource; + policyName?: string; policyDecision?: PolicyAction; risk?: RiskLevel; arguments?: unknown; diff --git a/src/config/presets.ts b/src/config/presets.ts index 256cde6a..7ee8c083 100644 --- a/src/config/presets.ts +++ b/src/config/presets.ts @@ -22,7 +22,8 @@ function buildSharedDefaults(): SharedDefaults { path: "~/.local/state/miftah/audit.jsonl", format: "jsonl", includeArguments: false, - redact: true + redact: true, + failureMode: "fail-closed" }, tooling: { collisionStrategy: "prefix-upstream" } }; diff --git a/src/config/schema.ts b/src/config/schema.ts index 83403fa5..57639ad6 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -158,7 +158,8 @@ const publicAuditSchema = z path: z.string().optional(), format: z.literal("jsonl").optional(), includeArguments: z.boolean().optional(), - redact: z.literal(true).optional() + redact: z.literal(true).optional(), + failureMode: z.enum(["fail-open", "fail-closed"]).optional() }) .strict(); @@ -168,7 +169,8 @@ const auditSchema = z path: z.string().optional(), format: z.literal("jsonl").optional(), includeArguments: z.boolean().optional(), - redact: z.boolean().optional() + redact: z.boolean().optional(), + failureMode: z.enum(["fail-open", "fail-closed"]).optional() }) .strict(); diff --git a/src/config/types.ts b/src/config/types.ts index 0ffecebc..5bc6f156 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -79,6 +79,7 @@ export interface AuditConfig { format?: "jsonl"; includeArguments?: boolean; redact?: true; + failureMode?: "fail-open" | "fail-closed"; } export interface ToolingConfig { diff --git a/src/mcp/server/miftah-server.ts b/src/mcp/server/miftah-server.ts index 29eeccc9..bf98ce9b 100644 --- a/src/mcp/server/miftah-server.ts +++ b/src/mcp/server/miftah-server.ts @@ -21,12 +21,18 @@ import { type Tool } from "@modelcontextprotocol/sdk/types.js"; import type { MiftahConfig } from "../../config/types.js"; -import { redactSecrets, redactUri, redactUrisInText } from "../../secrets/redact.js"; +import { SecretRedactor, redactUri } from "../../secrets/redact.js"; import { ProfileManager } from "../../profiles/profile-manager.js"; import { RoutingEngine } from "../../routing/routing-engine.js"; import { PolicyEngine } from "../../policy/policy-engine.js"; import { AuditLogger } from "../../audit/audit-logger.js"; -import { UpstreamProcessManager, type UpstreamHealth } from "../../upstream/upstream-process-manager.js"; +import { AuditScope, AuditTrail, type AuditScopeResult } from "../../audit/audit-trail.js"; +import type { AuditStatus } from "../../audit/audit-types.js"; +import { + UpstreamProcessManager, + type UpstreamHealth, + type UpstreamLifecycleEvent +} from "../../upstream/upstream-process-manager.js"; import { MultiUpstreamProcessManager } from "../../upstream/multi-upstream-process-manager.js"; import type { UpstreamSession } from "../../upstream/upstream-session.js"; import { MiftahError } from "../../utils/errors.js"; @@ -76,6 +82,23 @@ function textResult(text: string, isError = false): CallToolResult { return { content: [{ type: "text", text }], ...(isError ? { isError: true } : {}) }; } +function managementOperation(name: string): string { + if (name === "miftah_use_profile") return "profiles/switch"; + if (name === "miftah_reset_profile") return "profiles/reset"; + if (name === "miftah_restart_profile") return "upstreams/restart"; + return `management/${name.replace(/^miftah_/, "").replaceAll("_", "-")}`; +} + +function managementName(name: string, args: Record): string { + if (name === "miftah_use_profile" || name === "miftah_restart_profile" || name === "miftah_profile_info") { + return typeof args.profile === "string" ? args.profile : "profile"; + } + if (name === "miftah_reset_profile") return "default"; + if (name === "miftah_list_profiles") return "profiles"; + if (name === "miftah_list_upstream_tools") return typeof args.profile === "string" ? args.profile : "active-profile"; + return name; +} + interface ResourcePromptProxyAvailable { available: true; upstreamName?: string; @@ -94,6 +117,8 @@ export class MiftahServer { private readonly routing: RoutingEngine; private readonly policy: PolicyEngine; private readonly audit?: AuditLogger; + private readonly auditTrail: AuditTrail; + private readonly redactor: SecretRedactor; private readonly resourcePromptProxy: ResourcePromptProxyAvailability; private readonly toolRegistry: ToolRegistry; private readonly operationPipeline: OperationPipeline; @@ -108,6 +133,7 @@ export class MiftahServer { private readonly profiles: ProfileManager, private readonly upstreams: UpstreamProcessManager | MultiUpstreamProcessManager ) { + this.redactor = upstreams.getRedactor(); this.resourcePromptProxy = this.resourcePromptProxyAvailability(); this.server = new Server( { name: `miftah-${config.name}`, version: "0.1.1" }, @@ -143,7 +169,7 @@ export class MiftahServer { () => multiUpstreams.listUpstreams(), (profile, upstreamName, params) => this.discoverResources(profile, upstreamName, params), (profile, upstreamName, params) => this.discoverPrompts(profile, upstreamName, params), - (value) => redactSecrets(value, this.upstreams.getSecretValues()), + (value) => this.redactor.redact(value), undefined, config.tooling?.toolDiscoveryMode ?? "permissive" ); @@ -152,118 +178,200 @@ export class MiftahServer { if (config.audit?.enabled !== false && config.audit?.path) { this.audit = new AuditLogger(config.audit.path, { includeArguments: config.audit.includeArguments, - secretValues: [] + redactor: this.redactor, + failureMode: config.audit.failureMode }); } + this.auditTrail = new AuditTrail(config.name, this.audit); + this.upstreams.addLifecycleListener((event) => this.recordUpstreamLifecycle(event)); this.operationPipeline = new OperationPipeline({ - wrapper: config.name, profiles, routing: this.routing, policy: this.policy, upstreams, - writeAudit: (event) => this.writeAudit(event) + redactor: this.redactor }); this.registerHandlers(); } - connect(transport: Transport): Promise { - return this.server.connect(transport); + async connect(transport: Transport): Promise { + await this.server.connect(transport); + await this.auditTrail.writeLifecycle({ + operation: "wrapper/start", + name: this.config.name, + profile: this.profiles.current().activeProfile, + lockToProfile: this.config.security?.lockToProfile ?? undefined, + status: "success" + }).catch(() => undefined); } async close(): Promise { await this.server.close(); await this.upstreams.close(); + await this.auditTrail.writeLifecycle({ + operation: "wrapper/shutdown", + name: this.config.name, + profile: this.profiles.current().activeProfile, + status: "success" + }).catch(() => undefined); } private registerHandlers(): void { this.server.setRequestHandler(ListToolsRequestSchema, async () => { - const snapshot = await this.activeToolSnapshot(); - return { tools: [...managementTools, ...snapshot.getTools()] }; + const source = this.profiles.current(); + return this.runAudited( + { operation: "tools/list", name: "tools", sourceProfile: source.activeProfile }, + async (audit) => { + const upstream = this.auditUpstreamName(); + if (upstream) audit.update({ upstream }); + const { profile, snapshot } = await this.activeToolSnapshot(); + audit.update({ profile }); + return { tools: [...managementTools, ...snapshot.getTools()] }; + } + ); }); this.server.setRequestHandler(CallToolRequestSchema, async (request) => { const name = request.params.name; const args = request.params.arguments ?? {}; - if (managementTools.some((tool) => tool.name === name)) return this.handleManagement(name, args); - return this.handleUpstreamTool(name, args); + const source = this.profiles.current(); + const isManagementTool = managementTools.some((tool) => tool.name === name); + return this.runAudited( + { + operation: isManagementTool ? managementOperation(name) : "tools/call", + name: isManagementTool ? managementName(name, args) : name, + sourceProfile: source.activeProfile, + arguments: args + }, + (audit) => + isManagementTool ? this.handleManagement(name, args, audit) : this.handleUpstreamTool(name, args, audit, source), + (error) => textResult(error.message, true), + (result) => + result.isError + ? { status: "failure", errorCode: "UPSTREAM_CALL_FAILED" } + : { status: "success" } + ); }); if (this.resourcePromptProxy.available) { const upstreamName = this.resourcePromptProxy.upstreamName; this.server.setRequestHandler(ListResourcesRequestSchema, async (request) => { - const profile = this.profiles.current().activeProfile; - if (this.resourcePromptRegistry) { - try { - return await this.resourcePromptRegistry.listResources(profile, request.params?.cursor); - } finally { - await this.notifyResourceAvailabilityChange(profile); + const source = this.profiles.current(); + return this.runAudited( + { + operation: "resources/list", + name: "resources", + sourceProfile: source.activeProfile, + arguments: request.params ?? {} + }, + async (audit) => { + const upstream = this.resourcePromptRegistry ? undefined : this.auditUpstreamName(upstreamName); + if (upstream) audit.update({ upstream }); + if (this.resourcePromptRegistry) { + try { + return await this.resourcePromptRegistry.listResources(source.activeProfile, request.params?.cursor); + } finally { + await this.notifyResourceAvailabilityChange(source.activeProfile); + } + } + return redactDirectResourceList(await this.discoverResources(source.activeProfile, upstreamName, request.params)); } - } - return redactSecrets( - redactDirectResourceList(await this.discoverResources(profile, upstreamName, request.params)), - this.upstreams.getSecretValues() ); }); this.server.setRequestHandler(ReadResourceRequestSchema, async (request) => { const source = this.profiles.current(); - if (this.resourcePromptRegistry) { - try { - return await this.executeResourceRead(source, upstreamName, request.params); - } finally { - await this.notifyResourceAvailabilityChange(source.activeProfile); + return this.runAudited( + { + operation: "resources/read", + name: this.redactor.redactUri(request.params.uri), + sourceProfile: source.activeProfile, + arguments: { uri: this.redactor.redactUri(request.params.uri) } + }, + async (audit) => { + if (this.resourcePromptRegistry) { + try { + return await this.executeResourceRead(source, upstreamName, request.params, audit); + } finally { + await this.notifyResourceAvailabilityChange(source.activeProfile); + } + } + return this.executeResourceRead(source, upstreamName, request.params, audit); } - } - return this.executeResourceRead(source, upstreamName, request.params); + ); }); this.server.setRequestHandler(ListPromptsRequestSchema, async (request) => { - const profile = this.profiles.current().activeProfile; - if (this.resourcePromptRegistry) { - try { - return await this.resourcePromptRegistry.listPrompts(profile, request.params?.cursor); - } finally { - await this.notifyPromptAvailabilityChange(profile); + const source = this.profiles.current(); + return this.runAudited( + { + operation: "prompts/list", + name: "prompts", + sourceProfile: source.activeProfile, + arguments: request.params ?? {} + }, + async (audit) => { + const upstream = this.resourcePromptRegistry ? undefined : this.auditUpstreamName(upstreamName); + if (upstream) audit.update({ upstream }); + if (this.resourcePromptRegistry) { + try { + return await this.resourcePromptRegistry.listPrompts(source.activeProfile, request.params?.cursor); + } finally { + await this.notifyPromptAvailabilityChange(source.activeProfile); + } + } + return redactDirectPromptList(await this.discoverPrompts(source.activeProfile, upstreamName, request.params)); } - } - return redactSecrets( - redactDirectPromptList(await this.discoverPrompts(profile, upstreamName, request.params)), - this.upstreams.getSecretValues() ); }); this.server.setRequestHandler(GetPromptRequestSchema, async (request) => { const source = this.profiles.current(); - if (this.resourcePromptRegistry) { - try { - return await this.executePromptGet(source, upstreamName, request.params); - } finally { - await this.notifyPromptAvailabilityChange(source.activeProfile); + return this.runAudited( + { + operation: "prompts/get", + name: request.params.name, + sourceProfile: source.activeProfile, + arguments: { ...(request.params.arguments ?? {}), name: request.params.name } + }, + async (audit) => { + if (this.resourcePromptRegistry) { + try { + return await this.executePromptGet(source, upstreamName, request.params, audit); + } finally { + await this.notifyPromptAvailabilityChange(source.activeProfile); + } + } + return this.executePromptGet(source, upstreamName, request.params, audit); } - } - return this.executePromptGet(source, upstreamName, request.params); + ); }); } } - private async handleUpstreamTool(name: string, args: Record): Promise { - try { - const sourceState = this.profiles.current(); - const sourceProfile = sourceState.activeProfile; - const previous = this.toolRegistry.peek(sourceProfile) ?? this.invalidatedToolSnapshots.get(sourceProfile); - const sourceSnapshot = await this.toolRegistry.get(sourceProfile); - if (this.profiles.current().revision === sourceState.revision && previous !== undefined) { - await this.notifyToolListChanged(previous, sourceSnapshot); - this.invalidatedToolSnapshots.delete(sourceProfile); - } - const mapped = sourceSnapshot.resolve(name); - if (!mapped) { - throw new MiftahError( - "TOOL_NOT_FOUND", - `TOOL_NOT_FOUND: tool '${name}' is not exposed for profile '${sourceProfile}'` - ); - } - return await this.operationPipeline.execute({ + private async handleUpstreamTool( + name: string, + args: Record, + audit: AuditScope, + sourceState: CapturedProfileState + ): Promise { + const sourceProfile = sourceState.activeProfile; + const previous = this.toolRegistry.peek(sourceProfile) ?? this.invalidatedToolSnapshots.get(sourceProfile); + const sourceSnapshot = await this.toolRegistry.get(sourceProfile); + if (this.profiles.current().revision === sourceState.revision && previous !== undefined) { + await this.notifyToolListChanged(previous, sourceSnapshot); + this.invalidatedToolSnapshots.delete(sourceProfile); + } + const mapped = sourceSnapshot.resolve(name); + if (!mapped) { + throw new MiftahError( + "TOOL_NOT_FOUND", + `TOOL_NOT_FOUND: tool '${name}' is not exposed for profile '${sourceProfile}'` + ); + } + audit.update({ name: mapped.originalName }); + return this.operationPipeline.execute( + { source: sourceState, operation: "tools/call", routingName: mapped.originalName, @@ -286,100 +394,102 @@ export class MiftahServer { ); } return { - upstreamName: target.upstreamName, + upstreamName: this.auditUpstreamName(target.upstreamName), name: target.originalName, execute: (session) => session.callTool({ name: target.originalName, arguments: args }), redact: (result) => result }; } - }); - } catch (error) { - const safeMessage = redactSecrets( - redactUrisInText(error instanceof Error ? error.message : String(error)), - this.upstreams.getSecretValues() - ); - if (error instanceof MiftahError) { - return textResult(safeMessage, true); - } - return textResult(`UPSTREAM_CALL_FAILED: ${safeMessage}`, true); - } + }, + audit + ); } - private async handleManagement(name: string, args: Record): Promise { - try { - if (name === "miftah_list_profiles") { - const activeProfile = this.profiles.current().activeProfile; - return textResult(JSON.stringify(this.profiles.list().map((profile) => ({ - ...profile, - active: profile.name === activeProfile - })))); - } - if (name === "miftah_current_profile") { - const current = this.profiles.current(); - return textResult( - JSON.stringify({ - activeProfile: current.activeProfile, - defaultProfile: current.defaultProfile, - routingMode: this.config.routing?.mode ?? "hybrid" - }) - ); - } - if (name === "miftah_use_profile") { - const previousSnapshot = this.toolRegistry.peek(this.profiles.current().activeProfile); - const profile = requiredString(args, "profile"); - const switched = this.profiles.switch(profile); - this.routing.setActiveProfile(switched.activeProfile); - this.invalidateResourcePromptProfiles(switched.previousProfile, switched.activeProfile); - await this.notifyToolListChanged(previousSnapshot, this.toolRegistry.peek(switched.activeProfile)); - await this.notifyResourcePromptListChanged(); - return textResult(`Active profile changed from ${switched.previousProfile} to ${switched.activeProfile}.`); - } - if (name === "miftah_reset_profile") { - const previousSnapshot = this.toolRegistry.peek(this.profiles.current().activeProfile); - const reset = this.profiles.reset(); - this.routing.setActiveProfile(reset.activeProfile); - this.invalidateResourcePromptProfiles(reset.previousProfile, reset.activeProfile); - await this.notifyToolListChanged(previousSnapshot, this.toolRegistry.peek(reset.activeProfile)); - await this.notifyResourcePromptListChanged(); - return textResult(`Active profile reset from ${reset.previousProfile} to ${reset.activeProfile}.`); - } - if (name === "miftah_profile_info") return textResult(JSON.stringify(this.profiles.info(requiredString(args, "profile")))); - if (name === "miftah_health") { - return textResult( - JSON.stringify({ - configValid: true, - activeProfile: this.profiles.current().activeProfile, - resourcePromptProxy: this.resourcePromptProxy.available - ? { available: true } - : { available: false, reason: this.resourcePromptProxy.reason }, - upstreams: this.upstreams.listHealth() - }) - ); - } - if (name === "miftah_validate_config") return textResult(JSON.stringify({ ok: true, errors: [] })); - if (name === "miftah_list_upstream_tools") { - const profile = args.profile === undefined ? this.profiles.current().activeProfile : requiredString(args, "profile"); - const tools = (await this.toolRegistry.get(profile)).getTools(); - return textResult(JSON.stringify(tools.map((item) => ({ name: item.name, description: item.description })))); - } - if (name === "miftah_restart_profile") { - const profile = requiredString(args, "profile"); - await this.restartUpstreamProfile(profile); - return textResult("Profile restarted."); - } - if (name === "miftah_route_preview") { - const route = this.routing.resolve({ - toolName: requiredString(args, "toolName"), - args: isRecord(args.args) ? args.args : {} - }); - const profile = this.profiles.get(route.profile); - return textResult(JSON.stringify({ ...route, policy: this.policy.evaluate(profile.policy, requiredString(args, "toolName")) })); - } - return textResult(`Unknown management tool '${name}'`, true); - } catch (error) { - const message = error instanceof MiftahError ? `${error.code}: ${error.message}` : String(error); - return textResult(redactSecrets(message, this.upstreams.getSecretValues()), true); + private async handleManagement( + name: string, + args: Record, + audit: AuditScope + ): Promise { + if (name === "miftah_list_profiles") { + const activeProfile = this.profiles.current().activeProfile; + return textResult(JSON.stringify(this.profiles.list().map((profile) => ({ + ...profile, + active: profile.name === activeProfile + })))); + } + if (name === "miftah_current_profile") { + const current = this.profiles.current(); + return textResult( + JSON.stringify({ + activeProfile: current.activeProfile, + defaultProfile: current.defaultProfile, + routingMode: this.config.routing?.mode ?? "hybrid" + }) + ); + } + if (name === "miftah_use_profile") { + const previousSnapshot = this.toolRegistry.peek(this.profiles.current().activeProfile); + const profile = requiredString(args, "profile"); + const switched = this.profiles.switch(profile); + audit.update({ name: switched.activeProfile, profile: switched.activeProfile }); + this.routing.setActiveProfile(switched.activeProfile); + this.invalidateResourcePromptProfiles(switched.previousProfile, switched.activeProfile); + await this.notifyToolListChanged(previousSnapshot, this.toolRegistry.peek(switched.activeProfile)); + await this.notifyResourcePromptListChanged(); + return textResult(`Active profile changed from ${switched.previousProfile} to ${switched.activeProfile}.`); + } + if (name === "miftah_reset_profile") { + const previousSnapshot = this.toolRegistry.peek(this.profiles.current().activeProfile); + const reset = this.profiles.reset(); + audit.update({ name: reset.activeProfile, profile: reset.activeProfile }); + this.routing.setActiveProfile(reset.activeProfile); + this.invalidateResourcePromptProfiles(reset.previousProfile, reset.activeProfile); + await this.notifyToolListChanged(previousSnapshot, this.toolRegistry.peek(reset.activeProfile)); + await this.notifyResourcePromptListChanged(); + return textResult(`Active profile reset from ${reset.previousProfile} to ${reset.activeProfile}.`); + } + if (name === "miftah_profile_info") { + const profile = requiredString(args, "profile"); + const info = this.profiles.info(profile); + audit.update({ name: profile, profile }); + return textResult(JSON.stringify(info)); + } + if (name === "miftah_health") { + return textResult( + JSON.stringify({ + configValid: true, + activeProfile: this.profiles.current().activeProfile, + resourcePromptProxy: this.resourcePromptProxy.available + ? { available: true } + : { available: false, reason: this.resourcePromptProxy.reason }, + audit: this.auditTrail.health(), + upstreams: this.upstreams.listHealth() + }) + ); + } + if (name === "miftah_validate_config") return textResult(JSON.stringify({ ok: true, errors: [] })); + if (name === "miftah_list_upstream_tools") { + const profile = args.profile === undefined ? this.profiles.current().activeProfile : requiredString(args, "profile"); + audit.update({ name: profile, profile }); + const tools = (await this.toolRegistry.get(profile)).getTools(); + return textResult(JSON.stringify(tools.map((item) => ({ name: item.name, description: item.description })))); } + if (name === "miftah_restart_profile") { + const profile = requiredString(args, "profile"); + audit.update({ name: profile, profile }); + await this.restartUpstreamProfile(profile); + return textResult("Profile restarted."); + } + if (name === "miftah_route_preview") { + const route = this.routing.resolve({ + toolName: requiredString(args, "toolName"), + args: isRecord(args.args) ? args.args : {} + }); + const profile = this.profiles.get(route.profile); + audit.update({ profile: route.profile }); + return textResult(JSON.stringify({ ...route, policy: this.policy.evaluate(profile.policy, requiredString(args, "toolName")) })); + } + throw new MiftahError("TOOL_NOT_FOUND", `TOOL_NOT_FOUND: management tool '${name}' is not registered`); } private exposedToolName(name: string, upstreamName?: string): string { @@ -398,6 +508,13 @@ export class MiftahServer { return [undefined]; } + private auditUpstreamName(upstreamName?: string): string | undefined { + if (upstreamName !== undefined) return upstreamName; + if (!(this.upstreams instanceof MultiUpstreamProcessManager)) return "default"; + const names = this.upstreams.listUpstreams(); + return names.length === 1 ? names[0] : undefined; + } + private async discoverTools(profile: string): Promise { const profiles = this.config.tooling?.toolDiscoveryMode === "strict" ? Object.keys(this.config.profiles).sort() : [profile]; @@ -460,7 +577,7 @@ export class MiftahServer { { upstreamName: upstreamNames[index] ?? "default", code, - message: redactSecrets(error instanceof Error ? error.message : String(error), this.upstreams.getSecretValues()) + message: this.redactor.redactText(error instanceof Error ? error.message : String(error)) } ]; }) @@ -531,49 +648,59 @@ export class MiftahServer { private async executeResourceRead( source: CapturedProfileState, upstreamName: string | undefined, - params: ReadResourceRequest["params"] + params: ReadResourceRequest["params"], + audit: AuditScope ): Promise { - return this.operationPipeline.execute({ - source, - operation: "resources/read", - routingName: "resources/read", - policyName: "resources/read", - name: params.uri, - args: { uri: params.uri }, - resolveTarget: async (profile) => { - if (this.resourcePromptRegistry) return this.resolveAggregatedResource(profile, params); - return { - ...(upstreamName === undefined ? {} : { upstreamName }), - name: params.uri, - execute: (session) => session.readResource(params), - redact: redactDirectReadResult - }; - } - }); + return this.operationPipeline.execute( + { + source, + operation: "resources/read", + routingName: "resources/read", + policyName: "resources/read", + name: params.uri, + args: { uri: params.uri }, + resolveTarget: async (profile) => { + if (this.resourcePromptRegistry) return this.resolveAggregatedResource(profile, params); + const auditUpstream = this.auditUpstreamName(upstreamName); + return { + ...(auditUpstream === undefined ? {} : { upstreamName: auditUpstream }), + name: params.uri, + execute: (session) => session.readResource(params), + redact: redactDirectReadResult + }; + } + }, + audit + ); } private async executePromptGet( source: CapturedProfileState, upstreamName: string | undefined, - params: GetPromptRequest["params"] + params: GetPromptRequest["params"], + audit: AuditScope ): Promise { - return this.operationPipeline.execute({ - source, - operation: "prompts/get", - routingName: "prompts/get", - policyName: "prompts/get", - name: params.name, - args: { ...(params.arguments ?? {}), name: params.name }, - resolveTarget: async (profile) => { - if (this.resourcePromptRegistry) return this.resolveAggregatedPrompt(profile, params); - return { - ...(upstreamName === undefined ? {} : { upstreamName }), - name: params.name, - execute: (session) => session.getPrompt(params), - redact: redactDirectPromptResult - }; - } - }); + return this.operationPipeline.execute( + { + source, + operation: "prompts/get", + routingName: "prompts/get", + policyName: "prompts/get", + name: params.name, + args: { ...(params.arguments ?? {}), name: params.name }, + resolveTarget: async (profile) => { + if (this.resourcePromptRegistry) return this.resolveAggregatedPrompt(profile, params); + const auditUpstream = this.auditUpstreamName(upstreamName); + return { + ...(auditUpstream === undefined ? {} : { upstreamName: auditUpstream }), + name: params.name, + execute: (session) => session.getPrompt(params), + redact: redactDirectPromptResult + }; + } + }, + audit + ); } private async discoverResources( @@ -615,11 +742,7 @@ export class MiftahServer { const session = await this.upstreams.get(profile, upstreamName); return await operation(session); } catch (error) { - const safeMessage = redactSecrets( - redactUrisInText(error instanceof Error ? error.message : String(error)), - this.upstreams.getSecretValues() - ); - throw new Error(safeMessage, { cause: error }); + throw this.toSafeError(error); } } @@ -703,11 +826,59 @@ export class MiftahServer { } } - private async writeAudit(event: Parameters[0]): Promise { - if (this.audit) await this.audit.log(redactSecrets(event, this.upstreams.getSecretValues())); + private async runAudited( + input: { + operation: string; + name: string; + sourceProfile: string; + arguments?: Record; + }, + operation: (audit: AuditScope) => Promise, + errorResult?: (error: MiftahError) => Result, + resultAudit?: (result: Result) => AuditScopeResult + ): Promise { + const audit = this.auditTrail.beginOperation(input); + try { + await this.auditTrail.ensureWritable(); + const result = await operation(audit); + await audit.finish(resultAudit?.(result) ?? { status: "success" }); + return this.redactor.redact(result); + } catch (error) { + let safeError = this.toSafeError(error); + if (!audit.isFinalized) { + try { + await audit.finish({ status: this.auditStatus(safeError), errorCode: safeError.code }); + } catch (auditError) { + safeError = this.toSafeError(auditError); + } + } + if (errorResult) return errorResult(safeError); + throw safeError; + } } - private async activeToolSnapshot(): Promise { + private auditStatus(error: MiftahError): AuditStatus { + if ( + error.code === "POLICY_BLOCKED" || + error.code === "ROUTING_BLOCKED" || + error.code === "PROFILE_SWITCH_DISABLED" + ) { + return "denied"; + } + if (error.code === "POLICY_CONFIRMATION_REQUIRED") return "confirmation-required"; + if (error.code === "ROUTING_AMBIGUOUS") return "ambiguous"; + return "failure"; + } + + private toSafeError(error: unknown): MiftahError { + const message = this.redactor.redactText(error instanceof Error ? error.message : String(error)); + if (error instanceof MiftahError) { + return new MiftahError(error.code, message, this.redactor.redact(error.details)); + } + return new MiftahError("UPSTREAM_CALL_FAILED", `UPSTREAM_CALL_FAILED: ${message}`); + } + + private async activeToolSnapshot(): Promise<{ profile: string; snapshot: ToolSnapshot }> { for (;;) { const state = this.profiles.current(); const previous = @@ -716,7 +887,7 @@ export class MiftahServer { if (this.profiles.current().revision === state.revision) { if (previous !== undefined) await this.notifyToolListChanged(previous, snapshot); this.invalidatedToolSnapshots.delete(state.activeProfile); - return snapshot; + return { profile: state.activeProfile, snapshot }; } } } @@ -763,6 +934,7 @@ export class MiftahServer { if (snapshot?.isComplete()) this.invalidatedToolSnapshots.set(profile, snapshot); this.toolRegistry.invalidate(profile); } + if (health.profile === this.profiles.current().activeProfile) { this.invalidateResourcePromptAfterUpstreamFailure(health.profile); } @@ -775,6 +947,17 @@ export class MiftahServer { this.invalidateResourcePromptAfterUpstreamFailure(health.profile); } + private recordUpstreamLifecycle(event: UpstreamLifecycleEvent): void { + this.auditTrail.recordLifecycle({ + operation: `upstream/${event.type}`, + name: event.upstreamName, + profile: event.profile, + upstream: event.upstreamName, + status: event.status, + errorCode: event.errorCode + }); + } + private invalidateResourcePromptAfterUpstreamFailure(profile: string): void { if (this.resourcePromptRegistry) { const hadResources = this.resourcePromptRegistry.hasResourceRoutes(profile); diff --git a/src/mcp/server/operation-pipeline.ts b/src/mcp/server/operation-pipeline.ts index 87318207..297d4a11 100644 --- a/src/mcp/server/operation-pipeline.ts +++ b/src/mcp/server/operation-pipeline.ts @@ -1,10 +1,10 @@ -import type { AuditEvent } from "../../audit/audit-types.js"; +import type { AuditScope } from "../../audit/audit-trail.js"; import { PolicyEngine } from "../../policy/policy-engine.js"; import type { PolicyDecision } from "../../policy/policy-types.js"; import { ProfileManager } from "../../profiles/profile-manager.js"; import { RoutingEngine } from "../../routing/routing-engine.js"; import type { RoutingDecision } from "../../routing/routing-types.js"; -import { redactSecrets, redactUri, redactUrisInText } from "../../secrets/redact.js"; +import { SecretRedactor } from "../../secrets/redact.js"; import { MultiUpstreamProcessManager } from "../../upstream/multi-upstream-process-manager.js"; import { UpstreamProcessManager } from "../../upstream/upstream-process-manager.js"; import type { UpstreamSession } from "../../upstream/upstream-session.js"; @@ -36,12 +36,11 @@ export interface ProxiedOperation { } interface PipelineOptions { - readonly wrapper: string; readonly profiles: ProfileManager; readonly routing: RoutingEngine; readonly policy: PolicyEngine; readonly upstreams: UpstreamProcessManager | MultiUpstreamProcessManager; - readonly writeAudit: (event: AuditEvent) => Promise; + readonly redactor: SecretRedactor; } /** @@ -50,53 +49,35 @@ interface PipelineOptions { export class OperationPipeline { constructor(private readonly options: PipelineOptions) {} - async execute(operation: ProxiedOperation): Promise { - const startedAt = Date.now(); - let profile = operation.source.activeProfile; - let route: RoutingDecision | undefined; - let decision: PolicyDecision | undefined; - let name = operation.name; - let result: Result; - + async execute(operation: ProxiedOperation, audit: AuditScope): Promise { try { - route = this.options.routing.resolve( + const route = this.options.routing.resolve( { toolName: operation.routingName, args: operation.args }, operation.source.activeProfile ); - profile = route.profile; - decision = this.options.policy.evaluate(this.options.profiles.get(profile).policy, operation.policyName); + const profile = route.profile; + const profileConfig = this.options.profiles.get(profile); + const decision = this.options.policy.evaluate(profileConfig.policy, operation.policyName); + audit.update({ + profile, + routingReason: route.reason, + routingSource: routingSource(route), + policyName: profileConfig.policy ?? "default", + policyDecision: decision.action, + risk: decision.risk + }); this.assertPolicyAllows(operation, route, decision, profile); const target = await operation.resolveTarget(profile); - name = target.name; + audit.update({ + name: this.auditName(operation, target.name), + ...(target.upstreamName === undefined ? {} : { upstream: target.upstreamName }) + }); const session = await this.options.upstreams.get(profile, target.upstreamName); - result = target.redact(await target.execute(session)); - result = redactSecrets(result, this.options.upstreams.getSecretValues()); + return this.options.redactor.redact(target.redact(await target.execute(session))); } catch (error) { - const safeError = this.toSafeError(error); - await this.writeAudit({ - operation, - profile, - name, - startedAt, - route, - decision, - status: this.failureStatus(safeError), - errorCode: safeError.code - }); - throw safeError; + throw this.toSafeError(error); } - - await this.writeAudit({ - operation, - profile, - name, - startedAt, - route, - decision, - status: "success" - }); - return result; } private assertPolicyAllows( @@ -129,51 +110,21 @@ export class OperationPipeline { } } - private async writeAudit(input: { - operation: ProxiedOperation; - profile: string; - name: string; - startedAt: number; - route?: RoutingDecision; - decision?: PolicyDecision; - status: AuditEvent["status"]; - errorCode?: string; - }): Promise { - await this.options.writeAudit({ - wrapper: this.options.wrapper, - profile: input.profile, - operation: input.operation.operation, - name: this.auditName(input.operation, input.name), - status: input.status, - durationMs: Date.now() - input.startedAt, - ...(input.route ? { routingReason: input.route.reason } : {}), - ...(input.decision ? { policyDecision: input.decision.action, risk: input.decision.risk } : {}), - arguments: this.auditArguments(input.operation), - ...(input.errorCode ? { errorCode: input.errorCode } : {}) - }); - } - private auditName(operation: ProxiedOperation, name: string): string { - return operation.operation === "resources/read" ? redactUri(name) : name; - } - - private auditArguments(operation: ProxiedOperation): Record { - if (operation.operation !== "resources/read" || typeof operation.args.uri !== "string") { - return operation.args; - } - return { ...operation.args, uri: redactUri(operation.args.uri) }; - } - - private failureStatus(error: MiftahError): AuditEvent["status"] { - return error.code === "POLICY_BLOCKED" || error.code === "POLICY_CONFIRMATION_REQUIRED" ? "blocked" : "failure"; + return operation.operation === "resources/read" ? this.options.redactor.redactUri(name) : name; } private toSafeError(error: unknown): MiftahError { - const message = redactSecrets( - redactUrisInText(error instanceof Error ? error.message : String(error)), - this.options.upstreams.getSecretValues() - ); - if (error instanceof MiftahError) return new MiftahError(error.code, message, error.details); + const message = this.options.redactor.redactText(error instanceof Error ? error.message : String(error)); + if (error instanceof MiftahError) { + return new MiftahError(error.code, message, this.options.redactor.redact(error.details)); + } return new MiftahError("UPSTREAM_CALL_FAILED", `UPSTREAM_CALL_FAILED: ${message}`); } } + +function routingSource(route: RoutingDecision): "rule" | "active-profile" | "default-profile" | undefined { + if (route.reason.startsWith("rule:")) return "rule"; + if (route.reason === "active-profile" || route.reason === "default-profile") return route.reason; + return undefined; +} diff --git a/src/secrets/redact.ts b/src/secrets/redact.ts index 47244f82..d8c86d5a 100644 --- a/src/secrets/redact.ts +++ b/src/secrets/redact.ts @@ -50,21 +50,21 @@ function redactString(value: string, secretValues: readonly string[]): string { } /** Recursively redacts secrets while preserving the input's data shape. */ -function redactValue(value: unknown, secretValues: readonly string[], key?: string): unknown { +function redactValue(value: unknown, secretValues: readonly string[], key?: string, redactUris = false): unknown { if (key && isSecretKey(key)) { return "[REDACTED]"; } if (typeof value === "string") { - return redactString(value, secretValues); + return redactString(redactUris ? redactUrisInText(value) : value, secretValues); } if (Array.isArray(value)) { - return value.map((item) => redactValue(item, secretValues)); + return value.map((item) => redactValue(item, secretValues, undefined, redactUris)); } if (value && typeof value === "object") { return Object.fromEntries( Object.entries(value).map(([entryKey, entryValue]) => [ entryKey, - redactValue(entryValue, secretValues, entryKey) + redactValue(entryValue, secretValues, entryKey, redactUris) ]) ); } @@ -95,6 +95,11 @@ export class SecretRedactor { return redactValue(value, this.values()) as T; } + /** Redacts structured audit values, including URI credentials embedded in arbitrary string arguments. */ + redactForAudit(value: T): T { + return redactValue(value, this.values(), undefined, true) as T; + } + redactText(value: string): string { return this.redact(redactUrisInText(value)); } diff --git a/src/upstream/multi-upstream-process-manager.ts b/src/upstream/multi-upstream-process-manager.ts index bb2dd341..378ece13 100644 --- a/src/upstream/multi-upstream-process-manager.ts +++ b/src/upstream/multi-upstream-process-manager.ts @@ -4,6 +4,7 @@ import { UpstreamProcessManager, type UpstreamCapability, type UpstreamHealth, + type UpstreamLifecycleEvent, type UpstreamManagerOptions } from "./upstream-process-manager.js"; import { ProfileSessionLimiter } from "./profile-session-limiter.js"; @@ -15,6 +16,7 @@ import { SecretRedactor } from "../secrets/redact.js"; export class MultiUpstreamProcessManager { private readonly managers: Record; private readonly healthListeners = new Set<(health: UpstreamHealth) => void>(); + private readonly lifecycleListeners = new Set<(event: UpstreamLifecycleEvent) => void>(); private readonly limiter: ProfileSessionLimiter; private readonly redactor: SecretRedactor; @@ -32,6 +34,7 @@ export class MultiUpstreamProcessManager { this.limiter ); manager.addHealthListener((health) => this.publishHealth(health)); + manager.addLifecycleListener((event) => this.publishLifecycle(event)); return [name, manager]; }) ); @@ -58,6 +61,11 @@ export class MultiUpstreamProcessManager { return () => this.healthListeners.delete(listener); } + addLifecycleListener(listener: (event: UpstreamLifecycleEvent) => void): () => void { + this.lifecycleListeners.add(listener); + return () => this.lifecycleListeners.delete(listener); + } + recordCapabilitySuccess(profile: string, capability: UpstreamCapability, upstreamName?: string): void { this.manager(upstreamName).recordCapabilitySuccess(profile, capability); } @@ -110,7 +118,27 @@ export class MultiUpstreamProcessManager { } private publishHealth(health: UpstreamHealth): void { - for (const listener of this.healthListeners) listener(health); + this.notifyListeners(this.healthListeners, health, "health"); + } + + private publishLifecycle(event: UpstreamLifecycleEvent): void { + this.notifyListeners(this.lifecycleListeners, event, "lifecycle"); + } + + private notifyListeners( + listeners: ReadonlySet<(event: Event) => void>, + event: Event, + kind: "health" | "lifecycle" + ): void { + for (const listener of listeners) { + try { + listener(structuredClone(event)); + } catch { + process.emitWarning(`MIFTAH_LISTENER_FAILED: ignored a failing ${kind} listener`, { + code: "MIFTAH_LISTENER_FAILED" + }); + } + } } } diff --git a/src/upstream/upstream-process-manager.ts b/src/upstream/upstream-process-manager.ts index ce691336..1908dee8 100644 --- a/src/upstream/upstream-process-manager.ts +++ b/src/upstream/upstream-process-manager.ts @@ -42,6 +42,17 @@ type ShutdownFailureReason = "shutdown-timeout" | "shutdown-error"; /** Identifies an intentional reason a profile's upstream process stopped. */ export type UpstreamStopReason = "idle" | "manual" | "restart" | "shutdown" | ShutdownFailureReason; +export type UpstreamLifecycleType = "start" | "start-failure" | "crash" | "restart" | "restart-failure" | "idle" | "shutdown"; + +/** Describes an observable upstream lifecycle transition. */ +export interface UpstreamLifecycleEvent { + type: UpstreamLifecycleType; + profile: string; + upstreamName: string; + status: "success" | "failure"; + errorCode?: string; +} + export interface UpstreamCapabilityHealth { state: UpstreamCapabilityState; lastTransition: string; @@ -117,6 +128,7 @@ export class UpstreamProcessManager { private readonly restartExhausted = new Set(); private readonly processErrors = new Map(); private readonly healthListeners = new Set<(health: UpstreamHealth) => void>(); + private readonly lifecycleListeners = new Set<(event: UpstreamLifecycleEvent) => void>(); private readonly options: ResolvedOptions; private readonly limiter: ProfileSessionLimiter; private nextToken = 0; @@ -176,6 +188,11 @@ export class UpstreamProcessManager { return () => this.healthListeners.delete(listener); } + addLifecycleListener(listener: (event: UpstreamLifecycleEvent) => void): () => void { + this.lifecycleListeners.add(listener); + return () => this.lifecycleListeners.delete(listener); + } + recordCapabilitySuccess(profile: string, capability: UpstreamCapability, _upstreamName?: string): void { void _upstreamName; this.recordCapability(profile, capability, "available"); @@ -363,6 +380,12 @@ export class UpstreamProcessManager { this.setProcessState(profile, "running", { pid }); this.scheduleIdleShutdown(profile, entry); if (source === "automatic") this.scheduleStabilityWindow(profile, entry); + this.publishLifecycle({ + type: source === "demand" ? "start" : "restart", + profile, + upstreamName: this.upstreamName, + status: "success" + }); return session; } catch (error) { const pid = stdioTransport?.pid ?? null; @@ -374,6 +397,13 @@ export class UpstreamProcessManager { : new MiftahError("UPSTREAM_START_FAILED", `UPSTREAM_START_FAILED: startup for '${profile}' was cancelled`); if (current) { this.setProcessState(profile, "failed", { error: failure.message, resetCapabilities: true, pid: null }); + this.publishLifecycle({ + type: source === "demand" ? "start-failure" : "restart-failure", + profile, + upstreamName: this.upstreamName, + status: "failure", + errorCode: failure.code + }); } if (source !== "automatic" || !this.canAutomaticallyRetry(profile)) { this.limiter.release(profile, this.upstreamName); @@ -454,6 +484,13 @@ export class UpstreamProcessManager { resetCapabilities: true, pid: null }); + this.publishLifecycle({ + type: "crash", + profile, + upstreamName: this.upstreamName, + status: "failure", + errorCode: "UPSTREAM_START_FAILED" + }); if (this.options.restartOnCrash) { this.scheduleAutomaticRestart(profile, generation); } else { @@ -476,6 +513,13 @@ export class UpstreamProcessManager { pid: null, restartLimitReached: true }); + this.publishLifecycle({ + type: "restart-failure", + profile, + upstreamName: this.upstreamName, + status: "failure", + errorCode: "UPSTREAM_RESTART_LIMIT_EXCEEDED" + }); this.limiter.release(profile, this.upstreamName); return; } @@ -602,6 +646,26 @@ export class UpstreamProcessManager { resetCapabilities: true, lastStopReason: shutdownFailure ?? reason }); + const failureCode = + shutdownFailure === "shutdown-timeout" ? "UPSTREAM_SHUTDOWN_TIMEOUT" : "UPSTREAM_SHUTDOWN_FAILED"; + if (reason === "restart" && shutdownFailure !== undefined) { + this.publishLifecycle({ + type: "restart-failure", + profile, + upstreamName: this.upstreamName, + status: "failure", + errorCode: failureCode + }); + } else if (reason !== "restart") { + const failed = shutdownFailure !== undefined; + this.publishLifecycle({ + type: reason === "idle" ? "idle" : "shutdown", + profile, + upstreamName: this.upstreamName, + status: failed ? "failure" : "success", + ...(failed ? { errorCode: failureCode } : {}) + }); + } } /** Finalizes a session after a timeout or close error so lifecycle capacity is never stranded. */ @@ -814,7 +878,27 @@ export class UpstreamProcessManager { private publishHealth(health: UpstreamHealth): void { this.health.set(health.profile, health); - for (const listener of this.healthListeners) listener(structuredClone(health)); + this.notifyListeners(this.healthListeners, health, "health"); + } + + private publishLifecycle(event: UpstreamLifecycleEvent): void { + this.notifyListeners(this.lifecycleListeners, event, "lifecycle"); + } + + private notifyListeners( + listeners: ReadonlySet<(event: Event) => void>, + event: Event, + kind: "health" | "lifecycle" + ): void { + for (const listener of listeners) { + try { + listener(structuredClone(event)); + } catch { + process.emitWarning(`MIFTAH_LISTENER_FAILED: ignored a failing ${kind} listener`, { + code: "MIFTAH_LISTENER_FAILED" + }); + } + } } } diff --git a/tests/audit-outcomes.test.ts b/tests/audit-outcomes.test.ts new file mode 100644 index 00000000..5b51cf90 --- /dev/null +++ b/tests/audit-outcomes.test.ts @@ -0,0 +1,583 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { CallToolResultSchema } from "@modelcontextprotocol/sdk/types.js"; +import { mkdtemp, readFile, rm, unlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { AuditTrail } from "../src/audit/audit-trail.js"; +import { validateConfig } from "../src/config/validate-config.js"; +import { MiftahServer } from "../src/mcp/server/miftah-server.js"; +import { ProfileManager } from "../src/profiles/profile-manager.js"; +import { UpstreamProcessManager } from "../src/upstream/upstream-process-manager.js"; + +const fixture = join(dirname(fileURLToPath(import.meta.url)), "fixtures", "fake-upstream.mjs"); + +interface ToolHandler { + handleUpstreamTool( + name: string, + args: Record, + audit: ReturnType, + source: { activeProfile: string; revision: number } + ): Promise; +} + +async function waitForAuditEvent( + path: string, + matches: (event: Record) => boolean, + timeoutMs = 2_000 +): Promise> { + const deadline = Date.now() + timeoutMs; + for (;;) { + const events = (await readFile(path, "utf8")) + .trim() + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line) as Record); + const event = events.find(matches); + if (event) return event; + if (Date.now() >= deadline) throw new Error(`Timed out after ${timeoutMs}ms waiting for a matching audit event`); + await delay(10); + } +} + +describe("audit outcomes", () => { + it("records one terminal operation event for list, management, and unknown-tool requests", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-outcomes-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config, config.security), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await client.listTools(); + await client.callTool({ name: "miftah_use_profile", arguments: { profile: "personal" } }); + expect(await client.callTool({ name: "missing_tool", arguments: {} })).toMatchObject({ + isError: true, + content: [{ type: "text", text: expect.stringContaining("TOOL_NOT_FOUND") }] + }); + + const events = (await readFile(auditPath, "utf8")) + .trim() + .split("\n") + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); + expect(events).toHaveLength(3); + expect(new Set(events.map((event) => event.requestId)).size).toBe(3); + expect(events).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + operation: "tools/list", + name: "tools", + status: "success", + sourceProfile: "work", + profile: "work", + upstream: "default", + sessionId: expect.any(String) + }), + expect.objectContaining({ + operation: "profiles/switch", + name: "personal", + status: "success", + sourceProfile: "work", + profile: "personal", + sessionId: expect.any(String) + }), + expect.objectContaining({ + operation: "tools/call", + name: "missing_tool", + status: "failure", + sourceProfile: "personal", + profile: "personal", + errorCode: "TOOL_NOT_FOUND", + sessionId: expect.any(String) + }) + ]) + ); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("records wrapper and lazy upstream lifecycle outcomes", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-lifecycle-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { work: { env: { TEST_ACCOUNT_NAME: "work" } } }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await client.listTools(); + await client.close(); + await wrapper.close(); + + const lifecycleEvents = (await readFile(auditPath, "utf8")) + .trim() + .split("\n") + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "lifecycle"); + expect(lifecycleEvents).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + operation: "wrapper/start", + name: "accounts", + profile: "work", + status: "success" + }), + expect.objectContaining({ + operation: "upstream/start", + name: "default", + upstream: "default", + profile: "work", + status: "success" + }), + expect.objectContaining({ + operation: "upstream/shutdown", + name: "default", + upstream: "default", + profile: "work", + status: "success" + }), + expect.objectContaining({ + operation: "wrapper/shutdown", + name: "accounts", + profile: "work", + status: "success" + }) + ]) + ); + } finally { + await client.close().catch(() => undefined); + await wrapper.close().catch(() => undefined); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("redacts secret-bearing discovery metadata before it reaches MCP clients", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-discovery-redaction-")); + const auditPath = join(directory, "audit.jsonl"); + const secret = "discovery-output-secret"; + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { + env: { + TEST_ACCOUNT_NAME: "work", + API_TOKEN: secret, + TEST_INCLUDE_DISCOVERY_TOKEN: "true" + } + } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + const tools = await client.listTools(); + const resources = await client.listResources(); + const prompts = await client.listPrompts(); + + const clientOutput = JSON.stringify({ tools, resources, prompts }); + expect(clientOutput).not.toContain(secret); + expect(clientOutput).toContain("[REDACTED]"); + expect(await readFile(auditPath, "utf8")).not.toContain(secret); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("redacts discovery failures from client errors, health, and audit output", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-discovery-failure-")); + const auditPath = join(directory, "audit.jsonl"); + const secret = "discovery-error-secret"; + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { + env: { + TEST_ACCOUNT_NAME: "work", + API_TOKEN: secret, + TEST_FAIL_LIST_RESOURCES: "true" + } + } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await expect(client.listResources()).rejects.toThrow(/\[REDACTED\]/); + await expect(client.listResources()).rejects.not.toThrow(secret); + + const health = await client.callTool({ name: "miftah_health", arguments: {} }); + const output = JSON.stringify(health); + expect(output).not.toContain(secret); + expect(output).toContain("[REDACTED]"); + expect(await readFile(auditPath, "utf8")).not.toContain(secret); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("keeps MCP results available and exposes audit health when fail-open writes fail", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-mcp-fail-open-")); + const blockingPath = join(directory, "not-a-directory"); + await writeFile(blockingPath, "file"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { work: { env: { TEST_ACCOUNT_NAME: "work" } } }, + audit: { path: join(blockingPath, "audit.jsonl"), failureMode: "fail-open" } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + expect((await client.listTools()).tools.map((tool) => tool.name)).toContain("whoami"); + const health = CallToolResultSchema.parse( + await client.callTool({ name: "miftah_health", arguments: {} }, CallToolResultSchema) + ); + const content = health.content[0]; + if (content?.type !== "text") throw new Error("Expected a text health result"); + expect(JSON.parse(content.text)).toMatchObject({ + audit: { state: "failed", lastFailure: { errorCode: "AUDIT_WRITE_FAILED" } } + }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("fails closed with a stable error when an MCP audit write fails", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-mcp-fail-closed-")); + const blockingPath = join(directory, "not-a-directory"); + await writeFile(blockingPath, "file"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { work: { env: { TEST_ACCOUNT_NAME: "work" } } }, + audit: { path: join(blockingPath, "audit.jsonl"), failureMode: "fail-closed" } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await expect(client.listTools()).rejects.toThrow(/AUDIT_WRITE_FAILED/); + expect(await client.callTool({ name: "miftah_health", arguments: {} })).toMatchObject({ + isError: true, + content: [{ type: "text", text: expect.stringContaining("AUDIT_WRITE_FAILED") }] + }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("does not mutate profile state when a fail-closed audit sink is unavailable", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-mcp-preflight-")); + const blockingPath = join(directory, "not-a-directory"); + await writeFile(blockingPath, "file"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + }, + audit: { path: join(blockingPath, "audit.jsonl"), failureMode: "fail-closed" } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const profiles = new ProfileManager(config); + const wrapper = new MiftahServer(config, profiles, manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + expect(await client.callTool({ name: "miftah_use_profile", arguments: { profile: "personal" } })).toMatchObject({ + isError: true, + content: [{ type: "text", text: expect.stringContaining("AUDIT_WRITE_FAILED") }] + }); + expect(profiles.current().activeProfile).toBe("work"); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("records upstream crash and automatic recovery outcomes", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-recovery-")); + const auditPath = join(directory, "audit.jsonl"); + const crashPath = join(directory, "crash"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { + env: { + TEST_ACCOUNT_NAME: "work", + TEST_CRASH_ON_CALL_TOOL_PATH: crashPath + } + } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { + startupTimeoutMs: 1_000, + restartOnCrash: true, + maxRestarts: 2 + }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await client.listTools(); + await writeFile(crashPath, "crash"); + const crashedRequest = client.callTool({ name: "whoami", arguments: {} }).catch(() => undefined); + + await waitForAuditEvent(auditPath, (event) => event.operation === "upstream/crash" && event.status === "failure"); + await unlink(crashPath); + await waitForAuditEvent(auditPath, (event) => event.operation === "upstream/restart" && event.status === "success"); + await crashedRequest; + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("records blocked management profile switches as denied outcomes", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-switch-denied-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + }, + security: { allowProfileSwitchingFromMcp: false }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config, config.security), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + expect(await client.callTool({ name: "miftah_use_profile", arguments: { profile: "personal" } })).toMatchObject({ + isError: true, + content: [{ type: "text", text: expect.stringContaining("PROFILE_SWITCH_DISABLED") }] + }); + expect(await waitForAuditEvent( + auditPath, + (event) => event.operation === "profiles/switch" && event.errorCode === "PROFILE_SWITCH_DISABLED" + )).toMatchObject({ status: "denied" }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("records a static profile lock as wrapper startup metadata", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-lock-metadata-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + }, + security: { lockToProfile: "work" }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config, config.security), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + expect(await waitForAuditEvent( + auditPath, + (event) => event.operation === "wrapper/start" && event.kind === "lifecycle" + )).toMatchObject({ lockToProfile: "work" }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("keeps a tool call on the profile captured before an intervening switch", async () => { + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const profiles = new ProfileManager(config); + const wrapper = new MiftahServer(config, profiles, manager); + const source = profiles.current(); + profiles.switch("personal"); + const audit = new AuditTrail("accounts").beginOperation({ + operation: "tools/call", + name: "whoami", + sourceProfile: source.activeProfile + }); + + try { + expect( + await (wrapper as unknown as ToolHandler).handleUpstreamTool("whoami", {}, audit, source) + ).toMatchObject({ content: [{ type: "text", text: "work" }] }); + } finally { + await wrapper.close(); + } + }); + + it("records the inspected profile for profile-info operations", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-profile-info-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { env: { TEST_ACCOUNT_NAME: "work" } }, + personal: { env: { TEST_ACCOUNT_NAME: "personal" } } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + await client.callTool({ name: "miftah_profile_info", arguments: { profile: "personal" } }); + expect(await waitForAuditEvent( + auditPath, + (event) => event.operation === "management/profile-info" && event.name === "personal" + )).toMatchObject({ sourceProfile: "work", profile: "personal" }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("records upstream error results as failed tool operations", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-tool-result-error-")); + const auditPath = join(directory, "audit.jsonl"); + const config = validateConfig({ + version: "1", + name: "accounts", + defaultProfile: "work", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + work: { + env: { + TEST_ACCOUNT_NAME: "work", + TEST_RETURN_CALL_TOOL_ERROR: "true" + } + } + }, + audit: { path: auditPath } + }); + const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 1_000 }); + const wrapper = new MiftahServer(config, new ProfileManager(config), manager); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "test-client", version: "1.0.0" }); + + try { + await Promise.all([wrapper.connect(serverTransport), client.connect(clientTransport)]); + expect(await client.callTool({ name: "whoami", arguments: {} })).toMatchObject({ isError: true }); + expect(await waitForAuditEvent( + auditPath, + (event) => event.operation === "tools/call" && event.name === "whoami" + )).toMatchObject({ status: "failure", errorCode: "UPSTREAM_CALL_FAILED" }); + } finally { + await client.close(); + await wrapper.close(); + await rm(directory, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/audit.test.ts b/tests/audit.test.ts index 5b2b6b29..e1bf701f 100644 --- a/tests/audit.test.ts +++ b/tests/audit.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, stat, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readFile, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -30,6 +30,29 @@ describe("audit logger", () => { }); }); + it("redacts credential-bearing URI arguments when argument logging is enabled", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-uri-arguments-")); + const path = join(directory, "audit.jsonl"); + const secret = "audit-uri-secret"; + const logger = new AuditLogger(path, { includeArguments: true }); + + await logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "open_callback", + status: "success", + durationMs: 4, + arguments: { callbackUrl: `https://example.test/callback?access_token=${secret}` } + }); + + const line = await readFile(path, "utf8"); + expect(line).not.toContain(secret); + expect(JSON.parse(line)).toMatchObject({ + arguments: { callbackUrl: "https://example.test/callback?access_token=%5BREDACTED%5D" } + }); + }); + it.skipIf(process.platform === "win32")("creates audit directories and files with owner-only permissions", async () => { const root = await mkdtemp(join(tmpdir(), "miftah-audit-permissions-")); const directory = join(root, "private"); @@ -49,6 +72,27 @@ describe("audit logger", () => { expect((await stat(path)).mode & 0o077).toBe(0); }); + it.skipIf(process.platform === "win32")("does not tighten an existing audit parent directory", async () => { + const root = await mkdtemp(join(tmpdir(), "miftah-audit-existing-parent-")); + const directory = join(root, "shared"); + const path = join(directory, "audit.jsonl"); + await mkdir(directory, { mode: 0o755 }); + await chmod(directory, 0o755); + const logger = new AuditLogger(path); + + await logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4 + }); + + expect((await stat(directory)).mode & 0o777).toBe(0o755); + expect((await stat(path)).mode & 0o077).toBe(0); + }); + it("keeps the operation result available when fail-open audit writing fails", async () => { const root = await mkdtemp(join(tmpdir(), "miftah-audit-fail-open-")); const blockingPath = join(root, "not-a-directory"); diff --git a/tests/config-runtime-parity.test.ts b/tests/config-runtime-parity.test.ts index 4fce41b3..3059b493 100644 --- a/tests/config-runtime-parity.test.ts +++ b/tests/config-runtime-parity.test.ts @@ -136,7 +136,14 @@ describe("config runtime parity", () => { lockToProfile: null }, process: { startupTimeoutMs: 1_000 }, - audit: { enabled: true, path: "audit.jsonl", format: "jsonl", includeArguments: false, redact: true }, + audit: { + enabled: true, + path: "audit.jsonl", + format: "jsonl", + includeArguments: false, + redact: true, + failureMode: "fail-open" + }, tooling: { collisionStrategy: "prefix-upstream", toolRiskOverrides: { write_tool: "write" } }, secrets: { envFiles: [".env"], allowPlaintextSecrets: false } }); @@ -145,6 +152,7 @@ describe("config runtime parity", () => { expect(config.process?.startupTimeoutMs).toBe(1_000); expect(config.security?.redactSecrets).toBe(true); expect(config.audit?.redact).toBe(true); + expect(config.audit?.failureMode).toBe("fail-open"); }); it("accepts implemented lifecycle controls", () => { diff --git a/tests/config-schema-contract.test.ts b/tests/config-schema-contract.test.ts index 41b87dde..fef53814 100644 --- a/tests/config-schema-contract.test.ts +++ b/tests/config-schema-contract.test.ts @@ -111,7 +111,11 @@ describe("published config schema", () => { ]); expect(security).toMatchObject({ redactSecrets: { const: true } }); expect(security).not.toHaveProperty("requireProfileSwitchConfirmation"); - expect(audit).toMatchObject({ format: { const: "jsonl" }, redact: { const: true } }); + expect(audit).toMatchObject({ + format: { const: "jsonl" }, + redact: { const: true }, + failureMode: { enum: ["fail-open", "fail-closed"] } + }); expect(tooling).not.toHaveProperty("managementToolPrefix"); expect(tooling).not.toHaveProperty("upstreamToolNamespace"); expect(tooling).toMatchObject({ toolDiscoveryMode: { enum: ["permissive", "strict"] } }); diff --git a/tests/fixtures/fake-upstream.mjs b/tests/fixtures/fake-upstream.mjs index 20f0ab82..5f9abe10 100644 --- a/tests/fixtures/fake-upstream.mjs +++ b/tests/fixtures/fake-upstream.mjs @@ -146,7 +146,10 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { tools: [ { name: "whoami", - description: "Return the injected account.", + description: + process.env.TEST_INCLUDE_DISCOVERY_TOKEN === "true" + ? `Return the injected account ${process.env.API_TOKEN}` + : "Return the injected account.", inputSchema: whoamiInputSchema }, { @@ -200,6 +203,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { if (process.env.TEST_FAIL_CALL_TOOL === "true") { throw new Error(`test tool call failure: ${process.env.API_TOKEN}`); } + if (process.env.TEST_RETURN_CALL_TOOL_ERROR === "true") { + return { content: [{ type: "text", text: "test tool returned an error result" }], isError: true }; + } if (request.params.name === "whoami") { return { content: [{ type: "text", text: account }] }; } diff --git a/tests/mcp-wrapper.test.ts b/tests/mcp-wrapper.test.ts index 7064da73..5fe745f0 100644 --- a/tests/mcp-wrapper.test.ts +++ b/tests/mcp-wrapper.test.ts @@ -210,6 +210,7 @@ describe("Miftah MCP wrapper", () => { it("retries tool discovery when the active profile changes during listing", async () => { const directory = await mkdtemp(join(tmpdir(), "miftah-tool-list-race-")); const startedPath = join(directory, "tools-list-started"); + const auditPath = join(directory, "audit.jsonl"); const config = validateConfig({ version: "1", name: "accounts", @@ -229,7 +230,8 @@ describe("Miftah MCP wrapper", () => { TEST_WHOAMI_SCHEMA: "account" } } - } + }, + audit: { path: auditPath } }); const manager = new UpstreamProcessManager(config.upstream!, config.profiles, { startupTimeoutMs: 5_000 }); const wrapper = new MiftahServer(config, new ProfileManager(config), manager); @@ -259,6 +261,14 @@ describe("Miftah MCP wrapper", () => { required: ["account"] } }); + const events = (await readFile(auditPath, "utf8")) + .trim() + .split("\n") + .map((line) => JSON.parse(line) as Record); + expect(events.find((event) => event.kind === "operation" && event.operation === "tools/list")).toMatchObject({ + sourceProfile: "work", + profile: "personal" + }); } finally { await client.close(); await wrapper.close(); diff --git a/tests/operation-pipeline.test.ts b/tests/operation-pipeline.test.ts index 1009c59e..02fd069e 100644 --- a/tests/operation-pipeline.test.ts +++ b/tests/operation-pipeline.test.ts @@ -63,13 +63,14 @@ describe("operation pipeline", () => { const events = (await readFile(auditPath, "utf8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as Record); + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); expect(events).toHaveLength(3); expect(events).toEqual( expect.arrayContaining([ - expect.objectContaining({ operation: "tools/call", status: "blocked", policyDecision: "confirm" }), - expect.objectContaining({ operation: "resources/read", status: "blocked", policyDecision: "confirm" }), - expect.objectContaining({ operation: "prompts/get", status: "blocked", policyDecision: "confirm" }) + expect.objectContaining({ operation: "tools/call", status: "confirmation-required", policyDecision: "confirm" }), + expect.objectContaining({ operation: "resources/read", status: "confirmation-required", policyDecision: "confirm" }), + expect.objectContaining({ operation: "prompts/get", status: "confirmation-required", policyDecision: "confirm" }) ]) ); } finally { @@ -114,25 +115,26 @@ describe("operation pipeline", () => { const events = (await readFile(auditPath, "utf8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as Record); + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); expect(events).toHaveLength(3); expect(events).toEqual( expect.arrayContaining([ expect.objectContaining({ operation: "tools/call", - status: "blocked", + status: "denied", policyDecision: "deny", routingReason: "active-profile" }), expect.objectContaining({ operation: "resources/read", - status: "blocked", + status: "denied", policyDecision: "deny", routingReason: "active-profile" }), expect.objectContaining({ operation: "prompts/get", - status: "blocked", + status: "denied", policyDecision: "deny", routingReason: "active-profile" }) @@ -211,13 +213,14 @@ describe("operation pipeline", () => { const events = (await readFile(auditPath, "utf8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as Record); + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); expect(events).toHaveLength(3); expect(events).toEqual( expect.arrayContaining([ - expect.objectContaining({ operation: "tools/call", status: "failure", errorCode: "ROUTING_AMBIGUOUS" }), - expect.objectContaining({ operation: "resources/read", status: "failure", errorCode: "ROUTING_AMBIGUOUS" }), - expect.objectContaining({ operation: "prompts/get", status: "failure", errorCode: "ROUTING_AMBIGUOUS" }) + expect.objectContaining({ operation: "tools/call", status: "ambiguous", errorCode: "ROUTING_AMBIGUOUS" }), + expect.objectContaining({ operation: "resources/read", status: "ambiguous", errorCode: "ROUTING_AMBIGUOUS" }), + expect.objectContaining({ operation: "prompts/get", status: "ambiguous", errorCode: "ROUTING_AMBIGUOUS" }) ]) ); } finally { @@ -291,7 +294,8 @@ describe("operation pipeline", () => { const events = (await readFile(auditPath, "utf8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as Record); + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); expect(events).toHaveLength(3); expect(events).toEqual( expect.arrayContaining([ @@ -300,6 +304,7 @@ describe("operation pipeline", () => { name: "whoami", status: "success", profile: "work", + upstream: "default", routingReason: "active-profile", policyDecision: "allow", risk: "read", @@ -310,6 +315,7 @@ describe("operation pipeline", () => { name: "account://current", status: "success", profile: "work", + upstream: "default", routingReason: "active-profile", policyDecision: "allow", risk: "read", @@ -320,6 +326,7 @@ describe("operation pipeline", () => { name: "account_prompt", status: "success", profile: "work", + upstream: "default", routingReason: "active-profile", policyDecision: "allow", risk: "read", @@ -417,7 +424,8 @@ describe("operation pipeline", () => { const events = (await readFile(auditPath, "utf8")) .trim() .split("\n") - .map((line) => JSON.parse(line) as Record); + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "operation"); expect(events).toHaveLength(3); expect(events).toEqual( expect.arrayContaining([ diff --git a/tests/upstream-manager.test.ts b/tests/upstream-manager.test.ts index c06d3777..553be5db 100644 --- a/tests/upstream-manager.test.ts +++ b/tests/upstream-manager.test.ts @@ -35,6 +35,92 @@ async function waitFor( } describe("upstream process manager", () => { + it("isolates lifecycle listener failures from upstream state transitions", async () => { + const manager = new UpstreamProcessManager( + { + transport: "stdio", + command: process.execPath, + args: [fixture] + }, + { + work: { env: { TEST_ACCOUNT_NAME: "work" } } + }, + { startupTimeoutMs: 1_000 } + ); + const emitWarning = vi.spyOn(process, "emitWarning").mockImplementation(() => undefined); + manager.addLifecycleListener((event) => { + if (event.type === "start") throw new Error("listener failure"); + }); + + try { + await expect(manager.get("work")).resolves.toMatchObject({ profile: "work" }); + expect(manager.listHealth()).toMatchObject([{ profile: "work", processState: "running" }]); + expect(emitWarning).toHaveBeenCalledWith("MIFTAH_LISTENER_FAILED: ignored a failing lifecycle listener", { + code: "MIFTAH_LISTENER_FAILED" + }); + } finally { + emitWarning.mockRestore(); + await manager.close().catch(() => undefined); + } + }); + + it("isolates multi-upstream lifecycle listeners from each other's mutations", async () => { + const manager = new MultiUpstreamProcessManager({ + version: "1", + name: "bundle", + defaultProfile: "work", + upstreams: { + github: { transport: "stdio", command: process.execPath, args: [fixture] } + }, + profiles: { work: {} } + }); + const received: Array<{ type: string; status: string }> = []; + manager.addLifecycleListener((event) => { + event.status = "failure"; + }); + manager.addLifecycleListener((event) => { + received.push(event); + }); + + try { + await manager.get("work", "github"); + expect(received).toEqual(expect.arrayContaining([expect.objectContaining({ type: "start", status: "success" })])); + } finally { + await manager.close(); + } + }); + + it("continues multi-upstream lifecycle delivery after a listener fails", async () => { + const manager = new MultiUpstreamProcessManager({ + version: "1", + name: "bundle", + defaultProfile: "work", + upstreams: { + github: { transport: "stdio", command: process.execPath, args: [fixture] } + }, + profiles: { work: {} } + }); + const emitWarning = vi.spyOn(process, "emitWarning").mockImplementation(() => undefined); + const received: Array<{ type: string; status: string }> = []; + manager.addLifecycleListener((event) => { + if (event.type === "start") throw new Error("listener failure"); + }); + manager.addLifecycleListener((event) => { + received.push(event); + }); + + try { + await manager.get("work", "github"); + expect(received).toEqual(expect.arrayContaining([expect.objectContaining({ type: "start", status: "success" })])); + expect(emitWarning).toHaveBeenCalledWith("MIFTAH_LISTENER_FAILED: ignored a failing lifecycle listener", { + code: "MIFTAH_LISTENER_FAILED" + }); + } finally { + emitWarning.mockRestore(); + await manager.close(); + } + }); + it("starts one cached upstream per profile and forwards MCP operations", async () => { const manager = new UpstreamProcessManager( { @@ -600,6 +686,38 @@ describe("upstream process manager", () => { } }); + it("records a failed restart teardown before starting a replacement session", async () => { + const manager = new UpstreamProcessManager( + { + transport: "stdio", + command: process.execPath, + args: [fixture], + env: { TEST_SHUTDOWN_DELAY_MS: "500" } + }, + { work: {} }, + { startupTimeoutMs: 1_000, shutdownTimeoutMs: 50 } + ); + const events: Array<{ type: string; status: string; errorCode?: string }> = []; + manager.addLifecycleListener((event) => events.push(event)); + + try { + await manager.get("work"); + await manager.restart("work"); + expect(events).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: "restart-failure", + status: "failure", + errorCode: "UPSTREAM_SHUTDOWN_TIMEOUT" + }), + expect.objectContaining({ type: "restart", status: "success" }) + ]) + ); + } finally { + await manager.close(); + } + }); + it("releases capacity after a session close rejects", async () => { const manager = new UpstreamProcessManager( { From 3395a59c051950a737f8c7c752925c3a91ad6a4b Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Sat, 11 Jul 2026 11:09:27 +0400 Subject: [PATCH 4/5] fix(audit): harden streamed redaction Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- docs/config.md | 2 +- docs/security.md | 2 +- src/audit/audit-logger.ts | 3 +- src/secrets/redact.ts | 140 ++++++++++++++++++++++++++++++-------- tests/audit.test.ts | 63 ++++++++++++++++- tests/secrets.test.ts | 108 +++++++++++++++++++++++++++++ 7 files changed, 285 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 916d742f..34000a24 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,7 @@ Use `miftah doctor` to inspect config and upstream readiness without printing pr Set `audit.path` to record one terminal JSONL event for every supported MCP operation, including discovery, management, tool, resource, and prompt requests. Events include a per-process session ID, request/event ID, source and selected profiles, upstream, routing and policy metadata where applicable, terminal outcome, stable error code, and duration. Wrapper and upstream lifecycle transitions are recorded separately. Arguments are omitted unless `audit.includeArguments` is `true`. -New audit directories and files use owner-only permissions where the platform supports them. `audit.failureMode` defaults to `"fail-closed"`, which verifies the audit sink before dispatch and refuses the request if it cannot be prepared. Set it to `"fail-open"` only when availability outweighs that guarantee; the original operation remains available and `miftah_health` reports a redacted `AUDIT_WRITE_FAILED` audit-health entry. +New audit directories and files use owner-only permissions where the platform supports them. `audit.failureMode` defaults to `"fail-closed"`, which verifies the audit sink before dispatch and refuses the request if it cannot be prepared. A terminal write can still fail after an upstream side effect completes, so treat a post-dispatch `AUDIT_WRITE_FAILED` as an indeterminate outcome and do not blindly retry non-idempotent tools. Set it to `"fail-open"` only when availability outweighs that guarantee; the original operation remains available and `miftah_health` reports a redacted `AUDIT_WRITE_FAILED` audit-health entry. ## CLI diff --git a/docs/config.md b/docs/config.md index 832a3646..d7aa7877 100644 --- a/docs/config.md +++ b/docs/config.md @@ -48,7 +48,7 @@ Policies classify these operation names as `read`, `write`, or `destructive` usi Audit logging writes local JSONL when a path is configured. Every supported MCP request emits one terminal operation event with a request ID, per-process session ID, source/selected profiles, stable outcome/error code, duration, and any available upstream, routing, policy, and risk metadata; wrapper and upstream lifecycle transitions emit separate event records. Arguments are excluded unless `includeArguments` is true, and all configured secret values are redacted before writing. Audit directories and files are created with owner-only permissions where the platform supports them. -`audit.failureMode` accepts `"fail-closed"` (the default) or `"fail-open"`. Fail-closed verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared; a terminal write error also surfaces as `AUDIT_WRITE_FAILED`. Fail-open preserves the request result and exposes a redacted `AUDIT_WRITE_FAILED` entry through `miftah_health`; it should be used only when availability is more important than complete auditability. +`audit.failureMode` accepts `"fail-closed"` (the default) or `"fail-open"`. Fail-closed verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared; a terminal write error also surfaces as `AUDIT_WRITE_FAILED`. Because terminal writes occur after an upstream operation, that error can leave a non-idempotent operation's outcome indeterminate; do not blindly retry it. Fail-open preserves the request result and exposes a redacted `AUDIT_WRITE_FAILED` entry through `miftah_health`; it should be used only when availability is more important than complete auditability. ## Runtime-supported controls diff --git a/docs/security.md b/docs/security.md index 3b538c54..2c99c017 100644 --- a/docs/security.md +++ b/docs/security.md @@ -12,7 +12,7 @@ Miftah is a credential broker, so safe defaults are part of the product contract - audit files and directories are owner-only where platform support permits it, and audit-write failures are explicit; - provider tokens should be separate, least-privilege tokens per account and risk level. -Audit writes default to fail-closed: Miftah verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared. An operator can set `audit.failureMode` to `"fail-open"` for availability-sensitive deployments; Miftah then preserves the request outcome but exposes a redacted `AUDIT_WRITE_FAILED` health entry. This mode trades complete auditability for availability. +Audit writes default to fail-closed: Miftah verifies the configured sink before dispatch and refuses a request when the sink cannot be prepared. A terminal write can fail after an upstream side effect has completed, so a post-dispatch `AUDIT_WRITE_FAILED` has an indeterminate outcome and must not prompt a blind retry of a non-idempotent operation. An operator can set `audit.failureMode` to `"fail-open"` for availability-sensitive deployments; Miftah then preserves the request outcome but exposes a redacted `AUDIT_WRITE_FAILED` health entry. This mode trades complete auditability for availability. Miftah cannot reduce privileges granted by a provider token. A read-only Miftah policy is a local blocklist, not a replacement for provider-side scopes. Avoid putting real credentials in examples, commits, or support logs. diff --git a/src/audit/audit-logger.ts b/src/audit/audit-logger.ts index 4426a02a..6c5347e1 100644 --- a/src/audit/audit-logger.ts +++ b/src/audit/audit-logger.ts @@ -27,13 +27,14 @@ export class AuditLogger { } async log(event: AuditEvent): Promise { + const timestamp = new Date().toISOString(); const safeEvent = this.redactor.redactForAudit( !this.options.includeArguments ? { ...event, arguments: undefined } : event ); try { - await this.enqueue(() => this.writeLine(`${JSON.stringify({ timestamp: new Date().toISOString(), ...safeEvent })}\n`)); + await this.enqueue(() => this.writeLine(`${JSON.stringify({ timestamp, ...safeEvent })}\n`)); this.lastFailure = undefined; } catch (error) { const failure = this.recordFailure(error); diff --git a/src/secrets/redact.ts b/src/secrets/redact.ts index d8c86d5a..6acebace 100644 --- a/src/secrets/redact.ts +++ b/src/secrets/redact.ts @@ -2,6 +2,10 @@ import { createHmac, randomUUID } from "node:crypto"; const bearerPattern = /(Bearer\s+)[A-Za-z0-9._~+/=-]+/gi; const uriInTextPattern = /\b[a-z][a-z0-9+.-]*:\/\/[^\s"'<>]+/gi; +const maximumBufferedLineLength = 8_192; +const maximumPendingStreamLength = maximumBufferedLineLength * 2; +const redactedStreamLineMarker = "[REDACTED STREAM LINE]"; +const redactedStreamMarker = "[REDACTED STREAM]"; const providerTokenPatterns = [ /\bgh[pousr]_[A-Za-z0-9]{20,}\b/g, /\bgithub_pat_[A-Za-z0-9_]{20,}\b/g @@ -34,10 +38,23 @@ function isSecretKey(key: string): boolean { return (parts.includes("api") && parts.includes("key")) || (parts.includes("private") && parts.includes("key")); } +type UnfinishedSecretPredicate = (secret: string, value: string) => boolean; + /** Redacts configured values and recognized credential formats from text. */ -function redactString(value: string, secretValues: readonly string[]): string { +function redactString(value: string, secretValues: readonly string[]): string; +function redactString( + value: string, + secretValues: readonly string[], + hasUnfinishedSecret: UnfinishedSecretPredicate +): string | undefined; +function redactString( + value: string, + secretValues: readonly string[], + hasUnfinishedSecret?: UnfinishedSecretPredicate +): string | undefined { let result = value; for (const secret of secretValues) { + if (hasUnfinishedSecret?.(secret, value)) return undefined; if (secret.length > 0) { result = result.split(secret).join("[REDACTED]"); } @@ -49,6 +66,14 @@ function redactString(value: string, secretValues: readonly string[]): string { return result; } +function hasProperSecretPrefix(value: string, secret: string): boolean { + const maximumPrefixLength = Math.min(secret.length - 1, value.length); + for (let length = maximumPrefixLength; length > 0; length -= 1) { + if (value.endsWith(secret.slice(0, length))) return true; + } + return false; +} + /** Recursively redacts secrets while preserving the input's data shape. */ function redactValue(value: unknown, secretValues: readonly string[], key?: string, redactUris = false): unknown { if (key && isSecretKey(key)) { @@ -74,13 +99,19 @@ function redactValue(value: unknown, secretValues: readonly string[], key?: stri /** Shares a mutable set of known secret values across runtime output boundaries. */ export class SecretRedactor { private readonly secretValues = new Set(); + private secretSnapshot: readonly string[] = []; + private secretSnapshotDirty = true; + private maximumSecretLength = 0; constructor(secretValues: readonly string[] = []) { this.addAll(secretValues); } add(value: string): void { - if (value.length > 0) this.secretValues.add(value); + if (value.length === 0 || this.secretValues.has(value)) return; + this.secretValues.add(value); + this.secretSnapshotDirty = true; + this.maximumSecretLength = Math.max(this.maximumSecretLength, value.length); } addAll(values: readonly string[]): void { @@ -88,16 +119,16 @@ export class SecretRedactor { } values(): string[] { - return [...this.secretValues]; + return [...this.secretList()]; } redact(value: T): T { - return redactValue(value, this.values()) as T; + return redactValue(value, this.secretList()) as T; } /** Redacts structured audit values, including URI credentials embedded in arbitrary string arguments. */ redactForAudit(value: T): T { - return redactValue(value, this.values(), undefined, true) as T; + return redactValue(value, this.secretList(), undefined, true) as T; } redactText(value: string): string { @@ -110,41 +141,90 @@ export class SecretRedactor { createTextStream(): { write(value: string): string; flush(): string } { let pending = ""; + let activeLine = ""; + let suppressingOutput = false; + + const suppress = (marker: string): string => { + pending = ""; + activeLine = ""; + suppressingOutput = true; + return `${marker}\n`; + }; + + const emitPending = (): string => { + if (pending.length === 0) return ""; + const output = this.redactPending(pending); + if (output === undefined) return ""; + pending = ""; + return output; + }; + return { write: (value) => { - pending += value; - const lastLineBreak = pending.lastIndexOf("\n"); - if (lastLineBreak >= 0) { - const completeLines = pending.slice(0, lastLineBreak + 1); - pending = pending.slice(lastLineBreak + 1); - return this.redactText(completeLines); + if (suppressingOutput) return ""; + if (this.maximumSecretLength > maximumBufferedLineLength) return suppress(redactedStreamMarker); + + let offset = 0; + while (offset < value.length) { + const lineBreak = value.indexOf("\n", offset); + const hasLineBreak = lineBreak >= 0; + const end = hasLineBreak ? lineBreak : value.length; + const completeLineLength = activeLine.length + (end - offset) + (hasLineBreak ? 1 : 0); + if (completeLineLength > maximumBufferedLineLength) { + return suppress(redactedStreamLineMarker); + } + + activeLine += value.slice(offset, end + (hasLineBreak ? 1 : 0)); + if (!hasLineBreak) break; + const completedLine = redactUrisInText(activeLine); + if (pending.length + completedLine.length > maximumPendingStreamLength) { + return suppress(redactedStreamMarker); + } + pending += completedLine; + activeLine = ""; + offset = lineBreak + 1; } - const retainedLength = Math.max(1_024, ...[...this.secretValues].map((secret) => secret.length)); - if (pending.length <= retainedLength) return ""; - const requestedBoundary = pending.length - retainedLength; - const boundary = this.safeTextBoundary(pending, requestedBoundary); - const completeText = pending.slice(0, boundary); - pending = pending.slice(boundary); - return this.redactText(completeText); + + const output = emitPending(); + return output; }, flush: () => { - const completeText = this.redactText(pending); - pending = ""; - return completeText; + if (suppressingOutput) { + pending = ""; + activeLine = ""; + return ""; + } + if (this.maximumSecretLength > maximumBufferedLineLength) { + return pending.length > 0 || activeLine.length > 0 ? suppress(redactedStreamMarker) : ""; + } + if (activeLine.length > 0) { + const completedLine = redactUrisInText(activeLine); + if (pending.length + completedLine.length > maximumPendingStreamLength) { + return suppress(redactedStreamMarker); + } + pending += completedLine; + activeLine = ""; + } + // A truncated known-secret prefix remains sensitive even after stderr closes. + const output = emitPending(); + return pending.length > 0 ? output + suppress(redactedStreamMarker) : output; } }; } - private safeTextBoundary(value: string, requestedBoundary: number): number { - let boundary = requestedBoundary; - for (const secret of this.secretValues) { - let index = value.indexOf(secret); - while (index >= 0) { - if (index < boundary && index + secret.length > boundary) boundary = index; - index = value.indexOf(secret, index + 1); - } + private secretList(): readonly string[] { + if (this.secretSnapshotDirty) { + this.secretSnapshot = [...this.secretValues]; + this.secretSnapshotDirty = false; } - return boundary; + return this.secretSnapshot; + } + + private redactPending(value: string): string | undefined { + const uriSafeValue = redactUrisInText(value); + return redactString(uriSafeValue, this.secretList(), (secret, pendingValue) => + hasProperSecretPrefix(pendingValue, secret) + ); } } diff --git a/tests/audit.test.ts b/tests/audit.test.ts index e1bf701f..b5057149 100644 --- a/tests/audit.test.ts +++ b/tests/audit.test.ts @@ -1,7 +1,7 @@ import { chmod, mkdir, mkdtemp, readFile, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { AuditLogger } from "../src/audit/audit-logger.js"; describe("audit logger", () => { @@ -30,6 +30,67 @@ describe("audit logger", () => { }); }); + it("timestamps an event when logging begins rather than when its queued write runs", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-timestamp-")); + const path = join(directory, "audit.jsonl"); + const logger = new AuditLogger(path); + const loggedAt = new Date("2026-07-11T06:00:00.000Z"); + + vi.useFakeTimers({ toFake: ["Date"] }); + try { + vi.setSystemTime(loggedAt); + const write = logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4 + }); + vi.setSystemTime(new Date("2026-07-11T07:00:00.000Z")); + + await write; + + expect(JSON.parse(await readFile(path, "utf8")).timestamp).toBe(loggedAt.toISOString()); + } finally { + vi.useRealTimers(); + } + }); + + it("timestamps an event before synchronous redaction", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-audit-redaction-timestamp-")); + const path = join(directory, "audit.jsonl"); + const logger = new AuditLogger(path, { includeArguments: true }); + const loggedAt = new Date("2026-07-11T06:00:00.000Z"); + const redactedAt = new Date("2026-07-11T07:00:00.000Z"); + const argumentsWithTimeShift: Record = {}; + Object.defineProperty(argumentsWithTimeShift, "value", { + enumerable: true, + get: () => { + vi.setSystemTime(redactedAt); + return "safe"; + } + }); + + vi.useFakeTimers({ toFake: ["Date"] }); + try { + vi.setSystemTime(loggedAt); + await logger.log({ + wrapper: "github", + profile: "work", + operation: "tools/call", + name: "whoami", + status: "success", + durationMs: 4, + arguments: argumentsWithTimeShift + }); + + expect(JSON.parse(await readFile(path, "utf8")).timestamp).toBe(loggedAt.toISOString()); + } finally { + vi.useRealTimers(); + } + }); + it("redacts credential-bearing URI arguments when argument logging is enabled", async () => { const directory = await mkdtemp(join(tmpdir(), "miftah-audit-uri-arguments-")); const path = join(directory, "audit.jsonl"); diff --git a/tests/secrets.test.ts b/tests/secrets.test.ts index 525d2237..ecdd05f9 100644 --- a/tests/secrets.test.ts +++ b/tests/secrets.test.ts @@ -71,6 +71,114 @@ describe("secret redaction", () => { } }); + it("keeps a long newline-free URI intact until its query values can be redacted", () => { + const stream = new SecretRedactor().createTextStream(); + const secret = "stream-uri-secret"; + const uriStart = `https://example.test/${"a".repeat(1_100)}`; + + expect(stream.write(`prefix ${uriStart}`)).toBe(""); + expect(stream.write(`?access_token=${secret}`)).toBe(""); + + const output = stream.flush(); + expect(output).toContain("prefix "); + expect(output).not.toContain(secret); + expect(output).toContain("access_token=%5BREDACTED%5D"); + }); + + it("redacts a URI whose arbitrary scheme is split across stream writes", () => { + const stream = new SecretRedactor().createTextStream(); + const secret = "split-scheme-uri-secret"; + const scheme = `a${"-".repeat(1_100)}`; + + expect(stream.write(scheme)).toBe(""); + + const output = [ + stream.write(`://user:password@example.test/path?access_token=${secret}`), + stream.flush() + ].join(""); + + expect(output).not.toContain("password"); + expect(output).not.toContain(secret); + expect(output).toContain("access_token=%5BREDACTED%5D"); + }); + + it("bounds a long newline-free stream by redacting the complete line", () => { + const stream = new SecretRedactor().createTextStream(); + const secret = "overlong-uri-secret"; + const uri = `https://example.test/${"a".repeat(8_193)}`; + + const output = [ + stream.write(uri), + stream.write(`?access_token=${secret}`), + stream.write(" end\n"), + stream.flush() + ].join(""); + + expect(output).toBe("[REDACTED STREAM LINE]\n"); + expect(output).not.toContain(secret); + }); + + it("bounds an oversized complete line before forwarding it", () => { + const stream = new SecretRedactor().createTextStream(); + + expect(stream.write(`${"x".repeat(8_193)}\n`)).toBe("[REDACTED STREAM LINE]\n"); + }); + + it("streams stderr with a large configured secret registry", () => { + const redactor = new SecretRedactor( + Array.from({ length: 200_000 }, (_, index) => `configured-secret-${index}`) + ); + const stream = redactor.createTextStream(); + + expect(stream.write("diagnostic\n")).toBe("diagnostic\n"); + }); + + it("redacts a configured multiline secret split across stream writes", () => { + const stream = new SecretRedactor(["alpha\nbeta"]).createTextStream(); + + expect(stream.write("prefix alpha\n")).toBe(""); + expect(stream.write("beta\n")).toBe("prefix [REDACTED]\n"); + }); + + it("redacts a configured secret spanning multiple completed lines", () => { + const stream = new SecretRedactor(["alpha\nbeta\ngamma"]).createTextStream(); + + expect(stream.write("prefix alpha\nbeta\n")).toBe(""); + expect(stream.write("gamma\n")).toBe("prefix [REDACTED]\n"); + }); + + it("uses secrets registered before later stream writes", () => { + const redactor = new SecretRedactor(); + const stream = redactor.createTextStream(); + + expect(stream.write("prefix ")).toBe(""); + redactor.add("dynamic-stream-secret"); + + expect(stream.write("dynamic-stream-secret\n")).toBe("prefix [REDACTED]\n"); + }); + + it("suppresses stderr when a configured secret exceeds the stream cap", () => { + const stream = new SecretRedactor(["s".repeat(8_193)]).createTextStream(); + + expect(stream.write("diagnostic\n")).toBe("[REDACTED STREAM]\n"); + expect(stream.flush()).toBe(""); + }); + + it("bounds repeated incomplete multiline-secret prefixes", () => { + const stream = new SecretRedactor(["a\nb"]).createTextStream(); + + expect(stream.write("a\n".repeat(8_193))).toBe("[REDACTED STREAM]\n"); + expect(stream.flush()).toBe(""); + }); + + it("suppresses a flush that would exceed the pending stream cap", () => { + const stream = new SecretRedactor(["a\nb"]).createTextStream(); + + expect(stream.write("a\n".repeat(8_192))).toBe(""); + expect(stream.write("x".repeat(8_192))).toBe(""); + expect(stream.flush()).toBe("[REDACTED STREAM]\n"); + }); + it("removes URI userinfo, query values, and fragments from public identifiers", () => { expect( redactUri( From 66712f936fe022ea4ee3c1c2e14c9682f3fb1867 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Sat, 11 Jul 2026 11:25:04 +0400 Subject: [PATCH 5/5] test(redaction): cover capped stream boundary Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/secrets/redact.ts | 1 + tests/secrets.test.ts | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/secrets/redact.ts b/src/secrets/redact.ts index 6acebace..75bc1f2c 100644 --- a/src/secrets/redact.ts +++ b/src/secrets/redact.ts @@ -147,6 +147,7 @@ export class SecretRedactor { const suppress = (marker: string): string => { pending = ""; activeLine = ""; + // Dropping an overlong line can lose a multiline-secret prefix, so later stderr stays fail-closed. suppressingOutput = true; return `${marker}\n`; }; diff --git a/tests/secrets.test.ts b/tests/secrets.test.ts index ecdd05f9..2c28774d 100644 --- a/tests/secrets.test.ts +++ b/tests/secrets.test.ts @@ -124,6 +124,13 @@ describe("secret redaction", () => { expect(stream.write(`${"x".repeat(8_193)}\n`)).toBe("[REDACTED STREAM LINE]\n"); }); + it("fails closed after a capped line can contain a multiline secret prefix", () => { + const stream = new SecretRedactor(["abc\ndef"]).createTextStream(); + + expect(stream.write(`${"x".repeat(8_190)}abc`)).toBe("[REDACTED STREAM LINE]\n"); + expect(stream.write("\ndef\n")).toBe(""); + }); + it("streams stderr with a large configured secret registry", () => { const redactor = new SecretRedactor( Array.from({ length: 200_000 }, (_, index) => `configured-secret-${index}`)