feat: redact the OpenAI Responses API - #70
Conversation
POST /v1/responses is what current OpenAI clients use by default and it passed through cordon verbatim, so a modern client pointed at cordon sent raw PII upstream with X-Redacted: 0. The route now normalizes to provider openai, dialect responses. The request walk covers instructions (under REDACT_SYSTEM), input as a string or item list (input_text parts, function_call arguments as parsed JSON, function_call_output text) and flat function tool definitions; image and file parts are left alone. Non-streaming replies restore output_text and refusal parts. Streaming restores output_text.delta through the existing hold-back buffer, re-emitting each delta under its original item_id, output_index and content_index, and restores the full text carried by output_text.done, content_part.done, output_item.done and response.completed. The echo stub answers /v1/responses in both shapes; the proxy suite gains 19 checks and the stream suite 8.
github-actions
Bot
added
redaction
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: request changes — Responses refusal streaming changes the event type and therefore the response semantics.
Blocking — correctness: src/streaming.ts:632-638, src/providers.ts:318-322
if (type === "response.output_text.delta" || type === "response.refusal.delta") {
emitText(reider.push(j.delta ?? ""));
const data = { type: "response.output_text.delta", ...ctx, delta: text };
A streamed Responses refusal enters the shared branch, but emitText always calls the adapter that creates a response.output_text.delta frame. For example, an upstream response.refusal.delta containing a placeholder is re-emitted as response.output_text.delta, so a Responses client that consumes refusal events never receives its incremental refusal (and may instead expose it as ordinary output text). The subsequent response.refusal.done does not repair the incorrect delta event sequence. Preserve the inbound delta kind in the emission context (or provide distinct output-text/refusal frame builders), and add a streaming refusal regression test that verifies the event type and restored text.
// Preserve the upstream event kind when rebuilding a Responses delta.
const data = { type: ctx.type ?? "response.output_text.delta", ...ctx, delta: text };
I reviewed the endpoint routing, request field walk, non-streaming response slots, and the Responses streaming frame handling. CI's analyze, docker-build, and test jobs were pending at review time.
sprayberry-secondread
left a comment
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).
Three correctness issues found at 2956916b8b365bf0e6a25706d274187ca47b5ac4: one redaction gap and two streaming defects.
1. High: assistant history is forwarded with restored PII
src/redact/apply.ts:94:
if ((part.type === "text" || part.type === "input_text") && typeof part.text === "string") {The new Responses walk calls this helper for input[].content at line 137, but Responses also accepts prior assistant messages containing output_text parts. A normal stateless conversation appends the previous response's output to the next request's input. Cordon restores PII in that response, so an assistant history item such as {type:"message",role:"assistant",content:[{type:"output_text",text:"Contact john@acme.com",annotations:[]}]} reaches this condition, fails both type comparisons, and is forwarded upstream with the raw email. With a clean new user message, no other slot catches that email. The added tests only exercise input_text and do not cover the round trip.
Suggested fix:
In the Responses request walk, redact output_text.text and refusal.refusal
in prior assistant message content as well as input_text.text. Add a
multi-turn regression that feeds a restored assistant output back as input
and checks the captured upstream body contains no raw PII.
2. Medium: replacement-string expansion corrupts full-text SSE frames
src/streaming.ts:28:
return frame.replace(/^data:.*$/m, `data: ${JSON.stringify(j)}`);The replacement is a string, so JavaScript interprets $&, $\`` and $'inside model text as replacement metacharacters rather than literal content. For example, an ordinaryresponse.output_text.donecarryingtext: "price $&"gets its originaldata:` line inserted inside the JSON string without escaping. A minimal Node reproduction of this exact expression produced invalid JSON. This happens even without any redacted entities, because the reversible Responses path reframes every full-text event. Code snippets explaining JavaScript replacements are a concrete source of these strings.
Suggested fix:
return frame.replace(/^data:.*$/m, () => `data: ${JSON.stringify(j)}`);Add a streaming regression with literal replacement metacharacters, asserting every emitted data payload parses and retains the exact text.
3. Medium: refusal deltas are rewritten as output-text deltas
src/streaming.ts:110,113-115:
if (type === "response.output_text.delta" || type === "response.refusal.delta") {
// ...
const { type: _t, delta: _d, ...ctx } = j;
emitCtx = ctx;
emitText(reider.push(j.delta ?? ""));src/providers.ts:94-95:
const data = { type: "response.output_text.delta", ...ctx, delta: text };
return `event: response.output_text.delta\ndata: ${JSON.stringify(data)}\n\n`;For an upstream refusal part followed by response.refusal.delta with delta: "I cannot help", the original type is removed from the context and the adapter hardcodes both the JSON type and SSE event to response.output_text.delta. A consumer listening for refusal deltas receives none; a typed stream accumulator is instead given text events addressed to a refusal part. Restoring refusal.done later does not repair incremental event delivery. The stub emits only output-text events, so this branch is not tested.
Suggested fix:
Keep the incoming delta type in the emission context, including while a
placeholder tail is buffered, and derive both the SSE event name and JSON
type from it. Add a refusal stream regression with split placeholders and
assert it emits refusal.delta, never output_text.delta.
What's good: dialect selection keeps the existing provider identity for audit records, the shared JSON-argument helper avoids duplicating numeric redaction handling, and the streaming tests check all four full-text completion representations rather than only concatenated deltas.
Scope: read the production diff, stub changes and new proxy/stream assertions, plus surrounding streaming/request-walk context. GitHub test, Docker and CodeQL checks pass. No local test suite was run; the replacement-string issue was checked with a minimal standalone Node expression. No other review bodies were read.
A streamed response.refusal.delta shared the re-emit path with response.output_text.delta, but frameFromText hardcoded the output-text type on both the SSE event line and the data payload. A refusal reached the client as ordinary output text, so a client consuming refusal events never saw its incremental refusal. streaming.ts now keeps the upstream type in the emission context, and the Responses adapter emits that type (falling back to output_text for any other caller). Refs FIX-e5e0127b.
…ts text Adds a refusal-flavoured Responses stream to the stub (FORCE_REFUSAL in the input) and asserts the re-emitted deltas stay response.refusal.delta on both the event: line and the data payload, that no output_text delta leaks, and that the split placeholder is restored in the deltas and in refusal.done. Refs FIX-e5e0127b.
… reframing A stateless Responses client appends the previous reply's output to the next request's input. Those parts are output_text and refusal, which the request walk did not recognise, so a restored email went back upstream raw on the second turn. Both part kinds are now redacted, with a multi-turn regression that checks the captured upstream body. reframe replaced the data: line with a string, so $&, $` and $' in model text were replacement metacharacters and spliced the original line into the payload. It takes a function now.
|
All three Second Read findings and Redline's blocking item are addressed on the branch.
|
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: changes requested — Responses streaming mixes text from distinct output parts before restoration.
Blocking finding
High — src/streaming.ts:118-120
const { delta: _d, ...ctx } = j;aemitCtx = ctx;emitText(reider.push(j.delta ?? ""));`
There is one StreamReidentifier for the entire Responses stream, while emitCtx simply replaces the previous value with the most recent delta frame. Responses deltas are addressed by item_id/output_index/content_index, so text from separate content parts can be interleave. For example, if content part A emits <EMA, then content part B emits hello, then A another delta, the shared buffer combines them. The placeholder cannot be resolved, and any eventual re-emission is tagged with B's address rather than A's. The client receives corrupted, unrestored content even though each source part is valid.
Keep a separate reidentifier and address context per item_id/output_index/content_index (and flush the matching instance on that part's done event), rather than sharing state across the whole response stream.
const key = `${j.item_id}/${j.output_index}/${j.content_index}`;
const state = streams.get(key) ?? { reider: new StreamReidentifier(vault), ctx };
streams.set(key, state);
emitText(state.reider.push(j.delta ?? ""), 0, state.ctx);
// On the matching *.done event, flush and remove streams.get(key).
What I checked: the live-head diff for request normalization, request/response slot walking, reversible streaming and passthrough routing; all five reported CI checks are green. I did not run the local suite.
A Responses turn with more than one output item interleaves the deltas of its parts on the wire, addressed by item_id/output_index/content_index. One buffer for the whole stream spliced the second part's text into a placeholder the first had half-written and re-emitted it under the wrong address. Each part now keeps its own re-identifier and addressing, flushed on that part's own done frame; the frames that close the whole response flush whatever is still open. The stub streams two parts at once and the regression reassembles by address: without the fix the first part reads 'email AIL_101073_1> about card REDIT_CARD_101073_1>' and the second carries the lost '<EM'.
|
Fixed in Each Responses content part now keeps its own The stub now streams two content parts at once (one carrying PII, one not) and the regression reassembles the client stream by address. Against the previous head the two parts come back as: so the test discriminates rather than merely passing. With the fix: part 0 is
|
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: changes requested — per-part buffering is introduced, but closing one output item flushes unrelated still-open parts.
High — src/streaming.ts:172-174
if (type === "response.output_item.done" && j.item && typeof j.item === "object") {
flushAllParts();
res.write(reframe(frame, { ...j, item: reidentifyBody({ output: [j.item] }, provider, vault, dialect).output[0] }));
response.output_item.done closes only the item named by that frame, but flushAllParts() ends the re-identifier for every other item/content part. For example, if item A finishes while an interleaved item B has emitted the partial placeholder <EMA, this code calls StreamReidentifier.end() for B and can resolve/emit the email early. When B's remaining IL_...> delta arrives, its buffer has been cleared, so that suffix is emitted separately. The B client text is consequently corrupted even though B has not closed. The new regression completes all delta emission before either output_item.done, so it does not exercise this ordering.
// Flush only parts belonging to the item that is closing. Keep other
// item_id/output_index parts buffered until their own done frame.
flushPartsForOutputItem(j.item?.id, j.output_index);
res.write(reframe(frame, {
...j,
item: reidentifyBody({ output: [j.item] }, provider, vault, dialect).output[0],
}));
The per-address map and the regression for alternating deltas correctly address the previously reported cross-part splice; CI's test and triage checks are green, while docker-build and CodeQL analysis remain pending.
response.output_item.done ended the re-identifier for every open part, not just the item named by the frame. An item still mid-placeholder had its fragment emitted as-is, and the suffix that followed arrived on a fresh buffer, so the client saw the placeholder's tail in its text. Only the parts whose addressing matches the closing item are flushed; response.completed, incomplete, failed and [DONE] still flush the rest. The regression now cuts the second part's deltas inside its own placeholder and closes the first item at that moment. On the previous head the second part reads 'john@acme.comIL_F9D5A1_1> about card'.
|
Fixed in
Two rounds of the test were hollow before this one, and it is worth recording why:
The stub now gives the second part the same received (placeholdered) text, cuts its deltas four characters into a real placeholder, closes the first item at that moment, and finishes the second part afterwards. Reassembled by address on the previous head: the fragment
|
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: request changes — the Responses stream can drop a held placeholder tail when the upstream ends without a terminal SSE frame.
Blocking — correctness — src/streaming.ts:119
if (dialect === "responses") flushAllParts();
else flushTail();
The new per-part buffers are flushed only for [DONE] and the explicit Responses terminal events. If an upstream Responses connection ends after a response.output_text.delta that leaves (for example) <EMA held, but before output_text.done/response.completed arrives, the normal end-of-reader path still calls the single non-Responses flushTail() buffer. That buffer has received no Responses deltas, so the part holding <EMA… is never emitted. The client receives truncated assistant text rather than the restored email. The new test streams all terminal frames, so it does not exercise this introduced close-without-terminal path.
// In both the catch path and the normal end-of-stream safety path:
if (dialect === "responses") flushAllParts();
else flushTail();
Add a regression case that terminates a Responses SSE stream while one part is holding a split placeholder and verifies the client receives the restored complete text.
What's good: the rework correctly scopes response.output_item.done flushing to the matching item and retains per-part addressing; the added interleaving test covers the prior cross-item premature flush.
I reviewed the changed Responses request/response handling and streaming path. CI's test job is green; JavaScript analysis and the Docker build were still pending. I did not run the local suite per review policy.
The per-part buffers were flushed for the terminal frames only. When the upstream stopped without one, the reader's end path and the catch path flushed the chat-dialect buffer, which for a Responses stream has received nothing, so the client's text simply stopped where the last resolved delta ended. One flushHeld now serves all three. The regression stops a Responses stream with a placeholder still held: on the previous head the client receives 'email john@acme.com about card ' and the card never arrives.
|
Fixed in Measured both arms with a stub that stops a Responses stream while a placeholder is still held (no The card never reached the client before; the text just stopped. Two corrections to my own work on the way here, both of which your reviews are responsible for catching:
|
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: APPROVE — no blocking issues found in the live-head rework.
I reviewed the live head a1e3e480fad1d621d1afe7b03e8e53525464adce, concentrating on the rework in src/streaming.ts and its regression coverage. The new flushHeld() dispatches Responses streams to flushAllParts() on [DONE], reader errors, and normal EOF, so each held per-part tail is emitted with its saved addressing rather than flushing the unused chat buffer. The truncated-stream regression feeds a placeholder cut mid-token and asserts that both the email and card reach the client, which exercises the previously missing EOF path.
What's good: the change is small, uses the existing per-part flush mechanism, and preserves the existing chat/messages behavior. CI test and triage are green; docker-build and CodeQL analysis were pending when reviewed.
The OpenAI Responses API (#70) moves from Unreleased to 0.3.0. It is also the first image since 0.2.0, so it carries 0.2.1's dependency update, which was never tagged or imaged. package.json and the lockfile's own version move to 0.3.0 so release.yml accepts the v0.3.0 tag; the release notes extract as the 0.3.0 section alone (scripts/release-notes.mjs 0.3.0).
3 participants
Why
POST /v1/responsesis what current OpenAI clients use by default (client.responses.create, the Agents SDK, Codex), and cordon passed it through verbatim: a modern OpenAI client pointed at cordon sent raw PII upstream and gotX-Redacted: 0back, which reads as "nothing to redact" rather than "not covered".What
normalizeroutes/v1/responsesto provideropenai, dialectresponses(newDialectonCanonicalRequest; chat and messages keep their behaviour, the adapter is picked per dialect).requestTextSlots):instructionsunderREDACT_SYSTEM(andsystem/developeritems),inputas a string or an item list (input_textparts,function_call.argumentsparsed as JSON so a redacted number stays quoted,function_call_output.output), flat function tool definitions.input_image/input_fileparts are untouched. The JSON-string handling is now one helper shared with chat'stool_calls.output_textandrefusalparts are restored.response.output_text.delta(andrefusal.delta) go through the sameStreamReidentifieras chat; each re-emitted delta carries the upstream frame'sitem_id/output_index/content_index/sequence_number. The frames that carry the full text (output_text.done,content_part.done,output_item.done,response.completed/incomplete/failed) flush the held tail first and are restored in place.X-Redacted*headers, fail-closed path and audit record (provider: "openai")./v1/responses/{id}and other sub-paths still pass through verbatim;passthroughBaseknows the prefix.Tests
Stub answers
/v1/responsesnon-stream and stream (3-char deltas, so placeholders split across frames). Proxy suite 41 to 60, stream suite 11 to 19, unit 83 and fuzz 15 unchanged;npm testall suites green,tsc --noEmitclean.Live proof (cordon from source on :8820, echo stub on :8900)
CHANGELOG
Entry added under
## [Unreleased](Added). After #69 merges, the README's "What it does not do" bullet that says only/v1/chat/completionsand/v1/messagesare redacted needs/v1/responsesadded; not touched here to avoid the conflict.