Skip to content

Configure trusted forwarded header peers - #2629

Open
niemyjski wants to merge 31 commits into
issue/wait-for-saved-view-navigationfrom
issue/trusted-forwarded-headers
Open

niemyjski wants to merge 31 commits into
issue/wait-for-saved-view-navigationfrom
issue/trusted-forwarded-headers

Conversation

@niemyjski

@niemyjski niemyjski commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Forwarded client IPs and schemes were accepted from any connecting peer. Trust only loopback and explicitly configured proxies/networks, validate settings at startup, and retain one-hop processing by default. The existing global-admin echo adds the effective scheme for rollout verification.

Hosted dev and prod explicitly configure the verified dedicated gateway subnet, 10.60.64.0/24, in their environment values. Update those values when the gateway network changes. The reusable chart defaults to no additional trusted networks and rejects conflicting forwarding overrides. There is no subnet discovery script, Azure discovery permission requirement, or per-deployment discovery step.

Migration: self-hosted installations with remote reverse proxies must configure trusted peers before upgrading. Invalid settings, /0 networks, and the root ForwardedHeaders_Enabled shortcut fail startup. Production Helm waits for readiness; it does not automatically roll back.

Validation: both environment Helm lint/render checks pass, including exact trust values, stale-list replacement, conflicting-override rejection, and empty reusable defaults. The explicit values match the earlier live gateway inspection; saved forwarding overrides were absent and the configured route is gateway → Service → application, with no extra proxy. All three CI-tooling tests pass. The unchanged application code previously passed the backend build and 60 focused forwarding, echo authorization, and OpenAPI checks. CI is rerunning for the cleanup.

Rollout: merge prerequisite #2673 first; this PR is temporarily based on that navigation-test fix. Merging this change into main automatically deploys dev. After deployment, follow the configuration, admin-echo baseline, spoof, and direct-untrusted-peer checks on both app/API hosts before production promotion. These runtime checks remain pending; no deployment was performed during verification.

@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from e1c4804 to 07f5b55 Compare October 2, 2026 04:10
@niemyjski
niemyjski added this pull request to stack #2642 October 5, 2026 02:11
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from c2acf5a to 6688148 Compare October 13, 2026 02:39
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch 2 times, most recently from 92223ac to 3dc04bf Compare October 13, 2026 02:58
@niemyjski
niemyjski removed this pull request from stack #2642 October 13, 2026 03:05
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from 3dc04bf to 0e46b92 Compare October 13, 2026 03:05
@niemyjski
niemyjski added this pull request to stack #2659 October 13, 2026 03:05
@niemyjski
niemyjski removed this pull request from stack #2659 October 13, 2026 12:26
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from 9ae1938 to b4a4dc6 Compare October 13, 2026 12:26
@niemyjski
niemyjski added this pull request to stack #2660 October 13, 2026 12:27
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from b4a4dc6 to c876182 Compare October 13, 2026 12:40
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch 2 times, most recently from 8f51fff to 75fae6d Compare October 13, 2026 13:55
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from 75fae6d to 6ff97ff Compare October 14, 2026 02:46
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch 2 times, most recently from 874f431 to b6c99e2 Compare October 14, 2026 03:03
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from b6c99e2 to ae52daf Compare October 14, 2026 03:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d0f73af1c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/build.yaml Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b5091e48f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread k8s/scripts/discover-forwarded-headers.js Outdated
@niemyjski
niemyjski force-pushed the issue/trusted-forwarded-headers branch from 6e1a1ba to 6c3d290 Compare October 16, 2026 20:46
@niemyjski
niemyjski requested a review from ejsmith October 18, 2026 16:02
@ejsmith

ejsmith commented Oct 19, 2026

Copy link
Copy Markdown
Member

Pre-merge verification results for fc93c8910:

  • Fixed the discovery resource name to applicationloadbalancer.alb.networking.azure.io, matching the live cluster, and fixed Windows az.cmd invocation with validated, quoted arguments.
  • Ran both commands from the PR checkout using Node 24.15.0:
node k8s/scripts/discover-forwarded-headers.js ex-dev k8s/ex-dev-values.yaml /tmp/dev-forwarding.json
node k8s/scripts/discover-forwarded-headers.js ex-prod k8s/ex-prod-values.yaml /tmp/prod-forwarding.json

Both exited 0, printed Discovered 1 gateway subnet prefix(es), and generated:

{"forwardedHeaders":{"knownNetworks":["10.60.64.0/24"]}}
  • Subnet/readiness: both environments select kube-system/public-alb. Its Accepted=True and Deployment=True conditions match generation 1. Azure confirms association subnet ex-net-v6/ex-agc is 10.60.64.0/24, delegated to Microsoft.ServiceNetworking/trafficControllers.
  • Configured proxy path: live app/API ingress backends point to their corresponding Services on port 80; EndpointSlices point to app/API pods on port 8080. The pods have no additional proxy containers. Runtime peer/IP/scheme checks still require the post-deployment echo tests.
  • Existing configuration: helm get values succeeded for both releases. Neither has saved forwarding overrides. ForwardedHeaders_Enabled is absent from saved values, current ConfigMaps, and explicit app/API pod environment entries; no cleanup was needed.
  • Generated configuration: both local Helm renders produce EX_ForwardedHeaders__Enabled=true, EX_ForwardedHeaders__ForwardLimit=1, and EX_ForwardedHeaders__KnownNetworks__0=10.60.64.0/24.
  • Local validation: all five CI-tooling tests passed, including two new discovery regression scenarios; focused script lint, Node syntax checks, and diff checks passed. Hosted CI and automated review are still running at this head.

Remaining merge gate: these successful live runs used the existing authenticated Azure user identity, not the deployment service principal. They do not prove that the CI deployment identity has the required reads. Both commands still need successful execution with that identity before merge.

No merge, deployment, or cluster configuration mutation was performed. After dev deployment, the PR's admin-echo baseline, spoof, and authorized direct-backend checks remain required before production promotion.

@ejsmith
ejsmith removed this pull request from stack #2660 October 19, 2026 02:13
@ejsmith
ejsmith changed the base branch from main to issue/wait-for-saved-view-navigation October 19, 2026 02:13
@ejsmith
ejsmith added this pull request to stack #2675 October 19, 2026 02:13
@ejsmith

ejsmith commented Oct 19, 2026

Copy link
Copy Markdown
Member

Cleanup pushed at 9b8a9674f:

Local cleanup verification passed: Helm lint/render for dev and prod, exact forwarding keys, replacement of stale subnet lists, rejection of conflicting overrides, empty reusable defaults, manual self-hosted proxy configuration, and all three CI-tooling tests. The explicit values equal both previously generated discovery files. Frontend validation, all 1,048 unit tests, and the production build also passed on Node 24 for the extracted changes. CI and automated review are running on the updated PRs.

This supersedes the discovery/service-principal permission gate in my earlier verification comment: deployment no longer performs those reads. The earlier Azure-user inspection established configuration/topology, not runtime header behavior. After dev deploys, the documented admin-echo baseline, spoof, and authorized direct-backend checks still must pass before production promotion. Nothing was merged or deployed during this cleanup.

@github-actions

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.AppHost 23% 23% 128 ❌
Exceptionless.Core 78% 69% 10924 ✔
Exceptionless.Insulation 51% 43% 370 ➖
Exceptionless.Web 86% 71% 9237 ✔
Summary 80% (28337 / 35255) 69% (14028 / 20226) 20659 ✔

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants