diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 000000000..13beab928 --- /dev/null +++ b/.mcp.json @@ -0,0 +1,18 @@ +{ + "mcpServers": { + "chrome-devtools": { + "type": "stdio", + "command": "npx", + "args": ["-y", "chrome-devtools-mcp@latest"] + }, + "sentry": { + "type": "stdio", + "command": "npx", + "args": ["-y", "mcp-remote@latest", "https://mcp.sentry.dev/mcp"] + }, + "grafana": { + "type": "http", + "url": "https://grafana.37signals.com/mcp" + } + } +} diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..e8d1f5d63 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,179 @@ +# Fizzy + +This file provides guidance to AI coding agents working with this repository. + +## What is Fizzy? + +Fizzy is a collaborative project management and issue tracking application built by 37signals/Basecamp. It's a kanban-style tool for teams to create and manage cards (tasks/issues) across boards, organize work into columns representing workflow stages, and collaborate via comments, mentions, and assignments. + +## Development Commands + +### Setup and Server +```bash +bin/setup # Initial setup (installs gems, creates DB, loads schema) +bin/dev # Start development server (runs on port 3006) +``` + +Development URL: http://fizzy.localhost:3006 +Login with: david@37signals.com (development fixtures), password will appear in the browser console + +### Testing +```bash +bin/rails test # Run unit tests (fast) +bin/rails test test/path/file_test.rb # Run single test file +bin/rails test:system # Run system tests (Capybara + Selenium) +bin/ci # Run full CI suite (style, security, tests) + +# For parallel test execution issues, use: +PARALLEL_WORKERS=1 bin/rails test +``` + +CI pipeline (`bin/ci`) runs: +1. Rubocop (style) +2. Bundler audit (gem security) +3. Importmap audit +4. Brakeman (security scan) +5. Application tests +6. System tests + +### Database +```bash +bin/rails db:fixtures:load # Load fixture data +bin/rails db:migrate # Run migrations +bin/rails db:reset # Drop, create, and load schema +``` + +### Other Utilities +```bash +bin/rails dev:email # Toggle letter_opener for email preview +bin/jobs # Manage Solid Queue jobs +bin/kamal deploy # Deploy (requires 1Password CLI for secrets) +``` + +## Architecture Overview + +### Multi-Tenancy (URL-Based) + +Fizzy uses **URL path-based multi-tenancy**: +- Each Account (tenant) has a unique `external_account_id` (7+ digits) +- URLs are prefixed: `/{account_id}/boards/...` +- Middleware (`AccountSlug::Extractor`) extracts the account ID from the URL and sets `Current.account` +- The slug is moved from `PATH_INFO` to `SCRIPT_NAME`, making Rails think it's "mounted" at that path +- All models include `account_id` for data isolation +- Background jobs automatically serialize and restore account context + +**Key insight**: This architecture allows multi-tenancy without subdomains or separate databases, making local development and testing simpler. + +### Authentication & Authorization + +**Passwordless magic link authentication**: +- Global `Identity` (email-based) can have `Users` in multiple Accounts +- Users belong to an Account and have roles: admin, member, system +- Sessions managed via signed cookies +- Board-level access control via `Access` records + +### Core Domain Models + +**Account** → The tenant/organization +- Has users, boards, cards, tags, webhooks +- Has entropy configuration for auto-postponement + +**Identity** → Global user (email) +- Can have Users in multiple Accounts +- Session management tied to Identity + +**User** → Account membership +- Belongs to Account and Identity +- Has role (admin/member/system) +- Board access via explicit `Access` records + +**Board** → Primary organizational unit +- Has columns for workflow stages +- Can be "all access" or selective +- Can be published publicly with shareable key + +**Card** → Main work item (task/issue) +- Sequential number within each Account +- Rich text description and attachments +- Lifecycle: triage → columns → closed/not_now +- Automatically postpones after inactivity ("entropy") + +**Event** → Records all significant actions +- Polymorphic association to changed object +- Drives activity timeline, notifications, webhooks +- Has JSON `particulars` for action-specific data + +### Entropy System + +Cards automatically "postpone" (move to "not now") after inactivity: +- Account-level default entropy period +- Board-level entropy override +- Prevents endless todo lists from accumulating +- Configurable via Account/Board settings + +### UUID Primary Keys + +All tables use UUIDs (UUIDv7 format, base36-encoded as 25-char strings): +- Custom fixture UUID generation maintains deterministic ordering for tests +- Fixtures are always "older" than runtime records +- `.first`/`.last` work correctly in tests + +### Background Jobs (Solid Queue) + +Database-backed job queue (no Redis): +- Custom `FizzyActiveJobExtensions` prepended to ActiveJob +- Jobs automatically capture/restore `Current.account` +- Mission Control::Jobs for monitoring + +Key recurring tasks (via `config/recurring.yml`): +- Deliver bundled notifications (every 30 min) +- Auto-postpone stale cards (hourly) +- Cleanup jobs for expired links, deliveries + +### Sharded Full-Text Search + +16-shard MySQL full-text search instead of Elasticsearch: +- Shards determined by account ID hash (CRC32) +- Search records denormalized for performance +- Models in `app/models/search/` + +## Production Observability + +Grafana MCP tools provide access to production metrics and logs for performance analysis. + +### Datasources +| Name | UID | Use | +|------|-----|-----| +| Thanos (Prometheus) | `PC96415006F908B67` | Metrics, latencies | +| Loki | `e38bdfea-097e-47fa-a7ab-774fd2487741` | Application logs | + +### Key Metrics +- `rails_request_duration_seconds_bucket:rate1m:sum_by_app:quantiles{app="fizzy"}` - Request latency percentiles +- `rails_request_total:rate1m:sum_by_controller_action{app="fizzy"}` - Request rates by endpoint +- `fizzy_replica_wait_seconds` - Database replica consistency wait times + +### Loki Log Labels +Query: `{service_namespace="fizzy", deployment_environment_name="production", service_name="rails"}` + +Useful fields: `event_duration_ms`, `performance_time_db_ms`, `performance_time_cpu_ms`, `rails_endpoint`, `rails_controller` + +### Instrumentation +Yabeda-based metrics exported at `:9394/metrics`. Config in `config/initializers/yabeda.rb`. + +### Chrome MCP (Local Dev) +URL: `http://fizzy.localhost:3006` +Login: david@37signals.com (passwordless magic link auth - check rails console for link) + +Use Chrome MCP tools to interact with the running dev app for UI testing and debugging. + +### Sentry Error Tracking +Organization: `basecamp` | Project: `fizzy` | Region: `https://us.sentry.io` + +Use Sentry MCP tools to investigate production errors: +- `search_issues` - Find grouped issues by natural language query +- `get_issue_details` - Get full stacktrace and context for a specific issue +- `analyze_issue_with_seer` - AI-powered root cause analysis with code fix suggestions + +## Coding style + +Please read the separate file `STYLE.md` for some guidance on coding style. diff --git a/CLAUDE.md b/CLAUDE.md index a7141f6a6..43c994c2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,142 +1 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## What is Fizzy? - -Fizzy is a collaborative project management and issue tracking application built by 37signals/Basecamp. It's a kanban-style tool for teams to create and manage cards (tasks/issues) across boards, organize work into columns representing workflow stages, and collaborate via comments, mentions, and assignments. - -## Development Commands - -### Setup and Server -```bash -bin/setup # Initial setup (installs gems, creates DB, loads schema) -bin/dev # Start development server (runs on port 3006) -``` - -Development URL: http://fizzy.localhost:3006 -Login with: david@37signals.com (development fixtures), password will appear in the browser console - -### Testing -```bash -bin/rails test # Run unit tests (fast) -bin/rails test test/path/file_test.rb # Run single test file -bin/rails test:system # Run system tests (Capybara + Selenium) -bin/ci # Run full CI suite (style, security, tests) - -# For parallel test execution issues, use: -PARALLEL_WORKERS=1 bin/rails test -``` - -CI pipeline (`bin/ci`) runs: -1. Rubocop (style) -2. Bundler audit (gem security) -3. Importmap audit -4. Brakeman (security scan) -5. Application tests -6. System tests - -### Database -```bash -bin/rails db:fixtures:load # Load fixture data -bin/rails db:migrate # Run migrations -bin/rails db:reset # Drop, create, and load schema -``` - -### Other Utilities -```bash -bin/rails dev:email # Toggle letter_opener for email preview -bin/jobs # Manage Solid Queue jobs -bin/kamal deploy # Deploy (requires 1Password CLI for secrets) -``` - -## Architecture Overview - -### Multi-Tenancy (URL-Based) - -Fizzy uses **URL path-based multi-tenancy**: -- Each Account (tenant) has a unique `external_account_id` (7+ digits) -- URLs are prefixed: `/{account_id}/boards/...` -- Middleware (`AccountSlug::Extractor`) extracts the account ID from the URL and sets `Current.account` -- The slug is moved from `PATH_INFO` to `SCRIPT_NAME`, making Rails think it's "mounted" at that path -- All models include `account_id` for data isolation -- Background jobs automatically serialize and restore account context - -**Key insight**: This architecture allows multi-tenancy without subdomains or separate databases, making local development and testing simpler. - -### Authentication & Authorization - -**Passwordless magic link authentication**: -- Global `Identity` (email-based) can have `Users` in multiple Accounts -- Users belong to an Account and have roles: admin, member, system -- Sessions managed via signed cookies -- Board-level access control via `Access` records - -### Core Domain Models - -**Account** → The tenant/organization -- Has users, boards, cards, tags, webhooks -- Has entropy configuration for auto-postponement - -**Identity** → Global user (email) -- Can have Users in multiple Accounts -- Session management tied to Identity - -**User** → Account membership -- Belongs to Account and Identity -- Has role (admin/member/system) -- Board access via explicit `Access` records - -**Board** → Primary organizational unit -- Has columns for workflow stages -- Can be "all access" or selective -- Can be published publicly with shareable key - -**Card** → Main work item (task/issue) -- Sequential number within each Account -- Rich text description and attachments -- Lifecycle: triage → columns → closed/not_now -- Automatically postpones after inactivity ("entropy") - -**Event** → Records all significant actions -- Polymorphic association to changed object -- Drives activity timeline, notifications, webhooks -- Has JSON `particulars` for action-specific data - -### Entropy System - -Cards automatically "postpone" (move to "not now") after inactivity: -- Account-level default entropy period -- Board-level entropy override -- Prevents endless todo lists from accumulating -- Configurable via Account/Board settings - -### UUID Primary Keys - -All tables use UUIDs (UUIDv7 format, base36-encoded as 25-char strings): -- Custom fixture UUID generation maintains deterministic ordering for tests -- Fixtures are always "older" than runtime records -- `.first`/`.last` work correctly in tests - -### Background Jobs (Solid Queue) - -Database-backed job queue (no Redis): -- Custom `FizzyActiveJobExtensions` prepended to ActiveJob -- Jobs automatically capture/restore `Current.account` -- Mission Control::Jobs for monitoring - -Key recurring tasks (via `config/recurring.yml`): -- Deliver bundled notifications (every 30 min) -- Auto-postpone stale cards (hourly) -- Cleanup jobs for expired links, deliveries - -### Sharded Full-Text Search - -16-shard MySQL full-text search instead of Elasticsearch: -- Shards determined by account ID hash (CRC32) -- Search records denormalized for performance -- Models in `app/models/search/` - -## Coding style - -Please read the separate file `STYLE.md` for some guidance on coding style. +@AGENTS.md diff --git a/Gemfile b/Gemfile index a74c6ed43..d008f4907 100644 --- a/Gemfile +++ b/Gemfile @@ -34,6 +34,7 @@ gem "aws-sdk-s3", require: false gem "web-push" gem "net-http-persistent" gem "mittens" +gem "useragent", bc: "useragent" # Telemetry, logging, and operations gem "mission_control-jobs" @@ -49,7 +50,7 @@ gem "yabeda-prometheus-mmap" gem "yabeda-puma-plugin" gem "yabeda-rails" gem "webrick" # required for yabeda-prometheus metrics server -gem "prometheus-client-mmap", "~> 1.1" +gem "prometheus-client-mmap", "~> 1.3" gem "autotuner" gem "benchmark" # indirect dependency, being removed from Ruby 3.5 stdlib so here to quash warnings diff --git a/Gemfile.lock b/Gemfile.lock index d7a3121dc..f8001333c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -16,6 +16,12 @@ GIT json rails (>= 6.0.0) +GIT + remote: https://github.com/basecamp/useragent + revision: 433ca320a42db1266c4b89df74d0abdb9a880c5e + specs: + useragent (0.16.11) + GIT remote: https://github.com/basecamp/yabeda-activejob.git revision: 684973f77ff01d8b3dd75874538fae55961e15e6 @@ -27,7 +33,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: 077c3ad60db4c43cccb8f4637b53b8d8eb7a3c19 + revision: b22cb0c0cb39b103d816a66d560991acf0a57163 branch: main specs: actioncable (8.2.0.alpha) @@ -147,15 +153,15 @@ GEM activemodel (>= 7.0) activemodel-serializers-xml (~> 1.0) activesupport (>= 7.0) - addressable (2.8.7) - public_suffix (>= 2.0.2, < 7.0) + addressable (2.8.8) + public_suffix (>= 2.0.2, < 8.0) anyway_config (2.7.2) ruby-next-core (~> 1.0) ast (2.4.3) - autotuner (1.0.2) + autotuner (1.1.0) aws-eventstream (1.4.0) - aws-partitions (1.1175.0) - aws-sdk-core (3.234.0) + aws-partitions (1.1187.0) + aws-sdk-core (3.239.1) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) aws-sigv4 (~> 1.9) @@ -163,10 +169,10 @@ GEM bigdecimal jmespath (~> 1, >= 1.6.1) logger - aws-sdk-kms (1.115.0) - aws-sdk-core (~> 3, >= 3.234.0) + aws-sdk-kms (1.118.0) + aws-sdk-core (~> 3, >= 3.239.1) aws-sigv4 (~> 1.5) - aws-sdk-s3 (1.201.0) + aws-sdk-s3 (1.205.0) aws-sdk-core (~> 3, >= 3.234.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.5) @@ -178,9 +184,9 @@ GEM benchmark (0.5.0) bigdecimal (3.3.1) bindex (0.8.1) - bootsnap (1.18.6) + bootsnap (1.19.0) msgpack (~> 1.2) - brakeman (7.1.0) + brakeman (7.1.1) racc builder (3.3.0) bundler-audit (0.9.2) @@ -199,7 +205,7 @@ GEM logger (~> 1.5) chunky_png (1.4.0) concurrent-ruby (1.3.5) - connection_pool (2.5.4) + connection_pool (2.5.5) crack (1.0.1) bigdecimal rexml @@ -290,7 +296,7 @@ GEM logger mini_mime (1.1.5) mini_portile2 (2.8.9) - minitest (5.26.1) + minitest (5.26.2) mission_control-jobs (1.1.0) actioncable (>= 7.1) actionpack (>= 7.1) @@ -302,7 +308,7 @@ GEM stimulus-rails turbo-rails mittens (0.3.0) - mocha (2.7.1) + mocha (2.8.2) ruby2_keywords (>= 0.0.5) msgpack (1.8.0) net-http-persistent (4.0.6) @@ -333,10 +339,10 @@ GEM racc (~> 1.4) nokogiri (1.18.10-x86_64-linux-musl) racc (~> 1.4) - openssl (3.3.1) + openssl (3.3.2) ostruct (0.6.3) parallel (1.27.0) - parser (3.3.9.0) + parser (3.3.10.0) ast (~> 2.4.1) racc platform_agent (1.0.1) @@ -346,31 +352,31 @@ GEM prettyprint prettyprint (0.2.0) prism (1.6.0) - prometheus-client-mmap (1.2.10) + prometheus-client-mmap (1.3.0) base64 bigdecimal logger - rb_sys (~> 0.9.109) - prometheus-client-mmap (1.2.10-arm64-darwin) + rb_sys (~> 0.9.117) + prometheus-client-mmap (1.3.0-arm64-darwin) base64 bigdecimal logger - rb_sys (~> 0.9.109) - prometheus-client-mmap (1.2.10-x86_64-darwin) + rb_sys (~> 0.9.117) + prometheus-client-mmap (1.3.0-x86_64-darwin) base64 bigdecimal logger - rb_sys (~> 0.9.109) - prometheus-client-mmap (1.2.10-x86_64-linux-gnu) + rb_sys (~> 0.9.117) + prometheus-client-mmap (1.3.0-x86_64-linux-gnu) base64 bigdecimal logger - rb_sys (~> 0.9.109) - prometheus-client-mmap (1.2.10-x86_64-linux-musl) + rb_sys (~> 0.9.117) + prometheus-client-mmap (1.3.0-x86_64-linux-musl) base64 bigdecimal logger - rb_sys (~> 0.9.109) + rb_sys (~> 0.9.117) propshaft (1.3.1) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -378,7 +384,7 @@ GEM psych (5.2.6) date stringio - public_suffix (6.0.2) + public_suffix (7.0.0) puma (7.1.0) nio4r (~> 2.0) raabro (1.4.0) @@ -405,7 +411,7 @@ GEM rake-compiler-dock (1.9.1) rb_sys (0.9.117) rake-compiler-dock (= 1.9.1) - rdoc (6.15.1) + rdoc (6.16.0) erb psych (>= 4.0.0) tsort @@ -418,8 +424,8 @@ GEM rqrcode (3.1.0) chunky_png (~> 1.0) rqrcode_core (~> 2.0) - rqrcode_core (2.0.0) - rubocop (1.81.1) + rqrcode_core (2.0.1) + rubocop (1.81.7) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) @@ -430,14 +436,14 @@ GEM rubocop-ast (>= 1.47.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 4.0) - rubocop-ast (1.47.1) + rubocop-ast (1.48.0) parser (>= 3.3.7.2) prism (~> 1.4) - rubocop-performance (1.26.0) + rubocop-performance (1.26.1) lint_roller (~> 1.1) rubocop (>= 1.75.0, < 2.0) - rubocop-ast (>= 1.44.0, < 2.0) - rubocop-rails (2.33.4) + rubocop-ast (>= 1.47.1, < 2.0) + rubocop-rails (2.34.1) activesupport (>= 4.2.0) lint_roller (~> 1.1) rack (>= 1.1) @@ -453,7 +459,7 @@ GEM ffi (~> 1.12) logger ruby2_keywords (0.0.5) - rubyzip (3.2.1) + rubyzip (3.2.2) securerandom (0.4.1) selenium-webdriver (4.38.0) base64 (~> 0.2) @@ -461,10 +467,10 @@ GEM rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 4.0) websocket (~> 1.0) - sentry-rails (6.0.0) + sentry-rails (6.1.2) railties (>= 5.2.0) - sentry-ruby (~> 6.0.0) - sentry-ruby (6.0.0) + sentry-ruby (~> 6.1.2) + sentry-ruby (6.1.2) bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) sniffer (0.5.0) @@ -475,23 +481,23 @@ GEM activejob (>= 7.2) activerecord (>= 7.2) railties (>= 7.2) - solid_cache (1.0.8) + solid_cache (1.0.10) activejob (>= 7.2) activerecord (>= 7.2) railties (>= 7.2) - solid_queue (1.2.2) + solid_queue (1.2.4) activejob (>= 7.1) activerecord (>= 7.1) concurrent-ruby (>= 1.3.1) fugit (~> 1.11) railties (>= 7.1) thor (>= 1.3.1) - sqlite3 (2.7.4) + sqlite3 (2.8.0) mini_portile2 (~> 2.8.0) - sqlite3 (2.7.4-arm64-darwin) - sqlite3 (2.7.4-x86_64-darwin) - sqlite3 (2.7.4-x86_64-linux-gnu) - sqlite3 (2.7.4-x86_64-linux-musl) + sqlite3 (2.8.0-arm64-darwin) + sqlite3 (2.8.0-x86_64-darwin) + sqlite3 (2.8.0-x86_64-linux-gnu) + sqlite3 (2.8.0-x86_64-linux-musl) sshkit (1.24.0) base64 logger @@ -519,7 +525,6 @@ GEM unicode-emoji (~> 4.1) unicode-emoji (4.1.0) uri (1.1.1) - useragent (0.16.11) vcr (6.3.1) base64 web-console (4.2.1) @@ -530,7 +535,7 @@ GEM web-push (3.0.2) jwt (~> 3.0) openssl (~> 3.0) - webmock (3.26.0) + webmock (3.26.1) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) @@ -560,7 +565,7 @@ GEM yabeda-prometheus-mmap (0.4.0) prometheus-client-mmap yabeda (~> 0.10) - yabeda-puma-plugin (0.8.0) + yabeda-puma-plugin (0.9.0) json puma yabeda (~> 0.5) @@ -604,7 +609,7 @@ DEPENDENCIES mocha net-http-persistent platform_agent - prometheus-client-mmap (~> 1.1) + prometheus-client-mmap (~> 1.3) propshaft puma (>= 5.0) queenbee! @@ -626,6 +631,7 @@ DEPENDENCIES thruster trilogy (~> 2.9) turbo-rails + useragent! vcr web-console web-push diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 0ade45a66..89b94f079 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,7 @@ class ApplicationController < ActionController::Base include Authentication include Authorization include CurrentRequest, CurrentTimezone, SetPlatform + include RequestForgeryProtection include TurboFlash, ViewTransitions include Saas include RoutingHeaders diff --git a/app/controllers/cards/assignments_controller.rb b/app/controllers/cards/assignments_controller.rb index fdd2a86dc..886537fa1 100644 --- a/app/controllers/cards/assignments_controller.rb +++ b/app/controllers/cards/assignments_controller.rb @@ -2,7 +2,8 @@ class Cards::AssignmentsController < ApplicationController include CardScoped def new - @users = @board.users.active.alphabetically + @assigned_to = @card.assignees.active.alphabetically.where.not(id: Current.user) + @users = @board.users.active.alphabetically.where.not(id: @card.assignees).where.not(id: Current.user) fresh_when etag: [ @users, @card.assignees ] end diff --git a/app/controllers/cards/taggings_controller.rb b/app/controllers/cards/taggings_controller.rb index d388fae22..9190b20f1 100644 --- a/app/controllers/cards/taggings_controller.rb +++ b/app/controllers/cards/taggings_controller.rb @@ -2,7 +2,8 @@ class Cards::TaggingsController < ApplicationController include CardScoped def new - @tags = Current.account.tags.all.alphabetically + @tagged_with = @card.tags.alphabetically + @tags = Current.account.tags.all.alphabetically.where.not(id: @tagged_with) fresh_when etag: [ @tags, @card.tags ] end diff --git a/app/controllers/concerns/request_forgery_protection.rb b/app/controllers/concerns/request_forgery_protection.rb new file mode 100644 index 000000000..682e88a7d --- /dev/null +++ b/app/controllers/concerns/request_forgery_protection.rb @@ -0,0 +1,50 @@ +module RequestForgeryProtection + extend ActiveSupport::Concern + + included do + after_action :append_set_fetch_site_to_vary_header + end + + private + def append_set_fetch_site_to_vary_header + vary_header = response.headers["Vary"].to_s.split(",").map(&:strip).reject(&:blank?) + response.headers["Vary"] = (vary_header + [ "Sec-Fetch-Site" ]).join(",") + end + + def verified_request? + return true if request.get? || request.head? || !protect_against_forgery? + + origin = valid_request_origin? + token = any_authenticity_token_valid? + sec_fetch_site = safe_fetch_site? + report_on_forgery_protection_results(origin:, token:, sec_fetch_site:) + + origin && token + end + + SAFE_FETCH_SITES = %w[ same-origin same-site ] + + def safe_fetch_site? + SAFE_FETCH_SITES.include?(safe_fetch_site_value) + end + + def safe_fetch_site_value + request.headers["Sec-Fetch-Site"].to_s.downcase + end + + def report_on_forgery_protection_results(origin:, token:, sec_fetch_site:) + results = { origin:, token:, sec_fetch_site: } + + unless results.values.all? + info = results.transform_values { it ? "pass" : "fail" } + info[:origin] += " (#{request.origin})" + info[:sec_fetch_site] += " (#{safe_fetch_site_value})" + + Rails.logger.info "CSRF protection check: " + info.map { it.join(" ") }.join(", ") + + if (origin && token) != sec_fetch_site + Sentry.capture_message "CSRF protection mismatch", level: :info, extra: { info: info } + end + end + end +end diff --git a/app/javascript/controllers/navigable_list_controller.js b/app/javascript/controllers/navigable_list_controller.js index fe5fca86c..f1d22e92d 100644 --- a/app/javascript/controllers/navigable_list_controller.js +++ b/app/javascript/controllers/navigable_list_controller.js @@ -14,6 +14,7 @@ export default class extends Controller { hasNestedNavigation: { type: Boolean, default: false }, preventHandledKeys: { type: Boolean, default: false }, autoSelect: { type: Boolean, default: true }, + autoScroll: { type: Boolean, default: true }, onlyActOnFocusedItems: { type: Boolean, default: false } } @@ -80,10 +81,10 @@ export default class extends Controller { await nextFrame() - this.currentItem.scrollIntoView({ block: "nearest", inline: "nearest" }) + if (this.autoScrollValue) { this.currentItem.scrollIntoView({ block: "nearest", inline: "nearest" }) } if (this.hasNestedNavigationValue) { this.#activateNestedNavigableList() } - if (!skipFocus && this.focusOnSelectionValue) { this.currentItem.focus() } + if (!skipFocus && this.focusOnSelectionValue) { this.currentItem.focus({ preventScroll: !this.autoScrollValue }) } } isSelected(item) { @@ -160,7 +161,7 @@ export default class extends Controller { #relayNavigationToParentNavigableList(event) { const parentController = this.#parentNavigableListController if (parentController) { - parentController.element.focus() + parentController.element.focus({ preventScroll: !this.autoScrollValue }) parentController.navigate(event) } } diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb index db3c81bc7..89fea208a 100644 --- a/app/mailers/notification/bundle_mailer.rb +++ b/app/mailers/notification/bundle_mailer.rb @@ -11,6 +11,6 @@ class Notification::BundleMailer < ApplicationMailer mail \ to: bundle.user.identity.email_address, - subject: "Fizzy#{ " (#{ Current.account.name })" if @user.identity.accounts.many? }: Latest Activity" + subject: "Fizzy#{ " (#{ Current.account.name })" if @user.identity.accounts.many? }: New notifications" end end diff --git a/app/models/identity.rb b/app/models/identity.rb index fc1c12bcd..e9ff507a1 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -18,10 +18,6 @@ class Identity < ApplicationRecord end end - def staff? - email_address.ends_with?("@37signals.com") || email_address.ends_with?("@basecamp.com") - end - private def deactivate_users users.find_each(&:deactivate) diff --git a/app/models/user.rb b/app/models/user.rb index 94380b8d9..6140d41fa 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,8 +19,6 @@ class User < ApplicationRecord scope :with_avatars, -> { preload(:account, :avatar_attachment) } - delegate :staff?, to: :identity, allow_nil: true - def deactivate transaction do accesses.destroy_all diff --git a/app/views/boards/show/_columns.html.erb b/app/views/boards/show/_columns.html.erb index a7f8502e3..a6001da79 100644 --- a/app/views/boards/show/_columns.html.erb +++ b/app/views/boards/show/_columns.html.erb @@ -10,6 +10,7 @@ navigable_list_has_nested_navigation_value: true, navigable_list_prevent_handled_keys_value: true, navigable_list_auto_select_value: false, + navigable_list_auto_scroll_value: false, action: " keydown->navigable-list#navigate dragstart->drag-and-drop#dragStart diff --git a/app/views/cards/assignments/_user.html.erb b/app/views/cards/assignments/_user.html.erb new file mode 100644 index 000000000..0d4eff888 --- /dev/null +++ b/app/views/cards/assignments/_user.html.erb @@ -0,0 +1,8 @@ +
You have <%= link_to pluralize(@notifications.count, "new notification"), notifications_url %><%= " in #{ Current.account.name }" if @user.identity.accounts.many? %>.
diff --git a/app/views/mailers/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb index 4a6f84d30..7c5798262 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -1,4 +1,4 @@ -Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %> +Notifications since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %> You have <%= pluralize @notifications.count, "new notification" %><%= " in #{ Current.account.name }" if @user.identity.accounts.many? %>. <% @notifications.group_by(&:card).each do |card, notifications| %> diff --git a/config/database.yml b/config/database.yml index 2d993c369..fc05ed89c 100644 --- a/config/database.yml +++ b/config/database.yml @@ -2,9 +2,11 @@ if ENV["MIGRATE"].present? mysql_app_user_key = "MYSQL_ALTER_USER" mysql_app_password_key = "MYSQL_ALTER_PASSWORD" + max_execution_time_ms = 0 # No limit else mysql_app_user_key = "MYSQL_APP_USER" mysql_app_password_key = "MYSQL_APP_PASSWORD" + max_execution_time_ms = 5_000 end mysql_app_user = ENV[mysql_app_user_key] @@ -19,6 +21,7 @@ default: &default timeout: 5000 variables: transaction_isolation: READ-COMMITTED + max_execution_time: <%= max_execution_time_ms %> development: primary: diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 4226de1d9..8d35e2670 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -4,5 +4,6 @@ if !Rails.env.local? && ENV["SKIP_TELEMETRY"].blank? config.breadcrumbs_logger = %i[ active_support_logger http_logger ] config.send_default_pii = false config.release = ENV["GIT_REVISION"] + config.excluded_exceptions += [ "ActiveRecord::ConcurrentMigrationError" ] end end diff --git a/db/migrate/20251125110629_increase_user_agent_length.rb b/db/migrate/20251125110629_increase_user_agent_length.rb new file mode 100644 index 000000000..b0d6124f2 --- /dev/null +++ b/db/migrate/20251125110629_increase_user_agent_length.rb @@ -0,0 +1,6 @@ +class IncreaseUserAgentLength < ActiveRecord::Migration[8.2] + def change + change_column :sessions, :user_agent, :string, limit: 4096 + change_column :push_subscriptions, :user_agent, :string, limit: 4096 + end +end diff --git a/db/migrate/20251125130010_add_a_staff_flag_to_identities.rb b/db/migrate/20251125130010_add_a_staff_flag_to_identities.rb new file mode 100644 index 000000000..7df018601 --- /dev/null +++ b/db/migrate/20251125130010_add_a_staff_flag_to_identities.rb @@ -0,0 +1,5 @@ +class AddAStaffFlagToIdentities < ActiveRecord::Migration[8.2] + def change + add_column :identities, :staff, :boolean, null: false, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index f951af36b..b3a9f0503 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_11_21_112416) do +ActiveRecord::Schema[8.2].define(version: 2025_11_25_130010) do create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -300,6 +300,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_21_112416) do create_table "identities", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.string "email_address", null: false + t.boolean "staff", default: false, null: false t.datetime "updated_at", null: false t.index ["email_address"], name: "index_identities_on_email_address", unique: true end @@ -378,7 +379,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_21_112416) do t.text "endpoint" t.string "p256dh_key" t.datetime "updated_at", null: false - t.string "user_agent" + t.string "user_agent", limit: 4096 t.uuid "user_id", null: false t.index ["account_id"], name: "index_push_subscriptions_on_account_id" t.index ["user_id", "endpoint"], name: "index_push_subscriptions_on_user_id_and_endpoint", unique: true, length: { endpoint: 255 } @@ -653,7 +654,7 @@ ActiveRecord::Schema[8.2].define(version: 2025_11_21_112416) do t.uuid "identity_id", null: false t.string "ip_address" t.datetime "updated_at", null: false - t.string "user_agent" + t.string "user_agent", limit: 4096 t.index ["identity_id"], name: "index_sessions_on_identity_id" end diff --git a/test/controllers/allow_browser_test.rb b/test/controllers/allow_browser_test.rb new file mode 100644 index 000000000..529caa840 --- /dev/null +++ b/test/controllers/allow_browser_test.rb @@ -0,0 +1,64 @@ +require "test_helper" + +class AllowBrowserTest < ActionDispatch::IntegrationTest + test "Baidu browser is allowed" do + sign_in_as :kevin + + get cards_path, headers: { + "User-Agent" => "Mozilla/5.0 (Linux; Android 7.0; HUAWEI NXT-AL10 Build/HUAWEINXT-AL10; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/48.0.2564.116 Mobile Safari/537.36 baidubrowser/7.9.12.0 (Baidu; P1 7.0)NULL" + } + + assert_response :success + end + + test "nonsense user agent with bot in name is allowed" do + sign_in_as :kevin + + get cards_path, headers: { + "User-Agent" => "TotallyFakeBot/1.0 (NonsenseBrowser; Testing)" + } + + assert_response :success + end + + test "nonsense user agent is allowed" do + sign_in_as :kevin + + get cards_path, headers: { + "User-Agent" => "just some random nonsense text" + } + + assert_response :success + end + + test "old Chrome browser is rejected with 406" do + sign_in_as :kevin + + # Chrome 118 is below the modern threshold of Chrome 120 + get cards_path, headers: { + "User-Agent" => "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118 Safari/537.36" + } + + assert_response :not_acceptable + end + + test "Google Image Proxy is allowed" do + sign_in_as :kevin + + get cards_path, headers: { + "User-Agent" => "Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko Firefox/11.0 (via ggpht.com GoogleImageProxy)" + } + + assert_response :success + end + + test "Facebook/Twitter bot is allowed" do + sign_in_as :kevin + + get cards_path, headers: { + "User-Agent" => "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/601.2.4 (KHTML, like Gecko) Version/9.0.1 Safari/601.2.4 facebookexternalhit/1.1 Facebot Twitterbot/1.0" + } + + assert_response :success + end +end diff --git a/test/controllers/concerns/request_forgery_protection_test.rb b/test/controllers/concerns/request_forgery_protection_test.rb new file mode 100644 index 000000000..797c2039a --- /dev/null +++ b/test/controllers/concerns/request_forgery_protection_test.rb @@ -0,0 +1,148 @@ +require "test_helper" + +class RequestForgeryProtectionTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + + # Forgery protection is disabled in test environment so we need to + # enable it here + @original_allow_forgery_protection = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + end + + teardown do + ActionController::Base.allow_forgery_protection = @original_allow_forgery_protection + end + + test "don't report when Sec-Fetch-Site is same-origin and CSRF token matches" do + assert_log(excludes: "CSRF protection check") do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "same-origin" } + end + end + + test "don't report when Sec-Fetch-Site is same-site and CSRF token matches" do + assert_log(excludes: "CSRF protection check") do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "same-site" } + end + end + + test "fail and report when token doesn't match, regardless of Sec-Fetch-Site" do + assert_report + + assert_log(includes: [ "CSRF protection check", "sec_fetch_site pass (same-origin)", "token fail" ]) do + assert_no_difference -> { Board.count } do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: "invalid-token" }, + headers: { "Sec-Fetch-Site" => "same-origin" } + end + end + end + + test "fail and report when Origin doesn't match, regardless of Sec-Fetch-Site" do + assert_report + + assert_log(includes: [ "CSRF protection check", "sec_fetch_site pass (same-origin)", + "token pass", "origin fail (evil-site.com)" ]) do + assert_no_difference -> { Board.count } do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "same-origin", Origin: "evil-site.com" } + end + end + end + + test "succeed and report when Sec-Fetch-Site is cross-site and CSRF token matches" do + assert_report + + assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail (cross-site)", "token pass" ]) do + assert_difference -> { Board.count }, +1 do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "cross-site" } + end + end + end + + test "succeed and report when Sec-Fetch-Site is none" do + assert_report + + assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail (none)", "token pass" ]) do + assert_difference -> { Board.count }, +1 do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "none" } + end + end + end + + test "succeed and report when Sec-Fetch-Site is missing" do + assert_report + + assert_log(includes: [ "CSRF protection check", "sec_fetch_site fail ()", "token pass" ]) do + assert_difference -> { Board.count }, +1 do + post boards_path, params: { board: { name: "Test Board" }, authenticity_token: csrf_token } + end + end + end + + test "don't report and succeed for GET requests" do + assert_log(excludes: "CSRF protection check") do + get board_url(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" } + assert_response :success + end + end + + test "GET requests succeed regardless of Sec-Fetch-Site header" do + get board_url(boards(:writebook)), headers: { "Sec-Fetch-Site" => "cross-site" } + + assert_response :success + end + + test "appends Sec-Fetch-Site to existing Vary header" do + get board_url(boards(:writebook)), headers: { "Accept" => "text/html" } + + assert_response :success + vary_values = response.headers["Vary"].split(",").map(&:strip) + assert_includes vary_values, "Sec-Fetch-Site" + end + + test "appends Sec-Fetch-Site to Vary header on POST requests" do + post boards_path, + params: { board: { name: "Test Board" }, authenticity_token: csrf_token }, + headers: { "Sec-Fetch-Site" => "same-origin" } + + assert_response :redirect + assert_includes response.headers["Vary"], "Sec-Fetch-Site" + end + + private + def assert_log(includes: [], excludes: [], &block) + original_logger = Rails.logger + log_output = StringIO.new + Rails.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(log_output)) + + yield + + Array(includes).each { assert_includes(log_output.string, it) } + Array(excludes).each { assert_not_includes(log_output.string, it) } + ensure + Rails.logger = original_logger + end + + def assert_report + Sentry.expects(:capture_message).with do |message, **kwargs| + message == "CSRF protection mismatch" && kwargs[:level] == :info + end + end + + def csrf_token + @csrf_token ||= begin + get new_board_url + response.body[/name="authenticity_token" value="([^"]+)"/, 1] + end + end +end diff --git a/test/models/identity_test.rb b/test/models/identity_test.rb index 277c0cc1e..2e7687169 100644 --- a/test/models/identity_test.rb +++ b/test/models/identity_test.rb @@ -13,12 +13,6 @@ class IdentityTest < ActiveSupport::TestCase end end - test "staff?" do - assert Identity.new(email_address: "test@37signals.com").staff? - assert Identity.new(email_address: "test@basecamp.com").staff? - assert_not Identity.new(email_address: "test@example.com").staff? - end - test "join" do identity = identities(:david) account = accounts(:initech) diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index dc4bf9e43..612087682 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -142,7 +142,7 @@ class Notification::BundleTest < ActiveSupport::TestCase assert_not_nil email # Time in Madrid should be 15:30 (UTC+1 in winter) - assert_match /everything since 3pm/i, email.text_part&.body&.to_s + assert_match /notifications since 3pm/i, email.text_part&.body&.to_s end end