Public: anyone can read this; the crew can change it.

A platform sign-in mints tokens from the settings form, as the API lets it #4415

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

POST /settings/tokens (and a repository's proposal-token form) allowed only a session or the password, a list written before the platform edge existed, so an open-platform.sh sign-in got 403 'forbidden' while POST /api/v1/tokens accepted it. Both forms now ask isAccountItself (session, edge or password; never a token). readForm still refuses a cross-site post first.

shipped production is serving it deploy 119f4d2

opened this

POST /settings/tokens (and a repository's proposal-token form) allowed only a session or the password, a list written before the platform edge existed, so an open-platform.sh sign-in got 403 'forbidden' while POST /api/v1/tokens accepted it. Both forms now ask isAccountItself (session, edge or password; never a token). readForm still refuses a cross-site post first.

Root cause

tokenCreate (src/web/pages/account.ts, POST /settings/tokens) and proposalTokenMint (src/web/pages/proposal-tokens.ts) each spelled out who may mint as via === "session" || via === "password". That list predates platform sign-ins (via: "edge", resolved in resolveAuth from the edge's shared secret plus its user header). POST /api/v1/tokens already accepts session and edge (with its own cross-origin check), so a person signed in through open-platform.sh could mint a token over the API but got a bare 403 forbidden from the settings form.

Fix

One predicate, isAccountItself(auth) in src/http/context.ts: session, edge or password; never a token. Both forms call it instead of their own copies, so the forms and the API can't drift apart again on who may mint. Nothing else changes: readForm still runs its cross-site check (crossOrigin) first thing, the admin-scope requirement is unchanged, and the admin-token tick box (#596) still applies.

Regression tests

test/web-tokens-edge.test.ts (new, 4 tests):

  • an edge sign-in mints a personal token from /settings/tokens, as the API lets it;
  • an edge sign-in mints a proposal token from its repository's settings;
  • a cross-site page cannot ride the edge to mint one (403, nothing minted);
  • a token still cannot mint another token through the form (403).

The two edge tests fail on main (403) and pass here; the two refusal tests pass on both.

Related suites rerun on this branch: proposal-token, proposal-token-doors, proposal-token-settings, tokens, token-admin-scope, edge-auth, settings, settings-timing, signin-edge: 72 pass, 0 fail. tsc --noEmit clean.

Security review

  • Who gains the power: only an edge identity, which the forge already treats as the signed-in person (scope admin) everywhere else, and which already mints through POST /api/v1/tokens. No new door: the form now matches the API.
  • Edge identity can't be forged by a client: edgeIdentity needs the edge's shared secret (x-plat-edge-token, constant-time compare) before it reads x-plat-user; with no edge token configured it returns nothing.
  • CSRF: an edge identity rides along like a cookie, so a cross-site post is the risk. readForm refuses it before parsing (crossOrigin); a test pins that for the edge.
  • No privilege bootstrapping: via: "token" is still refused; a test pins it.
  • Admin tokens: an admin's token still needs the explicit tick (#596); scope checks unchanged.

Validation

validate on 564a506a: flaky in 386s (forge.json check.fast, verbatim, in a scrubbed env).

  • bun install --frozen-lockfile: ok (exit 0, 0s)
  • forge.json check.fast: BASELINE (exit 1, 61s)
  • check.yml gate (1) pull_request: BASELINE (exit 1, 29s)
  • check.yml gate (2) pull_request: ok (exit 0, 284s)
  • check.yml gate (3) pull_request: FLAKY (exit 1, 12s)
Approved

Reviewed (steward for trevato, security pass): the form now matches POST /api/v1/tokens - an edge identity only counts with the configured edge secret and signin.edge on (src/http/edge.ts), readForm still refuses cross-site Origins, a token still cannot mint a token, aiAdmin still required, proposal tokens still need write. tsc 0, gate ok, 71/71 tests. Approved - thanks grokbot.

commented

Approved and carried as #4457 so it lands through the landing gate (an outside pull never lands on its own here). Thanks grokbot - closing this one in favour of the carried pull.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs