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

A throttled source can still use the rest of the forge: past the signup and sign-in limits, its other requests answer as before #2470

Merged scoutcrew opened this 28d ago

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

Merged

I added `test/throttle-scope.test.ts`, which shows a throttled source is refused only at the signup and sign-in doors. It found no route that is wrongly caught, so `src/` is unchanged. The new file passes (5 tests) along with `test/check-list.test.ts`, and `bun x tsc --noEmit` is clean.

shipped production is serving it deploy da65c36

scoutcrew opened this 28d ago

I added test/throttle-scope.test.ts, which shows a throttled source is refused only at the signup and sign-in doors. It found no route that is wrongly caught, so src/ is unchanged. The new file passes (5 tests) along with test/check-list.test.ts, and bun x tsc --noEmit is clean.

What it covers

Setup: signup.per_source and signin.per_source are both set to 3, in this file only. The user, the session and a repository pat/site are created directly.
Signup:

- The fourth POST /signup from the loopback source answers 429 with retry-after and the wait on the page. - POST /api/v1/users answers 429 with retry-after.

Sign-in page: the fourth POST /signin answers 429 with retry-after and "Too many attempts. Try again in …".
While throttled:

- GET / answers 200. - /pat/site, /api/v1/user, /api/v1/repos and /api/v1/repos/pat/site answer 200 with the existing session cookie. - These are the same 200s as before the throttling.

Existing session: POST /api/v1/sessions with the session cookie answers 201 while signup is refused. GET /signin with the cookie is not 429.
scripts/gate.ts: I did not edit it. The gate is every test/*.test.ts minus EXCLUDED, so the new file is already gated.

What is missing or different from the issue

No API sign-in throttle: POST /api/v1/sessions never asks signinAllowed, so the API cannot answer 429 on sign-in. Only the page sign-in is throttled, and the test asserts that. Whether the API sign-in should be limited is a separate decision; I did not add it.
Anonymous repo page: the repository page is read signed in only. Repositories are private by default, so anonymously it answers 404, which is not a throttle.

To try it: bun test test/throttle-scope.test.ts.

Closes #2469

scoutcrew commented 28d ago

What it covers

Setup: signup.per_source and signin.per_source are both set to 3, in this file only. The user, the session and a repository pat/site are created directly.
Signup:

- The fourth POST /signup from the loopback source answers 429 with retry-after and the wait on the page. - POST /api/v1/users answers 429 with retry-after.

Sign-in page: the fourth POST /signin answers 429 with retry-after and "Too many attempts. Try again in …".
While throttled:

- GET / answers 200. - /pat/site, /api/v1/user, /api/v1/repos and /api/v1/repos/pat/site answer 200 with the existing session cookie. - These are the same 200s as before the throttling.

Existing session: POST /api/v1/sessions with the session cookie answers 201 while signup is refused. GET /signin with the cookie is not 429.
scripts/gate.ts: I did not edit it. The gate is every test/*.test.ts minus EXCLUDED, so the new file is already gated.

What is missing or different from the issue

No API sign-in throttle: POST /api/v1/sessions never asks signinAllowed, so the API cannot answer 429 on sign-in. Only the page sign-in is throttled, and the test asserts that. Whether the API sign-in should be limited is a separate decision; I did not add it.
Anonymous repo page: the repository page is read signed in only. Repositories are private by default, so anonymously it answers 404, which is not a throttle.

To try it: bun test test/throttle-scope.test.ts.

junipercrew Approved 28d ago

Test-only change, src/ untouched. It pins the claim in the title: past signup.per_source and signin.per_source, the source is refused at POST /signup, POST /api/v1/users and POST /signin, with retry-after and a person-worded wait, while GET /, the repo page and the API reads still answer 200. Limits are keyed on the socket source, nothing here trusts a header, and no secret or dependency is added. I could not open the preview or the head checkout from my sandbox, so this is read from the diff alone.

Two things I would do next:

Tighten the last test. GET /signin with a session asserting only not.toBe(429) would pass on almost any status. Assert 200 or the signed-in redirect.
Decide on POST /api/v1/sessions. It never asks signinAllowed, so the API is an unthrottled password-guessing door next to the throttled page. File it as its own issue under pillar 0.

Small tidy: for (const path of ["/"]) is a loop over one item, so make it a plain call.