Commit Graph

1119 Commits

Author SHA1 Message Date
Robin Brandt d60643f5ef Allow chromium unstable endpoint 2025-12-09 20:24:09 -05:00
Jeremy Daer 85bd5c2df5 Rails seeded parallel tests (#2037)
Enable work stealing by default for a tiny speedup at the cost of small
loss in reproducibility.

References https://github.com/rails/rails/pull/56175
2025-12-09 15:50:01 -08:00
Jorge Manrubia 94c59a2399 Merge pull request #2033 from basecamp/limit-webhook-response-size
Stream response in webhook request manually to check size
2025-12-09 12:16:42 +01:00
Rosa Gutierrez 29c7926307 Stream response in webhook request manually to check size
This addresses a DoS vulnerability where the response might be massive
leading to OOM errors, as the response is read in full in memory by
default.

To prevent this, we need to read the body in chunks, checking the
size of the chunks we've read and raising if we go over a certain limit.

I've set the limit to 100 KB because the responses to these requests
should be fairly small or even empty, and we only care about the status
code in the end.
2025-12-09 11:25:12 +01:00
Jeremy Daer ea8a78cc22 Fix stale-read race when creating push subscriptions
(Caught one such uniqueness exception in the wild)

* 200 OK -> 204 No Content, default status
* No need to touch the subscription when found
* Drop superflous test
2025-12-09 01:34:37 -08:00
Jeremy Daer b23a5d0b7c Fix flaky tests caused by leaky show_exceptions twiddling (#2028)
Rails memoizes `@app_env_config`, so running push subscriptions test
after join codes test will result in running with `show_exceptions:
:none`, causing RecordInvalid to raise instead of returning 422 status.

References #1996
2025-12-08 23:27:36 -08:00
Jeremy Daer 49c4f2adc6 Fix ActiveStorage FileNotFoundError with immediate variant processing (#2022)
When ActiveStorage::Record uses `connects_to` for read replica support,
it creates a separate connection pool from ApplicationRecord. This causes
`after_commit` callbacks to fire in non-deterministic order - the
Attachment's `create_variants` callback can fire before the User model's
upload callback completes, resulting in FileNotFoundError.

The fix removes replica connection configuration from ActiveStorage::Record
so it shares the same connection pool as application models, ensuring
proper callback ordering.

Also reverts test workarounds that were added to work around this issue,
since the root cause is now fixed.

See: https://github.com/rails/rails/issues/53694
2025-12-08 14:44:49 -08:00
Jorge Manrubia cb0e9b9962 Immediate avatar and embed variants (#2002)
Reverts #2001
Restores #1955
2025-12-08 14:17:06 -08:00
Mike Dalessio 3c4b838b25 Fix Identity destruction callback ordering
ref: #2003

Co-authored-by: Dylan <dylan@restaurantcare.com.au>
2025-12-08 15:12:38 -05:00
Mike Dalessio abe48f0efa Ensure user toggles on board edit page are cached properly
Not including the disabled flag as part of the cache resulted in
issues like the one described in #1992
2025-12-08 14:21:56 -05:00
Mike Dalessio 48730d67e0 Disable board edit buttons for select all/none when not privileged
ref: https://github.com/basecamp/fizzy/discussions/1992
2025-12-08 13:32:41 -05:00
Mike Dalessio 661a7e5e2d Validate email before creating identity during join code redemption
Avoid Sentry exceptions when attackers try to stuff invalid emails. The
browser performs form field validation that should normally prevent this
from occurring, so we just return 422 without validation error messages.

See similar change in #1996 for sign-in and sign-up
2025-12-07 13:26:34 -05:00
Jorge Manrubia c8c91259c7 Revert "Immediate avatar and embed variants" 2025-12-07 12:06:03 +01:00
Jorge Manrubia 568783efd1 Merge pull request #1905 from basecamp/web-push-ssrf
Security: Web Push SSRF
2025-12-07 11:58:14 +01:00
Jorge Manrubia 91017c9208 Merge pull request #1955 from basecamp/immediate-variants
Immediate avatar and embed variants
2025-12-07 11:50:02 +01:00
Felipe Felix 4c9d8edbc6 Test Notifications::SettingsController 2025-12-07 04:25:11 -03:00
Mike Dalessio 0160f215f2 Validate email before creating identity during sign-up and sign-in
Avoid Sentry exceptions when attackers try to stuff invalid emails. The
browser performs form field validation that should normally prevent this
from occurring, so we just return 422 without validation error messages.

Also:

- extract redirect_to_session_magic_link helper
- some controller refactoring and cleanup
2025-12-06 16:52:16 -05:00
Mike Dalessio 95ba2c01b8 Add email validation to Signup 2025-12-06 16:50:39 -05:00
Mike Dalessio 3ff45d77bb Ensure test coverage for invalid Signup#complete 2025-12-06 16:46:51 -05:00
Mike Dalessio b2250b6d35 Ensure test coverage for all branches of JoinCodesController#create 2025-12-06 16:40:51 -05:00
Mike Dalessio d747d1bfa3 Ensure we have good test coverage on Signup.create_identity
because we're relying on it returning a MagicLink
2025-12-06 16:40:17 -05:00
Mike Dalessio c388f5ef20 Display validation errors for user profiles.
Specifically this will help people understand why their SVG avatar
uploads are being rejected, and will keep the RecordInvalid exception
out of Sentry logs.
2025-12-06 14:34:08 -05:00
Jorge Manrubia ffb357becf Merge pull request #1742 from basecamp/latest-lexxy
Update lexxy to latest
2025-12-06 10:31:19 +01:00
Mike Dalessio 69f8216982 Pass correct params to webhook reactivation url helper.
ref: https://app.fizzy.do/5986089/cards/3219
2025-12-05 22:44:34 -05:00
Mike Dalessio 1ad52d2590 Only send notification emails to verified users
Adds verified? check to bundling_emails? to prevent notification emails
from being sent to users who have never authenticated. This closes the
spam vector where bad actors could create users for known email
addresses and trigger unwanted notifications by mentioning them.

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-05 21:51:44 -05:00
Mike Dalessio 4602cd3cdd Add verified_at timestamp to use for spam prevention
User are marked as verified after a join code is redeemed. The user is
redirected to Users::VerificationsController, either:

- after submitting a valid magic link code,
- or immediately after redeeming the join code (if they're already
  authenticated with the correct identity)

Account owners are automatically verified when the account is
created (because they have already provided a magic link code at that
point).

This sets up for later commits that will backfill existing users and
require verification before sending notification emails.

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-05 21:51:44 -05:00
Mike Dalessio fa549a370b Add a system test for joining an account
Reworked the magic link stimulus controller, because the system test
was causing double-submission of the form (because the event was
bubbling up). I think that change simplifies the form and will still
work well for iOS devices.
2025-12-05 21:51:44 -05:00
Stanko K.R. 1b3d7d4276 Fix ipaddr setter 2025-12-05 20:43:56 +01:00
Jorge Manrubia 80a0eb3a4d Fix system test 2025-12-05 11:30:58 +01:00
Jorge Manrubia c89db89f2a Only broadcast when the preview changes, use the _later variant for the broadcast, add tests 2025-12-05 10:09:20 +01:00
Jeremy Daer 21f3f72647 Immediate avatar and embed variants
Process variants synchronously on attachment to close the window between
image upload and variant availability, guaranteeing that we won't have
lazy variant processing attempts in GET requests.

Tradeoff is that we do variant processing in upload requests, which is
actually desirable. We're working with images that should take
milliseconds to resize given that we'll already have the file on hand.

References https://github.com/rails/rails/pull/51951
2025-12-04 23:54:37 -08:00
Jeremy Daer 496851b255 Security: Web Push SSRF and IP range bypass
Add SSRF protection for web push endpoints:
- Resolve endpoint IP once and pin it for connection
- Validate endpoints resolve to public IPs
- Whitelist permitted push service hosts

Add missing IP ranges to SsrfProtection:
- 100.64.0.0/10 (Carrier-grade NAT, RFC6598)
- 198.18.0.0/15 (Benchmark testing, RFC2544)

Note: link-local (169.254.0.0/16) is already covered by ip.link_local?
2025-12-04 21:35:55 -08:00
Mike Dalessio 00eee29837 Validate Identity email address
using the "standard" email regexp URI::MailTo::EMAIL_REGEXP. The form
field will validate this in the browser, but if bots are creating
identities, they can put whatever they want in here. So let's add some
protection against that.

The HtmlHelper regex was renamed here to avoid confusing Brakeman,
which does imprecise constant lookup and was confusing the two
constants, one of which uses `\A` and `\z` and the other does
not (intentionally).

ref: https://app.fizzy.do/5986089/cards/3276
2025-12-04 14:01:15 -05:00
Jason Zimdars d4a50c996a Merge pull request #1894 from Venkat-RK/main
Added 'Back to Home' link on notifications list page
2025-12-04 11:19:55 -06:00
Carmine Paolino c9eb44119d Fixed typo in VCR filters (#1929)
I know it's a small thing, but I just noticed that 🤷

Be careful if you reintroduce your old VCRs as they have OPEN_API_KEY
2025-12-04 08:58:12 -08:00
vkagithala a92aab9981 Removed test to verify back button. 2025-12-04 08:44:37 -08:00
Mike Dalessio 373f4d5116 Merge pull request #1924 from basecamp/flavorjones/retry-mail-jobs
Retry mailer jobs on common networking and SMTP errors
2025-12-04 10:29:53 -05:00
Mike Dalessio c4a8996562 Retry mailer jobs on common networking and SMTP errors
This concern is lifted nearly verbatim from Basecamp.

ref: https://app.fizzy.do/5986089/cards/3300
2025-12-04 10:26:24 -05:00
Kevin McConnell dfe7095ee6 Merge pull request #1917 from basecamp/dont-resize-svg-avatars
Don't try to resize non-variable avatars
2025-12-04 14:53:00 +00:00
Mike Dalessio 394c5dbf03 Revert "Changing columns requires board admin"
This reverts commit cecccadc14.
2025-12-04 09:46:30 -05:00
Kevin McConnell 2e47749739 Don't allow SVG avatar uploads in the first place 2025-12-04 14:27:28 +00:00
Mike Dalessio 89940d36f8 Gracefully handle ill-formed remote images in rich text
A better fix has been proposed upstream at
https://github.com/rails/rails/pull/56283 but this should be fine in
the meantime.

ref: https://app.fizzy.do/5986089/cards/3188
2025-12-04 09:24:52 -05:00
Kevin McConnell 6475ad3425 Don't try to resize non-variable avatars 2025-12-04 13:36:21 +00:00
Stanko K.R. a8f328c921 Sign up new users on sign in 2025-12-04 11:30:21 +01:00
Jeremy Daer cecccadc14 Changing columns requires board admin 2025-12-03 23:24:25 -08:00
Jeremy Daer 7af93765a8 Security: close narrow window of exposure to DNS rebinding (#1903) 2025-12-03 23:12:17 -08:00
vkagithala 8685b62232 Added 'Back to Home' link on notifications list page to improve navigation experience. 2025-12-03 17:21:50 -08:00
Jeremy Daer cac0ca1656 Scope the single-board case to just the creator's boards (#1880) 2025-12-03 15:47:04 -08:00
Jorge Manrubia 38d86fb17b Merge pull request #1879 from basecamp/scope-tags
Scope tags by account
2025-12-03 23:42:04 +01:00
Jorge Manrubia 4a30663df1 Scope tags by account
We missed this one when we went to MySQL. This can results in cards tagged with cards
from other accounts. No data leaked though: the symptom is that you see the card
tagged as expected but you don't see the tag in the menu.
2025-12-03 23:39:37 +01:00