Skip to content

Commit fdab913

Browse files
haasonsaashaasonsaas
committed
Harden spec loading and compile cache
1 parent a2f81b7 commit fdab913

7 files changed

Lines changed: 300 additions & 11 deletions

File tree

‎src/compile-cache.ts‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { compileOperations } from "./compiler.js";
55
import { loadOpenApiDocument } from "./openapi.js";
66
import type { CompileOptions, OperationModel } from "./types.js";
77

8+
const CACHE_FORMAT_VERSION = 2;
9+
810
interface CacheEntry {
911
hash: string;
1012
operations: OperationModel[];
@@ -17,8 +19,7 @@ export async function loadFromCompileCache(
1719
options: CompileOptions = {}
1820
): Promise<Map<string, OperationModel> | null> {
1921
try {
20-
const doc = await loadOpenApiDocument(specPath);
21-
const hash = computeHash({ doc, serverUrl, options });
22+
const hash = await computeSpecInputHash(specPath, serverUrl, options);
2223
const raw = await readFile(resolve(cachePath), "utf8");
2324
const parsed = JSON.parse(raw) as CacheEntry;
2425
if (parsed.hash !== hash) {
@@ -37,15 +38,26 @@ export async function compileWithCache(
3738
cachePath: string,
3839
options: CompileOptions = {}
3940
): Promise<Map<string, OperationModel>> {
40-
const cached = await loadFromCompileCache(specPath, serverUrl, cachePath, options);
41+
const doc = await loadOpenApiDocument(specPath);
42+
return compileDocumentWithCache(doc, specPath, serverUrl, cachePath, options);
43+
}
44+
45+
export async function compileDocumentWithCache(
46+
doc: Record<string, unknown>,
47+
specPath: string,
48+
serverUrl: string | undefined,
49+
cachePath: string,
50+
options: CompileOptions = {}
51+
): Promise<Map<string, OperationModel>> {
52+
const hash = await computeSpecInputHash(specPath, serverUrl, options);
53+
const cached = await readCacheEntry(cachePath, hash);
4154
if (cached) {
4255
return cached;
4356
}
4457

45-
const doc = await loadOpenApiDocument(specPath);
4658
const operations = compileOperations(doc, serverUrl, options);
4759
const entry: CacheEntry = {
48-
hash: computeHash({ doc, serverUrl, options }),
60+
hash,
4961
operations: [...operations.values()]
5062
};
5163

@@ -56,6 +68,34 @@ export async function compileWithCache(
5668
return operations;
5769
}
5870

71+
async function readCacheEntry(cachePath: string, hash: string): Promise<Map<string, OperationModel> | null> {
72+
try {
73+
const raw = await readFile(resolve(cachePath), "utf8");
74+
const parsed = JSON.parse(raw) as CacheEntry;
75+
if (parsed.hash !== hash) {
76+
return null;
77+
}
78+
79+
return new Map(parsed.operations.map((op) => [op.operationId, op]));
80+
} catch {
81+
return null;
82+
}
83+
}
84+
85+
async function computeSpecInputHash(
86+
specPath: string,
87+
serverUrl: string | undefined,
88+
options: CompileOptions
89+
): Promise<string> {
90+
const rawSpec = await readFile(resolve(specPath), "utf8");
91+
return computeHash({
92+
cacheFormatVersion: CACHE_FORMAT_VERSION,
93+
rawSpec,
94+
serverUrl,
95+
options
96+
});
97+
}
98+
5999
function computeHash(value: unknown): string {
60100
return createHash("sha256").update(JSON.stringify(value)).digest("hex");
61101
}

‎src/lint.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export function lintOpenApiDocument(doc: Record<string, unknown>, options: Compi
5555
});
5656
}
5757

58+
collectBrokenInternalRefDiagnostics(doc, doc, diagnostics);
59+
5860
const seenOperationIds = new Set<string>();
5961
const paths = isObject(doc.paths) ? (doc.paths as Record<string, unknown>) : {};
6062
for (const [path, rawPathItem] of Object.entries(paths)) {
@@ -139,6 +141,70 @@ export function lintOpenApiDocument(doc: Record<string, unknown>, options: Compi
139141
return diagnostics;
140142
}
141143

144+
function collectBrokenInternalRefDiagnostics(
145+
value: unknown,
146+
root: Record<string, unknown>,
147+
diagnostics: LintDiagnostic[],
148+
location = "$"
149+
): void {
150+
if (Array.isArray(value)) {
151+
for (let index = 0; index < value.length; index += 1) {
152+
collectBrokenInternalRefDiagnostics(value[index], root, diagnostics, `${location}[${index}]`);
153+
}
154+
return;
155+
}
156+
157+
if (!isObject(value)) {
158+
return;
159+
}
160+
161+
const record = value as Record<string, unknown>;
162+
const ref = typeof record["$ref"] === "string" ? record["$ref"] : undefined;
163+
if (ref?.startsWith("#/") && resolveJsonPointer(root, ref) === undefined) {
164+
diagnostics.push({
165+
level: "warning",
166+
code: "BROKEN_INTERNAL_REF",
167+
message: `Broken internal $ref: ${ref}`,
168+
location: `${location}.$ref`
169+
});
170+
}
171+
172+
for (const [key, child] of Object.entries(record)) {
173+
collectBrokenInternalRefDiagnostics(child, root, diagnostics, `${location}.${key}`);
174+
}
175+
}
176+
177+
function resolveJsonPointer(root: unknown, ref: string): unknown {
178+
if (ref === "#") {
179+
return root;
180+
}
181+
182+
const tokens = ref
183+
.slice(2)
184+
.split("/")
185+
.map((token) => token.replace(/~1/g, "/").replace(/~0/g, "~"));
186+
187+
let current: unknown = root;
188+
for (const token of tokens) {
189+
if (Array.isArray(current)) {
190+
const index = Number.parseInt(token, 10);
191+
if (!Number.isFinite(index)) {
192+
return undefined;
193+
}
194+
current = current[index];
195+
continue;
196+
}
197+
198+
if (!isObject(current)) {
199+
return undefined;
200+
}
201+
202+
current = (current as Record<string, unknown>)[token];
203+
}
204+
205+
return current;
206+
}
207+
142208
function isObject(value: unknown): value is object {
143209
return value !== null && typeof value === "object";
144210
}

‎src/openapi.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export async function loadOpenApiDocument(specPath: string): Promise<Record<stri
1818
validateShape(dereferenced as Record<string, unknown>, absPath);
1919
return dereferenced as Record<string, unknown>;
2020
} catch (error) {
21-
if (!isMissingPointerError(error)) {
21+
if (!isInternalMissingPointerError(error)) {
2222
throw error;
2323
}
2424

@@ -184,13 +184,18 @@ function mergeObjects(base: Record<string, unknown>, overlay: Record<string, unk
184184
return merged;
185185
}
186186

187-
function isMissingPointerError(error: unknown): boolean {
187+
function isInternalMissingPointerError(error: unknown): boolean {
188188
if (!error || typeof error !== "object") {
189189
return false;
190190
}
191191

192-
const candidate = error as { code?: unknown; name?: unknown };
193-
return candidate.code === "EMISSINGPOINTER" || candidate.name === "MissingPointerError";
192+
const candidate = error as { code?: unknown; name?: unknown; targetRef?: unknown };
193+
const isMissingPointer = candidate.code === "EMISSINGPOINTER" || candidate.name === "MissingPointerError";
194+
if (!isMissingPointer) {
195+
return false;
196+
}
197+
198+
return typeof candidate.targetRef === "string" && candidate.targetRef.startsWith("#");
194199
}
195200

196201
function isRecord(value: unknown): value is Record<string, unknown> {

‎src/server.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { paginateWithCursor } from "./pagination.js";
2525
import { observeLatency, observeStatus, renderPrometheus, metrics } from "./metrics.js";
2626
import type { CompileOptions, OperationModel, RuntimeOptions } from "./types.js";
2727
import { zodFromJsonSchema } from "./zod-schema.js";
28-
import { compileWithCache } from "./compile-cache.js";
28+
import { compileDocumentWithCache } from "./compile-cache.js";
2929
import { lintOpenApiDocument } from "./lint.js";
3030
import { loadOpenApiDocument } from "./openapi.js";
3131

@@ -487,7 +487,7 @@ async function loadRuntimeState(specPath: string, serverUrl: string | undefined,
487487
process.stderr.write(`${warnings.map((d) => `Warning [${d.code}] ${d.message}${d.location ? ` (${d.location})` : ""}`).join("\n")}\n`);
488488
}
489489

490-
const operations = await compileWithCache(specPath, serverUrl, cachePath, compile);
490+
const operations = await compileDocumentWithCache(doc, specPath, serverUrl, cachePath, compile);
491491
const validators = buildValidators(operations);
492492
return { operations, validators };
493493
}

‎test/compile-cache.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import test from "node:test";
2+
import assert from "node:assert/strict";
3+
import { mkdtemp, rm, writeFile } from "node:fs/promises";
4+
import { tmpdir } from "node:os";
5+
import { join } from "node:path";
6+
import { compileDocumentWithCache } from "../src/compile-cache.js";
7+
import { loadOpenApiDocument } from "../src/openapi.js";
8+
9+
test("compileDocumentWithCache keys cache off spec contents, not mutated in-memory docs", async (t) => {
10+
const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-cache-"));
11+
t.after(async () => {
12+
await rm(dir, { recursive: true, force: true });
13+
});
14+
15+
const specPath = join(dir, "spec.json");
16+
const cachePath = join(dir, "compile-cache.json");
17+
const spec = {
18+
openapi: "3.0.3",
19+
info: { title: "Cache test", version: "1.0.0" },
20+
servers: [{ url: "https://api.example.com" }],
21+
paths: {
22+
"/widgets": {
23+
get: {
24+
operationId: "listWidgets",
25+
responses: {
26+
"200": { description: "ok" }
27+
}
28+
}
29+
}
30+
}
31+
};
32+
33+
await writeFile(specPath, JSON.stringify(spec), "utf8");
34+
35+
const doc = await loadOpenApiDocument(specPath);
36+
const first = await compileDocumentWithCache(doc, specPath, undefined, cachePath);
37+
assert.deepEqual([...first.keys()], ["listWidgets"]);
38+
39+
const mutatedDoc = structuredClone(doc);
40+
delete ((mutatedDoc.paths as Record<string, unknown>)["/widgets"] as Record<string, unknown>).get;
41+
42+
const second = await compileDocumentWithCache(mutatedDoc, specPath, undefined, cachePath);
43+
assert.deepEqual([...second.keys()], ["listWidgets"]);
44+
});
45+
46+
test("compileDocumentWithCache invalidates when spec contents change", async (t) => {
47+
const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-cache-"));
48+
t.after(async () => {
49+
await rm(dir, { recursive: true, force: true });
50+
});
51+
52+
const specPath = join(dir, "spec.json");
53+
const cachePath = join(dir, "compile-cache.json");
54+
const initialSpec = {
55+
openapi: "3.0.3",
56+
info: { title: "Cache test", version: "1.0.0" },
57+
servers: [{ url: "https://api.example.com" }],
58+
paths: {
59+
"/widgets": {
60+
get: {
61+
operationId: "listWidgets",
62+
responses: {
63+
"200": { description: "ok" }
64+
}
65+
}
66+
}
67+
}
68+
};
69+
70+
await writeFile(specPath, JSON.stringify(initialSpec), "utf8");
71+
72+
const firstDoc = await loadOpenApiDocument(specPath);
73+
const first = await compileDocumentWithCache(firstDoc, specPath, undefined, cachePath);
74+
assert.deepEqual([...first.keys()], ["listWidgets"]);
75+
76+
const updatedSpec = {
77+
...initialSpec,
78+
paths: {
79+
"/gadgets": {
80+
post: {
81+
operationId: "createGadget",
82+
responses: {
83+
"201": { description: "created" }
84+
}
85+
}
86+
}
87+
}
88+
};
89+
await writeFile(specPath, JSON.stringify(updatedSpec), "utf8");
90+
91+
const secondDoc = await loadOpenApiDocument(specPath);
92+
const second = await compileDocumentWithCache(secondDoc, specPath, undefined, cachePath);
93+
assert.deepEqual([...second.keys()], ["createGadget"]);
94+
});

‎test/lint.test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,49 @@ test("lintOpenApiDocument warns on unsupported request media type", () => {
4747
const diagnostics = lintOpenApiDocument(doc, { strict: false });
4848
assert.ok(diagnostics.some((d) => d.code === "REQUEST_MEDIA_TYPE_UNSUPPORTED"));
4949
});
50+
51+
test("lintOpenApiDocument warns on broken internal refs", () => {
52+
const doc = {
53+
openapi: "3.0.3",
54+
servers: [{ url: "https://api.example.com" }],
55+
paths: {
56+
"/widgets": {
57+
get: {
58+
operationId: "listWidgets",
59+
responses: {
60+
"200": {
61+
description: "ok",
62+
content: {
63+
"application/json": {
64+
schema: { $ref: "#/components/schemas/WidgetList" }
65+
}
66+
}
67+
}
68+
}
69+
}
70+
}
71+
},
72+
components: {
73+
schemas: {
74+
WidgetList: {
75+
type: "object",
76+
properties: {
77+
items: {
78+
type: "array",
79+
items: { $ref: "#/components/schemas/MissingWidget" }
80+
}
81+
}
82+
}
83+
}
84+
}
85+
} as Record<string, unknown>;
86+
87+
const diagnostics = lintOpenApiDocument(doc, { strict: false });
88+
assert.ok(
89+
diagnostics.some(
90+
(d) =>
91+
d.code === "BROKEN_INTERNAL_REF" &&
92+
d.location === "$.components.schemas.WidgetList.properties.items.items.$ref"
93+
)
94+
);
95+
});

‎test/openapi.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,41 @@ test("loadOpenApiDocument tolerates missing internal refs while preserving valid
106106
);
107107
assert.ok(op.successResponseSchema);
108108
});
109+
110+
test("loadOpenApiDocument still fails on missing external refs", async (t) => {
111+
const dir = await mkdtemp(join(tmpdir(), "mcp-openapi-"));
112+
t.after(async () => {
113+
await rm(dir, { recursive: true, force: true });
114+
});
115+
116+
const specPath = join(dir, "missing-external-ref.json");
117+
const spec = {
118+
openapi: "3.0.3",
119+
info: {
120+
title: "Missing external ref",
121+
version: "1.0.0"
122+
},
123+
servers: [{ url: "https://api.example.com" }],
124+
paths: {
125+
"/widgets": {
126+
get: {
127+
operationId: "listWidgets",
128+
responses: {
129+
"200": {
130+
description: "ok",
131+
content: {
132+
"application/json": {
133+
schema: { $ref: "./schemas.json#/WidgetList" }
134+
}
135+
}
136+
}
137+
}
138+
}
139+
}
140+
}
141+
};
142+
143+
await writeFile(specPath, JSON.stringify(spec), "utf8");
144+
145+
await assert.rejects(() => loadOpenApiDocument(specPath));
146+
});

0 commit comments

Comments
 (0)