Skip to content

Commit 317d407

Browse files
tofikwestclaude
andcommitted
fix(evidence-export): tighten S3 missing-object check to specific error codes
The blanket `httpStatusCode === 404` fallback in `isS3MissingObjectError` misclassified `NoSuchBucket` (bucket misconfigured) as a per-attachment missing object. That would have produced an export that completed "successfully" with nothing but `_MISSING_*.txt` placeholders — worse than failing outright, because the customer would have no idea their bundle contains none of the actual evidence. Now we only treat the specific error codes `NoSuchKey` and `NotFound` as missing; everything else (including other 404s like `NoSuchBucket`) rethrows and aborts the archive. Added a regression test asserting NoSuchBucket aborts the archive rather than producing a placeholder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3814e33 commit 317d407

2 files changed

Lines changed: 45 additions & 12 deletions

File tree

apps/api/src/tasks/evidence-export/evidence-attachment-streamer.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,21 +135,18 @@ export async function appendAttachmentToArchive(params: {
135135
}
136136

137137
/**
138-
* True only for "the object does not exist" — NoSuchKey or HTTP 404.
139-
* Everything else (AccessDenied, SlowDown, NetworkError, timeouts) is treated
140-
* as a real failure that should surface, not a silent skip.
138+
* True only for "the object does not exist" — specifically `NoSuchKey` (or
139+
* `NotFound` for HeadObject semantics). Anything else — including the other
140+
* 404s like `NoSuchBucket`, or 403s like `AccessDenied` — is a real failure
141+
* that must surface, not a silent per-attachment skip. A misconfigured bucket
142+
* returning NoSuchBucket would otherwise produce an export full of placeholders
143+
* that looks "successful" but contains none of the customer's evidence.
141144
*/
142145
function isS3MissingObjectError(error: unknown): boolean {
143146
if (!error || typeof error !== 'object') return false;
144-
const err = error as {
145-
name?: string;
146-
Code?: string;
147-
$metadata?: { httpStatusCode?: number };
148-
};
149-
if (err.name === 'NoSuchKey' || err.name === 'NotFound') return true;
150-
if (err.Code === 'NoSuchKey' || err.Code === 'NotFound') return true;
151-
if (err.$metadata?.httpStatusCode === 404) return true;
152-
return false;
147+
const err = error as { name?: string; Code?: string };
148+
const code = err.name ?? err.Code;
149+
return code === 'NoSuchKey' || code === 'NotFound';
153150
}
154151

155152
function buildMissingPlaceholder(

apps/api/src/tasks/evidence-export/evidence-export.service.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,42 @@ describe('EvidenceExportService — streaming ZIPs', () => {
267267
expect(placeholder).toBeUndefined();
268268
});
269269

270+
it('aborts on NoSuchBucket (a 404 that is NOT a missing object)', async () => {
271+
const attachments = [
272+
{
273+
id: 'att_bucket',
274+
name: 'file.pdf',
275+
url: 'org_1/attachments/task/tsk_123/file.pdf',
276+
type: 'document',
277+
createdAt: new Date(),
278+
},
279+
];
280+
281+
// Bucket misconfiguration — returns HTTP 404 but must NOT be
282+
// treated as a single missing attachment. Otherwise the export looks
283+
// "successful" while silently containing only placeholders.
284+
const noSuchBucketError = Object.assign(new Error('NoSuchBucket'), {
285+
name: 'NoSuchBucket',
286+
$metadata: { httpStatusCode: 404 },
287+
});
288+
(s3Client!.send as jest.Mock).mockRejectedValue(noSuchBucketError);
289+
primeTaskQueries({ attachments });
290+
291+
const { archive } = await service.streamTaskEvidenceZip(
292+
'org_1',
293+
'tsk_123',
294+
);
295+
const mock = archive as unknown as MockArchive;
296+
297+
await expect(mock.finalized).rejects.toThrow('aborted');
298+
expect(mock.abort).toHaveBeenCalled();
299+
300+
const placeholder = mock.appendCalls.find((c) =>
301+
c.options.name.includes('_MISSING_'),
302+
);
303+
expect(placeholder).toBeUndefined();
304+
});
305+
270306
it('disambiguates duplicate filenames within attachments folder', async () => {
271307
const attachments = [
272308
{

0 commit comments

Comments
 (0)