Skip to content

Commit 1ee7d23

Browse files
AceRothstein71AceRothstein71
committed
fix(opencode): close audit-v11 wave H-B instance-store + mcp catalog findings
1 parent 77589d5 commit 1ee7d23

4 files changed

Lines changed: 367 additions & 11 deletions

File tree

‎packages/opencode/src/mcp/catalog.ts‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,35 @@ const SCHEMA_VALUE_KEYWORDS = new Set([
213213
"if",
214214
"then",
215215
"else",
216+
"contentSchema",
216217
])
217218
// Keywords whose value is an array of schemas.
218219
const SCHEMA_ARRAY_KEYWORDS = new Set(["anyOf", "oneOf", "allOf"])
220+
// Numeric keywords that must be non-negative integers, and those that must be finite numbers.
221+
// A wrongly-typed bound makes the whole document meta-invalid and ajv refuses to compile it.
222+
const NON_NEGATIVE_INTEGER_KEYWORDS = new Set([
223+
"minLength",
224+
"maxLength",
225+
"minItems",
226+
"maxItems",
227+
"minProperties",
228+
"maxProperties",
229+
"minContains",
230+
"maxContains",
231+
])
232+
const NUMBER_KEYWORDS = new Set(["minimum", "maximum", "exclusiveMinimum", "exclusiveMaximum", "multipleOf"])
233+
// String- and boolean-valued keywords; a wrong type is meta-invalid.
234+
const STRING_KEYWORDS = new Set([
235+
"pattern",
236+
"format",
237+
"contentEncoding",
238+
"contentMediaType",
239+
"$comment",
240+
"$id",
241+
"$anchor",
242+
"$dynamicAnchor",
243+
])
244+
const BOOLEAN_KEYWORDS = new Set(["uniqueItems", "deprecated", "readOnly", "writeOnly"])
219245

220246
const isPlainObject = (value: unknown): value is Record<string, unknown> =>
221247
typeof value === "object" && value !== null && !Array.isArray(value)
@@ -324,6 +350,56 @@ function boundSchema(
324350
out[key] = bounded
325351
continue
326352
}
353+
if (key === "$dynamicRef") {
354+
// ajv only resolves hash-fragment dynamic refs; an absolute/flat form is rejected.
355+
if (typeof bounded === "string" && bounded.startsWith("#")) out[key] = bounded
356+
continue
357+
}
358+
if (SCHEMA_ARRAY_KEYWORDS.has(key)) {
359+
// Array members must themselves be schemas (object or boolean). `prefixItems` already
360+
// filters, but `anyOf`/`oneOf`/`allOf` emitted a non-schema member verbatim, which is
361+
// meta-invalid and makes ajv refuse the whole manifest (NEW-V11-03).
362+
if (!Array.isArray(bounded)) continue
363+
const members = bounded.filter(isSchema)
364+
if (members.length > 0) out[key] = members
365+
continue
366+
}
367+
if (key === "enum") {
368+
if (Array.isArray(bounded) && bounded.length > 0) out[key] = bounded
369+
continue
370+
}
371+
if (key === "examples") {
372+
if (Array.isArray(bounded)) out[key] = bounded
373+
continue
374+
}
375+
if (NON_NEGATIVE_INTEGER_KEYWORDS.has(key)) {
376+
if (typeof bounded === "number" && Number.isInteger(bounded) && bounded >= 0) out[key] = bounded
377+
continue
378+
}
379+
if (NUMBER_KEYWORDS.has(key)) {
380+
if (typeof bounded !== "number" || !Number.isFinite(bounded)) continue
381+
if (key === "multipleOf" && bounded <= 0) continue
382+
out[key] = bounded
383+
continue
384+
}
385+
if (key === "pattern") {
386+
if (typeof bounded !== "string") continue
387+
try {
388+
new RegExp(bounded)
389+
} catch {
390+
continue
391+
}
392+
out[key] = bounded
393+
continue
394+
}
395+
if (STRING_KEYWORDS.has(key)) {
396+
if (typeof bounded === "string") out[key] = bounded
397+
continue
398+
}
399+
if (BOOLEAN_KEYWORDS.has(key)) {
400+
if (typeof bounded === "boolean") out[key] = bounded
401+
continue
402+
}
327403
if (NON_EMPTY_ARRAY_KEYWORDS.has(key) && Array.isArray(bounded) && bounded.length === 0) continue
328404
if (SCHEMA_MAP_KEYWORDS.has(key)) {
329405
if (!isPlainObject(bounded)) continue
@@ -407,7 +483,13 @@ function dropDanglingRefs(
407483
if (!isPlainObject(value)) return
408484
for (const [key, item] of Object.entries(value)) {
409485
if (kind === "schema" && key === "$ref") {
410-
if (typeof item !== "string" || !item.startsWith("#")) continue
486+
// Only local `#` pointers are resolvable in the emitted document. `urn:`/relative/
487+
// absolute refs make ajv (and strict providers) reject the whole manifest, so drop
488+
// them along with genuinely dangling local pointers (NEW-V11-04).
489+
if (typeof item !== "string" || !item.startsWith("#")) {
490+
delete value[key]
491+
continue
492+
}
411493
const def = /^#\/(\$defs|definitions)\/([^/]+)$/.exec(item)
412494
if (def) {
413495
if (!defs.get(def[1])?.has(def[2])) delete value[key]

‎packages/opencode/src/project/instance-store.ts‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
124124
yield* emitDisposed({ directory: ctx.directory, project: ctx.project.id })
125125
})
126126

127+
// Release a claimed entry: drop it from the cache (only if it is still the cached one, so a
128+
// reload that already installed a replacement is not clobbered) and resolve `closed` so a
129+
// `provide` waiting on the claim can reload. Must be uninterruptible: the whole point is that
130+
// this runs even when the teardown around it is interrupted or times out (REGRESSION-2).
131+
const releaseClaim = (directory: string, entry: Entry) =>
132+
Effect.uninterruptible(
133+
Effect.gen(function* () {
134+
if (cache.get(directory) === entry) cache.delete(directory)
135+
yield* Deferred.succeed(entry.closed, undefined)
136+
}),
137+
)
138+
127139
const disposeEntry = Effect.fnUntraced(function* (directory: string, entry: Entry, ctx: InstanceContext) {
128140
if (cache.get(directory) !== entry) return false
129141
// Claim the entry in one synchronous step, *before* draining. Setting `disposed` makes
@@ -144,15 +156,16 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
144156
// an explicit dispose that aborted here would silently strand the prompt (R-V10-01). A
145157
// stuck lease can never deadlock disposal (v8 NEW-05); the W5 handshake above keeps a
146158
// provide-after-dispose from ever using the disposed context.
147-
yield* awaitLease(entry)
148-
yield* disposeContext(ctx).pipe(
149-
Effect.ensuring(
150-
Effect.gen(function* () {
151-
if (cache.get(directory) === entry) cache.delete(directory)
152-
yield* Deferred.succeed(entry.closed, undefined)
153-
}),
154-
),
155-
)
159+
//
160+
// The `ensuring` wraps the *whole* post-claim region, not just `disposeContext`: an
161+
// interrupt or timeout inside `awaitLease` used to skip the finalizer entirely (the claim
162+
// is synchronous, the finalizer was attached after the drain), leaving `disposed=true`
163+
// with `closed` pending and the entry cached, so every later `provide` for this directory
164+
// awaited `closed` forever (REGRESSION-2). The finalizer must always run.
165+
yield* Effect.gen(function* () {
166+
yield* awaitLease(entry)
167+
yield* disposeContext(ctx)
168+
}).pipe(Effect.ensuring(releaseClaim(directory, entry)))
156169
return true
157170
})
158171

@@ -216,6 +229,13 @@ const layer: Layer.Layer<Service, never, Project.Service | InstanceBootstrap.Ser
216229
yield* awaitLease(reusable)
217230
yield* Effect.promise(() => runDisposers(directory))
218231
yield* emitDisposed({ directory, project: input.project?.id })
232+
} else if (previous) {
233+
// The previous entry is already claimed by a draining `disposeEntry`. Its late
234+
// teardown runs the directory-global disposers, so booting the replacement before
235+
// it finishes would tear the fresh entry's resources down (NEW-V11-07). `closed`
236+
// resolves after that teardown (or immediately if it was interrupted), so await it
237+
// to serialize reload against a claimed entry.
238+
yield* Deferred.await(previous.closed)
219239
}
220240
yield* completeLoad(directory, input, entry)
221241
}).pipe(Effect.forkIn(scope, { startImmediately: true }))

