mcp: UpstreamClient — typed openapi-fetch client gated by allowlist, use, origin, and credential

src/upstream/client.ts is the only way this service talks to the Artcraft API. Middleware refuses, before anything leaves the Worker: routes not allowlisted for the client's `use` (tools = read, sign-in/out = auth), per-request baseUrl overrides to any other origin, and concrete URLs that do not match the allowlisted template (independent second check). The credential is attached last, so a rejected request never carries it. Read clients require a credential; auth clients may sign in without one. Tests (13): headers and URL of a good request, path substitution, array query params as repeated keys (what actix_web_lab parses), 401 passed through as a result not an exception, credential required for read, sign-in without and sign-out with a credential, and the gate firing — auth route from read client, read route from auth client, unlisted path, foreign baseUrl — each with fetch provably not called; path parameters are percent-encoded so they cannot escape their segment; the rejection message never contains the credential. CLAUDE.md layout updated for the upstream module and generated files. pnpm check: typecheck, eslint, prettier, 77 tests passing. Claude-Session: https://claude.ai/code/session_01H3gfN2NCx3hBXSrZXYHUQ3
2026-11-11 04:16:55 +00:00 · 2026-08-24 20:15:40 -07:00
1 parent 4f810e44d6
commit afd2d0d6a9
@@ -324,14 +324,22 @@ mcp/
 ├── docs/backend-handoff.md # findings for the storyteller-web owner; not our work
 ├── docs/infra-request.md   # one-time batched asks for the backend team (Cloudflare, secrets, Google)
 ├── fake-upstream/          # separate Worker faking the allowlisted API routes; never imported by src/
-├── scripts/gen-api.ts      # fetch api.json → openapi-typescript
+├── scripts/gen-api.mjs     # fetch api.json → trim to allowlist → test/fixtures/api.json + src/upstream/schema.d.ts
 ├── src/
 │   ├── index.ts            # Worker entry: OAuthProvider wrapping the Hono app
 │   ├── app.ts              # Hono routes
 │   ├── auth/               # provider config, Authenticator impls, consent page (Hono JSX)
 │   ├── tokens/             # TokenResolver, Principal, GrantStore adapter
-│   ├── upstream/           # UpstreamClient, UpstreamCredential, allowlist, generated schema
+│   ├── upstream/
+│   │   ├── allowlist.json  # the ONLY upstream routes, with `use: auth | read`; also drives the generator
+│   │   ├── allowlist.ts    # validation + lookups by template path and by concrete pathname
+│   │   ├── credential.ts   # UpstreamCredential interface; session implementation (swap point 2)
+│   │   ├── client.ts       # openapi-fetch client; middleware enforces allowlist, use, origin, credential
+│   │   └── schema.d.ts     # GENERATED from the spec snapshot by `pnpm gen:api`; do not edit
 │   ├── mcp/                # McpServer factory, tools/, resources, prompts
 │   └── pages/              # landing, connections
