Skip to content

feat(runner): persistent API tokens, verified in the Worker (DEV-2583) - #252

Merged
demtario merged 5 commits into
masterfrom
feat/DEV-2583-persistent-api-tokens
Aug 21, 2026
Merged

feat(runner): persistent API tokens, verified in the Worker (DEV-2583)#252
demtario merged 5 commits into
masterfrom
feat/DEV-2583-persistent-api-tokens

Conversation

@demtario

@demtario demtario commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes DEV-2583.

Why

The nightly live canary has one secret-gated step: the authed share round-trip, which is the only test that drives the builder, R2 and D1 end to end against production. Its credential is secrets.E2E_BROKER_TOKEN — a per-user browser session JWT copied by hand out of sessionStorage.hot_token, which expires in about an hour. The workflow knows this and treats a dead token as a notice and a green run, because "the secret rots by design and rot is not a product failure". The honest reading is that the step is skipped every night and the coverage is theoretical.

There was no mint path to automate. authenticate() does not verify a JWT at all — it forwards the bearer to the login broker's /broker/userinfo and trusts the address that comes back (ADR-0007). Nothing in the system could issue a credential, and nothing could validate one it had not been handed by the broker.

What

A first-party credential, hot_pat_<id>_<secret>, minted from a new API tokens page, stored as a SHA-256 digest, verified inside the Worker, and revocable by anyone on the team. Full reasoning in ADR-0037; the decisions worth knowing before reading the diff:

  • The prefix is checked before anything touches the network, and the two paths never fall through to one another. This is a security property rather than a latency win: a bearer that reached the broker because the local lookup missed would have shipped our own permanent credential to a third-party host on Render, and the failure would have looked like a slow success.
  • Only a digest is stored. SHA-256, not bcrypt/argon2, and the ADR says why out loud because it is the reflex objection: this is a 256-bit random secret rather than a password, so there is no dictionary to run and no low-entropy guess space to grind, and the id makes the lookup a primary-key hit. A work factor would cost every authenticated request and defend nothing.
  • A token acts as its creator's address. sameOwner(), created_by, ?scope=mine and "My demos" are untouched, and no new address shape enters a system whose only membership check is endsWith("@handsontable.com"). Identity gains via (the token id) for the fence and the audit trail.
  • A capability fence, because this credential lives in a public repository's secrets and any team identity is currently a full admin. No admin writes, no session kills, no /api/chat or /api/theme, and no token management at all — reads included. So a leaked token cannot raise the spend ceiling, turn enforcement off, burn AI budget, mint itself a successor, or enumerate the credentials it would need to revoke. Admin reads stay open, which the session-leak spec needs. Fixed rule, not a scope field: this repo has never had a permission model and one consumer exists.
  • Every token is visible to, and revocable by, the whole team. This knowingly sets aside the standing warning that ?scope=all is visibility and must never quietly become permission — a permanent credential only its author can kill is worse than one anybody can, because the author will eventually be on holiday and the token will not expire on their behalf.
  • last_used_at is coarsened to the hour by a single conditional UPDATE, so the hot auth path cannot generate more than one write per token per hour and there is no read-then-write race to reason about.
  • The client accepts a token too, resolving identity against our own GET /api/profile instead of the broker. That is what keeps the live spec driving the real Share button rather than being rewritten into an API script. The cost is stated in the ADR: such a token is a browser session in a string, which is why the fence exists and why the workflow's trace scrubbing matters more than it used to.

