fix(redact,responses): close the credential and lifecycle-identity variant gaps

src/lib/redact.ts

@@ -13,12 +13,22 @@ const SECRET_VALUE_PATTERNS: Array<[RegExp, string]> = [
   [/\btid=[A-Za-z0-9-]+(?:;[A-Za-z0-9_.-]+=[^;\s"']*)+(?::[A-Za-z0-9+/=_-]+)?/g, REDACTED_SECRET],
   [/\b((?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)=)([^&\s"',;]+)/gi, `$1${REDACTED_SECRET}`],
   // Colon-labelled credentials. Upstream error bodies quote the offending header
-  // or field back at us ("x-api-key: abc…"), and the `=` rule above never fires
-  // for that shape, so the credential survived into client-visible error text.
-  // Header-style names are included because that is exactly what a provider
-  // echoes when it rejects a request. A `Bearer <token>` value is left to the
-  // dedicated rule above so its scheme prefix stays readable in diagnostics.
-  [/\b((?:x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|password|secret|token)\s*:\s*)(?!\s)(?!Bearer\b)([^\s"',;]+)/gi, `$1${REDACTED_SECRET}`],
+  // or field back at us ("x-api-key: abc…"), and the `=` rules never fire for
+  // that shape, so the credential survived into client-visible error text.
+  //
+  // The value class deliberately runs to end-of-line rather than stopping at a
+  // quote, space, or semicolon. A first attempt tokenized on those characters
+  // and leaked every delimiter-bearing variant: `x-api-key: "quoted…"` kept the
+  // whole quoted secret, `Authorization: Basic dXNlcjpwYXNz` kept the payload
+  // after the scheme, and `Cookie: a=1; b=2` kept everything after the first
+  // `;`. A credential header's value IS the rest of the line, so that is what
+  // gets masked.
+  //
+  // `Bearer` is the one readable exception: an auth scheme is diagnostically
+  // useful and the dedicated rule above already masks its token, so the scheme
+  // word is preserved and only what follows is consumed here. Other schemes
+  // (Basic, Digest, …) are masked whole, since their payload is the credential.
+  [/\b((?:x-api-key|x-goog-api-key|x-amz-security-token|api[_-]?key|apiKey|access[_-]?token|accessToken|refresh[_-]?token|refreshToken|id[_-]?token|client[_-]?secret|clientSecret|authorization|proxy-authorization|cookie|set-cookie|password|secret|token)\s*:\s*(?:Bearer\s+)?)(?!\s*$)[^\r\n]+/gi, `$1${REDACTED_SECRET}`],
   [/((?:"(?:api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|client[_-]?secret|refreshToken|accessToken|clientSecret|apiKey)"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$3`],
   // Raw JSON "token" field values (Copilot token exchange bodies echo the credential here).
   [/(("token"\s*:\s*"))([^"]+)(")/gi, `$1${REDACTED_SECRET}$4`],

src/server/responses-snapshot-repair.ts

@@ -368,6 +368,18 @@ export function createResponsesSnapshotBlockRewrite(
       && isPlainObject(event.part)) {
       if (outputIndex !== undefined) {
         const open = openItems.get(outputIndex);
+        // A PRESENT-but-mismatched item_id is contradictory lifecycle evidence,
+        // exactly like output_item.done and output_text.done: the stream is
+        // telling us our identity model for this index is wrong. Merely
+        // ignoring it left the item open and let the terminal fabricate a full
+        // closure sequence (content_part.added → output_text.done →
+        // content_part.done → output_item.done) on top of a stream we do not
+        // understand. Go fail-closed instead. An OMITTED item_id stays
+        // legitimate and is still correlated by output_index.
+        if (open && itemId !== undefined && itemId !== open.itemId) {
+          taintAndRelease();
+          return [changed ? jsonBlock(nextEvent) : block];
+        }
         // Correlate by item_id when present: a mismatched event must not
         // mutate (or suppress injections for) the tracked item (#893 review).
         if (open && (itemId === undefined || itemId === open.itemId)) {
@@ -398,6 +410,14 @@ export function createResponsesSnapshotBlockRewrite(
     }
     if (type === "response.output_text.delta" && typeof event.delta === "string" && outputIndex !== undefined) {
       const open = openItems.get(outputIndex);
+      // Same identity contract as the *.done terminals: a present-but-foreign
+      // item_id on a tracked index means our model of this index is wrong, and
+      // reconstructing from the text we DID accept would ship a message the
+      // upstream never assembled that way.
+      if (open && itemId !== undefined && itemId !== open.itemId) {
+        taintAndRelease();
+        return [changed ? jsonBlock(nextEvent) : block];
+      }
       if (open && (itemId === undefined || itemId === open.itemId)) {
         const deltaBytes = Buffer.byteLength(event.delta, "utf8");
         open.text += event.delta;

tests/redact.test.ts

@@ -60,6 +60,30 @@ describe("redactSecretString", () => {
     expect(redactSecretString("model: gpt-5.5\nstatus: 429\nrequest: ocx-abc123"))
       .toBe("model: gpt-5.5\nstatus: 429\nrequest: ocx-abc123");
   });
+
+  test("masks the WHOLE colon-labelled value, including delimiter-bearing forms", () => {
+    // Re-review of the first fix: tokenizing the value on quotes, spaces, and
+    // semicolons leaked every variant that contains one. A credential header's
+    // value is the rest of the line, so that is what must be masked.
+    expect(redactSecretString('x-api-key: "quotedcredential123456"'))
+      .toBe(`x-api-key: ${REDACTED_SECRET}`);
+    expect(redactSecretString("Authorization: Basic dXNlcjpwYXNz"))
+      .toBe(`Authorization: ${REDACTED_SECRET}`);
+    expect(redactSecretString("Cookie: session=secret-one; csrf=secret-two"))
+      .toBe(`Cookie: ${REDACTED_SECRET}`);
+  });
+
+  test("keeps the Bearer scheme readable while masking its token", () => {
+    // An auth scheme is diagnostically useful; the credential after it is not.
+    expect(redactSecretString("Authorization: Bearer abcdefgh12345678"))
+      .toBe(`Authorization: Bearer ${REDACTED_SECRET}`);
+  });
+
+  test("masks each credential line independently without eating the next", () => {
+    // End-of-line, not end-of-string: a multi-line error body must not collapse.
+    expect(redactSecretString("x-api-key: one-secret\nmodel: gpt-5.5\ncookie: two=secret"))
+      .toBe(`x-api-key: ${REDACTED_SECRET}\nmodel: gpt-5.5\ncookie: ${REDACTED_SECRET}`);
+  });
 });
 
 describe("redactSecrets", () => {

tests/responses-snapshot-repair.test.ts

@@ -227,6 +227,61 @@ describe("createResponsesSnapshotBlockRewrite", () => {
     expect(Object.hasOwn(terminal.response as Record<string, unknown>, "output")).toBe(false);
   });
 
+  test("a mismatched item_id on content_part.done also goes fail-closed", () => {
+    // Re-review: fixing output_text.done alone left the sibling terminal open.
+    // A foreign content_part.done was ignored, the item stayed open, and the
+    // completed terminal fabricated the whole closure sequence
+    // (content_part.added → output_text.done → content_part.done →
+    // output_item.done) on a stream whose identity model was already wrong.
+    const rewrite = createResponsesSnapshotBlockRewrite();
+    rewrite(dataBlock(ISSUE_FIXTURE.itemAdded));
+    rewrite(dataBlock({
+      type: "response.content_part.done",
+      item_id: "msg_OTHER",
+      output_index: 0,
+      part: { type: "output_text", text: "foreign" },
+    }));
+    rewrite(dataBlock(ISSUE_FIXTURE.delta));
+    const out = rewrite(dataBlock(ISSUE_FIXTURE.completed));
+    expect(typesOf(out)).not.toContain("response.content_part.added");
+    expect(typesOf(out)).not.toContain("response.content_part.done");
+    expect(typesOf(out)).not.toContain("response.output_text.done");
+    expect(typesOf(out)).not.toContain("response.output_item.done");
+    const terminal = eventsOf(out).find(event => event.type === "response.completed")!;
+    expect(Object.hasOwn(terminal.response as Record<string, unknown>, "output")).toBe(false);
+  });
+
+  test("a mismatched item_id on a text delta goes fail-closed instead of being dropped", () => {
+    // Reconstructing from only the deltas we accepted would ship a message the
+    // upstream never assembled that way.
+    const rewrite = createResponsesSnapshotBlockRewrite();
+    rewrite(dataBlock(ISSUE_FIXTURE.itemAdded));
+    rewrite(dataBlock({
+      type: "response.output_text.delta",
+      item_id: "msg_OTHER",
+      output_index: 0,
+      delta: "foreign",
+    }));
+    const out = rewrite(dataBlock(ISSUE_FIXTURE.completed));
+    expect(typesOf(out)).not.toContain("response.output_item.done");
+    const terminal = eventsOf(out).find(event => event.type === "response.completed")!;
+    expect(Object.hasOwn(terminal.response as Record<string, unknown>, "output")).toBe(false);
+  });
+
+  test("an omitted item_id stays legitimate on every correlated event", () => {
+    // The taint must fire on a PRESENT-but-wrong id only. A gateway that omits
+    // item_id is common and correlates by output_index alone; breaking that
+    // would fail closed on healthy streams.
+    const rewrite = createResponsesSnapshotBlockRewrite();
+    rewrite(dataBlock(ISSUE_FIXTURE.itemAdded));
+    rewrite(dataBlock({ type: "response.content_part.added", output_index: 0, part: { type: "output_text", text: "" } }));
+    rewrite(dataBlock({ type: "response.output_text.delta", output_index: 0, delta: "hello" }));
+    const out = rewrite(dataBlock(ISSUE_FIXTURE.completed));
+    expect(typesOf(out)).toContain("response.output_item.done");
+    const injected = eventsOf(out).filter(event => event.type === "response.output_text.done");
+    expect(injected.some(event => event.text === "hello")).toBe(true);
+  });
+
   test("a completed terminal with absent output and zero items gets the canonical empty list", () => {
     const rewrite = createResponsesSnapshotBlockRewrite();
     const out = rewrite(dataBlock({ type: "response.completed", response: { id: "r" } }));