Skip to content

docs: trust proxy spoke - #2535

Open
jonchurch wants to merge 8 commits into
expressjs:security-docsfrom
jonchurch:trust-proxy
Open

jonchurch wants to merge 8 commits into
expressjs:security-docsfrom
jonchurch:trust-proxy

Conversation

@jonchurch

Copy link
Copy Markdown
Member

This PR introduces the trust proxy spoke page for the security hub.

Suffice to say, I learned just how much I did not understand trust proxy while writing this!

Unrelated to this PR, but I do feel pretty strongly that we should rework this setting slightly in v6, specifically dropping the trust proxy: true option as I am convinced now that it is a footgun. We should also push users to setting an address range and not a hop count for this setting, which I realized very late into the process of writing this documentation, and these docs now reflect that better advice.

This PR also tweaks the two hub items I had previously for trust proxy, in order to better reflect what I have learned developers should ask themselves to determine if they are correctly using this setting, and if they are using it w/ a correctly configured proxy.

The structure I went for here is what I expect to do for the rest of the spokes:

  1. Introduce what Express does, and what goes wrong by default.
  2. One section per hub item, opening with the concrete problem and then the fix, under an anchorable heading
  3. What the setting or API can't do, and what the reader has to do elsewhere.
  4. Related links to the reference material, so the spoke explains the risk and the existing guide stays the reference.

Feedback on structure is highly relevant here!

@jonchurch
jonchurch requested a review from a team as a code owner October 16, 2026 19:46
@netlify

netlify Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 89e2ba5
🔍 Latest deploy log https://app.netlify.com/projects/expressjscom-preview/deploys/6aea4a37aba126cba4ba51a0
😎 Deploy Preview https://deploy-preview-2535--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 96 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@crandmck

crandmck commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

For reviewers' convenience, here are the rendered pages, which aren't actually linked from anywhere in the deploy preview site:

@bjohansebas bjohansebas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just a few minor comments; I’m not an expert on proxies.

@crandmck

crandmck commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

There's a fair amount to unpack here, and I'm going to have to come back to this with more detailed comments.

At a high level, I think we need to review and update the two other places that document trust proxy, linked in the "Related" section:

There is some duplication between these three docs, which is OK... IF there is a good reason for it:

  • The API reference doc should have exhaustive detail of the trust proxy options (does it?), and let's make sure all the examples illustrate good practices as outlined in this doc. Also add a link FROM the API ref section to this new "spoke" page.
  • What is the distinct purpose of "Express behind proxies"? Does it even make sense to keep that page, and if so, why? Let's just be very clear on its goal and then make sure its content speaks to that.

All three of these docs use CIDR notation in examples, with no explanation or references. While many developers may be familiar with it, we should at least say something like "We're assuming you're familiar with CIDR notation used for trust proxy option values" (with xref link). Also the API ref confusingly mixes IPv4 and IPv6 CIDR notation in the explanation of loopback, linklocal, and uniquelocal. This can easily be improved.

Finally, since the trust proxy setting is implemented using the proxy-addr package, for the sake of completeness, I think we should separately review https://github.com/jshttp/proxy-addr#readme to ensure there are no discrepancies with these docs. It's probably fine, but while we're here and this topic is fresh on our minds, it's worth at least a quick review IMO.

@crandmck crandmck left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some clarifications and copy edits

jonchurch and others added 6 commits November 3, 2026 14:55
Co-authored-by: Rand McKinney <crandmck@gmail.com>
Signed-off-by: Jon Church <me@jonchurch.com>
Co-authored-by: Rand McKinney <crandmck@gmail.com>
Signed-off-by: Jon Church <me@jonchurch.com>
better spell out the risk of longer or shorter paths when using count
@jonchurch
jonchurch requested a review from crandmck November 3, 2026 21:31

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants