fix: use request.headers.host (with port) in buildResourceUrl#251
Conversation
Fastify's request.hostname strips the port, so ACL matching fails on non-standard ports. The constructed URL http://localhost/path never matches the ACL's http://localhost:4443/path. Use request.headers.host which includes the port. Fixes #250
There was a problem hiding this comment.
Pull request overview
Fixes ACL matching on non-standard ports by ensuring URLs constructed for WAC checks include the request port (e.g., localhost:4443 instead of localhost).
Changes:
- Update
buildResourceUrlto preferrequest.headers.host(includes port) overrequest.hostname(strips port). - Update the HTML error page’s
baseUrlconstruction to use host-with-port as well.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use request.headers.host (includes port) instead of request.hostname (strips port) | ||
| const host = request.headers.host || request.hostname; | ||
| if (request.subdomainsEnabled && request.baseDomain && | ||
| request.hostname === request.baseDomain && !request.podName) { |
There was a problem hiding this comment.
buildResourceUrl is used for WAC authorization decisions, but this change switches to request.headers.host directly. The Host header (and with trustProxy: true, proxy-provided host headers) is client-influenced and can be spoofed, which risks incorrect ACL matching across virtual hosts. Consider deriving the authority from Fastify’s trusted properties and only appending a validated port (or validating that headers.host matches request.hostname), and include x-forwarded-host when running behind a proxy (consistent with src/ap/index.js:70-78).
| @@ -33,7 +35,7 @@ function buildResourceUrl(request, urlPath) { | |||
| return `${request.protocol}://${podName}.${request.baseDomain}${remainder}`; | |||
| } | |||
There was a problem hiding this comment.
In the subdomain-normalization branch, the returned URL omits any non-standard port even after introducing host with port. If the server runs subdomain mode on a non-default port, ACL URLs that include the port will still not match. Consider preserving the port when building ${podName}.${baseDomain} (e.g., by extracting the port from the incoming authority and appending it).
| const baseUrl = `${request.protocol}://${request.headers.host || request.hostname}`; | ||
|
|
There was a problem hiding this comment.
baseUrl is built using request.headers.host and then interpolated into HTML attributes (e.g., href="${baseUrl}/"). Because Host is user-controlled, this can enable reflected HTML injection/XSS if the header contains characters like quotes. Prefer a sanitized/validated authority (or escape before injecting into HTML) rather than using the raw header value.
Summary
buildResourceUrlusedrequest.hostnamewhich strips the port. ACL matching failed on non-standard ports because the constructed URLhttp://localhost/pathnever matched the ACL'shttp://localhost:4443/path.Fix: use
request.headers.hostwhich includes the port.Tested
Full 402 flow verified:
353 tests pass.
Fixes #250