Skip to content

fix(core)!: redact function-valued config off the public __config (P0)#477

Open
rejifald wants to merge 1 commit into
mainfrom
fix/core-p0-config-json
Open

fix(core)!: redact function-valued config off the public __config (P0)#477
rejifald wants to merge 1 commit into
mainfrom
fix/core-p0-config-json

Conversation

@rejifald

Copy link
Copy Markdown
Owner

The bug (CONTRACT.md P0)

__config is the public, JSON view of a stitch — the redacted surface that .report(),
trace sinks, mcp, .diagram(), and stitch export --openapi all read. It is supposed to
be plain data (ADR 0005 Decision 11 — it round-trips as JSON). It wasn't: it carried every
function an author configured
. A probe on main finds 13 live closures on it:

$.url  $.transform  $.paginate.next  $.paginate.items  $.acceptStatus  $.retry.on
$.throttle.on  $.idempotency.keyOf  $.cache.key  $.hooks.{onRequest,onResponse,onError,onRetry}

redactConfig only stripped the four capability handles (store/auth/adapter/clock)
and did a shallow spread, so every author function rode straight onto the public view.
Two consequences:

  1. Exfil-at-rest (ADR 0002 §4/§6): a closure can capture secrets from its lexical scope;
    the public config is exactly the surface that boundary exists to keep clean.
  2. Silent corruption: a function vanishes on JSON.stringify, so any serialized view of
    the stitch (trace, report, MCP payload) was lossy and could not round-trip.

The fix

redactConfig now strips every function-valued field so __config is plain JSON. The
engine already reads all that sugar off the non-enumerable __rawConfig (which also
backs fragment composition — asConfig), so nothing runtime-relevant changes. Details:

  • Each nested slot is rebuilt fresh (__rawConfig is never mutated).
  • A small stripFns drops function-valued fields by value, not by name — so it removes
    both a canonical derivation fn and its @deprecated alias (idempotency/cache
    key/keyOf) and won't rot on a future rename; a scalar-shorthand slot (cache: '1m')
    passes through untouched. A number[] on (status list) stays; only the predicate form goes.

Two consumers that introspected those functions off __config are rerouted:

  • toOpenApi detected a thunk endpoint via typeof cfg.url === 'function'. A thunk no
    longer survives redaction, so "absent endpoint" now subsumes "thunk endpoint" — the two
    branches fold into one skip-with-warning (the warning still names the thunk case).
  • pipelineStages (feeds .diagram() and mcp) drops the transform stage: a live
    closure can't be reported from the redacted view. Its two tests are updated to match.

Test

New contract-p0.spec.ts pins the invariant: zero fn-paths on __config, a lossless
JSON round-trip, field-by-field redaction (incl. the canonical cache.keyOf, which a
name-based strip would miss), and the sugar retained on __rawConfig. Verified red on
main
(3/4 fail, including the JSON round-trip) and green with the fix.

Scope note

The runtime leak is fixed here; the type RedactedStitchConfig still structurally
carries the fn forms (it's Omit<ResolvedStitchConfig, caps>), so the public view is typed
looser than it now behaves — #470 additionally narrows the type. One localized alias in
redactConfig bridges the still-loose type; it's commented to retire on that narrowing.

Carved out of the #470 contract-freeze sweep — the single highest-value semantic change in
it — so this load-bearing redaction fix lands and is reviewed on its own.

Verification

pnpm --filter stitchapi test (1218 pass) + check:types + eslint clean; full-workspace
pre-push suite green.

🤖 Generated with Claude Code

The enumerable `__config` — the public, JSON view of a stitch that `.report()`, traces,
`mcp`, `diagram`, and `stitch export --openapi` all read — leaked every function an author
configured: url/baseUrl thunks, `transform`, `hooks.*`, `paginate.next`/`items`, the
predicate forms of `acceptStatus`/`retry.on`/`throttle.on`, and the `idempotency`/`cache`
`keyOf`. Thirteen live closures on a surface that is meant to be plain JSON data
(CONTRACT.md P0). Two consequences: exfil-at-rest (a closure can capture secrets — ADR 0002
§4/§6) and silent corruption (a function drops on `JSON.stringify`, so every serialised view
of the stitch was lossy).

`redactConfig` now strips every function-valued field. The engine already reads all that
sugar off the non-enumerable `__rawConfig` (which also backs fragment composition), so
nothing runtime-relevant changes. Each nested slot is copied FRESH so `__rawConfig` is never
mutated; a dynamic `stripFns` drops the derivation fns without naming a field, so it removes
both a canonical name and its @deprecated alias (`key`/`keyOf`) and won't rot on a rename.

Two consumers that introspected those functions off `__config` are rerouted:

- `toOpenApi`: a thunk endpoint no longer survives redaction, so an absent endpoint now
  subsumes "thunk endpoint" — folded into the single skip-with-warning branch (the warning
  still names the thunk case).
- `pipelineStages` (diagram / mcp): drops the `transform` stage — a live closure can't be
  reported from the redacted view. Its two tests are updated to match.

Adds `contract-p0.spec.ts`: zero fn-paths on `__config`, a lossless JSON round-trip,
field-by-field redaction, and the sugar retained on `__rawConfig` (verified RED without the
fix). The type still carries the fn forms (loose); #470 additionally narrows
`RedactedStitchConfig`.

Carved out of the #470 contract-freeze sweep so this load-bearing redaction fix lands and is
reviewed on its own.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant