Skip to content

Fix security and reliability audit findings#7

Open
dnplkndll wants to merge 1 commit intokencovefrom
audit-bugfixes
Open

Fix security and reliability audit findings#7
dnplkndll wants to merge 1 commit intokencovefrom
audit-bugfixes

Conversation

@dnplkndll
Copy link
Copy Markdown

@dnplkndll dnplkndll commented Feb 28, 2026

Summary

  • Rate limit SMS endpoint — adds RateLimit.call (10 req/hr per user) to SubmittersSendSmsController to prevent Twilio credit abuse
  • Redis timeout config — adds connect_timeout: 2, read_timeout: 1, write_timeout: 1, reconnect_attempts: 1 to RedisCacheStore to prevent hung requests on Redis outage
  • Catch LoadError — widens rescue StandardError to also catch LoadError in rate_limit.rb and active_storage.rb initializer (prevents crashes from missing gems, same class of bug as the redis gem issue)
  • OAuth role validation — validates GOOGLE_AUTO_CREATE_ROLE env var against User::ROLES, falls back to admin with warning log if invalid; adds info log on user creation
  • Dockerfile HEALTHCHECK — adds HEALTHCHECK using the existing /up endpoint for container orchestration

Assessed and skipped

Finding Reason
innerHTML XSS (markdown_editor.js, dashboard_dropzone.js) Hardcoded SVG constant + server-rendered data attribute — no user input
CORS Access-Control-Allow-Origin: * By design — required for iframe embed of submit forms
Sidekiq Web UI auth Already gated by authenticated :user, ->(u) { u.sidekiq? } (admin only)

Test plan

  • Verify SMS send works normally, returns rate limit error after 10 sends/hr
  • Verify app boots with and without REDIS_URL set
  • Verify OAuth auto-create with valid and invalid GOOGLE_AUTO_CREATE_ROLE
  • Verify Docker HEALTHCHECK passes after container start
  • bundle exec rspec — no regressions

🤖 Generated with Claude Code


Note

Medium Risk
Moderate risk because it changes request-path behavior (SMS sending now rate-limited) and alters caching/initializer error handling/timeouts, which could affect runtime behavior under load or during outages.

Overview
Adds per-user rate limiting to the SMS invitation endpoint (SubmittersSendSmsController) and returns a friendly error when the limit is exceeded.

Hardens RateLimit by configuring Redis cache timeouts/reconnect attempts and by falling back to in-memory storage when Redis/gems fail to load; similarly broadens ActiveStorage initializer rescue to include LoadError.

Validates GOOGLE_AUTO_CREATE_ROLE against User::ROLES (with warning + fallback) and adds an info log on OAuth auto-created users, and adds a Docker HEALTHCHECK against /up for orchestration readiness.

Written by Cursor Bugbot for commit 41ccd3b. Configure here.

- Add rate limiting (10/hr per user) to SMS send endpoint to prevent Twilio abuse
- Add Redis timeout config (connect: 2s, read/write: 1s) to prevent hung requests
- Widen rescue to catch LoadError in rate_limit.rb and active_storage.rb initializer
- Validate OAuth auto-create role against known roles with fallback and logging
- Add HEALTHCHECK to Dockerfile using /up health endpoint

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 28, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch audit-bugfixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dnplkndll
Copy link
Copy Markdown
Author

bugbot run

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant