small security fixes
This commit is contained in:
@@ -0,0 +1,57 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { shouldUseSecureAuthCookie } from "./auth-cookie";
|
||||
|
||||
const ORIGINAL_ENV = {
|
||||
OVERLORD_AUTH_COOKIE_SECURE: process.env.OVERLORD_AUTH_COOKIE_SECURE,
|
||||
OVERLORD_TLS_OFFLOAD: process.env.OVERLORD_TLS_OFFLOAD,
|
||||
OVERLORD_TRUST_PROXY_HEADERS: process.env.OVERLORD_TRUST_PROXY_HEADERS,
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
for (const [key, value] of Object.entries(ORIGINAL_ENV)) {
|
||||
if (value === undefined) delete (process.env as any)[key];
|
||||
else (process.env as any)[key] = value;
|
||||
}
|
||||
});
|
||||
|
||||
describe("shouldUseSecureAuthCookie", () => {
|
||||
test("does not trust X-Forwarded-Proto by default", () => {
|
||||
delete process.env.OVERLORD_AUTH_COOKIE_SECURE;
|
||||
delete process.env.OVERLORD_TLS_OFFLOAD;
|
||||
delete process.env.OVERLORD_TRUST_PROXY_HEADERS;
|
||||
|
||||
const req = new Request("http://localhost/api/login", {
|
||||
headers: { "x-forwarded-proto": "https" },
|
||||
});
|
||||
|
||||
expect(shouldUseSecureAuthCookie(req)).toBe(false);
|
||||
});
|
||||
|
||||
test("treats TLS offload as externally secure without forwarded headers", () => {
|
||||
process.env.OVERLORD_TLS_OFFLOAD = "true";
|
||||
delete process.env.OVERLORD_TRUST_PROXY_HEADERS;
|
||||
delete process.env.OVERLORD_AUTH_COOKIE_SECURE;
|
||||
|
||||
const req = new Request("http://localhost/api/login");
|
||||
|
||||
expect(shouldUseSecureAuthCookie(req)).toBe(true);
|
||||
});
|
||||
|
||||
test("trusts X-Forwarded-Proto when proxy headers are explicitly trusted", () => {
|
||||
process.env.OVERLORD_TRUST_PROXY_HEADERS = "true";
|
||||
delete process.env.OVERLORD_TLS_OFFLOAD;
|
||||
delete process.env.OVERLORD_AUTH_COOKIE_SECURE;
|
||||
|
||||
const req = new Request("http://localhost/api/login", {
|
||||
headers: { "x-forwarded-proto": "https" },
|
||||
});
|
||||
|
||||
expect(shouldUseSecureAuthCookie(req)).toBe(true);
|
||||
});
|
||||
|
||||
test("honors explicit secure override", () => {
|
||||
process.env.OVERLORD_AUTH_COOKIE_SECURE = "true";
|
||||
const req = new Request("http://localhost/api/login");
|
||||
expect(shouldUseSecureAuthCookie(req)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,12 @@ function envFlagDisabled(value: string): boolean {
|
||||
return v === "0" || v === "false" || v === "no" || v === "off";
|
||||
}
|
||||
|
||||
function shouldTrustProxyHeaders(): boolean {
|
||||
const explicit = String(process.env.OVERLORD_TRUST_PROXY_HEADERS || "");
|
||||
if (explicit) return envFlagEnabled(explicit);
|
||||
return false;
|
||||
}
|
||||
|
||||
export function shouldUseSecureAuthCookie(req: Request): boolean {
|
||||
const mode = String(process.env.OVERLORD_AUTH_COOKIE_SECURE || "auto");
|
||||
if (envFlagEnabled(mode)) {
|
||||
@@ -17,16 +23,22 @@ export function shouldUseSecureAuthCookie(req: Request): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
const forwardedProto = req.headers
|
||||
.get("x-forwarded-proto")
|
||||
?.split(",")[0]
|
||||
?.trim()
|
||||
?.toLowerCase();
|
||||
if (forwardedProto === "https") {
|
||||
if (envFlagEnabled(String(process.env.OVERLORD_TLS_OFFLOAD || ""))) {
|
||||
return true;
|
||||
}
|
||||
if (forwardedProto === "http") {
|
||||
return false;
|
||||
|
||||
if (shouldTrustProxyHeaders()) {
|
||||
const forwardedProto = req.headers
|
||||
.get("x-forwarded-proto")
|
||||
?.split(",")[0]
|
||||
?.trim()
|
||||
?.toLowerCase();
|
||||
if (forwardedProto === "https") {
|
||||
return true;
|
||||
}
|
||||
if (forwardedProto === "http") {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
|
||||
@@ -13,6 +13,7 @@ import { requirePermission } from "../../rbac";
|
||||
import { AUTO_DEPLOY_TRIGGERS, ALLOWED_OS_FILTERS } from "../validation-constants";
|
||||
import { v4 as uuidv4 } from "uuid";
|
||||
import type { DeployOs } from "../deploy-utils";
|
||||
import { resolveContainedPath, sanitizeUploadFilename } from "../upload-security";
|
||||
|
||||
type AutoDeployRouteDeps = {
|
||||
DEPLOY_ROOT: string;
|
||||
@@ -89,12 +90,12 @@ export async function handleAutoDeployRoutes(
|
||||
return Response.json({ error: "Invalid trigger" }, { status: 400 });
|
||||
}
|
||||
|
||||
const filename = path.basename(file.name || "upload.bin");
|
||||
const filename = sanitizeUploadFilename(file.name, "upload.bin");
|
||||
const id = uuidv4();
|
||||
await fs.mkdir(deps.DEPLOY_ROOT, { recursive: true });
|
||||
const folder = path.join(deps.DEPLOY_ROOT, "auto", id);
|
||||
const folder = resolveContainedPath(deps.DEPLOY_ROOT, "auto", id);
|
||||
await fs.mkdir(folder, { recursive: true });
|
||||
const targetPath = path.join(folder, filename);
|
||||
const targetPath = resolveContainedPath(folder, filename);
|
||||
const bytes = new Uint8Array(await file.arrayBuffer());
|
||||
await fs.writeFile(targetPath, bytes);
|
||||
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { createHash } from "crypto";
|
||||
import fs from "fs/promises";
|
||||
import path from "path";
|
||||
import { v4 as uuidv4 } from "uuid";
|
||||
import { authenticateRequest } from "../../auth";
|
||||
import { AuditAction, logAudit } from "../../auditLog";
|
||||
@@ -9,6 +8,8 @@ import { metrics } from "../../metrics";
|
||||
import { encodeMessage } from "../../protocol";
|
||||
import { requirePermission } from "../../rbac";
|
||||
import { createUploadPull } from "../file-transfer-state";
|
||||
import { fetchPublicUrlBytes, validatePublicHttpUrl } from "../url-security";
|
||||
import { resolveContainedPath, sanitizeUploadFilename } from "../upload-security";
|
||||
|
||||
type RequestIpProvider = {
|
||||
requestIP: (req: Request) => { address?: string } | null | undefined;
|
||||
@@ -64,12 +65,12 @@ export async function handleDeployRoutes(
|
||||
return new Response("Missing file", { status: 400 });
|
||||
}
|
||||
|
||||
const filename = path.basename(file.name || "upload.bin");
|
||||
const filename = sanitizeUploadFilename(file.name, "upload.bin");
|
||||
const id = uuidv4();
|
||||
await fs.mkdir(deps.DEPLOY_ROOT, { recursive: true });
|
||||
const folder = path.join(deps.DEPLOY_ROOT, id);
|
||||
const folder = resolveContainedPath(deps.DEPLOY_ROOT, id);
|
||||
await fs.mkdir(folder, { recursive: true });
|
||||
const targetPath = path.join(folder, filename);
|
||||
const targetPath = resolveContainedPath(folder, filename);
|
||||
const bytes = new Uint8Array(await file.arrayBuffer());
|
||||
await fs.writeFile(targetPath, bytes);
|
||||
|
||||
@@ -112,47 +113,27 @@ export async function handleDeployRoutes(
|
||||
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(fileUrl);
|
||||
} catch {
|
||||
return new Response("Invalid URL", { status: 400 });
|
||||
}
|
||||
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
|
||||
return new Response("Only http and https URLs are allowed", { status: 400 });
|
||||
parsed = await validatePublicHttpUrl(fileUrl);
|
||||
} catch (err: any) {
|
||||
return new Response(err?.message || "Invalid URL", { status: 400 });
|
||||
}
|
||||
|
||||
const hostname = parsed.hostname.toLowerCase();
|
||||
const BLOCKED_HOSTS = ["localhost", "metadata.google.internal", "169.254.169.254"];
|
||||
if (
|
||||
BLOCKED_HOSTS.includes(hostname) ||
|
||||
hostname.endsWith(".internal") ||
|
||||
hostname.startsWith("127.") ||
|
||||
hostname === "[::1]" ||
|
||||
/^(10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.)/.test(hostname) ||
|
||||
hostname.startsWith("169.254.") ||
|
||||
hostname.startsWith("0.")
|
||||
) {
|
||||
return new Response("URLs pointing to private/internal addresses are not allowed", { status: 400 });
|
||||
}
|
||||
|
||||
const rawFilename = path.basename(parsed.pathname) || "download.bin";
|
||||
const filename = rawFilename.replace(/[^a-zA-Z0-9._\-]/g, "_").substring(0, 128) || "download.bin";
|
||||
const rawFilename = parsed.pathname.split("/").pop() || "download.bin";
|
||||
const filename = sanitizeUploadFilename(rawFilename, "download.bin");
|
||||
|
||||
let fileBytes: Uint8Array;
|
||||
try {
|
||||
const fetchRes = await fetch(fileUrl);
|
||||
if (!fetchRes.ok) {
|
||||
return new Response(`Remote fetch failed: ${fetchRes.status}`, { status: 502 });
|
||||
}
|
||||
fileBytes = new Uint8Array(await fetchRes.arrayBuffer());
|
||||
const fetched = await fetchPublicUrlBytes(parsed.toString());
|
||||
fileBytes = fetched.bytes;
|
||||
} catch (err: any) {
|
||||
return new Response(`Failed to fetch URL: ${err?.message || "network error"}`, { status: 502 });
|
||||
}
|
||||
|
||||
const id = uuidv4();
|
||||
await fs.mkdir(deps.DEPLOY_ROOT, { recursive: true });
|
||||
const folder = path.join(deps.DEPLOY_ROOT, id);
|
||||
const folder = resolveContainedPath(deps.DEPLOY_ROOT, id);
|
||||
await fs.mkdir(folder, { recursive: true });
|
||||
const targetPath = path.join(folder, filename);
|
||||
const targetPath = resolveContainedPath(folder, filename);
|
||||
await fs.writeFile(targetPath, fileBytes);
|
||||
|
||||
const os = deps.detectUploadOs(filename, fileBytes);
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
incrementSharedFileDownloadCount,
|
||||
} from "../../db";
|
||||
import { getUserById, canUploadFiles } from "../../users";
|
||||
import { resolveContainedPath, sanitizeUploadFilename } from "../upload-security";
|
||||
|
||||
type FileShareRouteDeps = {
|
||||
FILE_SHARE_ROOT: string;
|
||||
@@ -191,13 +192,13 @@ export async function handleFileShareRoutes(
|
||||
return Response.json({ error: "Missing file" }, { status: 400 });
|
||||
}
|
||||
|
||||
const filename = path.basename(file.name || "upload.bin");
|
||||
const filename = sanitizeUploadFilename(file.name, "upload.bin");
|
||||
const id = uuidv4();
|
||||
|
||||
await fs.mkdir(deps.FILE_SHARE_ROOT, { recursive: true });
|
||||
const folder = path.join(deps.FILE_SHARE_ROOT, id);
|
||||
const folder = resolveContainedPath(deps.FILE_SHARE_ROOT, id);
|
||||
await fs.mkdir(folder, { recursive: true });
|
||||
const targetPath = path.join(folder, filename);
|
||||
const targetPath = resolveContainedPath(folder, filename);
|
||||
|
||||
const bytes = new Uint8Array(await file.arrayBuffer());
|
||||
await fs.writeFile(targetPath, bytes);
|
||||
|
||||
@@ -1262,7 +1262,13 @@ export async function handlePluginRoutes(
|
||||
return new Response("Forbidden: Viewers cannot access interactive features", { status: 403 });
|
||||
}
|
||||
|
||||
const [, pluginId, assetPath] = pluginAssetMatch;
|
||||
const [, rawPluginId, assetPath] = pluginAssetMatch;
|
||||
let pluginId = "";
|
||||
try {
|
||||
pluginId = deps.sanitizePluginId(rawPluginId);
|
||||
} catch {
|
||||
return new Response("Invalid plugin id", { status: 400 });
|
||||
}
|
||||
try {
|
||||
requirePluginAccess(user, pluginId);
|
||||
} catch (error) {
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import fs from "fs/promises";
|
||||
import path from "path";
|
||||
import { v4 as uuidv4 } from "uuid";
|
||||
import { authenticateRequest } from "../../auth";
|
||||
import { AuditAction, logAudit } from "../../auditLog";
|
||||
@@ -8,6 +7,7 @@ import { metrics } from "../../metrics";
|
||||
import { encodeMessage } from "../../protocol";
|
||||
import { requirePermission } from "../../rbac";
|
||||
import { createUploadPull } from "../file-transfer-state";
|
||||
import { resolveContainedPath, sanitizeUploadFilename } from "../upload-security";
|
||||
|
||||
type RequestIpProvider = {
|
||||
requestIP: (req: Request) => { address?: string } | null | undefined;
|
||||
@@ -100,12 +100,12 @@ export async function handleWinRERoutes(
|
||||
return new Response("Missing file", { status: 400 });
|
||||
}
|
||||
|
||||
const filename = path.basename(file.name || "upload.exe");
|
||||
const filename = sanitizeUploadFilename(file.name, "upload.exe");
|
||||
const id = uuidv4();
|
||||
await fs.mkdir(deps.WINRE_ROOT, { recursive: true });
|
||||
const folder = path.join(deps.WINRE_ROOT, id);
|
||||
const folder = resolveContainedPath(deps.WINRE_ROOT, id);
|
||||
await fs.mkdir(folder, { recursive: true });
|
||||
const targetPath = path.join(folder, filename);
|
||||
const targetPath = resolveContainedPath(folder, filename);
|
||||
const bytes = new Uint8Array(await file.arrayBuffer());
|
||||
await fs.writeFile(targetPath, bytes);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user