Skip to content

Commit 466fd65

Browse files
dalmasontoclaude
andcommitted
fix: backup download blocked by CORS (reply.hijack dropped the headers)
Every /admin/backup/:address request failed in the browser with "No 'Access-Control-Allow-Origin' header is present", despite returning 200 with a perfectly valid zip. streamUserBackup wrote reply.raw directly behind reply.hijack(). That skips Fastify's reply lifecycle, and @fastify/cors sets Access-Control-Allow-Origin through reply.header() -- which is only flushed to the socket by send(). Hijacking meant those headers were never written, so the browser blocked a response the server considered successful. Only the two headers I set by hand on reply.raw survived, which is why the failure looked like a CORS misconfiguration rather than a bug in this route. Hand the archive to Fastify with reply.send(archive) instead. It pipes the stream just the same, but the reply lifecycle runs, so CORS and any other plugin headers are flushed normally. The zip test now asserts access-control-allow-origin on the response. Confirmed it fails against the hijack version and passes against this one -- status code alone never proved this route worked from a browser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 65c8854 commit 466fd65

2 files changed

Lines changed: 26 additions & 21 deletions

File tree

api/src/__test__/admin-backup-delete.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ test('the backup route streams a real, importable zip', async (t) => {
263263
const response = await fastify.inject({
264264
method: 'GET',
265265
url: `/admin/backup/${TARGET}`,
266-
headers: { nonce: 'test-nonce-admin' },
266+
headers: { nonce: 'test-nonce-admin', origin: 'http://localhost:5173' },
267267
});
268268

269269
t.equal(response.statusCode, 200, 'the backup is served');
@@ -274,6 +274,16 @@ test('the backup route streams a real, importable zip', async (t) => {
274274
'named after the user it belongs to'
275275
);
276276

277+
// Regression: an earlier cut wrote reply.raw behind reply.hijack(), which skips the
278+
// reply lifecycle. @fastify/cors sets Access-Control-Allow-Origin via reply.header(),
279+
// and those are only flushed by send() -- so the route returned a 200 with a valid
280+
// zip that every browser then blocked. Status alone does not prove this route works.
281+
t.equal(
282+
response.headers['access-control-allow-origin'],
283+
'http://localhost:5173',
284+
'and carries the CORS header, so a browser will actually surrender the body'
285+
);
286+
277287
const zip = await JSZip.loadAsync(response.rawPayload);
278288

279289
const manifestFile = zip.file('aqua.json');

api/src/services/backup_service.ts

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,29 +76,25 @@ export async function streamUserBackup(
7676

7777
const archive = new ZipArchive({ zlib: { level: 9 } });
7878

79-
reply.raw.setHeader('Content-Type', 'application/zip');
80-
reply.raw.setHeader(
81-
'Content-Disposition',
82-
`attachment; filename="workspace_${address}.zip"`
83-
);
84-
85-
// We are writing the socket ourselves from here on, so Fastify must not try to
86-
// send a reply of its own on top of the stream.
87-
reply.hijack();
88-
archive.pipe(reply.raw);
89-
9079
archive.on('warning', (err) => Logger.error(`Backup archive warning for ${address}:`, err));
91-
archive.on('error', (err) => {
92-
Logger.error(`Backup archive error for ${address}:`, err);
93-
reply.raw.destroy(err);
94-
});
80+
archive.on('error', (err) => Logger.error(`Backup archive error for ${address}:`, err));
81+
82+
// Hand the stream to Fastify rather than writing reply.raw ourselves. Writing raw
83+
// (or reply.hijack()) skips the reply lifecycle, and the headers other plugins set
84+
// through reply.header() -- @fastify/cors's Access-Control-Allow-Origin above all --
85+
// are only flushed to the socket by send(). Bypassing it yields a 200 carrying a
86+
// perfectly good zip that the browser then refuses to hand to JS.
87+
reply
88+
.header('Content-Type', 'application/zip')
89+
.header('Content-Disposition', `attachment; filename="workspace_${address}.zip"`)
90+
.send(archive);
9591

9692
const nameWithHash: { name: string; hash: string }[] = [];
9793
const seenAssets = new Set<string>();
9894

99-
// Past the hijack there is no status code left to send, so nothing in here may
100-
// throw: a failure has to tear the socket down, which is what tells the client
101-
// the zip it received is truncated rather than handing it a corrupt file.
95+
// The response is already streaming, so there is no status code left to send.
96+
// Destroying the archive propagates the error through the pipe Fastify set up,
97+
// which truncates the download rather than handing the client a corrupt zip.
10298
try {
10399
for (const record of latestRecords) {
104100
try {
@@ -146,8 +142,7 @@ export async function streamUserBackup(
146142
await archive.finalize();
147143
} catch (error) {
148144
Logger.error(`Backup stream failed for ${address}:`, error);
149-
archive.destroy();
150-
reply.raw.destroy();
145+
archive.destroy(error instanceof Error ? error : new Error(String(error)));
151146
}
152147
}
153148

0 commit comments

Comments
 (0)