‎packages/opencode/test/mcp/catalog.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,113 @@ describe("McpCatalog.convertTool bounds untrusted server input", () => {
389389
expect(() => new Ajv2020({ strict: false }).compile(schema)).not.toThrow()
390390
})
391391

392+
test("drops non-schema members of anyOf/oneOf/allOf but preserves boolean schemas", () => {
393+
const schema = emitted({
394+
type: "object",
395+
properties: {
396+
k: {
397+
type: ["string", "notatype", null],
398+
anyOf: [{ type: "string" }, 1, null, "x", true],
399+
oneOf: ["x", { type: "number" }],
400+
allOf: [null],
401+
},
402+
},
403+
})
404+
const k = (schema.properties as Record<string, Record<string, unknown>>).k
405+
406+
expect(k.type).toEqual(["string"])
407+
expect(k.anyOf).toEqual([{ type: "string" }, true])
408+
expect(k.oneOf).toEqual([{ type: "number" }])
409+
expect(k.allOf).toBeUndefined()
410+
expect(() => new Ajv2020({ strict: false }).compile(schema)).not.toThrow()
411+
})
412+
413+
test("drops source-typed bounds, contentSchema, enum and ref forms that ajv refuses", () => {
414+
const schema = emitted({
415+
type: "object",
416+
properties: {
417+
k: {
418+
type: "string",
419+
minLength: "x",
420+
maxItems: -1,
421+
minProperties: 1.5,
422+
multipleOf: 0,
423+
maximum: "x",
424+
pattern: 5,
425+
contentEncoding: 5,
426+
format: 5,
427+
uniqueItems: "x",
428+
examples: 5,
429+
enum: 5,
430+
contentSchema: { required: { a: 1 } },
431+
$dynamicRef: "urn:example:x",
432+
$ref: "urn:example:x",
433+
},
434+
regex: { type: "string", pattern: "(" },
435+
valid: {
436+
type: "string",
437+
minLength: 2,
438+
maxItems: 3,
439+
pattern: "^a+$",
440+
contentEncoding: "base64",
441+
format: "email",
442+
uniqueItems: true,
443+
examples: [1],
444+
},
445+
},
446+
})
447+
const props = schema.properties as Record<string, Record<string, unknown>>
448+
449+
for (const keyword of [
450+
"minLength",
451+
"maxItems",
452+
"minProperties",
453+
"multipleOf",
454+
"maximum",
455+
"pattern",
456+
"contentEncoding",
457+
"format",
458+
"uniqueItems",
459+
"examples",
460+
"enum",
461+
"$dynamicRef",
462+
"$ref",
463+
])
464+
expect(props.k[keyword]).toBeUndefined()
465+
expect(props.k.contentSchema).toEqual({})
466+
expect(props.regex.pattern).toBeUndefined()
467+
expect(props.valid).toMatchObject({
468+
minLength: 2,
469+
maxItems: 3,
470+
pattern: "^a+$",
471+
contentEncoding: "base64",
472+
format: "email",
473+
uniqueItems: true,
474+
examples: [1],
475+
})
476+
expect(() => new Ajv2020({ strict: false }).compile(schema)).not.toThrow()
477+
})
478+
479+
test("drops refs that are not resolvable local pointers", () => {
480+
const schema = emitted({
481+
type: "object",
482+
properties: {
483+
urn: { $ref: "urn:example:x" },
484+
relative: { $ref: "other.json#/A" },
485+
absolute: { $ref: "https://example.com/x" },
486+
local: { $ref: "#/$defs/Kept" },
487+
},
488+
$defs: { Kept: { type: "string" } },
489+
})
490+
const props = schema.properties as Record<string, Record<string, unknown>>
491+
492+
expect(props.urn.$ref).toBeUndefined()
493+
expect(props.relative.$ref).toBeUndefined()
494+
expect(props.absolute.$ref).toBeUndefined()
495+
expect(props.local.$ref).toBe("#/$defs/Kept")
496+
expect(() => new Ajv2020({ strict: false }).compile(schema)).not.toThrow()
497+
})
498+
392499
test("accepts only refs that resolve from the top-level document", () => {
393500
const schema = emitted({
394501
type: "object",

0 commit comments

Comments
 (0)