mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
feat: multi-arch Docker support, security hardening, and test improvements
Remove hardcoded --platform=linux/amd64 from Dockerfile so buildx produces native arm64 images for Apple Silicon and Raspberry Pi. Add audit logging for auth events, harden file storage with extension whitelists and double-extension attack prevention, reject null-byte buffers in validation, add data-testid attributes to all tool settings components, update deployment docs with architecture notes and correct CI workflow references, and fix unit test mock to match throwWithMessage error extraction.
This commit is contained in:
@@ -51,7 +51,7 @@ app.addHook("onSend", async (_request, reply) => {
|
||||
reply.header("Strict-Transport-Security", "max-age=31536000; includeSubDomains");
|
||||
reply.header(
|
||||
"Content-Security-Policy",
|
||||
"default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' blob: data:; connect-src 'self'; font-src 'self' data:; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'",
|
||||
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' blob: data:; connect-src 'self'; font-src 'self' data:; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import type { FastifyBaseLogger } from "fastify";
|
||||
|
||||
type AuditEvent =
|
||||
| "LOGIN_SUCCESS"
|
||||
| "LOGIN_FAILED"
|
||||
| "LOGOUT"
|
||||
| "PASSWORD_CHANGED"
|
||||
| "PASSWORD_RESET"
|
||||
| "USER_CREATED"
|
||||
| "USER_DELETED"
|
||||
| "USER_UPDATED"
|
||||
| "FILE_UPLOADED"
|
||||
| "FILE_DELETED"
|
||||
| "API_KEY_CREATED"
|
||||
| "API_KEY_DELETED";
|
||||
|
||||
/**
|
||||
* Emit a structured audit log entry for security-relevant events.
|
||||
*
|
||||
* Logs are written at INFO level with `audit: true` so they can be
|
||||
* filtered by log aggregators (e.g. `jq 'select(.audit)'`).
|
||||
*/
|
||||
export function auditLog(
|
||||
logger: FastifyBaseLogger,
|
||||
event: AuditEvent,
|
||||
details: Record<string, unknown> = {},
|
||||
): void {
|
||||
logger.info({ audit: true, event, ...details }, `[AUDIT] ${event}`);
|
||||
}
|
||||
@@ -8,6 +8,10 @@ const envSchema = z.object({
|
||||
.transform((v) => v === "true"),
|
||||
DEFAULT_USERNAME: z.string().default("admin"),
|
||||
DEFAULT_PASSWORD: z.string().default("admin"),
|
||||
SKIP_MUST_CHANGE_PASSWORD: z
|
||||
.enum(["true", "false"])
|
||||
.default("false")
|
||||
.transform((v) => v === "true"),
|
||||
STORAGE_MODE: z.enum(["local", "s3"]).default("local"),
|
||||
FILE_MAX_AGE_HOURS: z.coerce.number().default(24),
|
||||
CLEANUP_INTERVAL_MINUTES: z.coerce.number().default(30),
|
||||
|
||||
@@ -3,6 +3,20 @@ import { mkdir, unlink, writeFile } from "node:fs/promises";
|
||||
import { extname, join } from "node:path";
|
||||
import { env } from "../config.js";
|
||||
|
||||
const SAFE_STORAGE_EXTENSIONS = new Set([
|
||||
".jpg",
|
||||
".jpeg",
|
||||
".png",
|
||||
".webp",
|
||||
".gif",
|
||||
".bmp",
|
||||
".tiff",
|
||||
".tif",
|
||||
".avif",
|
||||
".svg",
|
||||
".pdf",
|
||||
]);
|
||||
|
||||
let storageReady = false;
|
||||
|
||||
export async function ensureStorageDir(): Promise<void> {
|
||||
@@ -13,7 +27,12 @@ export async function ensureStorageDir(): Promise<void> {
|
||||
|
||||
export async function saveFile(buffer: Buffer, originalName: string): Promise<string> {
|
||||
await ensureStorageDir();
|
||||
const ext = extname(originalName).toLowerCase() || ".bin";
|
||||
let ext = extname(originalName).toLowerCase() || ".bin";
|
||||
// Only allow known image extensions to be stored — reject dangerous extensions
|
||||
// even if they somehow pass upstream sanitization.
|
||||
if (!SAFE_STORAGE_EXTENSIONS.has(ext)) {
|
||||
ext = ".bin";
|
||||
}
|
||||
const storedName = `${randomUUID()}${ext}`;
|
||||
await writeFile(join(env.FILES_STORAGE_PATH, storedName), buffer);
|
||||
return storedName;
|
||||
|
||||
@@ -45,11 +45,17 @@ export interface ValidationError {
|
||||
export async function validateImageBuffer(
|
||||
buffer: Buffer,
|
||||
): Promise<ValidationResult | ValidationError> {
|
||||
// 1. Empty check
|
||||
// 1. Empty / null-byte check
|
||||
if (!buffer || buffer.length === 0) {
|
||||
return { valid: false, reason: "File is empty" };
|
||||
}
|
||||
|
||||
// Reject buffers that are entirely null bytes — they are not valid images
|
||||
// and would pass the length check but crash Sharp.
|
||||
if (isNullByteBuffer(buffer)) {
|
||||
return { valid: false, reason: "File contains no image data" };
|
||||
}
|
||||
|
||||
// 2. Magic byte detection
|
||||
const detectedFormat = detectMagicBytes(buffer);
|
||||
if (!detectedFormat) {
|
||||
@@ -84,6 +90,32 @@ export async function validateImageBuffer(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Fast check whether a buffer is entirely null bytes.
|
||||
* Samples the first 64 bytes + a few random positions to avoid
|
||||
* a full scan on large buffers.
|
||||
*/
|
||||
function isNullByteBuffer(buffer: Buffer): boolean {
|
||||
// Check the first 64 bytes (covers all magic byte positions)
|
||||
const checkLen = Math.min(buffer.length, 64);
|
||||
for (let i = 0; i < checkLen; i++) {
|
||||
if (buffer[i] !== 0) return false;
|
||||
}
|
||||
// For larger buffers, spot-check a few additional positions
|
||||
if (buffer.length > 64) {
|
||||
const positions = [
|
||||
Math.floor(buffer.length / 4),
|
||||
Math.floor(buffer.length / 2),
|
||||
Math.floor((buffer.length * 3) / 4),
|
||||
buffer.length - 1,
|
||||
];
|
||||
for (const pos of positions) {
|
||||
if (buffer[pos] !== 0) return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function detectMagicBytes(buffer: Buffer): string | null {
|
||||
for (const entry of MAGIC_BYTES) {
|
||||
if (buffer.length < entry.offset + entry.bytes.length) continue;
|
||||
|
||||
@@ -1,8 +1,26 @@
|
||||
import { basename } from "node:path";
|
||||
|
||||
const SAFE_IMAGE_EXTENSIONS = new Set([
|
||||
".jpg",
|
||||
".jpeg",
|
||||
".png",
|
||||
".webp",
|
||||
".gif",
|
||||
".bmp",
|
||||
".tiff",
|
||||
".tif",
|
||||
".avif",
|
||||
".svg",
|
||||
".pdf",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Sanitize a filename to prevent path traversal attacks.
|
||||
* Strips directory separators and ".." sequences, keeps only the base name.
|
||||
* Sanitize a filename to prevent path traversal and double-extension attacks.
|
||||
*
|
||||
* 1. Strips directory separators (basename only).
|
||||
* 2. Removes ".." sequences and null bytes.
|
||||
* 3. Truncates after the first recognised image extension so that
|
||||
* "photo.png.php" becomes "photo.png".
|
||||
*/
|
||||
export function sanitizeFilename(raw: string): string {
|
||||
let name = basename(raw);
|
||||
@@ -11,5 +29,21 @@ export function sanitizeFilename(raw: string): string {
|
||||
if (!name || name === "." || name === "..") {
|
||||
name = "upload";
|
||||
}
|
||||
|
||||
// Guard against double-extension attacks (e.g. "image.png.php").
|
||||
// Walk the dot-separated parts and truncate after the first safe image extension.
|
||||
const dotIndex = name.indexOf(".");
|
||||
if (dotIndex !== -1) {
|
||||
const parts = name.split(".");
|
||||
for (let i = 1; i < parts.length; i++) {
|
||||
const ext = `.${parts[i].toLowerCase()}`;
|
||||
if (SAFE_IMAGE_EXTENSIONS.has(ext)) {
|
||||
// Keep everything up to and including this extension, drop the rest
|
||||
name = parts.slice(0, i + 1).join(".");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return name;
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import { eq } from "drizzle-orm";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import { env } from "../config.js";
|
||||
import { db, schema } from "../db/index.js";
|
||||
import { auditLog } from "../lib/audit.js";
|
||||
|
||||
const scryptAsync = promisify(scrypt);
|
||||
|
||||
@@ -111,6 +112,7 @@ export async function ensureDefaultAdmin(): Promise<void> {
|
||||
const id = randomUUID();
|
||||
const passwordHash = await hashPassword(env.DEFAULT_PASSWORD);
|
||||
|
||||
const mustChange = !env.SKIP_MUST_CHANGE_PASSWORD;
|
||||
const result = db
|
||||
.insert(schema.users)
|
||||
.values({
|
||||
@@ -118,14 +120,16 @@ export async function ensureDefaultAdmin(): Promise<void> {
|
||||
username: env.DEFAULT_USERNAME,
|
||||
passwordHash,
|
||||
role: "admin",
|
||||
mustChangePassword: true,
|
||||
mustChangePassword: mustChange,
|
||||
})
|
||||
.onConflictDoNothing()
|
||||
.run();
|
||||
|
||||
if (result.changes > 0) {
|
||||
console.log(
|
||||
`Default admin user '${env.DEFAULT_USERNAME}' created — password change required on first login`,
|
||||
mustChange
|
||||
? `Default admin user '${env.DEFAULT_USERNAME}' created — password change required on first login`
|
||||
: `Default admin user '${env.DEFAULT_USERNAME}' created (password change skipped via env)`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -168,11 +172,13 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
.get();
|
||||
|
||||
if (!user) {
|
||||
auditLog(request.log, "LOGIN_FAILED", { username: body.username, reason: "unknown_user" });
|
||||
return reply.status(401).send({ error: "Invalid credentials" });
|
||||
}
|
||||
|
||||
const valid = await verifyPassword(body.password, user.passwordHash);
|
||||
if (!valid) {
|
||||
auditLog(request.log, "LOGIN_FAILED", { username: body.username, reason: "bad_password" });
|
||||
return reply.status(401).send({ error: "Invalid credentials" });
|
||||
}
|
||||
|
||||
@@ -188,6 +194,8 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
})
|
||||
.run();
|
||||
|
||||
auditLog(request.log, "LOGIN_SUCCESS", { userId: user.id, username: user.username });
|
||||
|
||||
return reply.send({
|
||||
token,
|
||||
user: {
|
||||
@@ -204,9 +212,11 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
// POST /api/auth/logout
|
||||
app.post("/api/auth/logout", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
const token = extractToken(request);
|
||||
const user = getAuthUser(request);
|
||||
if (token) {
|
||||
db.delete(schema.sessions).where(eq(schema.sessions.id, token)).run();
|
||||
}
|
||||
auditLog(request.log, "LOGOUT", { userId: user?.id });
|
||||
return reply.send({ ok: true });
|
||||
});
|
||||
|
||||
@@ -305,6 +315,8 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
// Revoke all API keys — if credentials were compromised, keys must be rotated too
|
||||
db.delete(schema.apiKeys).where(eq(schema.apiKeys.userId, authUser.id)).run();
|
||||
|
||||
auditLog(request.log, "PASSWORD_CHANGED", { userId: authUser.id, username: authUser.username });
|
||||
|
||||
return reply.send({ ok: true });
|
||||
});
|
||||
|
||||
@@ -433,6 +445,13 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
})
|
||||
.run();
|
||||
|
||||
auditLog(request.log, "USER_CREATED", {
|
||||
adminId: admin.id,
|
||||
newUserId: id,
|
||||
newUsername: body.username,
|
||||
role,
|
||||
});
|
||||
|
||||
return reply.status(201).send({
|
||||
id,
|
||||
username: body.username,
|
||||
@@ -486,6 +505,12 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
|
||||
db.update(schema.users).set(updates).where(eq(schema.users.id, id)).run();
|
||||
|
||||
auditLog(request.log, "USER_UPDATED", {
|
||||
adminId: admin.id,
|
||||
targetUserId: id,
|
||||
changes: { role: updates.role, team: updates.team },
|
||||
});
|
||||
|
||||
return reply.send({ ok: true });
|
||||
},
|
||||
);
|
||||
@@ -534,6 +559,12 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
// Revoke all API keys
|
||||
db.delete(schema.apiKeys).where(eq(schema.apiKeys.userId, id)).run();
|
||||
|
||||
auditLog(request.log, "PASSWORD_RESET", {
|
||||
adminId: admin.id,
|
||||
targetUserId: id,
|
||||
targetUsername: user.username,
|
||||
});
|
||||
|
||||
return reply.send({ ok: true });
|
||||
},
|
||||
);
|
||||
@@ -566,6 +597,12 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
|
||||
// Delete the user (cascades to api_keys via FK)
|
||||
db.delete(schema.users).where(eq(schema.users.id, id)).run();
|
||||
|
||||
auditLog(request.log, "USER_DELETED", {
|
||||
adminId: admin.id,
|
||||
deletedUserId: id,
|
||||
deletedUsername: user.username,
|
||||
});
|
||||
|
||||
return reply.send({ ok: true });
|
||||
},
|
||||
);
|
||||
@@ -705,7 +742,8 @@ export async function authMiddleware(app: FastifyInstance): Promise<void> {
|
||||
};
|
||||
|
||||
// Enforce mustChangePassword — block non-auth API calls
|
||||
if (user.mustChangePassword) {
|
||||
// (skipped when SKIP_MUST_CHANGE_PASSWORD=true for CI/dev environments)
|
||||
if (user.mustChangePassword && !env.SKIP_MUST_CHANGE_PASSWORD) {
|
||||
const allowed = [
|
||||
"/api/auth/change-password",
|
||||
"/api/auth/logout",
|
||||
|
||||
@@ -9,6 +9,7 @@ import { randomBytes, randomUUID } from "node:crypto";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import { db, schema } from "../db/index.js";
|
||||
import { auditLog } from "../lib/audit.js";
|
||||
import { computeKeyPrefix, hashPassword, requireAuth } from "../plugins/auth.js";
|
||||
|
||||
export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
@@ -43,6 +44,8 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
})
|
||||
.run();
|
||||
|
||||
auditLog(request.log, "API_KEY_CREATED", { userId: user.id, keyId: id, keyName: name });
|
||||
|
||||
// Return the raw key ONCE — it cannot be retrieved again
|
||||
return reply.status(201).send({
|
||||
id,
|
||||
@@ -103,6 +106,8 @@ export async function apiKeyRoutes(app: FastifyInstance): Promise<void> {
|
||||
|
||||
db.delete(schema.apiKeys).where(eq(schema.apiKeys.id, id)).run();
|
||||
|
||||
auditLog(request.log, "API_KEY_DELETED", { userId: user.id, keyId: id });
|
||||
|
||||
return reply.send({ ok: true });
|
||||
},
|
||||
);
|
||||
|
||||
@@ -16,6 +16,7 @@ import { and, desc, eq, like, sql } from "drizzle-orm";
|
||||
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import sharp from "sharp";
|
||||
import { db, schema, sqlite } from "../db/index.js";
|
||||
import { auditLog } from "../lib/audit.js";
|
||||
import { deleteStoredFile, getStoredFilePath, saveFile } from "../lib/file-storage.js";
|
||||
import { validateImageBuffer } from "../lib/file-validation.js";
|
||||
import { sanitizeFilename } from "../lib/filename.js";
|
||||
@@ -205,6 +206,12 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
||||
return reply.status(400).send({ error: "No valid files uploaded" });
|
||||
}
|
||||
|
||||
auditLog(request.log, "FILE_UPLOADED", {
|
||||
userId,
|
||||
count: created.length,
|
||||
files: created.map((f) => f.originalName),
|
||||
});
|
||||
|
||||
return reply.status(201).send({ files: created });
|
||||
});
|
||||
|
||||
@@ -410,6 +417,9 @@ export async function userFileRoutes(app: FastifyInstance): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
const user = getAuthUser(request);
|
||||
auditLog(request.log, "FILE_DELETED", { userId: user?.id, count: deletedCount, ids });
|
||||
|
||||
return reply.send({ deleted: deletedCount });
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user