[mgruner]

What this PR does / why we need it

Supersedes #510. After updating to Zammad 7.2.3, deployments behind a proxy that sets a Forwarded header answer with 500 errors:

ActionDispatch::RemoteIp::IpSpoofAttackError: IP spoofing attack?! HTTP_CLIENT_IP="10.135.14.4" HTTP_X_FORWARDED_FOR="10.166.3.27, 100.64.0.11, 10.135.14.4" HTTP_FORWARDED="for=100.64.0.11;host=…;proto=https"

Cause: Zammad 7.2.3 updated Rails from 8.0 to 8.1.4.1. Rails 8.1 RemoteIp uses the RFC 7239 Forwarded header before X-Forwarded-For, and 8.1.4.1 additionally raises if both headers name different clients. The chart's nginx sets CLIENT_IP and appends to X-Forwarded-For, but passes an outer Forwarded header through untouched, so Rails sees two conflicting hop chains.

Zammad already solved this in its own nginx and Apache configurations (since 7.2.0, #6305 in zammad/zammad): its reverse proxy configuration is authoritative, so Forwarded, X-Forwarded-Scheme and X-Forwarded-Ssl are cleared. The Docker images pick that up automatically, but the chart has its own nginx configuration and never got this change. The same is true for X-Forwarded-User and Remote-User. Zammad clears both on /, because /auth/sso takes the login from them. The chart passed both through, so a client could set its own SSO login if SSO is enabled and the ingress doesn't strip them.

This PR applies the same restrictions in the chart, so the default configuration is safe without any admin action:

Header	/	/cable	/ws
Forwarded	cleared	cleared	cleared
X-Forwarded-Scheme	cleared	cleared	cleared
X-Forwarded-Ssl	cleared	cleared	cleared
X-Forwarded-User	cleared	–	–
Remote-User	cleared	–	–

• The request scheme now comes only from X-Forwarded-Proto, which the ingress sets.
• New zammadConfig.nginx.scheme (""/http/https, like NGINX_SERVER_SCHEME in the Docker image): empty keeps the ingress's X-Forwarded-Proto, https sets it in all locations. Needed if TLS ends at a proxy in front of the ingress, which previously reported HTTPS only via Forwarded; otherwise login fails with "CSRF token verification failed!".
• extraHeaders, websocketExtraHeaders and the new cableExtraHeaders (taken from set proxy_set_header Forwarded "" #510) are rendered after the defaults. nginx skips an empty proxy_set_header, so an SSO setup can deliberately pass on its login header again, e.g. extraHeaders: ['X-Forwarded-User $http_x_forwarded_user'].

Which issue this PR fixes

• Supersedes set proxy_set_header Forwarded "" #510, which made the fix opt-in via extraHeaders / cableExtraHeaders. Please close set proxy_set_header Forwarded "" #510 when this is merged; closing keywords don't apply to PRs.

Special notes for your reviewer

• Major version bump to 22.0.0: SSO setups that relied on the ingress passing X-Forwarded-User / Remote-User through, and proxies that report HTTPS only via Forwarded / X-Forwarded-Ssl, need a values change. See the upgrade notes.
• Tested by rendering the ConfigMap and running it in nginx:stable (nginx -t passes). Requests sending all of the above headers were checked against an echo backend. With default values, / passes none of them on, and X-Forwarded-Proto stays. Headers added through extraHeaders / cableExtraHeaders are passed on again.

Checklist

• Chart Version bumped
• Upgrading instructions are documented in the zammad/README.md

Summary by CodeRabbit

• New Features

  • Added configurable extra headers for WebSocket and cable connections.
  • Added an option to set the request scheme passed to Zammad.

• Upgrade Notes

  • Chart version 22.0.0 changes forwarded-header handling: selected headers are cleared, and the request scheme is taken from X-Forwarded-Proto unless configured otherwise. On /, SSO-related headers are cleared before configured extra headers are applied.
  • When upgrading from 21.x, configure your proxy to set X-Forwarded-Proto and overwrite any SSO headers it passes through.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The chart version changes from 21.1.3 to 22.0.0. Nginx clears selected forwarded headers in the /ws, /cable, and / locations before applying configured extra headers. The / location also clears X-Forwarded-User and Remote-User. The chart adds nginx.scheme and cableExtraHeaders values and schema entries. The README documents the header behavior and proxy configuration for upgrades from chart versions 21.x.

Possibly related issues

• Issue 513 describes missing forwarded-header clearing in the chart nginx configuration. This PR adds that clearing, but does not implement the issue’s proposed entrypoint-based configuration.

Possibly related PRs

• zammad/zammad-helm#510: Proposes clearing the Forwarded header to address the same proxy-header behavior.

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 037ba

The change clears untrusted proxy headers by default and documents the upgrade impact for SSO and TLS-terminating proxies. No actionable merge-blocking risk is evident.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "Clear untrusted proxy headers in the chart's nginx" matches the main change. The changeset adds clearing of Forwarded, X-Forwarded-Scheme, X-Forwarded-Ssl, X-Forwarded-User, and Remote-User…
Linked Issues check		The only linked issue, #510, is closed and completed. It is marked as historical context only. No active directly linked target remains, so no linked-issue coding requirements apply to this PR. The PR…
Out of Scope Changes check		The changes match the PR's stated objectives. The configmap-nginx.yaml changes clear Forwarded, X-Forwarded-Scheme, and X-Forwarded-Ssl on /, /cable, and /ws. They clear X-Forwarded-User and Remote-Us…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Title check

Explanation

The title "Clear untrusted proxy headers in the chart's nginx" matches the main change. The changeset adds clearing of Forwarded, X-Forwarded-Scheme, X-Forwarded-Ssl, X-Forwarded-User, and Remote-User in the chart's nginx ConfigMap for /, /cable, and /ws. The title is specific and concise, and it names the primary behavior change without listing files.

Full details: Linked Issues check

Explanation

The only linked issue, #510, is closed and completed. It is marked as historical context only. No active directly linked target remains, so no linked-issue coding requirements apply to this PR. The PR description and the PR objectives state that this PR supersedes #510. The current PR goal is broader: clear untrusted proxy headers in the chart's nginx configuration. This goal is assessed from the PR description and objectives, not from #510's acceptance criteria. The PR summary reports rendering and an nginx -t check, but these were not re-run in this assessment.

Full details: Out of Scope Changes check

Explanation

The changes match the PR's stated objectives. The configmap-nginx.yaml changes clear Forwarded, X-Forwarded-Scheme, and X-Forwarded-Ssl on /, /cable, and /ws. They clear X-Forwarded-User and Remote-User on /. They set X-Forwarded-Proto only from zammadConfig.nginx.scheme, and add cableExtraHeaders after the defaults. The values.yaml, values.schema.json, and README.md changes document and validate these settings. The Chart.yaml bump to 22.0.0 is needed for the breaking header behavior, and the README gives the upgrade notes. These changes support the same goal and are not unrelated. Their correctness depends on the nginx rendering checks described in the PR, which this assessment did not re-run.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: fb9c3865-d2d1-4758-a1b8-2e1904f0f358

Commits

Reviewing files that changed from the base of the PR and between d85043b and 037ba1e.

Files selected for processing: 4

• zammad/README.md
• zammad/templates/configmap-nginx.yaml
• zammad/values.schema.json
• zammad/values.yaml

Files skipped from review as they are similar to previous changes: 1

• zammad/README.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.