mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix: first-run QA sweep of the single-container image (#413)
Fixes found by manually testing a fresh install end to end: - auth: the must-change-password gate returned 403 on public routes including /api/v1/health, so every fresh install showed a false "Reconnecting to server" banner on the forced password change screen. Public routes are now exempt (they need no session at all). Adds the gate's first direct tests. - multipart: @fastify/multipart's parts() iterator (9.4.0 and 10.0.0) ends on the request stream's "close", which on a reused keep-alive connection fires while an earlier part is still streaming to storage, silently dropping the parts behind it. The object eraser lost its mask file on every second POST per connection. Replaced with a busboy-driven iterator (lib/multipart-parts.ts) that ends on busboy's own "finish", installed for all routes via a preValidation hook; the tool-factory field-recovery workaround for the same bug is now unnecessary and removed. - eraser: the mask canvas backing store is natural resolution, but "absolute inset-0" does not stretch replaced elements, so the canvas rendered at intrinsic size and the brush ring, strokes, and exported mask were all misscaled on photos larger than the viewport. The canvas now gets an explicit CSS box at the fitted size. - compare slider: solid white divider with a dark halo so it stays visible over light images; still initialised at the painted region. - tool page: the AI bundle install prompt now centers in the content area instead of hugging the top. - api docs: disabled Scalar's cloud features (Ask AI, Generate MCP, Open API Client, dev toolbar), hid the "Powered by Scalar" footer link, and set the page title to "SnapOtter API Reference". The docs CSP blocks those cloud calls by design, so the buttons were dead UI. - docker: embedded Redis comes from packages.redis.io pinned to the 8.x major (was Debian's 7.0.15), matching the Compose stack and the documented claim. Build fails fast if the major ever drifts. - docs: DOCKERHUB.md quick start now leads with the one-command docker run (matching the README) with Compose as the production path; README says embedded Postgres 17 + Redis 8. Claude-Session: https://claude.ai/code/session_01XGB4pGvTvb7sUX4JN745U7
This commit is contained in:
@@ -13,6 +13,7 @@
|
||||
"migrate:sqlite": "tsx src/db/migrate-from-sqlite.ts"
|
||||
},
|
||||
"dependencies": {
|
||||
"@fastify/busboy": "^3.2.0",
|
||||
"@fastify/cookie": "^11.0.2",
|
||||
"@fastify/cors": "^11.0.0",
|
||||
"@fastify/multipart": "^9.0.0",
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
import type { Readable } from "node:stream";
|
||||
import { Busboy, type BusboyHeaders } from "@fastify/busboy";
|
||||
import type { FastifyRequest } from "fastify";
|
||||
import { env } from "../config.js";
|
||||
|
||||
export interface MultipartFilePart {
|
||||
type: "file";
|
||||
fieldname: string;
|
||||
filename: string;
|
||||
encoding: string;
|
||||
mimetype: string;
|
||||
file: Readable;
|
||||
}
|
||||
|
||||
export interface MultipartFieldPart {
|
||||
type: "field";
|
||||
fieldname: string;
|
||||
value: string;
|
||||
}
|
||||
|
||||
export type MultipartPart = MultipartFilePart | MultipartFieldPart;
|
||||
|
||||
const DONE = Symbol("multipart-done");
|
||||
|
||||
/**
|
||||
* Iterate multipart parts by driving busboy directly.
|
||||
*
|
||||
* Replaces @fastify/multipart's request.parts(): that iterator treats the
|
||||
* REQUEST stream's "close" event as end-of-parts, and on a reused keep-alive
|
||||
* connection the whole body can be read (firing "close") while the consumer
|
||||
* is still streaming an earlier part to storage. Every part busboy emits
|
||||
* after that moment lands behind the end marker and is silently dropped; in
|
||||
* practice the second multipart POST on a warm connection lost its trailing
|
||||
* parts (the object eraser's mask file, then the settings fields). Verified
|
||||
* against @fastify/multipart 9.4.0 and 10.0.0. Busboy's own "finish" fires
|
||||
* only after every part has been emitted, so iteration ends there instead,
|
||||
* and the request stream's "close" is deliberately not treated as an end
|
||||
* signal (a client abort surfaces as an "error" on the stream and as a
|
||||
* truncated-part error from busboy).
|
||||
*/
|
||||
export async function* multipartParts(request: FastifyRequest): AsyncGenerator<MultipartPart> {
|
||||
const raw = request.raw;
|
||||
const bb = new Busboy({
|
||||
headers: raw.headers as BusboyHeaders,
|
||||
limits: {
|
||||
fileSize: env.MAX_UPLOAD_SIZE_MB > 0 ? env.MAX_UPLOAD_SIZE_MB * 1024 * 1024 : undefined,
|
||||
files: env.MAX_BATCH_SIZE > 0 ? env.MAX_BATCH_SIZE : undefined,
|
||||
},
|
||||
});
|
||||
|
||||
const queue: Array<MultipartPart | Error | typeof DONE> = [];
|
||||
let wake: (() => void) | null = null;
|
||||
const push = (value: MultipartPart | Error | typeof DONE) => {
|
||||
queue.push(value);
|
||||
wake?.();
|
||||
wake = null;
|
||||
};
|
||||
|
||||
bb.on("file", (fieldname, stream, filename, encoding, mimetype) => {
|
||||
// Parity with @fastify/multipart's throwFileSizeLimit default: a stream
|
||||
// that hit the fileSize limit fails its consumer instead of silently
|
||||
// truncating the stored object.
|
||||
stream.on("limit", () => stream.destroy(new Error("request file too large")));
|
||||
push({
|
||||
type: "file",
|
||||
fieldname,
|
||||
filename: filename || "upload",
|
||||
encoding,
|
||||
mimetype,
|
||||
file: stream,
|
||||
});
|
||||
});
|
||||
bb.on("field", (fieldname, value) => push({ type: "field", fieldname, value }));
|
||||
bb.on("filesLimit", () => push(new Error("reached files limit")));
|
||||
bb.on("partsLimit", () => push(new Error("reached parts limit")));
|
||||
bb.on("error", (err: unknown) => push(err instanceof Error ? err : new Error(String(err))));
|
||||
bb.on("finish", () => push(DONE));
|
||||
raw.on("error", (err: Error) => push(err));
|
||||
|
||||
raw.pipe(bb);
|
||||
|
||||
try {
|
||||
while (true) {
|
||||
if (queue.length === 0) {
|
||||
await new Promise<void>((resolve) => {
|
||||
wake = resolve;
|
||||
});
|
||||
}
|
||||
const value = queue.shift();
|
||||
if (value === undefined) continue;
|
||||
if (value === DONE) return;
|
||||
if (value instanceof Error) throw value;
|
||||
yield value;
|
||||
}
|
||||
} finally {
|
||||
raw.unpipe(bb);
|
||||
bb.removeAllListeners();
|
||||
}
|
||||
}
|
||||
@@ -1243,21 +1243,17 @@ export async function authMiddleware(app: FastifyInstance): Promise<void> {
|
||||
role: user.role,
|
||||
};
|
||||
|
||||
// Enforce mustChangePassword — block non-auth API calls
|
||||
// (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",
|
||||
"/api/auth/session",
|
||||
"/api/v1/config/",
|
||||
];
|
||||
if (!allowed.some((p) => request.url.startsWith(p)) && request.url.startsWith("/api/")) {
|
||||
return reply.status(403).send({
|
||||
error: "Password change required",
|
||||
code: "MUST_CHANGE_PASSWORD",
|
||||
});
|
||||
}
|
||||
// Enforce mustChangePassword by blocking the authenticated API surface
|
||||
// until the password is rotated. Public routes stay reachable: they need
|
||||
// no session at all, so a 403 on the cookied variant adds no security and
|
||||
// breaks the SPA (the /api/v1/health poll used to trip a false
|
||||
// "Reconnecting to server" banner on the forced change-password screen).
|
||||
// Skipped when SKIP_MUST_CHANGE_PASSWORD=true for CI/dev environments.
|
||||
if (user.mustChangePassword && !env.SKIP_MUST_CHANGE_PASSWORD && !isPublic) {
|
||||
return reply.status(403).send({
|
||||
error: "Password change required",
|
||||
code: "MUST_CHANGE_PASSWORD",
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import multipart from "@fastify/multipart";
|
||||
import type { FastifyInstance } from "fastify";
|
||||
import { env } from "../config.js";
|
||||
import { multipartParts } from "../lib/multipart-parts.js";
|
||||
|
||||
export async function registerUpload(app: FastifyInstance): Promise<void> {
|
||||
await app.register(multipart, {
|
||||
@@ -9,4 +10,21 @@ export async function registerUpload(app: FastifyInstance): Promise<void> {
|
||||
files: env.MAX_BATCH_SIZE > 0 ? env.MAX_BATCH_SIZE : undefined,
|
||||
},
|
||||
});
|
||||
|
||||
// Swap the plugin's request.parts() for a busboy-driven iterator
|
||||
// (lib/multipart-parts.ts). The plugin's iterator ends when the REQUEST
|
||||
// stream closes, which on a reused keep-alive connection fires while an
|
||||
// earlier part is still streaming to storage, silently dropping every part
|
||||
// behind it (the object eraser lost its mask file on the second POST per
|
||||
// connection; dropped trailing settings fields were papered over by a
|
||||
// recovery workaround in tool-factory). request.file() callers
|
||||
// (features.ts, files.ts) stay on the plugin: the first part is always
|
||||
// emitted before the premature end marker can be queued, and field values
|
||||
// are recovered from busboy's side map.
|
||||
app.addHook("preValidation", async (request) => {
|
||||
if (request.isMultipart()) {
|
||||
const fixedParts = (() => multipartParts(request)) as unknown as typeof request.parts;
|
||||
(request as { parts: typeof request.parts }).parts = fixedParts;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
+32
-11
@@ -1,7 +1,7 @@
|
||||
import { readFileSync } from "node:fs";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import scalarPlugin from "@scalar/fastify-api-reference";
|
||||
import scalarPlugin, { type FastifyApiReferenceOptions } from "@scalar/fastify-api-reference";
|
||||
import { SECTIONS, TOOLS, toolSection } from "@snapotter/shared";
|
||||
import type { FastifyInstance } from "fastify";
|
||||
import yaml from "js-yaml";
|
||||
@@ -173,12 +173,23 @@ export async function docsRoutes(app: FastifyInstance): Promise<void> {
|
||||
reply.type("text/yaml").send(specContent);
|
||||
});
|
||||
|
||||
await app.register(scalarPlugin, {
|
||||
routePrefix: "/api/docs",
|
||||
configuration: {
|
||||
content: specContent,
|
||||
theme: "default",
|
||||
customCss: `
|
||||
// Scalar's "Ask AI" (Agent Scalar), "Generate MCP", "Open API Client", and
|
||||
// the Configure/Share/Deploy toolbar are all Scalar cloud features: they
|
||||
// upload or link the OpenAPI document to scalar.com, which the docs CSP
|
||||
// blocks by design (self-hosted docs make no external calls). Hide them
|
||||
// instead of shipping dead UI. `agent` is a source-level key the plugin's
|
||||
// configuration type doesn't list yet, hence the widened type.
|
||||
const configuration: NonNullable<FastifyApiReferenceOptions["configuration"]> & {
|
||||
agent?: { disabled?: boolean };
|
||||
} = {
|
||||
content: specContent,
|
||||
pageTitle: "SnapOtter API Reference",
|
||||
agent: { disabled: true },
|
||||
mcp: { disabled: true },
|
||||
hideClientButton: true,
|
||||
showDeveloperTools: "never",
|
||||
theme: "default",
|
||||
customCss: `
|
||||
:root {
|
||||
--scalar-color-1: #09090b;
|
||||
--scalar-color-2: #3f3f46;
|
||||
@@ -190,10 +201,20 @@ export async function docsRoutes(app: FastifyInstance): Promise<void> {
|
||||
--scalar-border-color: #e4e4e7;
|
||||
--scalar-font: system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif;
|
||||
}
|
||||
/* Hide the "Powered by Scalar" sidebar footer link. Scalar exposes no
|
||||
config flag for it (unlike the cloud buttons disabled above). */
|
||||
a[href^="https://www.scalar.com"],
|
||||
a[href^="https://scalar.com"] {
|
||||
display: none !important;
|
||||
}
|
||||
`,
|
||||
hideDownloadButton: false,
|
||||
hideTestRequestButton: true,
|
||||
hiddenClients: true,
|
||||
},
|
||||
hideDownloadButton: false,
|
||||
hideTestRequestButton: true,
|
||||
hiddenClients: true,
|
||||
};
|
||||
|
||||
await app.register(scalarPlugin, {
|
||||
routePrefix: "/api/docs",
|
||||
configuration,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -249,15 +249,15 @@ export function createToolRoute<T>(app: FastifyInstance, config: ToolRouteConfig
|
||||
let clientJobId: string | null = null;
|
||||
let fileCount = 0;
|
||||
const received: ReceivedUpload[] = [];
|
||||
// Track last part for post-loop field recovery
|
||||
let lastPart: { fields?: Record<string, unknown> } | undefined;
|
||||
|
||||
// Parse multipart parts (file parts stream to object storage)
|
||||
// Parse multipart parts (file parts stream to object storage).
|
||||
// request.parts() is the keep-alive-safe iterator from
|
||||
// lib/multipart-parts.ts (installed in plugins/upload.ts), which never
|
||||
// drops trailing parts, so no post-loop field recovery is needed.
|
||||
try {
|
||||
const parts = request.parts();
|
||||
|
||||
for await (const part of parts) {
|
||||
lastPart = part as { fields?: Record<string, unknown> };
|
||||
if (part.type === "file") {
|
||||
fileCount++;
|
||||
if (fileCount > maxInputs) {
|
||||
@@ -291,31 +291,6 @@ export function createToolRoute<T>(app: FastifyInstance, config: ToolRouteConfig
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The upstream parts iterator can terminate before yielding trailing
|
||||
// fields, but busboy has already populated part.fields on every part.
|
||||
if (lastPart?.fields) {
|
||||
const recover = (name: string): string | null => {
|
||||
const f = lastPart?.fields?.[name];
|
||||
const entry = Array.isArray(f) ? f[0] : f;
|
||||
if (entry != null && typeof (entry as { value?: unknown }).value === "string") {
|
||||
return (entry as { value: string }).value;
|
||||
}
|
||||
return null;
|
||||
};
|
||||
if (settingsRaw === null) {
|
||||
settingsRaw = recover("settings");
|
||||
}
|
||||
if (fileId === null) {
|
||||
fileId = recover("fileId");
|
||||
}
|
||||
if (clientJobId === null) {
|
||||
const raw = recover("clientJobId");
|
||||
if (raw !== null && raw.length > 0 && raw.length <= 128) {
|
||||
clientJobId = raw;
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
return reply.status(400).send({
|
||||
error: "Failed to parse multipart request",
|
||||
|
||||
Reference in New Issue
Block a user