Make the Control Plane embeddable in a cross-site iframe

[logical-and]

Reworked per review: this is now embeddability-only. The prefix-based authz (tokens.ts, bank-guard.ts, session prefix, middleware prefix checks, whoami, bank-selector isAdmin) has been dropped — see the discussion below. Went from 37 files / ~1050 added lines to 19 changed + 4 new.

What this adds

Lets another app embed the Control Plane in an iframe.

• HINDSIGHT_CP_FRAME_ANCESTORS — applied as a runtime frame-ancestors CSP in middleware. It has to be runtime: Next bakes next.config headers() at build time, so a container-time env would be silently ignored. Falls back to 'self', never *.
• HINDSIGHT_CP_COOKIE_SAMESITE=none — emits the session cookie as SameSite=None; Secure; Partitioned (CHIPS) so it survives in a cross-site frame. Default stays SameSite=Lax, so nothing changes unless you opt in.
• /api/auth/embed-login — the embedding page POSTs the access key (form-encoded or JSON, optional returnTo) and the frame lands on the dashboard in one round trip via 302 + Set-Cookie, with Cache-Control: no-store.
• Origin/CSRF gate — SameSite=None drops the browser's implicit CSRF defense, so this replaces it: cross-site state-changing /api/* requests are rejected unless the Origin is same-origin or in the frame-ancestors allowlist. Falls back to Sec-Fetch-Site; header-less non-browser clients still work. Deliberately coupled to the cookie change, and applied before the public-path bypass so it also covers login and embed-login (login-CSRF).
• Proxy redirect fix — post-login redirects were building Location from request.url, which behind a reverse proxy is the internal upstream (0.0.0.0:9999), sending the frame somewhere dead. Now built from x-forwarded-proto/x-forwarded-host.

Also included: bank-id traversal guard

Independent of the authz discussion, and a live bug on main today. Four routes hand-build their dataplane URL and interpolate bank_id unencoded:

• api/graph/route.ts
• api/documents/[documentId]/chunks/route.ts
• api/documents/[documentId]/reprocess/route.ts
• api/banks/[bankId]/observations/scopes/route.ts

A bank_id of x/../y normalizes to a different bank's path before it ever reaches the dataplane. These now go through dataplaneBankUrl(), which additionally throws on a path separator or .. segment rather than encoding it — a bank id is a single path segment, so those shapes are malformed by definition, and failing loudly stops the next hand-built URL from quietly reintroducing it.

Security model

Worth being explicit, since it drives the division of responsibility: the access key is all-or-nothing, so an embedded iframe is a full admin console over every bank. The embedding app decides who may load it, and the key must stay server-side and never reach the browser. For per-user scoping, put a reverse proxy in front that pins each embed to its own bank(s). This is documented in .env.example.

Testing

• tests/lib/auth/request-guard.test.ts — Origin/CSRF logic: safe methods, same-origin (incl. x-forwarded-host), allowlisted embed origin, scheme/port mismatch, CSP-keyword filtering, Sec-Fetch-Site fallback, unparseable origin
• tests/lib/hindsight-client.test.ts — traversal rejection + encoding, incl. asserting a normalized URL keeps its intended path
• tests/lib/auth/session.test.ts — extended for the SameSite=None; Secure; Partitioned opt-in and the Lax default

138 tests pass; i18n:check clean (added the forbidden key across all 10 locales); next build + standalone OK.

[benfrank241]

