fix(security): harden auth and outbound fetches

This commit is contained in:
SnapOtter
2026-06-29 17:54:12 +08:00
parent 6f85b3d12a
commit c6319cf8a9
34 changed files with 799 additions and 173 deletions
+84
View File
@@ -0,0 +1,84 @@
import { describe, expect, it, vi } from "vitest";
vi.mock("../../../apps/api/src/config.js", () => ({
env: {
API_KEYS_RATE_LIMIT_PER_MIN: 30,
SESSION_DURATION_HOURS: 168,
DEFAULT_PASSWORD: "Adminpass1",
SKIP_MUST_CHANGE_PASSWORD: true,
AUTH_ENABLED: true,
EXTERNAL_URL: "",
},
}));
vi.mock("../../../apps/api/src/db/index.js", () => ({
db: {},
schema: {},
}));
vi.mock("../../../apps/api/src/lib/audit.js", () => ({
auditFromRequest: () => vi.fn(),
sanitizeAuditInput: (value: string) => value,
}));
vi.mock("../../../apps/api/src/jobs/connection.js", () => ({
sharedRedis: () => ({
setex: vi.fn(),
get: vi.fn(),
del: vi.fn(),
}),
}));
vi.mock("../../../apps/api/src/lib/metrics.js", () => ({
authAttempts: { inc: vi.fn() },
}));
vi.mock("../../../apps/api/src/lib/settings-helpers.js", () => ({
getSettingNumber: vi.fn().mockResolvedValue(0),
getSettingString: vi.fn().mockResolvedValue("optional"),
}));
import { deriveApiKeyPermissionsForCreate } from "../../../apps/api/src/routes/api-keys.js";
describe("deriveApiKeyPermissionsForCreate", () => {
it("defaults a scoped caller's new key to the caller's effective scope", async () => {
const scoped = await deriveApiKeyPermissionsForCreate({
id: "u1",
username: "admin",
role: "admin",
apiKeyPermissions: ["tools:use", "apikeys:own"],
});
expect(scoped).toEqual({
permissions: ["tools:use", "apikeys:own"],
invalid: [],
});
});
it("rejects requested permissions outside the caller's API key scope", async () => {
const scoped = await deriveApiKeyPermissionsForCreate(
{
id: "u1",
username: "admin",
role: "admin",
apiKeyPermissions: ["tools:use", "apikeys:own"],
},
["tools:use", "users:manage"],
);
expect(scoped).toEqual({
permissions: ["tools:use", "users:manage"],
invalid: ["users:manage"],
});
});
it("keeps unscoped session-created keys unscoped when no scope is requested", async () => {
const scoped = await deriveApiKeyPermissionsForCreate({
id: "u1",
username: "admin",
role: "admin",
});
expect(scoped).toEqual({ permissions: null, invalid: [] });
});
});
+49 -2
View File
@@ -5,6 +5,12 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
const dbMock = vi.hoisted(() => {
let idx = 0;
let results: unknown[][] = [];
const queryResult = () => {
const promise = Promise.resolve(results[idx++] ?? []);
return Object.assign(promise, {
limit: () => promise,
});
};
return {
reset(r: unknown[][]) {
idx = 0;
@@ -13,6 +19,7 @@ const dbMock = vi.hoisted(() => {
nextResult() {
return results[idx++] ?? [];
},
queryResult,
};
});
@@ -20,12 +27,20 @@ vi.mock("../../../apps/api/src/db/index.js", () => ({
db: {
select: () => ({
from: () => ({
where: () => Promise.resolve(dbMock.nextResult()),
where: () => dbMock.queryResult(),
}),
}),
insert: () => ({
values: () => Promise.resolve({ rowCount: 1 }),
}),
},
schema: {
users: { username: "username" },
users: {
username: "username",
externalId: "external_id",
authProvider: "auth_provider",
email: "email",
},
},
}));
@@ -192,3 +207,35 @@ describe("findUniqueUsername", () => {
expect(result).toBe("user_5");
});
});
describe("resolveExternalUser", () => {
beforeEach(() => {
dbMock.reset([]);
});
it("denies SSO auto-create when the configured default role is disabled", async () => {
const mod = await import("../../../apps/api/src/lib/external-auth-resolver.js");
const logger = {
info: vi.fn(),
warn: vi.fn(),
};
const result = await mod.resolveExternalUser({
provider: "oidc",
externalId: "subject-1",
username: "subject",
autoCreate: true,
autoLink: false,
defaultRole: "disabled",
logger: logger as never,
ip: "127.0.0.1",
requestId: "req-1",
});
expect(result).toEqual({
user: null,
action: "denied",
deniedReason: "user_disabled",
});
expect(logger.warn).toHaveBeenCalledWith("oidc auto-create blocked: default role is disabled");
});
});
+59 -1
View File
@@ -21,7 +21,14 @@ vi.mock("../../../apps/api/src/plugins/auth.js", () => ({
getAuthUser: () => null,
}));
import { getPermissions, hasPermission } from "../../../apps/api/src/permissions.js";
import {
getEffectivePermissions,
getPermissions,
hasEffectiveToolAccess,
hasPermission,
isDisabledRole,
permissionsNotHeldBy,
} from "../../../apps/api/src/permissions.js";
describe("permissions", () => {
describe("getPermissions", () => {
@@ -91,4 +98,55 @@ describe("permissions", () => {
expect(await hasPermission("user", "settings:write")).toBe(false);
});
});
describe("effective scoped permissions", () => {
it("intersects role permissions with API key scope", async () => {
const perms = await getEffectivePermissions({
id: "u1",
username: "admin",
role: "admin",
apiKeyPermissions: ["tools:use", "apikeys:own"],
});
expect(perms).toEqual(["tools:use", "apikeys:own"]);
});
it("reports requested permissions outside the caller effective scope", async () => {
const invalid = await permissionsNotHeldBy(
{
id: "u1",
username: "admin",
role: "admin",
apiKeyPermissions: ["tools:use", "apikeys:own"],
},
["tools:use", "users:manage"],
);
expect(invalid).toEqual(["users:manage"]);
});
it("denies tool execution when API key scope lacks tools:use", async () => {
await expect(
hasEffectiveToolAccess(
{
id: "u1",
username: "admin",
role: "admin",
apiKeyPermissions: ["apikeys:own"],
},
"resize",
),
).resolves.toBe(false);
});
});
describe("disabled roles", () => {
it("identifies disabled SCIM roles", () => {
expect(isDisabledRole("disabled")).toBe(true);
expect(isDisabledRole("disabled:user")).toBe(true);
expect(isDisabledRole("user")).toBe(false);
});
it("returns no permissions for disabled roles", async () => {
await expect(getPermissions("disabled:admin")).resolves.toEqual([]);
});
});
});
+12 -3
View File
@@ -22,7 +22,10 @@ type Permission =
| "teams:manage"
| "features:manage"
| "system:health"
| "audit:read";
| "audit:read"
| "compliance:manage"
| "webhooks:manage"
| "security:manage";
const ALL_PERMISSIONS: Permission[] = [
"tools:use",
@@ -39,6 +42,9 @@ const ALL_PERMISSIONS: Permission[] = [
"features:manage",
"system:health",
"audit:read",
"compliance:manage",
"webhooks:manage",
"security:manage",
];
const ROLE_NAME_PATTERN = /^[a-z0-9_-]+$/;
@@ -263,8 +269,8 @@ describe("roles route logic", () => {
});
describe("ALL_PERMISSIONS constant", () => {
it("contains exactly 14 permissions", () => {
expect(ALL_PERMISSIONS).toHaveLength(14);
it("contains exactly 17 permissions", () => {
expect(ALL_PERMISSIONS).toHaveLength(17);
});
it("contains all expected permissions", () => {
@@ -280,6 +286,9 @@ describe("roles route logic", () => {
expect(ALL_PERMISSIONS).toContain("features:manage");
expect(ALL_PERMISSIONS).toContain("system:health");
expect(ALL_PERMISSIONS).toContain("audit:read");
expect(ALL_PERMISSIONS).toContain("compliance:manage");
expect(ALL_PERMISSIONS).toContain("webhooks:manage");
expect(ALL_PERMISSIONS).toContain("security:manage");
});
});
+20 -2
View File
@@ -3,13 +3,13 @@ import { MAX_REDIRECTS, safeFetch, validateFetchUrl } from "../../../apps/api/sr
describe("validateFetchUrl", () => {
it("allows valid public HTTP URL", async () => {
const result = await validateFetchUrl("https://images.unsplash.com/photo.jpg");
const result = await validateFetchUrl("https://93.184.216.34/photo.jpg");
expect(result).toHaveProperty("resolvedIp");
expect(typeof result.resolvedIp).toBe("string");
});
it("allows valid public HTTP URL without TLS", async () => {
const result = await validateFetchUrl("http://example.com/image.png");
const result = await validateFetchUrl("http://93.184.216.34/image.png");
expect(result).toHaveProperty("resolvedIp");
});
@@ -269,4 +269,22 @@ describe("safeFetch", () => {
"Redirect without Location header",
);
});
it("enforces maxBytes while reading HTTP responses", async () => {
mockFetch.mockResolvedValueOnce(
new Response(
new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(new Uint8Array(3));
controller.enqueue(new Uint8Array(3));
controller.close();
},
}),
{ status: 200 },
),
);
const res = await safeFetch("http://93.184.216.34/image.jpg", { maxBytes: 4 });
await expect(res.arrayBuffer()).rejects.toThrow("Response exceeds maximum size");
});
});
+1 -1
View File
@@ -63,7 +63,7 @@ vi.mock("../../../apps/api/src/routes/progress.js", () => ({
}));
vi.mock("../../../apps/api/src/plugins/auth.js", () => ({
getAuthUser: vi.fn(() => null),
getAuthUser: vi.fn(() => ({ id: "user-1", username: "test", role: "admin" })),
}));
vi.mock("../../../apps/api/src/lib/analytics.js", () => ({
+18
View File
@@ -932,6 +932,24 @@ describe("loadEnv", () => {
expect(env.CONCURRENT_JOBS).toBe(0);
});
it("defaults upload and batch limits to bounded values", async () => {
delete process.env.MAX_UPLOAD_SIZE_MB;
delete process.env.MAX_BATCH_SIZE;
const { loadEnv } = await import("../../../apps/api/src/lib/env.js");
const env = loadEnv();
expect(env.MAX_UPLOAD_SIZE_MB).toBe(100);
expect(env.MAX_BATCH_SIZE).toBe(100);
});
it("still accepts explicit 0 for unlimited upload and batch limits", async () => {
process.env.MAX_UPLOAD_SIZE_MB = "0";
process.env.MAX_BATCH_SIZE = "0";
const { loadEnv } = await import("../../../apps/api/src/lib/env.js");
const env = loadEnv();
expect(env.MAX_UPLOAD_SIZE_MB).toBe(0);
expect(env.MAX_BATCH_SIZE).toBe(0);
});
it("coerces negative numbers for numeric fields", async () => {
process.env.PORT = "-1";
const { loadEnv } = await import("../../../apps/api/src/lib/env.js");
+9
View File
@@ -1,9 +1,17 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const safeFetchMock = vi.hoisted(() => vi.fn());
vi.mock("../../../apps/api/src/lib/ssrf.js", () => ({
safeFetch: safeFetchMock,
}));
describe("webhook delivery", () => {
beforeEach(() => {
vi.restoreAllMocks();
vi.resetModules();
safeFetchMock.mockReset();
safeFetchMock.mockImplementation((url, options) => fetch(url, options));
});
afterEach(() => {
@@ -21,6 +29,7 @@ describe("webhook delivery", () => {
expect(result.success).toBe(true);
expect(result.attempts).toBe(1);
expect(safeFetchMock).toHaveBeenCalledOnce();
expect(fetchMock).toHaveBeenCalledOnce();
const [url, opts] = fetchMock.mock.calls[0];
expect(url).toBe("https://siem.example.com/input");