Skip to content

Commit 5356e23

Browse files
committed
fix(providers): harden discovery path and test isolation
1 parent 25d3f61 commit 5356e23

3 files changed

Lines changed: 27 additions & 7 deletions

File tree

src/providers/model-discovery.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,10 @@ export function providerModelDiscoverySpecError(spec: ProviderModelDiscoverySpec
9090
if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) {
9191
return "discovery path must be a query-free relative/origin path";
9292
}
93-
if (path.split("/").includes("..")) return "discovery path must not contain parent-directory segments";
93+
if (path.includes("\\")) return "discovery path must use forward slashes";
94+
if (path.split("/").some(segment => segment.replace(/%2e/gi, ".") === "..")) {
95+
return "discovery path must not contain parent-directory segments";
96+
}
9497
}
9598
const queryEntries = Object.entries(spec.query ?? {});
9699
if (queryEntries.length > 32) return "discovery query may contain at most 32 entries";

tests/helpers/provider-registry-discovery.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ interface RegistryDiscoveryOverrides {
55
preserveCustomDestination?: boolean;
66
}
77

8-
/** Temporarily override registry-owned discovery policy and always clear its cached catalog. */
8+
/** Temporarily override registry-owned discovery policy with a clean cache before and after. */
99
export async function withRegistryDiscovery<T>(
1010
providerId: string,
1111
spec: ProviderModelDiscoverySpec,
@@ -16,6 +16,7 @@ export async function withRegistryDiscovery<T>(
1616
if (!entry) throw new Error(`missing ${providerId} registry entry`);
1717
const originalDiscovery = entry.modelDiscovery;
1818
const originalPreserveCustomDestination = entry.preserveCustomDestination;
19+
clearModelCache(providerId);
1920
entry.modelDiscovery = spec;
2021
if (overrides.preserveCustomDestination !== undefined) {
2122
entry.preserveCustomDestination = overrides.preserveCustomDestination;

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { readFileSync } from "node:fs";
33
import { join } from "node:path";
44
import { gatherRoutedModels } from "../src/codex/catalog";
55
import { catalogHintsFromModelsApiItem } from "../src/codex/catalog/provider-fetch";
6-
import { clearModelCache } from "../src/codex/model-cache";
6+
import { clearModelCache, getFreshCached, setCached } from "../src/codex/model-cache";
77
import { buildModelsRequest } from "../src/oauth";
88
import { KEY_LOGIN_PROVIDERS, validateApiKey } from "../src/oauth/key-providers";
99
import { deriveKeyLoginMap, providerConfigSeed } from "../src/providers/derive";
@@ -67,10 +67,17 @@ describe("registry-owned provider model discovery", () => {
6767
.toContain("https");
6868
expect(providerModelDiscoverySpecError({ path: "models?unbounded=true" }))
6969
.toContain("query-free");
70-
expect(providerModelDiscoverySpecError({ path: "../../internal/models" }))
71-
.toContain("parent-directory");
72-
expect(providerModelDiscoverySpecError({ path: "models/../internal" }))
73-
.toContain("parent-directory");
70+
for (const path of [
71+
"../../internal/models",
72+
"models/../internal",
73+
"models/%2e%2e/internal",
74+
"models/.%2E/internal",
75+
"models/%2e./internal",
76+
]) {
77+
expect(providerModelDiscoverySpecError({ path })).toContain("parent-directory");
78+
}
79+
expect(providerModelDiscoverySpecError({ path: String.raw`models\..\internal` }))
80+
.toContain("forward slashes");
7481
expect(providerModelDiscoverySpecError({ path: "models/model..variant" })).toBeNull();
7582
expect(providerModelDiscoverySpecError({
7683
url: "https://api.example.test/models",
@@ -79,6 +86,15 @@ describe("registry-owned provider model discovery", () => {
7986
expect(providerModelDiscoverySpecError({ maxModels: 25 })).toBeNull();
8087
});
8188

89+
test("clears cached rows before applying a temporary registry discovery policy", async () => {
90+
setCached("together", []);
91+
expect(getFreshCached("together", 60_000)).toEqual([]);
92+
93+
await withTogetherDiscovery({ maxModels: 25 }, () => {
94+
expect(getFreshCached("together", 60_000)).toBeNull();
95+
});
96+
});
97+
8298
test("limits collision preservation to fixed API-key destinations", () => {
8399
for (const entry of PROVIDER_REGISTRY) {
84100
if (entry.preserveCustomDestination !== true) continue;

0 commit comments

Comments
 (0)