fix(sec): close defang bypass + thread pool client through recomputeSpacDeals#172
Open
sroussey wants to merge 2 commits into
Open
Conversation
…loses bypass) The prompt-injection defang scan ran the multi-pass HTML-entity decoder BEFORE the numeric-whitespace collapse pass, so a filer-controlled ` ` (or `
` / ` ` / `
` / `` / ``) got unwrapped to a literal `\n` / `\r` / `\v` / `\f` first — and the TAG_SHAPED middle character class `[\w \t-]` only admitted `\t` and space, so `</UNTRUSTED FILER DOCUMENT>` no longer matched the tag-shape regex once it decoded to `</UNTRUSTED\nFILER\nDOCUMENT>`. That left the lookalike intact and the fence un-redacted. Widening the mid-class to `[\w\s-]` admits every ASCII/Unicode whitespace codepoint, so the squash-and-compare callback fires on every variant and the fence redaction reaches its target. The `[_A-Z]` anchor is unchanged, so benign lowercase / non-fence tag shapes are still left literal. Tests cover , 
, , 
, 	 (regression), , , a mixed encoded+raw obfuscation, raw \r\n, raw \n, raw \v / \f, and two negative cases (`<NotAFence\nfoo>` and `<\nFOO>`) that must not redact.
… avoid deadlock with outer lock When a caller already holds an outer Postgres transaction wrapping a critical section (e.g. PR #170's `withSpacCikLock` will issue `BEGIN ... pg_advisory_xact_lock ...` on a checked-out client to serialize SPAC writes per CIK), `recomputeSpacDeals` checking out a *second* client from the shared pool to run its own BEGIN/COMMIT will deadlock the moment the pool is saturated by concurrent CIK locks — every connection in the pool holds an outer lock and waits on a second client that the pool can no longer hand out. Threading an optional `pgClient: PoolClient` through `RecomputeSpacDealsArgs` (and through `SpacReportWriter.recomputeAndSaveDeals`) lets the lock owner hand its connection to the inner ops; the Postgres branch then runs DELETE/INSERT directly on that client and skips its own BEGIN/COMMIT/ ROLLBACK/release (the caller still owns those). When `pgClient` is undefined the defensive default path is unchanged: own pool checkout + own transaction wrap. Adds a test asserting both paths — the back-compat case still emits BEGIN…COMMIT and releases its client, and the caller-supplied case runs the INSERT on the provided client without issuing any txn or release calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups on PR #169.
Fix 1 (CRITICAL): close the
defang bypass inwrapUntrustedThe prompt-injection defang scan in
sectionExtractors.tsruns the multi-pass HTML-entity decoder before the numeric-whitespace collapse pass. A filer-controlled (or
/ /
//) therefore gets unwrapped to a literal\n/\r/\v/\ffirst — and the priorTAG_SHAPEDmiddle character class[\w \t-]only admitted\tand space, so</UNTRUSTED FILER DOCUMENT>no longer matched the tag-shape regex once it decoded to</UNTRUSTED\nFILER\nDOCUMENT>. That left the lookalike intact and the fence un-redacted — defeating PR #169's prompt-injection hardening for this whole family of obfuscations.Widening the mid-class to
[\w\s-]admits every ASCII/Unicode whitespace codepoint, so the squash-and-compare callback fires on every variant and the fence redaction reaches its target. The[_A-Z]lead anchor is unchanged, so benign tag shapes (mixed-case lowercase-lead, no-letter-lead) are still left literal.New tests cover
,
, ,
,	(regression),,, a mixed encoded+raw obfuscation, raw\r\n, raw\n, raw\v/\f, plus two negative cases (<NotAFence\nfoo>and<\nFOO>) that must NOT redact.Fix 2 (HIGH): thread
pgClientthroughrecomputeSpacDealsPR #170 (stacked on this) introduces
withSpacCikLock, which on Postgres issuesBEGIN ... pg_advisory_xact_lock(...) ...on a checked-out client to serialize SPAC writes per CIK. With today's code, the lock body callsSpacReportWriter.recomputeAndSaveDeals→recomputeSpacDeals, which on the Postgres branch checks out a second client from the shared pool and runs its own BEGIN/COMMIT.Under load that pool deadlocks: every connection in the pool is the outer lock-holding client waiting on a second checkout that the pool can no longer hand out.
This PR adds an optional
pgClient: PoolClienttoRecomputeSpacDealsArgsand forwards it throughSpacReportWriter.recomputeAndSaveDealsso the lock owner can hand its connection to the inner ops. The Postgres branch then runs DELETE/INSERT directly on the supplied client and skips BEGIN/COMMIT/ROLLBACK/release()— the caller owns the transaction lifecycle. WhenpgClientis undefined the existing defensive path (own pool checkout + own txn wrap) is preserved unchanged, so callers that don't hold an outer lock keep getting atomic replays.Test plan
bun test src/sec/forms/registration-statements/s1/sectionExtractors.injection.test.ts— 29 tests, 0 fail.bun test src/storage/spac/— 45 tests across 11 files, 0 fail (includes the new back-compat test asserting the no-pgClientpath still wraps BEGIN/COMMIT and releases the client, and the supplied-pgClientpath neither releases nor wraps).bun run build— clean.Generated by Claude Code