Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
17 changes: 5 additions & 12 deletions e2e/helpers/dashboard-smoke-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,10 @@ export const SMOKE_SPEC_NAME = 'smoke-fixture-spec';
/** Dashboard backend base URL — must match DASHBOARD_PORT in playwright.smoke.config.ts. */
export const DASHBOARD_API_BASE_URL = 'http://127.0.0.1:5085';

// SFLW-51 (pre-existing on React 18, dev mode only): the vite dev proxy
// targets ws://localhost:<port> while the backend binds 127.0.0.1, so every
// /ws upgrade fails with a handshake 500 and the provider retries forever.
// This exact pattern is filtered with justification; ALL other console errors
// (render errors, React warnings-as-errors, route failures) still fail tests.
export const PRE_EXISTING_WS_PROXY_ERROR = /WebSocket connection to 'ws:\/\/[^']*\/ws[^']*' failed/;

/** Attaches console.error + pageerror collectors, filtering only the SFLW-51 pattern. */
/** Attaches console.error + pageerror collectors. Any console error fails the spec. */
export function collectConsoleErrors(page: Page, sink: string[]): void {
page.on('console', (message) => {
if (message.type() === 'error' && !PRE_EXISTING_WS_PROXY_ERROR.test(message.text())) {
if (message.type() === 'error') {
Comment thread
lbruton marked this conversation as resolved.
sink.push(`[console.error] ${message.text()}`);
}
});
Expand All @@ -59,9 +52,9 @@ export async function selectProject(page: Page, projectId: string): Promise<void
}

/**
* Standard smoke-spec opening: start collecting console errors (SFLW-51
* pattern filtered) BEFORE the first navigation so app-boot errors are
* captured too, then load the dashboard and select the seeded project.
* Standard smoke-spec opening: start collecting console errors BEFORE the
* first navigation so app-boot errors are captured too, then load the
* dashboard and select the seeded project.
* Returns the console-error sink for the spec's final assertion.
*/
export async function openSeededDashboard(page: Page, projectId: string): Promise<string[]> {
Expand Down
82 changes: 82 additions & 0 deletions src/core/__tests__/security-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { join } from 'path';
import { tmpdir } from 'os';
import {
isLocalhostAddress,
isLoopbackOrigin,
getSecurityConfig,
generateAllowedOrigins,
DEFAULT_SECURITY_CONFIG,
Expand Down Expand Up @@ -153,6 +154,30 @@ describe('security-utils', () => {
});
});

describe('isLoopbackOrigin', () => {
it('returns true for loopback origins on any port', () => {
expect(isLoopbackOrigin('http://localhost:5185')).toBe(true);
expect(isLoopbackOrigin('http://127.0.0.1:5173')).toBe(true);
expect(isLoopbackOrigin('http://[::1]:5185')).toBe(true);
});

it('returns false for non-loopback origins', () => {
expect(isLoopbackOrigin('https://specdash.lbruton.cc')).toBe(false);
expect(isLoopbackOrigin('http://192.168.1.10:5185')).toBe(false);
});
Comment thread
lbruton marked this conversation as resolved.

it('returns false for hostnames that merely start with "127." (CORS-bypass guard)', () => {
expect(isLoopbackOrigin('http://127.example.com:5185')).toBe(false);
expect(isLoopbackOrigin('http://127.0.0.1.evil.com:5185')).toBe(false);
expect(isLoopbackOrigin('http://127.0.0.256:5185')).toBe(false);
});

it('returns false for unparseable origins', () => {
expect(isLoopbackOrigin('not-a-url')).toBe(false);
expect(isLoopbackOrigin('')).toBe(false);
});
});

describe('RateLimiter', () => {
let rateLimiter: RateLimiter;

Expand Down Expand Up @@ -295,6 +320,63 @@ describe('security-utils', () => {

expect(callback).toHaveBeenCalledWith(expect.any(Error));
});

it('should allow an off-allowlist loopback origin in non-production (SFLW-51)', () => {
const original = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';
try {
const corsConfig = getCorsConfig({
...DEFAULT_SECURITY_CONFIG,
corsEnabled: true,
allowedOrigins: ['http://127.0.0.1:5000'],
}) as any;

const callback = vi.fn();
corsConfig.origin('http://127.0.0.1:5185', callback);

expect(callback).toHaveBeenCalledWith(null, true);
} finally {
process.env.NODE_ENV = original;
}
Comment thread
lbruton marked this conversation as resolved.
Outdated
});

it('should reject an off-allowlist loopback origin in production', () => {
const original = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
const corsConfig = getCorsConfig({
...DEFAULT_SECURITY_CONFIG,
corsEnabled: true,
allowedOrigins: ['http://127.0.0.1:5000'],
}) as any;

const callback = vi.fn();
corsConfig.origin('http://127.0.0.1:5185', callback);

expect(callback).toHaveBeenCalledWith(expect.any(Error));
} finally {
process.env.NODE_ENV = original;
}
Comment thread
lbruton marked this conversation as resolved.
Outdated
});

it('should reject a non-loopback cross-origin request even in non-production', () => {
const original = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';
try {
const corsConfig = getCorsConfig({
...DEFAULT_SECURITY_CONFIG,
corsEnabled: true,
allowedOrigins: ['http://127.0.0.1:5000'],
}) as any;

const callback = vi.fn();
corsConfig.origin('https://evil.example.com', callback);

expect(callback).toHaveBeenCalledWith(expect.any(Error));
} finally {
process.env.NODE_ENV = original;
}
Comment thread
lbruton marked this conversation as resolved.
Outdated
});
});

describe('createSecurityHeadersMiddleware', () => {
Expand Down
44 changes: 42 additions & 2 deletions src/core/security-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,35 @@ export function isLocalhostAddress(address: string): boolean {
); // Any 127.x.x.x address (includes 127.0.0.1)
}

/**
* Check if a CORS Origin header value points at the loopback interface,
* regardless of port (e.g. http://localhost:5185, http://127.0.0.1:5173).
* Used to permit the Vite dev server — which can run on any port — to reach
* the dashboard in non-production. Returns false for unparseable origins.
* @param origin - The Origin header value (e.g. "http://127.0.0.1:5185")
*/
export function isLoopbackOrigin(origin: string): boolean {
try {
// url.hostname keeps IPv6 in brackets ("[::1]"); strip them before checking.
const hostname = new URL(origin).hostname.replace(/^\[|\]$/g, '');
if (hostname === 'localhost' || hostname === '::1') {
return true;
}
// Strict IPv4 127.0.0.0/8: exactly four numeric octets, first === 127,
// each 0–255. Intentionally NOT isLocalhostAddress(), whose loose
// `startsWith('127.')` would also match a hostname like "127.example.com"
// — a CORS bypass we must not allow.
const octets = /^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(hostname);
if (!octets) {
return false;
}
const [, a, b, c, d] = octets;
return Number(a) === 127 && [a, b, c, d].every((o) => Number(o) <= 255);
} catch {
return false;
}
}

/**
* Get security configuration with secure defaults
* Note: Network binding validation (bindAddress/allowExternalAccess) is handled separately at the config layer
Expand Down Expand Up @@ -327,9 +356,20 @@ export function getCorsConfig(config: SecurityConfig) {
// Check if origin is in allowed list
if (config.allowedOrigins.includes(origin)) {
callback(null, true);
} else {
callback(new Error('Not allowed by CORS'));
return;
}

// In non-production, allow any loopback origin regardless of port. The
// Vite dev server can run on any port (e.g. the e2e harness uses 5185),
// and its proxied /ws upgrade carries that origin. The dashboard already
// binds localhost-only, so this does not widen exposure beyond the local
// machine. (SFLW-51)
if (process.env.NODE_ENV !== 'production' && isLoopbackOrigin(origin)) {
callback(null, true);
return;
}
Comment thread
lbruton marked this conversation as resolved.

callback(new Error('Not allowed by CORS'));
},
credentials: true,
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
Expand Down
8 changes: 6 additions & 2 deletions src/dashboard_frontend/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,16 @@ async function createConfig() {
},
server: {
proxy: {
// Target 127.0.0.1 to match the backend's IPv4 loopback bind exactly:
// Node >=17 may resolve "localhost" to ::1 (IPv6) first, where the
// backend does not listen. (The /ws upgrade itself is unblocked by the
// CORS fix in security-utils.ts — see SFLW-51.)
'/api': {
target: `http://localhost:${dashboardPort}`,
target: `http://127.0.0.1:${dashboardPort}`,
changeOrigin: true,
},
'/ws': {
target: `ws://localhost:${dashboardPort}`,
target: `ws://127.0.0.1:${dashboardPort}`,
Comment thread
lbruton marked this conversation as resolved.
Outdated
ws: true,
Comment thread
lbruton marked this conversation as resolved.
Outdated
},
},
Expand Down
Loading