Commit Graph
458 Commits
Author SHA1 Message Date
Jeremy Daer d7589951cf Check the trusted CA certificates at build time
ruby:4.0.7-slim already ships ca-certificates 20250419+deb13u1, which removes
emSign ECC Root CA - G3 and AAA Certificate Services. Fail the build if either
root comes back in the store Ruby's OpenSSL loads.
2026-10-21 14:02:55 -07:00
Sam RubyandClaude Opus 5.5 c8f74a0b0e Compute a page's script sources once per importmap and host
Every page parsed the importmap and joined each of its URLs against the
request's base URL, about a millisecond of CPU a page, though the answer
changes only with the importmap and the host. Keep the last answer, keyed
by both. One entry, so a request with a made-up Host header can't grow it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 21:06:01 +09:00
Sam RubyandClaude Opus 5.5 8d4a5264cf Raise the TLS alert from a subclass instead of extending one connection
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 21:04:16 +09:00
Jeremy Daer ce7b5d4ea0 Check a cable connection's session while it's open (#376)
* Check a cable connection's session while it's open

Signing out or changing the password ends sessions and then disconnects
their connections. A connection that found its session just before isn't
subscribed to its disconnects yet, so it missed that and stayed open.

Each connection now looks its session up every minute while it's open, and
closes once the session is gone, asking the browser to reconnect so one
holding a current session comes back. That also covers any other way a
connection misses the disconnect, such as a subscription that takes long to
register, and a lookup that fails is simply tried again.

* Check the session on the server's heartbeat rather than a timer of its own

A connection's own timer could outlive it: closing can run before opening finishes, or a channel's teardown can fail before the connection's. The heartbeat only reaches connections that are open, so nothing is left to stop.

* Skip the session check on a rejected connection

A rejected connection stays registered until its socket closes, and has no session to check, so its heartbeat mustn't fail and stop the heartbeat reaching the connections after it.
2026-10-20 20:42:54 -07:00
Jeremy Daer a10211fb8a Merge pull request #375 from basecamp/refuse-session-for-changed-password
Refuse a session for a password that changed after it was checked
2026-10-20 10:17:29 -07:00
Jeremy Daer a476cbfa24 Sign the browser out when its new session is refused after a password change
If the password changes again between this browser's change and its new session, that session is refused, and the browser is sent to sign in instead of an error.
2026-10-20 08:57:40 -07:00
Jeremy Daer c3faccb0f4 Read the password digest past the query cache
A digest cached earlier in the request would let the check pass on a password that has since changed.
2026-10-20 08:47:47 -07:00
Jeremy Daer 19319a1cff Refuse a session for a password that changed after it was checked
A password change ends every session, but a sign-in or sign-in link checked
against the old password just before the change could still save its session
just after, and that session outlived the change.

The session now checks, as it's saved, that the user's password is still the
one the request loaded. The save's transaction holds SQLite's write lock from
the start, so a change commits either before the check, which fails it, or
after the insert, and ends this session with the others. Sessions start
through the user's association so the check sees the user that was
authenticated, and a refused sign-in is answered like a wrong password.
2026-10-20 08:37:07 -07:00
Jeremy Daer ac073010c2 Merge pull request #372 from basecamp/password-check-on-digest-change
Check the current password whenever the saved password changes
2026-10-19 16:04:53 -07:00
Jeremy Daer 4a580302a5 Check the current password whenever the saved password changes
The profile asked for the current password when the request's password
field was filled in. The check now follows the save itself: the model
checks the challenge whenever the password digest is about to change,
and the profile always hands it the challenge. Editing a name, avatar
or email without a new password still needs none, and scripts that
reset a password, which set no challenge, are unaffected.

Only a string counts as a challenge, and the rate limit counts every
non-empty one, so what's limited is exactly what gets checked.
2026-10-19 15:05:19 -07:00
Jeremy Daer af4dd9fd59 Merge pull request #371 from basecamp/sign-out-elsewhere-on-password-change
Sign out the account's other sessions when its password changes
2026-10-19 14:08:53 -07:00
Jeremy Daer aa93269ceb Look the endpoint up past the query cache, and keep a subscription a race refused
The lookup under the write lock repeated the one made before
validation, so the query cache could answer it with that earlier miss
even after another connection had registered the endpoint. It runs
uncached now.

Enabling notifications unsubscribed the browser on any refusal, so a
session signed out between authenticating and saving lost the browser's
subscription too, and the next session had nothing to post. Only a
refused endpoint unsubscribes it now.
2026-10-19 13:34:34 -07:00
Jeremy Daer 3786ee5244 Post the browser's push subscription from every page, and register it once
Only room pages posted the subscription again, so a browser that
changed its password and closed the profile page went without
notifications until it next opened a room. A small controller in the
layout does it now, still once per session, and the bell is back to how
it was.

Two tabs posting at once after a password change could both miss the
row and insert two, so the endpoint is looked up again under the same
write lock as the session check.
2026-10-19 13:15:10 -07:00
Jeremy Daer 1af62a5bff Remove the fake push subscription after its system test
The browser is shared across system tests, and a script added to every
new document stays until it's removed, so later tests would have seen
the granted permission and the fake subscription too.
2026-10-19 12:55:14 -07:00
Jeremy Daer 6cab924097 Remove push subscriptions on a password change, and post them again
A push subscription belongs to the user, not a session, so a device
signed out by a password change kept getting notifications. Changing
the password now removes every subscription along with the sessions,
in the same transaction.

A browser still signed in keeps its own subscription in its service
worker, so the notifications controller posts it once per session: the
bell names the session by a digest of its id, never its token, and the
browser remembers the last one it posted for. That post never
unsubscribes the browser when the server refuses it.

A request that authenticated just before the change could put a
subscription back after it. The create action now checks the session
still exists and saves in one immediate transaction, after validating
the endpoint outside it.
2026-10-19 12:39:03 -07:00
Jeremy Daer 44a92ff6c6 Ask for the current password to change it
Changing the password now ends every other session, so whoever holds
one could shut the owner out of their own account with a new password.
The profile asks for the current password whenever the password
changes; without it, or with a wrong one, nothing is saved and the page
says why. Other profile edits don't need it, and the admin
reset-password script still works without it.

Attempts are rate limited like signing in, 10 in 3 minutes per address,
so the current password can't be guessed here instead.
2026-10-19 12:17:14 -07:00
Jeremy Daer 160ba295b5 End sign-in links made before a password change
A transfer link signed only the user id, so one copied before the
password changed still started a session for up to four hours after.
Bind it to the password salt with generates_token_for, so a new
password ends those links too. Links made before this upgrade stop
working; they last four hours at most.
2026-10-19 11:36:16 -07:00
Jeremy Daer e158388f3d Close live connections on the transaction that changed the password
An after_commit condition reads the record's last save, so another save
of the user in the same transaction skipped the disconnect even though
the sessions were gone. The callback that deletes them now registers
the disconnect on the transaction itself, which also leaves a rolled
back change alone. A password changed along with the avatar is covered
too: the attachment's touch doesn't hide the change.
2026-10-19 10:57:14 -07:00
Jeremy Daer b01bb7a0a7 Give the changing browser a new session rather than keeping its own
Keeping the session the password was changed from also kept any copy of
its cookie working, and a copied cookie is one of the ways someone else
gets into an account. Every session ends now, and the profile page
signs the browser that changed the password back in with a new one.
2026-10-19 10:45:34 -07:00
Jeremy Daer c6e0105491 Close live connections from the model on any password change
The sessions end wherever the password changes, but only the profile
page closed the live connections, so a change from a console left a
signed-out device receiving messages until it dropped. Close them after
commit from User, logging rather than failing if the realtime service
can't be reached, since the password is saved by then.
2026-10-19 10:33:46 -07:00
Jeremy Daer c78c3fd7ef Sign out the account's other sessions when its password changes
Changing the password is how someone shuts out whoever else is signed
in to their account, but every session opened with the old password
stayed valid. Now the others end and their live connections close, as
on sign-out. The session the password was changed from stays signed in.

Changing the password outside a request, as from a console, ends every
session.
2026-10-19 10:28:06 -07:00
Jeremy Daer 11d3fb9ce0 Merge pull request #365 from basecamp/enforce-script-policy
Enforce a script Content-Security-Policy
2026-10-15 19:20:35 -07:00
Jeremy Daer 2ba940c683 Allow an asset host only up to its own asset directory
An asset host's path can contain the asset directory too, as in
https://cdn.example.com/assets, and cutting each URL at the first one would
allow everything the host serves under it. Cut at the last one instead.
2026-10-15 19:13:26 -07:00
Jeremy Daer 0a494a10cb Leave the scheme off this origin's script sources
Behind a proxy that terminates TLS without X-Forwarded-Proto, Rails sees
http while the page is https, and an http source on a non-default port
doesn't match. A source without a scheme takes the page's own.
2026-10-15 19:01:12 -07:00
Jeremy Daer 2be2227f40 Allow scripts only from the asset directory, not the whole origin
Uploads are served from this origin too, so script-src 'self' would run one
that a cache still holds under a script type from before the upload guard.
Application pages now allow each importmap URL up to its asset directory
(relative root and asset host included), the inline importmap by hash, and
the service worker by worker-src.
2026-10-15 19:01:12 -07:00
Jeremy Daer 2a449401ec Leave a request without a policy when the policy is turned off
With DISABLE_CONTENT_SECURITY_POLICY=true there's no application policy,
and the hash callback started an empty one, so pages sent an empty
Content-Security-Policy header. Skip the callback then.
2026-10-15 19:01:12 -07:00
Jeremy Daer 9c37f41ff6 Keep a multi-range upload response's multipart type
A request for several byte ranges gets a 206 multipart/byteranges whose
boundary the client needs to split the body. Rewriting it to
octet-stream lost the boundary. No browser runs that type as a script,
so it passes through.
2026-10-15 19:01:12 -07:00
Jeremy Daer d713f2631d Say how long a pre-upgrade cached upload can last
Disk URLs are cached for an hour, but blob proxy URLs are cached
publicly and as immutable for a hundred years, so a copy taken that way
before the upgrade doesn't expire.
2026-10-15 19:01:12 -07:00
Jeremy Daer 562888251e Enforce a script policy that allows the importmap by hash
script-src 'self', object-src 'none' and base-uri 'self', enforced. The
two inline scripts the importmap helper emits are allowed by their
SHA-256, and the origins its URLs point to (a configured asset host, in
any form Rails accepts) are allowed by name. There is no per-request
nonce, so cached pages stay valid.

DISABLE_CONTENT_SECURITY_POLICY=true leaves the policy off, for an
install whose proxy or customization injects scripts.
2026-10-15 19:01:12 -07:00
Jeremy Daer cad2d7211d Serve uploads that aren't media as downloads
Uploaded files come from this origin, so one served with a JavaScript
type could run as one of the app's own scripts. Browsers accept sixteen
spellings of that type, and Disk URLs sign the type they were minted
with, so refusing a list of types at upload can't cover links already
out there. Instead, a middleware rewrites the type of anything Active
Storage reads out that isn't plainly media, a PDF or plain text to
application/octet-stream, whatever the URL says.
2026-10-15 19:01:12 -07:00
Jeremy Daer 821ff92dda Merge pull request #364 from basecamp/autolink-text-only
Link URLs only in a message's text
2026-10-15 18:59:57 -07:00
Jeremy Daer 9b271d0e19 Hand auto_link text escaped the way the serializer writes it
That's exactly what it was given before, with quotes and apostrophes
left as they are, rather than relying on its own sanitize pass to undo
h's &#39;. Covers an email address with an apostrophe.
2026-10-15 18:42:06 -07:00
Jeremy Daer 4e29c6e32b Keep a code block's leading newline when linking a message's text
A <pre> whose text starts with a newline needs one more written out to
survive being parsed again, as the html5 serializer does with
preserve_newline. Without it, linking text dropped one more of a code
block's leading blank lines than rendering did before.
2026-10-15 18:42:06 -07:00
Jeremy Daer 6052b123d5 Quote a mention as text when replying
Replying swaps each mention in the quoted copy for its text. Assigning
that text to outerHTML parsed it as html; replace the mention with a
text node instead.
2026-10-15 18:42:06 -07:00
Jeremy Daer 7ea05e18f8 Link URLs only in a message's text
auto_link finds URLs and email addresses with regular expressions over
the rendered html, which can't tell an attribute value from text, so it
could put a link inside an attribute. Hand it each text node on its own,
escaped, after the final sanitize, so links only ever land between tags.

Bump the presentation cache versions so nothing rendered the old way is
served again.
2026-10-15 18:42:06 -07:00
Jeremy Daer 1ac6dfde84 Merge pull request #366 from basecamp/bump-brakeman
Update Brakeman to 8.1.0
2026-10-15 18:41:55 -07:00
Jeremy Daer b2140cf2aa Update Brakeman to 8.1.0 2026-10-15 18:33:17 -07:00
Sam RubyandClaude Opus 5.5 edbc779f4d Test the pooled-connection class check on its own, and name the class
The rejection cases built plain Net::HTTP connections that were also
unpinned or proxied, so they failed the guard either way. They're built
as WebPush::Connections::HTTP now, so each fails only for its stated
reason, and a pinned, direct, TLS plain Net::HTTP is refused for being
one. The error says which class is required.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 23:41:45 +02:00
Sam RubyandClaude Opus 5.5 2daa01e26b Open pooled push connections as a Net::HTTP subclass that includes Stages
WebPush::Connections mixed Stages into each new connection with
`http.extend Stages` just before starting it. The connection is built in
one place, WebPush::PersistentRequest, so it can be built from a class
that already includes Stages: the same method lookup, without giving
every pooled connection its own singleton class. Connections#request
now also refuses a connection that isn't one, so a caller that builds a
plain Net::HTTP gets the ArgumentError rather than a connection whose
failures can't be told apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 23:41:45 +02:00
Rosa GutierrezandClaude Opus 5.5 2d4b05a74a Clear no_proxy in the link preview proxy test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 16:45:43 +02:00
Rosa GutierrezandClaude Opus 5.5 a7e9ea53dd Don't let an http_proxy environment variable bypass the pinned address in link previews
Opengraph::Fetch connects to the IP it checked, not to a fresh lookup of
the host. With http_proxy set, Net::HTTP would instead send the hostname
to the proxy, which looks it up again, so the connection could end up
somewhere other than the checked address. Pass nil as the proxy address,
as web push delivery already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 16:45:43 +02:00
GPT on behalf of DHH b52b244f38 Include executable template code in application KLOC 2026-10-09 12:52:12 +02:00
GPT on behalf of DHH 05c5a2c0d7 Count only backend application code in benchmark KLOC 2026-10-08 22:13:01 +02:00
GPT on behalf of DHH 81973c564c Document application KLOC alongside validated benchmarks 2026-10-08 22:07:01 +02:00
GPT on behalf of DHH 5796e5c837 Update verified Rails and Django benchmark figures 2026-10-08 21:08:12 +02:00
GPT on behalf of DHH 19c9e6b6f9 Merge Rails empty-mentions optimization by GPT on behalf of DHH 2026-10-08 21:05:15 +02:00
GPT on behalf of DHH 0aa339d81e Avoid room queries for messages without mentions 2026-10-08 20:46:01 +02:00
GPT on behalf of DHH b042182791 Use final published Rails benchmark results 2026-10-08 15:55:51 +02:00
GPT on behalf of DHH 6944a4a796 Update verified Campfire benchmark comparison 2026-10-08 15:38:48 +02:00
GPT on behalf of DHH 4bcc745f7a Interrupt WAL checkpoint shutdown and retain exclusive lock ownership 2026-10-08 15:28:42 +02:00