Review 5532

#5532 — fix(docsite): state the playground page's own framing headers

bhamodi · open, approve posted · reviewed at 91c44fd877d · view on GitHub

Verdict: approve — the loop merges it

Problem

Anyone can put the Astryx playground editor inside a frame on their own site today, and the person typing in it has no way to tell. Measured on main's config: GET /playground returns no X-Frame-Options and no frame-ancestors — the page loads, and the Monaco editor, the share link and the theme controls are all live inside the attacker's page. Only the nested /playground/preview frame refuses, because its frame-ancestors 'self' sees the foreign ancestor.

The PR's own body says the current arrangement "works". It does not: it breaks the preview pane, which is not the same as refusing to be embedded.

Solution

(1 decision · 11 lines, all config)

1. /playground declares X-Frame-Options: SAMEORIGIN and CSP frame-ancestors 'self' — the fix.

A page can state who is allowed to put it inside a frame. The playground page never stated anything, so its only protection was that the smaller preview page inside it refuses when it notices a foreign site in the chain — which stops the preview from rendering but leaves the page itself framed and usable. Now the playground page states the rule for itself, so the browser refuses to frame it before anything inside it loads. Responsibility moves from the child to the page that needs protecting.

It lives with its owner, apps/docsite/next.config.mjs's headers() layer, and matches the shape of the entry directly below it. The one seam is the same-origin /playground → /playground/preview iframe: frame-ancestors 'self' on the parent constrains who may frame the parent and says nothing about what the parent may frame, and the preview's own 'self' sees the chain and permits it. Measured — the preview route's header set is byte-identical before and after.

Impact

Nobody using the site notices anything: the two lines only appear on responses, and the measured delta on /playground is exactly those two header lines and nothing else. The person this reaches is the one who cannot do something any more — someone hosting <iframe src="https://astryx.dev/playground"> on their own page now gets a blank frame instead of a working editor.

What landing this newly exposes: every other route on the site is still in exactly the position /playground was in this morning. Measured, before and after: GET / and GET /components/Button return no framing headers at all. That gap is not new, but this PR makes the site's posture uneven — two routes declare, everything else is silent — and the silence is now the exception rather than the rule.

API

No API change. Nothing is exported, nothing is imported, no signature moves. @astryxdesign/docsite is private: true, so there is no consumer who could reach any of this.

Theme targets

n/a — structural only; the style grep over the file returns 0. No new theme targets.

Ossification

Nothing internal is added either — no helper, no constant, no shared shape. Two literal entries in a config array.

Breaking

• API — no. Private app, no exports touched.
• Visual — no. Measured: a diff of the full response header set on /playground, before vs after, in the same worktree and the same dev server — exactly two added lines (Content-Security-Policy: frame-ancestors 'self', X-Frame-Options: SAMEORIGIN). No body change is possible; the diff adds no element and no style.
• Theme — no. No targets, tokens or overrides involved.
• Behaviour — the one path the diff can reach is the preview iframe, and it survives: the parent's frame-ancestors governs the parent's own embedders, not its children, and the preview's 'self' is satisfied by a same-origin parent. /playground/ (trailing slash) still 308s to /playground and picks the headers up. Every other state — empty, loading, error, disabled, boundary — is not reachable: the diff adds no state, no default and no prop.

Performance & resources

Zero effects added, kept or moved. No runtime code at all: two static entries in next.config.mjs's headers() array, evaluated once at build. No listeners, no observers, no layout reads, no dependency, no bundle delta — the file is not bundled. The cost is two header lines on one route's responses.

Visual evidence

No frames. Reason: not renderable — this is build config. The change adds only response headers, and the measured before/after delta on /playground is those two header lines with the rest of the response identical. The page could not be rendered in a browser on this machine at all — src/app/globals.css imports a generated ../generated/themes.css that the repo's generation step produces and that step was not run, so every route 500s locally — but the header layer runs ahead of page rendering in Next, which is why the measurement below is the real one and not a proxy.