-└── test/                   # unit/, contract/, oauth/, e2e/, fixtures/
+└── test/
+    ├── fixtures/api.json   # GENERATED spec snapshot (trimmed); the contract tests' reference
+    ├── helpers/            # pure check functions so guardrail tests can prove they fire
+    └── upstream/, oauth/, e2e/, *.test.ts
 ```
@@ -0,0 +1,97 @@
+import createClient, { type Client, type Middleware } from "openapi-fetch";
+
+import { findRoute, findRouteForPathname, isAllowed, type UpstreamUse } from "./allowlist";
+import type { UpstreamCredential } from "./credential";
+import type { paths } from "./schema";
+
+/**
+ * The only way this service talks to the Artcraft API. A typed openapi-fetch client whose
+ * middleware refuses, before anything leaves the Worker, any request that is
+ *
+ * - not on the allowlist for this client's `use` (tools get `read`, sign-in/out gets `auth`),
+ * - not addressed to the configured upstream origin (per-request `baseUrl` overrides are
+ *   rejected), or
+ * - whose final URL does not match the allowlisted template (a second, independent check on
+ *   the concrete pathname).
+ *
+ * The credential, when present, is attached last so it is never sent on a rejected request.
+ */
+
+const USER_AGENT = "artcraft-mcp/0.0.0";
+
+export type UpstreamClient = Client<paths>;
+
+export interface UpstreamClientOptions {
+  /** Origin of the upstream API, e.g. `https://api.storyteller.ai` (from `Config`). */
+  readonly baseUrl: string;
+  /** Who this client serves; decides which allowlisted routes it may call. */
+  readonly use: UpstreamUse;
+  /** Required for `read` clients (every read route needs a user); optional for `auth`. */
+  readonly credential?: UpstreamCredential;
+  /** Injectable for tests; defaults to the global fetch. */
+  readonly fetch?: typeof globalThis.fetch;
+}
+
+export class UpstreamPathNotAllowedError extends Error {
+  override readonly name = "UpstreamPathNotAllowedError";
+
+  constructor(method: string, path: string, use: UpstreamUse) {
+    super(`${method.toUpperCase()} ${path} is not allowlisted for ${use} use`);
+  }
+}
+
+export class UpstreamOriginError extends Error {
+  override readonly name = "UpstreamOriginError";
+
+  constructor(expected: string, actual: string) {
+    super(`upstream request addressed to ${actual}; only ${expected} is allowed`);
+  }
+}
+
+export function createUpstreamClient(options: UpstreamClientOptions): UpstreamClient {
+  if (options.use === "read" && !options.credential) {
+    throw new Error("a read upstream client requires a credential");
+  }
+  const origin = new URL(options.baseUrl).origin;
+
+  const client = createClient<paths>({
+    baseUrl: origin,
+    fetch: options.fetch ?? globalThis.fetch,
+    headers: { accept: "application/json", "user-agent": USER_AGENT },
+  });
+
+  client.use(allowlistMiddleware(options.use, origin));
+  if (options.credential) client.use(credentialMiddleware(options.credential));
+  return client;
+}
+
+function allowlistMiddleware(use: UpstreamUse, origin: string): Middleware {
+  return {
+    onRequest({ request, schemaPath }) {
+      const route = findRoute(request.method, schemaPath);
+      if (!isAllowed(route, use)) {
+        throw new UpstreamPathNotAllowedError(request.method, schemaPath, use);
+      }
+
+      const url = new URL(request.url);
+      if (url.origin !== origin) {
+        throw new UpstreamOriginError(origin, url.origin);
+      }
+      const concreteRoute = findRouteForPathname(request.method, url.pathname);
+      if (concreteRoute !== route) {
+        throw new UpstreamPathNotAllowedError(request.method, url.pathname, use);
+      }
+      return undefined;
+    },
+  };
+}
+
+function credentialMiddleware(credential: UpstreamCredential): Middleware {
+  return {
+    onRequest({ request }) {
+      const headers = new Headers(request.headers);
+      credential.applyTo(headers);
+      return new Request(request, { headers });
+    },
+  };
+}
@@ -0,0 +1,173 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  createUpstreamClient,
+  UpstreamOriginError,
+  UpstreamPathNotAllowedError,
+} from "../../src/upstream/client";
+import { createSessionCredential, SESSION_HEADER_NAME } from "../../src/upstream/credential";
+
+const BASE_URL = "https://api.example.test";
+const SIGNED_SESSION = "eyJhbGciOiJIUzI1NiJ9.eyJzZXNzaW9uX3Rva2VuIjoic2Vzc2lvbl90ZXN0In0.c2ln";
+
+/** A fetch that records every request and answers with a fixed JSON body. */
+function recordingFetch(status = 200, body: unknown = { success: true }) {
+  const requests: Request[] = [];
+  const fetchImpl: typeof globalThis.fetch = (input, init) => {
+    requests.push(new Request(input, init));
+    return Promise.resolve(
+      new Response(JSON.stringify(body), {
+        status,
+        headers: { "content-type": "application/json" },
+      }),
+    );
+  };
+  return { requests, fetch: fetchImpl };
+}
+
+function readClient(fetchImpl: typeof globalThis.fetch) {
+  return createUpstreamClient({
+    baseUrl: BASE_URL,
+    use: "read",
+    credential: createSessionCredential(SIGNED_SESSION),
+    fetch: fetchImpl,
+  });
+}
+
+describe("read client", () => {
+  it("sends an allowlisted request with the session header, JSON accept, and our user agent", async () => {
+    const { requests, fetch } = recordingFetch();
+    const result = await readClient(fetch).GET("/v1/session");
+
+    expect(result.response.status).toBe(200);
+    expect(requests).toHaveLength(1);
+    const sent = requests[0];
+    expect(sent?.url).toBe(`${BASE_URL}/v1/session`);
+    expect(sent?.method).toBe("GET");
+    expect(sent?.headers.get(SESSION_HEADER_NAME)).toBe(SIGNED_SESSION);
+    expect(sent?.headers.get("accept")).toBe("application/json");
+    expect(sent?.headers.get("user-agent")).toMatch(/^artcraft-mcp\//);
+    expect(sent?.headers.get("cookie")).toBeNull();
+    expect(sent?.headers.get("authorization")).toBeNull();
+  });
+
+  it("substitutes path parameters and keeps the concrete URL on the template", async () => {
+    const { requests, fetch } = recordingFetch();
+    await readClient(fetch).GET("/v1/jobs/job/{token}", {
+      params: { path: { token: "jinf_abc123" } },
+    });
+    expect(requests[0]?.url).toBe(`${BASE_URL}/v1/jobs/job/jinf_abc123`);
+  });
+
+  it("serialises array query parameters as repeated keys (what the upstream parses)", async () => {
+    const { requests, fetch } = recordingFetch();
+    await readClient(fetch).GET("/v1/jobs/batch", {
+      params: { query: { tokens: ["jinf_a", "jinf_b"] } },
+    });
+    const url = new URL(requests[0]?.url ?? "");
+    expect(url.pathname).toBe("/v1/jobs/batch");
+    expect(url.searchParams.getAll("tokens")).toEqual(["jinf_a", "jinf_b"]);
+  });
+
+  it("passes an upstream 401 through as an error result, not an exception", async () => {
+    const { fetch } = recordingFetch(401, { success: false, error_code: 401 });
+    const result = await readClient(fetch).GET("/v1/credits/namespace/{namespace}", {
+      params: { path: { namespace: "artcraft" } },
+    });
+    expect(result.response.status).toBe(401);
+    expect(result.data).toBeUndefined();
+    expect(result.error).toEqual({ success: false, error_code: 401 });
+  });
+
+  it("requires a credential", () => {
+    expect(() =>
+      createUpstreamClient({ baseUrl: BASE_URL, use: "read", fetch: recordingFetch().fetch }),
+    ).toThrow(/requires a credential/);
+  });
+});
+
+describe("auth client", () => {
+  it("may sign in without a credential and sends no session header", async () => {
+    const { requests, fetch } = recordingFetch();
+    const client = createUpstreamClient({ baseUrl: BASE_URL, use: "auth", fetch });
+    await client.POST("/v1/login", {
+      body: { username_or_email: "localdev1", password: "localdev1pass" },
+    });
+    const sent = requests[0];
+    expect(sent?.url).toBe(`${BASE_URL}/v1/login`);
+    expect(sent?.headers.get(SESSION_HEADER_NAME)).toBeNull();
+    expect(sent?.headers.get("content-type")).toMatch(/^application\/json/);
+    expect(await sent?.json()).toEqual({
+      username_or_email: "localdev1",
+      password: "localdev1pass",
+    });
+  });
+
+  it("may sign out with a credential", async () => {
+    const { requests, fetch } = recordingFetch();
+    const client = createUpstreamClient({
+      baseUrl: BASE_URL,
+      use: "auth",
+      credential: createSessionCredential(SIGNED_SESSION),
+      fetch,
+    });
+    await client.POST("/v1/logout");
+    expect(requests[0]?.headers.get(SESSION_HEADER_NAME)).toBe(SIGNED_SESSION);
+  });
+});
+
+describe("the gate fires before anything leaves the Worker", () => {
+  it("refuses an auth route from a read client, without calling fetch", async () => {
+    const { requests, fetch } = recordingFetch();
+    await expect(
+      readClient(fetch).POST("/v1/login", { body: { username_or_email: "x", password: "y" } }),
+    ).rejects.toThrow(UpstreamPathNotAllowedError);
+    expect(requests).toHaveLength(0);
+  });
+
+  it("refuses a read route from an auth client", async () => {
+    const { requests, fetch } = recordingFetch();
+    const client = createUpstreamClient({ baseUrl: BASE_URL, use: "auth", fetch });
+    await expect(client.GET("/v1/session")).rejects.toThrow(/not allowlisted for auth use/);
+    expect(requests).toHaveLength(0);
+  });
+
+  it("refuses a path that is not on the allowlist at all", async () => {
+    const { requests, fetch } = recordingFetch();
+    const client = readClient(fetch) as unknown as { GET(path: string): Promise<unknown> };
+    await expect(client.GET("/v1/media_files/list")).rejects.toThrow(UpstreamPathNotAllowedError);
+    expect(requests).toHaveLength(0);
+  });
+
+  it("refuses a per-request baseUrl pointing anywhere else", async () => {
+    const { requests, fetch } = recordingFetch();
+    await expect(
+      readClient(fetch).GET("/v1/session", { baseUrl: "https://evil.example.test" }),
+    ).rejects.toThrow(UpstreamOriginError);
+    expect(requests).toHaveLength(0);
+  });
+
+  it("percent-encodes a path parameter so it can never escape its segment", async () => {
+    const { requests, fetch } = recordingFetch();
+    await readClient(fetch).GET("/v1/jobs/job/{token}", {
+      params: { path: { token: "../../v1/media_files/list" } },
+    });
+    const pathname = new URL(requests[0]?.url ?? "").pathname;
+    expect(pathname).toBe("/v1/jobs/job/..%2F..%2Fv1%2Fmedia_files%2Flist");
+    expect(pathname).toMatch(/^\/v1\/jobs\/job\/[^/]+$/);
+  });
+
+  it("does not include the credential in the rejection message", async () => {
+    const { fetch } = recordingFetch();
+    let rendered = "";
+    try {
+      await readClient(fetch).POST("/v1/login", {
+        body: { username_or_email: "x", password: "y" },
+      });
+    } catch (error) {
+      rendered = error instanceof Error ? `${error.name}: ${error.message}` : "not an Error";
+    }
+    expect(rendered).toMatch(/^UpstreamPathNotAllowedError: /);
+    expect(rendered).not.toContain(SIGNED_SESSION);
+  });
+});