Skip to content

Move TrackTrueClientIp middleware into saas engine#2677

Merged
flavorjones merged 1 commit into
mainfrom
true-client-ip-option
Mar 9, 2026
Merged

Move TrackTrueClientIp middleware into saas engine#2677
flavorjones merged 1 commit into
mainfrom
true-client-ip-option

Conversation

@flavorjones

@flavorjones flavorjones commented Mar 9, 2026

Copy link
Copy Markdown
Member

Summary

  • The True-Client-IP header is set by Cloudflare and can be spoofed in non-Cloudflare deployments
  • Moved the TrackTrueClientIp middleware and its test into the saas engine so it only loads for Cloudflare-fronted deployments

GHSA-cpch-9qg2-x8fq

Test plan

  • Existing middleware tests pass in saas mode
  • Verify middleware is present in saas mode (SAAS=true BUNDLE_GEMFILE=Gemfile.saas bin/rails middleware)
  • Verify middleware is absent in OSS mode (bin/rails middleware | grep TrueClient)

The True-Client-IP header is set by Cloudflare and is only trustworthy
when behind a Cloudflare proxy. In non-Cloudflare deployments, this
header is attacker-controlled and can be used to spoof IP addresses.

Moving the middleware into the saas engine ensures it only loads for our
Cloudflare-fronted production deployment, not for self-hosted OSS
instances.

GHSA-cpch-9qg2-x8fq
Copilot AI review requested due to automatic review settings March 9, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves the TrackTrueClientIp middleware installation into the fizzy-saas Rails engine so the True-Client-IPX-Forwarded-For behavior is only enabled when the SaaS engine is loaded.

Changes:

  • Adds a SaaS-engine test covering TrackTrueClientIp header mutation behavior.
  • Removes the middleware stack mutation from true_client_ip.rb and installs it via a Rails engine initializer instead.
  • Ensures the middleware code is required by the engine during boot.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
saas/test/lib/true_client_ip_test.rb Adds unit tests for TrackTrueClientIp behavior.
saas/lib/fizzy/saas/true_client_ip.rb Removes direct Rails.application.config.middleware mutation from the library file.
saas/lib/fizzy/saas/engine.rb Requires the middleware and installs it via an engine initializer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +65 to +67
initializer "fizzy_saas.true_client_ip" do |app|
app.config.middleware.insert_before ActionDispatch::RemoteIp, TrackTrueClientIp
end

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

The new engine initializer inserts TrackTrueClientIp into the middleware stack, but the added test only exercises the middleware’s behavior in isolation. Since the main change in this PR is where/when the middleware is installed, consider adding a saas-mode test that asserts Rails.application.middleware includes TrackTrueClientIp before ActionDispatch::RemoteIp (and ideally doesn’t get inserted twice).

Copilot uses AI. Check for mistakes.
@flavorjones flavorjones merged commit b6ea558 into main Mar 9, 2026
16 checks passed
@flavorjones flavorjones deleted the true-client-ip-option branch March 9, 2026 19:14
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.

2 participants