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

GET /api/v1/admin/people, /admin/invites and /admin/accounts carry an ETag and answer a repeat with 304 until who has access changes #9229

Merged opened this

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

Merged

`GET /api/v1/admin/people`, `/admin/invites` and `/admin/accounts` now return a weak `etag` computed over the exact bytes sent, with `cache-control: private, no-cache` and `vary: authorization, cookie`. A repeat that sends the tag in `If-None-Match` gets a 304 with the same headers and no body. The `securityFloor` headers are still applied because responses pass through the server as before.

shipped production is serving it deploy eb43263

opened this

GET /api/v1/admin/people, /admin/invites and /admin/accounts now return a weak etag computed over the exact bytes sent, with cache-control: private, no-cache and vary: authorization, cookie. A repeat that sends the tag in If-None-Match gets a 304 with the same headers and no body. The securityFloor headers are still applied because responses pass through the server as before.

The change is in src/http/api/admin.ts. A small local taggedJson(ctx, data) serializes the body once, tags it with weakEtag and checks it with holds. The three handlers call it in place of json(...), and only after accountAdmin and offsetQuery have passed. So a non-admin, a token without the scope the route asks for (admin for people, ro for invites and accounts) and a bad limit/cursor still answer exactly as before, with no tag, even with If-None-Match: *. The response bodies are unchanged byte for byte. No other admin route changed, and src/http/api.ts is untouched.

Tests: test/admin-people-etag.test.ts has 4 tests. All of them fail without the change and pass with it. They cover:

  • the 200 and 304 headers on each route, including the floor headers on the 304;
  • the tag moving when a person is added, made admin, has admin taken back, is suspended or is removed, and when an invite is made, used or revoked. Each time, the new tag gives a 304 again;
  • limit and cursor giving different tags, and one page's tag not producing a 304 on another page;
  • the refusal cases above.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. I also ran the 23 existing test files that call these routes, plus the new one: 178 pass, 1 skipped, 0 fail. scripts/fast.ts named 1113 files, too many to run here, so I chose those files myself.

Points to review:

  • The issue says offset, but this pager's offset parameter is cursor. The tests use cursor, and also check that a stray offset is refused with no tag.
  • Every admin sees the same lists, so "different viewers" is tested as different pages, which do get different bodies and different tags.
  • An admin-scope token held by a non-admin gets a 403 from a check that runs before these routes. The tests check that it answers the same with and without If-None-Match, rather than expecting one specific code.

The issue asks for the plan to be kept with keep_plan, but that tool wasn't available in this run, so the issue has no plan on it.

🤖 Generated with Claude Code

Closes #9224

commented

GET /api/v1/admin/people, /admin/invites and /admin/accounts now return a weak etag computed over the exact bytes sent, with cache-control: private, no-cache and vary: authorization, cookie. A repeat that sends the tag in If-None-Match gets a 304 with the same headers and no body. The securityFloor headers are still applied because responses pass through the server as before.

The change is in src/http/api/admin.ts. A small local taggedJson(ctx, data) serializes the body once, tags it with weakEtag and checks it with holds. The three handlers call it in place of json(...), and only after accountAdmin and offsetQuery have passed. So a non-admin, a token without the scope the route asks for (admin for people, ro for invites and accounts) and a bad limit/cursor still answer exactly as before, with no tag, even with If-None-Match: *. The response bodies are unchanged byte for byte. No other admin route changed, and src/http/api.ts is untouched.

Tests: test/admin-people-etag.test.ts has 4 tests. All of them fail without the change and pass with it. They cover:

  • the 200 and 304 headers on each route, including the floor headers on the 304;
  • the tag moving when a person is added, made admin, has admin taken back, is suspended or is removed, and when an invite is made, used or revoked. Each time, the new tag gives a 304 again;
  • limit and cursor giving different tags, and one page's tag not producing a 304 on another page;
  • the refusal cases above.

Checks run: bun scripts/gate.ts and bun x tsc --noEmit are clean. I also ran the 23 existing test files that call these routes, plus the new one: 178 pass, 1 skipped, 0 fail. scripts/fast.ts named 1113 files, too many to run here, so I chose those files myself.

Points to review:

  • The issue says offset, but this pager's offset parameter is cursor. The tests use cursor, and also check that a stray offset is refused with no tag.
  • Every admin sees the same lists, so "different viewers" is tested as different pages, which do get different bodies and different tags.
  • An admin-scope token held by a non-admin gets a 403 from a check that runs before these routes. The tests check that it answers the same with and without If-None-Match, rather than expecting one specific code.

The issue asks for the plan to be kept with keep_plan, but that tool wasn't available in this run, so the issue has no plan on it.

🤖 Generated with Claude Code

commented

GET /api/v1/admin/people, /admin/invites and /admin/accounts now return a weak etag computed over the exact bytes sent, with cache-control: private, no-cache and vary: authorization, cookie. A repeat that sends that tag in If-None-Match gets a 304 with the same headers and no body. The securityFloor headers stay on it because responses still pass through the server.

The first push failed the gate (bun scripts/gate.ts), not the deploy: the app booted and the preview answers. The gate requires a Try: line in a commit message whenever src/http/api/admin.ts changes, and the first commit had none. A follow-up empty commit adds it, and the gate now passes.

The change (src/http/api/admin.ts):

  • A small local taggedJson(ctx, data) serializes the body once, tags it with weakEtag and checks it with holds.
  • The three handlers call it in place of json(...), and only after accountAdmin and offsetQuery have passed. So these still answer exactly as before, with no tag, even with If-None-Match: *:

- a non-admin; - a token without the scope the route asks for (admin for people, ro for invites and accounts); - a bad limit or cursor.

  • Response bodies are byte-for-byte unchanged. No other admin route changed, and src/http/api.ts is untouched.

Tests (test/admin-people-etag.test.ts, 4 tests): all four fail without the change and pass with it. They cover:

  • the 200 and 304 headers on each route, including the floor headers on the 304;
  • the tag moving when a person is added, made admin, has admin taken back, is suspended or is removed, and when an invite is made, used or revoked; each new tag answers 304 again;
  • different pages getting different tags, and one page's tag not producing a 304 on another;
  • the refusal cases above.

Checks run: gate and bun x tsc --noEmit are clean. scripts/fast.ts named 1113 files, too many to run here, so I chose the tests myself:

  • before the first push, the 23 existing test files that call these routes plus the new one: 178 pass, 1 skipped, 0 fail;
  • after the fix, the new test plus admin-lists-paged, invites, accounts and admin-gate: 45 pass, 0 fail.

Points to review:

  • The issue says offset, but this pager's offset parameter is cursor, so the tests use cursor.
  • Every admin sees the same lists, so "two viewers shown different bodies" is tested as different pages.
  • An admin-scope token held by a non-admin gets a 403 from a check that runs before these routes. The tests check that it answers the same with and without If-None-Match, rather than expecting one specific code.
  • The issue asks for a plan kept with keep_plan, but that tool wasn't available in this run, so the issue has no plan on it.

Try: sign in as preview-admin on the preview and GET /api/v1/admin/people twice, sending the first etag as If-None-Match; the second answers 304.

🤖 Generated with Claude Code

Approved

Clean. taggedJson runs only after the admin, scope and query checks, so refusals carry no tag even with If-None-Match: *. The body is byte-identical to json(). The 304 carries the tag, cache-control and vary, and the security floor still applies. The tests cover each behaviour. This is a read-path caching change, so it adds no terminal screen or feature how-to. I did not run the preview or the tests myself. Note: the issue's offset is cursor in this pager, and the tests use cursor.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs