fix: resolve 6 bugs from enterprise audit (SIEM cursor, legal hold join, SCIM role, GDPR self-purge, export OOM, quota check)

This commit is contained in:
SnapOtter
2026-06-14 11:04:44 +08:00
parent b9dac59736
commit 7f62b1bf12
6 changed files with 64 additions and 32 deletions
+10 -11
View File
@@ -7,10 +7,10 @@
* cursor-based approach to avoid re-sending events. * cursor-based approach to avoid re-sending events.
* *
* State keys in the settings table: * State keys in the settings table:
* - siem_last_forwarded_id: cursor (last successfully forwarded audit log ID) * - siem_last_forwarded_at: cursor (last successfully forwarded createdAt timestamp)
* - siem_consecutive_failures: circuit breaker counter * - siem_consecutive_failures: circuit breaker counter
*/ */
import { asc, eq, gt } from "drizzle-orm"; import { asc, eq, gte } from "drizzle-orm";
import { env } from "../config.js"; import { env } from "../config.js";
import { db, schema } from "../db/index.js"; import { db, schema } from "../db/index.js";
import { decrypt, isEncrypted } from "../lib/encryption.js"; import { decrypt, isEncrypted } from "../lib/encryption.js";
@@ -19,7 +19,7 @@ import { readSiemConfig } from "../routes/enterprise/siem.js";
const BATCH_LIMIT = 500; const BATCH_LIMIT = 500;
const CIRCUIT_BREAKER_THRESHOLD = 5; const CIRCUIT_BREAKER_THRESHOLD = 5;
const CURSOR_KEY = "siem_last_forwarded_id"; const CURSOR_KEY = "siem_last_forwarded_at";
const FAILURES_KEY = "siem_consecutive_failures"; const FAILURES_KEY = "siem_consecutive_failures";
async function readSettingValue(key: string): Promise<string | null> { async function readSettingValue(key: string): Promise<string | null> {
@@ -31,10 +31,7 @@ async function readSettingValue(key: string): Promise<string | null> {
} }
async function upsertSetting(key: string, value: string): Promise<void> { async function upsertSetting(key: string, value: string): Promise<void> {
const [existing] = await db const [existing] = await db.select().from(schema.settings).where(eq(schema.settings.key, key));
.select()
.from(schema.settings)
.where(eq(schema.settings.key, key));
if (existing) { if (existing) {
await db await db
@@ -67,8 +64,10 @@ export async function runSiemForward(): Promise<{ forwarded: number } | void> {
// 3. Read cursor // 3. Read cursor
const cursor = await readSettingValue(CURSOR_KEY); const cursor = await readSettingValue(CURSOR_KEY);
// 4. Query audit_log for new rows // 4. Query audit_log for new rows (createdAt-based cursor; may re-deliver
const conditions = cursor ? gt(schema.auditLog.id, cursor) : undefined; // boundary events on restart, but SIEMs handle idempotent ingestion)
const cursorDate = cursor ? new Date(cursor) : null;
const conditions = cursorDate ? gte(schema.auditLog.createdAt, cursorDate) : undefined;
const rows = await db const rows = await db
.select() .select()
.from(schema.auditLog) .from(schema.auditLog)
@@ -104,8 +103,8 @@ export async function runSiemForward(): Promise<{ forwarded: number } | void> {
// 8. Update state based on result // 8. Update state based on result
if (result.success) { if (result.success) {
const lastId = rows[rows.length - 1].id; const lastRow = rows[rows.length - 1];
await upsertSetting(CURSOR_KEY, lastId); await upsertSetting(CURSOR_KEY, lastRow.createdAt.toISOString());
if (failureCount > 0) { if (failureCount > 0) {
await upsertSetting(FAILURES_KEY, "0"); await upsertSetting(FAILURES_KEY, "0");
} }
+3 -3
View File
@@ -145,7 +145,7 @@ async function storageTtlSweep(): Promise<{ removed: number; failed: number }> {
const heldUserIds = new Set(heldUserRows.map((r) => r.id)); const heldUserIds = new Set(heldUserRows.map((r) => r.id));
const heldTeamRows = await db const heldTeamRows = await db
.select({ name: schema.teams.name }) .select({ id: schema.teams.id })
.from(schema.teams) .from(schema.teams)
.where(eq(schema.teams.legalHold, true)); .where(eq(schema.teams.legalHold, true));
if (heldTeamRows.length > 0) { if (heldTeamRows.length > 0) {
@@ -155,7 +155,7 @@ async function storageTtlSweep(): Promise<{ removed: number; failed: number }> {
.where( .where(
inArray( inArray(
schema.users.team, schema.users.team,
heldTeamRows.map((r) => r.name), heldTeamRows.map((r) => r.id),
), ),
); );
for (const u of teamUsers) heldUserIds.add(u.id); for (const u of teamUsers) heldUserIds.add(u.id);
@@ -264,7 +264,7 @@ async function retentionSweep(): Promise<void> {
// Subquery to find users under legal hold (direct or via team) // Subquery to find users under legal hold (direct or via team)
const heldUsersSubquery = sql`( const heldUsersSubquery = sql`(
SELECT u.id FROM users u SELECT u.id FROM users u
LEFT JOIN teams t ON u.team = t.name LEFT JOIN teams t ON u.team = t.id
WHERE u.legal_hold = true OR t.legal_hold = true WHERE u.legal_hold = true OR t.legal_hold = true
)`; )`;
+13 -5
View File
@@ -50,14 +50,16 @@ export async function registerAuditExport(app: FastifyInstance): Promise<void> {
// Enterprise package not available // Enterprise package not available
} }
if (!featureEnabled) { if (!featureEnabled) {
return reply return reply.status(403).send({
.status(403) error: "Audit export requires an enterprise license with the audit_export feature",
.send({ error: "Audit export requires an enterprise license with the audit_export feature" }); });
} }
const parsed = querySchema.safeParse(request.query); const parsed = querySchema.safeParse(request.query);
if (!parsed.success) { if (!parsed.success) {
return reply.status(400).send({ error: "Invalid query parameters", details: parsed.error.issues }); return reply
.status(400)
.send({ error: "Invalid query parameters", details: parsed.error.issues });
} }
const { format, from, to, action, actorId, targetType, targetId } = parsed.data; const { format, from, to, action, actorId, targetType, targetId } = parsed.data;
@@ -84,11 +86,17 @@ export async function registerAuditExport(app: FastifyInstance): Promise<void> {
const where = conditions.length > 0 ? and(...conditions) : undefined; const where = conditions.length > 0 ? and(...conditions) : undefined;
const EXPORT_LIMIT = 100_000;
const entries = await db const entries = await db
.select() .select()
.from(schema.auditLog) .from(schema.auditLog)
.where(where) .where(where)
.orderBy(desc(schema.auditLog.createdAt)); .orderBy(desc(schema.auditLog.createdAt))
.limit(EXPORT_LIMIT);
if (entries.length === EXPORT_LIMIT) {
reply.header("X-Truncated", "true");
}
const rows = entries.map((e) => ({ const rows = entries.map((e) => ({
id: e.id, id: e.id,
+5
View File
@@ -223,6 +223,11 @@ export async function registerGdprRoutes(app: FastifyInstance): Promise<void> {
const targetUserId = request.params.id; const targetUserId = request.params.id;
// Guard: admin cannot purge themselves
if (targetUserId === user.id) {
return reply.status(400).send({ error: "Cannot purge your own account" });
}
// Check target user exists // Check target user exists
const [targetUser] = await db const [targetUser] = await db
.select({ .select({
+16 -12
View File
@@ -100,7 +100,7 @@ function toScimUser(
id: user.id, id: user.id,
userName: user.username, userName: user.username,
...(user.externalId ? { externalId: user.externalId } : {}), ...(user.externalId ? { externalId: user.externalId } : {}),
active: user.role !== "disabled", active: user.role !== "disabled" && !user.role.startsWith("disabled:"),
emails: user.email ? [{ value: user.email, primary: true }] : [], emails: user.email ? [{ value: user.email, primary: true }] : [],
name: { formatted: user.username }, name: { formatted: user.username },
groups: user.team ? [{ value: user.team, display: teamName ?? user.team }] : [], groups: user.team ? [{ value: user.team, display: teamName ?? user.team }] : [],
@@ -454,11 +454,13 @@ export async function registerScimRoutes(app: FastifyInstance): Promise<void> {
updates.email = email; updates.email = email;
} }
// Handle active/deactivation // Handle active/deactivation (preserve original role through disable/enable cycle)
if (active && existing.role === "disabled") { if (active && existing.role.startsWith("disabled:")) {
updates.role = "user"; updates.role = existing.role.slice("disabled:".length);
} else if (!active && existing.role !== "disabled") { } else if (active && existing.role === "disabled") {
updates.role = "disabled"; updates.role = "user"; // fallback when no previous role stored
} else if (!active && !existing.role.startsWith("disabled")) {
updates.role = `disabled:${existing.role}`;
// Revoke all sessions on deactivation // Revoke all sessions on deactivation
await db.delete(schema.sessions).where(eq(schema.sessions.userId, id)); await db.delete(schema.sessions).where(eq(schema.sessions.userId, id));
} }
@@ -527,10 +529,12 @@ export async function registerScimRoutes(app: FastifyInstance): Promise<void> {
const activeVal = const activeVal =
op.path === "active" ? op.value : (op.value as Record<string, unknown>).active; op.path === "active" ? op.value : (op.value as Record<string, unknown>).active;
const active = activeVal === true || activeVal === "true" || activeVal === "True"; const active = activeVal === true || activeVal === "true" || activeVal === "True";
if (active && existing.role === "disabled") { if (active && existing.role.startsWith("disabled:")) {
updates.role = "user"; updates.role = existing.role.slice("disabled:".length);
} else if (!active && existing.role !== "disabled") { } else if (active && existing.role === "disabled") {
updates.role = "disabled"; updates.role = "user"; // fallback when no previous role stored
} else if (!active && !existing.role.startsWith("disabled")) {
updates.role = `disabled:${existing.role}`;
await db.delete(schema.sessions).where(eq(schema.sessions.userId, id)); await db.delete(schema.sessions).where(eq(schema.sessions.userId, id));
} }
} }
@@ -605,11 +609,11 @@ export async function registerScimRoutes(app: FastifyInstance): Promise<void> {
return reply.status(404).send(scimError(404, "User not found")); return reply.status(404).send(scimError(404, "User not found"));
} }
// Soft-delete: set role to disabled, revoke sessions, clear password // Soft-delete: preserve original role so reactivation can restore it
await db await db
.update(schema.users) .update(schema.users)
.set({ .set({
role: "disabled", role: `disabled:${user.role}`,
passwordHash: null, passwordHash: null,
updatedAt: new Date(), updatedAt: new Date(),
}) })
+16
View File
@@ -261,6 +261,14 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
// Sanitize SVG uploads to prevent XXE, SSRF, and script injection // Sanitize SVG uploads to prevent XXE, SSRF, and script injection
const safeBuffer = isSvgBuffer(buffer) ? sanitizeSvg(buffer) : buffer; const safeBuffer = isSvgBuffer(buffer) ? sanitizeSvg(buffer) : buffer;
// Re-check quota with actual file size before persisting
try {
await checkStorageQuota(userId, safeBuffer.length);
} catch (err) {
const statusCode = (err as Error & { statusCode?: number }).statusCode ?? 413;
return reply.status(statusCode).send({ error: (err as Error).message });
}
const safeName = sanitizeFilename(part.filename ?? "upload"); const safeName = sanitizeFilename(part.filename ?? "upload");
const mimeType = formatToMime(validation.format); const mimeType = formatToMime(validation.format);
@@ -699,6 +707,14 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
// Sanitize SVG results to prevent XXE, SSRF, and script injection // Sanitize SVG results to prevent XXE, SSRF, and script injection
const safeResultBuffer = isSvgBuffer(fileBuffer) ? sanitizeSvg(fileBuffer) : fileBuffer; const safeResultBuffer = isSvgBuffer(fileBuffer) ? sanitizeSvg(fileBuffer) : fileBuffer;
// Re-check quota with actual file size before persisting
try {
await checkStorageQuota(userId, safeResultBuffer.length);
} catch (err) {
const statusCode = (err as Error & { statusCode?: number }).statusCode ?? 413;
return reply.status(statusCode).send({ error: (err as Error).message });
}
// Persist to disk // Persist to disk
const storedName = await saveFile(safeResultBuffer, resultName); const storedName = await saveFile(safeResultBuffer, resultName);