From 507a076d688429c491a21cb9b6795cbaf32706bb Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 08:43:38 -0700 Subject: [PATCH 1/5] refactor(brands): migrate brand-loader to StorageAdapter Replace direct fs calls with StorageAdapter interface so brand loading works against both local filesystem and Azure Blob Storage. - brand-loader.ts: all I/O through getStorageAdapter() - BrandProfile paths now store container-relative keys - Logo proxy and generate routes use adapter for reads - Tests rewritten to mock StorageAdapter instead of node:fs --- app/api/brands/[slug]/logos/[id]/route.ts | 9 +- app/api/generate/route.ts | 17 +- lib/cast/schemas.ts | 6 +- lib/cast/server/brand-loader.ts | 129 ++++++------ lib/cast/server/pipeline/resolve.ts | 6 +- tests/brand-loader.test.ts | 242 +++++++++++----------- 6 files changed, 203 insertions(+), 206 deletions(-) diff --git a/app/api/brands/[slug]/logos/[id]/route.ts b/app/api/brands/[slug]/logos/[id]/route.ts index 5622407..fa2f511 100644 --- a/app/api/brands/[slug]/logos/[id]/route.ts +++ b/app/api/brands/[slug]/logos/[id]/route.ts @@ -1,11 +1,11 @@ import { NextResponse } from "next/server" -import fs from "node:fs/promises" import { loadBrandProfile } from "@/lib/cast/server/brand-loader" import { BrandNotFoundError, BrandIncompleteError, BrandInvalidError, } from "@/lib/cast/errors" +import { getStorageAdapter } from "@/lib/cast/server/storage-adapter" export const runtime = "nodejs" @@ -50,11 +50,12 @@ export async function GET( ) } - // TODO(symlink-hardening): variant.path was safeJoin-validated by the loader; - // re-validate with realpath when production hardening lands. + // variant.path is a container-relative key validated by the loader, + // e.g. "brands/brisa/logos/droplet-on-dark.png". + const storage = await getStorageAdapter() let buf: Buffer try { - buf = await fs.readFile(variant.path) + buf = await storage.readFile("inputs", variant.path) } catch (err) { const code = (err as NodeJS.ErrnoException).code if (code === "ENOENT" || code === "EACCES" || code === "EPERM") { diff --git a/app/api/generate/route.ts b/app/api/generate/route.ts index 25c7496..0050b34 100644 --- a/app/api/generate/route.ts +++ b/app/api/generate/route.ts @@ -60,7 +60,7 @@ import { writeBriefSnapshot, writeReport, } from "@/lib/cast/server/storage" -import fs from "node:fs/promises" +import { getStorageAdapter } from "@/lib/cast/server/storage-adapter" import { emitAssetResolved, emitComplete, @@ -276,12 +276,12 @@ export async function runPipeline(args: RunPipelineArgs): Promise { let baseImageGenerationError: { stage: ErrorStage; message: string } | undefined if (resolved.source === "local" || resolved.source === "products") { try { - // "local" → repo-relative path via readAsset; "products" → absolute - // path resolved by brand-loader, read directly via fs. + // "local" → repo-relative path via readAsset; "products" → container-relative + // key via readBrandAsset. localBaseImage = resolved.source === "local" ? await readAsset(resolved.file) - : await readFile(resolved.file) + : await readBrandAsset(resolved.file) } catch (err) { const message = errMessage(err) for (const ratio of brief.ratios) { @@ -524,11 +524,10 @@ function errMessage(err: unknown): string { } /** - * Read a file from an absolute path (used for brand-can variants whose paths - * are already absolute, resolved by brand-loader). Kept separate from - * `readAsset` which takes repo-relative inputs/-rooted paths. + * Read a brand asset file via StorageAdapter using a container-relative key + * (e.g. `brands/brisa/products/can-citrus.png`). */ -async function readFile(absPath: string): Promise { - return fs.readFile(absPath) +async function readBrandAsset(key: string): Promise { + return (await getStorageAdapter()).readFile("inputs", key) } diff --git a/lib/cast/schemas.ts b/lib/cast/schemas.ts index 06d7f97..9ffad7f 100644 --- a/lib/cast/schemas.ts +++ b/lib/cast/schemas.ts @@ -239,11 +239,11 @@ export type BrandProfile = { theme?: "light" | "dark" }[] defaultLogoId: string - /** Absolute path to the brand's display font. `font.ttf` or `font.otf`. */ + /** Container-relative key to the brand's display font, e.g. `brands/acme/font.otf`. */ fontPath: string /** * Can/product PNG variants from products.json. - * `file` is an absolute path (resolved by brand-loader). + * `file` is a container-relative key (e.g. `brands/acme/products/can.png`). * Empty array when products.json is absent (e.g. Volt). */ canVariants: Array<{ @@ -255,7 +255,7 @@ export type BrandProfile = { }> /** * Background image variants from backgrounds.json. - * `file` is an absolute path (resolved by brand-loader). + * `file` is a container-relative key (e.g. `brands/acme/backgrounds/bg.png`). * Empty array when backgrounds.json is absent (e.g. Volt). */ backgroundVariants: Array<{ diff --git a/lib/cast/server/brand-loader.ts b/lib/cast/server/brand-loader.ts index eb3f1e7..c6332a7 100644 --- a/lib/cast/server/brand-loader.ts +++ b/lib/cast/server/brand-loader.ts @@ -10,10 +10,9 @@ * each with a structured `{ errors: [...] }` body. * - Banned-words list is the **union** of `getDefaultBannedWords()` + brand file * (deduped, lowercased). Defaults always apply; missing brand file is allowed. - * - Every disk read goes through `safeJoin("inputs", ...)`. + * - All file I/O goes through StorageAdapter (local FS or Azure Blob). */ -import fs from "node:fs/promises" import type { ZodType } from "zod" import { brandJsonSchema, @@ -30,8 +29,8 @@ import { BrandIncompleteError, BrandInvalidError, } from "@/lib/cast/errors" -import { safeJoin } from "@/lib/cast/server/safe-join" import { getDefaultBannedWords } from "@/lib/cast/banned-words" +import { getStorageAdapter, type StorageAdapter } from "@/lib/cast/server/storage-adapter" const CACHE_TTL_MS = 90_000 @@ -100,18 +99,22 @@ export async function loadBrandProfile(slug: string): Promise { return cached.profile } - // TODO(symlink-hardening): re-validate with realpath after lexical safeJoin. - const brandDir = safeJoin("inputs", "brands", slug) - await assertExists(brandDir, slug, slug) + const storage = await getStorageAdapter() - const brandJson = await readJson(slug, "brand.json") - const voiceJson = await readJson(slug, "voice.json") - const logosJson = await readJson(slug, "logos/logos.json") + // Brand dir must exist — check via a known child (brand.json). + const brandDirKey = `brands/${slug}` + if (!(await storage.fileExists("inputs", `${brandDirKey}/brand.json`))) { + throw new BrandNotFoundError(slug) + } + + const brandJson = await readJson(storage, slug, "brand.json") + const voiceJson = await readJson(storage, slug, "voice.json") + const logosJson = await readJson(storage, slug, "logos/logos.json") // banned-words.json is optional let bannedWordsRaw: unknown[] = [] try { - bannedWordsRaw = (await readJson(slug, "banned-words.json")) as unknown[] + bannedWordsRaw = (await readJson(storage, slug, "banned-words.json")) as unknown[] } catch (err) { if (!(err instanceof BrandIncompleteError)) throw err } @@ -119,7 +122,7 @@ export async function loadBrandProfile(slug: string): Promise { // products.json is optional — present for Brisa, absent for Volt let productsRaw: unknown = null try { - productsRaw = await readJson(slug, "products.json") + productsRaw = await readJson(storage, slug, "products.json") } catch (err) { if (!(err instanceof BrandIncompleteError)) throw err } @@ -127,14 +130,13 @@ export async function loadBrandProfile(slug: string): Promise { // backgrounds.json is optional — present for Brisa, absent for Volt let backgroundsRaw: unknown = null try { - backgroundsRaw = await readJson(slug, "backgrounds.json") + backgroundsRaw = await readJson(storage, slug, "backgrounds.json") } catch (err) { if (!(err instanceof BrandIncompleteError)) throw err } // font.ttf / font.otf is existence-checked only — either filename is accepted. - // TODO(symlink-hardening): re-validate fontPath with realpath - const fontPath = await resolveFontPath(slug) + const fontKey = await resolveFontKey(storage, slug) const brand = parse(slug, "brand.json", brandJsonSchema, brandJson) const voice = parse(slug, "voice.json", voiceJsonSchema, voiceJson) @@ -146,23 +148,22 @@ export async function loadBrandProfile(slug: string): Promise { ) const logos = parse(slug, "logos/logos.json", logosManifestSchema, logosJson) - // Verify each declared logo variant file actually exists on disk. + // Verify each declared logo variant file actually exists. const logoVariants: BrandProfile["logoVariants"] = [] for (const variant of logos.variants) { - // TODO(symlink-hardening): re-validate variant.path with realpath - const variantPath = safeJoin("inputs", "brands", slug, "logos", variant.file) - await assertExists(variantPath, slug, `logos/${variant.file}`) + const variantKey = `brands/${slug}/logos/${variant.file}` + await assertExists(storage, variantKey, slug, `logos/${variant.file}`) logoVariants.push({ id: variant.id, displayName: variant.displayName, - path: variantPath, + path: variantKey, theme: variant.theme, }) } const bannedWords = unionLowercase(getDefaultBannedWords(), bannedFromFile) - // Resolve can variant absolute paths + // Resolve can variant keys (container-relative) const canVariants: BrandProfile["canVariants"] = [] if (productsRaw !== null) { const productsManifest = parse( @@ -173,20 +174,19 @@ export async function loadBrandProfile(slug: string): Promise { ) for (const item of productsManifest.items) { const segments = item.file.split("/").filter(Boolean) - // TODO(symlink-hardening): re-validate with realpath - const absPath = safeJoin("inputs", "brands", slug, ...segments) - await assertExists(absPath, slug, item.file) + const itemKey = `brands/${slug}/${segments.join("/")}` + await assertExists(storage, itemKey, slug, item.file) canVariants.push({ id: item.id, sku: item.sku, - file: absPath, + file: itemKey, pose: item.pose, detail: item.detail, }) } } - // Resolve background variant absolute paths + // Resolve background variant keys (container-relative) const backgroundVariants: BrandProfile["backgroundVariants"] = [] if (backgroundsRaw !== null) { const backgroundsManifest = parse( @@ -197,12 +197,11 @@ export async function loadBrandProfile(slug: string): Promise { ) for (const item of backgroundsManifest.items) { const segments = item.file.split("/").filter(Boolean) - // TODO(symlink-hardening): re-validate with realpath - const absPath = safeJoin("inputs", "brands", slug, ...segments) - await assertExists(absPath, slug, item.file) + const itemKey = `brands/${slug}/${segments.join("/")}` + await assertExists(storage, itemKey, slug, item.file) backgroundVariants.push({ id: item.id, - file: absPath, + file: itemKey, ratio: item.ratio, sku: item.sku, luminance: item.luminance, @@ -217,7 +216,7 @@ export async function loadBrandProfile(slug: string): Promise { bannedWords, logoVariants, defaultLogoId: logos.default, - fontPath, + fontPath: fontKey, canVariants, backgroundVariants, } @@ -228,23 +227,24 @@ export async function loadBrandProfile(slug: string): Promise { /** List every brand directory under inputs/brands/. Returns sorted slugs. */ export async function listBrandSlugs(): Promise { - // TODO(symlink-hardening): re-validate with realpath - const dir = safeJoin("inputs", "brands") - let entries: import("node:fs").Dirent[] - try { - entries = await fs.readdir(dir, { withFileTypes: true }) - } catch (err) { - // Missing inputs/brands/ is allowed; permission/IO errors must surface. - if (isENOENT(err)) { - maybeWarnNoBrands("missing") - return [] + const storage = await getStorageAdapter() + // listFiles returns [] when the directory is missing (adapter handles ENOENT); + // permission/IO errors propagate. + const keys = await storage.listFiles("inputs", "brands") + if (keys.length === 0) { + maybeWarnNoBrands("missing") + return [] + } + // Extract unique slugs from keys like "brands/brisa/brand.json". + const slugSet = new Set() + for (const key of keys) { + const parts = key.split("/") + // parts[0] === "brands", parts[1] === slug + if (parts.length >= 2 && SLUG_RE.test(parts[1])) { + slugSet.add(parts[1]) } - throw err } - const slugs = entries - .filter((e) => e.isDirectory() && SLUG_RE.test(e.name)) - .map((e) => e.name) - .sort() + const slugs = [...slugSet].sort() if (slugs.length === 0) maybeWarnNoBrands("empty") return slugs } @@ -263,15 +263,12 @@ function maybeWarnNoBrands(reason: "missing" | "empty"): void { // --------------------------------------------------------------------------- -async function readJson(slug: string, rel: string): Promise { - // rel is a controlled internal constant ("brand.json", "voice.json", ...). - // We still safeJoin to be defensive. - const segments = rel.split("/").filter(Boolean) - // TODO(symlink-hardening): re-validate with realpath - const filePath = safeJoin("inputs", "brands", slug, ...segments) +async function readJson(storage: StorageAdapter, slug: string, rel: string): Promise { + const key = `brands/${slug}/${rel}` let raw: string try { - raw = await fs.readFile(filePath, "utf8") + const buf = await storage.readFile("inputs", key) + raw = buf.toString("utf8") } catch (err) { if (isENOENT(err)) { throw new BrandIncompleteError(slug, rel) @@ -288,19 +285,15 @@ async function readJson(slug: string, rel: string): Promise { } async function assertExists( - absPath: string, + storage: StorageAdapter, + key: string, slug: string, rel: string, ): Promise { - try { - await fs.access(absPath) - } catch (err) { - if (isENOENT(err)) { - // brand-dir miss is "not found"; child miss is "incomplete" - if (rel === slug) throw new BrandNotFoundError(slug) - throw new BrandIncompleteError(slug, rel) - } - throw err + if (!(await storage.fileExists("inputs", key))) { + // brand-dir miss is "not found"; child miss is "incomplete" + if (rel === slug) throw new BrandNotFoundError(slug) + throw new BrandIncompleteError(slug, rel) } } @@ -309,15 +302,11 @@ async function assertExists( * `font.ttf` or `font.otf` is accepted. If neither exists, throw * `BrandIncompleteError` against `font.ttf` (canonical name in the contract). */ -async function resolveFontPath(slug: string): Promise { +async function resolveFontKey(storage: StorageAdapter, slug: string): Promise { for (const file of ["font.ttf", "font.otf"] as const) { - // TODO(symlink-hardening): re-validate with realpath - const candidate = safeJoin("inputs", "brands", slug, file) - try { - await fs.access(candidate) - return candidate - } catch (err) { - if (!isENOENT(err)) throw err + const key = `brands/${slug}/${file}` + if (await storage.fileExists("inputs", key)) { + return key } } throw new BrandIncompleteError(slug, "font.ttf") diff --git a/lib/cast/server/pipeline/resolve.ts b/lib/cast/server/pipeline/resolve.ts index 027ad81..e3072bb 100644 --- a/lib/cast/server/pipeline/resolve.ts +++ b/lib/cast/server/pipeline/resolve.ts @@ -8,9 +8,9 @@ * matched by SKU (source: "products") * 3. No local asset found → GenAI fill (source: "genai") * - * Repo-relative path is returned for "local"; absolute path for "products" - * (brand-loader already resolved them). The orchestrator can call `readAsset` - * for "local" or `fs.readFile` for "products" to materialize a Buffer. + * Repo-relative path is returned for "local"; container-relative key for + * "products" (brand-loader already resolved them). The orchestrator reads + * both types via StorageAdapter. */ import type { BrandProfile } from "@/lib/cast/schemas" diff --git a/tests/brand-loader.test.ts b/tests/brand-loader.test.ts index 4dff5af..0060e56 100644 --- a/tests/brand-loader.test.ts +++ b/tests/brand-loader.test.ts @@ -1,17 +1,29 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest" +import type { StorageAdapter } from "@/lib/cast/server/storage-adapter" -// Mock node:fs/promises before importing the loader. -vi.mock("node:fs/promises", () => { - return { - default: { - readFile: vi.fn(), - access: vi.fn(), - readdir: vi.fn(), - }, - } -}) +// Mock getStorageAdapter to return a controllable adapter. +const mockAdapter: { + readFile: ReturnType + fileExists: ReturnType + listFiles: ReturnType + writeFile: ReturnType + deleteFile: ReturnType + deletePrefix: ReturnType + getPublicUrl: ReturnType +} = { + readFile: vi.fn(), + fileExists: vi.fn(), + listFiles: vi.fn(), + writeFile: vi.fn(), + deleteFile: vi.fn(), + deletePrefix: vi.fn(), + getPublicUrl: vi.fn(), +} + +vi.mock("@/lib/cast/server/storage-adapter", () => ({ + getStorageAdapter: vi.fn(async () => mockAdapter as unknown as StorageAdapter), +})) -import fs from "node:fs/promises" import { loadBrandProfile, listBrandSlugs, @@ -25,12 +37,6 @@ import { BrandInvalidError, } from "@/lib/cast/errors" -const mockedFs = fs as unknown as { - readFile: ReturnType - access: ReturnType - readdir: ReturnType -} - const VALID_BRAND = { displayName: "Acme", colors: { primary: "#000000", accent: "#ff00ff" }, @@ -56,13 +62,20 @@ function enoent(): NodeJS.ErrnoException { } function mountValidBrandFixture(slug = "acme"): void { - mockedFs.access.mockImplementation(async () => undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith(`brands/${slug}/brand.json`)) return JSON.stringify(VALID_BRAND) - if (path.endsWith(`brands/${slug}/voice.json`)) return JSON.stringify(VALID_VOICE) - if (path.endsWith(`brands/${slug}/logos/logos.json`)) return JSON.stringify(VALID_LOGOS) - if (path.endsWith(`brands/${slug}/banned-words.json`)) throw enoent() + mockAdapter.fileExists.mockImplementation(async (_c: string, key: string) => { + // brand.json existence check (brand dir proxy) + font + logo variants + if (key === `brands/${slug}/brand.json`) return true + if (key === `brands/${slug}/font.ttf`) return true + if (key === `brands/${slug}/logos/primary.png`) return true + return false + }) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === `brands/${slug}/brand.json`) return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === `brands/${slug}/voice.json`) return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === `brands/${slug}/logos/logos.json`) return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === `brands/${slug}/banned-words.json`) throw enoent() + if (key === `brands/${slug}/products.json`) throw enoent() + if (key === `brands/${slug}/backgrounds.json`) throw enoent() throw enoent() }) } @@ -70,9 +83,9 @@ function mountValidBrandFixture(slug = "acme"): void { beforeEach(() => { _clearBrandCache() _resetBrandWarnings() - mockedFs.readFile.mockReset() - mockedFs.access.mockReset() - mockedFs.readdir.mockReset() + mockAdapter.readFile.mockReset() + mockAdapter.fileExists.mockReset() + mockAdapter.listFiles.mockReset() }) afterEach(() => { @@ -87,17 +100,16 @@ describe("loadBrandProfile", () => { }) it("throws BrandNotFoundError when brand dir is missing", async () => { - mockedFs.access.mockRejectedValue(enoent()) + mockAdapter.fileExists.mockResolvedValue(false) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandNotFoundError) }) it("throws BrandIncompleteError when a required file is missing", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("voice.json")) throw enoent() - if (path.endsWith("brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("logos/logos.json")) return JSON.stringify(VALID_LOGOS) + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/voice.json") throw enoent() + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) throw enoent() }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf( @@ -106,8 +118,8 @@ describe("loadBrandProfile", () => { }) it("throws BrandInvalidError on JSON parse failure", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async () => "{ this is not json") + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockResolvedValue(Buffer.from("{ this is not json")) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) }) @@ -124,61 +136,62 @@ describe("loadBrandProfile", () => { it("returns the cached profile on a second call within TTL", async () => { mountValidBrandFixture() const a = await loadBrandProfile("acme") - const readsAfterFirst = mockedFs.readFile.mock.calls.length + const readsAfterFirst = mockAdapter.readFile.mock.calls.length const b = await loadBrandProfile("acme") expect(b).toBe(a) // referentially identical → from cache - expect(mockedFs.readFile.mock.calls.length).toBe(readsAfterFirst) + expect(mockAdapter.readFile.mock.calls.length).toBe(readsAfterFirst) }) it("re-reads after TTL expiry (90 s)", async () => { vi.useFakeTimers() mountValidBrandFixture() await loadBrandProfile("acme") - const readsAfterFirst = mockedFs.readFile.mock.calls.length + const readsAfterFirst = mockAdapter.readFile.mock.calls.length vi.advanceTimersByTime(90_001) await loadBrandProfile("acme") - expect(mockedFs.readFile.mock.calls.length).toBeGreaterThan(readsAfterFirst) + expect(mockAdapter.readFile.mock.calls.length).toBeGreaterThan(readsAfterFirst) }) it("re-reads after _clearBrandCache()", async () => { mountValidBrandFixture() await loadBrandProfile("acme") - const readsAfterFirst = mockedFs.readFile.mock.calls.length + const readsAfterFirst = mockAdapter.readFile.mock.calls.length _clearBrandCache() await loadBrandProfile("acme") - expect(mockedFs.readFile.mock.calls.length).toBeGreaterThan(readsAfterFirst) + expect(mockAdapter.readFile.mock.calls.length).toBeGreaterThan(readsAfterFirst) }) it("accepts font.otf when font.ttf is absent", async () => { - mockedFs.access.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("/font.ttf")) throw enoent() - return undefined + mockAdapter.fileExists.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/font.ttf") return false + return true }) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("brands/acme/brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("brands/acme/voice.json")) return JSON.stringify(VALID_VOICE) - if (path.endsWith("brands/acme/logos/logos.json")) return JSON.stringify(VALID_LOGOS) - if (path.endsWith("brands/acme/banned-words.json")) throw enoent() + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/products.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() throw enoent() }) const profile = await loadBrandProfile("acme") - expect(profile.fontPath.replace(/\\/g, "/")).toMatch(/\/font\.otf$/) + expect(profile.fontPath).toBe("brands/acme/font.otf") }) it("throws BrandIncompleteError when neither font.ttf nor font.otf exists", async () => { - mockedFs.access.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (/\/font\.(ttf|otf)$/.test(path)) throw enoent() - return undefined + mockAdapter.fileExists.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/font.ttf") return false + if (key === "brands/acme/font.otf") return false + return true }) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("brands/acme/brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("brands/acme/voice.json")) return JSON.stringify(VALID_VOICE) - if (path.endsWith("brands/acme/logos/logos.json")) return JSON.stringify(VALID_LOGOS) - if (path.endsWith("brands/acme/banned-words.json")) throw enoent() + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/products.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() throw enoent() }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf( @@ -189,19 +202,18 @@ describe("loadBrandProfile", () => { describe("listBrandSlugs", () => { it("returns sorted slugs that match SLUG_RE", async () => { - mockedFs.readdir.mockResolvedValue([ - { name: "zeta", isDirectory: () => true }, - { name: "alpha", isDirectory: () => true }, - { name: "INVALID", isDirectory: () => true }, // fails SLUG_RE - { name: "not-a-dir", isDirectory: () => false }, - ] as unknown as never) + mockAdapter.listFiles.mockResolvedValue([ + "brands/zeta/brand.json", + "brands/alpha/brand.json", + "brands/INVALID/brand.json", // fails SLUG_RE + ]) const slugs = await listBrandSlugs() expect(slugs).toEqual(["alpha", "zeta"]) }) - it("returns [] when inputs/brands/ is missing (ENOENT)", async () => { + it("returns [] when inputs/brands/ is missing (empty listFiles)", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) - mockedFs.readdir.mockRejectedValue(enoent()) + mockAdapter.listFiles.mockResolvedValue([]) expect(await listBrandSlugs()).toEqual([]) warn.mockRestore() }) @@ -209,7 +221,7 @@ describe("listBrandSlugs", () => { it("re-throws non-ENOENT errors (e.g. EACCES)", async () => { const eacces = new Error("EACCES") as NodeJS.ErrnoException eacces.code = "EACCES" - mockedFs.readdir.mockRejectedValue(eacces) + mockAdapter.listFiles.mockRejectedValue(eacces) await expect(listBrandSlugs()).rejects.toThrow("EACCES") }) }) @@ -217,7 +229,7 @@ describe("listBrandSlugs", () => { describe("listBrandSlugs warn-once", () => { it("warns exactly once when inputs/brands/ is missing", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) - mockedFs.readdir.mockRejectedValue(enoent()) + mockAdapter.listFiles.mockResolvedValue([]) await listBrandSlugs() await listBrandSlugs() expect(warn).toHaveBeenCalledTimes(1) @@ -227,9 +239,9 @@ describe("listBrandSlugs warn-once", () => { it("warns exactly once when no slugs match SLUG_RE", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) - mockedFs.readdir.mockResolvedValue([ - { name: "INVALID", isDirectory: () => true }, - ] as unknown as never) + mockAdapter.listFiles.mockResolvedValue([ + "brands/INVALID/brand.json", + ]) await listBrandSlugs() await listBrandSlugs() expect(warn).toHaveBeenCalledTimes(1) @@ -239,9 +251,9 @@ describe("listBrandSlugs warn-once", () => { it("does NOT warn when at least one valid slug is present", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) - mockedFs.readdir.mockResolvedValue([ - { name: "acme", isDirectory: () => true }, - ] as unknown as never) + mockAdapter.listFiles.mockResolvedValue([ + "brands/acme/brand.json", + ]) await listBrandSlugs() expect(warn).not.toHaveBeenCalled() warn.mockRestore() @@ -257,19 +269,18 @@ describe("tryLoadBrand", () => { }) it("returns { ok: false, error: BrandNotFoundError } when brand dir is missing", async () => { - mockedFs.access.mockRejectedValue(enoent()) + mockAdapter.fileExists.mockResolvedValue(false) const result = await tryLoadBrand("acme") expect(result.ok).toBe(false) if (!result.ok) expect(result.error).toBeInstanceOf(BrandNotFoundError) }) it("returns { ok: false, error: BrandIncompleteError } on missing required file", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("voice.json")) throw enoent() - if (path.endsWith("brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("logos/logos.json")) return JSON.stringify(VALID_LOGOS) + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/voice.json") throw enoent() + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) throw enoent() }) const result = await tryLoadBrand("acme") @@ -278,8 +289,8 @@ describe("tryLoadBrand", () => { }) it("returns { ok: false, error: BrandInvalidError } on parse failure", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async () => "{ not json") + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockResolvedValue(Buffer.from("{ not json")) const result = await tryLoadBrand("acme") expect(result.ok).toBe(false) if (!result.ok) expect(result.error).toBeInstanceOf(BrandInvalidError) @@ -288,7 +299,7 @@ describe("tryLoadBrand", () => { it("re-throws non-brand errors (e.g. EACCES)", async () => { const eacces = new Error("EACCES") as NodeJS.ErrnoException eacces.code = "EACCES" - mockedFs.access.mockRejectedValue(eacces) + mockAdapter.fileExists.mockRejectedValue(eacces) await expect(tryLoadBrand("acme")).rejects.toThrow("EACCES") }) }) @@ -302,22 +313,21 @@ describe("canVariants — products.json", () => { } it("populates canVariants when products.json is present", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("brands/acme/brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("brands/acme/voice.json")) return JSON.stringify(VALID_VOICE) - if (path.endsWith("brands/acme/logos/logos.json")) return JSON.stringify(VALID_LOGOS) - if (path.endsWith("brands/acme/products.json")) return JSON.stringify(VALID_PRODUCTS) - if (path.endsWith("brands/acme/banned-words.json")) throw enoent() - if (path.endsWith("brands/acme/backgrounds.json")) throw enoent() + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify(VALID_PRODUCTS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() throw enoent() }) const profile = await loadBrandProfile("acme") expect(profile.canVariants).toHaveLength(2) expect(profile.canVariants[0].sku).toBe("TST-CIT-12") expect(profile.canVariants[0].pose).toBe("upright-center") - expect(profile.canVariants[0].file.replace(/\\/g, "/")).toContain("can-citrus.png") + expect(profile.canVariants[0].file).toBe("brands/acme/products/can-citrus.png") }) it("returns empty canVariants when products.json is absent", async () => { @@ -327,15 +337,14 @@ describe("canVariants — products.json", () => { }) it("throws BrandInvalidError when products.json has invalid schema", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("brands/acme/brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("brands/acme/voice.json")) return JSON.stringify(VALID_VOICE) - if (path.endsWith("brands/acme/logos/logos.json")) return JSON.stringify(VALID_LOGOS) - if (path.endsWith("brands/acme/products.json")) return JSON.stringify({ items: [] }) // min(1) violation - if (path.endsWith("brands/acme/banned-words.json")) throw enoent() - if (path.endsWith("brands/acme/backgrounds.json")) throw enoent() + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify({ items: [] })) // min(1) violation + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() throw enoent() }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) @@ -351,15 +360,14 @@ describe("backgroundVariants — backgrounds.json", () => { } it("populates backgroundVariants when backgrounds.json is present", async () => { - mockedFs.access.mockResolvedValue(undefined) - mockedFs.readFile.mockImplementation(async (p: string) => { - const path = String(p).replace(/\\/g, "/") - if (path.endsWith("brands/acme/brand.json")) return JSON.stringify(VALID_BRAND) - if (path.endsWith("brands/acme/voice.json")) return JSON.stringify(VALID_VOICE) - if (path.endsWith("brands/acme/logos/logos.json")) return JSON.stringify(VALID_LOGOS) - if (path.endsWith("brands/acme/backgrounds.json")) return JSON.stringify(VALID_BACKGROUNDS) - if (path.endsWith("brands/acme/banned-words.json")) throw enoent() - if (path.endsWith("brands/acme/products.json")) throw enoent() + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/backgrounds.json") return Buffer.from(JSON.stringify(VALID_BACKGROUNDS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/products.json") throw enoent() throw enoent() }) const profile = await loadBrandProfile("acme") From 8926f12ff7b53b101fc3b3ab1d48293a7ff0eef5 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 08:43:49 -0700 Subject: [PATCH 2/5] fix(tests): correct Dirent type casts for Node 22 types Use Awaited> instead of Dirent[] to match the NonSharedBuffer generic parameter in newer @types/node. --- tests/storage-adapter.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/storage-adapter.test.ts b/tests/storage-adapter.test.ts index 6883cb3..18bc181 100644 --- a/tests/storage-adapter.test.ts +++ b/tests/storage-adapter.test.ts @@ -180,16 +180,16 @@ describe("LocalFsAdapter", () => { vi.mocked(fs.readdir).mockResolvedValue([ { name: "a.png", isFile: () => true, isDirectory: () => false }, { name: "subdir", isFile: () => false, isDirectory: () => true }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) // When readdir is called for the subdir, return one file vi.mocked(fs.readdir).mockResolvedValueOnce([ { name: "a.png", isFile: () => true, isDirectory: () => false }, { name: "subdir", isFile: () => false, isDirectory: () => true }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) vi.mocked(fs.readdir).mockResolvedValueOnce([ { name: "b.png", isFile: () => true, isDirectory: () => false }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) const result = await adapter.listFiles("outputs", "camp") @@ -202,17 +202,17 @@ describe("LocalFsAdapter", () => { // Top-level: one directory vi.mocked(fs.readdir).mockResolvedValueOnce([ { name: "market", isFile: () => false, isDirectory: () => true }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) // market/: one file + one directory vi.mocked(fs.readdir).mockResolvedValueOnce([ { name: "product", isFile: () => false, isDirectory: () => true }, { name: "brief.json", isFile: () => true, isDirectory: () => false }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) // market/product/: two files vi.mocked(fs.readdir).mockResolvedValueOnce([ { name: "1x1.png", isFile: () => true, isDirectory: () => false }, { name: "16x9.png", isFile: () => true, isDirectory: () => false }, - ] as unknown as import("node:fs").Dirent[]) + ] as unknown as Awaited>) const result = await adapter.listFiles("outputs", "camp") From dcb934e30cd3853c5730c580792da52cc92fba70 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 09:12:43 -0700 Subject: [PATCH 3/5] fix(brands): validate product/background paths for traversal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reject item.file values containing .., ., backslashes, or absolute paths before building container-relative keys. Surfaces violations as BrandInvalidError (400) instead of an unexpected PathTraversalError. Addresses Copilot review on PR #51: - brand-loader.ts — "item.file contains backslashes or traversal segments" - brand-loader.ts — "backgrounds.json item.file is not constrained" --- lib/cast/server/brand-loader.ts | 26 +++++++++++++++-- tests/brand-loader.test.ts | 51 +++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/lib/cast/server/brand-loader.ts b/lib/cast/server/brand-loader.ts index c6332a7..ea64d80 100644 --- a/lib/cast/server/brand-loader.ts +++ b/lib/cast/server/brand-loader.ts @@ -173,7 +173,7 @@ export async function loadBrandProfile(slug: string): Promise { productsRaw, ) for (const item of productsManifest.items) { - const segments = item.file.split("/").filter(Boolean) + const segments = validateItemFile(slug, "products.json", item.file) const itemKey = `brands/${slug}/${segments.join("/")}` await assertExists(storage, itemKey, slug, item.file) canVariants.push({ @@ -196,7 +196,7 @@ export async function loadBrandProfile(slug: string): Promise { backgroundsRaw, ) for (const item of backgroundsManifest.items) { - const segments = item.file.split("/").filter(Boolean) + const segments = validateItemFile(slug, "backgrounds.json", item.file) const itemKey = `brands/${slug}/${segments.join("/")}` await assertExists(storage, itemKey, slug, item.file) backgroundVariants.push({ @@ -334,6 +334,28 @@ function parse( return result.data } +/** + * Reject item.file values containing backslashes, traversal segments (.., .), + * or absolute paths. Surfaces violations as BrandInvalidError so callers get + * a clear 400 instead of an unexpected PathTraversalError from the adapter. + */ +function validateItemFile(slug: string, manifestFile: string, raw: string): string[] { + const segments = raw.split(/[\\/]/).filter(Boolean) + for (const seg of segments) { + if (seg === "." || seg === "..") { + throw new BrandInvalidError(slug, manifestFile, [ + { path: ["file"], message: `path traversal segment "${seg}" in "${raw}"` }, + ]) + } + } + if (/^[a-zA-Z]:/.test(raw) || raw.startsWith("/")) { + throw new BrandInvalidError(slug, manifestFile, [ + { path: ["file"], message: `absolute path not allowed: "${raw}"` }, + ]) + } + return segments +} + function unionLowercase(a: readonly string[], b: readonly string[]): string[] { const set = new Set() for (const s of [...a, ...b]) { diff --git a/tests/brand-loader.test.ts b/tests/brand-loader.test.ts index 0060e56..7d59203 100644 --- a/tests/brand-loader.test.ts +++ b/tests/brand-loader.test.ts @@ -349,6 +349,40 @@ describe("canVariants — products.json", () => { }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) }) + + it("throws BrandInvalidError when item.file contains traversal segment", async () => { + const TRAVERSAL_PRODUCTS = { + items: [{ id: "bad", sku: "BAD-001", file: "../../../etc/passwd", pose: "upright-center", detail: "evil" }], + } + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify(TRAVERSAL_PRODUCTS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() + throw enoent() + }) + await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) + }) + + it("throws BrandInvalidError when item.file contains backslashes", async () => { + const BACKSLASH_PRODUCTS = { + items: [{ id: "bad", sku: "BAD-001", file: "products\\..\\secrets.png", pose: "upright-center", detail: "evil" }], + } + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify(BACKSLASH_PRODUCTS)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() + throw enoent() + }) + await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) + }) }) describe("backgroundVariants — backgrounds.json", () => { @@ -382,4 +416,21 @@ describe("backgroundVariants — backgrounds.json", () => { const profile = await loadBrandProfile("acme") expect(profile.backgroundVariants).toEqual([]) }) + + it("throws BrandInvalidError when item.file contains traversal segment", async () => { + const TRAVERSAL_BG = { + items: [{ id: "bad", file: "../../../etc/shadow", ratio: "1x1", sku: "BAD-001", luminance: "dark" }], + } + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/backgrounds.json") return Buffer.from(JSON.stringify(TRAVERSAL_BG)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/products.json") throw enoent() + throw enoent() + }) + await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) + }) }) From 0b0c69eaee4d1fa27952e3b8941916429f022655 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 09:25:07 -0700 Subject: [PATCH 4/5] fix(brands): reject any backslash in item.file container keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot review on PR #51 (round 2): - brand-loader.ts:validateItemFile — reject backslashes upfront instead of silently splitting on them; container keys are forward-slash only - brand-loader.test.ts — rename traversal-via-backslash test, add pure backslash test case (products\\can.png) --- lib/cast/server/brand-loader.ts | 12 +++++++++--- tests/brand-loader.test.ts | 19 ++++++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/cast/server/brand-loader.ts b/lib/cast/server/brand-loader.ts index ea64d80..f91d1c3 100644 --- a/lib/cast/server/brand-loader.ts +++ b/lib/cast/server/brand-loader.ts @@ -336,11 +336,17 @@ function parse( /** * Reject item.file values containing backslashes, traversal segments (.., .), - * or absolute paths. Surfaces violations as BrandInvalidError so callers get - * a clear 400 instead of an unexpected PathTraversalError from the adapter. + * or absolute paths. Container keys use forward slashes only — any backslash + * is rejected outright. Surfaces violations as BrandInvalidError so callers + * get a clear 400 instead of an unexpected PathTraversalError from the adapter. */ function validateItemFile(slug: string, manifestFile: string, raw: string): string[] { - const segments = raw.split(/[\\/]/).filter(Boolean) + if (raw.includes("\\")) { + throw new BrandInvalidError(slug, manifestFile, [ + { path: ["file"], message: `backslashes not allowed in container key: "${raw}"` }, + ]) + } + const segments = raw.split("/").filter(Boolean) for (const seg of segments) { if (seg === "." || seg === "..") { throw new BrandInvalidError(slug, manifestFile, [ diff --git a/tests/brand-loader.test.ts b/tests/brand-loader.test.ts index 7d59203..be51458 100644 --- a/tests/brand-loader.test.ts +++ b/tests/brand-loader.test.ts @@ -367,7 +367,7 @@ describe("canVariants — products.json", () => { await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) }) - it("throws BrandInvalidError when item.file contains backslashes", async () => { + it("throws BrandInvalidError when item.file contains traversal via backslashes", async () => { const BACKSLASH_PRODUCTS = { items: [{ id: "bad", sku: "BAD-001", file: "products\\..\\secrets.png", pose: "upright-center", detail: "evil" }], } @@ -383,6 +383,23 @@ describe("canVariants — products.json", () => { }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) }) + + it("throws BrandInvalidError when item.file contains backslashes without traversal", async () => { + const BACKSLASH_ONLY = { + items: [{ id: "bad", sku: "BAD-001", file: "products\\can.png", pose: "upright-center", detail: "evil" }], + } + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify(BACKSLASH_ONLY)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() + throw enoent() + }) + await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) + }) }) describe("backgroundVariants — backgrounds.json", () => { From 81055840ab5bf1f1f0d120e84bab6964089ff969 Mon Sep 17 00:00:00 2001 From: Aaron Davis Date: Sat, 9 May 2026 10:08:31 -0700 Subject: [PATCH 5/5] fix(brands): guard empty item.file segments + filter listBrandSlugs by brand.json - validateItemFile: reject inputs that split to zero segments (defense-in-depth) - listBrandSlugs: only list slugs whose brands//brand.json key exists, consistent with loadBrandProfile semantics - Add tests: empty-segments rejection, partial-brand exclusion (224 pass) --- lib/cast/server/brand-loader.ts | 12 +++++++++--- tests/brand-loader.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/cast/server/brand-loader.ts b/lib/cast/server/brand-loader.ts index f91d1c3..a12c41b 100644 --- a/lib/cast/server/brand-loader.ts +++ b/lib/cast/server/brand-loader.ts @@ -235,12 +235,13 @@ export async function listBrandSlugs(): Promise { maybeWarnNoBrands("missing") return [] } - // Extract unique slugs from keys like "brands/brisa/brand.json". + // Extract unique slugs from keys that include brand.json — only list + // brands that loadBrandProfile() will be able to load. const slugSet = new Set() for (const key of keys) { const parts = key.split("/") - // parts[0] === "brands", parts[1] === slug - if (parts.length >= 2 && SLUG_RE.test(parts[1])) { + // Match keys like "brands//brand.json" + if (parts.length >= 3 && parts[2] === "brand.json" && SLUG_RE.test(parts[1])) { slugSet.add(parts[1]) } } @@ -359,6 +360,11 @@ function validateItemFile(slug: string, manifestFile: string, raw: string): stri { path: ["file"], message: `absolute path not allowed: "${raw}"` }, ]) } + if (segments.length === 0) { + throw new BrandInvalidError(slug, manifestFile, [ + { path: ["file"], message: `empty container key: "${raw}"` }, + ]) + } return segments } diff --git a/tests/brand-loader.test.ts b/tests/brand-loader.test.ts index be51458..5eb3642 100644 --- a/tests/brand-loader.test.ts +++ b/tests/brand-loader.test.ts @@ -211,6 +211,16 @@ describe("listBrandSlugs", () => { expect(slugs).toEqual(["alpha", "zeta"]) }) + it("excludes slugs that have files but no brand.json", async () => { + mockAdapter.listFiles.mockResolvedValue([ + "brands/alpha/brand.json", + "brands/partial/logos/logo.png", // no brand.json → excluded + "brands/partial/voice.json", // still no brand.json + ]) + const slugs = await listBrandSlugs() + expect(slugs).toEqual(["alpha"]) + }) + it("returns [] when inputs/brands/ is missing (empty listFiles)", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) mockAdapter.listFiles.mockResolvedValue([]) @@ -400,6 +410,23 @@ describe("canVariants — products.json", () => { }) await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) }) + + it("throws BrandInvalidError when item.file resolves to empty segments", async () => { + const EMPTY_KEY = { + items: [{ id: "empty", sku: "BAD-001", file: "///", pose: "upright-center", detail: "evil" }], + } + mockAdapter.fileExists.mockResolvedValue(true) + mockAdapter.readFile.mockImplementation(async (_c: string, key: string) => { + if (key === "brands/acme/brand.json") return Buffer.from(JSON.stringify(VALID_BRAND)) + if (key === "brands/acme/voice.json") return Buffer.from(JSON.stringify(VALID_VOICE)) + if (key === "brands/acme/logos/logos.json") return Buffer.from(JSON.stringify(VALID_LOGOS)) + if (key === "brands/acme/products.json") return Buffer.from(JSON.stringify(EMPTY_KEY)) + if (key === "brands/acme/banned-words.json") throw enoent() + if (key === "brands/acme/backgrounds.json") throw enoent() + throw enoent() + }) + await expect(loadBrandProfile("acme")).rejects.toBeInstanceOf(BrandInvalidError) + }) }) describe("backgroundVariants — backgrounds.json", () => {