Skip to content

Allow puma 7.2.x in the service bundler group - #11228

Merged
ehelms merged 1 commit into
theforeman:developfrom
log0u7:raise-puma-constraint
Sep 11, 2026
Merged

Allow puma 7.2.x in the service bundler group#11228
ehelms merged 1 commit into
theforeman:developfrom
log0u7:raise-puma-constraint

Conversation

@log0u7

@log0u7 log0u7 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Problem

puma >= 5.5 and < 7.2.1 is affected by the PROXY protocol v1 vulnerabilities CVE-2026-47736 and CVE-2026-47737 (crafted PROXY protocol headers can cause request injection on keep-alive connections and DoS). The current constraint >= 5.1, '< 7' keeps every affected release in the allowed range and blocks the fixed ones (the first patched release above 6.6.1 is 7.2.1).

Fix

Raise the constraint in bundler.d/service.rb to >= 7.2.1', '< 8'.

Verification

Verified against Foreman 5.0.0-rc2 sources on a CentOS Stream 10 container with puma 7.2.1 and nokogiri 1.19.4: the application boots, migrates, seeds, and serves - users/login returns 200 and /api/v2/ping returns 200. The full source-built image is exercised continuously in our CI (built, smoke-tested and scanned on every commit).

Note: concurrent-ruby cannot follow in the same move: dynflow pins ~> 1.1.3 and concurrent-ruby-edge ~> 0.6.0 (see the companion issue on Dynflow/dynflow proposing that relaxation).

@log0u7
log0u7 force-pushed the raise-puma-constraint branch 2 times, most recently from d38a566 to d2d53e5 Compare September 6, 2026 23:33
@log0u7
log0u7 marked this pull request as ready for review September 6, 2026 23:33
@log0u7
log0u7 requested a review from a team as a code owner September 6, 2026 23:33
@log0u7
log0u7 force-pushed the raise-puma-constraint branch from d2d53e5 to c60f225 Compare September 6, 2026 23:47
@log0u7

log0u7 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

The test (13, *, 22) JavaScript failures on this PR are the known time-of-day flake in DateTimePicker.test.js (does not show error when current date with future time is entered with isFutureOnly).

Evidence: identical commits passed the full JavaScript workflow at 22:55 UTC (raise-puma-constraint, docs) and only started failing in the 23:35-23:51 UTC window. The PR changes a Ruby bundler constraint and an asciidoc file respectively - neither touches the frontend.

Happy to re-trigger after the flake window; a re-run from a maintainer would also do it.

@log0u7

log0u7 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

CI status summary - none of the three failure families touch this PR's changes:

  1. JavaScript DateTimePicker.test.js (isFutureOnly): time-of-day flake. Identical commits passed at 22:55 UTC (raise-puma: run 34067109675, docs: run 34067110161) and only fail in the 23:35-23:51 UTC window (3 consecutive runs).

  2. Katello ContentHostsRoutingTest (10 failures): all share Errno::ENOENT - test/fixtures/hosts.yml in before_setup - the fixture generation step of the CI job, unrelated to a bundler constraint change.

  3. Integration HostgroupJSTest#test_0002: Timeout::Error after 3 MinitestRetry attempts - the same webpack:compile test:integration apipie:cache job is failing on develop itself since Sep 4 (run 33853935145).

A re-run after the midnight UTC window would likely go green; happy to re-push or leave it to the maintainers.

@log0u7
log0u7 force-pushed the raise-puma-constraint branch from c60f225 to 1e8a8ba Compare September 7, 2026 00:56
@log0u7

log0u7 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Update: re-triggered after midnight UTC (00:56) - now the mirror test fails: shows error when current date with past time is entered with isFutureOnly. So the whole isFutureOnly test family is date-boundary sensitive around UTC midnight (fails in both directions: 'future time' variant before midnight, 'past time' variant after).

Identical content passed fully at 22:55 UTC. We will re-trigger once more in the morning UTC, away from any boundary; a maintainer re-run is equally welcome.

@log0u7
log0u7 force-pushed the raise-puma-constraint branch from 1e8a8ba to 00138f7 Compare September 7, 2026 08:12
@ehelms

ehelms commented Sep 10, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution @log0u7. I have two questions for you:

  1. My understanding is that Puma 7 renamed hooks and https://github.com/theforeman/foreman/blob/develop/config/puma/production.rb#L56 will break
  2. Did you consider going all the way to Puma 8?

@log0u7

log0u7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review @ehelms! Answers to both questions:

1. on_worker_boot in config/puma/production.rb:

Puma 7 renamed the hook but kept on_worker_boot as a deprecated alias (alias_method :on_worker_boot, :before_worker_boot in puma 7.x dsl.rb, still present in 8.0.2 with the deprecation message). I verified the actual behavior on Foreman 5.0-stable sources with puma 7.2.1:

Use 'before_worker_boot', 'on_worker_boot' is deprecated and will be removed in v8

The dynflow worker initialization in config/puma/production.rb still executes through the alias (the server boots and serves users/login: 200 with your stock production.rb). So puma 7.2.1 does not break Foreman's config, it deprecates it - the rename belongs in the puma 8 migration, not in this CVE-driven constraint raise.

2. Why not puma 8?

I deliberately scoped this PR to the smallest change that unblocks the CVEs: the first patched release above the currently-allowed 6.6.1 is 7.2.1, and the 7.x series keeps the deprecated hooks Foreman's config relies on. Going straight to 8.0.2 would work today (the alias still exists in 8.0.2) but locks the constraint below a major version that will eventually drop the alias, and gives maintainers less room to land a proper before_worker_boot rename in a dedicated PR. If you'd rather go to 8.0.2 directly I can raise the ceiling to < 9 instead - but the safer increment is what this PR ships. Happy to adjust if you prefer the bigger jump.

@ehelms

ehelms commented Sep 10, 2026

Copy link
Copy Markdown
Member

I appreciate aiming for smallest diff possible. I will also point out the two CVEs in question, Foreman is not vulnerable to as they require setting set_remote_address proxy_protocol: :v1 explicitly and our puma config does not do that.

If we are going to take the time to upgrade Puma (which we should), then we should address the deprecations rather than leaving them hanging around to find later. I would feel different about this if we were leaving the lower bound of Puma but raising the upper bound.

Did you consider that idea? Raising the upper bound but keeping the lower bound to allow a smoother update to packaging?

@log0u7
log0u7 force-pushed the raise-puma-constraint branch from 00138f7 to 1298417 Compare September 10, 2026 11:21
@log0u7

log0u7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Fair point on reachability: stock Foreman does not arm PROXY protocol parsing - remote_address defaults to :socket and set_remote_address 'proxy_protocol: :v1' is not configured - so neither CVE applies to the default deployment. CVE-2026-47736's unbounded pre-parse buffering only runs when expect_proxy_proto is set.

One nuance worth recording though: CVE-2026-47737's keep-alive re-parse means the spoofing surface appears as soon as any deployment puts Puma behind a send-proxy upstream (HAProxy / K8s ingress setups do exactly that to preserve client IPs). At that point a second PROXY header injected between keep-alive requests rewrites remote_addr and defeats any IP-based allowlist or rate limiting - the kind of post-config-change surprise worth closing proactively.

Agreed on >= 5.1, < 9 - updated to keep the lower bound and raise the ceiling, so packaging can advance on its own schedule while source builds resolve to the patched releases. The on_worker_boot deprecation cleanup can land separately whenever packaging moves to 7+.

puma >= 5.5 and < 7.2.1 is affected by the PROXY protocol v1
vulnerabilities CVE-2026-47736 and CVE-2026-47737 (crafted PROXY
protocol headers can cause request injection on keep-alive connections
and unauthenticated memory exhaustion when PROXY protocol v1 parsing is
armed). The first patched release above 6.6.1 is 7.2.1, so the previous
'< 7' ceiling kept every affected version in the allowed range and left
no resolver path to the fixes.

Raise the ceiling to '< 9' while keeping the '>= 5.1' lower bound, so
packaging (RPMs ship puma 6.6.1 today) can advance on its own schedule
while source builds resolve to the patched releases.

Reachability note (from the PR discussion): stock Foreman never arms
PROXY protocol parsing - remote_address defaults to :socket and
set_remote_address 'proxy_protocol: :v1' is not configured - so neither
CVE applies to the default deployment. The raise is still worthwhile:
CVE-2026-47737 becomes reachable the moment any deployment puts Puma
behind a send-proxy upstream (HAProxy/K8s ingress setups), as the
keep-alive re-parse lets a second PROXY header rewrite remote_addr and
defeat IP-based allowlists and rate limiting.

Verified against Foreman 5.0.0-rc2 sources: with puma 7.2.1 the
application boots, migrates, seeds and serves (users/login 200,
/api/v2/ping 200); config/puma/production.rb's on_worker_boot still
runs through the deprecated alias that puma 7 and 8 keep.
@log0u7
log0u7 force-pushed the raise-puma-constraint branch from 1298417 to c07ec76 Compare September 10, 2026 11:25
@log0u7

log0u7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

CI status on the >= 5.1, < 9 push (commit c07ec76): everything green except Katello / test:katello with a single failure:

Failure: Katello::Host::RepublishRepositoriesTest::Version Republish Repositories#test_0001_plans with default values

The trace shows the action planned is Actions::Katello::Repository::BulkMetadataGenerate where the test expects the republish action - a dynflow planning/mocking mismatch inside Katello's test suite, not something this PR touches (it only changes a version ceiling in bundler.d/service.rb, outside the katello bundler group).

Note this Katello job also failed on PR #38932 yesterday (run 34463960913, same workflow), and the previous run of this PR was fully green (36/36) on content identical except the version bounds. Happy to re-trigger.

@ehelms

ehelms commented Sep 10, 2026

Copy link
Copy Markdown
Member

I agree it appears not to have anything to do with this change. I opened a PR with an attempted fix.

@ehelms
ehelms merged commit 39bc86e into theforeman:develop Sep 11, 2026
60 of 63 checks passed
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