Nothing bounds username length, and an account with a multi-KB username cannot log in at all #184

Open
opened 2026-08-18 02:33:32 +02:00 by clawbot · 2 comments
Collaborator

Fell out of writing the log-bound tests for #176. Deliberately NOT milestoned 1.0.0 — see reachability below.

Nothing validates username length at account creation. The session cookie is written before the user logged in log line, and gorilla/securecookie rejects an encoded value past 4 KB, so an account whose username is a few KB cannot authenticate: the login answers 500, not a validation error. The account exists and is permanently unusable, and the failure names nothing an operator could act on.

Reachability is what keeps it off the milestone: the bootstrap admin is created by the service with a fixed username, and there is no user-creation UI. So today this needs direct database access to produce, and someone with that has better options. It is filed because the absent bound is real, the failure mode is silent and confusing, and it becomes reachable the moment a second user can be created — which the APIKey model and the empty /api/v1 group suggest is intended eventually.

Definition of done

  • Username length is bounded at creation, with a limit derived from what the session cookie can actually carry rather than picked round. Say where the number comes from.
  • The limit is enforced wherever a user can be created, including any future path — a database-level constraint would survive a new handler that forgets to validate, so consider that rather than only a handler check.
  • An over-long username produces a validation error, not a 500.
  • Check whether any other stored value ends up in the session cookie and is similarly unbounded.
  • A test that an over-long username is rejected at creation, and one that an account at the limit can still log in.

Implementation requirements

  • Branch from next, PR based on next, single commit, title ending (closes #N).
  • Do not modify TODO.md (see #112).
  • Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint needs Docker.
  • Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
Fell out of writing the log-bound tests for https://git.eeqj.de/sneak/webhooker/issues/176. Deliberately NOT milestoned 1.0.0 — see reachability below. Nothing validates username length at account creation. The session cookie is written **before** the `user logged in` log line, and `gorilla/securecookie` rejects an encoded value past 4 KB, so an account whose username is a few KB cannot authenticate: the login answers **500**, not a validation error. The account exists and is permanently unusable, and the failure names nothing an operator could act on. Reachability is what keeps it off the milestone: the bootstrap admin is created by the service with a fixed username, and there is no user-creation UI. So today this needs direct database access to produce, and someone with that has better options. It is filed because the absent bound is real, the failure mode is silent and confusing, and it becomes reachable the moment a second user can be created — which the `APIKey` model and the empty `/api/v1` group suggest is intended eventually. ## Definition of done - Username length is bounded at creation, with a limit derived from what the session cookie can actually carry rather than picked round. Say where the number comes from. - The limit is enforced wherever a user can be created, including any future path — a database-level constraint would survive a new handler that forgets to validate, so consider that rather than only a handler check. - An over-long username produces a validation error, not a 500. - Check whether any other stored value ends up in the session cookie and is similarly unbounded. - A test that an over-long username is rejected at creation, and one that an account at the limit can still log in. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112). - Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` needs Docker. - Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
clawbot self-assigned this 2026-08-18 02:33:32 +02:00
Author
Collaborator

Plan. The code the issue names is unchanged on next (4a72413). Username in internal/database/model_user.go has no length bound. Users are created by the first-boot admin setup; webhooker resetpw only changes passwords. There is no user-creation UI.

  • The limit: derive it from what the session cookie can hold, as the issue asks, and state the arithmetic in one comment next to the constant. A plain limit below the derived ceiling is fine, as long as the comment says why.
  • Enforce it in two places:
    • in the code that creates a user, with a validation error rather than a 500;
    • in the database, so a future creation path cannot skip it. Before 1.0, REPO_POLICIES.md says to change the existing schema definition rather than add a migration file. Use whatever mechanism the tree already uses for column constraints.
  • Other session values: check whether anything else stored in the session cookie is unbounded. Say what you found on the PR in one line.
  • Tests:
    • an over-long username is rejected at creation;
    • an account whose username is exactly at the limit can log in;
    • each fails when its check is removed.
  • Admin login security is settled; do not change the login flow beyond what the bound needs.

Model: opus-5-5

Plan. The code the issue names is unchanged on `next` (`4a72413`). `Username` in `internal/database/model_user.go` has no length bound. Users are created by the first-boot admin setup; `webhooker resetpw` only changes passwords. There is no user-creation UI. - **The limit:** derive it from what the session cookie can hold, as the issue asks, and state the arithmetic in one comment next to the constant. A plain limit below the derived ceiling is fine, as long as the comment says why. - **Enforce it in two places:** - in the code that creates a user, with a validation error rather than a 500; - in the database, so a future creation path cannot skip it. Before 1.0, `REPO_POLICIES.md` says to change the existing schema definition rather than add a migration file. Use whatever mechanism the tree already uses for column constraints. - **Other session values:** check whether anything else stored in the session cookie is unbounded. Say what you found on the PR in one line. - **Tests:** - an over-long username is rejected at creation; - an account whose username is exactly at the limit can log in; - each fails when its check is removed. - Admin login security is settled; do not change the login flow beyond what the bound needs. Model: opus-5-5
Author
Collaborator

Built in #338: usernames are limited to 1024 bytes, about half of what the session cookie can carry. The User model rejects a longer one with a validation error, and a check constraint on the users table rejects it for any path that bypasses the model. No other value in the session cookie is unbounded.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/webhooker/pulls/338: usernames are limited to 1024 bytes, about half of what the session cookie can carry. The `User` model rejects a longer one with a validation error, and a check constraint on the `users` table rejects it for any path that bypasses the model. No other value in the session cookie is unbounded. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#184