Fix: detect PII / encoded payloads split across sibling text nodes (#203)

[twschiller]

Summary

• Closes the last Critical bypass tracked in Audit: rule-bypass findings (red-team pass) #203: PII / encoded payloads / secrets rendered with one wrapper element per fragment (e.g. React <span>-per-digit-group cards) used to slip past the per-text-node regex.
• Same change also closes Audit: rule-bypass findings (red-team pass) #203 / item 7 (encoded-payload split-across-whitespace) because both rules share the inline-text-redact factory.

Approach

• collectTextNodesWithInlineGroups (new) tags each text node with an inline-formatting-context id. Group id bumps on entering/leaving a block-level element, on <br>, and on shadow-root boundaries. The existing collectTextNodesShadowPiercing becomes a thin wrapper that drops the group tag — no callers of the non-group walker change.
• walkTextNodeGroupsChunked (new sibling of walkTextNodesChunked) threads the group ids through the same chunked-yield driver.
• defineInlineTextRedactRule switches to the new walker. Each chunk is bucketed by group; single-node buckets keep the existing single-node fast path; multi-node buckets concatenate values, run collectMatches against the concatenation, and route each match to either replaceMatchesInTextNode (within-node) or replaceMatchAcrossTextNodes (new).
• replaceMatchAcrossTextNodes scrubs the carrier — first node keeps its prefix, last node keeps its suffix, interior nodes have their nodeValue blanked. Wrapping inline elements stay in the DOM (per the "scrub, don't detach framework-owned nodes" rule). Reveal restores the matched characters as a single text node at the placeholder's position.
• Per-node minLength no longer filters at the walker level (it would drop legitimate cross-node candidates whose individual nodes sit below the floor); the group's concatenated length is checked instead, preserving the cheap early-out for prose nodes.

Demo site

• New "3a. Alternate payment" block on Checkout.tsx exercises five span-split shapes (card, SSN, phone, JWT, base64 payload) so the before/after diff visualizes the closed gap.

Test plan

• bun run check (extension)
• bun run typecheck (extension)
• bun run test (extension — 1716 tests, all green; 16 new cross-node tests across pii / secrets / encoded-payload / placeholder helper + 4 new property tests)
• bun run check (demo-site)
• pre-commit run --all-files
• Spot-check demo site in browser: card / SSN / phone / JWT / encoded payload on the new section 3a should each render as […hidden] chips, and reveal-on-click should restore the original characters.

Notes / accepted trade-offs

• Reveal collapses the matched-text split into one text node — the original per-span breakdown isn't reconstructed. Visual characters are the same; wrapper elements stay.
• Cross-chunk-boundary cross-node matches are not detected (>100 text nodes in a single inline-formatting context). Vanishingly rare with the default chunkSize=100.
• Late-inserted siblings that complete an existing cross-node match aren't picked up by the incremental watcher (its scan root is the new subtree, not the joined context). Same limitation as the pre-existing per-node case missing late additions to existing text nodes.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agent-browser-shield-demo-site	Ready	Preview, Comment	Jun 7, 2026 6:33pm

[unblocked]

1 issue found.

About Unblocked

Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.

📖 Documentation — Learn more in our docs.

💬 Ask questions — Mention @unblocked to request a review or summary, or ask follow-up questions.

👍 Give feedback — React to comments with 👍 or 👎 to help us improve.

⚙️ Customize — Adjust settings in your preferences.

[unblocked]

When two or more non-overlapping matches both resolve to the same start.index === end.index (i.e., both sit entirely within the same text node in a multi-node bucket), replaceMatchesInTextNode is called once per match with a single-element array. The first call (rightmost, due to toReversed()) detaches the original text node from the DOM via parentNode.replaceChild(fragment, textNode). Every subsequent call for the same node finds textNode.parentNode === null, so the ?.replaceChild is a no-op and the match is silently dropped — leaking the sensitive data it was supposed to redact.

Example: <p><span>SSN: 123-45-6789 Card: 4111 1111 1111 1111</span><span> end</span></p> — the card match is processed first and detaches node 0; the SSN match then fails silently.

Fix: group all single-node matches by their node index and pass each group as a batch to replaceMatchesInTextNode, which already handles multiple matches correctly in a single call. Something like:

const singleNodeMatches = new Map<number, InlineMatch[]>();
const crossNodeMatches: typeof matches = [];

for (const match of matches) {
  const start = locateStart(layout, match.start);
  const end = locateEnd(layout, match.end);
  if (start.index === end.index) {
    let arr = singleNodeMatches.get(start.index);
    if (!arr) { arr = []; singleNodeMatches.set(start.index, arr); }
    arr.push({ start: start.offset, end: end.offset, label: match.label });
  } else {
    crossNodeMatches.push(match);
  }
}

// Apply cross-node matches last-to-first (existing logic)
for (const match of crossNodeMatches.toReversed()) { ... }

// Apply batched single-node matches (one replaceMatchesInTextNode call per node)
for (const [index, nodeMatches] of singleNodeMatches) {
  const node = layout.nodes[index];
  if (node) {
    replaceMatchesInTextNode(node, nodeMatches, ruleId);
  }
}

[twschiller]

Fixed in 201f44c — confirmed the bug reproduces (SSN + card in one span of a two-span bucket: only the rightward match landed; the SSN leaked) and that the same root cause covered a related shape (cross-node phone + single-node card sharing a node, where the phone's lastNode truncation silently no-op'd because card had already detached it, leaving 4567 visible).

The sequential-application model is fundamentally racy here — any operation that detaches a text node breaks every later operation targeting it, and any operation that truncates a boundary node shifts every later offset in that node. Rather than try to order around it, I batched: new replaceMatchesAcrossTextNodes (plural) snapshots every node's original nodeValue up front, plans each affected node's content from that snapshot, and applies one replaceChild per node. Each text node is mutated exactly once, so no offsets ever go stale.

Regression tests cover both shapes (your example + the cross-node variant). Full suite is green (1718 tests).

— Claude Code, on behalf of @twschiller