diff --git a/.changeset/sdk-shared-credential-transport.md b/.changeset/sdk-shared-credential-transport.md new file mode 100644 index 000000000..212587a9c --- /dev/null +++ b/.changeset/sdk-shared-credential-transport.md @@ -0,0 +1,38 @@ +--- +"@sapiom/analytics-core": minor +"@sapiom/core": minor +"@sapiom/agent-core": minor +"@sapiom/browser-pay": minor +"@sapiom/mcp": patch +"@sapiom/tools": patch +--- + +The tenant API key now only travels over HTTPS, or plain HTTP to a loopback +host, from every SDK client, and never follows a redirect to another origin. +One function, `credentialFetch` from the new `@sapiom/analytics-core/credential` +subpath, is the only code that sets the key header. + +Behavior changes, shipped as 0.x minor bumps: a plain `http://` URL to a +non-loopback host is now refused, and the key headers are stripped on a +cross-origin redirect. `@sapiom/core` and `@sapiom/browser-pay` gain their +first runtime dependency, `@sapiom/analytics-core`. + +- `@sapiom/core`, `@sapiom/agent-core` and `@sapiom/mcp`: a plain `http://` + URL to a non-loopback host (for example a Docker service name or a LAN IP) + now fails before the request is sent. On a trusted private network, opt in + with `new SapiomClient({ allowInsecureHttp: true })` (core), + `createClient({ allowInsecureHttp: true })` (agent-core) or + `SAPIOM_ALLOW_INSECURE_HTTP=1` (all clients). `localhost`, `*.localhost`, + `127.0.0.1` and `[::1]` work as before. Core does not retry a refused channel. +- `@sapiom/analytics-core`: on such an endpoint the batch is still sent, without + the key. A redirect to a refused channel drops the batch. +- Every client follows redirects itself: the first hop to another origin drops + `x-api-key`, `x-sapiom-api-key` and every `x-sapiom-*` header. +- `@sapiom/core`: `getHttpClient().defaults.headers` no longer carries the key; + read `defaults.credential` instead. +- `@sapiom/browser-pay`: the payment relay uses the shared rule, so a + `*.localhost` base URL and `SAPIOM_ALLOW_INSECURE_HTTP=1` are now accepted, and + a refused base URL throws a `CredentialChannelError`. Redirects are still + refused. +- `@sapiom/tools`: no behavior change. A refused call now throws a + `CredentialChannelError`, a `TypeError` subclass that keeps `name: "TypeError"`. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9f637a212..0be454b92 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -109,6 +109,11 @@ jobs: - name: Check Agent Studio visible terminology run: pnpm terminology:check + # Keep literal tenant-key header writes in the shared owner, except for + # the exact-path exceptions in scripts/credential-header-allowlist.json. + - name: Check tenant credential headers + run: pnpm credential-header:check + # Fixture tests for the gate itself: a typo'd declaration field must fail # rather than be silently stripped downstream. This glob also runs the # terminology guard's forbidden-copy, compatibility, and stale-allowlist diff --git a/package.json b/package.json index e35c285d2..fed7abb07 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "examples:check:test": "node --test scripts/*.test.mjs", "examples:test": "node scripts/examples-test.mjs", "terminology:check": "node scripts/agent-studio-terminology-check.mjs", + "credential-header:check": "node scripts/credential-header-check.mjs", "terminology:check:test": "node --test scripts/agent-studio-terminology-check.test.mjs", "provider-copy:check": "node scripts/provider-neutral-copy-check.mjs", "provider-copy:check:test": "node --test scripts/provider-neutral-copy-check.test.mjs", diff --git a/packages/agent-core/src/__tests__/client.test.ts b/packages/agent-core/src/__tests__/client.test.ts index 7576b62ec..feadc28cc 100644 --- a/packages/agent-core/src/__tests__/client.test.ts +++ b/packages/agent-core/src/__tests__/client.test.ts @@ -238,6 +238,90 @@ describe('signal', () => { }); }); +describe('GatewayClient credential channel', () => { + const ENV = 'SAPIOM_ALLOW_INSECURE_HTTP'; + const ORIGINAL = process.env[ENV]; + beforeEach(() => { + delete process.env[ENV]; + }); + afterAll(() => { + if (ORIGINAL === undefined) delete process.env[ENV]; + else process.env[ENV] = ORIGINAL; + }); + + const sentKey = (spy: jest.SpyInstance, call: number) => + (spy.mock.calls[call][1] as RequestInit & { headers: Record }).headers[ + 'x-api-key' + ]; + + it.each([ + ['get', (c: GatewayClient) => c.get('/foo')], + ['post', (c: GatewayClient) => c.post('/foo', { a: 1 })], + ['postAtHostRoot', (c: GatewayClient) => c.postAtHostRoot('/v1/studio-feedback', {})], + ['openStream', (c: GatewayClient) => c.openStream('/executions/e1/stream')], + ])('%s refuses plaintext to a non-loopback host as NETWORK, before sending', async (_m, call) => { + const spy = mockFetch([{ status: 200, body: {} }]); + const client = createClient({ host: 'http://gw.internal', apiKey: 'sk_live' }); + await expect(call(client)).rejects.toMatchObject({ + code: 'NETWORK', + hint: expect.stringContaining('@sapiom/agent-core: refusing plaintext HTTP to http://gw.internal'), + }); + expect(spy).not.toHaveBeenCalled(); + }); + + it('names its own opt-in in the refusal', async () => { + mockFetch([{ status: 200, body: {} }]); + const client = createClient({ host: 'http://gw.internal', apiKey: 'sk_live' }); + await expect(client.get('/foo')).rejects.toMatchObject({ + hint: expect.stringContaining( + 'createClient({ allowInsecureHttp: true }) or SAPIOM_ALLOW_INSECURE_HTTP=1', + ), + }); + }); + + it.each([ + ['a loopback host', { host: 'http://localhost:3000' }, undefined], + ['the client option', { host: 'http://gw.internal', allowInsecureHttp: true }, undefined], + ['the env var', { host: 'http://gw.internal' }, '1'], + ])('sends over plain http to %s', async (_label, options, env) => { + if (env) process.env[ENV] = env; + const spy = mockFetch([{ status: 200, body: {} }]); + await createClient({ apiKey: 'sk_live', ...options }).get('/foo'); + expect(sentKey(spy, 0)).toBe('sk_live'); + }); + + it('drops x-api-key on a redirect to another origin (get)', async () => { + const spy = jest + .spyOn(global, 'fetch' as any) + .mockResolvedValueOnce( + new Response(null, { status: 307, headers: { location: 'https://other.example/foo' } }), + ) + .mockResolvedValueOnce(new Response('{"ok":true}', { status: 200 })); + const client = createClient({ host: 'https://example.com', apiKey: 'sk_live' }); + await expect(client.get('/foo')).resolves.toEqual({ ok: true }); + expect(spy.mock.calls.map((c) => c[0])).toEqual([ + 'https://example.com/v1/workflows/foo', + 'https://other.example/foo', + ]); + expect([sentKey(spy, 0), sentKey(spy, 1)]).toEqual(['sk_live', undefined]); + }); + + it('drops x-api-key on a redirect to another origin (openStream), keeping the body open', async () => { + const spy = jest + .spyOn(global, 'fetch' as any) + .mockResolvedValueOnce( + new Response(null, { status: 307, headers: { location: 'https://other.example/s' } }), + ) + .mockResolvedValueOnce(new Response('data: 1\n\n', { status: 200 })); + const client = createClient({ host: 'https://example.com', apiKey: 'sk_live' }); + const res = await client.openStream('/executions/e1/stream'); + expect(spy.mock.calls[1][0]).toBe('https://other.example/s'); + expect([sentKey(spy, 0), sentKey(spy, 1)]).toEqual(['sk_live', undefined]); + expect(res.bodyUsed).toBe(false); + expect(await res.text()).toBe('data: 1\n\n'); + }); +}); + // ── emitEvent ───────────────────────────────────────────────────────────────── describe('emitEvent', () => { diff --git a/packages/agent-core/src/client.ts b/packages/agent-core/src/client.ts index f68cc2ffa..f6d8eb581 100644 --- a/packages/agent-core/src/client.ts +++ b/packages/agent-core/src/client.ts @@ -1,8 +1,11 @@ -/** - * Configurable HTTP client for the Sapiom workflows backend API. All inputs are - * passed explicitly (base URL + API key) — no process.env reads, no global - * state — so the client is usable from a CLI, an MCP tool, or a test harness. - */ +/** Explicit credentials keep this client reusable by CLI, MCP, and test callers. + * The shared channel policy also reads SAPIOM_ALLOW_INSECURE_HTTP. */ +import { + credentialFetch, + resolveCredentialPolicy, + type Credential, + type CredentialPolicy, +} from '@sapiom/analytics-core/credential'; import { AgentOperationError } from './errors.js'; /** @@ -17,6 +20,9 @@ export interface ClientOptions { host?: string; /** API key sent as `x-api-key`. Must start with `sk_`. */ apiKey: string; + /** Permit plaintext credentials only for a trusted private network. + * Explicit configuration overrides SAPIOM_ALLOW_INSECURE_HTTP. */ + allowInsecureHttp?: boolean; } /** Result shape from a failed gateway request. */ @@ -42,12 +48,17 @@ export class GatewayClient { private readonly host: string; /** `${host}/v1/workflows` — what get/post/request/openStream paths are relative to. */ private readonly base: string; - private readonly apiKey: string; + private readonly credential: Credential; + private readonly policy: CredentialPolicy; constructor(opts: ClientOptions) { this.host = stripTrailingSlashes(opts.host ?? DEFAULT_WORKFLOWS_HOST); this.base = `${this.host}/v1/workflows`; - this.apiKey = opts.apiKey; + this.credential = { header: 'x-api-key', value: opts.apiKey }; + this.policy = resolveCredentialPolicy(opts.allowInsecureHttp, { + label: '@sapiom/agent-core', + optIn: 'createClient({ allowInsecureHttp: true })', + }); } /** `path` is relative to `/v1/workflows`. */ @@ -110,12 +121,19 @@ export class GatewayClient { let res: Response; try { - res = await fetch(url, { - method, - headers: { 'x-api-key': this.apiKey, 'content-type': 'application/json' }, - body: payload, - signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), - }); + // Preserve the NETWORK boundary so callers can surface channel refusals through hint. + res = await credentialFetch( + fetch, + url, + { + method, + headers: { 'content-type': 'application/json' }, + body: payload, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }, + this.credential, + this.policy, + ); } catch (err) { throw networkError(url, err); } @@ -160,10 +178,7 @@ export class GatewayClient { path: string, opts: { signal?: AbortSignal; lastEventId?: string } = {}, ): Promise { - const headers: Record = { - 'x-api-key': this.apiKey, - accept: 'text/event-stream', - }; + const headers: Record = { accept: 'text/event-stream' }; if (opts.lastEventId) { headers['last-event-id'] = opts.lastEventId; } @@ -171,7 +186,13 @@ export class GatewayClient { const url = `${this.base}${path}`; let res: Response; try { - res = await fetch(url, { method: 'GET', headers, signal: opts.signal }); + res = await credentialFetch( + fetch, + url, + { method: 'GET', headers, signal: opts.signal }, + this.credential, + this.policy, + ); } catch (err) { throw networkError(url, err); } diff --git a/packages/analytics-core/CONTRACT.md b/packages/analytics-core/CONTRACT.md index b4acfb15a..4e8991890 100644 --- a/packages/analytics-core/CONTRACT.md +++ b/packages/analytics-core/CONTRACT.md @@ -21,6 +21,10 @@ Authentication is optional and does exactly one thing: a **valid** to the events (see [Server-stamped fields](#server-stamped-fields-never-client-trusted)). An invalid or missing key never causes a rejection. +The key is only sent over https or to a loopback host (or with +`SAPIOM_ALLOW_INSECURE_HTTP=1`); on any other endpoint the batch goes out +without it. A redirect to another origin drops the key. + This URL is exported by `@sapiom/analytics-core` as the constant `SAPIOM_COLLECTOR_ENDPOINT` and is the emitter's **default endpoint**: an emitter with no explicit `endpoint` configured delivers here, unless the diff --git a/packages/analytics-core/package.json b/packages/analytics-core/package.json index 3f5e843b4..fa29ff412 100644 --- a/packages/analytics-core/package.json +++ b/packages/analytics-core/package.json @@ -22,12 +22,20 @@ "types": "./dist/cjs/testing/index.d.ts", "import": "./dist/esm/testing/index.js", "require": "./dist/cjs/testing/index.js" + }, + "./credential": { + "types": "./dist/cjs/credential.d.ts", + "import": "./dist/esm/credential.js", + "require": "./dist/cjs/credential.js" } }, "typesVersions": { "*": { "testing": [ "./dist/cjs/testing/index.d.ts" + ], + "credential": [ + "./dist/cjs/credential.d.ts" ] } }, diff --git a/packages/analytics-core/src/__tests__/http-sender-channel.test.ts b/packages/analytics-core/src/__tests__/http-sender-channel.test.ts new file mode 100644 index 000000000..1ae2f13ce --- /dev/null +++ b/packages/analytics-core/src/__tests__/http-sender-channel.test.ts @@ -0,0 +1,144 @@ +import { ALLOW_INSECURE_HTTP_ENV } from "../credential.js"; +import { HttpSender } from "../http-sender.js"; +import type { FetchRequestInit, FetchResponseLike } from "../types.js"; + +type Call = { url: string; init: FetchRequestInit }; + +function senderWith( + endpoint: string, + responses: FetchResponseLike[], + apiKey: string | null = "sk_live", +) { + const calls: Call[] = []; + const debug: Array<[string, unknown]> = []; + const sender = new HttpSender({ + endpoint, + apiKey: apiKey ?? undefined, + debug: (message, detail) => debug.push([message, detail]), + fetchImpl: async (url, init) => { + calls.push({ url, init }); + return responses.shift() ?? { ok: true, status: 202 }; + }, + }); + return { sender, calls, debug }; +} + +const redirectTo = (location: string, status = 307): FetchResponseLike => ({ + ok: false, + status, + headers: new Headers({ location }), +}); + +const keyOf = (call: Call) => call.init.headers["x-sapiom-api-key"]; + +describe("HttpSender credential channel", () => { + const ORIGINAL = process.env[ALLOW_INSECURE_HTTP_ENV]; + beforeEach(() => { + delete process.env[ALLOW_INSECURE_HTTP_ENV]; + }); + afterAll(() => { + if (ORIGINAL === undefined) delete process.env[ALLOW_INSECURE_HTTP_ENV]; + else process.env[ALLOW_INSECURE_HTTP_ENV] = ORIGINAL; + }); + + it("sends the batch without the key to an endpoint that is not https or loopback", async () => { + const { sender, calls, debug } = senderWith( + "http://collector.internal/v1/analytics/collector?t=secret", + [{ ok: true, status: 202 }], + ); + await expect(sender.send([])).resolves.toBe("ok"); + expect(calls).toHaveLength(1); + expect(keyOf(calls[0])).toBeUndefined(); + expect(calls[0].init.body).toBe('{"events":[]}'); + expect(debug).toEqual([ + [ + "endpoint is not https or loopback; batch sent without the API key", + "http://collector.internal", + ], + ]); + }); + + it.each([ + ["https", "https://collector.example/c"], + ["loopback", "http://127.0.0.1:4318/c"], + ])("carries the key over %s", async (_label, endpoint) => { + const { sender, calls, debug } = senderWith(endpoint, []); + await expect(sender.send([])).resolves.toBe("ok"); + expect(keyOf(calls[0])).toBe("sk_live"); + expect(calls[0].init.redirect).toBe("manual"); + expect(debug).toEqual([]); + }); + + it("carries the key over plain http once opted in through the env", async () => { + process.env[ALLOW_INSECURE_HTTP_ENV] = "1"; + const { sender, calls } = senderWith("http://collector.internal/c", []); + await sender.send([]); + expect(keyOf(calls[0])).toBe("sk_live"); + }); + + it("without a key, sends no header and says nothing", async () => { + const { sender, calls, debug } = senderWith( + "http://collector.internal/c", + [], + null, + ); + await expect(sender.send([])).resolves.toBe("ok"); + expect("x-sapiom-api-key" in calls[0].init.headers).toBe(false); + expect(debug).toEqual([]); + }); + + it("drops the key on a redirect to another origin, resending the body", async () => { + const { sender, calls } = senderWith("https://collector.example/c", [ + redirectTo("https://other.example/c"), + { ok: false, status: 429 }, + ]); + await expect(sender.send([])).resolves.toBe("retry"); + expect(calls.map((c) => c.url)).toEqual([ + "https://collector.example/c", + "https://other.example/c", + ]); + expect(calls.map(keyOf)).toEqual(["sk_live", undefined]); + expect(calls[1].init.body).toBe('{"events":[]}'); + expect(calls.every((c) => c.init.redirect === "manual")).toBe(true); + }); + + it("keeps the key on a same-origin redirect", async () => { + const { sender, calls } = senderWith("https://collector.example/c", [ + redirectTo("/v2/c"), + { ok: true, status: 202 }, + ]); + await expect(sender.send([])).resolves.toBe("ok"); + expect(calls.map(keyOf)).toEqual(["sk_live", "sk_live"]); + }); + + it("drops the batch, without a second call, when a redirect leaves https", async () => { + const { sender, calls, debug } = senderWith("https://collector.example/c", [ + redirectTo("http://evil.internal/c"), + ]); + await expect(sender.send([])).resolves.toBe("drop"); + expect(calls).toHaveLength(1); + expect(debug).toHaveLength(1); + expect(debug[0][0]).toBe("collector redirect refused; batch dropped"); + expect(String(debug[0][1])).toContain("http://evil.internal"); + }); + + it("treats a redirect status with no headers as the final response", async () => { + const { sender, calls } = senderWith("https://collector.example/c", [ + { ok: false, status: 307 }, + ]); + await expect(sender.send([])).resolves.toBe("drop"); + expect(calls).toHaveLength(1); + }); + + it("still retries a failed connection", async () => { + const sender = new HttpSender({ + endpoint: "https://collector.example/c", + apiKey: "sk_live", + debug: () => undefined, + fetchImpl: async () => { + throw new TypeError("fetch failed"); + }, + }); + await expect(sender.send([])).resolves.toBe("retry"); + }); +}); diff --git a/packages/tools/src/_client/credential-policy.spec.ts b/packages/analytics-core/src/credential.spec.ts similarity index 51% rename from packages/tools/src/_client/credential-policy.spec.ts rename to packages/analytics-core/src/credential.spec.ts index 7657c5cdd..64bb0cd18 100644 --- a/packages/tools/src/_client/credential-policy.spec.ts +++ b/packages/analytics-core/src/credential.spec.ts @@ -2,16 +2,23 @@ import { createHash, type BinaryToTextEncoding } from "node:crypto"; import { ALLOW_INSECURE_HTTP_ENV, + CredentialChannelError, assertCredentialMayTravel, + credentialFetch, isLoopbackHostname, matchesIntegrity, + mayCarryCredential, resolveCredentialPolicy, withoutBodyHeaders, withoutCrossOriginHeaders, -} from "./credential-policy.js"; +} from "./credential.js"; -const STRICT = { allowInsecureHttp: false }; -const OPTED_IN = { allowInsecureHttp: true }; +const WORDING = { + label: "@sapiom/test", + optIn: "createClient({ allowInsecureHttp: true })", +}; +const STRICT = { allowInsecureHttp: false, ...WORDING }; +const OPTED_IN = { allowInsecureHttp: true, ...WORDING }; describe("isLoopbackHostname", () => { it.each([ @@ -67,6 +74,24 @@ describe("assertCredentialMayTravel", () => { ); }); + it("names the label, the client's opt-in and the env var", () => { + expect(check("http://api.internal/v1")).toThrow( + "@sapiom/test: refusing plaintext HTTP to http://api.internal. " + + "The tenant API key only travels over https or to a loopback host " + + "(localhost, *.localhost, 127.0.0.1, [::1]). For a trusted private network, " + + "opt in with createClient({ allowInsecureHttp: true }) or SAPIOM_ALLOW_INSECURE_HTTP=1.", + ); + }); + + it("names only the env var when the client has no opt-in of its own", () => { + expect(() => + assertCredentialMayTravel(new URL("http://api.internal/v1"), { + allowInsecureHttp: false, + label: "@sapiom/mcp", + }), + ).toThrow(/^@sapiom\/mcp: .*opt in with SAPIOM_ALLOW_INSECURE_HTTP=1\.$/); + }); + it("allows plaintext to a non-loopback host once opted in", () => { expect(check("http://api:3000/v1", OPTED_IN)).not.toThrow(); }); @@ -85,7 +110,9 @@ describe("assertCredentialMayTravel", () => { } catch (e) { thrown = e; } + expect(thrown).toBeInstanceOf(CredentialChannelError); expect(thrown).toBeInstanceOf(TypeError); + expect((thrown as Error).name).toBe("TypeError"); expect((thrown as { cause?: unknown }).cause).toBeUndefined(); expect((thrown as Error).message).not.toMatch(/secret|token=abc/); }); @@ -101,6 +128,33 @@ describe("assertCredentialMayTravel", () => { }); }); +describe("mayCarryCredential", () => { + it.each([ + "https://api.internal/", + "http://localhost:3000/", + "http://vault.localhost:3999/", + "http://127.0.0.5/", + "http://[::1]:3000/", + ])("%s may carry the key", (url) => { + expect(mayCarryCredential(new URL(url), STRICT)).toBe(true); + }); + + it("plaintext to a non-loopback host may not, unless opted in", () => { + expect(mayCarryCredential(new URL("http://api.internal/"), STRICT)).toBe( + false, + ); + expect(mayCarryCredential(new URL("http://api.internal/"), OPTED_IN)).toBe( + true, + ); + }); + + it("another scheme may not, even opted in", () => { + expect(mayCarryCredential(new URL("ws://localhost/"), OPTED_IN)).toBe( + false, + ); + }); +}); + describe("resolveCredentialPolicy", () => { const ORIGINAL = process.env[ALLOW_INSECURE_HTTP_ENV]; afterEach(() => { @@ -110,7 +164,9 @@ describe("resolveCredentialPolicy", () => { it("is strict when nothing is set", () => { delete process.env[ALLOW_INSECURE_HTTP_ENV]; - expect(resolveCredentialPolicy(undefined).allowInsecureHttp).toBe(false); + expect(resolveCredentialPolicy(undefined, WORDING).allowInsecureHttp).toBe( + false, + ); }); it.each([ @@ -122,14 +178,22 @@ describe("resolveCredentialPolicy", () => { ["", false], ])("env %j opts in: %s", (value, expected) => { process.env[ALLOW_INSECURE_HTTP_ENV] = value; - expect(resolveCredentialPolicy(undefined).allowInsecureHttp).toBe(expected); + expect(resolveCredentialPolicy(undefined, WORDING).allowInsecureHttp).toBe( + expected, + ); + }); + + it("carries the client's wording", () => { + expect(resolveCredentialPolicy(undefined, WORDING)).toMatchObject(WORDING); }); it("an explicit value beats the env", () => { process.env[ALLOW_INSECURE_HTTP_ENV] = "1"; - expect(resolveCredentialPolicy(false).allowInsecureHttp).toBe(false); + expect(resolveCredentialPolicy(false, WORDING).allowInsecureHttp).toBe( + false, + ); delete process.env[ALLOW_INSECURE_HTTP_ENV]; - expect(resolveCredentialPolicy(true).allowInsecureHttp).toBe(true); + expect(resolveCredentialPolicy(true, WORDING).allowInsecureHttp).toBe(true); }); }); @@ -209,7 +273,111 @@ describe("matchesIntegrity", () => { ], ["an empty digest for a known algorithm", "sha256-"], ["a malformed digest for a known algorithm", "sha256-!!!"], + ["a digest of only padding", `sha256-${"=".repeat(100_000)}x`], ])("rejects %s", (_label, metadata) => { expect(matchesIntegrity(BODY, metadata)).toBe(false); }); }); + +describe("credentialFetch", () => { + type Call = { url: string; headers: Record }; + const recorder = (...responses: unknown[]) => { + const calls: Call[] = []; + const fetchImpl = (async (url: string, init: RequestInit) => { + calls.push({ url, headers: init.headers as Record }); + return responses.shift(); + }) as unknown as typeof fetch; + return { calls, fetchImpl }; + }; + const KEY = { header: "x-api-key", value: "sk_real" } as const; + + it("sets the key after the caller's headers, which cannot forge it", async () => { + const { calls, fetchImpl } = recorder(new Response("ok")); + await credentialFetch( + fetchImpl, + "https://api.example/x", + { headers: { "X-Api-Key": "forged", accept: "application/json" } }, + KEY, + STRICT, + ); + expect(calls[0].headers).toEqual({ + accept: "application/json", + "x-api-key": "sk_real", + }); + }); + + it.each([ + ["a Headers instance", new Headers({ "content-type": "application/json" })], + [ + "a tuple list", + [["content-type", "application/json"]] as [string, string][], + ], + ])("reads caller headers given as %s", async (_label, headers) => { + const { calls, fetchImpl } = recorder(new Response("ok")); + await credentialFetch( + fetchImpl, + "https://api.example/x", + { headers }, + KEY, + STRICT, + ); + expect(calls[0].headers).toEqual({ + "content-type": "application/json", + "x-api-key": "sk_real", + }); + }); + + it("refuses an untrusted channel before calling fetch", async () => { + const { calls, fetchImpl } = recorder(); + await expect( + credentialFetch(fetchImpl, "http://api.internal/x", {}, KEY, STRICT), + ).rejects.toBeInstanceOf(CredentialChannelError); + expect(calls).toHaveLength(0); + }); + + it("without a credential, sends no key and does not check the first URL", async () => { + const { calls, fetchImpl } = recorder(new Response("ok")); + await credentialFetch( + fetchImpl, + "http://api.internal/x", + { headers: { accept: "*/*" } }, + undefined, + STRICT, + ); + expect(calls).toEqual([ + { url: "http://api.internal/x", headers: { accept: "*/*" } }, + ]); + }); + + it("returns a minimal redirect with no headers as is, after one call", async () => { + const minimal = { ok: false, status: 307 }; + const { calls, fetchImpl } = recorder(minimal); + await expect( + credentialFetch(fetchImpl, "https://api.example/x", {}, KEY, STRICT), + ).resolves.toBe(minimal); + expect(calls).toHaveLength(1); + }); + + it("follows a minimal redirect carrying headers to a response with no clone", async () => { + const { calls, fetchImpl } = recorder( + { + ok: false, + status: 307, + headers: new Headers({ location: "https://other.example/y" }), + }, + { ok: true, status: 202 }, + ); + const response = await credentialFetch( + fetchImpl, + "https://api.example/x", + {}, + KEY, + STRICT, + ); + expect(response).toMatchObject({ ok: true, status: 202, redirected: true }); + expect(calls.map((c) => c.headers["x-api-key"])).toEqual([ + "sk_real", + undefined, + ]); + }); +}); diff --git a/packages/tools/src/_client/credential-policy.ts b/packages/analytics-core/src/credential.ts similarity index 63% rename from packages/tools/src/_client/credential-policy.ts rename to packages/analytics-core/src/credential.ts index 0a641c8aa..07943e3e4 100644 --- a/packages/tools/src/_client/credential-policy.ts +++ b/packages/analytics-core/src/credential.ts @@ -1,9 +1,5 @@ -/** - * Where the tenant credential may travel: over https, or plain http to a - * loopback host. Redirects are followed here rather than by `fetch`, because - * `fetch` drops `authorization` on an origin change but keeps custom headers - * such as `x-sapiom-api-key`. - */ +/** Centralize channel and redirect checks so clients share one tenant-credential policy. + * Harness exceptions to the literal-header guard are tracked in SAP-4112. */ import { createHash } from "node:crypto"; export const ALLOW_INSECURE_HTTP_ENV = "SAPIOM_ALLOW_INSECURE_HTTP"; @@ -11,14 +7,22 @@ export const ALLOW_INSECURE_HTTP_ENV = "SAPIOM_ALLOW_INSECURE_HTTP"; export interface CredentialPolicy { /** Plain http to a non-loopback host is allowed (trusted private network). */ readonly allowInsecureHttp: boolean; + /** Prefix of every error message: the package the caller uses. */ + readonly label: string; + /** The client's own opt-in, named in a refusal next to the env var. */ + readonly optIn?: string; } /** Explicit config wins, then `SAPIOM_ALLOW_INSECURE_HTTP=1` (or `true`). */ export function resolveCredentialPolicy( explicit: boolean | undefined, + wording: { label: string; optIn?: string }, ): CredentialPolicy { const env = process.env[ALLOW_INSECURE_HTTP_ENV]?.trim().toLowerCase(); - return { allowInsecureHttp: explicit ?? (env === "1" || env === "true") }; + return { + allowInsecureHttp: explicit ?? (env === "1" || env === "true"), + ...wording, + }; } /** `localhost`, `*.localhost` (RFC 6761, used by the local services stack), 127/8, `[::1]`. */ @@ -32,31 +36,43 @@ export function isLoopbackHostname(hostname: string): boolean { ); } -/** - * Throws, before anything is sent, when `url` is not a channel the credential - * may use. A TypeError with no `cause`, unlike a failed connection. Only the - * origin is echoed: a path or query can carry a secret. - */ +/** Extend TypeError, name included, so existing catches and recorded errors are unchanged. + * Retrying with the same URL and policy cannot resolve the refusal. */ +export class CredentialChannelError extends TypeError {} + +/** Let analytics omit an unsafe credential without rejecting the initial batch request. */ +export function mayCarryCredential( + url: URL, + policy: CredentialPolicy, +): boolean { + if (url.protocol === "https:") return true; + return ( + url.protocol === "http:" && + (policy.allowInsecureHttp || isLoopbackHostname(url.hostname)) + ); +} + +/** Refuse unsafe credential channels before sending to them. + * Echo only origins because paths and queries may contain secrets. */ export function assertCredentialMayTravel( url: URL, policy: CredentialPolicy, redirectedFrom?: URL, ): void { - if (url.protocol === "https:") return; + if (mayCarryCredential(url, policy)) return; const from = redirectedFrom ? ` (redirected from ${redirectedFrom.origin})` : ""; if (url.protocol !== "http:") { - throw new TypeError( - `@sapiom/tools: cannot request '${url.protocol}' URLs, expected http or https${from}`, + throw new CredentialChannelError( + `${policy.label}: cannot request '${url.protocol}' URLs, expected http or https${from}`, ); } - if (policy.allowInsecureHttp || isLoopbackHostname(url.hostname)) return; - throw new TypeError( - `@sapiom/tools: refusing plaintext HTTP to ${url.origin}${from}. The tenant API key only ` + + const optIn = policy.optIn ? `${policy.optIn} or ` : ""; + throw new CredentialChannelError( + `${policy.label}: refusing plaintext HTTP to ${url.origin}${from}. The tenant API key only ` + `travels over https or to a loopback host (localhost, *.localhost, 127.0.0.1, [::1]). ` + - `For a trusted private network, opt in with createClient({ allowInsecureHttp: true }) ` + - `or ${ALLOW_INSECURE_HTTP_ENV}=1.`, + `For a trusted private network, opt in with ${optIn}${ALLOW_INSECURE_HTTP_ENV}=1.`, ); } @@ -129,8 +145,12 @@ function isStreamBody(body: RequestInit["body"]): boolean { const SRI_ALGORITHMS = ["sha256", "sha384", "sha512"]; /** base64url and a missing `=` pad compare equal, as in Node's `fetch`. */ -const normalizeDigest = (digest: string): string => - digest.replace(/-/g, "+").replace(/_/g, "/").replace(/=+$/, ""); +function normalizeDigest(digest: string): string { + // A scan, not `/=+$/`: that regex is quadratic on a long run of `=` in the metadata. + let end = digest.length; + while (end > 0 && digest.charCodeAt(end - 1) === 61 /* '=' */) end -= 1; + return digest.slice(0, end).replace(/-/g, "+").replace(/_/g, "/"); +} /** * Subresource Integrity, as `fetch` applies `integrity`: unknown algorithms and @@ -162,6 +182,7 @@ async function assertIntegrity( response: Response, metadata: string, url: URL, + policy: CredentialPolicy, ): Promise { if ( response.body !== null && @@ -174,15 +195,16 @@ async function assertIntegrity( } await response.body?.cancel().catch(() => undefined); throw new TypeError( - `@sapiom/tools: the response from ${url.origin} does not match the request's integrity metadata`, + `${policy.label}: the response from ${url.origin} does not match the request's integrity metadata`, ); } -/** - * Each hop is its own fetch, so none can report what `follow` would: set it on - * the final response and on every clone of it. - */ +/** Manual hops cannot preserve fetch's redirected flag, so mark final responses and clones. + * Injected minimal responses need not implement clone. */ function markRedirected(response: Response): Response { + if (typeof response.clone !== "function") { + return Object.defineProperty(response, "redirected", { value: true }); + } const clone = response.clone.bind(response); return Object.defineProperties(response, { redirected: { value: true }, @@ -223,11 +245,13 @@ export async function fetchKeepingCredential( headers: hopHeaders, redirect: "manual", }); + // A response without `headers` (a minimal injected fetch) cannot be followed. const location = REDIRECT_STATUSES.has(response.status) - ? response.headers.get("location") + ? ((response.headers as Headers | undefined)?.get?.("location") ?? null) : null; if (location === null) { - if (integrity) await assertIntegrity(response, integrity, current); + if (integrity) + await assertIntegrity(response, integrity, current, policy); return redirects > 0 ? markRedirected(response) : response; } // Never handed to the caller: free the connection, even if we refuse the hop. @@ -237,17 +261,17 @@ export async function fetchKeepingCredential( assertCredentialMayTravel(next, policy, current); if (redirects === MAX_REDIRECTS) { throw new TypeError( - `@sapiom/tools: more than ${MAX_REDIRECTS} redirects from ${start.origin}`, + `${policy.label}: more than ${MAX_REDIRECTS} redirects from ${start.origin}`, ); } if (init.mode === "same-origin" && next.origin !== start.origin) { throw new TypeError( - `@sapiom/tools: a "same-origin" request to ${start.origin} was redirected to ${next.origin}`, + `${policy.label}: a "same-origin" request to ${start.origin} was redirected to ${next.origin}`, ); } if (response.status !== 303 && isStreamBody(body)) { throw new TypeError( - `@sapiom/tools: cannot resend a streamed request body after a ${response.status} from ${current.origin}`, + `${policy.label}: cannot resend a streamed request body after a ${response.status} from ${current.origin}`, ); } const verb = (method ?? "GET").toUpperCase(); @@ -267,3 +291,35 @@ export async function fetchKeepingCredential( href = next.href; } } + +export interface Credential { + readonly header: "x-api-key" | "x-sapiom-api-key"; + readonly value: string; +} + +/** Set the credential after caller headers so they cannot replace it. + * Apply shared channel checks and remove credentials when following origin changes. */ +export async function credentialFetch( + fetchImpl: typeof globalThis.fetch, + url: string, + init: RequestInit, + credential: Credential | undefined, + policy: CredentialPolicy, +): Promise { + const callerHeaders = headerRecord(init.headers); + if (!credential) { + return fetchKeepingCredential(fetchImpl, url, init, callerHeaders, policy); + } + assertCredentialMayTravel(new URL(url), policy); + const headers = pick(callerHeaders, (name) => name !== credential.header); + headers[credential.header] = credential.value; + return fetchKeepingCredential(fetchImpl, url, init, headers, policy); +} + +// Object.entries reads a Headers instance as empty and a tuple list by index. +function headerRecord(headers: RequestInit["headers"]): Record { + if (headers instanceof Headers || Array.isArray(headers)) { + return Object.fromEntries(new Headers(headers)); + } + return { ...(headers as Record | undefined) }; +} diff --git a/packages/analytics-core/src/http-sender.ts b/packages/analytics-core/src/http-sender.ts index 22deac8ef..ced42ec4c 100644 --- a/packages/analytics-core/src/http-sender.ts +++ b/packages/analytics-core/src/http-sender.ts @@ -1,3 +1,11 @@ +import { + CredentialChannelError, + credentialFetch, + mayCarryCredential, + resolveCredentialPolicy, + type Credential, + type CredentialPolicy, +} from "./credential.js"; import type { DebugHook, Envelope, FetchLike } from "./types.js"; /** @@ -59,7 +67,13 @@ export interface HttpSenderOptions { * Never rejects — every failure maps to a {@link SendOutcome}. */ export class HttpSender { - constructor(private readonly options: HttpSenderOptions) {} + private readonly policy: CredentialPolicy; + + constructor(private readonly options: HttpSenderOptions) { + this.policy = resolveCredentialPolicy(undefined, { + label: "@sapiom/analytics-core", + }); + } async send(events: Envelope[]): Promise { try { @@ -74,31 +88,55 @@ export class HttpSender { return "drop"; } - const headers: Record = { - "content-type": "application/json", - }; - if (this.options.apiKey) { - headers["x-sapiom-api-key"] = this.options.apiKey; - } - - const response = await fetchImpl(this.options.endpoint, { - method: "POST", - headers, - body, - signal: createTimeoutSignal( - this.options.timeoutMs ?? REQUEST_TIMEOUT_MS, - ), - }); + const response = await credentialFetch( + // Only `{ ok, status }` is read, and `credentialFetch` tolerates a + // response that has nothing more. + fetchImpl as unknown as typeof fetch, + this.options.endpoint, + { + method: "POST", + headers: { "content-type": "application/json" }, + body, + signal: createTimeoutSignal( + this.options.timeoutMs ?? REQUEST_TIMEOUT_MS, + ), + }, + this.credentialFor(this.options.endpoint), + this.policy, + ); if (response.ok) return "ok"; if (response.status === 429 || response.status >= 500) return "retry"; // Anything else (400 not-JSON, 413 too large, …): retrying cannot help. return "drop"; } catch (error) { + if (error instanceof CredentialChannelError) { + // A redirect to a channel the key may not use: resending lands there again. + this.options.debug("collector redirect refused; batch dropped", error); + return "drop"; + } this.options.debug("collector request failed", error); return "retry"; } } + /** + * Analytics never fails the host over its channel: on an endpoint the key + * may not use, the batch still goes out, anonymous. + */ + private credentialFor(endpoint: string): Credential | undefined { + const { apiKey } = this.options; + if (!apiKey) return undefined; + const url = new URL(endpoint); + if (mayCarryCredential(url, this.policy)) { + return { header: "x-sapiom-api-key", value: apiKey }; + } + this.options.debug( + "endpoint is not https or loopback; batch sent without the API key", + url.origin, + ); + return undefined; + } + private resolveFetch(): FetchLike | null { if (this.options.fetchImpl) return this.options.fetchImpl; const globalFetch = (globalThis as { fetch?: FetchLike }).fetch; diff --git a/packages/analytics-core/src/types.ts b/packages/analytics-core/src/types.ts index 0ca654e88..df5019362 100644 --- a/packages/analytics-core/src/types.ts +++ b/packages/analytics-core/src/types.ts @@ -17,6 +17,8 @@ export type EventSource = export interface FetchResponseLike { ok: boolean; status: number; + /** Read only on a redirect, to follow its `location`. */ + headers?: { get(name: string): string | null }; } /** Request options the sender passes to `fetch` (or an injected replacement). */ @@ -25,6 +27,8 @@ export interface FetchRequestInit { headers: Record; body: string; signal?: AbortSignal; + /** Redirects are followed by the sender, which checks each hop. */ + redirect?: "manual"; } /** diff --git a/packages/browser-pay/package.json b/packages/browser-pay/package.json index 6b0c44ff2..76dafca66 100644 --- a/packages/browser-pay/package.json +++ b/packages/browser-pay/package.json @@ -30,6 +30,9 @@ "require": "./dist/cjs/index.js" } }, + "dependencies": { + "@sapiom/analytics-core": "workspace:^" + }, "files": [ "dist", "!dist/**/*.tsbuildinfo", diff --git a/packages/browser-pay/src/relay.test.ts b/packages/browser-pay/src/relay.test.ts index 2b2b55d0f..326a9ec47 100644 --- a/packages/browser-pay/src/relay.test.ts +++ b/packages/browser-pay/src/relay.test.ts @@ -66,7 +66,7 @@ describe("httpPaymentRelay", () => { expect(() => httpPaymentRelay({ ...options, baseUrl: "http://example.com" }), ).toThrow( - "Payment relay baseUrl must be https (http is only allowed on loopback)", + "@sapiom/browser-pay: refusing plaintext HTTP to http://example.com", ); for (const baseUrl of [ "https://api.sapiom.ai#/payment/relay", diff --git a/packages/browser-pay/src/relay.ts b/packages/browser-pay/src/relay.ts index 1cd25d52b..9105393bf 100644 --- a/packages/browser-pay/src/relay.ts +++ b/packages/browser-pay/src/relay.ts @@ -1,3 +1,10 @@ +import { + assertCredentialMayTravel, + credentialFetch, + resolveCredentialPolicy, + type CredentialPolicy, +} from "@sapiom/analytics-core/credential"; + import { parseRelayResponse, type RelayRequest, @@ -66,14 +73,12 @@ function isAbortError(error: unknown): boolean { ); } -function assertBaseUrl(baseUrl: string): void { +function assertBaseUrl(baseUrl: string, policy: CredentialPolicy): void { let url: URL; try { url = new URL(baseUrl); } catch { - throw new Error( - "Payment relay baseUrl must be https (http is only allowed on loopback)", - ); + throw new Error("Payment relay baseUrl must be a valid URL"); } if ( baseUrl.includes("#") || @@ -85,17 +90,8 @@ function assertBaseUrl(baseUrl: string): void { "Payment relay baseUrl must not contain a query or fragment", ); } - const host = url.hostname; - if (url.protocol === "https:") return; - if ( - url.protocol === "http:" && - (host === "localhost" || host === "127.0.0.1" || host === "[::1]") - ) { - return; - } - throw new Error( - "Payment relay baseUrl must be https (http is only allowed on loopback)", - ); + // Refuse at construction, not on the first payment. + assertCredentialMayTravel(url, policy); } function requireHttpOptions(options: HttpRelayOptions): { @@ -103,11 +99,15 @@ function requireHttpOptions(options: HttpRelayOptions): { apiKey: string; transactionId: string; fetchImpl: typeof fetch; + policy: CredentialPolicy; } { if (!options || typeof options.baseUrl !== "string" || !options.baseUrl) { throw new Error("Payment relay baseUrl is required"); } - assertBaseUrl(options.baseUrl); + const policy = resolveCredentialPolicy(undefined, { + label: "@sapiom/browser-pay", + }); + assertBaseUrl(options.baseUrl, policy); if (!options.transactionId) { throw new Error("Payment relay transactionId is required"); } @@ -122,6 +122,7 @@ function requireHttpOptions(options: HttpRelayOptions): { apiKey, transactionId: options.transactionId, fetchImpl: options.fetchImpl ?? fetch, + policy, }; } @@ -142,17 +143,22 @@ export function httpPaymentRelay(options: HttpRelayOptions): PaymentRelay { const url = `${config.baseUrl}/v1/transactions/${encodeURIComponent(config.transactionId)}/payment/relay`; let response: Response; try { - response = await config.fetchImpl(url, { - method: "POST", - redirect: "manual", - headers: { - accept: "application/json", - "content-type": "application/json", - "x-api-key": config.apiKey, + response = await credentialFetch( + config.fetchImpl, + url, + { + method: "POST", + redirect: "manual", + headers: { + accept: "application/json", + "content-type": "application/json", + }, + body: JSON.stringify(request), + signal: call?.signal, }, - body: JSON.stringify(request), - signal: call?.signal, - }); + { header: "x-api-key", value: config.apiKey }, + config.policy, + ); } catch (error) { throw transportError(error, call?.signal); } diff --git a/packages/core/README.md b/packages/core/README.md index dfcd70a34..65855269d 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -138,6 +138,9 @@ new SapiomClient(config: SapiomClientConfig) - `servicesDomain?: string` - Optional: Services domain for gateway clients (default: services.sapiom.ai) - `timeout?: number` - Optional: Request timeout in ms (default: 30000) - `headers?: Record` - Optional: Additional headers +- `allowInsecureHttp?: boolean` - Optional: Let the API key travel over plain `http://` to a non-loopback host, for a trusted private network (default: false) + +The API key only travels over `https://`, or plain `http://` to a loopback host (`localhost`, `*.localhost`, `127.0.0.1`, `[::1]`). Any other `http://` `baseURL` or `servicesDomain` (a Docker service name, a LAN IP) makes every request fail with a `CredentialChannelError` before anything is sent. On a trusted private network, opt in with `allowInsecureHttp: true` or `SAPIOM_ALLOW_INSECURE_HTTP=1`. Redirects to another origin never carry the key. #### Methods @@ -225,6 +228,7 @@ All integrations automatically read from environment: - `SAPIOM_API_KEY` (required) - `SAPIOM_BASE_URL` or `SAPIOM_API_URL` (optional) - `SAPIOM_TIMEOUT` (optional, in milliseconds) +- `SAPIOM_ALLOW_INSECURE_HTTP=1` (optional) - let the API key travel over plain `http://` to a non-loopback host ## Error Handling diff --git a/packages/core/package.json b/packages/core/package.json index b4c1ffa69..0f049fcef 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -52,7 +52,9 @@ "format": "prettier --write \"src/**/*.ts\"", "prepublishOnly": "pnpm build" }, - "dependencies": {}, + "dependencies": { + "@sapiom/analytics-core": "workspace:^" + }, "devDependencies": { "@types/jest": "^29.5.14", "@types/node": "^20.11.30", diff --git a/packages/core/src/client/HttpClient.channel.test.ts b/packages/core/src/client/HttpClient.channel.test.ts new file mode 100644 index 000000000..5099f9ebd --- /dev/null +++ b/packages/core/src/client/HttpClient.channel.test.ts @@ -0,0 +1,153 @@ +import { + ALLOW_INSECURE_HTTP_ENV, + CredentialChannelError, +} from "@sapiom/analytics-core/credential"; + +import { SapiomClient } from "./SapiomClient"; + +const mockFetch = jest.fn(); +global.fetch = mockFetch as any; + +const json = (status = 200, headers: Record = {}) => + new Response(status === 200 ? '{"ok":true}' : null, { + status, + headers: { "content-type": "application/json", ...headers }, + }); + +const sentHeaders = (call: number): Record => + mockFetch.mock.calls[call][1].headers; + +// A retry delay would show as a slow test; zero it so only the call count matters. +const RETRY = { maxAttempts: 3, baseDelayMs: 0 }; + +describe("HttpClient credential channel", () => { + const ORIGINAL = process.env[ALLOW_INSECURE_HTTP_ENV]; + beforeEach(() => { + mockFetch.mockReset(); + delete process.env[ALLOW_INSECURE_HTTP_ENV]; + }); + afterAll(() => { + if (ORIGINAL === undefined) delete process.env[ALLOW_INSECURE_HTTP_ENV]; + else process.env[ALLOW_INSECURE_HTTP_ENV] = ORIGINAL; + }); + + it("refuses plaintext to a non-loopback API host before sending, without retrying", async () => { + const client = new SapiomClient({ + apiKey: "sk_live", + baseURL: "http://api.internal", + retry: RETRY, + }); + const refusal = client.request({ url: "/x" }); + await expect(refusal).rejects.toBeInstanceOf(CredentialChannelError); + await expect(refusal).rejects.toThrow( + /^@sapiom\/core: refusing plaintext HTTP to http:\/\/api\.internal\..*new SapiomClient\(\{ allowInsecureHttp: true \}\) or SAPIOM_ALLOW_INSECURE_HTTP=1\.$/, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("refuses plaintext to a non-loopback vault host", async () => { + const client = new SapiomClient({ + apiKey: "sk_live", + servicesDomain: "http://svc.internal", + retry: RETRY, + }); + await expect(client.vault.getAll("demo")).rejects.toBeInstanceOf( + CredentialChannelError, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it("sends to loopback API and vault hosts", async () => { + mockFetch.mockImplementation(async () => json()); + const client = new SapiomClient({ + apiKey: "sk_live", + baseURL: "http://localhost:3000", + servicesDomain: "http://localhost:3999", + }); + await client.request({ url: "/x" }); + await client.vault.getAll("demo"); + expect(mockFetch.mock.calls.map((c) => c[0])).toEqual([ + "http://localhost:3000/v1/x", + "http://vault.localhost:3999/v2/secrets/demo", + ]); + expect(sentHeaders(0)["x-api-key"]).toBe("sk_live"); + expect(sentHeaders(1)["x-sapiom-api-key"]).toBe("sk_live"); + }); + + it.each([ + ["the client option", { allowInsecureHttp: true }, undefined], + ["the env var", {}, "1"], + ])( + "sends plaintext to a non-loopback host when opted in by %s", + async (_label, options, env) => { + if (env) process.env[ALLOW_INSECURE_HTTP_ENV] = env; + mockFetch.mockImplementation(async () => json()); + const client = new SapiomClient({ + apiKey: "sk_live", + baseURL: "http://api.internal", + ...options, + }); + await client.request({ url: "/x" }); + expect(sentHeaders(0)["x-api-key"]).toBe("sk_live"); + }, + ); + + it("setApiKey changes the key both clients send", async () => { + mockFetch.mockImplementation(async () => json()); + const client = new SapiomClient({ apiKey: "sk_old" }); + client.setApiKey("sk_new"); + await client.request({ url: "/x" }); + await client.vault.getAll("demo"); + expect(sentHeaders(0)["x-api-key"]).toBe("sk_new"); + expect(sentHeaders(1)["x-sapiom-api-key"]).toBe("sk_new"); + }); + + it("drops the key and x-sapiom-* headers on a redirect to another origin", async () => { + mockFetch + .mockResolvedValueOnce( + json(307, { location: "https://other.example/landed" }), + ) + .mockResolvedValueOnce(json()); + const client = new SapiomClient({ + apiKey: "sk_live", + headers: { "x-sapiom-trace": "t1" }, + }); + await client.request({ url: "/x" }); + expect(mockFetch.mock.calls.map((c) => c[0])).toEqual([ + "https://api.sapiom.ai/v1/x", + "https://other.example/landed", + ]); + expect(sentHeaders(0)["x-api-key"]).toBe("sk_live"); + expect(sentHeaders(1)).toEqual({ "Content-Type": "application/json" }); + }); + + it("keeps the key on a same-origin redirect", async () => { + mockFetch + .mockResolvedValueOnce(json(307, { location: "/v2/x" })) + .mockResolvedValueOnce(json()); + const client = new SapiomClient({ apiKey: "sk_live" }); + await client.request({ url: "/x" }); + expect(mockFetch.mock.calls[1][0]).toBe("https://api.sapiom.ai/v2/x"); + expect(sentHeaders(1)["x-api-key"]).toBe("sk_live"); + }); + + it("rejects a redirect to plaintext after one call, without retrying", async () => { + mockFetch.mockResolvedValue( + json(302, { location: "http://evil.internal/x" }), + ); + const client = new SapiomClient({ apiKey: "sk_live", retry: RETRY }); + await expect(client.request({ url: "/x" })).rejects.toBeInstanceOf( + CredentialChannelError, + ); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it("still retries a failed connection", async () => { + mockFetch + .mockRejectedValueOnce(new TypeError("fetch failed")) + .mockResolvedValueOnce(json()); + const client = new SapiomClient({ apiKey: "sk_live", retry: RETRY }); + await expect(client.request({ url: "/x" })).resolves.toEqual({ ok: true }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/core/src/client/HttpClient.ts b/packages/core/src/client/HttpClient.ts index c1176a621..9a9e75e05 100644 --- a/packages/core/src/client/HttpClient.ts +++ b/packages/core/src/client/HttpClient.ts @@ -1,5 +1,13 @@ import { randomUUID } from "node:crypto"; +import { + CredentialChannelError, + credentialFetch, + resolveCredentialPolicy, + type Credential, + type CredentialPolicy, +} from "@sapiom/analytics-core/credential"; + /** * Request configuration for the HTTP client */ @@ -33,21 +41,29 @@ export interface HttpClientConfig { headers: Record; retry?: RetryConfig; versionPrefix?: string | null; + /** The tenant key. Sent by `credentialFetch`, never through `headers`. */ + credential?: Credential; + /** Plain http to a non-loopback host may carry the credential. */ + allowInsecureHttp?: boolean; } -/** - * Core HTTP client for making API requests. - * Uses native fetch for zero external dependencies. - */ +/** Share credential transport policy so API and vault requests enforce the same channel constraints. */ export class HttpClient { private readonly baseURL: string; private readonly timeout: number; private headers: Record; private readonly retry: RetryConfig; private readonly versionPrefix: string | null; + private credential: Credential | undefined; + private readonly policy: CredentialPolicy; constructor(config: HttpClientConfig) { this.baseURL = config.baseURL; + this.credential = config.credential; + this.policy = resolveCredentialPolicy(config.allowInsecureHttp, { + label: "@sapiom/core", + optIn: "new SapiomClient({ allowInsecureHttp: true })", + }); this.timeout = config.timeout; this.headers = { ...config.headers }; const retry = config.retry ?? DEFAULT_RETRY; @@ -68,6 +84,10 @@ export class HttpClient { this.headers[key] = value; } + setCredential(credential: Credential): void { + this.credential = credential; + } + /** * Get client defaults (for testing and compatibility) */ @@ -75,6 +95,7 @@ export class HttpClient { baseURL: string; timeout: number; headers: Record; + credential: Credential | undefined; retry: RetryConfig; versionPrefix: string | null; } { @@ -82,6 +103,7 @@ export class HttpClient { baseURL: this.baseURL, timeout: this.timeout, headers: this.headers, + credential: this.credential, retry: this.retry, versionPrefix: this.versionPrefix, }; @@ -134,6 +156,9 @@ export class HttpClient { private isRetryable(error: unknown): boolean { if (!(error instanceof Error)) return false; + // A refused channel, first URL or redirect hop: it would be refused again + if (error instanceof CredentialChannelError) return false; + // Timeouts are NOT safe to retry (server may have processed) if (error.message.includes("Request timeout")) return false; @@ -161,12 +186,18 @@ export class HttpClient { const timeoutId = setTimeout(() => controller.abort(), this.timeout); try { - const response = await fetch(url, { - method: config.method || "GET", - headers, - body: this.prepareRequestBody(config.body), - signal: controller.signal, - }); + const response = await credentialFetch( + fetch, + url, + { + method: config.method || "GET", + headers, + body: this.prepareRequestBody(config.body), + signal: controller.signal, + }, + this.credential, + this.policy, + ); clearTimeout(timeoutId); diff --git a/packages/core/src/client/SapiomClient.test.ts b/packages/core/src/client/SapiomClient.test.ts index e44cd34cf..45e223cd4 100644 --- a/packages/core/src/client/SapiomClient.test.ts +++ b/packages/core/src/client/SapiomClient.test.ts @@ -52,7 +52,10 @@ describe("SapiomClient", () => { client.setApiKey("new-api-key"); const httpClient = client.getHttpClient(); - expect(httpClient.defaults.headers["x-api-key"]).toBe("new-api-key"); + expect(httpClient.defaults.credential).toEqual({ + header: "x-api-key", + value: "new-api-key", + }); }); }); diff --git a/packages/core/src/client/SapiomClient.ts b/packages/core/src/client/SapiomClient.ts index c4e2850fe..2eee0504e 100644 --- a/packages/core/src/client/SapiomClient.ts +++ b/packages/core/src/client/SapiomClient.ts @@ -1,6 +1,7 @@ import { ApiKeyAPI } from "./ApiKeyAPI.js"; import { HttpClient, HttpRequestConfig } from "./HttpClient.js"; import type { RetryConfig } from "./HttpClient.js"; +import type { Credential } from "@sapiom/analytics-core/credential"; import { IdentityManager } from "./IdentityManager.js"; import { TransactionAPI } from "./TransactionAPI.js"; import { VaultAPI } from "./VaultAPI.js"; @@ -51,6 +52,10 @@ export interface SapiomClientConfig { * @see IdentityManagerOptions.backgroundRefresh */ backgroundRefresh?: boolean; + + /** Permit plaintext credentials only for a trusted private network. + * Explicit configuration overrides SAPIOM_ALLOW_INSECURE_HTTP. */ + allowInsecureHttp?: boolean; } /** @@ -104,9 +109,10 @@ export class SapiomClient { timeout: config.timeout || 30000, headers: { "Content-Type": "application/json", - "x-api-key": config.apiKey, ...config.headers, }, + credential: { header: "x-api-key", value: config.apiKey }, + allowInsecureHttp: config.allowInsecureHttp, retry: config.retry, }); @@ -115,9 +121,10 @@ export class SapiomClient { timeout: config.timeout || 30000, headers: { "Content-Type": "application/json", - "x-sapiom-api-key": config.apiKey, ...config.headers, }, + credential: { header: "x-sapiom-api-key", value: config.apiKey }, + allowInsecureHttp: config.allowInsecureHttp, retry: config.retry, versionPrefix: null, }); @@ -135,8 +142,11 @@ export class SapiomClient { * Update the API key */ setApiKey(apiKey: string): void { - this.httpClient.setHeader("x-api-key", apiKey); - this.vaultHttpClient.setHeader("x-sapiom-api-key", apiKey); + this.httpClient.setCredential({ header: "x-api-key", value: apiKey }); + this.vaultHttpClient.setCredential({ + header: "x-sapiom-api-key", + value: apiKey, + }); } /** @@ -148,6 +158,7 @@ export class SapiomClient { baseURL: string; timeout: number; headers: Record; + credential: Credential | undefined; retry: RetryConfig; versionPrefix: string | null; }; diff --git a/packages/mcp/README.md b/packages/mcp/README.md index ba227f666..3cbf415d7 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -59,6 +59,11 @@ Both resolve from built-in presets, so no config file is required. A custom target can be defined in `~/.sapiom/credentials.json` (the server prints the expected shape if it encounters an unknown environment name). +The API key only travels over `https://`, or plain `http://` to a loopback host +(`localhost`, `*.localhost`, `127.0.0.1`, `[::1]`). A custom target on any other +`http://` host is refused before the request is sent. On a trusted private +network, set `SAPIOM_ALLOW_INSECURE_HTTP=1` in the server's `env`. + ## Authentication The first networked call (`link`, `deploy`, `run`, `inspect`, `signal`) needs a diff --git a/packages/mcp/src/tools/app-links-api.test.ts b/packages/mcp/src/tools/app-links-api.test.ts new file mode 100644 index 000000000..f1a98b4d4 --- /dev/null +++ b/packages/mcp/src/tools/app-links-api.test.ts @@ -0,0 +1,106 @@ +import { CredentialChannelError } from "@sapiom/analytics-core/credential"; +import { + afterAll, + afterEach, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +import { appLinksFetch } from "./app-links-api.js"; + +const ENV = "SAPIOM_ALLOW_INSECURE_HTTP"; +const ORIGINAL_ENV = process.env[ENV]; +const ORIGINAL_FETCH = globalThis.fetch; + +function mockFetch(...responses: Response[]): ReturnType { + const fetchMock = vi.fn(); + for (const r of responses) fetchMock.mockResolvedValueOnce(r); + globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch; + return fetchMock; +} + +const sentKey = (fetchMock: ReturnType, call: number) => + (fetchMock.mock.calls[call][1] as { headers: Record }) + .headers["x-api-key"]; + +describe("appLinksFetch credential channel", () => { + beforeEach(() => { + delete process.env[ENV]; + }); + afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH; + }); + afterAll(() => { + if (ORIGINAL_ENV === undefined) delete process.env[ENV]; + else process.env[ENV] = ORIGINAL_ENV; + }); + + it("refuses plaintext to a non-loopback host as a network failure, nothing sent", async () => { + const fetchMock = mockFetch(); + const result = await appLinksFetch( + "http://api.internal", + "sk_live", + "GET", + "/v1/app-links", + ); + expect(result).toMatchObject({ + kind: "network", + url: "http://api.internal/v1/app-links", + }); + const cause = (result as { cause: unknown }).cause; + expect(cause).toBeInstanceOf(CredentialChannelError); + expect(String(cause)).toMatch( + /@sapiom\/mcp: refusing plaintext HTTP.*opt in with SAPIOM_ALLOW_INSECURE_HTTP=1\.$/, + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it.each([ + ["the env opt-in", "http://api.internal", "1"], + ["a loopback host", "http://localhost:3000", undefined], + ])("sends over plain http with %s", async (_label, apiURL, env) => { + if (env) process.env[ENV] = env; + const fetchMock = mockFetch(new Response('{"items":[]}', { status: 200 })); + const result = await appLinksFetch( + apiURL, + "sk_live", + "GET", + "/v1/app-links", + ); + expect(result).toEqual({ + kind: "response", + status: 200, + ok: true, + data: { items: [] }, + }); + expect(sentKey(fetchMock, 0)).toBe("sk_live"); + }); + + it("drops x-api-key on a redirect to another origin", async () => { + const fetchMock = mockFetch( + new Response(null, { + status: 307, + headers: { location: "https://other.example/v1/app-links" }, + }), + new Response('{"items":[]}', { status: 200 }), + ); + await appLinksFetch( + "https://api.sapiom.ai", + "sk_live", + "POST", + "/v1/app-links", + { name: "x" }, + ); + expect(fetchMock.mock.calls.map((c) => c[0])).toEqual([ + "https://api.sapiom.ai/v1/app-links", + "https://other.example/v1/app-links", + ]); + expect([sentKey(fetchMock, 0), sentKey(fetchMock, 1)]).toEqual([ + "sk_live", + undefined, + ]); + }); +}); diff --git a/packages/mcp/src/tools/app-links-api.ts b/packages/mcp/src/tools/app-links-api.ts index 65b0a16e0..92ba1c31a 100644 --- a/packages/mcp/src/tools/app-links-api.ts +++ b/packages/mcp/src/tools/app-links-api.ts @@ -17,6 +17,10 @@ * `AgentOperationError`, while every App Links tool throws `PreviewOperationError` * (the sandbox-preview family these tools belong to). */ +import { + credentialFetch, + resolveCredentialPolicy, +} from "@sapiom/analytics-core/credential"; import { PreviewOperationError } from "@sapiom/sandbox-preview"; export type ToolResult = { @@ -73,14 +77,8 @@ export type AppLinksResponse = | { kind: "response"; status: number; ok: boolean; data: unknown } | { kind: "network"; url: string; cause: unknown }; -/** - * One JSON call against the backend's App Links REST API, authenticated with the - * cached `sapiom_authenticate` credential as `x-api-key`. - * - * The URL is concatenated, not `new URL(route, base)`: a custom `apiURL` carrying - * a base path (a local proxy, a tunnel) would have it silently dropped by the - * latter. A body-less method (`GET`, `DELETE`) sends no body and no content-type. - */ +/** Concatenate routes so a configured proxy base path is preserved. + * Refusals map to network; a refused redirect can follow an already-sent request. */ export async function appLinksFetch( apiURL: string, apiKey: string, @@ -91,14 +89,19 @@ export async function appLinksFetch( const url = `${apiURL.replace(/\/+$/, "")}${route}`; let res: Response; try { - res = await fetch(url, { - method, - headers: { - "x-api-key": apiKey, - ...(body === undefined ? {} : { "content-type": "application/json" }), + res = await credentialFetch( + fetch, + url, + { + method, + headers: + body === undefined ? {} : { "content-type": "application/json" }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), }, - ...(body === undefined ? {} : { body: JSON.stringify(body) }), - }); + { header: "x-api-key", value: apiKey }, + // Per call: the env opt-in is read when the request is made. + resolveCredentialPolicy(undefined, { label: "@sapiom/mcp" }), + ); } catch (cause) { return { kind: "network", url, cause }; } diff --git a/packages/tools/src/_client/index.ts b/packages/tools/src/_client/index.ts index 6f6e6acf7..8d1f391ee 100644 --- a/packages/tools/src/_client/index.ts +++ b/packages/tools/src/_client/index.ts @@ -1,31 +1,5 @@ -/** - * Auth + transport core for `@sapiom/tools` — the private "runtime half" every - * capability namespace wraps. Capability modules call THIS; they never import each - * other's runtime, so the cross-capability mesh (`repo.pushFromSandbox(sandbox)`) - * has no package/module cycles. See docs/plans/capability-authoring-sdk.md. - * - * The credential resolves from one of two sources, same surface either way: - * - explicit: createClient({ apiKey }) — standalone / open-source use - * - ambient: SAPIOM_API_KEY (env), which the workflow engine injects per - * execution so step authors write zero auth plumbing. - * - * Attribution (which agent / trace a call belongs to) is execution context the - * runtime owns, NOT a parameter of any operation — so it lives here on the - * transport, set once and injected on every request, never threaded through - * capability methods. The engine sets it per execution by constructing the - * per-execution client; `withAttribution(...)` is a quiet escape hatch for the - * router / standalone cases. Capability method signatures deliberately have no - * attribution argument: that absence keeps LLM-authored step code from setting - * (and getting wrong) context it doesn't own. - * - * Being the single HTTP choke point also makes this the (one) instrumentation - * seam: every call enqueues a `capability.call` usage event — synchronously, - * never awaited, live by default. See `./analytics.ts`. - * - * It is also where the credential's channel is enforced: https, or plain http - * to loopback only, and never across a redirect to another origin. See - * `./credential-policy.ts`. - */ +/** Shared transport keeps credential handling consistent across capability modules. + * See @sapiom/analytics-core/credential for channel and redirect constraints. */ import { CAPABILITY_CALL_EVENT, analyticsFor, @@ -33,11 +7,10 @@ import { type AnalyticsHolder, } from "./analytics.js"; import { - assertCredentialMayTravel, - fetchKeepingCredential, + credentialFetch, resolveCredentialPolicy, type CredentialPolicy, -} from "./credential-policy.js"; +} from "@sapiom/analytics-core/credential"; import { VERSION } from "../_generated/version.js"; import { TransportHttpError, @@ -53,6 +26,12 @@ import { */ const CLIENT_MARKER = `sapiom-tools/${VERSION}`; +/** Shared-policy refusals must identify this package's public opt-in. */ +const POLICY_WORDING = { + label: "@sapiom/tools", + optIn: "createClient({ allowInsecureHttp: true })", +}; + /** * Per-request attribution recorded with the gateway transaction. Every field is * optional and maps 1:1 to an `x-sapiom-*` header the collapsed flow understands. @@ -236,7 +215,10 @@ export class Transport { config.resumeToken ?? process.env.SAPIOM_CAPABILITY_RESUME_TOKEN ?? undefined; - this.policy = resolveCredentialPolicy(config.allowInsecureHttp); + this.policy = resolveCredentialPolicy( + config.allowInsecureHttp, + POLICY_WORDING, + ); } /** @@ -298,7 +280,6 @@ export class Transport { ); } const headers: Record = { - [options.authHeader ?? DEFAULT_AUTH_HEADER]: this.apiKey, "x-sapiom-client": CLIENT_MARKER, ...attributionToHeaders(this.attribution), }; @@ -312,12 +293,14 @@ export class Transport { try { // A malformed URL or a refused channel throws before anything is sent, and // is still counted as a failed call. - assertCredentialMayTravel(new URL(url), this.policy); - response = await fetchKeepingCredential( + response = await credentialFetch( this.fetchImpl, url, - init, - headers, + { ...init, headers }, + { + header: options.authHeader ?? DEFAULT_AUTH_HEADER, + value: this.apiKey, + }, this.policy, ); } catch (error) { diff --git a/packages/tools/src/_client/transport-security.spec.ts b/packages/tools/src/_client/transport-security.spec.ts index 902b5d3ae..672203738 100644 --- a/packages/tools/src/_client/transport-security.spec.ts +++ b/packages/tools/src/_client/transport-security.spec.ts @@ -2,7 +2,10 @@ import { createHash } from "node:crypto"; import { Transport, type TransportConfig } from "./index.js"; import { capabilityCall } from "./capability-call.js"; -import { ALLOW_INSECURE_HTTP_ENV, MAX_REDIRECTS } from "./credential-policy.js"; +import { + ALLOW_INSECURE_HTTP_ENV, + MAX_REDIRECTS, +} from "@sapiom/analytics-core/credential"; interface Hop { url: string; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f4ead315..3955c6475 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -281,6 +281,10 @@ importers: version: 5.9.3 packages/browser-pay: + dependencies: + '@sapiom/analytics-core': + specifier: workspace:^ + version: link:../analytics-core devDependencies: '@sapiom/tools': specifier: workspace:* @@ -372,6 +376,10 @@ importers: version: 5.9.3 packages/core: + dependencies: + '@sapiom/analytics-core': + specifier: workspace:^ + version: link:../analytics-core devDependencies: '@types/jest': specifier: ^29.5.14 diff --git a/scripts/credential-header-allowlist.json b/scripts/credential-header-allowlist.json new file mode 100644 index 000000000..9b4c3034e --- /dev/null +++ b/scripts/credential-header-allowlist.json @@ -0,0 +1,22 @@ +[ + { + "id": "harness-run-state", + "path": "packages/harness/src/core/run-state.ts", + "ticket": "SAP-4112" + }, + { + "id": "harness-mcp-config", + "path": "packages/harness/src/core/inject/mcp-config.ts", + "ticket": "SAP-4112" + }, + { + "id": "harness-definition-slug-resolver", + "path": "packages/harness/src/core/definition-slug-resolver.ts", + "ticket": "SAP-4112" + }, + { + "id": "harness-opencode-bridge", + "path": "packages/harness/src/server/opencode-bridge.ts", + "ticket": "SAP-4112" + } +] diff --git a/scripts/credential-header-check.mjs b/scripts/credential-header-check.mjs new file mode 100644 index 000000000..0576b7710 --- /dev/null +++ b/scripts/credential-header-check.mjs @@ -0,0 +1,115 @@ +#!/usr/bin/env node +/** Enforce shared credential transport for literal header writes (SAP-4106). + * Exact-path exceptions must still match; see credential-header-allowlist.json. */ +import { readFile, readdir } from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); +export const REPOSITORY_ROOT = path.resolve(SCRIPT_DIR, ".."); +const ALLOWLIST_PATH = path.join( + SCRIPT_DIR, + "credential-header-allowlist.json", +); + +/** Exempt the shared owner because it must attach credentials after checking the channel. */ +const OWNER = "packages/analytics-core/src/credential.ts"; + +const HEADER = String.raw`["'\x60]x-(?:sapiom-)?api-key["'\x60]`; +const FORBIDDEN = [ + new RegExp(String.raw`${HEADER}\s*:`, "gi"), + new RegExp(String.raw`\[\s*${HEADER}\s*\]\s*=(?!=)`, "gi"), + new RegExp(String.raw`\.(?:setHeader|set|append)\(\s*${HEADER}`, "gi"), +]; + +const SCANNED = /\.(?:ts|tsx|mts|js|mjs)$/; +const TEST_FILE = /\.(?:test|spec)\.[cm]?[jt]sx?$/; +const SKIPPED_DIRS = new Set(["node_modules", "dist", "__tests__"]); + +/** Preserve source line numbers so CI diagnostics point directly to matched header sites. */ +export function findCredentialHeaderSites(text) { + const lines = new Set(); + // Whole text, not line by line: a call's arguments may span lines. + for (const re of FORBIDDEN) { + for (const match of text.matchAll(re)) { + lines.add(text.slice(0, match.index).split("\n").length); + } + } + return [...lines].sort((a, b) => a - b); +} + +async function collect(root, dir, out) { + for (const entry of await readdir(path.join(root, dir), { + withFileTypes: true, + })) { + const child = path.posix.join(dir, entry.name); + if (entry.isDirectory()) { + if (!SKIPPED_DIRS.has(entry.name)) await collect(root, child, out); + } else if (SCANNED.test(entry.name) && !TEST_FILE.test(entry.name)) { + out.push(child); + } + } +} + +/** Exclude the shared owner so the guard audits consumers of the credential policy. */ +export async function sourceFiles(root = REPOSITORY_ROOT) { + const files = []; + for (const pkg of await readdir(path.join(root, "packages"), { + withFileTypes: true, + })) { + if (!pkg.isDirectory()) continue; + const src = path.posix.join("packages", pkg.name, "src"); + try { + await collect(root, src, files); + } catch (error) { + if (error?.code !== "ENOENT") throw error; + } + } + return files.filter((file) => file !== OWNER).sort(); +} + +/** Reject unapproved header sites and obsolete exceptions so exemptions remain tied to matching files. */ +export function audit(sources, allowlist) { + const allowed = new Set(allowlist.map((entry) => entry.path)); + const matched = new Set(); + const violations = []; + for (const { path: file, content } of sources) { + const lines = findCredentialHeaderSites(content); + if (lines.length === 0) continue; + matched.add(file); + if (!allowed.has(file)) { + for (const line of lines) violations.push(`${file}:${line}`); + } + } + const stale = allowlist.filter((entry) => !matched.has(entry.path)); + return { violations, stale }; +} + +export async function auditRepository(root = REPOSITORY_ROOT) { + const allowlist = JSON.parse(await readFile(ALLOWLIST_PATH, "utf8")); + const sources = await Promise.all( + (await sourceFiles(root)).map(async (file) => ({ + path: file, + content: await readFile(path.join(root, file), "utf8"), + })), + ); + return audit(sources, allowlist); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const { violations, stale } = await auditRepository(); + for (const site of violations) { + console.error( + `${site}: sets the tenant key header by hand. Pass the key to ` + + `credentialFetch from @sapiom/analytics-core/credential instead.`, + ); + } + for (const entry of stale) { + console.error( + `scripts/credential-header-allowlist.json: "${entry.id}" (${entry.path}) ` + + `no longer sets the header; remove the entry.`, + ); + } + if (violations.length > 0 || stale.length > 0) process.exit(1); + console.log("credential-header:check passed"); +} diff --git a/scripts/credential-header-check.test.mjs b/scripts/credential-header-check.test.mjs new file mode 100644 index 000000000..7b1aa635b --- /dev/null +++ b/scripts/credential-header-check.test.mjs @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { + audit, + auditRepository, + findCredentialHeaderSites, +} from "./credential-header-check.mjs"; + +describe("credential header guard", () => { + for (const [label, line] of [ + ["an object key", `fetch(url, { headers: { "x-api-key": key } });`], + ["a single-quoted object key", `const h = { 'X-Sapiom-Api-Key': k };`], + ["a bracket assignment", `h["x-sapiom-api-key"] = k;`], + ["setHeader", `client.setHeader("x-api-key", k);`], + ["Headers.set", `headers.set('x-api-key', k);`], + ["Headers.append", "headers.append(`x-sapiom-api-key`, k);"], + ["a setter split over lines", `headers.set(\n "x-api-key",\n k,\n);`], + ["an assignment split over lines", `h[\n "x-api-key"\n] = k;`], + ["an object key before a wrapped value", `({ "x-api-key":\n key })`], + ]) { + it(`flags ${label}`, () => { + assert.deepEqual(findCredentialHeaderSites(`ok\n${line}`), [2]); + }); + } + + for (const [label, line] of [ + ["a header name as a value", `const opts = { authHeader: "x-api-key" };`], + ["a type union", `type H = "x-api-key" | "x-sapiom-api-key";`], + ["a Set entry", `const s = new Set(["x-api-key", "cookie"]);`], + ["a comparison", `if (h["x-api-key"] === k) return;`], + ["a credential passed by value", `{ header: "x-api-key", value: key }`], + ]) { + it(`does not flag ${label}`, () => { + assert.deepEqual(findCredentialHeaderSites(line), []); + }); + } + + it("exempts an allowlisted file and fails a stale entry", () => { + const sources = [ + { path: "packages/a/src/x.ts", content: `({ "x-api-key": k })` }, + { path: "packages/b/src/y.ts", content: `({ "x-api-key": k })` }, + { path: "packages/c/src/z.ts", content: `credentialFetch()` }, + ]; + const allowlist = [ + { id: "a", path: "packages/a/src/x.ts", ticket: "SAP-1" }, + { id: "c", path: "packages/c/src/z.ts", ticket: "SAP-1" }, + ]; + const { violations, stale } = audit(sources, allowlist); + assert.deepEqual(violations, ["packages/b/src/y.ts:1"]); + assert.deepEqual( + stale.map((entry) => entry.id), + ["c"], + ); + }); + + it("passes on the repository", async () => { + assert.deepEqual(await auditRepository(), { violations: [], stale: [] }); + }); +});