Fixes

Fixed an issue where media manager only shows files uploaded in specific sections.

Fixed an issue with icon picker names.

Removed jumbo rounding and a few other style tweaks.

src/lib/icons/icon-catalog.ts

@@ -10,7 +10,6 @@ import {
   faCheck,
   faCircleCheck,
   faClock,
-  faCoffee,
   faCompass,
   faCrown,
   faDiamond,
@@ -76,7 +75,6 @@ export const ICON_CATALOG: IconDefinition[] = [
   faBolt,
   faSun,
   faLeaf,
-  faCoffee,
   faMugSaucer,
   faGlassCheers,
   faHandSparkles,

src/lib/media/__tests__/media-storage.test.ts

@@ -0,0 +1,98 @@
+import { afterAll, beforeAll, describe, expect, it } from "vitest";
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path from "node:path";
+
+/**
+ * Real filesystem I/O against a throwaway temp directory (via
+ * MEDIA_STORAGE_ROOT), not mocks — this module's whole job is getting
+ * real file paths right, so a mocked `fs` would test nothing useful. No
+ * DATABASE_URL/Prisma dependency here (media-storage.ts is DB-free), so
+ * this stays safe to run in the plain unit suite. The module is
+ * dynamically imported after MEDIA_STORAGE_ROOT is set, since it reads
+ * that env var once at module-load time.
+ */
+describe("media-storage", () => {
+  let tempRoot: string;
+  let storage: typeof import("../media-storage");
+
+  beforeAll(async () => {
+    tempRoot = await mkdtemp(path.join(tmpdir(), "nextchair-media-test-"));
+    process.env.MEDIA_STORAGE_ROOT = tempRoot;
+    storage = await import("../media-storage");
+  });
+
+  afterAll(async () => {
+    delete process.env.MEDIA_STORAGE_ROOT;
+    await rm(tempRoot, { recursive: true, force: true });
+  });
+
+  const PNG_BYTES = Buffer.from(
+    "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=",
+    "base64",
+  );
+
+  it("stores a file and returns a stable /api/media/ URL", async () => {
+    const result = await storage.storeMediaFile({ buffer: PNG_BYTES, mimeType: "image/png", category: "branding" });
+    expect(result.url).toMatch(/^\/api\/media\/branding\/[a-f0-9-]+\.png$/);
+  });
+
+  it("readStoredMediaFile resolves the stored file back with the right bytes and content type", async () => {
+    const stored = await storage.storeMediaFile({ buffer: PNG_BYTES, mimeType: "image/png", category: "gallery" });
+    const segments = stored.url.replace(storage.MEDIA_URL_PREFIX, "").split("/");
+
+    const file = await storage.readStoredMediaFile(segments);
+    expect(file).not.toBeNull();
+    expect(file?.mimeType).toBe("image/png");
+    expect(file?.buffer.equals(PNG_BYTES)).toBe(true);
+  });
+
+  it("readStoredMediaFile returns null for a file that was never written", async () => {
+    const file = await storage.readStoredMediaFile(["branding", "does-not-exist.png"]);
+    expect(file).toBeNull();
+  });
+
+  it("deleteMediaFile removes the file — a subsequent read returns null", async () => {
+    const stored = await storage.storeMediaFile({ buffer: PNG_BYTES, mimeType: "image/png", category: "staff" });
+    const segments = stored.url.replace(storage.MEDIA_URL_PREFIX, "").split("/");
+
+    expect(await storage.readStoredMediaFile(segments)).not.toBeNull();
+    await storage.deleteMediaFile(stored.url);
+    expect(await storage.readStoredMediaFile(segments)).toBeNull();
+  });
+
+  it("deleteMediaFile on an external (non-/api/media/) URL is a safe no-op", async () => {
+    await expect(storage.deleteMediaFile("https://example.com/logo.png")).resolves.toBeUndefined();
+  });
+
+  it("rejects unsupported mime types before ever touching the filesystem", async () => {
+    await expect(storage.storeMediaFile({ buffer: PNG_BYTES, mimeType: "image/svg+xml", category: "branding" })).rejects.toThrow();
+  });
+
+  it("blocks path traversal via '..' segments", () => {
+    expect(storage.resolveMediaFilePath(["..", "..", "etc", "passwd"])).toBeNull();
+    expect(storage.resolveMediaFilePath(["branding", ".."])).toBeNull();
+    expect(storage.resolveMediaFilePath(["branding", "..", "..", "package.json"])).toBeNull();
+  });
+
+  it("blocks segments that smuggle a path separator", () => {
+    expect(storage.resolveMediaFilePath(["branding/../../../etc/passwd"])).toBeNull();
+    expect(storage.resolveMediaFilePath(["a\\b"])).toBeNull();
+  });
+
+  it("readStoredMediaFile returns null for a path-traversal attempt even if a file exists there", async () => {
+    // Prove the guard rejects it via the real read path, not just the resolver in isolation.
+    const file = await storage.readStoredMediaFile(["..", "..", "..", "etc", "passwd"]);
+    expect(file).toBeNull();
+  });
+
+  it("mimeTypeForFilename covers every allowed extension and rejects unknown ones", () => {
+    expect(storage.mimeTypeForFilename("x.png")).toBe("image/png");
+    expect(storage.mimeTypeForFilename("x.jpg")).toBe("image/jpeg");
+    expect(storage.mimeTypeForFilename("x.jpeg")).toBe("image/jpeg");
+    expect(storage.mimeTypeForFilename("x.webp")).toBe("image/webp");
+    expect(storage.mimeTypeForFilename("x.gif")).toBe("image/gif");
+    expect(storage.mimeTypeForFilename("x.svg")).toBeNull();
+    expect(storage.mimeTypeForFilename("x")).toBeNull();
+  });
+});

src/lib/media/media-storage.ts

@@ -1,19 +1,30 @@
 import { randomUUID } from "node:crypto";
-import { mkdir, unlink, writeFile } from "node:fs/promises";
+import { access, mkdir, readFile, stat, unlink, writeFile } from "node:fs/promises";
 import path from "node:path";
 
 /**
- * Local-disk storage for the Media Manager (see src/services/media) —
- * files live under public/media/<category>/..., served for free by
- * Next's own static file handling for public/ (no custom GET route
- * needed). A deliberate v1 choice for this app's single-container Docker
- * deployment model, not a cloud object-storage integration — see
- * docker-compose.local.yaml's media volume for why this survives a
- * container restart. Swapping to S3-compatible storage later only means
- * changing this one file; every caller only ever sees a public URL.
+ * Local-disk storage for the Media Manager (see src/services/media).
+ *
+ * Files live under a storage root OUTSIDE `public/` (`storage/media` by
+ * default — `/app/storage/media` in the Docker image, matching the
+ * `next-chair-media-data` volume in docker-compose.local.yaml). This was
+ * previously `public/media/`, served by Next's own static file handling
+ * — that turned out not to work: verified directly against a real
+ * production build/container that Next.js 16's `next start` does NOT
+ * dynamically serve files added to `public/` after the build (a
+ * build-time-present file served 200; a file written into `public/media`
+ * at runtime served 404, even though it existed on disk with correct
+ * permissions). See src/app/api/media/[...path]/route.ts, the dedicated
+ * route that now serves these files itself instead of relying on Next's
+ * public-asset resolution.
  *
  * SVG is deliberately not in the allow-list — an uploaded SVG can embed
  * <script>, making it a stored-XSS vector if ever served inline.
+ *
+ * Kept provider-neutral by design: every caller only ever sees a
+ * MediaAsset.url string and calls storeMediaFile/readStoredMediaFile/
+ * deleteMediaFile — swapping this file for an S3-compatible backend
+ * later doesn't require touching any caller.
  */
 const ALLOWED_MIME_EXTENSIONS: Record<string, string> = {
   "image/png": "png",
@@ -22,17 +33,27 @@ const ALLOWED_MIME_EXTENSIONS: Record<string, string> = {
   "image/gif": "gif",
 };
 
+const EXTENSION_MIME_TYPES: Record<string, string> = {
+  png: "image/png",
+  jpg: "image/jpeg",
+  jpeg: "image/jpeg",
+  webp: "image/webp",
+  gif: "image/gif",
+};
+
 export const MEDIA_MAX_UPLOAD_BYTES = Number(process.env.MEDIA_MAX_UPLOAD_BYTES ?? 5_000_000);
 
-const MEDIA_ROOT = path.join(process.cwd(), "public", "media");
-const MEDIA_URL_PREFIX = "/media/";
+/** Overridable so a non-Docker deployment can point this somewhere else; defaults to a directory next to the app, outside public/. */
+const MEDIA_ROOT = path.resolve(process.env.MEDIA_STORAGE_ROOT || path.join(process.cwd(), "storage", "media"));
+
+export const MEDIA_URL_PREFIX = "/api/media/";
 
 export function isAllowedMediaMimeType(mimeType: string): boolean {
   return mimeType in ALLOWED_MIME_EXTENSIONS;
 }
 
 export interface StoredMediaFile {
-  /** Public path — what gets saved on MediaAsset.url and rendered directly. */
+  /** Public path — what gets saved on MediaAsset.url and rendered directly. Resolved by src/app/api/media/[...path]/route.ts, not Next's static handling. */
   url: string;
 }
 
@@ -40,6 +61,54 @@ function sanitizeCategory(category: string): string {
   return category.replace(/[^a-z0-9_-]/gi, "").toLowerCase() || "other";
 }
 
+/**
+ * Resolves a `/api/media/<...segments>` route param array to an absolute
+ * path, refusing anything that would escape MEDIA_ROOT — the only
+ * defense that actually matters against path traversal (`..` segments,
+ * absolute-path segments, encoded separators once decoded by Next's own
+ * routing). Returns null for anything unsafe rather than throwing, so
+ * callers can turn that straight into a 404 without distinguishing
+ * "malicious" from "malformed."
+ */
+export function resolveMediaFilePath(segments: string[]): string | null {
+  if (segments.length === 0) return null;
+  if (segments.some((segment) => !segment || segment === "." || segment === ".." || segment.includes("/") || segment.includes("\\"))) {
+    return null;
+  }
+  const absolutePath = path.resolve(MEDIA_ROOT, ...segments);
+  if (absolutePath !== MEDIA_ROOT && !absolutePath.startsWith(MEDIA_ROOT + path.sep)) return null;
+  return absolutePath;
+}
+
+export function mimeTypeForFilename(filename: string): string | null {
+  const extension = filename.split(".").pop()?.toLowerCase();
+  if (!extension) return null;
+  return EXTENSION_MIME_TYPES[extension] ?? null;
+}
+
+export interface StoredMediaFileContent {
+  buffer: Buffer;
+  mimeType: string;
+}
+
+/** Used only by GET /api/media/[...path] — returns null for "doesn't exist" or "not a regular file," never throws, so the route can turn that into a clean 404. */
+export async function readStoredMediaFile(segments: string[]): Promise<StoredMediaFileContent | null> {
+  const absolutePath = resolveMediaFilePath(segments);
+  if (!absolutePath) return null;
+
+  const mimeType = mimeTypeForFilename(absolutePath);
+  if (!mimeType) return null;
+
+  try {
+    const fileStat = await stat(absolutePath);
+    if (!fileStat.isFile()) return null;
+    const buffer = await readFile(absolutePath);
+    return { buffer, mimeType };
+  } catch {
+    return null;
+  }
+}
+
 export async function storeMediaFile(params: {
   buffer: Buffer;
   mimeType: string;
@@ -51,8 +120,20 @@ export async function storeMediaFile(params: {
   const safeCategory = sanitizeCategory(params.category);
   const filename = `${randomUUID()}.${extension}`;
   const dir = path.join(MEDIA_ROOT, safeCategory);
+  const absolutePath = path.join(dir, filename);
+
   await mkdir(dir, { recursive: true });
-  await writeFile(path.join(dir, filename), params.buffer);
+  await writeFile(absolutePath, params.buffer);
+
+  // Never report success on a write we can't actually read back — see
+  // this file's header. A silent permission/filesystem issue on write
+  // (e.g. a volume mounted read-only) should surface as a failed upload,
+  // not a broken URL discovered later.
+  try {
+    await access(absolutePath);
+  } catch {
+    throw new Error("Upload failed: the file could not be verified after writing.");
+  }
 
   return { url: `${MEDIA_URL_PREFIX}${safeCategory}/${filename}` };
 }
@@ -61,8 +142,7 @@ export async function storeMediaFile(params: {
 export async function deleteMediaFile(url: string): Promise<void> {
   if (!url.startsWith(MEDIA_URL_PREFIX)) return;
   const relative = url.slice(MEDIA_URL_PREFIX.length);
-  const absolutePath = path.join(MEDIA_ROOT, relative);
-  // Path-traversal guard — a corrupted/crafted url should never resolve outside MEDIA_ROOT.
-  if (!absolutePath.startsWith(MEDIA_ROOT + path.sep)) return;
+  const absolutePath = resolveMediaFilePath(relative.split("/"));
+  if (!absolutePath) return;
   await unlink(absolutePath).catch(() => {});
 }