fix(providers): harden discovery contract cleanup

Author: olddonkey

src/providers/model-discovery.ts

@@ -90,6 +90,7 @@ export function providerModelDiscoverySpecError(spec: ProviderModelDiscoverySpec
     if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) {
       return "discovery path must be a query-free relative/origin path";
     }
+    if (path.split("/").includes("..")) return "discovery path must not contain parent-directory segments";
   }
   const queryEntries = Object.entries(spec.query ?? {});
   if (queryEntries.length > 32) return "discovery query may contain at most 32 entries";

tests/helpers/provider-registry-discovery.ts

@@ -0,0 +1,32 @@
+import { clearModelCache } from "../../src/codex/model-cache";
+import { PROVIDER_REGISTRY, type ProviderModelDiscoverySpec } from "../../src/providers/registry";
+
+interface RegistryDiscoveryOverrides {
+  preserveCustomDestination?: boolean;
+}
+
+/** Temporarily override registry-owned discovery policy and always clear its cached catalog. */
+export async function withRegistryDiscovery<T>(
+  providerId: string,
+  spec: ProviderModelDiscoverySpec,
+  run: () => Promise<T> | T,
+  overrides: RegistryDiscoveryOverrides = {},
+): Promise<T> {
+  const entry = PROVIDER_REGISTRY.find(row => row.id === providerId);
+  if (!entry) throw new Error(`missing ${providerId} registry entry`);
+  const originalDiscovery = entry.modelDiscovery;
+  const originalPreserveCustomDestination = entry.preserveCustomDestination;
+  entry.modelDiscovery = spec;
+  if (overrides.preserveCustomDestination !== undefined) {
+    entry.preserveCustomDestination = overrides.preserveCustomDestination;
+  }
+  try {
+    return await run();
+  } finally {
+    if (originalDiscovery === undefined) delete entry.modelDiscovery;
+    else entry.modelDiscovery = originalDiscovery;
+    if (originalPreserveCustomDestination === undefined) delete entry.preserveCustomDestination;
+    else entry.preserveCustomDestination = originalPreserveCustomDestination;
+    clearModelCache(providerId);
+  }
+}

tests/provider-connection-test.test.ts

@@ -4,8 +4,8 @@ import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { handleManagementAPI } from "../src/server/management-api";
 import { saveConfig } from "../src/config";
-import { PROVIDER_REGISTRY } from "../src/providers/registry";
 import type { OcxConfig } from "../src/types";
+import { withRegistryDiscovery } from "./helpers/provider-registry-discovery";
 
 const TEST_DIR = join(tmpdir(), "ocx-conn-test");
 const previousHome = process.env.OPENCODEX_HOME;
@@ -178,13 +178,9 @@ describe("POST /api/providers/test (WP040 connectivity probe)", () => {
   });
 
   test("reports only eligible deduplicated models from a registry discovery contract", async () => {
-    const entry = PROVIDER_REGISTRY.find(row => row.id === "together");
-    if (!entry) throw new Error("missing together registry entry");
-    const original = entry.modelDiscovery;
-    entry.modelDiscovery = {
+    await withRegistryDiscovery("together", {
       filter: { anyOf: [{ path: ["type"], equalsAny: ["chat"] }] },
-    };
-    try {
+    }, async () => {
       globalThis.fetch = (async () => Response.json({
         data: [
           { id: "chat-model", type: "chat" },
@@ -202,10 +198,7 @@ describe("POST /api/providers/test (WP040 connectivity probe)", () => {
       const { body } = await probe(config, "together");
       expect(body.ok).toBe(true);
       expect(body.models).toBe(1);
-    } finally {
-      if (original === undefined) delete entry.modelDiscovery;
-      else entry.modelDiscovery = original;
-    }
+    });
   });
 
   test("Google's models-array response shape is accepted (x-goog-api-key path)", async () => {

tests/provider-model-discovery-contract.test.ts

@@ -17,6 +17,7 @@ import { PROVIDER_REGISTRY, type ProviderModelDiscoverySpec } from "../src/provi
 import { routeModel } from "../src/router";
 import type { OcxConfig, OcxProviderConfig } from "../src/types";
 import { withStubbedProviderFetch } from "./helpers/catalog-provider-fetch";
+import { withRegistryDiscovery } from "./helpers/provider-registry-discovery";
 
 const FIXTURE = readFileSync(join(import.meta.dir, "fixtures/provider-model-discovery.json"), "utf8");
 const originalFetch = globalThis.fetch;
@@ -36,37 +37,7 @@ async function withTogetherDiscovery<T>(
   spec: ProviderModelDiscoverySpec,
   run: () => Promise<T> | T,
 ): Promise<T> {
-  const entry = togetherEntry();
-  const original = entry.modelDiscovery;
-  const originalPreserveCustomDestination = entry.preserveCustomDestination;
-  entry.modelDiscovery = spec;
-  entry.preserveCustomDestination = true;
-  try {
-    return await run();
-  } finally {
-    if (original === undefined) delete entry.modelDiscovery;
-    else entry.modelDiscovery = original;
-    if (originalPreserveCustomDestination === undefined) delete entry.preserveCustomDestination;
-    else entry.preserveCustomDestination = originalPreserveCustomDestination;
-    clearModelCache("together");
-  }
-}
-
-async function withRegistryDiscovery<T>(
-  providerId: string,
-  spec: ProviderModelDiscoverySpec,
-  run: () => Promise<T> | T,
-): Promise<T> {
-  const entry = PROVIDER_REGISTRY.find(row => row.id === providerId);
-  if (!entry) throw new Error(`missing ${providerId} registry entry`);
-  const original = entry.modelDiscovery;
-  entry.modelDiscovery = spec;
-  try {
-    return await run();
-  } finally {
-    if (original === undefined) delete entry.modelDiscovery;
-    else entry.modelDiscovery = original;
-  }
+  return withRegistryDiscovery("together", spec, run, { preserveCustomDestination: true });
 }
 
 function togetherConfig(overrides: Partial<OcxProviderConfig> = {}): OcxConfig {
@@ -96,6 +67,11 @@ describe("registry-owned provider model discovery", () => {
       .toContain("https");
     expect(providerModelDiscoverySpecError({ path: "models?unbounded=true" }))
       .toContain("query-free");
+    expect(providerModelDiscoverySpecError({ path: "../../internal/models" }))
+      .toContain("parent-directory");
+    expect(providerModelDiscoverySpecError({ path: "models/../internal" }))
+      .toContain("parent-directory");
+    expect(providerModelDiscoverySpecError({ path: "models/model..variant" })).toBeNull();
     expect(providerModelDiscoverySpecError({
       url: "https://api.example.test/models",
       path: "models",