Skip to content

Commit 28b2a1d

Browse files
authored
Fix failing x-forwarded-host tests (#14505)
* Fix failing x-forwarded-host tests * allow setting the manifest * fix other test and add changeset * fix another thing * h4 * set it back * replace heading with bold since CI complains
1 parent ec307b0 commit 28b2a1d

File tree

6 files changed

+53
-19
lines changed

6 files changed

+53
-19
lines changed

.changeset/fix-manifest-setter.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes `Cannot set property manifest` error in test utilities by adding a protected setter for the manifest property

.changeset/secure-forwarded-host-validation.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ export default defineConfig({
2525

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

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

3032
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.
3133

packages/astro/src/core/app/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ export class App {
146146
return this.#manifest;
147147
}
148148

149+
protected set manifest(value: SSRManifest) {
150+
this.#manifest = value;
151+
}
152+
149153
protected matchesAllowedDomains(forwardedHost: string, protocol?: string): boolean {
150154
return App.validateForwardedHost(forwardedHost, this.#manifest.allowedDomains, protocol);
151155
}
@@ -280,7 +284,7 @@ export class App {
280284
}
281285

282286
// Validate X-Forwarded-Host against allowedDomains if configured
283-
if (forwardedHost && !this.matchesAllowedDomains(forwardedHost, protocol)) {
287+
if (forwardedHost && !this.matchesAllowedDomains(forwardedHost, protocol?.replace(':', ''))) {
284288
// If not allowed, ignore the X-Forwarded-Host header
285289
forwardedHost = null;
286290
}

packages/astro/test/fixtures/i18n-routing-subdomain/astro.config.mjs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,11 @@ export default defineConfig({
1818
}
1919
},
2020
site: "https://example.com",
21+
security: {
22+
allowedDomains: [
23+
{ hostname: 'example.pt' },
24+
{ hostname: 'it.example.com' },
25+
{ hostname: 'example.com' }
26+
]
27+
}
2128
})

packages/astro/test/i18n-routing.test.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2140,6 +2140,13 @@ describe('i18n routing does not break assets and endpoints', () => {
21402140
root: './fixtures/i18n-routing-subdomain/',
21412141
output: 'server',
21422142
adapter: testAdapter(),
2143+
security: {
2144+
allowedDomains: [
2145+
{ hostname: 'example.pt' },
2146+
{ hostname: 'it.example.com' },
2147+
{ hostname: 'example.com' }
2148+
]
2149+
}
21432150
});
21442151
await fixture.build();
21452152
app = await fixture.loadTestAdapterApp();

packages/astro/test/units/app/node.test.js

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,28 @@ describe('NodeApp', () => {
5959

6060
describe('x-forwarded-host', () => {
6161
it('parses host from single-value x-forwarded-host header', () => {
62-
const result = NodeApp.createRequest({
63-
...mockNodeRequest,
64-
headers: {
65-
'x-forwarded-host': 'www2.example.com',
62+
const result = NodeApp.createRequest(
63+
{
64+
...mockNodeRequest,
65+
headers: {
66+
'x-forwarded-host': 'www2.example.com',
67+
},
6668
},
67-
});
69+
{ allowedDomains: [{ hostname: '**.example.com' }] }
70+
);
6871
assert.equal(result.url, 'https://www2.example.com/');
6972
});
7073

7174
it('parses host from multi-value x-forwarded-host header', () => {
72-
const result = NodeApp.createRequest({
73-
...mockNodeRequest,
74-
headers: {
75-
'x-forwarded-host': 'www2.example.com,www3.example.com',
75+
const result = NodeApp.createRequest(
76+
{
77+
...mockNodeRequest,
78+
headers: {
79+
'x-forwarded-host': 'www2.example.com,www3.example.com',
80+
},
7681
},
77-
});
82+
{ allowedDomains: [{ hostname: '**.example.com' }] }
83+
);
7884
assert.equal(result.url, 'https://www2.example.com/');
7985
});
8086

@@ -171,14 +177,17 @@ describe('NodeApp', () => {
171177
});
172178

173179
it('prefers port from x-forwarded-host', () => {
174-
const result = NodeApp.createRequest({
175-
...mockNodeRequest,
176-
headers: {
177-
host: 'example.com:443',
178-
'x-forwarded-host': 'example.com:3000',
179-
'x-forwarded-port': '443',
180+
const result = NodeApp.createRequest(
181+
{
182+
...mockNodeRequest,
183+
headers: {
184+
host: 'example.com:443',
185+
'x-forwarded-host': 'example.com:3000',
186+
'x-forwarded-port': '443',
187+
},
180188
},
181-
});
189+
{ allowedDomains: [{ hostname: 'example.com' }] }
190+
);
182191
assert.equal(result.url, 'https://example.com:3000/');
183192
});
184193
});

0 commit comments

Comments
 (0)