Skip to content

Commit 978e6d3

Browse files
qzioqzio
committed
more tests and tweaks
1 parent 7a57fa1 commit 978e6d3

File tree

2 files changed

+163
-29
lines changed

2 files changed

+163
-29
lines changed

‎packages/astro/src/core/app/validate-headers.ts‎

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export function validateForwardedHeaders(
9090
return result
9191
}
9292

93-
let allowedDomain: RemotePattern = {}
93+
// require a hostname
94+
if (!forwardedHost || forwardedHost.length === 0) {
95+
return result;
96+
}
9497

9598
// Validate host (extract port from hostname for validation)
9699
// Reject empty strings and sanitize to prevent path injection
@@ -103,40 +106,46 @@ export function validateForwardedHeaders(
103106
// do not match anything other than hostname here.
104107
const found = allowedDomains.find((pattern) => matchHostname(testUrl, pattern.hostname, true));
105108
if (found) {
106-
// also check for ports in the forwarded header, if there is a mismatch, return
107-
if (port) {
108-
if (found.port && found.port !== port) {
109-
return result;
109+
// we might need to set the port...
110+
if (forwardedPort || port) {
111+
if (found.port) {
112+
113+
if (port) {
114+
if (forwardedPort && forwardedPort !== port) {
115+
// weird case.
116+
return result;
117+
}
118+
if (found.port === port) {
119+
result.port = port;
120+
}
121+
} else {
122+
if (found.port === forwardedPort) {
123+
result.port = forwardedPort;
124+
}
125+
}
126+
} else {
127+
// If no port patterns, reject the header (strict security default)
110128
}
111129
}
112-
result.host = sanitized;
113-
allowedDomain = found
114-
}
115-
}
116-
}
117-
}
118130

119-
// If host is not valid, we cannot trust protocol or port from forwarded headers
120-
if (!result.host || !allowedDomain.hostname) {
121-
return result;
122-
}
131+
// if a protocol is configured, ensure it is correct.
132+
if (found.protocol) {
133+
if (found.protocol !== forwardedProtocol) {
134+
return result;
135+
}
123136

124-
// Validate port
125-
if (forwardedPort && allowedDomain.port && allowedDomain.port === forwardedPort) {
126-
result.port = forwardedPort;
127-
}
137+
// all good
138+
result.protocol = found.protocol;
139+
} else if (forwardedProtocol && /^https?$/.test(forwardedProtocol)) {
140+
// fallback to allow if it is not specified in the allowedDomains, but only if it is http or https
141+
result.protocol = forwardedProtocol;
142+
}
128143

129-
// Validate protocol
130-
if (forwardedProtocol) {
131-
if (allowedDomain.protocol) {
132-
if (allowedDomain.protocol === forwardedProtocol) {
133-
result.protocol = forwardedProtocol;
144+
result.host = sanitized;
145+
return result
146+
}
134147
}
135-
} else if (/^https?$/.test(forwardedProtocol)) {
136-
// allowdDomains does not contain protocol, allow protocol
137-
result.protocol = forwardedProtocol;
138148
}
139149
}
140-
141-
return result;
150+
return result
142151
}

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,131 @@ describe('node', () => {
439439
);
440440
assert.equal(result.url, 'https://example.com/');
441441
});
442+
it('accepts x-forwarded-proto when allowedDomains has protocol and hostname', () => {
443+
const result = createRequest(
444+
{
445+
...mockNodeRequest,
446+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
447+
headers: {
448+
host: 'myapp.example.com',
449+
'x-forwarded-host': 'myapp.example.com',
450+
'x-forwarded-proto': 'https',
451+
},
452+
},
453+
{ allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] },
454+
);
455+
// Without the fix, protocol validation fails due to hostname mismatch
456+
// and falls back to socket.encrypted (false → http)
457+
assert.equal(result.url, 'https://myapp.example.com/');
458+
});
459+
460+
it('rejects x-forwarded-proto when it does not match protocol in allowedDomains', () => {
461+
const result = createRequest(
462+
{
463+
...mockNodeRequest,
464+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
465+
headers: {
466+
host: 'myapp.example.com',
467+
'x-forwarded-proto': 'http',
468+
},
469+
},
470+
{ allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] },
471+
);
472+
// http is not in allowedDomains (only https), protocol falls back to socket (false → http)
473+
// Host validation also fails because http doesn't match the pattern's protocol: 'https'
474+
assert.equal(result.url, 'http://localhost/');
475+
});
476+
477+
it('accepts x-forwarded-proto with wildcard hostname pattern in allowedDomains', () => {
478+
const result = createRequest(
479+
{
480+
...mockNodeRequest,
481+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
482+
headers: {
483+
'x-forwarded-host': 'joel.example.com',
484+
'x-forwarded-proto': 'https',
485+
},
486+
},
487+
{ allowedDomains: [{ protocol: 'https', hostname: '**.example.com' }] },
488+
);
489+
assert.equal(result.url, 'https://joel.example.com/');
490+
});
491+
492+
it('constructs correct URL behind reverse proxy with all forwarded headers', () => {
493+
// Simulates: Reverse proxy terminates TLS, connects to Astro via HTTP,
494+
// forwards original protocol/host/port via X-Forwarded-* headers
495+
const result = createRequest(
496+
{
497+
...mockNodeRequest,
498+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
499+
headers: {
500+
host: 'myapp.example.com',
501+
'x-forwarded-proto': 'https',
502+
'x-forwarded-host': 'myapp.example.com',
503+
},
504+
},
505+
{ allowedDomains: [{ protocol: 'https', hostname: 'myapp.example.com' }] },
506+
);
507+
assert.equal(result.url, 'https://myapp.example.com/');
508+
});
509+
510+
it('hmmm', () => {
511+
const result = createRequest(
512+
{
513+
...mockNodeRequest,
514+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
515+
headers: {
516+
host: 'something-other.com',
517+
'x-forwarded-proto': 'https',
518+
'x-forwarded-host': 'myapp.example.com',
519+
},
520+
},
521+
{ allowedDomains: [{ protocol: 'https', hostname: 'another.example.com' }] },
522+
);
523+
// should be 'http://something-other.com/' no?
524+
assert.equal(result.url, 'http://localhost/');
525+
});
526+
527+
it('trick validate-headers', () => {
528+
const result = createRequest(
529+
{
530+
...mockNodeRequest,
531+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
532+
headers: {
533+
host: 'this-header-does-nothing.com',
534+
'x-forwarded-host': 'myapp.example.com',
535+
},
536+
},
537+
{
538+
allowedDomains: [
539+
{ protocol: 'http', hostname: 'local.domain' },
540+
{ protocol: 'https', hostname: 'myapp.example.com' }
541+
]
542+
},
543+
);
544+
545+
assert.equal(result.url, 'http://localhost/');
546+
});
547+
548+
it('trick validate-headers 2', () => {
549+
const result = createRequest(
550+
{
551+
...mockNodeRequest,
552+
socket: { encrypted: false, remoteAddress: '2.2.2.2' },
553+
headers: {
554+
'x-forwarded-proto': 'https',
555+
},
556+
},
557+
{
558+
allowedDomains: [
559+
{ protocol: 'https', hostname: 'myapp.example.com' }
560+
]
561+
},
562+
);
563+
564+
assert.equal(result.url, 'http://localhost/');
565+
});
566+
442567
});
443568

444569
describe('x-forwarded-port', () => {

0 commit comments

Comments
 (0)