fix(SEC-API-25+26): Enable strict ValidationPipe + tighten CORS origin
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
- Set forbidNonWhitelisted: true in ValidationPipe to reject requests with unknown DTO properties, preventing mass assignment vulnerabilities - Reject requests with no Origin header in production (SEC-API-26) - Restrict localhost:3001 to development mode only - Update CORS tests to cover production/development origin validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -10,12 +10,59 @@ import { describe, it, expect } from "vitest";
|
|||||||
* - origin: must be specific origins, NOT wildcard (security requirement with credentials)
|
* - origin: must be specific origins, NOT wildcard (security requirement with credentials)
|
||||||
* - Access-Control-Allow-Credentials: true header
|
* - Access-Control-Allow-Credentials: true header
|
||||||
* - Access-Control-Allow-Origin: specific origin (not *)
|
* - Access-Control-Allow-Origin: specific origin (not *)
|
||||||
|
* - No-origin requests blocked in production (SEC-API-26)
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Replicates the CORS origin validation logic from main.ts
|
||||||
|
* so we can test it in isolation.
|
||||||
|
*/
|
||||||
|
function buildOriginValidator(nodeEnv: string | undefined): {
|
||||||
|
allowedOrigins: string[];
|
||||||
|
isDevelopment: boolean;
|
||||||
|
validate: (
|
||||||
|
origin: string | undefined,
|
||||||
|
callback: (err: Error | null, allow?: boolean) => void
|
||||||
|
) => void;
|
||||||
|
} {
|
||||||
|
const isDevelopment = nodeEnv !== "production";
|
||||||
|
|
||||||
|
const allowedOrigins = [
|
||||||
|
process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000",
|
||||||
|
"https://app.mosaicstack.dev",
|
||||||
|
"https://api.mosaicstack.dev",
|
||||||
|
];
|
||||||
|
|
||||||
|
if (isDevelopment) {
|
||||||
|
allowedOrigins.push("http://localhost:3001");
|
||||||
|
}
|
||||||
|
|
||||||
|
const validate = (
|
||||||
|
origin: string | undefined,
|
||||||
|
callback: (err: Error | null, allow?: boolean) => void
|
||||||
|
): void => {
|
||||||
|
if (!origin) {
|
||||||
|
if (isDevelopment) {
|
||||||
|
callback(null, true);
|
||||||
|
} else {
|
||||||
|
callback(new Error("CORS: Origin header is required"));
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (allowedOrigins.includes(origin)) {
|
||||||
|
callback(null, true);
|
||||||
|
} else {
|
||||||
|
callback(new Error(`Origin ${origin} not allowed by CORS`));
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
return { allowedOrigins, isDevelopment, validate };
|
||||||
|
}
|
||||||
|
|
||||||
describe("CORS Configuration", () => {
|
describe("CORS Configuration", () => {
|
||||||
describe("Configuration requirements", () => {
|
describe("Configuration requirements", () => {
|
||||||
it("should document required CORS settings for cookie-based auth", () => {
|
it("should document required CORS settings for cookie-based auth", () => {
|
||||||
// This test documents the requirements
|
|
||||||
const requiredSettings = {
|
const requiredSettings = {
|
||||||
origin: ["http://localhost:3000", "https://app.mosaicstack.dev"],
|
origin: ["http://localhost:3000", "https://app.mosaicstack.dev"],
|
||||||
credentials: true,
|
credentials: true,
|
||||||
@@ -30,35 +77,25 @@ describe("CORS Configuration", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should NOT use wildcard origin with credentials (security violation)", () => {
|
it("should NOT use wildcard origin with credentials (security violation)", () => {
|
||||||
// Wildcard origin with credentials is a security violation
|
|
||||||
// This test ensures we never use that combination
|
|
||||||
const validConfig1 = { origin: "*", credentials: false };
|
const validConfig1 = { origin: "*", credentials: false };
|
||||||
const validConfig2 = { origin: "http://localhost:3000", credentials: true };
|
const validConfig2 = { origin: "http://localhost:3000", credentials: true };
|
||||||
const invalidConfig = { origin: "*", credentials: true };
|
const invalidConfig = { origin: "*", credentials: true };
|
||||||
|
|
||||||
// Valid configs
|
|
||||||
expect(validConfig1.origin === "*" && !validConfig1.credentials).toBe(true);
|
expect(validConfig1.origin === "*" && !validConfig1.credentials).toBe(true);
|
||||||
expect(validConfig2.origin !== "*" && validConfig2.credentials).toBe(true);
|
expect(validConfig2.origin !== "*" && validConfig2.credentials).toBe(true);
|
||||||
|
|
||||||
// Invalid config check - this combination should NOT be allowed
|
|
||||||
const isInvalidCombination = invalidConfig.origin === "*" && invalidConfig.credentials;
|
const isInvalidCombination = invalidConfig.origin === "*" && invalidConfig.credentials;
|
||||||
expect(isInvalidCombination).toBe(true); // This IS an invalid combination
|
expect(isInvalidCombination).toBe(true);
|
||||||
// We will prevent this in our CORS config
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("Origin validation", () => {
|
describe("Origin validation", () => {
|
||||||
it("should define allowed origins list", () => {
|
it("should define allowed origins list", () => {
|
||||||
const allowedOrigins = [
|
const { allowedOrigins } = buildOriginValidator("development");
|
||||||
process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000",
|
|
||||||
"http://localhost:3001", // API origin (dev)
|
|
||||||
"https://app.mosaicstack.dev", // Production web
|
|
||||||
"https://api.mosaicstack.dev", // Production API
|
|
||||||
];
|
|
||||||
|
|
||||||
expect(allowedOrigins).toHaveLength(4);
|
|
||||||
expect(allowedOrigins).toContain("http://localhost:3000");
|
expect(allowedOrigins).toContain("http://localhost:3000");
|
||||||
expect(allowedOrigins).toContain("https://app.mosaicstack.dev");
|
expect(allowedOrigins).toContain("https://app.mosaicstack.dev");
|
||||||
|
expect(allowedOrigins).toContain("https://api.mosaicstack.dev");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should match exact origins, not partial matches", () => {
|
it("should match exact origins, not partial matches", () => {
|
||||||
@@ -77,4 +114,124 @@ describe("CORS Configuration", () => {
|
|||||||
expect(typeof envOrigin).toBe("string");
|
expect(typeof envOrigin).toBe("string");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("Development mode CORS behavior", () => {
|
||||||
|
it("should allow requests with no origin in development", () => {
|
||||||
|
const { validate } = buildOriginValidator("development");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate(undefined, (err, allow) => {
|
||||||
|
expect(err).toBeNull();
|
||||||
|
expect(allow).toBe(true);
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should include localhost:3001 in development origins", () => {
|
||||||
|
const { allowedOrigins } = buildOriginValidator("development");
|
||||||
|
|
||||||
|
expect(allowedOrigins).toContain("http://localhost:3001");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow valid origins in development", () => {
|
||||||
|
const { validate } = buildOriginValidator("development");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate("http://localhost:3000", (err, allow) => {
|
||||||
|
expect(err).toBeNull();
|
||||||
|
expect(allow).toBe(true);
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should reject invalid origins in development", () => {
|
||||||
|
const { validate } = buildOriginValidator("development");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate("http://evil.com", (err) => {
|
||||||
|
expect(err).toBeInstanceOf(Error);
|
||||||
|
expect(err?.message).toContain("not allowed by CORS");
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("Production mode CORS behavior (SEC-API-26)", () => {
|
||||||
|
it("should reject requests with no origin in production", () => {
|
||||||
|
const { validate } = buildOriginValidator("production");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate(undefined, (err) => {
|
||||||
|
expect(err).toBeInstanceOf(Error);
|
||||||
|
expect(err?.message).toBe("CORS: Origin header is required");
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should NOT include localhost:3001 in production origins", () => {
|
||||||
|
const { allowedOrigins } = buildOriginValidator("production");
|
||||||
|
|
||||||
|
expect(allowedOrigins).not.toContain("http://localhost:3001");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should allow valid production origins", () => {
|
||||||
|
const { validate } = buildOriginValidator("production");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate("https://app.mosaicstack.dev", (err, allow) => {
|
||||||
|
expect(err).toBeNull();
|
||||||
|
expect(allow).toBe(true);
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should reject invalid origins in production", () => {
|
||||||
|
const { validate } = buildOriginValidator("production");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate("http://evil.com", (err) => {
|
||||||
|
expect(err).toBeInstanceOf(Error);
|
||||||
|
expect(err?.message).toContain("not allowed by CORS");
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should reject malicious origins that try partial matching", () => {
|
||||||
|
const { validate } = buildOriginValidator("production");
|
||||||
|
|
||||||
|
return new Promise<void>((resolve) => {
|
||||||
|
validate("https://app.mosaicstack.dev.evil.com", (err) => {
|
||||||
|
expect(err).toBeInstanceOf(Error);
|
||||||
|
expect(err?.message).toContain("not allowed by CORS");
|
||||||
|
resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("ValidationPipe strict mode (SEC-API-25)", () => {
|
||||||
|
it("should document that forbidNonWhitelisted must be true", () => {
|
||||||
|
// This verifies the configuration intent:
|
||||||
|
// forbidNonWhitelisted: true rejects requests with unknown properties
|
||||||
|
// preventing mass-assignment vulnerabilities
|
||||||
|
const validationPipeConfig = {
|
||||||
|
transform: true,
|
||||||
|
whitelist: true,
|
||||||
|
forbidNonWhitelisted: true,
|
||||||
|
transformOptions: {
|
||||||
|
enableImplicitConversion: false,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
expect(validationPipeConfig.forbidNonWhitelisted).toBe(true);
|
||||||
|
expect(validationPipeConfig.whitelist).toBe(true);
|
||||||
|
expect(validationPipeConfig.transformOptions.enableImplicitConversion).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ async function bootstrap() {
|
|||||||
new ValidationPipe({
|
new ValidationPipe({
|
||||||
transform: true,
|
transform: true,
|
||||||
whitelist: true,
|
whitelist: true,
|
||||||
forbidNonWhitelisted: false,
|
forbidNonWhitelisted: true,
|
||||||
transformOptions: {
|
transformOptions: {
|
||||||
enableImplicitConversion: false,
|
enableImplicitConversion: false,
|
||||||
},
|
},
|
||||||
@@ -48,21 +48,32 @@ async function bootstrap() {
|
|||||||
|
|
||||||
// Configure CORS for cookie-based authentication
|
// Configure CORS for cookie-based authentication
|
||||||
// SECURITY: Cannot use wildcard (*) with credentials: true
|
// SECURITY: Cannot use wildcard (*) with credentials: true
|
||||||
|
const isDevelopment = process.env.NODE_ENV !== "production";
|
||||||
|
|
||||||
const allowedOrigins = [
|
const allowedOrigins = [
|
||||||
process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000",
|
process.env.NEXT_PUBLIC_APP_URL ?? "http://localhost:3000",
|
||||||
"http://localhost:3001", // API origin (dev)
|
|
||||||
"https://app.mosaicstack.dev", // Production web
|
"https://app.mosaicstack.dev", // Production web
|
||||||
"https://api.mosaicstack.dev", // Production API
|
"https://api.mosaicstack.dev", // Production API
|
||||||
];
|
];
|
||||||
|
|
||||||
|
// Development-only origins (not allowed in production)
|
||||||
|
if (isDevelopment) {
|
||||||
|
allowedOrigins.push("http://localhost:3001"); // API origin (dev)
|
||||||
|
}
|
||||||
|
|
||||||
app.enableCors({
|
app.enableCors({
|
||||||
origin: (
|
origin: (
|
||||||
origin: string | undefined,
|
origin: string | undefined,
|
||||||
callback: (err: Error | null, allow?: boolean) => void
|
callback: (err: Error | null, allow?: boolean) => void
|
||||||
): void => {
|
): void => {
|
||||||
// Allow requests with no origin (e.g., mobile apps, Postman)
|
// SECURITY: In production, reject requests with no Origin header.
|
||||||
|
// In development, allow no-origin requests (Postman, curl, mobile apps).
|
||||||
if (!origin) {
|
if (!origin) {
|
||||||
callback(null, true);
|
if (isDevelopment) {
|
||||||
|
callback(null, true);
|
||||||
|
} else {
|
||||||
|
callback(new Error("CORS: Origin header is required"));
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user