Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
99 changes: 99 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 @@ -51,6 +52,13 @@ describe('security-utils', () => {
expect(isLocalhostAddress('myserver')).toBe(false);
});

it('should return false for hostnames that merely start with "127." and invalid octets', () => {
expect(isLocalhostAddress('127.example.com')).toBe(false);
expect(isLocalhostAddress('127.0.0.1.evil.com')).toBe(false);
expect(isLocalhostAddress('127.0.0.256')).toBe(false);
expect(isLocalhostAddress('127.0.0')).toBe(false);
});

it('should return false for empty string', () => {
expect(isLocalhostAddress('')).toBe(false);
});
Expand Down Expand Up @@ -153,6 +161,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 @@ -233,6 +265,17 @@ describe('security-utils', () => {
});

describe('getCorsConfig', () => {
const ORIGINAL_NODE_ENV = process.env.NODE_ENV;
afterEach(() => {
// Restore precisely: assigning `undefined` would set the literal string
// "undefined" and leak into later tests, so delete when originally unset.
if (ORIGINAL_NODE_ENV === undefined) {
delete process.env.NODE_ENV;
} else {
process.env.NODE_ENV = ORIGINAL_NODE_ENV;
}
});

it('should return false when CORS is disabled', () => {
const config: SecurityConfig = {
...DEFAULT_SECURITY_CONFIG,
Expand Down Expand Up @@ -295,6 +338,62 @@ describe('security-utils', () => {

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

it('should allow an off-allowlist loopback origin in non-production (SFLW-51)', () => {
process.env.NODE_ENV = 'development';
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);
});

it('should reject an off-allowlist loopback origin in production', () => {
process.env.NODE_ENV = 'production';
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));
});

it('should reject a non-loopback cross-origin request even in non-production', () => {
process.env.NODE_ENV = 'development';
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));
});

it('should NOT relax to loopback when the allowlist excludes loopback (Copilot review)', () => {
process.env.NODE_ENV = 'development';
const corsConfig = getCorsConfig({
...DEFAULT_SECURITY_CONFIG,
corsEnabled: true,
allowedOrigins: ['https://staging.example.com'],
}) as any;

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

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

describe('createSecurityHeadersMiddleware', () => {
Expand Down
61 changes: 52 additions & 9 deletions src/core/security-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,38 @@ export function generateAllowedOrigins(port: number): string[] {
}

/**
* Check if an IP address is localhost
* Check if an address is loopback: "localhost", "::1", or a literal in the
* IPv4 127.0.0.0/8 block. Uses a strict IPv4 match (each octet 0–255) rather
* than a `127.` prefix, so a hostname like "127.example.com" is NOT loopback
* — a prefix check would be a CORS-bypass / bindAddress-validation hole.
* @param address - IP address or hostname to check
* @returns true if the address is localhost (127.x.x.x, localhost, or ::1)
*/
export function isLocalhostAddress(address: string): boolean {
return (
address === 'localhost' ||
address === '::1' || // IPv6 localhost
address.startsWith('127.')
); // Any 127.x.x.x address (includes 127.0.0.1)
if (address === 'localhost' || address === '::1') {
return true;
}
// Strict IPv4 127.0.0.0/8 (each octet 0–255).
return /^127\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)$/.test(
address,
);
}

/**
* 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 literals in brackets ("[::1]"); strip them so
// isLocalhostAddress sees a bare "::1".
const hostname = new URL(origin).hostname.replace(/^\[|\]$/g, '');
return isLocalhostAddress(hostname);
Comment thread
lbruton marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} catch {
return false;
}
}

/**
Expand Down Expand Up @@ -316,6 +338,12 @@ export function getCorsConfig(config: SecurityConfig) {
return false; // Disable CORS
}

// Only relax for loopback dev origins when the allowlist already trusts
// loopback (the default config does). If a user overrode allowedOrigins to
// exclude loopback — e.g. a shared dev/staging box with NODE_ENV!=='production'
// — respect that and do NOT widen it back open. (SFLW-51)
const allowlistTrustsLoopback = config.allowedOrigins.some(isLoopbackOrigin);

return {
origin: (origin: string, callback: (error: Error | null, allow?: boolean) => void) => {
// Allow requests with no origin (e.g., curl, Postman)
Expand All @@ -327,9 +355,24 @@ 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' &&
allowlistTrustsLoopback &&
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
13 changes: 11 additions & 2 deletions src/dashboard_frontend/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import react from '@vitejs/plugin-react';
// Can be overridden via VITE_DASHBOARD_PORT environment variable
const dashboardPort = process.env.VITE_DASHBOARD_PORT || '5000';

// Dashboard host the dev proxy targets. Defaults to 127.0.0.1 to match the
Comment thread
Copilot marked this conversation as resolved.
Outdated
Comment thread
Copilot marked this conversation as resolved.
Outdated
// backend's default IPv4 loopback bind (Node >=17 may resolve "localhost" to
// ::1 first, where the backend does not listen). Override via VITE_DASHBOARD_HOST
// for backends bound to ::1 or another address. (SFLW-51)
const dashboardHost = process.env.VITE_DASHBOARD_HOST || '127.0.0.1';

// Dynamically import Tailwind CSS v4 plugin
async function createConfig() {
const { default: tailwindcss } = await import('@tailwindcss/vite');
Expand All @@ -22,12 +28,15 @@ async function createConfig() {
},
server: {
proxy: {
// Target dashboardHost (default 127.0.0.1) to match the backend's bind
// exactly. (The /ws upgrade itself is unblocked by the CORS fix in
// security-utils.ts — see SFLW-51.)
'/api': {
target: `http://localhost:${dashboardPort}`,
target: `http://${dashboardHost}:${dashboardPort}`,
changeOrigin: true,
},
'/ws': {
target: `ws://localhost:${dashboardPort}`,
target: `ws://${dashboardHost}:${dashboardPort}`,
ws: true,
},
},
Expand Down
Loading