Rails main (e3d5c569) moves Campfire's pin forward ten months. Because
Campfire loads the 8.2 framework defaults, the ones added since then take
effect too, among them header-only forgery protection, Herb as the HTML
template engine, strict Accept headers and immediate blob analysis.
Changes it needed:
- Minitest 6 has no minitest/unit; the test helper no longer requires it.
- Rack::Sendfile is no longer in the middleware stack; DebugLocks goes
before ActionDispatch::Executor, as the Rails guide now says.
- Time::DATE_FORMATS is deprecated; :epoch is registered through
ActiveSupport::TimeFormats.
- Channel test subscriptions expose stream_names; streams is private.
- sentry-rails declares its Action Cable handle_open and handle_close
wrappers private, which Rails 8.2 calls from outside the connection, so
no /cable connection succeeded. An initializer makes them public until
getsentry/sentry-ruby#2972 ships (issue #2975).
- Lexxy renders editor content through Rails' editor adapter when Rails
has one, which asks a mention for its editor partial. It is the same
users/mention partial the mention prompt already inserts.
- redis-client moves to 0.30.1: Rails main's Redis cache store, which
production uses, requires 0.28.0 or later, and assets:precompile
(the Docker build) aborted on 0.25.2.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit b4d3880a3d88059dd5b0b884f1f5f1a6f82aa95c)
Rails main compiles HTML templates through Herb under the 8.2 framework
defaults, which Campfire loads. Herb rejects `case` and its first `when`
in a single ERB tag, so the three pwa/ partials failed to compile, and
`herb:check` rejects ERB output in attribute names, which the account
settings' switch used for `checked`. Give `case` its own tag and build
the switch with tag.input; both render the same under Erubi.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 244e77241b)
A bot's reply becomes a message when the status is 200 and the type is text,
and an attachment otherwise, but the attachment branch never looked at the
status. Whenever a bot's endpoint failed, its error page landed in the room as
a file: a proxy's 502 page as attachment.html, a 404 as attachment.text.
Apply the text branch's 200 check to attachments too. The error reply test
answered without a content type, which skipped the attachment branch, so it
now answers with an HTML error page.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVFo3Lt9T8M5NR7KxVvsZ2
Search showed the last 100 matches by created_at, so SQLite collected every
message containing the words and sorted them before keeping a page. A common
word in a large account meant sorting most of its history on every search.
Ordering by the full-text index's rowid, which is the message id, lets SQLite
walk the index from the newest match and stop once the page is full.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVFo3Lt9T8M5NR7KxVvsZ2
install-edge.svg lives in app/assets/images/external/, alongside the
other install icons, but the Edge branch of pwa/_install_instructions
asked for it at the top level. Propshaft raises MissingAssetError, so a
browser the useragent gem reports as Edge got a 500 on the profile page
and anywhere else the partial renders.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`as: "json"` is a @rails/request.js option; plain fetch ignores it and
sends `Accept: */*`, so Autocompletable::UsersController answered with
its HTML format and response.json() threw. The new ping form never
showed any suggestions. Send an explicit Accept header instead.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
link_to passed `action: "soft-keyboard#open"` as a plain option, so it
rendered as an `action` attribute, which Stimulus never reads. The link
in messages/_actions.html.erb already passes it under `data:`; this one
now does too, so tapping "Add a boost" on a touch device opens the
keyboard for the boost form as the menu's link does.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The search box keeps only word characters, but FTS5 still reads AND, OR,
NOT and NEAR in the query as operators, so searching for "AND" or for
"salt AND" raised "fts5: syntax error" and answered with a 500. Quoting
each word makes FTS5 look for it as text, which is how it already treats
the same words in lower case.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuTtwJKhb77Dv7EQqv2C3C
The original room shows its welcome box until it holds more than a page of
messages, and paged? answered that with a COUNT(*) over every message in
the room on each visit. Asking whether there is a row past the first page
answers the same question while reading at most a page of index entries.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuTtwJKhb77Dv7EQqv2C3C
* Preload only uncached messages and reduce rendering overhead
* Keep benchmark summaries without raw JSON results
* Use Ruby benchmark drivers and keep generated results out of the repo
The editor keeps an attachment's content as it finds it, so a hand-written
embed with a url but no href reached the editor with its markup unvalidated,
and a mention saved under Trix's editor carried the generic octet-stream
content type the editor doesn't permit and was dropped on save. Rebuilding
each attachment from its attachable renders the hardened preview partial
and restores the mention content type.
Main strips a javascript: href from a preview node before it's parsed, so
tell legacy Trix attachments apart by their filename instead. escapeHTML
moved to the string helpers, and the unfurling system test now drives
the Lexxy editor.
The companion to #279: the same per-instance `extend` appeared in
test/lib/rails_ext/action_text_attachables_test.rb. `attachable_sgid` is
`to_sgid(expires_in: nil, for: ActionText::Attachable::LOCATOR_NAME).to_s`,
so mint that directly; the minted bytes and the assertion are unchanged.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The invalid-sgid test made one live Room attachable — `rooms(:pets).tap
{ |r| r.extend ActionText::Attachable }` — only to call
`attachable_sgid` on it. That method is `to_sgid(expires_in: nil,
for: ActionText::Attachable::LOCATOR_NAME).to_s`, so the test can mint
the same sgid directly and drop the per-instance extend; the assertion
(a signature that does not verify resolves to MissingAttachable) is
unchanged, and so are the bytes it tampers with.
Link previews fetch a page's OpenGraph title and description, and
Opengraph::Metadata strips tags from both before the values reach the
browser. When a field consists entirely of a markup tag, stripping
leaves it blank, the metadata fails its presence validation, and the
unfurl endpoint returns no content, so no preview is produced.
Add regression tests at the model and controller layers that pin this:
a title or description made only of a markup tag is stripped to blank
and rejected, and the endpoint answers 204. The existing sanitize tests
only cover fields that keep non-blank text after stripping, so this
blank-and-rejected path was previously untested.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
Pasting a link builds the preview by interpolating the unfurled metadata
into an HTML string. The image URL went into src="..." unescaped, so a
page whose og:image carries a double quote closes the attribute early and
everything after it becomes attributes on the preview's img element.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
A domain name ends in a word, which is what keeps it from reading as an
address. "0x7f.0.0.1" carries a dot and a letter, so the previous shape
check let it through while a browser fetched 127.0.0.1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
A browser rewrites the many spellings of an address into one before it
fetches, so "http://2130706433/rooms/1" arrives at 127.0.0.1 while a
comparison here still reads the digits. A preview names a page on the
public internet, so require its host to look like a domain name and leave
the rewriting race alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
Ruby leaves a percent-escape in URI#host, so "https://%77ww.example.com"
read as a different host than the one Campfire answers on while a browser
unescaped it straight back to us. A host that carries an escape, or a
trailing dot, is now measured the way the browser will read it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
A preview belongs to the page it previews, so both URLs point somewhere
else. An absolute URL on our own host passed the scheme and host checks,
and every reader's browser fetched it with their session attached, which
turns a message into a GET request made on the reader's behalf.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
Ruby parses "https:/rooms/1" as an HTTPS URL with no host, and a browser
resolves it against whatever origin Campfire is served from, so the scheme
check alone still let a message body aim the preview at a path here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
A link preview's link and image come from attributes on the message body,
which the composer fills in from the unfurl the server performed. A body
written by hand can put anything in those attributes, and the preview
partial rendered them as they were.
Keep the link and the image only when they parse as absolute http or https
URLs, so nothing in a message body can aim either one at another scheme or
at a path on this Campfire, and render the title and the description as
text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186eyzivcTn6wqjEE4Wnxdt
The bot HTTP API carries the bot key as a URL path segment
(/rooms/:room_id/:bot_key/...). config.filter_parameters redacts query
and form parameters but never path segments, so the key was written
verbatim to the request log (the "Started POST ..." line) and to any
log line echoing the pagination Link header.
Add a log formatter that redacts the bot-key path segment wherever it
appears in a formatted line, and wire it into the production logger.
Action Cable authorizes a Connection once at the WebSocket handshake and
never re-checks it. Destroying the session record refuses future handshakes
and HTTP requests bearing the cookie, but a socket opened before sign out
keeps its handshake-time current_user and keeps authorizing new
subscriptions and delivering frames as the signed-out user.
Reset the user's remote connections when the session is terminated. Clients
tear down and reconnect: the signed-out device carries a destroyed session
and cleared cookie and is rejected at the fresh handshake, while the user's
other devices with still-valid sessions reconnect and stay live. This reuses
the existing reset_remote_connections primitive already used on membership
removal, for the same reason.
Run the disconnect last and best-effort, after the session record and cookie
are already gone, so sign out completes even when the realtime service is
unreachable.
Active Storage mounts its direct-upload write endpoints -- POST
/rails/active_storage/direct_uploads and the disk-service PUT at
/rails/active_storage/disk/:token -- on framework controllers that inherit
from ActiveStorage::BaseController, so they never pass through
ApplicationController's require_authentication. Anyone who can read the
public login page can lift a CSRF token and Rails session cookie, POST to
the metadata endpoint, and receive a signed disk PUT URL without holding a
Campfire session_token.
That is enough to allocate ActiveStorage::Blob rows and persist bytes to
disk anonymously. The blobs stay unattached (no message can be created
without an account) and nothing purges them, so an unauthenticated caller
can grow storage without bound. Because the recommended self-host layout
co-locates uploaded files and the SQLite database on one /rails/storage
volume, that growth eventually makes database writes fail -- blocking login
and messaging until an administrator frees space and purges the blobs.
Campfire uploads attachments through MessagesController as a normal
multipart POST and does not use direct uploads at all, so these endpoints
have no legitimate anonymous caller. Require a valid Campfire session
before the metadata endpoint allocates a blob or the disk endpoint accepts
an upload; both return 401 to anonymous callers. Serving (disk#show,
representations, blob redirects) is unchanged.
* Guard push-subscription endpoints against SSRF
Web push delivery POSTed to the endpoint URL a user supplied when
registering a subscription, with no scheme, host, or private-network
check -- unlike the OpenGraph unfurl path, which already routes through
the shared SSRF address policy (surfguard). Any authenticated user could
register a subscription whose endpoint pointed at an internal address and
have the server fetch it on every chat message: blind SSRF for internal
recon and reachability probing, plus a thread-pool DoS on the delivery
pool.
Validate the endpoint when the subscription is saved: it must be HTTPS,
its host must belong to a known browser push service (allowlist), and it
must resolve to a public IP. On every delivery, re-resolve the host and
pin the connection to that public IP so a later DNS rebind can't redirect
the request to an internal address. If no public IP resolves at delivery
time -- a rebind, or a subscription that predates this validation --
delivery is skipped rather than falling back to re-resolving the raw
host.
Adds model, controller, and delivery-pinning tests, plus a DNS stub
helper for deterministic resolution in tests.
* Route push endpoint guarding through RestrictedHTTP::PrivateNetworkGuard
The endpoint SSRF check already delegated its private-network classification
to surfguard, but called Surfguard.resolve_public_ips directly rather than
through RestrictedHTTP::PrivateNetworkGuard -- the hostname-in, address-out
shim #241 established as the app's single guarded-outbound entry point and
that Opengraph::Fetch resolves through. Route push resolution through the same
guard so once-campfire keeps one place that resolves and classifies outbound
addresses. Behavior is unchanged: the guard raises Violation when a permitted
host resolves only to blocked addresses (previously an empty list -> nil) and
propagates Surfguard::Unresolvable for a host that resolves to nothing; both
map to a nil endpoint IP, which fails validation and skips delivery. The
allowlist, HTTPS/443 constraints, and per-delivery IP pinning are unchanged.
* Address push-SSRF review: defer DNS off enqueue path, disable proxy on pinned path, revalidate on re-registration
- Resolve the guarded endpoint IP lazily inside WebPush::Notification#deliver
(on the bounded delivery worker) instead of eagerly when the notification is
built on the serial enqueue path, so a slow resolver can't stall the push job
before any delivery starts. resolved_endpoint_ip only reads the already-loaded
endpoint attribute, so it is safe off the AR connection.
- Pin the delivery socket with an explicit nil proxy address so http_proxy/
https_proxy can't route the request through a proxy that re-resolves the host
and defeats the ipaddr pin.
- Revalidate an existing subscription on re-registration so a row predating
endpoint validation gets the same 422 as a fresh create instead of being kept
alive by touch.
- Regression tests: resolution deferred to delivery, pin survives proxy env,
legacy invalid row rejected with 422.
* Bound the web-push delivery queue (max_queue, not the ignored queue_size)
Concurrent::ThreadPoolExecutor takes :max_queue; :queue_size was silently
ignored, leaving the delivery backlog unbounded (max_queue: 0). A flood of
valid push subscriptions could accumulate queued deliveries without limit --
more acute now that each delivery task also resolves DNS. Using max_queue: 10000
activates the intended cap; overflow raises RejectedExecutionError under the
default :abort policy, which deliver_later already rescues (push is best-effort,
retried on the next message).
* Trim push-SSRF guard comments to match house style
Apply the review suggestions on the push-subscription SSRF guard: replace
the verbose rationale comments with terse one-liners (or drop them where
the code speaks for itself). No behavior change -- the Surfguard-shim
routing, per-delivery public-IP pin, and bounded delivery queue are
untouched.
* Take the SSRF address policy from surfguard instead of keeping our own copy
Four other apps carried this same classification and the five had drifted into
four different ideas of what "internal" means. It now comes from the surfguard
gem, which is their union. resolve and Violation keep their shapes, so the
opengraph callers are unchanged.
Two verdicts change.
SIIT (::ffff:0:0:0/96) is now recognised. It is the third way an IPv4 address
rides inside an IPv6 one and the only one Ruby has no predicate for --
ipv4_mapped?, ipv4_compat?, private?, loopback? and link_local? are all false
for ::ffff:0:a9fe:a9fe, so it fell through to the IPv6 branch unrecognised and
reached the metadata endpoint. Note the extra group: ::ffff:0:0:0/96 is not the
IPv4-mapped ::ffff:0:0/96 the guard already refused, and the two do not overlap.
The RFC 8215 local-use NAT64 block is now refused whole rather than decoded.
Reading its low 32 bits as an embedded IPv4 is only correct for a /96 Pref64;
the block can host any length from /32 to /96 and the position is not
recoverable from the address alone (RFC 6052 2.2), so the decode reads the
wrong octets. It is never globally routed, so refusing it costs nothing. The
well-known /96 is still decoded and re-checked, so DNS64 for public sites on
IPv6-only hosts keeps working.
Resolution moves from Resolv.getaddress to Resolv.getaddresses, so the guard
sees every address a host answers with rather than only the first.
* Distinguish a DNS lookup failure from a private-IP block in the guard
Advance the surfguard pin so resolve_public_ips raises Unresolvable when a
host resolves to nothing and returns an empty list only when it resolves to a
blocked address. The shim lets Unresolvable propagate as a lookup failure --
matching the old Resolv.getaddress behavior -- and reserves Violation for a
resolved-but-blocked address, so a transient DNS miss is no longer reported as
an SSRF attempt.
Bots can only create. A lifecycle notification — an alert that fires and then
resolves, a deploy that starts and finishes, a backup that runs — therefore has
to post a second message, and the room becomes an append-only log of states
rather than a view of the current one.
Adds PATCH and DELETE inside the existing bot_key scope, routed to
Messages::ByBotsController. The body is read the way create reads it, so
updating a message is the same request shape as posting one.
No new authorization: both actions already run through ensure_can_administer,
and can_administer? grants access only to a record the user created, so a bot
key reaches that bot's own messages and no others. set_room narrows it again by
looking the room up through the bot's own memberships. A leaked bot key gains
what it could already do by posting: write to rooms that bot belongs to.
update answers head :ok rather than the redirect, which meant extracting the
update and its broadcast into update_message — calling super and then head
would double render, since the parent redirects inside the action. destroy
needs no split, because the parent renders implicitly like create does.
- Use jbuilder instead of hashes
- Use resource instead of direct HTTP verbs
Verbs only make sense if you have one or two routes, if there are
multiple that emulate what resource does then it's better to use resource.
- Paginate using link headers
- Cache responses
Sam Ruby
Stanko Krtalić
Marcello Costagliola
David Heinemeier Hansson
Edward Tippett
Rosa Gutierrez
Jeremy Daer
Ronald Lokers