diff --git a/packages/core/src/config-summary.ts b/packages/core/src/config-summary.ts index 35f299b0..1a069749 100644 --- a/packages/core/src/config-summary.ts +++ b/packages/core/src/config-summary.ts @@ -44,9 +44,9 @@ export function pipelineStages( const pages = cfg.paginate.pages ?? cfg.paginate.max ?? 50; stages.push(opts.detailed ? `paginate (max ${pages})` : 'paginate'); } - // Post-response order matches the engine (engine.ts): transform → unwrap → validate. The body - // is transformed, then the unwrap path is read, then the result is validated against `output`. - if (cfg.transform) stages.push('transform'); + // Post-response order matches the engine (engine.ts): unwrap → validate. The unwrap path is + // read, then the result is validated against `output`. (`transform` is a live closure and lives + // only on `__rawConfig` — P0 — so the redacted public view can't report it as a stage.) if (cfg.unwrap) stages.push(`unwrap: ${cfg.unwrap}`); if (cfg.output) stages.push('validate'); if (cfg.cache) stages.push('cache'); diff --git a/packages/core/src/openapi.ts b/packages/core/src/openapi.ts index 00ca8880..07b0a42d 100644 --- a/packages/core/src/openapi.ts +++ b/packages/core/src/openapi.ts @@ -13,8 +13,9 @@ // projects onto `__config` from the live `auth` (the credential itself is stripped; a strategy with // no declarable scheme, e.g. a jar-mode `cookieSession`, is simply left unannotated). Query/header // parameters declared via an `input` SCHEMA (rather than the URL template) are still not enumerated. -// A stitch whose endpoint is a thunk (resolved at call time) cannot be exported statically; it is -// reported as a warning, never dropped silently. +// A stitch whose endpoint is a thunk (resolved at call time) cannot be exported statically; the +// thunk never survives redaction (P0 — no functions on `__config`), so it shows up here as an +// absent endpoint and is reported as a warning, never dropped silently. import { compact } from './compact'; import type { StitchRegistry } from './registry'; import { isStandardSchema } from './standard-schema'; @@ -181,14 +182,13 @@ const BODY_CONTENT_TYPE: Record< // Resolve a stitch's endpoint to a single string. `url` (string) is the whole endpoint; otherwise // `baseUrl` + `path` are joined — and when there is no `baseUrl`, `path` carries the whole endpoint -// (the `stitch('https://…')` string form composes to `path`). A thunk `url`/`baseUrl` is resolved -// at call time and cannot be exported statically. -// `null` = nothing to export; `{ thunk: true }` = endpoint resolved at call time (unexportable). -type ResolvedEndpoint = { value: string } | { thunk: true } | null; +// (the `stitch('https://…')` string form composes to `path`). A thunk `url`/`baseUrl` never survives +// redaction (P0 — no functions on `__config`), so an absent endpoint here means "never configured OR +// resolved at call time" — either way there is nothing static to export. +// `null` = nothing to export. +type ResolvedEndpoint = { value: string } | null; function endpointOf(cfg: RedactedStitchConfig): ResolvedEndpoint { - if (typeof cfg.url === 'function' || typeof cfg.baseUrl === 'function') - return { thunk: true }; if (typeof cfg.url === 'string') return { value: cfg.url }; const base = typeof cfg.baseUrl === 'string' ? cfg.baseUrl.replace(/\/+$/, '') : ''; @@ -346,12 +346,8 @@ export function toOpenApi( const cfg = stitch.__config; const endpoint = endpointOf(cfg); if (endpoint === null) { - warnings.push(`skipped "${key}": no url/baseUrl/path to export`); - continue; - } - if ('thunk' in endpoint) { warnings.push( - `skipped "${key}": endpoint is a thunk (resolved at call time), not a static string`, + `skipped "${key}": no static url/baseUrl/path to export (a thunk endpoint is resolved at call time and never survives redaction)`, ); continue; } diff --git a/packages/core/src/stitch.ts b/packages/core/src/stitch.ts index 672677c1..a5caec1c 100644 --- a/packages/core/src/stitch.ts +++ b/packages/core/src/stitch.ts @@ -662,9 +662,24 @@ export interface SharedRuntime { register?: (s: Stitch) => void; } +// Return a config slot with every function-valued own field dropped (CONTRACT.md P0): a derivation +// fn — `paginate.next`/`items`, a predicate `retry.on`/`throttle.on`, the `key`/`keyOf` on +// `idempotency`/`cache` (canonical or @deprecated alias) — never reaches `__config`, while fn-free +// data (`pages`, `attempts`, a `number[]` `on`, `ttl`) stays. A scalar-shorthand slot (`cache: '1m'`, +// `retry: 3`) has no fields to strip and passes through untouched. Shallow (these slots carry their +// functions at depth 1) and non-mutating — a fresh object is built, so `__rawConfig` is left intact. +function stripFns(slot: unknown): unknown { + if (slot === null || typeof slot !== 'object' || Array.isArray(slot)) + return slot; + return Object.fromEntries( + Object.entries(slot).filter(([, v]) => typeof v !== 'function'), + ); +} + // `__config` is the PUBLIC view; strip the live secret-bearing handles so the running store, -// credential, and transport cannot be read back off a stitch (ADR 0002 §4/§6, exfil-at-rest). -// The full config lives on `__rawConfig` for fragment composition (see `asConfig`). +// credential, and transport cannot be read back off a stitch (ADR 0002 §4/§6, exfil-at-rest), and +// strip EVERY function-valued field so it is plain JSON data (CONTRACT.md P0). The full config — +// handles and function sugar alike — lives on `__rawConfig` for fragment composition (see `asConfig`). export function redactConfig(cfg: ResolvedStitchConfig): RedactedStitchConfig { // Split off the live `Surface` so the spread carries no `kind: Surface`; the rest still holds // the live store/auth/adapter handles, stripped next. @@ -676,8 +691,7 @@ export function redactConfig(cfg: ResolvedStitchConfig): RedactedStitchConfig { clock?: unknown; }; // Strip the live, secret-bearing handles so the running store, credential, and transport cannot - // be read back off a stitch (ADR 0002 §4/§6, exfil-at-rest). The full config lives on the - // non-enumerable `__rawConfig` for fragment composition (see `asConfig`). + // be read back off a stitch (ADR 0002 §4/§6, exfil-at-rest). delete redacted.store; delete redacted.auth; delete redacted.adapter; @@ -691,6 +705,43 @@ export function redactConfig(cfg: ResolvedStitchConfig): RedactedStitchConfig { // Normalise the surface to its id string so __config round-trips as JSON (ADR 0005 Decision 11): // never expose the live Surface (its hooks don't serialise), only its identity. if (kind) redacted.kind = kind.id; + // CONTRACT.md P0: strip EVERY function-valued field so the public `__config` is plain JSON — the + // url/baseUrl thunks, `transform`, `hooks`, `paginate.next`/`items`, the predicate forms of + // `retry.on`/`throttle.on`/`acceptStatus` (a `number[]` stays), `idempotency.keyOf`, and + // `cache.key`. The engine reads all that sugar off the non-enumerable `__rawConfig`; nothing + // reads it off `__config` (see e.g. `toOpenApi`, which detects a thunked endpoint off the raw + // config). Mutate through a plain-data alias: the redacted view drops the fns and several fields + // become partial JSON shapes the still-fn-typed resolved fields reject (`__config`'s shape is + // pinned by the P0 spec + the JSON-roundtrip contract gate). Each reassignment builds a FRESH + // object so `cfg` — i.e. `__rawConfig` — is never mutated. (#470 narrows the types, retiring the + // alias.) + const data = redacted as { + url?: unknown; + baseUrl?: unknown; + transform?: unknown; + hooks?: unknown; + acceptStatus?: unknown; + paginate?: unknown; + retry?: unknown; + throttle?: unknown; + idempotency?: unknown; + cache?: unknown; + }; + if (typeof cfg.url === 'function') delete data.url; + if (typeof cfg.baseUrl === 'function') delete data.baseUrl; + delete data.transform; + delete data.hooks; + if (typeof cfg.acceptStatus === 'function') delete data.acceptStatus; + // Each nested slot is copied FRESH (so `cfg` — i.e. `__rawConfig` — is never mutated), then has + // its function-valued fields dropped: `paginate.next`/`items`, the predicate `retry.on`/ + // `throttle.on` (a `number[]` stays — only functions go), and the `key`/`keyOf` derivations on + // `idempotency`/`cache`. The dynamic strip covers both a canonical name and its @deprecated alias + // without naming either (so it neither trips the no-deprecated lint nor rots on a future rename). + if (cfg.paginate) data.paginate = stripFns(cfg.paginate); + if (cfg.retry) data.retry = stripFns(cfg.retry); + if (cfg.throttle) data.throttle = stripFns(cfg.throttle); + if (cfg.idempotency) data.idempotency = stripFns(cfg.idempotency); + if (cfg.cache) data.cache = stripFns(cfg.cache); return redacted; } diff --git a/packages/core/test/config-summary.spec.ts b/packages/core/test/config-summary.spec.ts index 3f6fc258..a5217b48 100644 --- a/packages/core/test/config-summary.spec.ts +++ b/packages/core/test/config-summary.spec.ts @@ -68,14 +68,14 @@ describe('pipelineStages', () => { output: () => true, cache: '1m', }); - // Post-response order is transform → unwrap → validate (engine.ts), bookended by call/result. + // Post-response order is unwrap → validate (engine.ts), bookended by call/result. `transform` + // is a live closure (P0 — off the public `__config`), so the redacted summary omits it. expect(pipelineStages(full, { detailed: true })).toEqual([ 'call', 'throttle', 'POST https://api.example.com/widgets', 'retry ×3', 'paginate (max 7)', - 'transform', 'unwrap: data', 'validate', 'cache', diff --git a/packages/core/test/contract-p0.spec.ts b/packages/core/test/contract-p0.spec.ts new file mode 100644 index 00000000..fc25dc2b --- /dev/null +++ b/packages/core/test/contract-p0.spec.ts @@ -0,0 +1,102 @@ +// CONTRACT.md P0 regression: the enumerable `__config` is plain JSON data. Every function-valued +// field an author can write — endpoint thunks, `transform`, `paginate.next`/`items`, `hooks.*`, the +// predicate forms of `acceptStatus`/`retry.on`/`throttle.on`, `idempotency.keyOf`, `cache.keyOf` — +// must be stripped by redaction; the engine reads that sugar off the non-enumerable `__rawConfig`. +// This guards two things at once: the exfil-at-rest surface (a public config view must not carry +// live author closures — ADR 0002) and JSON-serialisability (a function silently drops on +// `JSON.stringify`, so a leaked one corrupts every trace / report / `mcp` view of the stitch). +import { stitch } from '../src'; +import type { StitchConfig } from '../src'; + +import { describe, expect, test } from 'vitest'; + +// Recursively collect the paths of every function-valued field on an object graph. `__config` only +// carries own enumerable data, so a plain `Object.entries` walk is the honest P0 probe. +function fnPaths(value: unknown, path = '$'): string[] { + if (typeof value === 'function') return [path]; + if (value === null || typeof value !== 'object') return []; + const out: string[] = []; + for (const [k, v] of Object.entries(value as Record)) { + out.push(...fnPaths(v, `${path}.${k}`)); + } + return out; +} + +const rawConfigOf = (f: unknown): StitchConfig => + (f as { __rawConfig: StitchConfig }).__rawConfig; + +describe('CONTRACT.md P0 — __config is plain JSON data', () => { + // Every function-valued slot an author can populate, in one stitch. + const laden = stitch({ + name: 'p0-fn-laden', + url: () => 'https://api.example.test/items', + transform: (body) => body, + paginate: { + next: () => undefined, + items: () => [], + pages: 3, + }, + hooks: { + onRequest: () => undefined, + onResponse: () => undefined, + onError: () => undefined, + onRetry: () => undefined, + }, + acceptStatus: (status) => status === 404, + retry: { attempts: 2, on: (status) => status >= 500 }, + throttle: { rate: '2/s', on: (status) => status === 429 }, + idempotency: { keyOf: () => 'p0-idem' }, + cache: { + ttl: '1m', + // The canonical derivation-fn name (not the @deprecated `key`): redaction must strip + // BOTH, so exercising `keyOf` here pins the name-agnostic strip. + keyOf: () => 'p0-cache', + vary: ['accept'], + methods: ['GET'], + }, + }); + + test('the fn-laden stitch exposes zero function-valued paths on __config', () => { + expect(fnPaths(laden.__config)).toEqual([]); + }); + + test('__config survives a JSON round-trip unchanged', () => { + expect(JSON.parse(JSON.stringify(laden.__config))).toEqual( + laden.__config, + ); + }); + + test('function-valued sugar is redacted field by field', () => { + const cfg = laden.__config; + // A thunked endpoint has no static string — the slot is absent, not stringified. + expect(cfg.url).toBeUndefined(); + expect(cfg).not.toHaveProperty('transform'); + expect(cfg).not.toHaveProperty('hooks'); + // Fn-free data survives; the paginate/keyOf/predicate fns are gone. + expect(cfg.paginate).not.toHaveProperty('next'); + expect(cfg.paginate).not.toHaveProperty('items'); + expect(cfg.paginate?.pages).toBe(3); + expect(cfg.acceptStatus).toBeUndefined(); // the predicate form is redacted + expect(cfg.retry).not.toHaveProperty('on'); // the predicate `on` is redacted + expect(cfg.retry?.attempts).toBe(2); + expect(cfg.throttle).not.toHaveProperty('on'); + expect(cfg.throttle?.rate).toBe('2/s'); + expect(cfg.idempotency).not.toHaveProperty('keyOf'); + expect(cfg.cache).not.toHaveProperty('keyOf'); + expect(cfg.cache).not.toHaveProperty('key'); + expect(cfg.cache?.ttl).toBe('1m'); + }); + + test('the engine-side sugar lives on the non-enumerable __rawConfig', () => { + const raw = rawConfigOf(laden); + expect(typeof raw.url).toBe('function'); + expect(typeof raw.transform).toBe('function'); + expect(typeof raw.paginate?.next).toBe('function'); + expect(typeof raw.hooks?.onRequest).toBe('function'); + expect(typeof raw.acceptStatus).toBe('function'); + expect(fnPaths(raw.cache)).toContain('$.keyOf'); // the cache derivation fn is retained raw + // Neither meta property is enumerable — a spread / JSON view of the stitch leaks nothing. + expect(Object.keys(laden)).not.toContain('__config'); + expect(Object.keys(laden)).not.toContain('__rawConfig'); + }); +}); diff --git a/packages/core/test/diagram.spec.ts b/packages/core/test/diagram.spec.ts index 67e4d6b7..35c7af6a 100644 --- a/packages/core/test/diagram.spec.ts +++ b/packages/core/test/diagram.spec.ts @@ -45,10 +45,11 @@ describe('toMermaid', () => { expect(diagram).toContain('(["result"])'); }); - test('post-response stages render in engine order: transform -> unwrap -> validate', () => { + test('post-response stages render in engine order: unwrap -> validate', () => { // The engine processes the response body transform → unwrap → validate (engine.ts), and the - // diagram's contract is "the configured request pipeline … in engine order". So the diagram - // must place validate LAST of the three, not first. + // diagram's contract is "the configured request pipeline … in engine order". `transform` is a + // live closure (P0 — off the public `__config`), so the diagram omits it; of the two stages it + // can report, validate must come LAST, not first. const { diagram } = toMermaid({ proc: stitch({ baseUrl: 'https://api.example.com', @@ -58,11 +59,11 @@ describe('toMermaid', () => { output: z.object({ id: z.number() }), }), }); - const iTransform = diagram.indexOf('transform'); const iUnwrap = diagram.indexOf('unwrap: data'); const iValidate = diagram.indexOf('validate'); - expect(iTransform).toBeGreaterThanOrEqual(0); - expect(iUnwrap).toBeGreaterThan(iTransform); // unwrap after transform + // `transform` is a closure and never reaches the redacted `__config`, so it isn't diagrammed. + expect(diagram).not.toContain('transform'); + expect(iUnwrap).toBeGreaterThanOrEqual(0); expect(iValidate).toBeGreaterThan(iUnwrap); // validate after unwrap });