From a14c46ccf43c5d935169fc7ef9599fe8b07d47f6 Mon Sep 17 00:00:00 2001 From: Jan Jaap Date: Tue, 4 Aug 2026 12:19:23 +0200 Subject: [PATCH] fix(profiles): resolve inherits by declared name, emit filament_vendor, log degrades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related silent failures in the bundled-profile inheritance walk. 1. Slash-named parents broke the walk. Some bundled profiles declare a `name` containing a literal `/` while the file on disk substitutes something else — and the substitute is not consistent: "Bambu Support For PA/PET @base" -> "Bambu Support For PA PET @base.json" "Bambu Support For PLA/PETG @base" -> "Bambu Support For PLA-PETG @base.json" profile-resolver derived the parent's path from the `inherits` string, so the `/` acted as a directory separator, the read ENOENTed, and the whole remaining ancestor chain was dropped without a word. Because the walk is shared with the slice path and /resolved-process, a user preset inheriting such a base sliced under-specified, silently. Parents are now located by each bundled file's *declared* `name` via a directory index rather than by re-deriving a path — the sanitization scheme is undocumented and demonstrably differs per case, so guessing at it is the wrong approach. The direct path is still tried first and the index is built lazily only on a miss, memoised per category directory: resolveProfile runs once per preset in a ~2500-file listing loop and the basename matches for the overwhelming majority, so the hot path stays a single readFile and cold-listing latency is unchanged. This also closes a path traversal: a `..` (or absolute path) in an `inherits` value previously reached outside the profiles directory by the same mechanism. The direct lookup now refuses anything that is not a plain basename, and the index can only ever name files enumerated from inside the category directory. 2. `filament_vendor` sits on the family base ("Bambu ABS @base" -> ["Bambu Lab"]), one hop up the chain the walk already traverses, and was never read. It now gets the same treatment as colour, reusing firstScalar so an empty string or non-string yields null rather than a fabricated value. It has only one spelling — no second key was invented. 3. Both degrade paths in GET /profiles/bundled logged nothing, so a systematic resolution failure would present as the endpoint quietly answering all-null again. Both now warn, and the listing emits a resolved-count line — which also covers the dangling-parent case, whose degrade is silent by design and never throws. Co-Authored-By: Claude Opus 5 --- src/routes/profiles/route.ts | 57 +++- src/routes/slicing/profile-resolver.ts | 149 ++++++++- tests/e2e/bundled-filament-resolution.spec.ts | 289 ++++++++++++++++++ tests/unit/profile-resolver.spec.ts | 209 ++++++++++++- 4 files changed, 685 insertions(+), 19 deletions(-) create mode 100644 tests/e2e/bundled-filament-resolution.spec.ts diff --git a/src/routes/profiles/route.ts b/src/routes/profiles/route.ts index 073b0b2..066cef7 100644 --- a/src/routes/profiles/route.ts +++ b/src/routes/profiles/route.ts @@ -50,6 +50,11 @@ type BundledFilament = { // when possible without being handed a fabricated value. filament_type: string | null; filament_colour: string | null; + // Also one hop up the same chain: `filament_vendor` sits on the family + // base (`Bambu ABS @base` -> `["Bambu Lab"]`) in both slicers, never on + // the per-printer leaf. It comes free with the walk we already do, and + // lets Bambuddy group/filter the Standard tier by brand. + filament_vendor: string | null; }; type BundledIndex = { printer: { name: string; base_id: string | null }[]; @@ -85,6 +90,18 @@ router.get("/bundled", async (_req, res) => { bundledPath, )) as BundledFilament[], }; + // Resolution counts, once per cache fill. The per-preset degrade paths warn + // when the walk *throws*, but the commonest silent failure mode — a + // dangling `inherits` that resolves to nothing, which by design neither + // throws nor warns — shows up only as fields quietly going `null` across + // the tier. This line is what makes that regression self-diagnosing. + const resolved = (key: keyof BundledFilament) => + result.filament.filter((f) => f[key] !== null).length; + console.info( + `[profiles/bundled] listing built: ${result.printer.length} printer, ${result.process.length} process, ${result.filament.length} filament ` + + `(filament_type ${resolved("filament_type")}, filament_vendor ${resolved("filament_vendor")}, filament_colour ${resolved("filament_colour")} resolved)`, + ); + bundledIndexCache = result; bundledIndexCachedAt = now; res.status(200).json(result); @@ -114,13 +131,8 @@ async function readBundledDir( const filePath = path.join(dir, entry); try { const raw = await fs.promises.readFile(filePath, "utf8"); - const json = JSON.parse(raw) as { - name?: string; - inherits?: string; + const json = JSON.parse(raw) as RawFilamentFields & { instantiation?: string; - filament_type?: string | string[]; - filament_colour?: string | string[]; - default_filament_colour?: string | string[]; }; // Bundled profiles ship a mix of concrete presets and abstract bases // (e.g. `fdm_filament_pla`). Skip the latter so the slicer modal only @@ -137,9 +149,13 @@ async function readBundledDir( } else { out.push(base); } - } catch { + } catch (err) { // Corrupted / unreadable individual file — skip without breaking the - // rest of the listing. + // rest of the listing, but say so: a systematic read failure would + // otherwise present as a silently empty (or short) listing. + console.warn( + `[profiles/bundled] skipping unreadable bundled profile ${filePath}: ${err instanceof Error ? err.message : String(err)}`, + ); continue; } } @@ -149,10 +165,14 @@ async function readBundledDir( } type RawFilamentFields = { + name?: string; inherits?: string; filament_type?: string | string[]; filament_colour?: string | string[]; default_filament_colour?: string | string[]; + // Only one spelling exists for this one — unlike colour, there is no + // `default_filament_vendor` in either bundled tree. Do not invent one. + filament_vendor?: string | string[]; }; /** @@ -173,24 +193,39 @@ type RawFilamentFields = { async function filamentMetadata( leaf: RawFilamentFields, bundledProfilesPath: string, -): Promise<{ filament_type: string | null; filament_colour: string | null }> { +): Promise<{ + filament_type: string | null; + filament_colour: string | null; + filament_vendor: string | null; +}> { let fields: RawFilamentFields = leaf; const needsWalk = typeof leaf.inherits === "string" && leaf.inherits.length > 0 && - (firstScalar(leaf.filament_type) === null || colourOf(leaf) === null); + (firstScalar(leaf.filament_type) === null || + colourOf(leaf) === null || + firstScalar(leaf.filament_vendor) === null); if (needsWalk) { try { fields = (await resolveProfile({ ...leaf }, "filament", { bundledProfilesPath, })) as RawFilamentFields; - } catch { + } catch (err) { + // Degrading to leaf-only metadata is deliberate (one preset loses a + // match; failing would cost the caller the whole Standard tier) — but + // it must not be *silent*. If resolution ever breaks systematically + // this endpoint would otherwise go back to answering all-`null` with + // no trace of why. + console.warn( + `[profiles/bundled] inherits walk failed for filament preset "${leaf.name ?? ""}" (inherits="${leaf.inherits}"); falling back to leaf-only metadata: ${err instanceof Error ? err.message : String(err)}`, + ); fields = leaf; } } return { filament_type: firstScalar(fields.filament_type), filament_colour: colourOf(fields), + filament_vendor: firstScalar(fields.filament_vendor), }; } diff --git a/src/routes/slicing/profile-resolver.ts b/src/routes/slicing/profile-resolver.ts index c0acb42..9177779 100644 --- a/src/routes/slicing/profile-resolver.ts +++ b/src/routes/slicing/profile-resolver.ts @@ -47,19 +47,16 @@ export async function resolveProfile( } const parentName = current.inherits; - const parentPath = path.join( + const found = await readParentProfile( options.bundledProfilesPath, category, - `${parentName}.json`, + parentName, ); - - let parentRaw: string; - try { - parentRaw = await fs.readFile(parentPath, "utf-8"); - } catch { + if (!found) { delete current.inherits; break; } + const { filePath: parentPath, raw: parentRaw } = found; let parent: ProfileJson; try { @@ -96,6 +93,144 @@ export async function resolveProfile( return current; } +/** + * Locate and read the bundled file an `inherits:` value names. + * + * Two things make this more than a `path.join`: + * + * 1. **A profile's declared `name` is not always its file basename.** Some + * bundled profiles carry a literal `/` in `name` / `inherits` while the + * file on disk sanitizes it — and the two slicers do not agree on the + * replacement character: + * + * inherits "Bambu Support For PA/PET @base" -> "Bambu Support For PA PET @base.json" + * inherits "Bambu Support For PLA/PETG @base" -> "Bambu Support For PLA-PETG @base.json" + * + * Deriving the path from the name treats the `/` as a directory + * separator, the read ENOENTs, and the whole remaining ancestor chain is + * dropped silently — so a preset inheriting such a base sliced + * under-specified. Because the sanitization scheme is undocumented and + * demonstrably inconsistent, we do not guess at it: we index the + * directory by each file's *declared* `name` and look the parent up + * there. + * + * 2. **`inherits` is attacker-shaped input** on the user-upload paths. A + * `..` segment in it reached outside the profiles directory by the same + * path-derivation mechanism. The direct lookup is now refused for any + * value that is not a plain basename, so nothing outside the category + * directory is reachable; the index only ever contains files enumerated + * from inside it. + * + * Ordering is deliberate: the direct read is tried **first** and the index + * is built lazily only when it misses. `resolveProfile` runs once per preset + * in a ~2500-file listing loop and the file basename matches the declared + * name for the overwhelming majority, so the hot path stays exactly one + * `readFile` — no directory enumeration, no behavioural change, no + * regression to the cold-listing latency. The index cost is paid only for + * the rare slash-named case, and only once per directory. + * + * Returns `null` when the parent cannot be located; callers preserve the + * existing "drop `inherits` and stop walking" behaviour for that case. + */ +async function readParentProfile( + bundledProfilesPath: string, + category: ProfileCategory, + parentName: string, +): Promise<{ filePath: string; raw: string } | null> { + const dir = path.join(bundledProfilesPath, category); + + if (isPlainBasename(parentName)) { + const direct = path.join(dir, `${parentName}.json`); + try { + return { filePath: direct, raw: await fs.readFile(direct, "utf-8") }; + } catch { + // Fall through: the name may be declared by a file whose basename was + // sanitized, or it may simply be dangling. + } + } + + const indexed = (await getNameIndex(dir)).get(parentName); + if (indexed === undefined) return null; + try { + return { filePath: indexed, raw: await fs.readFile(indexed, "utf-8") }; + } catch { + return null; + } +} + +/** + * True when `name` can safely be used as a file basename inside the category + * directory — i.e. it cannot escape it. Anything with a path separator (a + * slash-bearing profile name included) is refused here and resolved through + * the index instead, which is exactly right: the index cannot name a file + * the directory listing did not produce. + */ +function isPlainBasename(name: string): boolean { + return ( + name.length > 0 && + !name.includes("/") && + !name.includes("\\") && + !name.includes("\0") && + name !== "." && + name !== ".." && + !path.isAbsolute(name) + ); +} + +type NameIndex = Map; + +// Keyed by the category directory, which already encodes +// (bundledProfilesPath, category). The bundled tree lives in the slicer's +// read-only `resources/profiles/` and only changes when the container image +// is rebuilt, so a process-lifetime cache is safe — and necessary: rebuilding +// per `resolveProfile` call would make the bundled listing O(n^2) in the +// number of profiles. +const nameIndexCache = new Map>(); + +/** Test seam: drop the memoised directory indexes. */ +export function resetProfileNameIndexCache(): void { + nameIndexCache.clear(); +} + +async function getNameIndex(dir: string): Promise { + const cached = nameIndexCache.get(dir); + if (cached) return cached; + const building = buildNameIndex(dir); + nameIndexCache.set(dir, building); + try { + return await building; + } catch { + // A transient FS failure must not poison the cache for the process + // lifetime; an absent directory simply resolves nothing, as before. + nameIndexCache.delete(dir); + return new Map(); + } +} + +async function buildNameIndex(dir: string): Promise { + const index: NameIndex = new Map(); + const entries = await fs.readdir(dir); + for (const entry of entries) { + if (!entry.endsWith(".json")) continue; + const filePath = path.join(dir, entry); + let declared: unknown; + try { + const raw = await fs.readFile(filePath, "utf-8"); + declared = (JSON.parse(raw) as ProfileJson).name; + } catch { + // Unreadable or malformed bundled file: not indexable. It stays + // reachable by basename via the direct path, where a JSON error is + // still reported rather than swallowed. + continue; + } + if (typeof declared !== "string" || declared.length === 0) continue; + // First writer wins, so a duplicate declared name resolves + // deterministically (readdir order) rather than depending on call order. + if (!index.has(declared)) index.set(declared, filePath); + } + return index; +} + /** * Strip "auto" sentinel values from a user-exported delta in place. * diff --git a/tests/e2e/bundled-filament-resolution.spec.ts b/tests/e2e/bundled-filament-resolution.spec.ts new file mode 100644 index 0000000..80f7d7a --- /dev/null +++ b/tests/e2e/bundled-filament-resolution.spec.ts @@ -0,0 +1,289 @@ +/** + * `GET /profiles/bundled` — the resolution paths that used to fail quietly. + * + * Three separate silent failures are pinned here: + * + * 1. **Slash-named ancestors.** Some bundled profiles declare a `name` with a + * literal `/` while the file on disk substitutes something else — and the + * substitute is not consistent (`PA/PET` -> `PA PET`, `PLA/PETG` -> + * `PLA-PETG`). Deriving the parent's path from the `inherits` string made + * the `/` act as a directory separator; the read ENOENTed and the entire + * remaining ancestor chain was dropped without a word. + * + * 2. **`filament_vendor`.** It sits on the family base, one hop up the chain + * the walk already traverses, and was simply never read. + * + * 3. **Silent degrade.** A preset whose chain cannot be resolved falls back + * to leaf-only metadata rather than failing the listing — correct, but it + * used to leave no trace, so a systematic resolution failure would present + * as the endpoint quietly answering all-`null` again. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from "vitest"; +import type { MockInstance } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { request } from "./setup"; + +type Entry = { + name: string; + filament_type: string | null; + filament_colour: string | null; + filament_vendor: string | null; +}; + +let root: string; +let previousPath: string | undefined; + +function write(dir: string, file: string, json: Record) { + fs.writeFileSync(path.join(dir, `${file}.json`), JSON.stringify(json), "utf8"); +} + +beforeAll(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), "bundled-resolution-")); + const filament = path.join(root, "filament"); + fs.mkdirSync(filament); + fs.mkdirSync(path.join(root, "machine")); + fs.mkdirSync(path.join(root, "process")); + + // --- vendor one hop up a plain chain ------------------------------------ + write(filament, "fdm_filament_abs", { + name: "fdm_filament_abs", + instantiation: "false", + filament_type: ["ABS"], + }); + write(filament, "Bambu ABS @base", { + name: "Bambu ABS @base", + inherits: "fdm_filament_abs", + instantiation: "false", + filament_vendor: ["Bambu Lab"], + default_filament_colour: ["#000000"], + }); + write(filament, "Bambu ABS @BBL H2S", { + name: "Bambu ABS @BBL H2S", + inherits: "Bambu ABS @base", + instantiation: "true", + }); + + // --- vendor genuinely absent / empty / not a string ---------------------- + write(filament, "Generic PLA @BBL H2S", { + name: "Generic PLA @BBL H2S", + instantiation: "true", + filament_type: ["PLA"], + }); + write(filament, "Empty Vendor @BBL H2S", { + name: "Empty Vendor @BBL H2S", + instantiation: "true", + filament_type: ["PLA"], + filament_vendor: "", + }); + write(filament, "Numeric Vendor @BBL H2S", { + name: "Numeric Vendor @BBL H2S", + instantiation: "true", + filament_type: ["PLA"], + filament_vendor: [42], + }); + + // --- slash-named ancestor, sanitization "/" -> " " ----------------------- + write(filament, "Bambu Support For PA PET @base", { + name: "Bambu Support For PA/PET @base", + instantiation: "false", + filament_type: ["PA"], + filament_vendor: ["Bambu Lab"], + default_filament_colour: ["#FFFFFF"], + }); + write(filament, "Bambu Support For PA PET @BBL X1C", { + name: "Bambu Support For PA/PET @BBL X1C", + inherits: "Bambu Support For PA/PET @base", + instantiation: "true", + }); + + // --- slash-named ancestor, sanitization "/" -> "-" ----------------------- + write(filament, "Bambu Support For PLA-PETG @base", { + name: "Bambu Support For PLA/PETG @base", + instantiation: "false", + filament_type: ["PLA"], + filament_vendor: ["Bambu Lab"], + }); + write(filament, "Bambu Support For PLA-PETG @BBL X1C", { + name: "Bambu Support For PLA/PETG @BBL X1C", + inherits: "Bambu Support For PLA/PETG @base", + instantiation: "true", + }); + + // --- degrade: cyclic ancestors hit the depth cap and throw --------------- + write(filament, "cycle_a", { + name: "cycle_a", + instantiation: "false", + inherits: "cycle_b", + }); + write(filament, "cycle_b", { + name: "cycle_b", + instantiation: "false", + inherits: "cycle_a", + // If any of this leaked into the leaf the degrade would be reporting + // half-resolved data, which is worse than leaf-only. + filament_type: ["ASA"], + filament_vendor: ["Ghost Vendor"], + }); + write(filament, "Cyclic @BBL H2S", { + name: "Cyclic @BBL H2S", + inherits: "cycle_a", + instantiation: "true", + filament_type: ["TPU"], + }); + + // --- degrade: corrupt ancestor ------------------------------------------- + fs.writeFileSync( + path.join(filament, "broken_base.json"), + "{ not valid json", + "utf8", + ); + write(filament, "Corrupt Ancestor @BBL H2S", { + name: "Corrupt Ancestor @BBL H2S", + inherits: "broken_base", + instantiation: "true", + filament_type: ["PC"], + }); + + previousPath = process.env.BUNDLED_PROFILES_PATH; + process.env.BUNDLED_PROFILES_PATH = root; +}); + +afterAll(() => { + if (previousPath === undefined) delete process.env.BUNDLED_PROFILES_PATH; + else process.env.BUNDLED_PROFILES_PATH = previousPath; + fs.rmSync(root, { recursive: true, force: true }); +}); + +describe("GET /profiles/bundled — inheritance resolution", () => { + let byName: Map; + let warnSpy: MockInstance; + let infoSpy: MockInstance; + + beforeAll(async () => { + // The listing sits behind a 1h in-process cache, so the spies must be in + // place before the one request that actually builds it. + warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + infoSpy = vi.spyOn(console, "info").mockImplementation(() => {}); + const res = await request.get("/profiles/bundled").expect(200); + byName = new Map( + (res.body.filament as Entry[]).map((f) => [f.name, f] as const), + ); + }); + + afterAll(() => { + warnSpy.mockRestore(); + infoSpy.mockRestore(); + }); + + it("resolves a parent whose file substitutes a space for the name's slash", () => { + expect(byName.get("Bambu Support For PA/PET @BBL X1C")).toMatchObject({ + filament_type: "PA", + filament_vendor: "Bambu Lab", + filament_colour: "#FFFFFF", + }); + }); + + it("resolves a parent whose file substitutes a hyphen for the name's slash", () => { + // The two observed sanitizations differ, so both are pinned: a fix that + // guesses at one substitution scheme passes one of these and fails the + // other. + expect(byName.get("Bambu Support For PLA/PETG @BBL X1C")).toMatchObject({ + filament_type: "PLA", + filament_vendor: "Bambu Lab", + }); + }); + + it("emits filament_vendor resolved from an ancestor", () => { + expect(byName.get("Bambu ABS @BBL H2S")).toMatchObject({ + filament_type: "ABS", + filament_colour: "#000000", + filament_vendor: "Bambu Lab", + }); + }); + + it("emits filament_vendor: null when no profile in the chain states one", () => { + const entry = byName.get("Generic PLA @BBL H2S"); + expect(entry).toHaveProperty("filament_vendor"); + expect(entry?.filament_vendor).toBeNull(); + }); + + it("emits filament_vendor: null for an empty string rather than an empty value", () => { + expect(byName.get("Empty Vendor @BBL H2S")?.filament_vendor).toBeNull(); + }); + + it("emits filament_vendor: null for a non-string value", () => { + expect(byName.get("Numeric Vendor @BBL H2S")?.filament_vendor).toBeNull(); + }); + + it("degrades a cyclic chain to leaf-only metadata without failing the listing", () => { + expect(byName.get("Cyclic @BBL H2S")).toMatchObject({ + filament_type: "TPU", + filament_colour: null, + // NOT "Ghost Vendor": a chain that blew the depth cap resolved nothing. + filament_vendor: null, + }); + }); + + it("degrades a corrupt ancestor to leaf-only metadata", () => { + expect(byName.get("Corrupt Ancestor @BBL H2S")).toMatchObject({ + filament_type: "PC", + filament_vendor: null, + }); + }); + + it("still returns every other preset when some chains fail to resolve", () => { + expect([...byName.keys()].sort()).toEqual([ + "Bambu ABS @BBL H2S", + "Bambu Support For PA/PET @BBL X1C", + "Bambu Support For PLA/PETG @BBL X1C", + "Corrupt Ancestor @BBL H2S", + "Cyclic @BBL H2S", + "Empty Vendor @BBL H2S", + "Generic PLA @BBL H2S", + "Numeric Vendor @BBL H2S", + ]); + }); + + it("warns when an inherits walk degrades to leaf-only metadata", () => { + const warnings = warnSpy.mock.calls.map((c) => String(c[0])); + expect( + warnings.some( + (m) => + m.includes("inherits walk failed") && m.includes("Cyclic @BBL H2S"), + ), + ).toBe(true); + expect( + warnings.some( + (m) => + m.includes("inherits walk failed") && + m.includes("Corrupt Ancestor @BBL H2S"), + ), + ).toBe(true); + }); + + it("warns when an individual bundled file cannot be read or parsed", () => { + const warnings = warnSpy.mock.calls.map((c) => String(c[0])); + expect( + warnings.some( + (m) => + m.includes("skipping unreadable bundled profile") && + m.includes("broken_base.json"), + ), + ).toBe(true); + }); + + it("reports how many presets resolved each field once the listing is built", () => { + // The dangling-parent degrade is silent by design and never throws, so a + // tier-wide regression to all-null would produce no warning at all. This + // line is what makes that case diagnosable. + const lines = infoSpy.mock.calls.map((c) => String(c[0])); + const built = lines.find((m) => m.includes("[profiles/bundled] listing built")); + expect(built).toBeDefined(); + expect(built).toContain("8 filament"); + // ABS + 2 slash-named bases = 3 vendors resolved. + expect(built).toContain("filament_vendor 3"); + }); +}); diff --git a/tests/unit/profile-resolver.spec.ts b/tests/unit/profile-resolver.spec.ts index d5dfb98..56586b5 100644 --- a/tests/unit/profile-resolver.spec.ts +++ b/tests/unit/profile-resolver.spec.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { promises as fs } from "fs"; import * as os from "os"; import * as path from "path"; @@ -7,6 +7,7 @@ import { mergeProfiles, normalizeFromField, resolveProfile, + resetProfileNameIndexCache, getDefaultBundledProfilesPath, type ProfileJson, } from "../../src/routes/slicing/profile-resolver"; @@ -281,6 +282,212 @@ describe("resolveProfile", () => { }); }); +/** + * A bundled profile's declared `name` is not always its file basename, and + * `inherits` is user-controlled on the upload paths. Both facts break a + * resolver that derives the parent's path from the `inherits` string. + */ +describe("resolveProfile — parent lookup by declared name", () => { + let bundledProfilesPath: string; + let filamentDir: string; + + const write = (dir: string, file: string, json: Record) => + fs.writeFile(path.join(dir, `${file}.json`), JSON.stringify(json), "utf8"); + + beforeAll(async () => { + bundledProfilesPath = await fs.mkdtemp( + path.join(os.tmpdir(), "test-inherits-index-"), + ); + filamentDir = path.join(bundledProfilesPath, "filament"); + await fs.mkdir(filamentDir, { recursive: true }); + await fs.mkdir(path.join(bundledProfilesPath, "machine"), { + recursive: true, + }); + + // --- Observed sanitization #1: "/" -> " " ------------------------------ + // inherits value : "Bambu Support For PA/PET @base" + // file on disk : "Bambu Support For PA PET @base.json" + await write(filamentDir, "fdm_filament_pa", { + name: "fdm_filament_pa", + filament_type: ["PA-CF"], + filament_vendor: ["Bambu Lab"], + }); + await write(filamentDir, "Bambu Support For PA PET @base", { + name: "Bambu Support For PA/PET @base", + inherits: "fdm_filament_pa", + default_filament_colour: ["#FFFFFF"], + }); + + // --- Observed sanitization #2: "/" -> "-" ------------------------------ + // The replacement character differs from case #1, which is exactly why + // re-deriving a path from the name is the wrong approach. + await write(filamentDir, "fdm_filament_pla", { + name: "fdm_filament_pla", + filament_type: ["PLA"], + }); + await write(filamentDir, "Bambu Support For PLA-PETG @base", { + name: "Bambu Support For PLA/PETG @base", + inherits: "fdm_filament_pla", + }); + + // A file whose basename and declared name agree — the overwhelmingly + // common case, and the one the direct-path fast lane must keep serving. + await write(filamentDir, "Bambu PLA Basic @base", { + name: "Bambu PLA Basic @base", + filament_type: ["PLA"], + }); + + // The traversal target: a real, readable file OUTSIDE the filament + // directory that a `..` in `inherits` used to reach. + await write(path.join(bundledProfilesPath, "machine"), "secret", { + name: "secret", + exfiltrated: "yes", + }); + + resetProfileNameIndexCache(); + }); + + afterAll(async () => { + resetProfileNameIndexCache(); + await fs.rm(bundledProfilesPath, { recursive: true, force: true }); + }); + + it("resolves a slash-bearing parent whose file substitutes a space", async () => { + const leaf: ProfileJson = { + type: "filament", + name: "Bambu Support For PA/PET @BBL X1C", + inherits: "Bambu Support For PA/PET @base", + }; + const result = await resolveProfile(leaf, "filament", { + bundledProfilesPath, + }); + // Resolved through the slash-named base AND on to its own ancestor. + expect(result.filament_type).toEqual(["PA-CF"]); + expect(result.filament_vendor).toEqual(["Bambu Lab"]); + expect(result.default_filament_colour).toEqual(["#FFFFFF"]); + expect(result.inherits).toBeUndefined(); + expect(result.name).toBe("Bambu Support For PA/PET @BBL X1C"); + }); + + it("resolves a slash-bearing parent whose file substitutes a hyphen", async () => { + const leaf: ProfileJson = { + type: "filament", + name: "Bambu Support For PLA/PETG @BBL X1C", + inherits: "Bambu Support For PLA/PETG @base", + }; + const result = await resolveProfile(leaf, "filament", { + bundledProfilesPath, + }); + expect(result.filament_type).toEqual(["PLA"]); + expect(result.inherits).toBeUndefined(); + }); + + it("still resolves parents whose basename matches their declared name", async () => { + const leaf: ProfileJson = { + type: "filament", + inherits: "Bambu PLA Basic @base", + }; + const result = await resolveProfile(leaf, "filament", { + bundledProfilesPath, + }); + expect(result.filament_type).toEqual(["PLA"]); + }); + + it("does not read outside the category directory via a '..' inherits", async () => { + // Sanity-check the fixture: the file the traversal aims at really exists + // and really is readable, so a passing assertion below means the lookup + // refused it rather than the file merely being absent. + await expect( + fs.readFile( + path.join(bundledProfilesPath, "machine", "secret.json"), + "utf-8", + ), + ).resolves.toContain("exfiltrated"); + + const readSpy = vi.spyOn(fs, "readFile"); + try { + const leaf: ProfileJson = { + type: "filament", + name: "evil", + inherits: "../machine/secret", + }; + const result = await resolveProfile(leaf, "filament", { + bundledProfilesPath, + }); + // Nothing from outside the filament directory leaked in, and the + // unresolvable parent was dropped exactly like any dangling one. + expect(result.exfiltrated).toBeUndefined(); + expect(result.inherits).toBeUndefined(); + // And the escaping path was never even opened. + const opened = readSpy.mock.calls.map((c) => String(c[0])); + expect(opened.some((p) => p.includes("secret.json"))).toBe(false); + } finally { + readSpy.mockRestore(); + } + }); + + it("does not read outside the category directory via an absolute inherits", async () => { + const absolute = path.join(bundledProfilesPath, "machine", "secret"); + const result = await resolveProfile( + { type: "filament", inherits: absolute }, + "filament", + { bundledProfilesPath }, + ); + expect(result.exfiltrated).toBeUndefined(); + expect(result.inherits).toBeUndefined(); + }); + + it("builds the directory index at most once across many resolutions", async () => { + // The listing loop calls resolveProfile once per preset (~2500 files). + // Re-enumerating the directory per call would be O(n^2); this pins the + // memoisation that prevents it. + resetProfileNameIndexCache(); + const readdirSpy = vi.spyOn(fs, "readdir"); + try { + for (let i = 0; i < 5; i += 1) { + await resolveProfile( + { type: "filament", inherits: "Bambu Support For PA/PET @base" }, + "filament", + { bundledProfilesPath }, + ); + } + expect(readdirSpy).toHaveBeenCalledTimes(1); + } finally { + readdirSpy.mockRestore(); + } + }); + + it("does not enumerate the directory when the direct path hits", async () => { + resetProfileNameIndexCache(); + const readdirSpy = vi.spyOn(fs, "readdir"); + try { + await resolveProfile( + { type: "filament", inherits: "Bambu PLA Basic @base" }, + "filament", + { bundledProfilesPath }, + ); + expect(readdirSpy).not.toHaveBeenCalled(); + } finally { + readdirSpy.mockRestore(); + } + }); + + it("still drops a genuinely dangling inherits after the index misses", async () => { + resetProfileNameIndexCache(); + const result = await resolveProfile( + { + type: "filament", + name: "orphan", + inherits: "No Such/Base @base", + }, + "filament", + { bundledProfilesPath }, + ); + expect(result.inherits).toBeUndefined(); + expect(result.name).toBe("orphan"); + }); +}); + describe("ensureProfileType", () => { it("stamps type from category when missing", () => { // BambuStudio's "Export Preset Bundle" omits `type:` on System-tier