BEFORE (origin/main's next.config.mjs, same worktree, same server)
  GET /playground              → no X-Frame-Options, no Content-Security-Policy
  GET /playground/preview      → XFO SAMEORIGIN; CSP frame-ancestors 'self'; base-uri 'none'; … (full set)
  GET /                        → none
  GET /components/Button       → none

AFTER (PR head)
  GET /playground              → XFO SAMEORIGIN; CSP frame-ancestors 'self'          [NEW]
  GET /playground/preview      → unchanged, byte-identical
  GET /                        → none                                                 [still]
  GET /components/Button       → none                                                 [still]
  GET /playground/             → 308 → /playground

Reproduced by running the docsite's dev server against the head, curling each route for its x-frame-options and content-security-policy headers, then reverting apps/docsite/next.config.mjs to origin/main's copy in the same worktree for the before row.

A11y & i18n

n/a — structural only; the grep for aria-, role=, useTranslator and t('@astryx returns 0. No element is added, removed or renamed; no string reaches a user or an AT; no direction-sensitive property appears. pr-a11y and pr-rtl both report skipping on this head, which is the path filter behaving correctly for a diff that touches no component.

Judgement

approve and merge. A defect fix against the existing posture — no surface, no capability, no design call — so nothing here needs a human ruling.

1. /playground and /playground/preview are now a hand-maintained two-entry
   allowlist, and everything else on the site declares nothing
   → the next route someone adds under the playground is embeddable, silently
     and with no error, exactly as /playground was until this diff
                                        · apps/docsite/next.config.mjs:29
                                          source: '/playground',

2. The body says today's arrangement "works"
   → it reads as tidying, so the next person to touch these headers will not
     know the parent page was genuinely framable      · PR body, "Summary"

Neither blocks. Finding 2 is folded into the review's first clause rather than given its own sentence — the correction is what the author needs, not the argument for it.

Not raised: X-Frame-Options is obsolete and ignored wherever frame-ancestors is present. main already pairs them on the preview entry, so it is not this PR's.

CI: 4 workflow runs on this head, all success. Vercel is red — and it is red on every open PR from this author, including #5533 (scripts), #5531 (cli) and #5526 (a workflow file), none of which touch the docsite, plus another contributor's fork PRs; it passes on non-fork PRs. Fork infrastructure, not this diff. review-required is pending on the community-contribution gate, which the approval clears; an auto-merge will therefore not fire while Vercel stays red, which is a fact about the gate rather than a hold.

Three things found and not spent on the author: apps/sandbox and apps/template-viewer have no headers() at all — same shape, different app and different deploy, so outside this PR's radius; next dev refuses to start without --webpack on this repo, since Turbopack is Next 16's default and the docsite has a webpack config, which is pre-existing and unrelated; and #5523, same author, rewrites the playground↔preview channel and drops allow-same-origin — different files, no conflict, and it does not change the framing question either way.

Not verified: that the playground page still renders and the preview iframe still loads in a real browser at this head — every route 500s locally on a missing generated CSS file that was not built. The header layer is unaffected by that, but the page was not seen.

The review, as posted

Thanks, this is right — /playground really was serving no framing headers. Every other route still serves none; a /:path* default would cover them. Fine as a follow-up.

Full review

[Reviewed by Robohands]

No inlines: one finding, and the summary already carries it at 28 words — an inline at next.config.mjs:29 would be the same sentence with a line number on it.

Rounds

One review, two gate passes.

• Gate 1 — failed on four counts, all inside the comment. 135 words against a 30-word cap; no merge trailer, and prose about merging in the body instead; a paragraph restating the author's own change back to them; and an inline duplicating the summary. Everything above the review block was graded sound and carried forward unchanged. The gate's own summary of the instinct behind it: the draft was proud of its evidence.
• Gate 2 — clean. 28 words, one nit, no code block, no closing question. The one factual clause corrects the author's body with information they do not have, the finding is filed at the config entry whose scope causes it rather than at the body sentence describing it, the anchor was opened at head with its text pasted, and the head was re-fetched at hand-off and was unmoved. The Vercel claim was measured, not assumed: red on 10 of this author's 10 open PRs including three that touch no docsite file, green on three non-fork PRs.

What changed before posting