Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-manifest-setter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes `Cannot set property manifest` error in test utilities by adding a protected setter for the manifest property
4 changes: 3 additions & 1 deletion .changeset/secure-forwarded-host-validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export default defineConfig({

The patterns support wildcards (`*` and `**`) for flexible hostname matching and can optionally specify protocol and port.

### Breaking change
Additionally, this fixes a bug where protocol validation was incorrectly formatted, causing valid `X-Forwarded-Host` headers to be rejected when `allowedDomains` was configured.

__Breaking change__

Previously, `Astro.url` would reflect the value of the `X-Forwarded-Host` header. While this header is commonly used by reverse proxies like Nginx to communicate the original host, it can be sent by any client, potentially allowing malicious actors to poison caches with incorrect URLs.

Expand Down
6 changes: 5 additions & 1 deletion packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ export class App {
return this.#manifest;
}

protected set manifest(value: SSRManifest) {
this.#manifest = value;
}

protected matchesAllowedDomains(forwardedHost: string, protocol?: string): boolean {
return App.validateForwardedHost(forwardedHost, this.#manifest.allowedDomains, protocol);
}
Expand Down Expand Up @@ -280,7 +284,7 @@ export class App {
}

// Validate X-Forwarded-Host against allowedDomains if configured
if (forwardedHost && !this.matchesAllowedDomains(forwardedHost, protocol)) {
if (forwardedHost && !this.matchesAllowedDomains(forwardedHost, protocol?.replace(':', ''))) {
Copy link
Member

Choose a reason for hiding this comment

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

What's this replace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the host header includes a port, ala example.com:8080

Copy link
Member

Choose a reason for hiding this comment

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

It's when the protocol includes a : actually, I just noticed the code above, so https: -> https, got it

// If not allowed, ignore the X-Forwarded-Host header
forwardedHost = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,11 @@ export default defineConfig({
}
},
site: "https://example.com",
security: {
allowedDomains: [
{ hostname: 'example.pt' },
{ hostname: 'it.example.com' },
{ hostname: 'example.com' }
]
}
})
7 changes: 7 additions & 0 deletions packages/astro/test/i18n-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2140,6 +2140,13 @@ describe('i18n routing does not break assets and endpoints', () => {
root: './fixtures/i18n-routing-subdomain/',
output: 'server',
adapter: testAdapter(),
security: {
allowedDomains: [
{ hostname: 'example.pt' },
{ hostname: 'it.example.com' },
{ hostname: 'example.com' }
]
}
});
await fixture.build();
app = await fixture.loadTestAdapterApp();
Expand Down
43 changes: 26 additions & 17 deletions packages/astro/test/units/app/node.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,28 @@ describe('NodeApp', () => {

describe('x-forwarded-host', () => {
it('parses host from single-value x-forwarded-host header', () => {
const result = NodeApp.createRequest({
...mockNodeRequest,
headers: {
'x-forwarded-host': 'www2.example.com',
const result = NodeApp.createRequest(
{
...mockNodeRequest,
headers: {
'x-forwarded-host': 'www2.example.com',
},
},
});
{ allowedDomains: [{ hostname: '**.example.com' }] }
);
assert.equal(result.url, 'https://www2.example.com/');
});

it('parses host from multi-value x-forwarded-host header', () => {
const result = NodeApp.createRequest({
...mockNodeRequest,
headers: {
'x-forwarded-host': 'www2.example.com,www3.example.com',
const result = NodeApp.createRequest(
{
...mockNodeRequest,
headers: {
'x-forwarded-host': 'www2.example.com,www3.example.com',
},
},
});
{ allowedDomains: [{ hostname: '**.example.com' }] }
);
assert.equal(result.url, 'https://www2.example.com/');
});

Expand Down Expand Up @@ -171,14 +177,17 @@ describe('NodeApp', () => {
});

it('prefers port from x-forwarded-host', () => {
const result = NodeApp.createRequest({
...mockNodeRequest,
headers: {
host: 'example.com:443',
'x-forwarded-host': 'example.com:3000',
'x-forwarded-port': '443',
const result = NodeApp.createRequest(
{
...mockNodeRequest,
headers: {
host: 'example.com:443',
'x-forwarded-host': 'example.com:3000',
'x-forwarded-port': '443',
},
},
});
{ allowedDomains: [{ hostname: 'example.com' }] }
);
assert.equal(result.url, 'https://example.com:3000/');
});
});
Expand Down
Loading