`BaseStore#download` and `download!` previously returned open `File`
objects via `get_from_cache` (which called `File.open`). Nearly all
callers (14 of 16) only needed the `.path` and never closed the handle,
leaking file descriptors. Under load this can exhaust FDs and crash
with `Errno::EMFILE`.
This commit changes `download` and `download!` to return file path
strings instead of `File` objects. Since the cached file at
`tmp/download_cache/` persists on disk, callers that need to read
content can simply use `File.read(path)`.
The public API is simplified to two methods:
- `download`: safe, rescues all errors and returns `nil` (absorbs
the former `download_safe` behavior)
- `download!`: raises `DownloadError` on failure
A deprecated `download_safe` alias is kept for plugin compatibility.
The block/yield pattern is removed entirely since it's no longer needed.
There are no file handles to manage. All call sites are updated to
use the path directly, dropping `{ |f| f.path }` blocks. The two
callers that actually read file content are updated:
- `static_controller.rb` → `File.read(path)`
- `digest_rag_upload.rb` → `File.open(path)` (streams content)
Previously, the HTML variant of email notifications replaced the entire
poll with a "Click to view the poll" link, while the plain-text variant
included all the poll options. This meant the HTML email contained less
information than its plain-text counterpart.
Now the `reduce_cooked` callback preserves the poll title and options
list, strips the interactive parts (vote counts, buttons) and
`data-poll-option-id` attributes, and still appends the link to vote.
This works for all poll types (regular, multiple, number, ranked_choice)
since they all share the same `.poll-container > ul/ol > li` structure.
https://meta.discourse.org/t/393728
In development mode, AnonymousCache is disabled by default, so we hadn't
noticed this. And we haven't started using pitchfork in test mode yet.
Pitchfork adds the `Rack::Lint` middleware to the top of the stack in
non-production environments:
https://github.com/Shopify/pitchfork/blob/c95f7a6e/lib/pitchfork.rb#L100.
This causes a failure on our non-rack-spec-compliant call to
`env[Rack::RACK_INPUT].size`.
Pitchfork implements the `#size` method, but marks it as for
backward-compatibility for Rack < 1.2:
https://github.com/Shopify/pitchfork/blob/c95f7a6e/lib/pitchfork/tee_input.rb#L60-L71.
The implementation reads the entire string from the stream and checks
the length. In our case, we can just try reading one byte to determine
whether any body has been received. This is compliant with the modern
Rack spec, and will pass the `Rack::Lint` middleware checks.
RSS polling from one Discourse site to another was failing when using
API key authentication via query parameters (e.g., /c/category/123.rss
?api_key=XXX&api_username=YYY). This affected private or login_required
sites where RSS feeds require authentication.
The issue occurred because FinalDestination defaults to HEAD requests
for URL resolution. Discourse's authentication system only allows API key
authentication via query parameters for GET requests (defined in
PARAMETER_API_PATTERNS in Auth::DefaultCurrentUserProvider).
Additionally, both `fetch_raw_feed` and `set_image_as_thumbnail` used a
two-step approach: `FinalDestination#resolve` (HEAD) to follow
redirects, then a separate `Excon` GET to download the content — making two full
HTTP requests per URL.
Switch both methods to use `FinalDestination#get`, which handles
redirect following, SSRF protection, and body streaming in a single request. This
is the same approach used by `FileHelper` and `RetrieveTitle` elsewhere
in the codebase.
Ref - t/174086
When searching for chatables (users, groups, channels), results that
exactly match the search term now appear before prefix matches, which
in turn appear before partial matches. Previously, match quality was
computed client-side based on the result names, but this was unreliable
for DM channels where the match depends on participant usernames that
aren't always directly visible.
The server now computes a match_quality score (exact=1, prefix=2,
partial=3) via SQL CASE expressions and returns it through the
serializer. For DM channels, MIN() is used across all participants
so the best match among them determines the channel's rank.
The client-side sorting was simplified to use the server-provided
scores, and the type priority was reordered to users > DM channels >
category channels > groups, which better matches how people typically
search in chat.
LIKE patterns are now escaped with sanitize_sql_like to prevent
wildcard characters in search terms from producing unexpected results.
Missing visibility authorization in
`PostVoting::CommentsController#flag` allows users to flag comments on
posts/topics they cannot access
This is annoying, but not just a bug.
The `stop_streaming_response` endpoint in `BotController` allowed any
authenticated user who could see a bot reply post to cancel its
streaming generation, even if they were not authorized bot users (not in
`ai_bot_allowed_groups` or `ai_bot_enabled` was false).
This is extremely minor, the PMs usually will not contain other users
unless invited anyway.
Non-listable and disabled badges exposed to anonymous users via XHR JSON
requests to `GET /badges.json`.
This is minor badges are not a security feature, but this is nice
defense in depth
Users can bypass "Slow Mode" topic restrictions by sending `auto_track:
false` in the JSON body when creating posts via `POST /posts.json`
Slow mode is not a security feature, but it is nice to seal this edge
case. Instead of leaning on topic user to get the information we now
look directly at posts by a user
Unauthenticated author spoofing via `discourse_username` parameter in
the `/embed/comments` endpoint allows an anonymous attacker to create
embedded topics. This feature has been deprecated for years now (since
3.2) so this commit removes it.
Lean no meta tags instead.
Modernize the house ads admin interface to align with
Discourse admin UI patterns:
- Extract HouseAdForm component using FormKit, replacing
the controller-heavy approach with observers and buffered
state
- Redesign index page with DPageSubheader, tabbed nav
(Ads/Settings), d-admin-table, and empty state component
- Simplify show page to BackButton + HouseAdForm
- Harden backend controller: use Discourse::NotFound,
derive id from URL params instead of body, remove id
from permitted params to prevent injection, remove
update-creates-if-missing behavior
- Add guard in route for missing house ads
- Fix group mapping to handle both object and scalar values
- Rewrite system spec as full lifecycle test with page object
- Use fab! over let in request specs, add coverage for 404
and id injection edge cases
---------
Co-authored-by: Martin Brennan <martin@discourse.org>
Empty-scopes bypass allows untrusted client registration via
`UserApiKeyClientsController#create` — sending `scopes=","` creates a
client with zero persisted scopes, bypassing downstream scope
restrictions.
This hardens the param to avoid this kind of empty scope registration.
The `retry_response` endpoint in `BotController` only checked post
visibility (`guardian.ensure_can_see!`) but did not enforce
`ai_bot_enabled` or `ai_bot_allowed_groups` membership.
This defends this endpoint in depth. In reality nobody has access anyway
to these PMs and risk around regeneration is very low.
This first commits adds the upcoming change directory, which allows you
to ask to your llm: "Help me add an upcoming change" and will get you
through the various questions/tasks required to do this.
Facebook's crawler requires image dimension metadata to reliably display
image previews when Discourse links are shared. Currently, we only emit
`og:image` without the structured dimension properties, which causes
Facebook (and other consumers) to skip the image in share previews.
This adds `og:image:width` and `og:image:height` meta tags for topic
pages by exposing the image upload's dimensions through TopicView and
passing them to `crawlable_meta_data`. The dimensions are only emitted
when both width and height are available on the upload record.
https://meta.discourse.org/t/395484
in CSS we check `< 48rem` but in `narrow-desktop.js` we check
`document.body.getBoundingClientRect().width < 768`. At 100% zoom these
are the same.
At other zoom levels, REMs scale with zoom but
`document.body.getBoundingClientRect().width` falls out of sync and
compares to the static 768px — so we get a mismatch and in some cases
this causes unexpected styling issues
What we can do to sync these up is use `window.matchMedia("(min-width:
48rem)")` instead
This is not considered a security issue as enumeration on a string is
very time consuming and would only give this info: this slug exists.
The fix ensures the slug is only for logged in and staff, and adds
tests.
When `unicode_usernames` is enabled and the default locale is
non-English, `Jobs::EnsureDbConsistency` renames automatic groups to
localized Unicode names (e.g., "admins" → "管理员"). Visiting the group's
assigned topics page then triggers a 500 error
(`URI::InvalidComponentError`) because the Unicode group name isn't
properly encoded when building pagination URLs.
The encoding of route params affected by `unicode_usernames` was
scattered across multiple places:
- `:username` was encoded in `page_params`
- `:group_name` was encoded in `construct_url_with`
- `:groupname` (assign plugin) was not encoded anywhere → 500 error
This consolidates all encoding into `construct_url_with`:
- `page_params` now passes raw values for all route params (`:username`,
`:group_name`, `:groupname`, `:period`) via a single loop
- `construct_url_with` encodes all three Unicode-sensitive params
(`:username`, `:group_name`, `:groupname`) in both `page_params` and
`opts` when `unicode_usernames` is enabled
- The unencode block now uses simple string splitting instead of
`URI.parse`, which cannot handle Unicode characters in paths
https://meta.discourse.org/t/396092
Watched words failed to match in CJK (Chinese, Japanese, Korean) and
other spaceless scripts because word boundary detection relied on
whitespace or non-word characters. Languages like Chinese don't use
spaces between words, so "测试" inside "这是一个测试文本" was never matched.
Introduce a SPACELESS_SCRIPTS constant covering Han, Hiragana, Katakana,
Hangul, Thai, Lao, Myanmar, Khmer, and Tibetan Unicode ranges. Update
`match_word_regexp` for both Ruby and JS engines so that characters from
these scripts are treated as word boundaries. This allows a CJK watched
word to match when surrounded by other CJK characters, and a Latin
watched word to match when adjacent to CJK text (e.g., "Test" in
"我的Test很好"), while still preventing partial Latin matches (e.g.,
"Testing" does not match "Test").
Also fix the admin watched word testing modal to use `RegExp.exec()`
with capture group extraction instead of `String.match()`, since the new
boundary patterns include a leading consuming group.
Remove the outdated "non-chrome browsers do not support lookbehind"
comment — all major browsers have supported lookbehind since 2023.
https://meta.discourse.org/t/71288https://meta.discourse.org/t/396109
This was a missing early check for passkey logins. Users wouldn't be
able to log in, even though the passkey login check would return true
because the current user provider would still return `nil`.
This does improve the UI though, previously we wouldn't show an error
message, now we will, via a modal (like in regular and email logins).
This was only doable by admins so not considered as security, but this
could cause errors if an admin was doing it inadvertently. This commit
ensures it's not possible anymore and adds tests for it.
- Extracts a shared `DiskCacheEviction.evict` utility used by both
avatar proxy cache (`tmp/avatar_proxy/`) and download cache
(`tmp/download_cache/`)
- `base_store.rb` no longer sorts the entire file list on every
`cache_file` call, eviction only runs when count exceeds the limit
- Fixes a concurrency bug in `proxy_avatar` where a file could be
evicted between `File.exist?` and `send_file`, now rescues
`Errno::ENOENT` / `ActionController::MissingFile` and falls back to
`render_blank`
Automatic group names (everyone, staff, admins, etc.) are translated
based on the site's default locale. However, the `AUTO_GROUPS` JS
constant had hardcoded English names which were used directly in several
places — most notably during category creation. This caused the
"everyone" group to always display its English name in the permissions
table, even on sites with a different default locale.
This removes the `name` and `display_name` fields from `AUTO_GROUPS`,
keeping only `id` and `automatic`. A new `groupsById` getter on the
`Site` model provides a lookup map built from server-provided group data
(which contains the properly localized names). All consumers now resolve
group names via `site.groupsById[groupId].name` instead of reading from
the constant.
The avatar-flair system is also refactored from name-based to ID-based
lookups, and group identity checks in `user.js` now compare by `id`
rather than `name`.
https://meta.discourse.org/t/395225
The ProseMirror emoji input rule was converting text shortcuts
like `:)` into emoji images regardless of the
`enable_emoji_shortcuts` site setting. This adds a guard to
skip the conversion when the setting is disabled.
This moves the splash screen SVG processing (SMIL and script stripping)
to upload rather than render for the `splash_screen_image` setting.
While testing I also found a case where SVG dimensions could make splash
images very tiny, so I strip out the dimensions (as long as a viewbox is
present) so the SVG can scale to fit the wrapper better.
---------
Co-authored-by: Régis Hanol <regis@hanol.fr>
Co-authored-by: Martin Brennan <martin@discourse.org>
What is the problem?
The `approve_suspect_users` feature flags new accounts as suspect if
they have a bio/website but minimal reading activity. The
`Jobs::EnqueueSuspectUsers` scheduled job runs every 2 hours and targets
accounts that are at least 1 day old. If an admin suspends a spammy user
before that job runs, the suspended user still gets added to the review
queue, creating unnecessary noise for moderators reviewing an
already-handled case.
What is the solution?
Added the `User.not_suspended` scope to the query in
`Jobs::EnqueueSuspectUsers` so that suspended users are excluded. Users
who have already been suspended by an admin have been dealt with and
should not appear in the suspect users review queue.
This commit adds a list of channels associated with
the category under a new Chat tab in the category UI.
This tab is only shown if `enable_simplified_category_creation`
is enabled, and the chat plugin is enabled. The tab also only
shows if the category has any associated channels.
A plugin API needs to be added (`registerEditCategoryTab`) to
allow the chat plugin to register the tab on the category page
with the associated component. We also allow the chat API channels
controller to accept a chatable_id to list the channels associated with
the category.
---------
Co-authored-by: Régis Hanol <regis@hanol.fr>
Some upcoming changes are all-or-nothing — they affect the whole site
or only admins, so enabling them per-group makes no sense. This adds a
`disallow_enabled_for_groups` flag to upcoming change metadata that
restricts the "Enabled for" dropdown to only "Everyone" and "No One",
and clears any existing group assignments server-side as a safety net.
The process_user_email method was replacing valid emails with random
@email.invalid addresses because the deduplication check treated a
user's own email as a duplicate.
The @emails hash is populated during import_users, so when
import_user_emails runs later, the check !@emails.has_key?(email) fails
for the user's own email, triggering the random_email fallback.
Updated the condition to allow an email if it belongs to the same user:
(!@emails.has_key?(email) || @emails[email] == user_id)
Adds support for including the `client_id` as a query parameter in the
endpoint for rp initiated logout. It's optional, turned on by a boolean
site setting, so we don't negatively impact existing users.
Meta: /t/387638
Customer requested this because although [it's optional in the
spec](https://openid.net/specs/openid-connect-rpinitiated-1_0.html#RPLogout),
their IdP requires it.
When someone replies to a message in a DM channel (where threading is
disabled), the system internally creates a thread to track the reply
chain. However, the unread count query was excluding all thread
messages, which meant these replies never showed up as unread in the
sidebar.
The fix has three parts:
1. Update ChannelUnreadsQuery to include DM thread replies in the
unread_count. We specifically check for DirectMessage channels with
threading disabled, and only count messages from other users that
haven't been read yet.
2. When creating a reply in a DM, add all channel participants to the
thread (not just the sender and OM author). This ensures everyone has a
thread membership that can be used for tracking read state.
3. When marking a DM channel as read, also mark any thread memberships
as read. Without this, the unread indicator would persist even after
viewing the channel since the thread membership's last_read wasn't being
updated.
Ref - https://meta.discourse.org/t/384734
Régis Hanol
David Taylor
Martin Brennan
Sam
Joffrey JAFFEUX
Kris
Penar Musaraj
Krzysztof Kotlarek
Ruben Oussoren
Jordan Vidrine
Chris Alberti
Loïc Guitaut
Rafael dos Santos Silva