Skip to content
Open
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
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ If you'd like to use [profiler.firefox.com](https://profiler.firefox.com) via UR
FX_PROFILER_HOST="0.0.0.0" yarn start
```

You'll probably also want to add your non-localhost domains to the `allowedHosts` property in `server.js`.
When using `FX_PROFILER_HOST="0.0.0.0"`, any hostname is allowed so you can access the profiler from other devices on your network. If you want to expose only a specific hostname instead, set `FX_PROFILER_HOST` to that hostname directly and it will be added to the allowed hosts automatically.

## Finding something to work on

Expand Down
21 changes: 14 additions & 7 deletions scripts/lib/dev-server.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,27 @@ const EXTRA_HEADERS = {
};

// Allowed hosts for dev server
const ALLOWED_HOSTS = ['localhost', '.app.github.dev'];
const BASE_ALLOWED_HOSTS = ['localhost', '.app.github.dev'];

function isHostAllowed(hostHeader) {
function isHostAllowed(hostHeader, host) {
Copy link
Contributor

Choose a reason for hiding this comment

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

host as an argument name is not very descriptive, maybe boundHost might be better? Or extraAllowedHost?

if (!hostHeader) {
return false;
}

// Extract hostname without port
// When binding to all interfaces, allow any host.
if (host === '0.0.0.0') {
return true;
}

const hostname = hostHeader.split(':')[0];

// Check exact match or suffix match for wildcard patterns
return ALLOWED_HOSTS.some((allowedHost) => {
// Include the configured host in addition to the defaults.
const allowedHosts = BASE_ALLOWED_HOSTS.includes(host)
? BASE_ALLOWED_HOSTS
: [...BASE_ALLOWED_HOSTS, host];
Comment on lines +42 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just always do [...BASE_ALLOWED_HOSTS, host], checking the same host twice is not a problem.


return allowedHosts.some((allowedHost) => {
if (allowedHost.startsWith('.')) {
// Wildcard pattern like '.app.github.dev'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this comment? It seems useful

return hostname.endsWith(allowedHost);
}
return hostname === allowedHost;
Expand Down Expand Up @@ -75,7 +82,7 @@ export async function startDevServer(buildConfig, options = {}) {
// Create HTTP server
const server = http.createServer((req, res) => {
// Validate Host header
if (!isHostAllowed(req.headers.host)) {
if (!isHostAllowed(req.headers.host, host)) {
res.writeHead(403, { 'Content-Type': 'text/plain' });
res.end('Invalid Host header');
return;
Expand Down