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

The secrets list and a person's sign-ins read name their Server-Timing phases #5018

Merged scoutcrew opened this 22d ago

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

Merged

`GET /api/v1/secrets/:scope` and `GET /api/v1/users/:username/signins` now send a `Server-Timing` header naming `session`, the read (`secrets` or `signins`), then `render`, in that order. I haven't seen this on a running preview. The new tests and the 83 existing tests that touch these routes pass, but `bun x tsc --noEmit` failed on missing `bun` types, so there's no type-check result.

shipped production is serving it deploy d5155e2

scoutcrew opened this 22d ago

GET /api/v1/secrets/:scope and GET /api/v1/users/:username/signins now send a Server-Timing header naming session, the read (secrets or signins), then render, in that order. I haven't seen this on a running preview. The new tests and the 83 existing tests that touch these routes pass, but bun x tsc --noEmit failed on missing bun types, so there's no type-check result.

What changed: the change is in src/http/api.ts. session is noted first, using the same notePhase/takeSessionTime call as …/tags. The access check (requireScope, requireAccount) stays outside the read's phase. render times only the json(…) call. No name, token or value goes into a phase. Bodies, status codes and access rules are untouched.
Refused reads: a 401 or 403 still names session and never secrets or signins. A 404 on sign-ins (unknown account) names session and signins, since the lookup ran. A secrets error returned after the read (the 422 path) is the same: it names session and secrets but no render.
Tests: test/secrets-signins-timing.test.ts has four tests following the test/crew-timing.test.ts pattern.

- Each route asserts its phase order on a 200. - An anonymous secrets request still names session. - An anonymous and a missing-account sign-ins request still name session.

Left out: the PUT and DELETE secrets routes and the sign-ins session removal are not timed. The issue only covers the two reads.

Closes #5006

scoutcrew commented 22d ago

What changed: the change is in src/http/api.ts. session is noted first, using the same notePhase/takeSessionTime call as …/tags. The access check (requireScope, requireAccount) stays outside the read's phase. render times only the json(…) call. No name, token or value goes into a phase. Bodies, status codes and access rules are untouched.
Refused reads: a 401 or 403 still names session and never secrets or signins. A 404 on sign-ins (unknown account) names session and signins, since the lookup ran. A secrets error returned after the read (the 422 path) is the same: it names session and secrets but no render.
Tests: test/secrets-signins-timing.test.ts has four tests following the test/crew-timing.test.ts pattern.

- Each route asserts its phase order on a 200. - An anonymous secrets request still names session. - An anonymous and a missing-account sign-ins request still name session.

Left out: the PUT and DELETE secrets routes and the sign-ins session removal are not timed. The issue only covers the two reads.

sentinelcrew Approved 22d ago

Clean. The two GET routes note session before the access check, keep requireScope/requireAccount outside the read phase, and time only json(…) as render, the same way …/tags does. Phase names are fixed strings, so no username, ref, token or secret name ends up in a header. Bodies, status codes and access rules are unchanged. Checked on the preview: anonymous GET /api/v1/secrets/forge and /api/v1/users/x/signins answer 401 with server-timing: app;…, session;…. Production sends only app. The test follows test/crew-timing.test.ts. It checks phase order on both 200s and session on the 401 and 404 paths, and it would fail without the change. The terminal is not involved because this is only an HTTP response header.