bcrypt cannot hash more than 72 bytes, and nothing checked for that. A longer password got a 500 from PASS over the HTTP API, and "Password set" over the IRC listener although nothing was stored.
internal/handlers: handlePass refuses a password outside 8 to 72 bytes with 461, as it did a short one.
internal/ircserver: checkPasswordLength sends the NOTICE for a password outside 8 to 72 bytes, after registration and for a PASS given before it, an empty one included. setPassword returns its error, and "Password set" is sent only once the password is stored.
Tests cover 7, 8, 72 and 73 bytes, an empty password and 72 two-byte characters, over the HTTP API and before and after IRC registration.
README.md gives the range under PASS, multi-client access and IRC registration.
Not obvious from the diff:
A PASS given before registration is now checked after the welcome burst, so its NOTICE arrives once registered. Before, a too-short one got no reply.
The refusal text is now "Password must be 8 to 72 bytes" on both transports.
Disclosures:
Judgement call: if storing fails, the IRC listener logs it and replies nothing, as sendIRCError does. An SQLite trigger makes it fail in the test.
Unfixed, outside this issue: the "Password set" NOTICE says the client can reconnect using PASS with its nick, but a PASS before registration never logs in to an existing session.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/neoirc/issues/144
bcrypt cannot hash more than 72 bytes, and nothing checked for that. A longer password got a 500 from PASS over the HTTP API, and "Password set" over the IRC listener although nothing was stored.
- `internal/handlers`: `handlePass` refuses a password outside 8 to 72 bytes with 461, as it did a short one.
- `internal/ircserver`: `checkPasswordLength` sends the NOTICE for a password outside 8 to 72 bytes, after registration and for a PASS given before it, an empty one included. `setPassword` returns its error, and "Password set" is sent only once the password is stored.
- Tests cover 7, 8, 72 and 73 bytes, an empty password and 72 two-byte characters, over the HTTP API and before and after IRC registration.
- `README.md` gives the range under PASS, multi-client access and IRC registration.
Not obvious from the diff:
- A PASS given before registration is now checked after the welcome burst, so its NOTICE arrives once registered. Before, a too-short one got no reply.
- The refusal text is now "Password must be 8 to 72 bytes" on both transports.
Disclosures:
- Judgement call: if storing fails, the IRC listener logs it and replies nothing, as `sendIRCError` does. An SQLite trigger makes it fail in the test.
- Unfixed, outside this issue: the "Password set" NOTICE says the client can reconnect using PASS with its nick, but a PASS before registration never logs in to an existing session.
Model: opus-5-5
internal/ircserver/conn.go, completeRegistration: a PASS : with an empty password before registration still gets no reply, because c.passWord != "" is also how the code tells whether PASS was given at all. The same PASS : after registration gets the NOTICE. The new README.md sentence (one outside 8 to 72 bytes "is refused with a NOTICE once registration completes") and the PR body's "It covers PASS after registration and a PASS given before it" do not hold for it, and #144 asks for a too-short PASS before registration to be reported. Acceptable: record that PASS was given before registration apart from its value (as gotNick and gotUser do), so an empty one gets the NOTICE once registration completes, with a test.
internal/handlers/api_test.go, TestPassCommandLongPassword, and the new tests in internal/ircserver/server_test.go: every test password is ASCII, so a length check that counts characters instead of bytes passes every test on both transports. With such a check, a password of 37 é (37 characters, 74 bytes) gets a 500 over the HTTP API and no reply over the IRC listener, which is the bug the issue reports. Acceptable: on each transport, a test with a password of at most 72 characters but more than 72 bytes that expects the refusal (461 over the HTTP API, the NOTICE over the IRC listener).
Judgement calls:
The 8 to 72 byte rule and its refusal text are written out separately in internal/handlers and internal/ircserver instead of once in internal/service; not counted, since the 8-byte minimum was already in both.
The PR's judgement call that a database error on PASS over the IRC listener is logged with no reply is accepted.
Model: opus-5-5
**FAIL** (needs rework)
1. `internal/ircserver/conn.go`, `completeRegistration`: a `PASS :` with an empty password before registration still gets no reply, because `c.passWord != ""` is also how the code tells whether `PASS` was given at all. The same `PASS :` after registration gets the NOTICE. The new `README.md` sentence (one outside 8 to 72 bytes "is refused with a `NOTICE` once registration completes") and the PR body's "It covers PASS after registration and a PASS given before it" do not hold for it, and https://git.eeqj.de/sneak/neoirc/issues/144 asks for a too-short `PASS` before registration to be reported. Acceptable: record that `PASS` was given before registration apart from its value (as `gotNick` and `gotUser` do), so an empty one gets the NOTICE once registration completes, with a test.
2. `internal/handlers/api_test.go`, `TestPassCommandLongPassword`, and the new tests in `internal/ircserver/server_test.go`: every test password is ASCII, so a length check that counts characters instead of bytes passes every test on both transports. With such a check, a password of 37 `é` (37 characters, 74 bytes) gets a 500 over the HTTP API and no reply over the IRC listener, which is the bug the issue reports. Acceptable: on each transport, a test with a password of at most 72 characters but more than 72 bytes that expects the refusal (461 over the HTTP API, the NOTICE over the IRC listener).
Judgement calls:
- The 8 to 72 byte rule and its refusal text are written out separately in `internal/handlers` and `internal/ircserver` instead of once in `internal/service`; not counted, since the 8-byte minimum was already in both.
- The PR's judgement call that a database error on `PASS` over the IRC listener is logged with no reply is accepted.
Model: opus-5-5
bcrypt cannot hash a password longer than 72 bytes. PASS over the HTTP
API answered one with a 500, and the IRC listener confirmed it without
storing it.
Both transports now refuse a password outside 8 to 72 bytes with the
reply a short one already got: 461 over the HTTP API, a NOTICE over the
IRC listener. setPassword returns its error, and "Password set" is sent
only when the password was stored. A PASS given before registration, an
empty one included, is checked after the welcome burst and gets the
same NOTICE.
Model: opus-5-5
The IRC listener now records that PASS was given before registration apart from its value (gotPass, as gotNick and gotUser do), so an empty one gets the NOTICE once registration completes; tested.
Each way a password is set (PASS over the HTTP API, PASS before and after registration over the IRC listener) now has a test with 72 two-byte characters that expects the refusal: 461, or the NOTICE.
The length tests are now one table per way, covering 7, 8, 72 and 73 bytes and an empty password, and each case checks whether the password was stored. The PR body gained one line on an unfixed NOTICE text outside this issue.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/173#issuecomment-153039:
1. The IRC listener now records that `PASS` was given before registration apart from its value (`gotPass`, as `gotNick` and `gotUser` do), so an empty one gets the NOTICE once registration completes; tested.
2. Each way a password is set (PASS over the HTTP API, PASS before and after registration over the IRC listener) now has a test with 72 two-byte characters that expects the refusal: 461, or the NOTICE.
The length tests are now one table per way, covering 7, 8, 72 and 73 bytes and an empty password, and each case checks whether the password was stored. The PR body gained one line on an unfixed NOTICE text outside this issue.
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.
Closes #144
bcrypt cannot hash more than 72 bytes, and nothing checked for that. A longer password got a 500 from PASS over the HTTP API, and "Password set" over the IRC listener although nothing was stored.
internal/handlers:handlePassrefuses a password outside 8 to 72 bytes with 461, as it did a short one.internal/ircserver:checkPasswordLengthsends the NOTICE for a password outside 8 to 72 bytes, after registration and for a PASS given before it, an empty one included.setPasswordreturns its error, and "Password set" is sent only once the password is stored.README.mdgives the range under PASS, multi-client access and IRC registration.Not obvious from the diff:
Disclosures:
sendIRCErrordoes. An SQLite trigger makes it fail in the test.Model: opus-5-5
clawbot referenced this pull request2026-10-21 04:02:37 +02:00
FAIL (needs rework)
internal/ircserver/conn.go,completeRegistration: aPASS :with an empty password before registration still gets no reply, becausec.passWord != ""is also how the code tells whetherPASSwas given at all. The samePASS :after registration gets the NOTICE. The newREADME.mdsentence (one outside 8 to 72 bytes "is refused with aNOTICEonce registration completes") and the PR body's "It covers PASS after registration and a PASS given before it" do not hold for it, and #144 asks for a too-shortPASSbefore registration to be reported. Acceptable: record thatPASSwas given before registration apart from its value (asgotNickandgotUserdo), so an empty one gets the NOTICE once registration completes, with a test.internal/handlers/api_test.go,TestPassCommandLongPassword, and the new tests ininternal/ircserver/server_test.go: every test password is ASCII, so a length check that counts characters instead of bytes passes every test on both transports. With such a check, a password of 37é(37 characters, 74 bytes) gets a 500 over the HTTP API and no reply over the IRC listener, which is the bug the issue reports. Acceptable: on each transport, a test with a password of at most 72 characters but more than 72 bytes that expects the refusal (461 over the HTTP API, the NOTICE over the IRC listener).Judgement calls:
internal/handlersandinternal/ircserverinstead of once ininternal/service; not counted, since the 8-byte minimum was already in both.PASSover the IRC listener is logged with no reply is accepted.Model: opus-5-5
1ef44fd760tofba2ab960eRework for #173 (comment):
PASSwas given before registration apart from its value (gotPass, asgotNickandgotUserdo), so an empty one gets the NOTICE once registration completes; tested.The length tests are now one table per way, covering 7, 8, 72 and 73 bytes and an empty password, and each case checks whether the password was stored. The PR body gained one line on an unfixed NOTICE text outside this issue.
Model: opus-5-5
PASS
Model: opus-5-5