Skip to content

Commit ab8db29

Browse files
fix(pii): mask offloaded large payloads chunk-by-chunk and retry transient mask failures
A block output past the 16MB inline materialization ceiling aborted the run before masking even started: the redaction path hydrated the whole offloaded value at once, and the pre-flight size assert fired on the manifest's total byteSize. Large-array manifests now page one stored chunk at a time (materialize -> mask -> re-store, rebuilt via the manifest writer with preview derived from masked items), so peak heap stays ~one chunk regardless of payload size. Single refs up to the 64MB durable cap hydrate with a raised budget and run serially outside the concurrency pool. Mask-batch chunk requests now retry transient failures (network errors, 408/429/5xx, honoring Retry-After) with jittered backoff, so a single ALB blip or Presidio pod restart no longer fails a whole payload's redaction. Nested-ref masking now runs the string pass before ref substitution, fixing a latent double-mask when a masked nested value shrinks back inline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A1JYstmLHk9qMGyBDqYRcJ
1 parent 58c87b2 commit ab8db29

5 files changed

Lines changed: 465 additions & 58 deletions

File tree

apps/sim/lib/guardrails/mask-client.test.ts

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
*/
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockToken, mockBaseUrl } = vi.hoisted(() => ({
6+
const { mockToken, mockBaseUrl, mockSleep } = vi.hoisted(() => ({
77
mockToken: vi.fn(),
88
mockBaseUrl: vi.fn(),
9+
mockSleep: vi.fn(),
910
}))
1011

1112
vi.mock('@/lib/auth/internal', () => ({ generateInternalToken: mockToken }))
1213
vi.mock('@/lib/core/utils/urls', () => ({ getInternalApiBaseUrl: mockBaseUrl }))
14+
vi.mock('@sim/utils/helpers', () => ({ sleep: mockSleep }))
1315

1416
import { maskPIIBatchViaHttp } from '@/lib/guardrails/mask-client'
1517

@@ -20,6 +22,7 @@ describe('maskPIIBatchViaHttp', () => {
2022
vi.clearAllMocks()
2123
mockToken.mockResolvedValue('tok')
2224
mockBaseUrl.mockReturnValue('http://app.internal:3000')
25+
mockSleep.mockResolvedValue(undefined)
2326
fetchMock = vi.fn(async (_url: string, init: { body: string }) => {
2427
const { texts } = JSON.parse(init.body) as { texts: string[] }
2528
return new Response(JSON.stringify({ masked: texts.map((t) => `M(${t})`) }), {
@@ -52,10 +55,59 @@ describe('maskPIIBatchViaHttp', () => {
5255
expect(fetchMock).toHaveBeenCalledTimes(3) // 2000-per-request cap
5356
})
5457

55-
it('throws on a non-2xx response so the caller can scrub', async () => {
56-
fetchMock.mockResolvedValueOnce(new Response('boom', { status: 500 }))
58+
it('throws immediately on a deterministic 4xx without retrying', async () => {
59+
fetchMock.mockResolvedValueOnce(new Response('bad request', { status: 400 }))
5760

5861
await expect(maskPIIBatchViaHttp(['a'], [])).rejects.toThrow(/mask-batch request failed/)
62+
expect(fetchMock).toHaveBeenCalledTimes(1)
63+
expect(mockSleep).not.toHaveBeenCalled()
64+
})
65+
66+
it('retries a transient 5xx with backoff and then succeeds', async () => {
67+
fetchMock.mockResolvedValueOnce(new Response('deploying', { status: 503 }))
68+
69+
const out = await maskPIIBatchViaHttp(['a'], [])
70+
71+
expect(out).toEqual(['M(a)'])
72+
expect(fetchMock).toHaveBeenCalledTimes(2)
73+
expect(mockSleep).toHaveBeenCalledTimes(1)
74+
})
75+
76+
it('retries a rejected fetch (network error) and then succeeds', async () => {
77+
fetchMock.mockRejectedValueOnce(new TypeError('fetch failed'))
78+
79+
const out = await maskPIIBatchViaHttp(['a'], [])
80+
81+
expect(out).toEqual(['M(a)'])
82+
expect(fetchMock).toHaveBeenCalledTimes(2)
83+
})
84+
85+
it('gives up after the retry budget is exhausted on a persistent 5xx', async () => {
86+
fetchMock.mockImplementation(async () => new Response('down', { status: 503 }))
87+
88+
await expect(maskPIIBatchViaHttp(['a'], [])).rejects.toThrow(/mask-batch request failed/)
89+
expect(fetchMock).toHaveBeenCalledTimes(8)
90+
expect(mockSleep).toHaveBeenCalledTimes(7)
91+
})
92+
93+
it('mints a fresh internal token per attempt', async () => {
94+
fetchMock.mockResolvedValueOnce(new Response('deploying', { status: 503 }))
95+
96+
await maskPIIBatchViaHttp(['a'], [])
97+
98+
expect(mockToken).toHaveBeenCalledTimes(2)
99+
})
100+
101+
it('does not retry a shape mismatch (deterministic server bug)', async () => {
102+
fetchMock.mockResolvedValueOnce(
103+
new Response(JSON.stringify({ nope: true }), {
104+
status: 200,
105+
headers: { 'content-type': 'application/json' },
106+
})
107+
)
108+
109+
await expect(maskPIIBatchViaHttp(['a'], [])).rejects.toThrow(/unexpected result/)
110+
expect(fetchMock).toHaveBeenCalledTimes(1)
59111
})
60112

61113
it('returns [] without any request for empty input', async () => {

apps/sim/lib/guardrails/mask-client.ts

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { sleep } from '@sim/utils/helpers'
2+
import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry'
13
import type { GuardrailsMaskBatchResult } from '@/lib/api/contracts'
24
import { generateInternalToken } from '@/lib/auth/internal'
35
import { env } from '@/lib/core/config/env'
@@ -18,6 +20,39 @@ import type { CustomPiiPattern } from '@/lib/guardrails/pii-entities'
1820
*/
1921
const CHUNK_CONCURRENCY = env.PII_MASK_CHUNK_CONCURRENCY ?? 64
2022

23+
/**
24+
* Per-chunk retry budget for transient failures (network errors, 408/429/5xx).
25+
* A large payload fans out into many chunk requests, so a single blip — an ALB
26+
* 502 during a deploy, a Presidio pod restart — must not fail the whole
27+
* redaction (and, on the execution-altering stages, abort the run). With the
28+
* default 500ms→30s jittered backoff this rides out ~2 minutes of outage per
29+
* chunk before giving up. Deterministic failures (4xx, shape mismatches) throw
30+
* immediately.
31+
*/
32+
const MAX_CHUNK_ATTEMPTS = 8
33+
34+
const RETRYABLE_STATUSES = new Set([408, 429, 500, 502, 503, 504])
35+
36+
class MaskChunkHttpError extends Error {
37+
constructor(
38+
message: string,
39+
readonly status: number,
40+
readonly retryAfterMs: number | null
41+
) {
42+
super(message)
43+
this.name = 'MaskChunkHttpError'
44+
}
45+
}
46+
47+
function isRetryableChunkError(error: unknown): boolean {
48+
if (error instanceof MaskChunkHttpError) {
49+
return RETRYABLE_STATUSES.has(error.status)
50+
}
51+
// A rejected fetch (connection refused/reset, DNS, socket drop) is transient;
52+
// anything else (shape mismatch, token minting failure) is deterministic.
53+
return error instanceof TypeError
54+
}
55+
2156
/**
2257
* Mask PII across many strings via the internal app-container endpoint.
2358
*
@@ -29,8 +64,10 @@ const CHUNK_CONCURRENCY = env.PII_MASK_CHUNK_CONCURRENCY ?? 64
2964
* concurrency, so a large payload fans out rather than serializing; order is
3065
* preserved, so the returned array matches `texts` length.
3166
*
32-
* Rejects on any non-2xx, timeout, or shape mismatch so the caller can apply
33-
* its own fail-safe (scrubbing rather than leaking).
67+
* Transient chunk failures (network errors, 408/429/5xx) retry with jittered
68+
* backoff (see {@link MAX_CHUNK_ATTEMPTS}); only a deterministic failure or an
69+
* exhausted retry budget rejects, so the caller can apply its own fail-safe
70+
* (scrubbing rather than leaking).
3471
*/
3572
export async function maskPIIBatchViaHttp(
3673
texts: string[],
@@ -64,8 +101,29 @@ async function postChunk(
64101
language: string | undefined,
65102
customPatterns: CustomPiiPattern[] | undefined
66103
): Promise<string[]> {
67-
// Mint per request: a single token (5min TTL) can expire mid-batch when a
68-
// large execution fans out into many sequential chunk requests.
104+
for (let attempt = 1; ; attempt++) {
105+
try {
106+
return await postChunkOnce(url, texts, entityTypes, language, customPatterns)
107+
} catch (error) {
108+
if (attempt >= MAX_CHUNK_ATTEMPTS || !isRetryableChunkError(error)) {
109+
throw error
110+
}
111+
const retryAfterMs = error instanceof MaskChunkHttpError ? error.retryAfterMs : null
112+
await sleep(backoffWithJitter(attempt, retryAfterMs))
113+
}
114+
}
115+
}
116+
117+
async function postChunkOnce(
118+
url: string,
119+
texts: string[],
120+
entityTypes: string[],
121+
language: string | undefined,
122+
customPatterns: CustomPiiPattern[] | undefined
123+
): Promise<string[]> {
124+
// Mint per attempt: a single token (5min TTL) can expire mid-batch when a
125+
// large execution fans out into many sequential chunk requests or a chunk
126+
// spends its retry budget waiting out an outage.
69127
const token = await generateInternalToken()
70128

71129
// boundary-raw-fetch: internal server-to-server call to the app container (internal JWT auth, configurable base URL)
@@ -80,7 +138,11 @@ async function postChunk(
80138

81139
if (!response.ok) {
82140
const detail = await response.text().catch(() => '')
83-
throw new Error(`PII mask-batch request failed (${response.status}): ${detail.slice(0, 200)}`)
141+
throw new MaskChunkHttpError(
142+
`PII mask-batch request failed (${response.status}): ${detail.slice(0, 200)}`,
143+
response.status,
144+
parseRetryAfter(response.headers.get('retry-after'))
145+
)
84146
}
85147

86148
const data = (await response.json()) as GuardrailsMaskBatchResult

0 commit comments

Comments
 (0)