thanks @logical-and — this is a genuinely substantial and well-documented contribution (constant-time token compare, the bankAllowed predicate reused across layers so they can't drift, backwards-compat via the admin key resolving to the empty prefix, i18n parity across all 10 locales, 45 unit tests). appreciate the care.

routing it to @nicoloboschi to own rather than reviewing-to-merge here, because this is squarely a maintainer call on two axes, not something to fast-track:

1. it's a new security/auth capability (~660 lines, 29 files) — multi-token prefix scoping, HMAC session cookies, cross-site iframe embedding with SameSite=None; Secure; Partitioned, runtime CSP frame-ancestors, and prefix-isolation enforced across every CP API route. that's exactly the surface where a subtle gap = cross-scope data leak / auth bypass / clickjacking, so it needs a careful security pass, not a functional one. specific things worth Nicolo's scrutiny: the bankAllowed boundary (u2 vs u20, and <prefix>--* matching against real bank-id formats), that the three enforcement points (middleware path/query, body-route assertBankAllowed, and the /api/banks list filter) can't be bypassed by an unlisted route or an encoded id, the frame-ancestors fallback ('self', never *), and the cross-site cookie posture.

2. it introduces a product direction — per-user iframe embedding of the CP via a token-prefix model, plus three new HINDSIGHT_CP_* env vars. whether we want to support that embedding model (and this shape of it) in the CP is an architecture decision for the maintainer; per our norms a non-trivial auth/security feature like this usually wants a design nod before it lands.

no code objection from me on a first read — it's clearly thoughtfully built. just want the direction + the security review to be Nicolo's. cc @nicoloboschi.

[nicoloboschi]

The architecture here is sound — one bankAllowed predicate reused across all three enforcement layers so they can't drift, HMAC-signed prefix in the cookie, constant-time compares, frame-ancestors defaulting to 'self' and never *. Nice work on the layering.

That said, there are a few security issues, and one is an exploitable scope bypass. Flagging before merge.

Must fix

1. Prefix scoping can be bypassed via path traversal in bank_id (high)

Four routes interpolate bankId into the dataplane URL without encoding:

• src/app/api/graph/route.ts:45
• src/app/api/documents/[documentId]/chunks/route.ts:28
• src/app/api/documents/[documentId]/reprocess/route.ts:25 (a write)
• src/app/api/banks/[bankId]/observations/scopes/route.ts:20

A session scoped to u2 requests GET /api/graph?bank_id=u2--x/../victim. Middleware calls bankAllowed("u2", "u2--x/../victim"), which passes the startsWith("u2--") check. The route then builds the dataplane URL and WHATWG URL normalization collapses the dot segment:

http://dataplane/v1/default/banks/u2--x/../victim/graph  →  .../banks/victim/graph

Result: full cross-tenant read of another user's graph and document chunks, plus a reprocess write. dataplaneBankUrl() correctly uses encodeURIComponent — these four just bypass it.

Suggested fix in two places:

1. Route all four through dataplaneBankUrl().
2. Also reject bank ids containing /, \, or .. inside bankAllowed() itself, so the predicate is safe by construction no matter how a caller builds its URL. Part 2 is what prevents the next route from silently reintroducing this.

2. SameSite=None removes the only CSRF defense (high)

There's no CSRF token and no Origin / Sec-Fetch-Site check anywhere in the control plane today — sameSite: "lax" was carrying that load implicitly. Once HINDSIGHT_CP_COOKIE_SAMESITE=none is set, any site the victim visits can issue authenticated state-changing requests with their cookie: create banks, retain, PATCH memories, reprocess documents, import transfers. Admin sessions included.

The embedding use case genuinely needs SameSite=None, so the fix isn't to drop it — it's to add an Origin check on all non-GET /api/* requests in middleware, allowing same-origin plus the origins already configured in HINDSIGHT_CP_FRAME_ANCESTORS. The allowlist exists; it just needs to gate requests as well as framing.

3. /api/auth/embed-login is unauthenticated login-CSRF (medium-high)

It matches /api/auth/ in PUBLIC_PATTERNS, accepts a form-encoded POST, and force-recreates the session cookie. Any cross-origin page can therefore silently overwrite a visitor's session with an attacker-supplied token. Combined with SameSite=None this sticks: an admin who browses a hostile page gets swapped into the attacker's scope, and anything they subsequently retain or upload lands in the attacker's bank.

Same Origin check applies here. The 302 response should also carry Cache-Control: no-store.

Separately, neither /login nor /embed-login is rate-limited, so both are brute-force oracles against HINDSIGHT_CP_TOKENS.

Should fix

4. An empty prefix in HINDSIGHT_CP_TOKENS silently grants admin

tokens.ts:74 accepts any string for prefix, and bankAllowed("", …) returns true unconditionally. So a config typo — {"token":"x","prefix":""} — hands out full cross-bank access with no warning at all. Worth rejecting empty prefixes at parse time, alongside the existing missing-field validation.

5. /api/chunks/[chunkId]/route.ts has no bank scoping

No path-collection match, no query param, no body guard. Any scoped session can read any chunk by id. UUIDs make it non-enumerable in practice, but it's an exception to the PR's "every route that takes a bank_id" claim — either scope it or note why it's exempt.

6. No revocation path for scoped tokens

Sessions are HMAC'd with the admin key, so removing a token from HINDSIGHT_CP_TOKENS leaves its outstanding cookies valid for up to 24h. Rotating the admin key is the only kill switch and it logs everyone out. At minimum worth calling out in the .env.example comment.

Minor

• constantTimeEqual early-returns on length mismatch, leaking token length. Standard tradeoff, but the comment in resolveToken claims timing doesn't leak which token matched — that only holds for equal-length tokens.
• bank-selector.tsx: .catch(() => setIsAdmin(true)) fails open. Cosmetic only since the server enforces, but it'll flash admin chrome to scoped users on a transient whoami failure.
• Middleware overwrites Content-Security-Policy wholesale rather than merging. Fine today since nothing else sets one, but it will silently clobber a future policy.

Test coverage

The tests only cover session.ts round-trips. Nothing exercises the layers that actually carry the security guarantee: bankAllowed (especially the u2/u20 boundary and traversal-shaped inputs), apiTargetBankId, or the middleware 403 paths. The PR verified those by hand on a live deployment — that verification should be tests. Issue #1 above is exactly the case a bankAllowed unit test would have caught.

[logical-and]

Thanks @nicoloboschi, this is a great review — especially catching the traversal bypass. Pushed 3afe7d2 addressing everything. Rundown:

Must fix

1. Path-traversal scope bypass — fixed at both layers you suggested. The four routes (graph, documents/[id]/chunks, documents/[id]/reprocess, banks/[id]/observations/scopes) now go through dataplaneBankUrl() (with encodeURIComponent on the documentId too). And bankAllowed() now rejects any bank id containing /, \, or a .. segment up front via a new bankIdHasTraversal() — even under the admin/empty prefix — so the predicate is safe by construction and a future route that hand-builds a URL can't silently reintroduce it. The exact u2--x/../victim case is now a unit test.

2. SameSite=None / CSRF — added an Origin check on all non-GET /api/* in middleware. Cross-site writes are rejected unless the Origin is same-origin (honoring x-forwarded-host behind the proxy) or one of the origins already in HINDSIGHT_CP_FRAME_ANCESTORS. Falls back to Sec-Fetch-Site when there's no Origin, and allows header-less non-browser clients (a browser CSRF always carries one of the two).

3. embed-login login-CSRF — the Origin gate runs before the PUBLIC_PATTERNS bypass, so it covers /api/auth/login and /api/auth/embed-login too; only a configured embed origin can drive the auto-login. Added Cache-Control: no-store to the 302. (Rate-limiting /login + /embed-login I left out for now since there's no shared limiter in the CP — happy to add one if you'd prefer it in this PR.)

Should fix

4. Empty prefix → admin — HINDSIGHT_CP_TOKENS entries with an empty or non-string prefix are now dropped at parse time with a warning, instead of silently resolving to the admin scope.

5. /api/chunks/[chunkId] — now scoped: it enforces the session prefix against the chunk's bank_id from the response (the chunk's bank isn't known until then, since it's addressed by a global id). Admin/no-access-key still pass through.

6. Revocation — documented the limitation in .env.example (removing a token leaves its cookies valid until expiry; rotating HINDSIGHT_CP_ACCESS_KEY is the kill switch).

Minor — corrected the resolveToken comment re: token-length timing; bank-selector now fails closed (isAdmin defaults to false, no admin-chrome flash on a transient whoami failure). Left the CSP-overwrite note as-is since nothing else sets a policy today, but happy to merge instead if you want.

Tests — added tests/lib/auth/tokens.test.ts (the u2/u20 boundary, traversal-shaped inputs, empty-prefix rejection) and tests/lib/auth/request-guard.test.ts (apiTargetBankId + the Origin/CSRF logic). To keep those pure helpers testable without pulling next-intl into the test, I extracted apiTargetBankId/isCrossSiteWrite into src/lib/auth/request-guard.ts. Full suite green (102 tests), plus i18n:check and next build.

[logical-and]

Rebased onto current main (d80ef72) — the branch had fallen 234 commits behind and gone conflicting, so it wasn't mergeable. It is now conflict-free.

Only one real conflict, in bank-selector.tsx: main added the logo-spin useEffect in the same spot as this PR's whoami scope lookup. Both are independent effects, so both are kept. Everything else (including the 10 locale files) merged cleanly.

Re-verified on the new base: full CP suite green (130 tests, up from 102 as it now includes main's newer tests), i18n:check clean, next build + standalone OK. The net diff is unchanged from the reviewed head — same 1795 lines, security fixes all intact.

@nicoloboschi no rush, but this is ready whenever you have time for a re-review — the 3afe7d2 round addressed all six points from your review (details in my earlier comment). Still happy to add the /login + /embed-login rate limiting or the CSP header merge if you'd like either in this PR rather than a follow-up.

[nicoloboschi]

Thanks for this — the security-sensitive parts are noticeably careful (HMAC over the prefix, constant-time compares, traversal baked into the predicate, the open-redirect and CSRF handling). But I'd like to split it, and take only half.

Happy to take: the embeddability pieces

These stand on their own and don't create a durable security boundary we have to keep correct forever:

• HINDSIGHT_CP_FRAME_ANCESTORS runtime CSP
• SameSite=None; Secure; Partitioned cookie option
• embed-login auto-login convenience
• the Origin/CSRF gate (isCrossSiteWrite) — this must stay coupled to the cookie change, not to the authz. The moment we ship SameSite=None we've dropped the browser's CSRF defense, and the Origin check is what replaces it, independent of any scoping.

Would rather not carry: the prefix-based authz

The reason is architectural, not about the code quality. The scoping is a security boundary enforced at the Control Plane proxy layer over a bank-blind dataplane — everything still lives in the single default tenant, and the only thing keeping u2 out of u5's data is the CP remembering to check the bank_id prefix on every route. That's a bouncer at the door, not a lock on each room. Coverage is complete today (I checked — path/query routes via middleware, body routes via assertBankAllowed, the global-id chunk route via a response check), but it's allowlist-by-enumeration with no default-deny: any future route that reads bank_id from a body and isn't added to the guard list, or uses a differently-named param, silently leaks cross-tenant. I don't want the OSS admin console to own an authz boundary that has to be re-proven correct on every new route.

Suggested alternative: enforce scoping in the deployment layer

For per-user embedding, put a pinned reverse proxy in front of the CP, one route/instance per embedded user, hardwired to that user's bank(s), rejecting any request that references a different bank (including the global-id routes). Because it's pinned it doesn't need to reimplement the prefix cleverness or parse bodies for reads — it enforces a fixed bank_id and 403s everything else. That keeps the boundary where this kind of policy belongs and out of code we'd have to maintain.

The trade-off to be explicit about: without the authz, the embedded iframe is full-admin (the single access key sees all banks). So the embed is only safe when the outer app/proxy controls who can load it and the key never reaches the browser — which is exactly "the proxy owns authz." That's the right division of responsibility here: we ship an embeddable admin console; the embedder scopes it.

Would you be up for reworking this into an embeddability-only PR (CSP + cookie + embed-login + CSRF gate), and we drop tokens.ts / bank-guard.ts / the session prefix / the middleware prefix checks / whoami / the bank-selector isAdmin gating? Happy to review that promptly.

[logical-and]

That's a fair call, and the reasoning lands — "allowlist-by-enumeration with no default-deny" is exactly the right characterization, and an OSS admin console shouldn't own a boundary that has to be re-proven on every new route. Reworked to embeddability-only in d656cf5.

Kept: runtime frame-ancestors CSP, SameSite=None; Secure; Partitioned cookie, embed-login, and the Origin/CSRF gate — coupled to the cookie change exactly as you asked, and applied before the public-path bypass so it covers login/embed-login too. Also kept the proxy redirect fix (Location was being built from the internal upstream host).

Dropped: tokens.ts, bank-guard.ts, the session prefix, the middleware prefix checks, whoami, and the bank-selector isAdmin gating. session.ts is back to the plain boolean verify — the only change there now is the cookie options. 37 files → 19 changed + 4 new.

I documented the trade-off you named directly in .env.example: the embedded iframe is a full admin console, the embedder decides who can load it, the key never reaches the browser, and per-user scoping belongs in a pinned proxy out front.

---

One thing I did keep, and want to flag explicitly — the bank-id traversal guard, because it's not part of the authz story and it's a live bug on main right now.

Four routes hand-build their dataplane URL and interpolate bank_id unencoded (graph, documents/[id]/chunks, documents/[id]/reprocess, banks/[id]/observations/scopes). Independent of any prefix scoping, a bank_id of x/../y normalizes to a different bank's path before it reaches the dataplane — so a caller authorized for one bank reads or reprocesses another, and reprocess is a write. That's true today with the single admin key; it just isn't very interesting when every caller is already admin. It gets sharper under the proxy model you're proposing, since a pinned proxy that fixes bank_id=u2 can still be walked out of by u2/../u5 unless the CP refuses the shape.

So those four now go through dataplaneBankUrl(), and that helper throws on a /, \, or .. segment rather than encoding it. Encoding alone would make it safe at that one call site, but a bank id is a single path segment — those shapes are malformed by definition, and throwing means the next route that hand-builds a URL fails loudly instead of silently resolving to someone else's bank. Happy to split it into its own PR if you'd rather review it separately from the embedding work.

The rate-limiting on /login + /embed-login I still haven't added (no shared limiter in the CP) — say the word if you want it here rather than as a follow-up.