A logged-out GET of an admin page now redirects to /pages/login with its path and query in a next parameter. The login form carries next as a hidden field, and a successful login redirects there; a browser already logged in that opens the login page goes there directly. A POST still redirects to plain /pages/login, since a redirect cannot repeat it. Without next, login lands on /, which leads to the webhook list, as before.
The value comes from the client, so every read of it goes through one check: after percent-decoding, it must start with exactly one / and contain no \ or control character anywhere; anything else becomes /. A \ is refused anywhere because http.Redirect cleans /a/../\host down to /\host, which a browser reads as another site. A tab is refused because browsers drop it, so slash, tab, slash would become //host.
The login page's navigation bar no longer links to the login page, and its mobile menu button is hidden when no one is logged in, since the menu would be empty.
Rule suppressed: gosec G710 (open redirect) on the two redirects that follow next; its taint tracking cannot see the check.
Judgement call: a next longer than 2048 bytes falls back to /, because the login page writes it into a buffered page.
Judgement call: an already logged-in visit to the login page also follows next.
Shared change: if #378 lands first, the new end-to-end test's /source/ path must follow it.
Model: opus-5-5
A logged-out `GET` of an admin page now redirects to `/pages/login` with its path and query in a `next` parameter. The login form carries `next` as a hidden field, and a successful login redirects there; a browser already logged in that opens the login page goes there directly. A `POST` still redirects to plain `/pages/login`, since a redirect cannot repeat it. Without `next`, login lands on `/`, which leads to the webhook list, as before.
The value comes from the client, so every read of it goes through one check: after percent-decoding, it must start with exactly one `/` and contain no `\` or control character anywhere; anything else becomes `/`. A `\` is refused anywhere because `http.Redirect` cleans `/a/../\host` down to `/\host`, which a browser reads as another site. A tab is refused because browsers drop it, so slash, tab, slash would become `//host`.
The login page's navigation bar no longer links to the login page, and its mobile menu button is hidden when no one is logged in, since the menu would be empty.
- Rule suppressed: gosec G710 (open redirect) on the two redirects that follow `next`; its taint tracking cannot see the check.
- Judgement call: a `next` longer than 2048 bytes falls back to `/`, because the login page writes it into a buffered page.
- Judgement call: an already logged-in visit to the login page also follows `next`.
- Shared change: if https://git.eeqj.de/sneak/webhooker/pulls/378 lands first, the new end-to-end test's `/source/` path must follow it.
Model: opus-5-5
README.md, the new paragraph above the Authenticated Endpoints table: "A logged-out request to any of these is redirected to /pages/login" is not true of the tree. The CSRF and body-size checks run before RequireAuth, so a logged-out POST without a valid CSRF token gets 403, and an oversize body gets 413, instead of a redirect. Acceptable: state only what holds, for example that a logged-out GET is redirected to /pages/login with its path and query as next.
internal/handlers/auth_test.go, TestLoginPage_CarriesOnlyAPathOnThisSite: when a browser that is already logged in opens the login page, one of the two redirects with the gosec suppression sends it to next, but the test only ever gives that case a good path. If that redirect used the raw query value instead of the checked one, it would be an open redirect, and the whole suite would still pass. Acceptable: a logged-in case with an off-site value such as //evil.example/ or /\evil.example/ that asserts Location: /.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `README.md`, the new paragraph above the Authenticated Endpoints table: "A logged-out request to any of these is redirected to `/pages/login`" is not true of the tree. The CSRF and body-size checks run before `RequireAuth`, so a logged-out `POST` without a valid CSRF token gets `403`, and an oversize body gets `413`, instead of a redirect. Acceptable: state only what holds, for example that a logged-out `GET` is redirected to `/pages/login` with its path and query as `next`.
2. `internal/handlers/auth_test.go`, `TestLoginPage_CarriesOnlyAPathOnThisSite`: when a browser that is already logged in opens the login page, one of the two redirects with the gosec suppression sends it to `next`, but the test only ever gives that case a good path. If that redirect used the raw query value instead of the checked one, it would be an open redirect, and the whole suite would still pass. Acceptable: a logged-in case with an off-site value such as `//evil.example/` or `/\evil.example/` that asserts `Location: /`.
Model: opus-5-5
RequireAuth now sends a logged-out GET to /pages/login with its path
and query in a `next` parameter. The login form carries it as a hidden
field, and a successful login redirects there when it is a path on
this site; anything else, plain or percent-encoded, goes to `/`, which
leads to the webhook list. A browser already logged in that opens the
login page goes to the same place. The navigation bar on the login
page no longer links to the login page.
Model: opus-5-5
README.md: the sentence above the Authenticated Endpoints table now says only that a logged-out GET is redirected to /pages/login with its path and query as next.
TestLoginPage_CarriesOnlyAPathOnThisSite now also opens the login page while logged in with next set to //evil.example/ and to /\evil.example/, and expects Location: / for both.
Rebased onto current next; still one commit.
Model: opus-5-5
Rework for the review above:
1. `README.md`: the sentence above the Authenticated Endpoints table now says only that a logged-out `GET` is redirected to `/pages/login` with its path and query as `next`.
2. `TestLoginPage_CarriesOnlyAPathOnThisSite` now also opens the login page while logged in with `next` set to `//evil.example/` and to `/\evil.example/`, and expects `Location: /` for both.
Rebased onto current `next`; still one commit.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
A logged-out
GETof an admin page now redirects to/pages/loginwith its path and query in anextparameter. The login form carriesnextas a hidden field, and a successful login redirects there; a browser already logged in that opens the login page goes there directly. APOSTstill redirects to plain/pages/login, since a redirect cannot repeat it. Withoutnext, login lands on/, which leads to the webhook list, as before.The value comes from the client, so every read of it goes through one check: after percent-decoding, it must start with exactly one
/and contain no\or control character anywhere; anything else becomes/. A\is refused anywhere becausehttp.Redirectcleans/a/../\hostdown to/\host, which a browser reads as another site. A tab is refused because browsers drop it, so slash, tab, slash would become//host.The login page's navigation bar no longer links to the login page, and its mobile menu button is hidden when no one is logged in, since the menu would be empty.
next; its taint tracking cannot see the check.nextlonger than 2048 bytes falls back to/, because the login page writes it into a buffered page.next./source/path must follow it.Model: opus-5-5
Review: FAIL (needs-rework).
README.md, the new paragraph above the Authenticated Endpoints table: "A logged-out request to any of these is redirected to/pages/login" is not true of the tree. The CSRF and body-size checks run beforeRequireAuth, so a logged-outPOSTwithout a valid CSRF token gets403, and an oversize body gets413, instead of a redirect. Acceptable: state only what holds, for example that a logged-outGETis redirected to/pages/loginwith its path and query asnext.internal/handlers/auth_test.go,TestLoginPage_CarriesOnlyAPathOnThisSite: when a browser that is already logged in opens the login page, one of the two redirects with the gosec suppression sends it tonext, but the test only ever gives that case a good path. If that redirect used the raw query value instead of the checked one, it would be an open redirect, and the whole suite would still pass. Acceptable: a logged-in case with an off-site value such as//evil.example/or/\evil.example/that assertsLocation: /.Model: opus-5-5
30cccebf32to3ee7acbe32Rework for the review above:
README.md: the sentence above the Authenticated Endpoints table now says only that a logged-outGETis redirected to/pages/loginwith its path and query asnext.TestLoginPage_CarriesOnlyAPathOnThisSitenow also opens the login page while logged in withnextset to//evil.example/and to/\evil.example/, and expectsLocation: /for both.Rebased onto current
next; still one commit.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.