Three things the build caught that reasoning would not have

  • The vite dev proxy's /api key is a bare prefix, so /api-tokens was proxied to the Worker and 500'd. That key was always wider than the demos.handsontable.com/api/* route it stands in for — the same bug /d had already been regex'd for. Fixed the same way.
  • A token_hash that is not a usable string 500'd out of the auth path instead of refusing. NOT NULL in the schema, so this only reaches a hand-edited row or one caught mid-backfill, but a 500 there would be reported as an outage. Guarded, with a test.
  • The DEV_AUTH_EMAIL loopback bypass ran ahead of the token branch. Every developer is told to put that in .dev.vars, so a presented token was accepted as a personvia never set, fence never engaged, wrangler dev behaving as the exact opposite of production on the one thing this feature is careful about. The token check now runs first, wherever the Worker is running.

Verification

  • pnpm test — 879 tests, 0 failures. Two new specs: pipeline/api-token.test.mjs (the credential's own rules) and pipeline/token-routes.test.mjs (the real router, not a re-declared copy of its checks).
  • pnpm -r typecheck — clean, run through raw pnpm rather than the token-filtering proxy, which has reported a false green before.
  • Playwright — 92 passed, exit 0, across the deterministic specs plus the new e2e/api-tokens.spec.ts. Run on a dedicated port, because another worktree already owned 4173 and would have silently served the wrong build.
  • The tests were checked against a neutered implementation, not just watched to pass: deleting the chat fence reddens one case, disabling the auth branch reddens eight, and disabling the token-session gate on the new page reddens its own.
  • The shared D1 fake models api_tokens including both conditional UPDATEs, so the idempotent revoke and the hour coarsening are actually exercised rather than assumed. fakeKV gained the list() shape adminSessions() reads.

Before this ships

  • wrangler d1 migrations apply for 0006_api_tokens.sql (the deploy workflow does this; the hand-run lists in run-and-deploy.md are updated too).
  • The GitHub secret is renamed E2E_BROKER_TOKEN -> E2E_API_TOKEN. It needs creating from a token minted on /api-tokens after deploy, and the old secret can then be deleted. Absent, the step still skips with a notice; present and not validating now fails the run, because a credential with no expiry that stops working means revoked or broken.

🤖 Generated with Claude Code


Note

High Risk
Adds a new never-expiring credential class, changes Worker authentication, and fences token capabilities. A leak or fence miss is security-critical; CI now fails if the GitHub secret is present but invalid.

Overview
Adds persistent first-party API tokens (hot_pat_<id>_<secret>) so the nightly authed share canary can run without a hand-copied, hourly-expiring broker JWT (ADR-0037).

The Worker now prefix-branches before any network call: tokens are verified locally against a SHA-256 digest in D1 and act as the creator’s email (Identity.via). They cannot manage tokens, spend AI budget, change guardrail settings, or kill sessions. Listing and revoke are org-wide. The SPA gets /api-tokens (one-time plaintext reveal, team list, confirm-to-revoke) and treats a token session as signed-in via /api/profile, hiding Ask AI / Style.

Live E2E switches E2E_BROKER_TOKENE2E_API_TOKEN. Absent still skips; a present token that fails /api/profile fails the run. Vite’s /api proxy is regex-scoped so /api-tokens is not swallowed.

Reviewed by Cursor Bugbot for commit 3b227f3. Bugbot is set up for automated code reviews on this repo. Configure here.

demtario and others added 2 commits August 21, 2026 10:11
The nightly live canary's authed step is gated on a hand-copied broker
session JWT that expires in about an hour, so the one test that drives
builder + R2 + D1 against production is skipped every night behind a
green run.

Records the decision: a first-party `hot_pat_<id>_<secret>` credential,
prefix-discriminated before any network call, stored as a SHA-256 digest,
verified inside the Worker, acting as its creator's address, fenced off
admin writes / chat / theme / token management, and revocable by anyone
on the team.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The nightly canary's authed step was gated on a hand-copied broker session
JWT that expires in about an hour, so the one test that drives builder +
R2 + D1 against production was skipped every night behind a green run.
There was no mint path to automate: `authenticate()` forwards the bearer
to the broker and trusts the address it returns, and nothing in the
system could issue or validate a credential of its own.

Adds `hot_pat_<id>_<secret>` (ADR-0037): minted from a new API tokens
page, stored as a SHA-256 digest in `api_tokens`, prefix-discriminated
before any network call so our own permanent credential is never posted
to the broker's third-party host, and verified with one primary-key read
plus the existing constant-time compare. A token acts as its creator's
address, so `sameOwner()`, `created_by` and `?scope=mine` are untouched.

Capability fence, because this credential lives in a public repository's
secrets and any team identity is currently a full admin: no admin writes,
no session kills, no `/api/chat` or `/api/theme`, and no token management
at all — reads included, so a leaked token can neither mint a successor
nor enumerate the credentials it would need to revoke. Admin *reads* stay
open, which the session-leak spec needs.

Every token is visible to and revocable by the whole team: the author of
a permanent credential will eventually be on holiday, and it will not
expire on their behalf.

Also fixes the vite proxy's `/api` key, which was a bare prefix and so
swallowed `/api-tokens` — wider than the `demos.handsontable.com/api/*`
route it stands in for, the same bug `/d` was already regex'd for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario demtario self-assigned this Aug 21, 2026
Comment thread runner/workers/api/src/auth.ts Outdated
Comment thread runner/apps/authoring/src/ApiTokens.tsx
Comment thread runner/apps/authoring/src/ApiTokens.tsx Outdated
demtario and others added 2 commits August 21, 2026 12:00
…ow (DEV-2583)

Three findings from Bugbot on #252, all reproduced before fixing.

RFC 7235 spells the header `auth-scheme 1*SP token68`, so `Bearer  <token>`
— two spaces, a stray tab, a typo in a `curl -H` — is well-formed. Slicing
a fixed "Bearer " off the front left that whitespace attached, the value
then missed the `hot_pat_` prefix test, and both of the things ADR-0037
exists to prevent followed: the credential was forwarded to the broker,
and `presentsToken()` read it as no credential at all, so a token could
have spent AI budget as an anonymous visitor. `bearerFrom()` is now the
single parser for both callers, and two route-level tests fail against
the old parsing.

The one-time reveal field outlined itself with `border` on `surfaceSunken`
— #222222 against #353535 in dark, i.e. no visible edge on the one field
that must stay obvious after a mint. `controlBorder`, which the shared
`fieldInput` already uses for this reason.

`onApiTokens` was gated on a token session only in the editor, so the
account-menu row stayed live on Settings, My Demos, Guide and the tokens
page itself — an enabled control leading to a page that explains it
cannot be used.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…fixes (DEV-2583)

Second high-effort review pass on #252.

The revoke confirmation used React's `autoFocus` on Cancel, which loses to
`Dialog`'s own focus effect — that runs after the layout phase and lands on
the content's first focusable, i.e. **Revoke**. Opening the dialog and
pressing Enter therefore revoked a live credential without the question
having been answered. `data-autofocus` is the hatch `Dialog` documents for
exactly this and what every other destructive confirm in the app uses. An
e2e case now asserts focus and that Enter revokes nothing; it fails
against the old markup.

`bearerFrom()` matched the auth scheme case-sensitively. RFC 7235 makes it
case-insensitive, and the difference is load-bearing rather than pedantic:
`bearer hot_pat_…` returned null, fell through to the `DEV_AUTH_EMAIL`
bypass on a loopback host, and was granted a *person* identity with no
`via` — so the capability fence did not engage locally, which is the exact
failure the branch ordering exists to prevent.

A failed listing rendered "No tokens yet.", telling the reader of a page
whose only job is enumerating live credentials that there are none to worry
about. Failure is now its own state, distinct from "still loading".

Neither mint nor revoke had the `isSessionExpired` branch that `MyDemos`
uses, so an expired session left an enabled form that 401'd on every click;
and a failed revoke painted its error behind the modal scrim, where the
dialog hid it. Both fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 98d00b1. Configure here.

Comment thread runner/apps/authoring/src/ApiTokens.tsx Outdated
A regression from the previous commit. Making failure its own render state
put `loadFailed` ahead of `tokens` in the branch, while Create stayed live
after a failed read — so a mint wrote its row into state and the page kept
showing the failure message instead. The plaintext callout appeared, the
row did not, and the credential that had just been created could not be
revoked from the page. That is the worst state this feature has.

The notice is now a line beside the list rather than instead of it, so the
minted row and its Revoke are reachable. The notice stays up either way:
one row known locally is not the list, and the page must not imply it is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario
demtario merged commit 4826e73 into master Aug 21, 2026
7 checks passed
@demtario
demtario deleted the feat/DEV-2583-persistent-api-tokens branch August 21, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant