diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e0ecb..cd295a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,16 @@ All notable changes to this project will be documented in this file. The format ### Changed +- [#366](https://github.com/mohanagy/miftah/issues/366) Completed the capability-gated MRTR confirmation boundary with packaged modern-client evidence, authenticated profile-context binding, deterministic one-time replay rejection, explicit cancelled audit outcomes, request-scoped upstream cleanup, and documented safe fallbacks. The experimental Tasks extension remains unimplemented after evaluating startup, OAuth, diagnostics, audit export, and recovery; Miftah makes no Tasks interoperability claim. - [#363](https://github.com/mohanagy/miftah/issues/363) Replaced the monolithic MCP TypeScript SDK v1 dependency with the stable v2 split packages and migrated runtime schemas to Zod 4. Runtime consumers receive only `client`, `core`, and `server`; the Node adapter and frozen legacy server remain build/test dependencies. Direct consumers of the old monolithic SDK deep imports must move to the corresponding split package. The CLI bundles the v2 Node adapter with patched `@hono/node-server` and Hono builds so a fresh Miftah install does not inherit the Node package's still-vulnerable 1.x adapter range; custom embedding hosts own their direct Node adapter version. Confirmation-required tools, resources, prompts, and profile transitions now use the v2 `input_required` flow with integrity-bound one-time continuation state across request-scoped modern HTTP instances, while the SDK legacy shim preserves form elicitation for initialized clients. Native OAuth callback completion now carries the authorization-server issuer required by the v2 provider contract; Miftah continues to validate and round-trip that issuer without exposing tokens or client secrets. +### Fixed + +- [#384](https://github.com/mohanagy/miftah/issues/384) Made continuation tamper coverage deterministic by mutating a decoded signature byte and re-encoding it canonically, so every test input changes authenticated bytes rather than possibly changing only unused base64url padding bits. + ### Security +- [#366](https://github.com/mohanagy/miftah/issues/366) Bound form-approval continuations to the separately keyed authenticated request-context correlation in addition to the exact source/selected profiles, upstream, operation, target, and arguments. Cross-chat, cross-principal, cross-profile, mismatched, expired, and replayed state fails closed without disclosing handles, state, input responses, or operation arguments. - [#365](https://github.com/mohanagy/miftah/issues/365) Fails closed on missing, malformed, duplicated, mismatched, or incorrectly encoded modern routing headers before routing, policy, audit, or upstream execution. Because Miftah's low-level proxy cannot yet validate schema-declared parameter headers end to end, modern HTTP strips `x-mcp-header` declarations and rejects every `Mcp-Param-*` header with a fixed non-reflective response; legacy HTTP and STDIO retain their existing argument behavior. Positive-TTL sharing remains disabled so catalogs cannot cross principals or survive profile, policy, upstream, or configuration changes. - [#367](https://github.com/mohanagy/miftah/issues/367) Bound client information, tokens, discovery, and callback completion to the exact configured authorization-server issuer. Missing, duplicated, or mismatched RFC 9207 `iss` callback values fail before code redemption; issuer changes require a fresh connection identity and authorization; fixed errors, callback pages, configuration, and audit output do not expose authorization codes, tokens, client secrets, or raw provider details. diff --git a/docs/config.md b/docs/config.md index ab0e195..fbfe5e4 100644 --- a/docs/config.md +++ b/docs/config.md @@ -349,6 +349,8 @@ The default `security.approvalMode` is `"human"`. Clients that advertise MCP **f Approval lifecycle events record request, approval, denial, expiry, and consumption when audit logging is configured. They contain safe profile/upstream/operation metadata, expiry, and `approvalMechanism` (`"form"` or `"delegated-agent"`) only; they never contain an approval bearer or full operation arguments. +On the modern `2026-07-28` protocol, form-capable clients complete confirmation through `input_required` and retry the exact operation with `inputResponses` plus opaque one-time `requestState`. Miftah additionally binds that state to the authenticated profile-context correlation when a trusted embedding host enables stateless chat-scoped profiles. An input-required round is audited as `confirmation-required`, an aborted operation as `cancelled`, a rejected operation as `failure`, and a completed operation as `success`. See the [MRTR and Tasks decision](plans/2026-08-11-mrtr-tasks-decision.md) for the retry, cancellation, fallback, and Tasks boundaries. + 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; route previews and proxied operations add sanitized `routingEvidence` when a collector snapshot is available and canonical `routingMatcherEvidence` for a static matcher result or ambiguity. 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. Every newly written record carries writer-controlled `schemaVersion: 1`; an event cannot select or override that marker. Miftah continues to read existing unversioned local JSONL records as legacy input. Additive fields may be introduced to schema version 1, but an incompatible audit-record interpretation requires a later minor release, an **Unreleased** changelog entry, and a documented reader or export migration path. When integrity is enabled, the schema marker is inside the chained payload and is therefore tamper-evident. diff --git a/docs/library-api.md b/docs/library-api.md index d3ecadf..af2992f 100644 --- a/docs/library-api.md +++ b/docs/library-api.md @@ -50,6 +50,8 @@ For a custom HTTP host, pass the same factory to `createMcpHandler` from `@model This matrix describes Miftah's tested serving boundary, not a promise that every optional feature added to any future MCP revision is implemented. The SDK v2 serving entry owns protocol-era negotiation; Miftah continues to own broker routing, policy, audit, OAuth, profile state, upstream lifecycle, and cancellation propagation. +Modern form confirmations use the protocol's `input_required` retry flow and bind one-time continuation state to the exact operation and authenticated profile context. The installed-package contract exercises that round trip through the CLI-owned HTTP server. Tasks are intentionally not advertised or implemented; see the [MRTR and Tasks decision](plans/2026-08-11-mrtr-tasks-decision.md). + For modern Streamable HTTP, the serving entry validates `Mcp-Method` and `Mcp-Name` against the parsed JSON-RPC request before constructing Miftah's per-request server. Miftah currently declines the optional `Mcp-Param-*` extension: its CLI-owned modern HTTP catalog strips `x-mcp-header` schema keywords and its ingress rejects parameter headers without reflecting or forwarding them. Modern cacheable results are explicitly private and immediately stale (`ttlMs: 0`); deterministic resource, resource-template, and prompt ordering makes repeated uncached catalogs stable. STDIO and legacy initialized HTTP behavior remain unchanged. Confirmation-required tools, resource reads, prompt reads, and profile transitions return the MCP `input_required` result on the modern era. Miftah binds the continuation to the exact operation with bounded, integrity-protected, one-time state shared by the server factory, so a fresh request-scoped HTTP instance can safely finish the approval without retaining raw operation arguments. The SDK's legacy shim translates the same handler flow into form elicitation for initialized clients. diff --git a/docs/plans/2026-08-11-mrtr-tasks-decision.md b/docs/plans/2026-08-11-mrtr-tasks-decision.md new file mode 100644 index 0000000..9710d19 --- /dev/null +++ b/docs/plans/2026-08-11-mrtr-tasks-decision.md @@ -0,0 +1,67 @@ +# MRTR and Tasks Decision + +Status: Multi Round-Trip Requests are implemented for confirmation workflows. The Tasks extension remains unimplemented until the interoperability and durability gates below are met. + +Issue: [#366](https://github.com/mohanagy/miftah/issues/366) + +Specification sources: [MCP 2026-07-28 announcement](https://blog.modelcontextprotocol.io/posts/2026-07-28/), [SEP-2322 MRTR](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2322), [SEP-2663 Tasks extension](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2663), [Tasks overview](https://modelcontextprotocol.io/extensions/tasks/overview), and the [experimental Tasks repository](https://github.com/modelcontextprotocol/ext-tasks) + +## Decision + +Miftah uses `input_required` for operations that are otherwise ready to run but need one exact human confirmation. This covers policy approval and account/profile transition confirmation. The response embeds a generic form request; the client returns `inputResponses` and the byte-exact opaque `requestState` on a fresh retry. + +Missing setup data and recovery are not converted into MRTR in this release. Setup is a local CLI or Console workflow that may change configuration and credentials, while recovery currently returns bounded diagnostics and an explicit next action. Neither has one resumable MCP operation with a stable authorization boundary. Adding an interactive round merely to gather arbitrary configuration would widen the credential and mutation surface. + +The `io.modelcontextprotocol/tasks` extension remains unimplemented. No evaluated Miftah operation currently clears all selection gates, and the extension is still explicitly experimental. Consequently there is no selected Miftah task and the requirement for a durable task identifier plus authenticated lookup is not applicable yet. Miftah makes no Tasks interoperability claim. + +## MRTR security and lifecycle contract + +- Form MRTR is enabled only when the client declares `elicitation.form`. The embedded request contains only a generic boolean `approved` field and no target arguments, profile-context handle, credential, or continuation secret. +- Continuation state is short-lived, integrity protected, bounded in memory, and bound to the exact source profile, selected profile, upstream, operation, target, normalized arguments, and authenticated request-context correlation when present. +- A mismatched operation, profile, handle, chat, or principal cannot consume another request's state. Accepted state is consumed atomically before upstream work and cannot be replayed. +- A declined, expired, malformed, missing, or already-consumed response fails closed. A rejected mismatch does not consume the legitimate caller's pending continuation. +- The approval check happens before acquiring the selected upstream. Cancelling later work propagates the request signal to that upstream, records `REQUEST_CANCELLED`, releases the request-scoped modern runtime, and does not create or change a profile lock or lease. +- Clients without the declared form capability receive `POLICY_CONFIRMATION_REQUIRED` (or the corresponding profile-confirmation code) and an actionable explanation. No bearer is disclosed in the default human mode. The existing explicitly configured `delegated-agent` fallback remains connection-bound and one-time for legacy automation. + +## Audit outcome vocabulary + +The audit journal distinguishes each workflow state without storing `requestState`, `inputResponses`, profile-context handles, or approval bearers: + +| Workflow meaning | Audit representation | +| --- | --- | +| Incomplete and waiting for input | operation status `confirmation-required`, plus approval action `requested` | +| Cancelled by the caller | operation status `cancelled` with `REQUEST_CANCELLED` | +| Failed or rejected | operation status `failure` with a stable redacted error code | +| Completed | operation status `success`; an approved MRTR also records `approved` then `consumed` | + +Each request round has its own operation record. A confirmation-required first round is therefore complete as a protocol exchange but incomplete as the requested business operation. Every round applies the normal operation-argument policy: arguments are omitted unless `audit.includeArguments` is enabled, and enabled arguments pass through the configured secret redaction before storage. Continuation state, input responses, profile-context handles, and approval bearers remain excluded in either mode. Dedicated approval records never store operation arguments. + +## Tasks evaluation + +| Candidate | Decision | Reason | +| --- | --- | --- | +| Upstream startup | Defer | Startup is lazy and bounded by configured timeouts. A task must not hide a failed child start or keep an orphan process after cancellation. | +| OAuth authorization | Defer | Browser authorization already uses an issuer-bound connection identity and an explicit loopback handoff. A task would require durable encrypted state, authenticated owner lookup, expiry, and a supported client resume flow. | +| Diagnostics and readiness | Defer | `doctor`, `test-profile`, and health checks are bounded status operations with explicit redacted results; converting them would add polling without a demonstrated latency need. | +| Audit export | Defer | Export is a local CLI snapshot to an explicit private path, not a remote MCP operation. Turning it into a task would introduce remote file ownership and download authorization questions. | +| Recovery | Defer | Current recovery returns stable error codes and concrete next commands. No single recovery action is both long-running and safe to resume automatically. | + +Tasks may be reconsidered only when all of these gates are satisfied: + +1. the extension and Miftah's SDK line expose a supported end-to-end server and client contract; +2. a supported real host demonstrates one concrete operation that routinely exceeds a normal request budget; +3. the task store has a durable opaque identifier, authenticated principal/chat/profile ownership, bounded expiry, encrypted sensitive state, idempotent creation, and atomic terminal transitions; +4. `tasks/get`, `tasks/update`, and `tasks/cancel` enforce the same ownership for task lookup and terminal result retrieval, `inputResponses` updates, and cancellation, and survive process or connection loss; +5. cancellation proves no profile lock, lease, OAuth handoff, process reservation, or upstream child is left behind; +6. packaged interoperability tests cover reconnect, duplicate creation, replay, expiry, cancellation, redaction, and audit outcomes. + +## Executable evidence and claim boundary + +- The packed-artifact contract starts the installed `miftah serve --transport http`, connects the supported MCP TypeScript client in the modern era, completes one form round trip, verifies one upstream mutation, and checks `requested`, `approved`, and `consumed` audit actions. +- The stateless profile-context runtime test alternates two authenticated chats against fresh request-scoped servers. A stolen continuation with the other chat's valid handle fails, the original chat succeeds once, and replay fails. +- The modern HTTP cancellation test observes one upstream cancellation notification, request-scoped upstream cleanup, unchanged `none`/`not-required` lock and lease state, and the explicit cancelled audit outcome. +- Source and package tests establish Miftah behavior with the supported TypeScript client. They do not establish Tasks compatibility or compatibility with an untested host. + +## Stop rule + +Do not implement or advertise Tasks because an operation is merely asynchronous or inconvenient. Stop unless a real supported client, a concrete long-running Miftah use case, authenticated durable ownership, cleanup semantics, and packaged reconnect evidence all exist together. diff --git a/src/approvals/approval-store.ts b/src/approvals/approval-store.ts index a10e454..6840402 100644 --- a/src/approvals/approval-store.ts +++ b/src/approvals/approval-store.ts @@ -15,6 +15,8 @@ export interface ApprovalBinding { readonly profile: string; readonly upstream: string; readonly operation: string; + /** Internal keyed correlation for the authenticated request context; never exposed as approval metadata. */ + readonly requestCorrelation?: string; /** The actual target identifier, retained only inside the keyed binding digest. */ readonly name: string; /** A safe target label suitable for management output and audit events. */ @@ -321,6 +323,7 @@ export class ApprovalStore { profile: binding.profile, upstream: binding.upstream, operation: binding.operation, + requestCorrelation: binding.requestCorrelation, name: binding.name, arguments: binding.arguments }) diff --git a/src/audit/audit-types.ts b/src/audit/audit-types.ts index a0ae008..416b705 100644 --- a/src/audit/audit-types.ts +++ b/src/audit/audit-types.ts @@ -42,7 +42,14 @@ export interface AuditHealth { } export type AuditEventKind = "operation" | "lifecycle" | "approval" | "profile"; -export type AuditStatus = "success" | "failure" | "blocked" | "denied" | "confirmation-required" | "ambiguous"; +export type AuditStatus = + | "success" + | "failure" + | "cancelled" + | "blocked" + | "denied" + | "confirmation-required" + | "ambiguous"; export type AuditRoutingSource = "rule" | "matcher" | "active-profile" | "default-profile" | "setup-profile"; export type ApprovalAuditAction = "requested" | "approved" | "denied" | "expired" | "consumed"; export type ProfileAuditAction = diff --git a/src/cli/exit-codes.ts b/src/cli/exit-codes.ts index 87bf35e..f5bccbf 100644 --- a/src/cli/exit-codes.ts +++ b/src/cli/exit-codes.ts @@ -76,6 +76,7 @@ export const ERROR_EXIT_CODES = { UPSTREAM_CALL_FAILED: CLI_EXIT_CODES.upstream, UPSTREAM_HTTP_ERROR: CLI_EXIT_CODES.upstream, UPSTREAM_PROTOCOL_ERROR: CLI_EXIT_CODES.upstream, + REQUEST_CANCELLED: CLI_EXIT_CODES.operation, AUDIT_WRITE_FAILED: CLI_EXIT_CODES.operation, UPSTREAM_SELECTION_AMBIGUOUS: CLI_EXIT_CODES.upstream, ROUTING_AMBIGUOUS: CLI_EXIT_CODES.policy, diff --git a/src/mcp/server/miftah-server.ts b/src/mcp/server/miftah-server.ts index 02fd18b..8e0fb0b 100644 --- a/src/mcp/server/miftah-server.ts +++ b/src/mcp/server/miftah-server.ts @@ -1947,9 +1947,10 @@ export class MiftahServer { profile, upstream: "profiles", operation: `profiles/${action}`, + requestCorrelation: authenticated.auditCorrelation, name: profile, displayName: `profile '${profile}'`, - arguments: { profile, requestCorrelation: authenticated.auditCorrelation } + arguments: { profile } }, context, profileSwitchApprovalErrors @@ -3085,6 +3086,7 @@ export class MiftahServer { /** Maps a safe domain error to its terminal audit outcome without exposing diagnostic detail. */ private auditStatus(error: MiftahError): AuditStatus { + if (error.code === "REQUEST_CANCELLED") return "cancelled"; if ( error.code === "POLICY_BLOCKED" || error.code === "ROUTING_BLOCKED" || diff --git a/src/mcp/server/operation-pipeline.ts b/src/mcp/server/operation-pipeline.ts index efacdac..cfbdc25 100644 --- a/src/mcp/server/operation-pipeline.ts +++ b/src/mcp/server/operation-pipeline.ts @@ -192,6 +192,9 @@ export class OperationPipeline { profile, upstream: target.upstreamName ?? "default", operation: operation.operation, + ...(operation.source.profileContextCorrelation === undefined + ? {} + : { requestCorrelation: operation.source.profileContextCorrelation }), name: target.name, displayName: this.auditName(operation, target.name), arguments: operation.args @@ -214,6 +217,9 @@ export class OperationPipeline { return this.options.redactor.redact(target.redact(await target.execute(session, operation.upstreamRequestOptions))); } catch (error) { if (error instanceof ApprovalInputRequiredSignal) throw error; + if (operation.upstreamRequestOptions?.signal?.aborted || operation.approvalContext?.signal.aborted) { + throw new MiftahError("REQUEST_CANCELLED", "REQUEST_CANCELLED: request was cancelled"); + } const safeError = this.toSafeError(error); const matcherEvidence = matcherEvidenceFromError(safeError); if (matcherEvidence !== undefined) { diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 8dab0b7..3cc233a 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -63,6 +63,7 @@ export type MiftahErrorCode = | "UPSTREAM_CALL_FAILED" | "UPSTREAM_HTTP_ERROR" | "UPSTREAM_PROTOCOL_ERROR" + | "REQUEST_CANCELLED" | "AUDIT_WRITE_FAILED" | "UPSTREAM_SELECTION_AMBIGUOUS" | "ROUTING_AMBIGUOUS" diff --git a/tests/approval-continuation-store.test.ts b/tests/approval-continuation-store.test.ts index 4c50af0..a4542a5 100644 --- a/tests/approval-continuation-store.test.ts +++ b/tests/approval-continuation-store.test.ts @@ -40,8 +40,20 @@ describe("approval continuation store", () => { mechanism: "form" }); expect(() => store.pending(continuation, binding({ name: "changed" }))).toThrow("APPROVAL_INVALID"); - const changedLastCharacter = state.endsWith("A") ? "B" : "A"; - expect(() => store.verify(`${state.slice(0, -1)}${changedLastCharacter}`)).toThrow("APPROVAL_INVALID"); + const [payload, signature] = state.split(".") as [string, string]; + const tamperedSignature = Buffer.from(signature, "base64url"); + tamperedSignature[0] = tamperedSignature[0]! ^ 0x01; + const tamperedState = `${payload}.${tamperedSignature.toString("base64url")}`; + expect(() => store.verify(tamperedState)).toThrow("APPROVAL_INVALID"); + + const principalBound = { ...original, requestCorrelation: "principal-a" }; + const principalState = store.mint(principalBound, approval("approval-principal")); + const principalContinuation = store.verify(principalState); + expect(() => store.pending(principalContinuation, { + ...principalBound, + requestCorrelation: "principal-b" + })).toThrow("APPROVAL_INVALID"); + expect(store.pending(principalContinuation, principalBound).id).toBe("approval-principal"); store.complete(continuation); expect(() => store.verify(state)).toThrow("APPROVAL_INVALID"); diff --git a/tests/cli-exit-codes.test.ts b/tests/cli-exit-codes.test.ts index 530c84b..3ed26b7 100644 --- a/tests/cli-exit-codes.test.ts +++ b/tests/cli-exit-codes.test.ts @@ -70,6 +70,7 @@ const expectedErrorExitCodes: Record = { UPSTREAM_CALL_FAILED: CLI_EXIT_CODES.upstream, UPSTREAM_HTTP_ERROR: CLI_EXIT_CODES.upstream, UPSTREAM_PROTOCOL_ERROR: CLI_EXIT_CODES.upstream, + REQUEST_CANCELLED: CLI_EXIT_CODES.operation, AUDIT_WRITE_FAILED: CLI_EXIT_CODES.operation, UPSTREAM_SELECTION_AMBIGUOUS: CLI_EXIT_CODES.upstream, ROUTING_AMBIGUOUS: CLI_EXIT_CODES.policy, diff --git a/tests/mcp-v2-serving.test.ts b/tests/mcp-v2-serving.test.ts index 872664a..74b5e2a 100644 --- a/tests/mcp-v2-serving.test.ts +++ b/tests/mcp-v2-serving.test.ts @@ -19,12 +19,20 @@ const remoteUpstreams: FakeRemoteUpstream[] = []; afterEach(async () => { await Promise.all(httpServers.splice(0).map((server) => server.close())); await Promise.all(remoteUpstreams.splice(0).map((upstream) => upstream.close())); - await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { + recursive: true, + force: true, + // Keep cleanup bounded while tolerating a transient late filesystem entry + // after the server and upstream close boundaries have completed. + maxRetries: 5, + retryDelay: 50 + }))); }); async function configPath(upstream?: { readonly url?: string; readonly env?: Readonly>; + readonly auditPath?: string; }): Promise { const directory = await mkdtemp(join(tmpdir(), "miftah-v2-serving-")); temporaryDirectories.push(directory); @@ -44,6 +52,7 @@ async function configPath(upstream?: { } : { transport: "streamable-http", url: upstream.url }, profiles: { work: {} }, + ...(upstream?.auditPath === undefined ? {} : { audit: { path: upstream.auditPath } }), server: { http: { port: 0, maxSessions: 4, sessionIdleTimeoutMs: 1_000 } } }) ); @@ -58,6 +67,36 @@ async function waitFor(condition: () => boolean, timeoutMs = 4_000): Promise) => boolean, + timeoutMs = 4_000 +): Promise> { + const deadline = Date.now() + timeoutMs; + for (;;) { + try { + const event = (await readFile(path, "utf8")) + .trim() + .split("\n") + .filter(Boolean) + .flatMap((line) => { + try { + return [JSON.parse(line) as Record]; + } catch { + // A concurrent append can expose a partial trailing JSONL line. + return []; + } + }) + .find(matches); + if (event !== undefined) return event; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + if (Date.now() >= deadline) throw new Error("Timed out waiting for the audit event."); + await delay(25); + } +} + function mutateRequestHeaders( targetMethod: string, mutate: (headers: Headers) => void, @@ -296,10 +335,16 @@ describe("MCP SDK v2 serving interoperability", () => { expect(elicitationRequests).toHaveLength(1); expect(JSON.stringify(elicitationRequests)).not.toContain(secretArgument); expect(await readFile(createCountPath, "utf8")).toBe("1\n"); - const approvalActions = (await readFile(auditPath, "utf8")) + const auditText = await readFile(auditPath, "utf8"); + expect(auditText).not.toContain(secretArgument); + const auditEvents = auditText .trim() .split("\n") - .map((line) => JSON.parse(line) as Record) + .map((line) => JSON.parse(line) as Record); + const operationEvents = auditEvents.filter((event) => event.kind === "operation"); + expect(operationEvents.length).toBeGreaterThanOrEqual(2); + for (const event of operationEvents) expect(event).not.toHaveProperty("arguments"); + const approvalActions = auditEvents .filter((event) => event.kind === "approval") .map((event) => event.approvalAction); expect(approvalActions).toEqual(["requested", "approved", "consumed"]); @@ -413,9 +458,15 @@ describe("MCP SDK v2 serving interoperability", () => { }); it("propagates modern HTTP request cancellation to the selected upstream", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-v2-cancellation-")); + temporaryDirectories.push(directory); + const auditPath = join(directory, "audit.jsonl"); const upstream = await startFakeRemoteUpstream({ callToolDelayMs: 5_000 }); remoteUpstreams.push(upstream); - const server = await startMiftahHttpServer(await configPath({ url: upstream.streamableHttpUrl })); + const server = await startMiftahHttpServer(await configPath({ + url: upstream.streamableHttpUrl, + auditPath + })); httpServers.push(server); const transport = new StreamableHTTPClientTransport(server.url); const client = new Client( @@ -425,12 +476,23 @@ describe("MCP SDK v2 serving interoperability", () => { try { await client.connect(transport); + const closedSessionsBeforeCall = upstream.closedStreamableSessionIds().length; const controller = new AbortController(); const pending = client.callTool({ name: "whoami", arguments: {} }, { signal: controller.signal }); await waitFor(() => upstream.toolCallRequests() === 1); controller.abort(); await expect(pending).rejects.toBeDefined(); await waitFor(() => upstream.cancelledNotifications() === 1); + await waitFor(() => upstream.closedStreamableSessionIds().length > closedSessionsBeforeCall); + await expect(waitForAuditEvent( + auditPath, + (event) => event.operation === "tools/call" && event.name === "whoami" + )).resolves.toMatchObject({ + status: "cancelled", + errorCode: "REQUEST_CANCELLED", + profileLeaseState: "not-required", + profileLockState: "none" + }); } finally { await client.close(); } diff --git a/tests/mcp-wrapper.test.ts b/tests/mcp-wrapper.test.ts index b0705b8..7149da4 100644 --- a/tests/mcp-wrapper.test.ts +++ b/tests/mcp-wrapper.test.ts @@ -1979,7 +1979,7 @@ describe("Miftah MCP wrapper", () => { await expect.poll(toolCallOperations).toHaveLength(1); const operations = await toolCallOperations(); expect(operations).toEqual([ - expect.objectContaining({ status: "failure", errorCode: "UPSTREAM_CALL_FAILED", name: "whoami" }) + expect.objectContaining({ status: "cancelled", errorCode: "REQUEST_CANCELLED", name: "whoami" }) ]); expect((await readFile(cancelledPath, "utf8")).trim().split("\n")).toHaveLength(1); } finally { diff --git a/tests/mrtr-tasks-decision.test.ts b/tests/mrtr-tasks-decision.test.ts new file mode 100644 index 0000000..24f62df --- /dev/null +++ b/tests/mrtr-tasks-decision.test.ts @@ -0,0 +1,47 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const decision = readFileSync( + new URL("../docs/plans/2026-08-11-mrtr-tasks-decision.md", import.meta.url), + "utf8" +); + +describe("MRTR and Tasks decision", () => { + it("records the selected capability-gated MRTR boundary and safe fallback", () => { + expect(decision).toContain("Status: Multi Round-Trip Requests are implemented for confirmation workflows"); + expect(decision).toContain("`elicitation.form`"); + expect(decision).toContain("authenticated request-context correlation"); + expect(decision).toContain("cannot consume another request's state"); + expect(decision).toContain("Clients without the declared form capability"); + expect(decision).toContain("No bearer is disclosed in the default human mode"); + }); + + it("distinguishes lifecycle outcomes and records why Tasks remain unimplemented", () => { + for (const outcome of ["Incomplete and waiting for input", "Cancelled by the caller", "Failed or rejected", "Completed"]) { + expect(decision).toContain(outcome); + } + expect(decision).toContain("The `io.modelcontextprotocol/tasks` extension remains unimplemented"); + expect(decision).toContain("there is no selected Miftah task"); + expect(decision).toContain("durable opaque identifier"); + expect(decision).toContain("`tasks/get`, `tasks/update`, and `tasks/cancel`"); + expect(decision).toContain("`inputResponses` updates"); + expect(decision).toContain("packaged reconnect evidence"); + expect(decision).toContain("Miftah makes no Tasks interoperability claim"); + }); + + it("applies the audit retention contract to every MRTR round", () => { + expect(decision).toContain("Every round applies the normal operation-argument policy"); + expect(decision).toContain("arguments are omitted unless `audit.includeArguments` is enabled"); + expect(decision).toContain("enabled arguments pass through the configured secret redaction"); + expect(decision).toContain("Continuation state, input responses, profile-context handles, and approval bearers remain excluded"); + expect(decision).toContain("Dedicated approval records never store operation arguments"); + }); + + it("links the primary protocol and extension sources", () => { + expect(decision).toContain("https://blog.modelcontextprotocol.io/posts/2026-07-28/"); + expect(decision).toContain("https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2322"); + expect(decision).toContain("https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2663"); + expect(decision).toContain("https://modelcontextprotocol.io/extensions/tasks/overview"); + expect(decision).toContain("https://github.com/modelcontextprotocol/ext-tasks"); + }); +}); diff --git a/tests/package-contract.test.ts b/tests/package-contract.test.ts index 13e1d67..0d3dafe 100644 --- a/tests/package-contract.test.ts +++ b/tests/package-contract.test.ts @@ -7,6 +7,7 @@ import { fileURLToPath } from "node:url"; import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { beforeAll, describe, expect, it } from "vitest"; +import { Client, StreamableHTTPClientTransport } from "@modelcontextprotocol/client"; import { startFakeRemoteUpstream } from "./helpers/fake-remote-upstream.js"; interface PackageManifest { @@ -1520,15 +1521,67 @@ describe("packed artifact contract", () => { server: { http: { port: 0 } } }) ); + const httpServeAuditPath = join(cliContractDirectory, "http serve audit.jsonl"); + const httpServeCreateCountPath = join(cliContractDirectory, "http serve create count"); + const httpMrtrConfigPath = await writeCliConfig( + "http MRTR config.json", + cliConfig("packed-cli-http-serve", { + work: { + policy: "confirm", + env: { TEST_CREATE_ITEM_COUNT_PATH: httpServeCreateCountPath } + } + }, [fakeStdioUpstreamFixture], { + policies: { confirm: { requireConfirmation: ["create_item"] } }, + audit: { path: httpServeAuditPath }, + server: { http: { port: 0 } } + }) + ); const installedCliEntry = join(directory, "node_modules", "@lubab", "miftah", "dist", "cli", "main.js"); const httpServe = await startInstalledCli( installedCliEntry, - ["serve", "--transport", "http", "--config", httpServeConfigPath], + ["serve", "--transport", "http", "--config", httpMrtrConfigPath], cliContractDirectory ); try { expect(httpServe.stdout).toMatch(/^Miftah HTTP server listening on http:\/\/127\.0\.0\.1:\d+\/mcp\n$/u); expect(httpServe.stderr).toBe(""); + const endpoint = httpServe.stdout.match(/http:\/\/127\.0\.0\.1:\d+\/mcp/u)?.[0]; + if (endpoint === undefined) throw new Error("Installed HTTP CLI did not report its MCP endpoint."); + const client = new Client( + { name: "packed-miftah-mrtr-client", version: "1.0.0" }, + { + versionNegotiation: { mode: "auto" }, + capabilities: { elicitation: { form: {} } } + } + ); + const elicitationRequests: unknown[] = []; + client.setRequestHandler("elicitation/create", async (request) => { + elicitationRequests.push(request); + return { action: "accept", content: { approved: true } }; + }); + const packedMrtrArgument = "packed-mrtr-sensitive-name"; + try { + await client.connect(new StreamableHTTPClientTransport(new URL(endpoint))); + expect(client.getNegotiatedProtocolVersion()).toBe("2026-07-28"); + expect(await client.callTool({ + name: "create_item", + arguments: { name: packedMrtrArgument } + })).toMatchObject({ content: [{ type: "text", text: `created:${packedMrtrArgument}` }] }); + } finally { + await client.close(); + } + expect(elicitationRequests).toHaveLength(1); + expect(JSON.stringify(elicitationRequests)).not.toContain(packedMrtrArgument); + expect(await readFile(httpServeCreateCountPath, "utf8")).toBe("1\n"); + const httpServeAudit = await readFile(httpServeAuditPath, "utf8"); + expect(httpServeAudit).not.toContain(packedMrtrArgument); + const approvalActions = httpServeAudit + .trim() + .split("\n") + .map((line) => JSON.parse(line) as Record) + .filter((event) => event.kind === "approval") + .map((event) => event.approvalAction); + expect(approvalActions).toEqual(["requested", "approved", "consumed"]); } finally { await httpServe.stop(); } diff --git a/tests/stateless-profile-context-runtime.test.ts b/tests/stateless-profile-context-runtime.test.ts index 3285910..8eaa310 100644 --- a/tests/stateless-profile-context-runtime.test.ts +++ b/tests/stateless-profile-context-runtime.test.ts @@ -1,13 +1,19 @@ import { CallToolResultSchema } from "@modelcontextprotocol/core"; import { InMemoryTransport } from "@modelcontextprotocol/server"; import type { AuthInfo, Transport, TransportSendOptions, JSONRPCMessage, Tool } from "@modelcontextprotocol/server"; -import { Client } from "@modelcontextprotocol/client"; -import { mkdir, mkdtemp, readFile, rm } from "node:fs/promises"; +import { + Client, + StreamableHTTPClientTransport, + isInputRequiredResult, + withInputRequired +} from "@modelcontextprotocol/client"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; import { createAuthenticatedRequestContextBoundary } from "../src/http/authenticated-request-context.js"; +import { startMiftahHttpServer } from "../src/http/miftah-http-server.js"; import { MiftahServer } from "../src/mcp/server/miftah-server.js"; import { ProfileManager } from "../src/profiles/profile-manager.js"; import { @@ -18,6 +24,7 @@ import { type ModernProfileContextRuntimeOptions } from "../src/profiles/profile-context-handle.js"; import { validateConfig } from "../src/config/validate-config.js"; +import { createMiftahServerFactory } from "../src/runtime/create-miftah-runtime.js"; import { UpstreamProcessManager } from "../src/upstream/upstream-process-manager.js"; const fixture = join(dirname(fileURLToPath(import.meta.url)), "fixtures", "fake-upstream.mjs"); @@ -403,6 +410,159 @@ describe("modern stateless profile-context runtime", () => { } }, 30_000); + it("binds one-time MRTR approval state to the authenticated chat and exact profile handle", async () => { + const directory = await mkdtemp(join(tmpdir(), "miftah-stateless-mrtr-binding-")); + const configPath = join(directory, "miftah.json"); + const auditPath = join(directory, "audit.jsonl"); + const createCountPath = join(directory, "create-count"); + await writeFile(configPath, JSON.stringify({ + version: "1", + name: "accounts", + defaultProfile: "personal", + upstream: { transport: "stdio", command: process.execPath, args: [fixture] }, + profiles: { + personal: { env: { TEST_ACCOUNT_NAME: "personal" } }, + work: { + policy: "confirm", + env: { TEST_ACCOUNT_NAME: "work", TEST_CREATE_ITEM_COUNT_PATH: createCountPath } + } + }, + policies: { confirm: { requireConfirmation: ["create_item"] } }, + audit: { path: auditPath, includeArguments: true }, + server: { http: { port: 0 } } + })); + const requestBoundary = createAuthenticatedRequestContextBoundary({ + deploymentId: "miftah.example/mrtr-binding", + bindingKey: Buffer.alloc(32, 0x51), + auditKey: Buffer.alloc(32, 0x52), + clock: () => nowMs, + verifiedClaimsProvider: (request) => { + const extra = (request as AuthInfo | undefined)?.extra; + return extra?.verifiedClaims as ReturnType | undefined; + } + }); + const modernProfileContext = { + handles: new ProfileContextHandleService({ + deploymentId: "miftah.example/mrtr-binding", + profiles: ["personal", "work"], + keyringProvider: () => ({ + activeEpoch: 1, + epochs: [{ epoch: 1, key: Buffer.alloc(32, 0x53), activatedAtMs: nowMs - 1_000 }] + }), + auditKey: Buffer.alloc(32, 0x54), + revocations: new InMemoryProfileContextRevocationStore(), + clock: () => nowMs + }), + authenticatedRequestContext: requestBoundary + }; + let authentication: AuthInfo | undefined = authInfo("chat-a"); + const server = await startMiftahHttpServer(configPath, { + modernServerFactory: createMiftahServerFactory(configPath, { modernProfileContext }) + }); + const nodeServer = (server as unknown as { + readonly server: { + prependListener(event: "request", listener: (request: { auth?: AuthInfo }) => void): void; + }; + }).server; + nodeServer.prependListener("request", (request) => { + request.auth = authentication; + }); + const client = new Client( + { name: "modern-mrtr-binding-client", version: "1.0.0" }, + { + versionNegotiation: { mode: { pin: "2026-07-28" } }, + capabilities: { elicitation: { form: {} } }, + inputRequired: { autoFulfill: false } + } + ); + const accepted = { approval: { action: "accept" as const, content: { approved: true } } }; + + const requestCreate = async ( + handle: string, + continuation?: { readonly requestState: string; readonly inputResponses: typeof accepted } + ) => client.request( + { + method: "tools/call", + params: { + name: "create_item", + arguments: { name: "principal-bound", [PROFILE_CONTEXT_ARGUMENT]: handle }, + ...(continuation ?? {}) + } + }, + withInputRequired(CallToolResultSchema), + { allowInputRequired: true } + ); + + try { + await client.connect(new StreamableHTTPClientTransport(server.url)); + + const handleA = profileHandle(await client.callTool({ + name: "miftah_use_profile", + arguments: { profile: "work" } + })); + authentication = authInfo("chat-b"); + const handleB = profileHandle(await client.callTool({ + name: "miftah_use_profile", + arguments: { profile: "work" } + })); + + authentication = authInfo("chat-a"); + const firstRound = await requestCreate(handleA); + if (!isInputRequiredResult(firstRound) || firstRound.requestState === undefined) { + throw new Error(`Expected an integrity-bound approval continuation: ${JSON.stringify(firstRound)}`); + } + expect(firstRound.inputRequests).toHaveProperty("approval"); + const continuation = { requestState: firstRound.requestState, inputResponses: accepted }; + + authentication = authInfo("chat-b"); + expect(await requestCreate(handleB, continuation)).toMatchObject({ + isError: true, + content: [{ type: "text", text: expect.stringContaining("APPROVAL_INVALID") }] + }); + + authentication = authInfo("chat-a"); + expect(await requestCreate(handleA, continuation)).toMatchObject({ + content: [{ type: "text", text: "created:principal-bound" }] + }); + await expect(requestCreate(handleA, continuation)).rejects.toThrow(/requestState/u); + expect(await readFile(createCountPath, "utf8")).toBe("1\n"); + + const auditText = await readFile(auditPath, "utf8"); + expect(auditText).not.toContain(handleA); + expect(auditText).not.toContain(handleB); + expect(auditText).not.toContain(firstRound.requestState); + expect(auditText).not.toContain('"requestState"'); + expect(auditText).not.toContain('"inputResponses"'); + const events = auditText + .trim() + .split("\n") + .map((line) => JSON.parse(line) as Record); + const operationEvents = events.filter( + (event) => event.kind === "operation" && event.operation === "tools/call" && event.name === "create_item" + ); + expect(operationEvents.length).toBeGreaterThanOrEqual(3); + for (const event of operationEvents) expect(event.arguments).toEqual({ name: "principal-bound" }); + expect(events).toEqual(expect.arrayContaining([ + expect.objectContaining({ + kind: "operation", + operation: "tools/call", + status: "confirmation-required", + errorCode: "POLICY_CONFIRMATION_REQUIRED" + }), + expect.objectContaining({ + kind: "operation", + operation: "tools/call", + status: "failure", + errorCode: "APPROVAL_INVALID" + }), + expect.objectContaining({ kind: "operation", operation: "tools/call", status: "success" }) + ])); + } finally { + await Promise.allSettled([client.close(), server.close()]); + await rm(directory, { recursive: true, force: true, maxRetries: 5, retryDelay: 25 }); + } + }, 20_000); + it("enforces strict cross-profile tool discovery whenever modern mode is enabled", async () => { const directory = await mkdtemp(join(tmpdir(), "miftah-stateless-strict-discovery-")); const auditPath = join(directory, "audit.jsonl"); diff --git a/vitest.config.ts b/vitest.config.ts index 3d37c8c..3484b38 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -10,6 +10,10 @@ export default defineConfig({ include: ["tests/**/*.test.ts"], restoreMocks: true, clearMocks: true, + // Hosted runners can need more than Vitest's five-second default for + // filesystem, ACL, and audit tests; keep a finite CI cap while preserving + // the faster hang signal during local development. + testTimeout: process.env.GITHUB_ACTIONS === "true" ? 10_000 : 5_000, // Real upstream fixtures have one-second startup limits; run files serially to prevent contention. fileParallelism: false, // Replace the fork between serial files so process-backed tests cannot retain