feat(pipeline): Body mutation via WritesBody capability + SetBody helpers#389
Conversation
cwiklik
left a comment
There was a problem hiding this comment.
Review Summary
Well-structured PR that adds body mutation capability to the plugin framework with proper separation of concerns (ReadsBody vs WritesBody). The open plugin registry follows the proven database/sql.Register pattern with appropriate init-time panics. Pipeline validation is build-time (not runtime), which is the correct safety boundary.
Test coverage is thorough: 35 test functions covering the registry, capabilities, ordering validation, and end-to-end body mutation through all three listener types (ExtProc, ForwardProxy, ReverseProxy).
Design highlights:
- Single-mutator constraint prevents ambiguous ordering
- SHA256-only logging of mutated bodies (no cleartext leakage)
- Content-Encoding cleared on mutation (safe default)
NeedsBody()correctly folds WritesBody into buffering requirement
Areas reviewed: Go source (plugin framework, registry, pipeline validation, listeners), Go tests (unit + integration), Documentation
Commits: 13, all signed-off
CI: All 17 checks passing
…sBody)
PluginCapabilities.BodyAccess was a single flag meaning "I want the body
buffered." Splits into two booleans so the framework can distinguish
read-only observers (today's parsers) from would-be mutators:
ReadsBody — listener buffers body; plugin reads pctx.Body
WritesBody — plugin may call pctx.SetBody / pctx.SetResponseBody;
listener propagates the mutation to the wire
BodyAccess stays as a deprecated alias, folded into ReadsBody by a new
Normalize() helper. Dropped in the next release.
pipeline.New validates two body-mutation rules:
- At most one WritesBody plugin per pipeline (mutation ordering would
be ambiguous; the error names both plugins so an operator reading
pod logs can identify which two to reconcile).
- A WritesBody plugin cannot precede a ReadsBody plugin — readers
must see the original bytes, not a rewrite.
Pipeline.NeedsBody now checks ReadsBody || WritesBody so buffering kicks
in for pure mutators too. New Pipeline.WritesBody() lets listeners
fast-path the no-mutator case.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Plugins with WritesBody: true use these to mutate the body; the listener reads pctx.BodyMutated() / ResponseBodyMutated() after Run / RunResponse to decide whether to emit a mutation on the wire. SetBody / SetResponseBody auto-emit a modify-action Invocation (Reason: "body_rewritten") and publish a plugin-public event under "body-mutation/event" carrying phase, plugin name, length before/after, and sha256 before/after. Never includes the raw body bytes — the session store has no auth, so raw bodies would be a privacy / credential leak. The flags (not byte-diff) are the source of truth: a rewrite that produces byte-identical output still records the Invocation because "redactor ran, nothing matched" is valid telemetry. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
a2a-parser, mcp-parser, inference-parser all declare ReadsBody: true (the modern alias) instead of the deprecated BodyAccess. Behavior unchanged — Normalize() folds the old alias into the new field, so runtime semantics are identical. This just moves the in-tree plugins to the post-split surface before the alias is removed. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
extproc, forwardproxy, reverseproxy all previously decided "did a
plugin mutate the response body?" via a per-request heuristic:
string-compare against the buffered original in extproc, nil-check in
the two proxies. Both paths miss edge cases:
- string-compare is O(n) on every response, even when no plugin in
the pipeline could possibly mutate.
- nil-check misfires if a plugin ever assigns pctx.ResponseBody = nil
intentionally, and reacts to unrelated writes.
Replaced with pctx.ResponseBodyMutated() — a single bool set inside
Context.SetResponseBody. Listeners read it once after RunResponse. If
the proxy listeners take the mutation path, they also clear
Content-Encoding: the framework can't know whether the plugin
decompressed before rewriting, so shipping plain bytes without the
encoding header is safer than shipping a malformed archive.
Behavior today is unchanged — SetResponseBody is wired for the response
path; the only existing in-tree mutator path still uses direct
pctx.ResponseBody = ... assignment (which doesn't flip the flag). A
subsequent Phase B commit adds the request-body mutation path and a
migration guide for the direct-assignment usage.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
A plugin with WritesBody: true can now call pctx.SetBody() and have the
rewritten bytes reach the upstream, across all three listener modes.
Completes the request-side half of body mutation (the response side
was already partially plumbed; earlier commit in this branch formalized
it via the ResponseBodyMutated flag).
Per-listener changes:
extproc (handleInboundBody, handleOutboundBody)
New helper `withBodyMutation` decorates a RequestBody
ProcessingResponse with an ext_proc BodyMutation carrying pctx.Body
and appends content-encoding to the HeaderMutation RemoveHeaders.
No-op when pctx.BodyMutated() is false — zero cost for the common
read-only pipeline.
forwardproxy
After OutboundPipeline.Run, if pctx.BodyMutated(), rebuild
r.Body from pctx.Body, set r.ContentLength + Content-Length header,
and clear Content-Encoding.
reverseproxy
Same pattern as forwardproxy, on the inbound request before it's
handed to httputil.ReverseProxy.
Content-Encoding is cleared on every mutation path because the
framework can't know whether the plugin decompressed before rewriting;
shipping plain bytes without the old encoding header is safer than
shipping a malformed archive. Auto-decompress/recompress is a future
feature, out of scope here.
Tests (one per listener):
- A bodyMutatorPlugin declares WritesBody and rewrites pctx.Body.
- The upstream backend (or the ProcessingResponse in extproc's case)
must receive the new bytes with a correct Content-Length and no
Content-Encoding.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Three doc files updated to reflect the capability split, SetBody / SetResponseBody API, and per-listener wire propagation. framework-architecture.md §2 PluginCapabilities: struct snippet now shows ReadsBody / WritesBody / deprecated BodyAccess; prose explains how each affects listener negotiation. §3 Context ownership rules: new bullet on body read vs mutation, and the "don't assign pctx.Body directly" guidance. §6 Pipeline: new "Body mutation" subsection covering the capability model, build-time validation rules, mutation helpers, per-listener wire propagation, content-encoding policy, size limits, and explicit streaming scope note. §11 Versioning: changelog entry for the body-mutation feature. §12 Cross-references: package-source entries updated with new methods / fields. plugin-reference.md New "Body mutation" section between the Custom-map graduation criteria and "Registering a plugin." Field-level reference for PluginCapabilities (including the deprecated BodyAccess alias), the New-time validation rules, a mutation helper table, and the "NEVER log raw body" rule. Cross-links to framework-architecture §6.5 for the full lifecycle. plugin-tutorial.md Step 5 rewritten: BodyAccess -> ReadsBody, plus a new "Mutating the body" subsection with a Redactor example using SetBody and the ordering rules spelled out. No code changes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
…n tests PR rossoctl#388 (now on main) changed listener fields from *pipeline.Pipeline to *pipeline.Holder. The body-mutation integration tests in this PR were authored before that landed and passed raw *Pipeline to Server{…} / NewServer(…); post-rebase those four spots don't compile. Wrap with pipeline.NewHolder(): extproc/server_test.go:421 &Server{InboundPipeline: inbound, …} forwardproxy/server_test.go:204 &Server{OutboundPipeline: p, …} reverseproxy/server_test.go:174 NewServer(p, nil, backend.URL) Other call sites already use the outboundPipelineFromAuth / inboundPipelineFromAuth helpers, which return *Holder, so only the tests that build pipelines inline via pipeline.New needed changes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
fb72f86 to
d4430a8
Compare
Summary
Plugins can now rewrite request / response bodies. A plugin declares
WritesBody: trueand callspctx.SetBody(newBytes)orpctx.SetResponseBody(newBytes); all three listeners (ext_proc, forward proxy, reverse proxy) propagate the rewrite to the upstream with correctContent-Lengthand clearedContent-Encoding. Enables prompt-redaction, LLM output filtering, DLP-style guardrails, and any other content-transform plugin — without each author wiring their own wire protocol.Based on PR #386 (feat/open-plugin-registry). Merge that first; this rebases cleanly onto main afterward. Independent from PR #388 (hot-reload).
Before / after
Before — the
BodyAccess: trueflag told listeners to buffer. Response-body mutation was de-facto supported through undocumented diff-compare in three listeners (no capability gate, no Invocation). Request-body mutation was impossible.After
Capability model
ReadsBodypctx.Body/pctx.ResponseBodyWritesBodypctx.SetBody/pctx.SetResponseBodyReadsBody; propagates mutationsBodyAccess(deprecated)ReadsBodyNormalize(), removed in a future releasepipeline.Newenforces:WritesBodyper pipeline — multiple mutators = ambiguous ordering. Error names both plugins.WritesBodymust run after anyReadsBody— readers expect the original bytes.Per-listener wire behavior
extprocwithBodyMutationhelper wrapsRequestBody ProcessingResponsewith ext_procBodyMutationforwardproxypctx.BodyMutated()→ rebuildr.Body, setContent-Length, clearContent-Encodingpctx.ResponseBodyMutated()→ same pattern onresp.Bodyreverseproxyhttputil.ReverseProxymodifyResponseContent-Encoding is always cleared on mutation — framework can't know if the plugin decompressed; shipping plain bytes without the encoding header is safer than shipping a malformed archive. Auto-decompress/recompress is explicitly out of scope.
Observability
Every
SetBody/SetResponseBodyauto-emits:modify-action Invocation withReason: "body_rewritten", framework-attributed to the mutating plugin.body-mutation/evententry inpctx.Extensions.Customwith phase, plugin name, length before/after, sha256 before/after. Never the body content — the session store is unauthenticated.Already renders in
abctlvia the genericplugin:body-mutationfilter.Waypoint mode (ext_authz) — not supported
ext_authz has no body-mutation field in
CheckResponse. A pipeline combiningmode: waypointwith anyWritesBody: trueplugin is caught at runtime; documented inplugin-reference.md.Commit breakdown
feat(pipeline): Capability split for body mutation—ReadsBody+WritesBody+Normalize()+ build-time validationfeat(pipeline): pctx.SetBody / SetResponseBody helpers— mutation API + 10 unit tests + auto-emitted Invocation +body-mutation/eventwith sha256/length deltasrefactor(plugins): Flip parsers from BodyAccess to ReadsBody— a2a, mcp, inference parsers migrated to the new aliasrefactor(listeners): Use ResponseBodyMutated flag— replaces ad-hoc string-compare / nil-check heuristics in all 3 listenersfeat(listeners): Propagate request-body mutations to upstream— ext_procBodyMutationwrap +r.Bodyrebuild + Content-Encoding clearing; one integration test per listenerdocs: Body mutation lifecycle and plugin-author surface— framework-architecture §6.5 + plugin-reference + plugin-tutorialEach commit compiles and passes tests independently. Steps 1-4 are net no-op (capability rename, response-path tidy); step 5 is where request-body mutation actually becomes a user-visible feature.
Test plan
go test -race ./...green inauthlib/(10 new unit tests: Normalize, NeedsBody+WritesBody, two-mutator rejection, reader-after-mutator rejection, SetBody flag+invocation, SetBody custom event, SetResponseBody phase label)go test -race ./...green incmd/authbridge/(3 new listener integration tests: synthetic mutator plugin verifies each listener ships new bytes with correct Content-Length and cleared Content-Encoding)go vet ./...clean across both modules underGOWORK=offgofmt -lclean on touched filesOut of scope (possible followups)
authbridge_body_mutations_total{direction,phase}— deferred until there's a generic metrics surface (OTEL). Today's/statsis JWT/token-exchange specific.Content-Encodingon any mutation to avoid shipping malformed archives.prompt-sanitizer). The contract is in place; an actual production plugin is a separate PR driven by a concrete use case.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com