fix(export): let the keepsake download through X-Frame-Options on iOS
The keepsake download navigates a hidden, same-origin iframe (deliberately: a top-level navigation to a 404/429 would unload the PWA). Caddy stamped a site-wide `X-Frame-Options: DENY` that also covered the proxied `/api/*`. Blink hands a `Content-Disposition: attachment` response to the download manager at the network layer, so Chromium never noticed. WebKit enforces XFO on the frame navigation first and aborts the load — so on iOS Safari, the app's primary platform, tapping Download did nothing at all, silently. Carve the two export endpoints out to SAMEORIGIN, which still blocks cross-origin framing. Implemented as two disjoint matchers rather than an override: Caddy applies the FIRST `header` directive outermost, so it wins on write and a later, more specific `header` is silently ignored (verified against the running test stack). Also close the test gap that let this ship: - `06-export` ran on chromium-desktop only; add it to `webkit-iphone`, the only engine that enforces XFO on the download frame. - No test in the suite ever clicked a download button — every archive assertion used Node `fetch`, which has no frame and no XFO enforcement. Add a spec that clicks it and awaits a real `download` event. Verified falsifiable: with the blanket DENY reinstated it fails and reports the WebKit refusal as the cause. - Fix `ExportPage`'s card-scoped locators, which matched nothing: the cards carry `class="card p-5"` (a Tailwind `@apply` component class), never the `rounded-xl` the page object looked for. This had left the "shows enabled download buttons" test red on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
16
Caddyfile
16
Caddyfile
@@ -9,10 +9,24 @@
|
||||
header {
|
||||
Strict-Transport-Security "max-age=31536000; includeSubDomains"
|
||||
X-Content-Type-Options "nosniff"
|
||||
X-Frame-Options "DENY"
|
||||
Referrer-Policy "strict-origin-when-cross-origin"
|
||||
}
|
||||
|
||||
# X-Frame-Options: DENY everywhere EXCEPT the keepsake download endpoints, which
|
||||
# are navigated in a HIDDEN, SAME-ORIGIN iframe so a 404/429 can't unload the PWA
|
||||
# (see frontend/src/routes/export/+page.svelte). WebKit enforces XFO *before*
|
||||
# honouring Content-Disposition, so a blanket DENY makes the download silently do
|
||||
# nothing on iOS Safari — the app's primary platform. SAMEORIGIN still blocks
|
||||
# cross-origin framing.
|
||||
#
|
||||
# Split into two disjoint matchers rather than an override: Caddy applies the
|
||||
# FIRST header directive outermost, so it wins on write — a later, more specific
|
||||
# `header` would be silently ignored.
|
||||
@framable path /api/v1/export/zip /api/v1/export/html
|
||||
@not_framable not path /api/v1/export/zip /api/v1/export/html
|
||||
header @framable X-Frame-Options "SAMEORIGIN"
|
||||
header @not_framable X-Frame-Options "DENY"
|
||||
|
||||
# SvelteKit frontend — static assets with long-lived cache (content-hashed filenames)
|
||||
@hashed_assets path_regexp hashed /_app/immutable/.*\.[a-f0-9]{8,}\.(js|css|woff2)$
|
||||
header @hashed_assets Cache-Control "public, max-age=31536000, immutable"
|
||||
|
||||
@@ -12,10 +12,17 @@
|
||||
# Mirror prod's security headers (minus HSTS, which is HTTPS-only).
|
||||
header {
|
||||
X-Content-Type-Options "nosniff"
|
||||
X-Frame-Options "DENY"
|
||||
Referrer-Policy "strict-origin-when-cross-origin"
|
||||
}
|
||||
|
||||
# Mirror prod's export carve-out: the keepsake download targets a hidden
|
||||
# same-origin iframe, and WebKit enforces XFO before Content-Disposition.
|
||||
# Two disjoint matchers, not an override — see the comment in ../Caddyfile.
|
||||
@framable path /api/v1/export/zip /api/v1/export/html
|
||||
@not_framable not path /api/v1/export/zip /api/v1/export/html
|
||||
header @framable X-Frame-Options "SAMEORIGIN"
|
||||
header @not_framable X-Frame-Options "DENY"
|
||||
|
||||
reverse_proxy /api/* app:3000
|
||||
reverse_proxy /media/* app:3000
|
||||
reverse_proxy /health app:3000
|
||||
|
||||
@@ -37,12 +37,19 @@ export class ExportPage {
|
||||
/**
|
||||
* The "Download" button inside the card whose heading is `heading`.
|
||||
*
|
||||
* Scoped to the card element (`div.rounded-xl`) rather than "any div containing the
|
||||
* heading" — the latter also matches the page wrapper, which contains BOTH cards' buttons.
|
||||
* Scoped to the card element rather than "any div containing the heading" — the latter
|
||||
* also matches the page wrapper, which contains BOTH cards' buttons.
|
||||
*
|
||||
* The scope class is `div.card` (see the ZIP/HTML cards in
|
||||
* frontend/src/routes/export/+page.svelte). It was previously `div.rounded-xl`, which
|
||||
* matched NOTHING: `.card` is a Tailwind `@apply` component class
|
||||
* (frontend/src/lib/styles/components.css) so the DOM only ever carries `class="card p-5"`
|
||||
* — and the utility it applies is `rounded-2xl` anyway. Both card-scoped locators were
|
||||
* therefore dead, which is why the "shows enabled download buttons" test was red.
|
||||
*/
|
||||
private cardButton(heading: string): Locator {
|
||||
return this.page
|
||||
.locator('div.rounded-xl')
|
||||
.locator('div.card')
|
||||
.filter({ has: this.page.getByRole('heading', { name: heading, exact: true }) })
|
||||
.getByRole('button', { name: 'Download', exact: true });
|
||||
}
|
||||
|
||||
@@ -146,8 +146,18 @@ export default defineConfig({
|
||||
// that on `@smoke` — which exists on exactly two specs — meant the entire iOS guarantee was
|
||||
// one happy path and one join test. Every other UA here is a secondary browser and a smoke
|
||||
// check is proportionate; WebKit is not. Give it the core journeys the guest actually walks:
|
||||
// join/recover, upload, and browse the feed.
|
||||
testMatch: ['**/__smoke/**', '**/01-auth/**', '**/02-upload/**', '**/03-feed/**'],
|
||||
// join/recover, upload, browse the feed — and take the keepsake home.
|
||||
//
|
||||
// 06-export is here because WebKit is the ONLY engine that enforces X-Frame-Options on the
|
||||
// hidden download iframe. Excluding it is what let a site-wide `XFO: DENY` ship a keepsake
|
||||
// download that silently did nothing on iOS. See 06-export/download-iframe.spec.ts.
|
||||
testMatch: [
|
||||
'**/__smoke/**',
|
||||
'**/01-auth/**',
|
||||
'**/02-upload/**',
|
||||
'**/03-feed/**',
|
||||
'**/06-export/**',
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'firefox-android',
|
||||
|
||||
103
e2e/specs/06-export/download-iframe.spec.ts
Normal file
103
e2e/specs/06-export/download-iframe.spec.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* Regression guard — the keepsake download must actually download, in WebKit.
|
||||
*
|
||||
* The bug this exists to catch: `/export` streams the archive by pointing a HIDDEN,
|
||||
* SAME-ORIGIN iframe at `/api/v1/export/zip` (deliberately — a top-level navigation to a
|
||||
* 404/429 would unload the PWA). Caddy stamped a site-wide `X-Frame-Options: DENY` that
|
||||
* also covered `/api/*`. Blink hands a `Content-Disposition: attachment` response to the
|
||||
* download manager at the network layer, so Chromium never noticed; WebKit enforces XFO on
|
||||
* the frame navigation FIRST and aborts the load. Result: on iOS Safari — the app's primary
|
||||
* platform — tapping Download did nothing, silently, with no error anywhere.
|
||||
*
|
||||
* Why the old suite was structurally blind to it:
|
||||
* - `06-export` ran on `chromium-desktop` only (webkit-iphone's testMatch excluded it),
|
||||
* - and no test in the entire suite ever CLICKED a download button; every archive
|
||||
* assertion used Node `fetch`, which has no frame and therefore no XFO enforcement.
|
||||
*
|
||||
* So this spec must keep both properties to be worth anything: a real click, in WebKit.
|
||||
*/
|
||||
import { test, expect } from '../../fixtures/test';
|
||||
import { ExportPage } from '../../page-objects';
|
||||
import { BASE } from '../../helpers/env';
|
||||
|
||||
const SLUG = 'e2e-test-event';
|
||||
|
||||
function post(path: string, jwt: string) {
|
||||
return fetch(BASE + path, { method: 'POST', headers: { Authorization: `Bearer ${jwt}` } });
|
||||
}
|
||||
|
||||
/** The real export job runs image processing; give it head-room over the tiny fixtures. */
|
||||
async function releaseAndWait(jwt: string) {
|
||||
expect((await post('/api/v1/host/gallery/release', jwt)).status).toBe(204);
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const res = await fetch(BASE + '/api/v1/export/status', {
|
||||
headers: { Authorization: `Bearer ${jwt}` },
|
||||
});
|
||||
const s = await res.json();
|
||||
return s.released === true && s.zip?.status === 'done';
|
||||
},
|
||||
{ timeout: 60_000, intervals: [500] }
|
||||
)
|
||||
.toBe(true);
|
||||
}
|
||||
|
||||
test.describe('Export — the download actually fires in the browser', () => {
|
||||
test.slow();
|
||||
|
||||
test('clicking Download triggers a real download event', async ({ page, host, signIn, db }) => {
|
||||
await db.setExportReleased(SLUG, false);
|
||||
await releaseAndWait(host.jwt);
|
||||
|
||||
await signIn(page, host);
|
||||
const exportPage = new ExportPage(page);
|
||||
await exportPage.goto();
|
||||
await expect(exportPage.zipDownloadButton).toBeEnabled({ timeout: 10_000 });
|
||||
|
||||
// Capture the frame-level refusal that XFO produces, so a failure reports the CAUSE
|
||||
// rather than just a timeout. WebKit logs "Refused to display ... in a frame because it
|
||||
// set 'X-Frame-Options'"; Chromium logs nothing here, which is the whole problem.
|
||||
const refusals: string[] = [];
|
||||
page.on('console', (m) => {
|
||||
if (/X-Frame-Options|Refused to display/i.test(m.text())) refusals.push(m.text());
|
||||
});
|
||||
|
||||
const downloadPromise = page.waitForEvent('download', { timeout: 30_000 });
|
||||
await exportPage.zipDownloadButton.click();
|
||||
|
||||
const download = await downloadPromise.catch((err) => {
|
||||
throw new Error(
|
||||
`No download event fired after clicking the ZIP button.` +
|
||||
(refusals.length
|
||||
? ` The browser refused the iframe navigation: ${refusals.join(' | ')}`
|
||||
: ' No X-Frame-Options refusal was logged; check the ticket/readiness path.') +
|
||||
`\n${err}`
|
||||
);
|
||||
});
|
||||
|
||||
expect(download.suggestedFilename()).toMatch(/\.zip$/i);
|
||||
// The stream must produce real bytes, not a zero-length placeholder.
|
||||
const path = await download.path();
|
||||
expect(path).toBeTruthy();
|
||||
expect(refusals, 'no frame should have been refused').toEqual([]);
|
||||
});
|
||||
|
||||
test('the export endpoints are framable same-origin; everything else stays DENY', async () => {
|
||||
// Locks the Caddy carve-out itself, independently of any browser. Cheap, and it fails
|
||||
// loudly at the exact layer that regressed if someone reinstates a blanket DENY.
|
||||
for (const path of ['/api/v1/export/zip', '/api/v1/export/html']) {
|
||||
const res = await fetch(BASE + path);
|
||||
expect(res.headers.get('x-frame-options')?.toUpperCase(), `${path} must be framable`).toBe(
|
||||
'SAMEORIGIN'
|
||||
);
|
||||
}
|
||||
|
||||
for (const path of ['/', '/api/v1/feed', '/api/v1/event']) {
|
||||
const res = await fetch(BASE + path);
|
||||
expect(res.headers.get('x-frame-options')?.toUpperCase(), `${path} must stay DENY`).toBe(
|
||||
'DENY'
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user