From 5e14554bf95be4325d55e03a9572311c3cd0edc6 Mon Sep 17 00:00:00 2001 From: artale Date: Tue, 16 Jun 2026 15:56:53 +0200 Subject: [PATCH] fix: harden factory capability probes --- package.json | 3 +- src/fable5/factory-capabilities.test.ts | 12 +++++++ src/fable5/factory-capabilities.ts | 48 ++++++++++++++++++------- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index d359b08..82bf8e7 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,8 @@ "SKILLS/", "deploy/", "Dockerfile", - ".env.example" + ".env.example", + "factory-capabilities.yaml" ], "scripts": { "build": "tsc", diff --git a/src/fable5/factory-capabilities.test.ts b/src/fable5/factory-capabilities.test.ts index 691893c..7e93657 100644 --- a/src/fable5/factory-capabilities.test.ts +++ b/src/fable5/factory-capabilities.test.ts @@ -28,4 +28,16 @@ describe("factory capabilities", () => { expect(report.missing).toContain("factory-status"); expect(formatCapabilityReport(report)).toContain("FAIL git-proxy provides factory-status, discovery: HTTP 404"); }); + + it("fails closed for malformed registry entries", async () => { + const services = parseFactoryCapabilities(`services: + broken: + provides: [factory-status] +`); + const report = await probeFactoryCapabilities(services, async () => ({ ok: true, status: 200 })); + + expect(report.requiredOk).toBe(false); + expect(report.missing).toContain("factory-status"); + expect(formatCapabilityReport(report)).toContain("FAIL broken provides factory-status: missing url"); + }); }); diff --git a/src/fable5/factory-capabilities.ts b/src/fable5/factory-capabilities.ts index eee1e9a..7d334cd 100644 --- a/src/fable5/factory-capabilities.ts +++ b/src/fable5/factory-capabilities.ts @@ -20,29 +20,34 @@ export interface CapabilityReport { requiredOk: boolean; } -type FetchLike = (url: string) => Promise<{ ok: boolean; status: number }>; +type FetchLike = (url: string, init?: { signal?: AbortSignal }) => Promise<{ ok: boolean; status: number }>; export function parseFactoryCapabilities(text: string): FactoryCapabilityService[] { + // ponytail: tiny YAML subset for this checked-in registry; add a parser only when the format grows. const services: FactoryCapabilityService[] = []; let current: Partial | null = null; for (const raw of text.split(/\r?\n/)) { const line = raw.trim(); if (!line || line === "services:" || line.startsWith("#")) continue; + const service = raw.match(/^ ([A-Za-z0-9_-]+):\s*$/); if (service) { if (current?.name) services.push(normalizeService(current)); current = { name: service[1] }; continue; } + if (!current) continue; const prop = line.match(/^(url|provides|required):\s*(.*)$/); if (!prop) continue; + const [, key, value] = prop; if (key === "url") current.url = value; if (key === "required") current.required = value === "true"; - if (key === "provides") current.provides = value.replace(/^\[/, "").replace(/\]$/, "").split(",").map((s) => s.trim()).filter(Boolean); + if (key === "provides") current.provides = parseInlineList(value); } + if (current?.name) services.push(normalizeService(current)); return services; } @@ -51,15 +56,12 @@ export function loadFactoryCapabilities(file: string): FactoryCapabilityService[ return parseFactoryCapabilities(fs.readFileSync(file, "utf-8")); } -export async function probeFactoryCapabilities(services: FactoryCapabilityService[], fetcher: FetchLike = fetch): Promise { - const results = await Promise.all(services.map(async (svc): Promise => { - try { - const res = await fetcher(svc.url); - return { ...svc, ok: res.ok, status: res.status, error: res.ok ? undefined : `HTTP ${res.status}` }; - } catch (error) { - return { ...svc, ok: false, error: error instanceof Error ? error.message : String(error) }; - } - })); +export async function probeFactoryCapabilities( + services: FactoryCapabilityService[], + fetcher: FetchLike = fetch, + timeoutMs = 5_000, +): Promise { + const results = await Promise.all(services.map((svc) => probeCapability(svc, fetcher, timeoutMs))); const available = [...new Set(results.filter((r) => r.ok).flatMap((r) => r.provides))]; const missing = [...new Set(results.filter((r) => !r.ok).flatMap((r) => r.provides))]; return { results, available, missing, requiredOk: results.every((r) => !r.required || r.ok) }; @@ -70,7 +72,7 @@ export function formatCapabilityReport(report: CapabilityReport): string { "", " Factory Capabilities", " ─────────────────────────────", - ...report.results.map((r) => ` ${r.ok ? "OK " : "FAIL"} ${r.name} provides ${r.provides.join(", ")}${r.ok ? "" : `: ${r.error ?? `HTTP ${r.status ?? "unknown"}`}`}`), + ...report.results.map((r) => ` ${r.ok ? "OK " : "FAIL"} ${r.name} provides ${r.provides.join(", ") || "nothing"}${r.ok ? "" : `: ${r.error ?? `HTTP ${r.status ?? "unknown"}`}`}`), "", ` available: ${report.available.join(", ") || "none"}`, ` missing: ${report.missing.join(", ") || "none"}`, @@ -78,11 +80,31 @@ export function formatCapabilityReport(report: CapabilityReport): string { ].join("\n"); } +async function probeCapability(svc: FactoryCapabilityService, fetcher: FetchLike, timeoutMs: number): Promise { + if (!svc.url) return { ...svc, ok: false, error: "missing url" }; + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), timeoutMs); + try { + const res = await fetcher(svc.url, { signal: controller.signal }); + return { ...svc, ok: res.ok, status: res.status, error: res.ok ? undefined : `HTTP ${res.status}` }; + } catch (error) { + const message = error instanceof Error && error.name === "AbortError" ? "timed out" : error instanceof Error ? error.message : String(error); + return { ...svc, ok: false, error: message }; + } finally { + clearTimeout(timeout); + } +} + +function parseInlineList(value: string): string[] { + return value.replace(/^\[/, "").replace(/\]$/, "").split(",").map((s) => s.trim()).filter(Boolean); +} + function normalizeService(svc: Partial): FactoryCapabilityService { return { name: svc.name ?? "unknown", url: svc.url ?? "", provides: svc.provides ?? [], - required: svc.required ?? false, + required: svc.required ?? true, }; }