fix(tool-cordis): gate façade services on inject, and make tools.get read-only
Two review findings (#220) on the sandbox context façade: - Undeclared services were reachable: the façade resolved any live global via ctx.get(name), so ctx.bash worked without inject: ['bash']. A cross-mount consumer could then depend on a provider cordis never saw — unmounting the provider would neither park the consumer nor unwind its registered tools, leaving a model-visible tool that fails only at execution. The façade now reads ctx.fiber.inject and refuses any service the mount did not declare (with a teaching error naming the inject fix), so the dependency is always visible to cordis and its activation/unload semantics bind. - ctx.tools.get returned the live ToolDefinition, including execute — mount code could call another tool directly and bypass ToolRegistry.execute and its pre/post-execute hooks and accounting. get now returns the same read-only name/description/parameters view as schemas(), never an invocable. Adds inject-gate and schema-view regression cases to sandbox-context.spec.ts (undeclared property/get denied, declared allowed, the cross-mount zombie-tool scenario refused at call time, get exposes no execute). Package stays at per-file 100% coverage. RFC, mount description, and tool-catalog updated.
This commit is contained in:
@@ -185,15 +185,19 @@ export function sandboxRegisterTool(ctx: Context, tool: unknown): () => void {
|
||||
const CTX_VERBS = new Set(['on', 'once', 'provide', 'timeout', 'interval', 'setTimeout', 'setInterval', 'throttle', 'debounce'])
|
||||
|
||||
/**
|
||||
* The tool-registry façade: only `register` (marker-guarded), plus the
|
||||
* read-only `schemas` / `get` a mount may legitimately want. No other registry
|
||||
* method (nothing that could re-enter the raw context) is exposed.
|
||||
* The tool-registry façade: `register` (marker-guarded) plus READ-ONLY
|
||||
* metadata (`schemas`, and `get` returning a schema view, never the live
|
||||
* `ToolDefinition`). Exposing the raw definition would hand mount code the
|
||||
* tool's `execute` function, letting it call another tool directly and bypass
|
||||
* `ToolRegistry.execute` — the pre/post-execute waterfall (permission gates,
|
||||
* accounting) and result normalization. So `get` returns the same
|
||||
* name/description/parameters view as `schemas()`, and nothing invocable.
|
||||
*/
|
||||
function sandboxTools(ctx: Context): Record<string, unknown> {
|
||||
return {
|
||||
register: (tool: unknown): (() => void) => sandboxRegisterTool(ctx, tool),
|
||||
schemas: () => ctx.tools.schemas(),
|
||||
get: (name: string) => ctx.tools.get(name),
|
||||
get: (name: string) => ctx.tools.schemas().find(schema => schema.name === name),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -233,53 +237,85 @@ function guardedService(service: object, name: string): unknown {
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* The service names a plugin declared in `inject`, as a lookup set. Whatever
|
||||
* declaration style the plugin used — an `inject: ['bash', 'tools']` array or
|
||||
* the `{ required, optional }` object form — cordis resolves it into a single
|
||||
* name-keyed map on the fiber before `apply` runs (`{ bash: null, tools: null }`),
|
||||
* so the gate just reads that map's keys. A mount may reach only the services
|
||||
* it declared — that is what lets cordis park the mount when a declared
|
||||
* provider unmounts.
|
||||
*/
|
||||
function declaredInjects(ctx: Context): Set<string> {
|
||||
return new Set(Object.keys(ctx.fiber.inject))
|
||||
}
|
||||
|
||||
/**
|
||||
* The sandbox context façade handed to a mounted plugin's `apply` in place of
|
||||
* the real `ctx`. A whitelist (see the module doc): the registration/eventing
|
||||
* verbs, the timer helpers, a guarded `tools`, and injected services resolved
|
||||
* through a guarded `get` / property access. Every framework-plumbing member
|
||||
* is denied with a teaching error; there is no context-valued member to reach.
|
||||
* through a guarded `get` / property access. A service is reachable only if the
|
||||
* plugin DECLARED it in `inject` — an undeclared service is denied even when a
|
||||
* global provider exists, so cordis's activation/unload semantics (park the
|
||||
* mount when a declared provider goes away) actually bind. Every
|
||||
* framework-plumbing member is denied with a teaching error; there is no
|
||||
* context-valued member to reach.
|
||||
*/
|
||||
function sandboxContext(ctx: Context): Context {
|
||||
const tools = sandboxTools(ctx)
|
||||
// Resolve a named service to a guarded wrapper, or undefined when absent.
|
||||
const resolveService = (name: string): unknown => {
|
||||
if (name === 'tools') return tools
|
||||
const service: unknown = ctx.get(name)
|
||||
return service === undefined ? undefined : guardedService(service as object, name)
|
||||
const declared = declaredInjects(ctx)
|
||||
// A framework member or an undeclared service — distinguish the two so the
|
||||
// error teaches the right fix (declare it in inject vs it is withheld).
|
||||
const denyRead = (prop: string): never => {
|
||||
if (ctx.get(prop) !== undefined) {
|
||||
throw new Error(
|
||||
`service "${prop}" is not injected. Declare it: inject: ['${prop}', …] on your plugin, `
|
||||
+ 'so cordis parks this mount if the provider is later unmounted.',
|
||||
)
|
||||
}
|
||||
throw new Error(
|
||||
`sandbox ctx does not expose "${prop}". Available: ctx.tools.register / ctx.on / ctx.provide / `
|
||||
+ 'the timer helpers (ctx.setTimeout, ctx.interval, …) and any service you declared in inject. '
|
||||
+ 'Framework internals (root, fiber, registry, extend, plugin, …) are withheld by design.',
|
||||
)
|
||||
}
|
||||
const get = (name: string): unknown => resolveService(name)
|
||||
// Read a service for either access path (property or `get`). `tools` is the
|
||||
// façade's own surface. An UNDECLARED name is denied with the teaching
|
||||
// error; a DECLARED one resolves to the guarded service. A declared inject
|
||||
// is required in cordis (the fiber only activates once every declared
|
||||
// service is live), so at `apply`/`execute` time `ctx.get(name)` is present
|
||||
// for a declared name — no undefined case to handle here.
|
||||
const readService = (name: string): unknown => {
|
||||
if (name === 'tools') return tools
|
||||
if (!declared.has(name)) return denyRead(name)
|
||||
return guardedService(ctx.get(name) as object, name)
|
||||
}
|
||||
const get = (name: string): unknown => readService(name)
|
||||
return new Proxy({}, {
|
||||
get(_target, prop) {
|
||||
if (prop === 'tools') return tools
|
||||
if (prop === 'get') return get
|
||||
if (typeof prop !== 'string') return undefined
|
||||
// Lazy verb forwarder — reads `ctx[verb]` only when called, so a plugin
|
||||
// that never uses a timer never triggers the timer mixin's inject check.
|
||||
// that never uses a timer never triggers the timer mixin's inject check
|
||||
// (cordis raises its own "without inject" error there for undeclared timer use).
|
||||
if (CTX_VERBS.has(prop)) {
|
||||
return (...args: unknown[]): unknown => {
|
||||
const method = ctx[prop as keyof Context]
|
||||
return Reflect.apply(method as (...a: unknown[]) => unknown, ctx, args)
|
||||
}
|
||||
}
|
||||
// A declared-and-injected service reads as a ctx property; resolve it
|
||||
// through the same guard. Absent → the deny path (framework plumbing,
|
||||
// an un-injected service, or a typo) with one teaching error.
|
||||
const service = resolveService(prop)
|
||||
if (service !== undefined) return service
|
||||
throw new Error(
|
||||
`sandbox ctx does not expose "${prop}". Available: ctx.tools.register / ctx.on / ctx.provide / `
|
||||
+ 'the timer helpers (ctx.setTimeout, ctx.interval, …) and any service you declared in inject. '
|
||||
+ 'Framework internals (root, fiber, registry, extend, plugin, …) are withheld by design.',
|
||||
)
|
||||
return readService(prop)
|
||||
},
|
||||
// A façade is not the real ctx; block writes rather than let mount code
|
||||
// stash state on a throwaway object and think it persisted.
|
||||
set(_target, prop) {
|
||||
throw new Error(`sandbox ctx is read-only; cannot assign "${String(prop)}"`)
|
||||
},
|
||||
// `in` reflects reachability: the façade surface plus DECLARED services
|
||||
// (whether or not currently live). Does not resolve/wrap — no throw.
|
||||
has: (_target, prop) => prop === 'tools' || prop === 'get'
|
||||
|| (typeof prop === 'string' && (CTX_VERBS.has(prop) || resolveService(prop) !== undefined)),
|
||||
|| (typeof prop === 'string' && (CTX_VERBS.has(prop) || declared.has(prop))),
|
||||
}) as unknown as Context
|
||||
}
|
||||
|
||||
|
||||
@@ -125,12 +125,14 @@ export function apply(ctx: Context, config: Config): void {
|
||||
'Mount a NEW cordis plugin into the live runtime that is running THIS agent '
|
||||
+ '(self-modification). `code` runs as the body of an async JavaScript function '
|
||||
+ 'in an isolated sandbox and MUST `return` a plugin. Two forms: '
|
||||
+ 'FUNCTION form `return (ctx) => { … }` — cannot declare inject, uses whatever '
|
||||
+ 'services are on the parent context, and accessing a service without inject '
|
||||
+ '(e.g. ctx.bash) throws; use it only when you need no injected services. '
|
||||
+ 'FUNCTION form `return (ctx) => { … }` — declares no inject, so it can register '
|
||||
+ 'tools, listen to events, and provide services, but reaching ANY service (e.g. '
|
||||
+ 'ctx.bash) throws; use it only when you need no services. '
|
||||
+ 'OBJECT form `return { name?, inject: [\'bash\', \'llm\', …], apply(ctx) { … } }` '
|
||||
+ '— declares dependencies, and cordis activates the plugin only after the '
|
||||
+ 'services exist; PREFER this form for any plugin that needs bash, llm, sessions, etc. '
|
||||
+ 'services exist; PREFER this form. You may reach ONLY the services you list in '
|
||||
+ 'inject: an undeclared service throws even if it exists, because an undeclared '
|
||||
+ 'dependency would not be cleaned up if its provider is unmounted. '
|
||||
+ 'BEFORE calling a service from your code, read cordis_inspect what:"api" — it lists '
|
||||
+ 'method signatures AND the type shapes of their arguments/returns (do not guess a '
|
||||
+ 'field\'s type; e.g. a bash run\'s stdout is an object, not a string). '
|
||||
|
||||
@@ -220,8 +220,6 @@ describe('cordis_mount', () => {
|
||||
return {
|
||||
name: 'raw-register-get',
|
||||
apply(ctx) {
|
||||
const sp = ctx.get('systemPrompt')
|
||||
console.log('systemPrompt is', typeof sp)
|
||||
ctx.get('tools').register({ name: 'raw_via_get', description: 'raw', parameters: {}, async execute() { return [] } })
|
||||
},
|
||||
}
|
||||
|
||||
@@ -154,3 +154,142 @@ describe('sandbox context façade — escape surface is closed', () => {
|
||||
expect(result.isError).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('sandbox context façade — inject gate on services', () => {
|
||||
it('denies an undeclared live service (property access), naming the inject fix', async () => {
|
||||
// `systemPrompt` is a live global service in the setup harness, but this
|
||||
// mount does not declare it — reaching it would let the mount depend on a
|
||||
// provider cordis does not know about, so it is refused.
|
||||
const ctx = await setup()
|
||||
const result = await call(ctx, 'cordis_mount', {
|
||||
code: 'return { name: \'undeclared\', inject: [\'tools\'], apply(ctx) { const s = ctx.systemPrompt } }',
|
||||
})
|
||||
expect(result.isError).toBe(true)
|
||||
expect(text(result)).toContain('service "systemPrompt" is not injected')
|
||||
expect(text(result)).toContain('inject: [\'systemPrompt\', …]')
|
||||
})
|
||||
|
||||
it('denies an undeclared live service reached through ctx.get too', async () => {
|
||||
const ctx = await setup()
|
||||
const result = await call(ctx, 'cordis_mount', {
|
||||
code: 'return { name: \'undeclared-get\', inject: [\'tools\'], apply(ctx) { ctx.get(\'systemPrompt\') } }',
|
||||
})
|
||||
expect(result.isError).toBe(true)
|
||||
expect(text(result)).toContain('service "systemPrompt" is not injected')
|
||||
})
|
||||
|
||||
it('allows a service the mount DID declare in inject', async () => {
|
||||
const ctx = await setup()
|
||||
const result = await call(ctx, 'cordis_mount', {
|
||||
code: `
|
||||
return {
|
||||
name: 'declared',
|
||||
inject: ['systemPrompt', 'tools'],
|
||||
apply(ctx) { console.log('has systemPrompt:', typeof ctx.systemPrompt) }
|
||||
}
|
||||
`,
|
||||
})
|
||||
expect(result.isError).toBe(false)
|
||||
expect(text(result)).toContain('state: active')
|
||||
})
|
||||
|
||||
it('a cross-mount consumer must declare the provider — the undeclared path is refused, not left as a zombie tool', async () => {
|
||||
// The finding's scenario: a consumer registers a tool built on a provider's
|
||||
// service WITHOUT declaring inject. cordis would then never park the
|
||||
// consumer when the provider unmounts, leaving a tool that fails only at
|
||||
// execution. The gate refuses the undeclared access up front, so the
|
||||
// dependency is always visible to cordis.
|
||||
const ctx = await setup()
|
||||
await call(ctx, 'cordis_mount', {
|
||||
code: 'return { name: \'greeter-provider\', apply(ctx) { ctx.provide(\'greeter\', { greet: (n) => \'hi \' + n }) } }',
|
||||
})
|
||||
const undeclared = await call(ctx, 'cordis_mount', {
|
||||
code: `
|
||||
return {
|
||||
name: 'sloppy-consumer',
|
||||
inject: ['tools'],
|
||||
apply(ctx) {
|
||||
harness.registerTool(ctx, harness.defineTool({
|
||||
name: 'greet_undeclared',
|
||||
description: 'uses greeter without declaring it',
|
||||
parameters: { n: { type: 'string', required: true } },
|
||||
async execute(args) { return [{ type: 'text', text: ctx.greeter.greet(args.n) }] },
|
||||
}))
|
||||
},
|
||||
}
|
||||
`,
|
||||
})
|
||||
// The tool registers (its execute is lazy), but calling it hits the gate:
|
||||
// `ctx.greeter` is undeclared, so it fails with the teaching error rather
|
||||
// than silently working and later stranding.
|
||||
expect(undeclared.isError).toBe(false)
|
||||
const called = await call(ctx, 'greet_undeclared', { n: 'x' })
|
||||
expect(called.isError).toBe(true)
|
||||
expect(text(called)).toContain('service "greeter" is not injected')
|
||||
})
|
||||
})
|
||||
|
||||
describe('sandbox tools façade — get is a read-only schema view', () => {
|
||||
it('ctx.tools.get returns a schema, not the live ToolDefinition with execute', async () => {
|
||||
// The finding: returning the raw ToolDefinition hands mount code the
|
||||
// tool's execute function, letting it bypass ToolRegistry.execute (and its
|
||||
// pre/post hooks). get now returns the same name/description/parameters
|
||||
// view as schemas(), with no execute. Asserted via a self-made tool that
|
||||
// reports the shape it saw — world-checked, not self-reported.
|
||||
const ctx = await setup()
|
||||
await call(ctx, 'cordis_mount', {
|
||||
code: `
|
||||
return {
|
||||
name: 'reporter',
|
||||
inject: ['tools'],
|
||||
apply(ctx) {
|
||||
harness.registerTool(ctx, harness.defineTool({
|
||||
name: 'report_view',
|
||||
description: 'reports the shape of a tool view',
|
||||
parameters: {},
|
||||
async execute() {
|
||||
const view = ctx.tools.get('cordis_mount')
|
||||
return [{ type: 'text', text: JSON.stringify({
|
||||
hasExecute: 'execute' in view,
|
||||
hasPresentCall: 'presentCall' in view,
|
||||
name: view.name,
|
||||
keys: Object.keys(view).sort(),
|
||||
}) }]
|
||||
},
|
||||
}))
|
||||
},
|
||||
}
|
||||
`,
|
||||
})
|
||||
const reported = await call(ctx, 'report_view', {})
|
||||
expect(reported.isError).toBe(false)
|
||||
const shape = JSON.parse(text(reported)) as { hasExecute: boolean; hasPresentCall: boolean; name: string; keys: string[] }
|
||||
expect(shape.hasExecute).toBe(false)
|
||||
expect(shape.hasPresentCall).toBe(false)
|
||||
expect(shape.name).toBe('cordis_mount')
|
||||
expect(shape.keys).toEqual(['description', 'name', 'parameters'])
|
||||
})
|
||||
|
||||
it('ctx.tools.get returns undefined for an unknown tool', async () => {
|
||||
const ctx = await setup()
|
||||
await call(ctx, 'cordis_mount', {
|
||||
code: `
|
||||
return {
|
||||
name: 'unknown-probe',
|
||||
inject: ['tools'],
|
||||
apply(ctx) {
|
||||
harness.registerTool(ctx, harness.defineTool({
|
||||
name: 'probe_unknown',
|
||||
description: 'reports whether an unknown tool resolves',
|
||||
parameters: {},
|
||||
async execute() {
|
||||
return [{ type: 'text', text: String(ctx.tools.get('no_such_tool') === undefined) }]
|
||||
},
|
||||
}))
|
||||
},
|
||||
}
|
||||
`,
|
||||
})
|
||||
expect(text(await call(ctx, 'probe_unknown', {}))).toBe('true')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user