Skip to content

Harden Astro.clientAddress when using from Node adapter#15742

Merged
matthewp merged 2 commits intomainfrom
node-ip-addr
Mar 4, 2026
Merged

Harden Astro.clientAddress when using from Node adapter#15742
matthewp merged 2 commits intomainfrom
node-ip-addr

Conversation

@matthewp
Copy link
Contributor

@matthewp matthewp commented Mar 3, 2026

Changes

  • X-Forwarded-For is now gated by security.allowedDomains, consistent with how X-Forwarded-Host, X-Forwarded-Proto, and X-Forwarded-Port are already handled.

Testing

Added and updated unit tests in packages/astro/test/units/app/node.test.js

Docs

N/A, bug fix

@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: 8e01ece

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Mar 3, 2026
@matthewp matthewp changed the title Harden X-Forwarded-For to respect allowedDomains for clientAddress Harden Astro.clientAddress when using from Node adapter Mar 3, 2026
@matthewp matthewp marked this pull request as ready for review March 3, 2026 21:01
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 3, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing node-ip-addr (8e01ece) with main (918d394)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (20b05c0) during the generation of this report, so 918d394 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

There are some tests that need updating.

Also a bit concerned... isn't this, technically, a breaking change? But I don't have strong feelings. However, maybe we should update the docs https://docs.astro.build/en/reference/adapter-reference/#clientaddress

We should mention that clientAddress values must match values inside allowedDomains

@matthewp
Copy link
Contributor Author

matthewp commented Mar 4, 2026

@ematipico Yes breaking, but in line with the other enforcements over X-Forwarded- headers, this was just one that was missing. This is enforced for X-Forwarded-Host and X-Forwarded-Port. If you have X-Forwarded-For then you have those headers too (behind a proxy) and are already using allowedDomain otherwise your Astro.url is missing the domain, so I don't think this realistically breaks for many people. And it's in a major anyways.

@matthewp matthewp merged commit 9d9699c into main Mar 4, 2026
26 checks passed
@matthewp matthewp deleted the node-ip-addr branch March 4, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants