mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(api): authorize the standalone upload and preview routes (#707)
POST /api/v1/upload, POST /api/v1/preview and POST /api/v1/preview/generate authenticated but never authorized, sitting between requireFileAccess (which guards /api/v1/files) and toolAccessMiddleware (scoped to /api/v1/tools/). A key scoped to settings:read alone could still stage bytes behind the unauthenticated download URL and drive Sharp, libheif, LibRaw and FFmpeg. Upload now takes requireFileAccess; both preview routes take tools:use. requireFileAccess moves to permissions.ts so the modules share one definition.
This commit is contained in:
@@ -384,6 +384,44 @@ export async function requireToolAccess(
|
||||
return user;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate the file routes on the file permissions.
|
||||
*
|
||||
* Either grant is enough: `files:all` is the broader one, so a holder of it is
|
||||
* never locked out for lacking `files:own`. This is the same rule
|
||||
* `requireApiKeyManagement` in routes/api-keys.ts already applies to the
|
||||
* identical apikeys:own / apikeys:all pair, and the roles editor presents Files
|
||||
* and API Keys as the same shape of group.
|
||||
*
|
||||
* Runs before the resource is resolved so a caller without the permission gets
|
||||
* 403 rather than a 404 that would confirm whether an id exists.
|
||||
*
|
||||
* Lives here rather than in routes/user-files.ts because /api/v1/upload needs the
|
||||
* same gate and is registered outside that module.
|
||||
*
|
||||
* See SEC-20260726-C01.
|
||||
*/
|
||||
export async function requireFileAccess(
|
||||
request: FastifyRequest,
|
||||
reply: FastifyReply,
|
||||
): Promise<AuthUser | null> {
|
||||
const user = getAuthUser(request);
|
||||
if (!user) {
|
||||
reply.status(401).send({ error: "Authentication required", code: "AUTH_REQUIRED" });
|
||||
return null;
|
||||
}
|
||||
|
||||
if (
|
||||
!(await hasEffectivePermission(user, "files:own")) &&
|
||||
!(await hasEffectivePermission(user, "files:all"))
|
||||
) {
|
||||
reply.status(403).send({ error: "Insufficient permissions", code: "FORBIDDEN" });
|
||||
return null;
|
||||
}
|
||||
|
||||
return user;
|
||||
}
|
||||
|
||||
export async function requireOwnershipOrPermission(
|
||||
request: FastifyRequest,
|
||||
reply: FastifyReply,
|
||||
|
||||
@@ -17,8 +17,8 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
|
||||
import { env } from "../config.js";
|
||||
import { db, schema } from "../db/index.js";
|
||||
import { getStoredFilePath } from "../lib/file-storage.js";
|
||||
import { hasEffectivePermission } from "../permissions.js";
|
||||
import { getAuthUser, requireAuth } from "../plugins/auth.js";
|
||||
import { hasEffectivePermission, requirePermission } from "../permissions.js";
|
||||
import { requireAuth } from "../plugins/auth.js";
|
||||
|
||||
const PREVIEW_DIR = ".previews";
|
||||
let previewDirReady = false;
|
||||
@@ -243,8 +243,11 @@ export async function filePreviewRoutes(app: FastifyInstance): Promise<void> {
|
||||
config: { rateLimit: { max: 60, timeWindow: "1 minute" } },
|
||||
},
|
||||
async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
// Optional auth -- the preview is for the user's own uploaded file
|
||||
getAuthUser(request);
|
||||
// Spawns FFmpeg over caller-supplied bytes for the tool workflow, so it takes
|
||||
// the same grant as running a tool. This used to call getAuthUser and discard
|
||||
// the result, leaving it open to any authenticated principal no matter what
|
||||
// the operator had scoped them down to.
|
||||
if (!(await requirePermission("tools:use")(request, reply))) return;
|
||||
|
||||
const parts = request.parts();
|
||||
let fileBuffer: Buffer | null = null;
|
||||
|
||||
@@ -10,6 +10,7 @@ import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
||||
import { decodeHeic } from "../lib/heic-converter.js";
|
||||
import { getObjectSize, getObjectStream, putObject } from "../lib/object-storage.js";
|
||||
import { isSvgBuffer, sanitizeSvg } from "../lib/svg-sanitize.js";
|
||||
import { requireFileAccess, requirePermission } from "../permissions.js";
|
||||
|
||||
/**
|
||||
* Guard against path traversal in URL params.
|
||||
@@ -85,6 +86,11 @@ export async function fileRoutes(app: FastifyInstance): Promise<void> {
|
||||
"/api/v1/upload",
|
||||
{ config: { rateLimit: { max: 60, timeWindow: "1 minute" } } },
|
||||
async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
// Persists bytes that /api/v1/download then serves without auth, so this
|
||||
// needs the same grant as the rest of the file surface. The tool-access
|
||||
// middleware only covers /api/v1/tools/, and never reached this route.
|
||||
if (!(await requireFileAccess(request, reply))) return;
|
||||
|
||||
const jobId = randomUUID();
|
||||
|
||||
const uploadedFiles: Array<{
|
||||
@@ -231,6 +237,10 @@ export async function fileRoutes(app: FastifyInstance): Promise<void> {
|
||||
// ── POST /api/v1/preview ──────────────────────────────────────
|
||||
// Returns a WebP preview for formats browsers can't display (HEIC/HEIF).
|
||||
app.post("/api/v1/preview", async (request: FastifyRequest, reply: FastifyReply) => {
|
||||
// Decodes attacker-supplied bytes through Sharp, libheif and LibRaw on behalf
|
||||
// of the tool workflow, so it belongs behind the same grant as running a tool.
|
||||
if (!(await requirePermission("tools:use")(request, reply))) return;
|
||||
|
||||
const data = await request.file();
|
||||
if (!data) {
|
||||
return reply.status(400).send({ error: "No file provided" });
|
||||
|
||||
@@ -33,8 +33,7 @@ import { decodeToSharpCompat, needsCliDecode } from "../lib/format-decoders.js";
|
||||
import { decodeHeic } from "../lib/heic-converter.js";
|
||||
import { isSvgBuffer, sanitizeSvg } from "../lib/svg-sanitize.js";
|
||||
import { pdfFirstPagePreview, videoPosterPreview } from "../modality/preview.js";
|
||||
import { hasEffectivePermission } from "../permissions.js";
|
||||
import { type AuthUser, requireAuth } from "../plugins/auth.js";
|
||||
import { hasEffectivePermission, requireFileAccess } from "../permissions.js";
|
||||
|
||||
// ── Helpers ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -117,38 +116,6 @@ function serializeFile(row: typeof schema.userFiles.$inferSelect) {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate the file-library routes on the file permissions.
|
||||
*
|
||||
* Either grant is enough: `files:all` is the broader one, so a holder of it is
|
||||
* never locked out for lacking `files:own`. This is the same rule
|
||||
* `requireApiKeyManagement` in routes/api-keys.ts already applies to the
|
||||
* identical apikeys:own / apikeys:all pair, and the roles editor presents Files
|
||||
* and API Keys as the same shape of group.
|
||||
*
|
||||
* Runs before the resource is resolved so a caller without the permission gets
|
||||
* 403 rather than a 404 that would confirm whether an id exists.
|
||||
*
|
||||
* See SEC-20260726-C01.
|
||||
*/
|
||||
async function requireFileAccess(
|
||||
request: FastifyRequest,
|
||||
reply: FastifyReply,
|
||||
): Promise<AuthUser | null> {
|
||||
const user = requireAuth(request, reply);
|
||||
if (!user) return null;
|
||||
|
||||
if (
|
||||
!(await hasEffectivePermission(user, "files:own")) &&
|
||||
!(await hasEffectivePermission(user, "files:all"))
|
||||
) {
|
||||
reply.status(403).send({ error: "Insufficient permissions", code: "FORBIDDEN" });
|
||||
return null;
|
||||
}
|
||||
|
||||
return user;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a user (and their team) has exceeded their storage quota.
|
||||
* Uses the pre-computed storageUsed counter on the users table.
|
||||
|
||||
Reference in New Issue
Block a user