From e51c5259d44588bdf8bc43da33ca53835446e23c Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 13:43:25 -0700 Subject: [PATCH 1/5] refactor(routes): eliminate direct fs calls from route handlers Move filesystem operations behind StorageAdapter: - detected-assets: use detectAssetFiles() instead of fs.access() - upload: use saveAssetFile() instead of fs.mkdir/unlink/writeFile() - outputs: use readOutputFile() instead of fs.readFile() readOutputFile validates individual path segments (absolute, .., null bytes) before delegating to the adapter, preserving the same rejection behavior as the former safeJoin call in the route handler. Acceptance: no node:fs/promises imports remain in app/api/. --- app/api/detected-assets/route.ts | 26 ++------------- app/api/outputs/[...path]/route.ts | 26 +++------------ app/api/upload/route.ts | 29 +++-------------- lib/cast/server/storage.ts | 52 ++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 69 deletions(-) diff --git a/app/api/detected-assets/route.ts b/app/api/detected-assets/route.ts index 7d311bb..6e3d60a 100644 --- a/app/api/detected-assets/route.ts +++ b/app/api/detected-assets/route.ts @@ -1,7 +1,5 @@ import { NextResponse } from "next/server" -import fs from "node:fs/promises" -import { safeJoin } from "@/lib/cast/server/safe-join" -import { isENOENT } from "@/lib/cast/server/api-helpers" +import { detectAssetFiles } from "@/lib/cast/server/storage" import { SLUG_RE } from "@/lib/cast/schemas" export const runtime = "nodejs" @@ -13,11 +11,9 @@ export const runtime = "nodejs" * Resolver lookup order: png, jpg, jpeg, webp (first hit wins). * * Every slug is SLUG_RE-validated before any filesystem call. Lookups go - * through safeJoin('inputs', 'assets', ...). + * through the StorageAdapter via `detectAssetFiles`. */ -const LOOKUP_ORDER = ["png", "jpg", "jpeg", "webp"] as const - export async function GET(req: Request): Promise { const url = new URL(req.url) const slugsParam = url.searchParams.get("slugs") ?? "" @@ -42,23 +38,7 @@ export async function GET(req: Request): Promise { } } - const results: { slug: string; foundFile: string | null }[] = [] - for (const slug of slugs) { - let found: string | null = null - for (const ext of LOOKUP_ORDER) { - // TODO(symlink-hardening): re-validate with realpath - const candidate = safeJoin("inputs", "assets", `${slug}.${ext}`) - try { - await fs.access(candidate) - found = `${slug}.${ext}` - break - } catch (err) { - if (!isENOENT(err)) throw err - // miss — try next ext - } - } - results.push({ slug, foundFile: found }) - } + const results = await detectAssetFiles(slugs) return NextResponse.json(results, { headers: { "Cache-Control": "no-store" }, diff --git a/app/api/outputs/[...path]/route.ts b/app/api/outputs/[...path]/route.ts index 2969091..2eca9de 100644 --- a/app/api/outputs/[...path]/route.ts +++ b/app/api/outputs/[...path]/route.ts @@ -1,7 +1,6 @@ import { NextResponse } from "next/server" -import fs from "node:fs/promises" -import nodePath from "node:path" -import { safeJoin, PathTraversalError } from "@/lib/cast/server/safe-join" +import { readOutputFile } from "@/lib/cast/server/storage" +import { PathTraversalError } from "@/lib/cast/server/safe-join" import { isENOENT } from "@/lib/cast/server/api-helpers" export const runtime = "nodejs" @@ -39,28 +38,11 @@ export async function GET( return new NextResponse(null, { status: 404 }) } - let resolved: string - try { - // TODO(symlink-hardening): re-validate with realpath before readFile. - resolved = safeJoin("outputs", ...segments) - } catch (err) { - if (err instanceof PathTraversalError) { - return new NextResponse(null, { status: 404 }) - } - throw err - } - - // Defense in depth — safeJoin already rejected absolute/.. but extension - // check ran on the raw segment. Re-check on the resolved path too. - if (nodePath.extname(resolved).toLowerCase() !== ".png") { - return new NextResponse(null, { status: 404 }) - } - let bytes: Buffer try { - bytes = await fs.readFile(resolved) + bytes = await readOutputFile(...segments) } catch (err) { - if (isENOENT(err)) { + if (err instanceof PathTraversalError || isENOENT(err)) { return new NextResponse(null, { status: 404 }) } return new NextResponse(null, { status: 500 }) diff --git a/app/api/upload/route.ts b/app/api/upload/route.ts index 3420566..f096b77 100644 --- a/app/api/upload/route.ts +++ b/app/api/upload/route.ts @@ -1,7 +1,6 @@ import { NextResponse } from "next/server" -import fs from "node:fs/promises" -import { safeJoin } from "@/lib/cast/server/safe-join" -import { isENOENT, jsonError } from "@/lib/cast/server/api-helpers" +import { saveAssetFile } from "@/lib/cast/server/storage" +import { jsonError } from "@/lib/cast/server/api-helpers" import { magicBytesMatch } from "@/lib/cast/server/magic-bytes" import { UPLOAD_MAX_BYTES, UPLOAD_MAX_DISPLAY } from "@/lib/cast/upload-constraints" import { SLUG_RE } from "@/lib/cast/schemas" @@ -27,7 +26,6 @@ const MIME_TO_EXT: Record = { "image/jpeg": "jpg", "image/webp": "webp", } -const ALL_EXTS = ["png", "jpg", "jpeg", "webp"] as const export async function POST(req: Request): Promise { // Require Content-Length so we can short-circuit oversize bodies before @@ -104,31 +102,14 @@ export async function POST(req: Request): Promise { ]) } - // Ensure inputs/assets/ exists. - // TODO(symlink-hardening): re-validate assetsDir with realpath - const assetsDir = safeJoin("inputs", "assets") - await fs.mkdir(assetsDir, { recursive: true }) - - // Delete-then-write: clear any existing variant for this slug. - for (const e of ALL_EXTS) { - // TODO(symlink-hardening): re-validate with realpath - const existing = safeJoin("inputs", "assets", `${productSlug}.${e}`) - try { - await fs.unlink(existing) - } catch (err) { - if (!isENOENT(err)) throw err - } - } - - // TODO(symlink-hardening): re-validate target with realpath - const target = safeJoin("inputs", "assets", `${productSlug}.${ext}`) - await fs.writeFile(target, bytes) + // Save via StorageAdapter — deletes any existing variant, then writes the new file. + const savedAs = await saveAssetFile(productSlug, ext, bytes) return NextResponse.json( { ok: true, productSlug, - savedAs: `inputs/assets/${productSlug}.${ext}`, + savedAs, size: bytes.byteLength, }, { headers: { "Cache-Control": "no-store" } }, diff --git a/lib/cast/server/storage.ts b/lib/cast/server/storage.ts index 7792f90..632b565 100644 --- a/lib/cast/server/storage.ts +++ b/lib/cast/server/storage.ts @@ -11,6 +11,7 @@ import path from "node:path" import { getStorageAdapter } from "@/lib/cast/server/storage-adapter" +import { PathTraversalError } from "@/lib/cast/server/safe-join" import type { AspectRatio } from "@/lib/cast/schemas" const ASSET_EXTS = ["png", "jpg", "jpeg", "webp"] as const @@ -100,3 +101,54 @@ export async function writeReport( await (await getStorageAdapter()).writeFile("outputs", key, data, "application/json") return path.posix.join("outputs", key) } + +/** + * Detect which asset files exist for the given product slugs. + * Returns `{ slug, foundFile }` pairs where `foundFile` is the filename + * (e.g. `"slug.png"`) or `null` if no asset was found. + */ +export async function detectAssetFiles( + slugs: string[], +): Promise<{ slug: string; foundFile: string | null }[]> { + const results: { slug: string; foundFile: string | null }[] = [] + for (const slug of slugs) { + const found = await findLocalAsset(slug) + results.push({ slug, foundFile: found ? path.posix.basename(found) : null }) + } + return results +} + +/** + * Save an uploaded asset file, replacing any existing variant for the slug. + * Deletes all existing extensions first, then writes the new file. + * Returns the repo-relative path (e.g. `"inputs/assets/slug.png"`). + */ +export async function saveAssetFile( + productSlug: string, + ext: string, + bytes: Uint8Array, +): Promise { + const adapter = await getStorageAdapter() + for (const e of ASSET_EXTS) { + await adapter.deleteFile("inputs", `assets/${productSlug}.${e}`) + } + const key = `assets/${productSlug}.${ext}` + await adapter.writeFile("inputs", key, Buffer.from(bytes)) + return path.posix.join("inputs", key) +} + +/** + * Read a file from the outputs container. + * Validates individual path segments before delegating to the adapter — + * rejects absolute paths, parent traversal, and null bytes. + * Throws if the file does not exist (ENOENT) or the path is invalid. + */ +export async function readOutputFile(...segments: string[]): Promise { + for (const seg of segments) { + if (!seg || seg.includes("\0") || seg === ".." || path.isAbsolute(seg)) { + throw new PathTraversalError(`invalid output path segment: "${seg}"`) + } + } + const key = segments.join("/") + return (await getStorageAdapter()).readFile("outputs", key) +} From c6deeeadbdaf5135bced8940d5f53bea7911128f Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 13:43:32 -0700 Subject: [PATCH 2/5] feat(server): add barrel export for server module boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create lib/cast/server/index.ts that re-exports the entire public API surface. Documents the import boundary: app/api/* → lib/cast/server/* only No direct fs, Azure, or Qdrant calls in route handlers This barrel becomes the Fastify service API surface when the backend migrates off Next.js API routes. --- lib/cast/server/index.ts | 65 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 lib/cast/server/index.ts diff --git a/lib/cast/server/index.ts b/lib/cast/server/index.ts new file mode 100644 index 0000000..09a8703 --- /dev/null +++ b/lib/cast/server/index.ts @@ -0,0 +1,65 @@ +/** + * Server module barrel export — the public API surface. + * + * Everything exported here becomes the Fastify service API when the backend + * migrates from Next.js API routes. Route handlers in `app/api/` are thin + * pass-throughs that map HTTP → these functions → HTTP responses. + * + * Import rules: + * ✅ app/api/* → lib/cast/server/* (routes call server functions) + * ✅ lib/cast/server/* → lib/cast/* (server uses shared schemas/types) + * ❌ app/api/* → node:fs/promises (no direct I/O in route handlers) + * ❌ app/api/* → @azure/* (no direct Azure calls in routes) + * ❌ components/ → lib/cast/server/* (client code never imports server) + */ + +// -- Config (env var accessors) --------------------------------------------- +// `getGenAIMode` is deliberately excluded — use the re-export from +// pipeline/genai which wraps it with the canonical GenAIMode type. +export { + getOpenAIApiKey, + type StorageBackend, + getStorageBackend, + getAzureConnectionString, + isAzureEnabled, + getQdrantUrl, + getQdrantApiKey, + isQdrantEnabled, + getFatigueThreshold, + type AdsProvider, + getAdsProvider, + getApiBaseUrl, +} from "./config" + +// -- Helpers ---------------------------------------------------------------- +export * from "./api-helpers" +export * from "./magic-bytes" +export * from "./safe-join" +export * from "./retry" + +// -- Loaders ---------------------------------------------------------------- +export * from "./brand-loader" +export * from "./brief-loader" + +// -- Storage ---------------------------------------------------------------- +export * from "./storage" +export * from "./storage-adapter" +export * from "./azure-blob-adapter" + +// -- Metadata --------------------------------------------------------------- +export * from "./metadata" + +// -- NDJSON streaming ------------------------------------------------------- +export * from "./ndjson-emit" + +// -- Pipeline stages -------------------------------------------------------- +export * from "./pipeline/compose" +export * from "./pipeline/compliance" +export * from "./pipeline/genai" +export * from "./pipeline/manifest-builder" +export * from "./pipeline/resize" +export * from "./pipeline/resolve" +export * from "./pipeline/write" + +// -- MCP tools -------------------------------------------------------------- +export * from "./mcp-tools" From 91e1acd83625ba06797016807e64f1a9b46e4e54 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 13:56:14 -0700 Subject: [PATCH 3/5] fix(storage): validate ext against ASSET_EXTS and normalize path segments saveAssetFile: narrow ext param to AssetExt union type with runtime check, preventing arbitrary extensions from reaching the storage layer. readOutputFile: split segments on / and \ before validation so embedded separators cannot smuggle traversal components past per-segment checks. Pre-check rejects absolute and empty raw segments before normalization. --- lib/cast/server/storage.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/cast/server/storage.ts b/lib/cast/server/storage.ts index 632b565..42c9375 100644 --- a/lib/cast/server/storage.ts +++ b/lib/cast/server/storage.ts @@ -15,6 +15,7 @@ import { PathTraversalError } from "@/lib/cast/server/safe-join" import type { AspectRatio } from "@/lib/cast/schemas" const ASSET_EXTS = ["png", "jpg", "jpeg", "webp"] as const +type AssetExt = (typeof ASSET_EXTS)[number] /** * Scan `inputs/assets/` for a product photo named after `productSlug`. @@ -125,9 +126,12 @@ export async function detectAssetFiles( */ export async function saveAssetFile( productSlug: string, - ext: string, + ext: AssetExt, bytes: Uint8Array, ): Promise { + if (!(ASSET_EXTS as readonly string[]).includes(ext)) { + throw new Error(`invalid asset extension "${ext}" — allowed: ${ASSET_EXTS.join(", ")}`) + } const adapter = await getStorageAdapter() for (const e of ASSET_EXTS) { await adapter.deleteFile("inputs", `assets/${productSlug}.${e}`) @@ -144,11 +148,20 @@ export async function saveAssetFile( * Throws if the file does not exist (ENOENT) or the path is invalid. */ export async function readOutputFile(...segments: string[]): Promise { + // Reject obviously invalid raw segments before normalization. for (const seg of segments) { - if (!seg || seg.includes("\0") || seg === ".." || path.isAbsolute(seg)) { + if (!seg || path.isAbsolute(seg)) { throw new PathTraversalError(`invalid output path segment: "${seg}"`) } } - const key = segments.join("/") + // Normalize: split on both / and \ so embedded separators can't smuggle + // traversal components past the per-segment check. + const parts = segments.flatMap((s) => s.split(/[/\\]/)).filter(Boolean) + for (const part of parts) { + if (part === "." || part === ".." || part.includes("\0")) { + throw new PathTraversalError(`invalid output path segment: "${part}"`) + } + } + const key = parts.join("/") return (await getStorageAdapter()).readFile("outputs", key) } From 554dbd9def0a0ae7a4dec383e5ce6d4e6ab223bf Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 13:56:21 -0700 Subject: [PATCH 4/5] fix(server): narrow barrel exports to hide internals and lazy imports Replace `export *` from storage-adapter with explicit named exports, hiding _resetStorageAdapter and LocalFsAdapter test seams from the public API surface. Replace eager re-export of azure-blob-adapter with type-only export so @azure/storage-blob stays lazy-loaded via dynamic import(). --- lib/cast/server/index.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/cast/server/index.ts b/lib/cast/server/index.ts index 09a8703..1f3dd61 100644 --- a/lib/cast/server/index.ts +++ b/lib/cast/server/index.ts @@ -43,8 +43,15 @@ export * from "./brief-loader" // -- Storage ---------------------------------------------------------------- export * from "./storage" -export * from "./storage-adapter" -export * from "./azure-blob-adapter" +export { + type Container, + type StorageAdapter, + getStorageAdapter, +} from "./storage-adapter" +// AzureBlobAdapter is NOT re-exported — it is lazy-loaded by +// getStorageAdapter() via dynamic import(). Eagerly re-exporting it +// would pull in @azure/storage-blob for every consumer of this barrel. +export type { AzureBlobAdapter } from "./azure-blob-adapter" // -- Metadata --------------------------------------------------------------- export * from "./metadata" From 99a38cb880cb016265c6f8e653ff7f8a79648740 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 14:28:22 -0700 Subject: [PATCH 5/5] docs(outputs): update hardening comment to reflect readOutputFile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review on PR #55 (round 2): - outputs/[...path]/route.ts — stale reference to safeJoin replaced with readOutputFile's segment validation description. --- app/api/outputs/[...path]/route.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/api/outputs/[...path]/route.ts b/app/api/outputs/[...path]/route.ts index 2eca9de..5f8d42f 100644 --- a/app/api/outputs/[...path]/route.ts +++ b/app/api/outputs/[...path]/route.ts @@ -12,7 +12,8 @@ export const runtime = "nodejs" * prior run; this route is the only way the browser can pull a generated PNG. * * Hardening: - * - safeJoin against ROOTS.outputs (rejects `..`, absolute, null bytes) + * - readOutputFile validates segments (rejects `..`, absolute, null bytes, + * backslash-smuggled components) before delegating to the StorageAdapter * - .png whitelist — anything else 404s, never reveals MIME of other files * - X-Content-Type-Options: nosniff so a malicious upstream can't trick * the browser into rendering the bytes as HTML/JS