Repository navigation
Description
Six behaviours of accounts, sessions, the instance setting and the sign-in exemptions that no spec, ADR or issue describes. None contradicts a source; each needs a ruling on whether it is a rule, a bug, or an accepted limit. Found while writing the tenancy and accounts area doc (spec 027 slice C, #106), which states each marked "no spec source". No behaviour changes in that pull request.
1. A session never expires
Authentication#start_new_session_for (app/controllers/concerns/authentication.rb:41-44) sets cookies.signed.permanent[:session_id] (Rails' permanent is 20 years), and nothing ends a sessions row except sign-out, a password change by an admin or the operator command, deleting the user, or a restore (app/models/session.rb, User#end_sessions!). There is no idle timeout, no absolute lifetime and no sweep (config/recurring.yml has none). Spec 004 FR-2 says only that signing out ends the current session, and NFR Security that the cookie is HTTP-only and same-site.
Question: is a session meant to last until it is ended, or should it expire (idle, absolute, or both) and be swept? A lost phone or a shared browser keeps its access until an admin resets the password.
2. sessions.ip_address and sessions.user_agent are stored and read by nothing
start_new_session_for writes request.remote_ip and request.user_agent into every session (authentication.rb:41). No page, task or log line reads them, and no spec says to keep them (the code is in the Rails 8 authentication generator's shape). They are personal data about a collector, held for as long as the session lives (see 1).
Question: keep them for a "your sessions" list later, or stop storing them?
3. Nothing makes instance_settings a single row
InstanceSetting.current is first || create! (app/models/instance_setting.rb:3). The table has no constraint tying it to one row; a code comment calls it "a single row" (spec 004 FR-4 says "an instance-wide sign-up setting"). Two requests that both find no row can each insert one; first then reads whichever has the lowest id. In practice the sign-up and command paths create it inside a write transaction, and an admin's first read is the only other writer, so the window is small.
Question: accept it, or make current create the row in a migration (and guard it with a unique index or a check)?
4. The sign-in rate limit's counters live in the cache
SessionsController (app/controllers/sessions_controller.rb:4-7) stores its 10-per-3-minutes counters in Rails.application.config.x.sign_in_rate_limit_store || Rails.cache; production's cache is Solid Cache (config/environments/production.rb:47), so a cache clear or a cache-database restore resets every counter, and the counters share Solid Cache's size and eviction limits. Spec 004 AC-4.8 states the limit, its key and its 429, not where the counts are kept.
Question: is the cache the right home (and the reset on a cache clear accepted), or should the counts survive it?
5. A sign-up that leaves out the confirmation field isn't checked for a match
Registration permits password_confirmation (app/models/registration.rb:5) and has_secure_password validates the confirmation only when the attribute is present (not nil). The sign-up form always sends it, so a browser can't skip the check, but a request without the field creates the account with the password alone. Spec 004 AC-4.1 requires "a matching confirmation" and AC-4.3 refuses "a confirmation that doesn't match"; neither says what a missing one does.
Question: require the field (a blank or absent confirmation is refused), or accept that only a submitted, differing confirmation is refused?
6. Scanner measurement mode answers only local requests
MeasurementMode#require_local_request (app/controllers/concerns/measurement_mode.rb:17-19) makes Scanner::Measurements::PhotosController, ReplaysController and StripsController answer 404 unless request.local?. These controllers also skip the sign-in requirement (allow_unauthenticated_access), because a local headless browser drives them. Spec 007 AC-5.1 and FR-6 make every measurement route 404 when measurement mode is off (its default outside development) and say the replay is headless; none says the requests must come from the machine itself.
Question: is "local requests only, in addition to measurement mode being on" the rule (then spec 007 or the scanner area doc says so), or only a belt-and-braces check?
🤖 Generated with Claude Code
Activity
[-]Accounts and sessions: four behaviours no spec describes[/-][+]Accounts and sessions: five behaviours no spec describes[/+][-]Accounts and sessions: five behaviours no spec describes[/-][+]Accounts and sessions: six behaviours no spec describes[/+]docs(architecture): add the tenancy and accounts area doc (#106)
docs(architecture): fix the implementation review's findings (#106)
plainprogrammer commentedon Nov 9, 2026
Rulings (maintainer, 2026-11-09), all as recommended:
ip_addressanduser_agenton sessions, and clear the stored values. Ships with 1.Follow-up issues will be linked here.
plainprogrammer commentedon Nov 9, 2026
Follow-ups: #228 (rulings 1 and 2, sessions expire and stop storing IP and user agent), #229 (ruling 3, one instance-settings row), #230 (rulings 5 and 4, sign-up confirmation and the rate-limit store), #231 (ruling 6, with #219's rulings in the scanner doc). Close this issue when they have merged.
plainprogrammer commentedon Nov 9, 2026
All six rulings are delivered: #228 (PR #240), #229 (PR #234), #230 (PR #232) and #231 (PR #236), all merged.