diff --git a/README.md b/README.md index b5c076c..bf53df4 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,15 @@ wrong, and an agent that cannot reach a shared type does not stop needing it. So an out-of-scope file call is put to you, and saying yes records the grant for the rest of the conversation. +**A tool that names no path is judged on the path it would use.** `grep`, `glob` +and `recall` all default to a directory when the model omits one — the session +directory for the first two, a marker-chosen one for the third — and on a session +whose facet is narrower than its directory, that default reads outside the facet. +An absent argument is not an absent path, so those three are judged against the +default they would have searched, and the refusal says which argument to add. +`lib/scope.js` holds the list; a tool with no path argument at all is still left +alone, because it has no default that could leave. + ### The `facet` tool | action | effect | @@ -208,7 +217,10 @@ Honest boundaries, because each one is a decision rather than an oversight: - **`bash` is not guarded.** A shell command cannot be judged reliably from argv, so the facet is enforced on the file tools and `bash` remains a legitimate way out. The prompt tells the model this rather than pretending - otherwise. + otherwise. It is not a theoretical hole: the first deployment to run this had + an agent scoped to one module use a shell command to parse a config file + outside its facet, having been told in the same prompt not to. A prompt is not + a guard, which is what the rest of this plugin is for. - **Tools that name a path in a shape the table cannot read are not guessed at** — including MCP tools. See `PATH_ARGUMENTS` in `lib/scope.js`; it is a list of tools whose path argument is a plain top-level string. @@ -225,26 +237,29 @@ Honest boundaries, because each one is a decision rather than an oversight: Verified: -- 73 tests (`npm test`) over the scope algebra, the policy state, the guard, and +- 81 tests (`npm test`) over the scope algebra, the policy state, the guard, and the browser half — including that containment is decided on canonical targets (a symlink inside the facet pointing outside it is refused), that a prefix is not a parent (`/a/room` does not contain `/a/roommate`), that a malformed tool call never makes the guard throw, that a live `mode` edit decides the next call without the guard being remounted, that an agent narrowing the user has since narrowed past is dropped rather than honoured, that `reset` lands on the - declared scope rather than the session directory, and that every `facet` result - satisfies the tool's own declared output schema. + declared scope rather than the session directory, that a search naming no path + is refused when the directory it defaults to is outside the facet, and that + every `facet` result satisfies the tool's own declared output schema. - The bundle's patch layers parse, merge and land correctly against the real profile: `dsh --profile web --patch ./cordis.patch.yml --patch ./presets/facet.patch.yml --dump-config` shows `facet-policy`, `preset-facet`, the `facet-guard` row and the compaction config, with `preset-standard` untouched. The optional `standard-compaction` overlay was checked the same way. -**Not yet verified: a live session.** The plugin has not been loaded by a running -harness, because that needs the bundle installed. The first thing to check after -installing is that `preset-facet` is not `broken` — a preset whose child row -fails to resolve still appears on the roster, with its diagnostic, and cannot -compose a session: +**Verified live.** The bundle is installed in a `web` profile and composing +sessions: the runtime binding is in the running session's context, the crosshair +panel writes the declaration, and the guard decides the calls it names. That is +also where the `bash` boundary above was confirmed the hard way. The first thing +to check after any change is still that `preset-facet` is not `broken` — a preset +whose child row fails to resolve still appears on the roster, with its +diagnostic, and cannot compose a session: ```sh dsh --profile web --dump-config | grep -A 3 preset-facet @@ -309,7 +324,7 @@ lib/scope.js the pure half — argument extraction, text, reason lib/guard.js preset row: rule, binding, tool, pre-execute guard presets/facet.patch.yml the `facet` agent preset presets/standard-compaction.patch.yml optional: the same budget for `standard` -test/ 78 tests +test/ 81 tests ``` `client/client.js` is a browser half rather than a module the host imports, and diff --git a/lib/guard.js b/lib/guard.js index 341b8f8..e84e575 100644 --- a/lib/guard.js +++ b/lib/guard.js @@ -37,6 +37,8 @@ import { FACET_TOOL, facetContextText, facetRuleText, + IMPLICIT_SESSION_PATHS, + implicitSearchReason, isWideningCall, outsideReason, pathArgumentsFor, @@ -181,7 +183,20 @@ export function apply(ctx, config) { if (scope === undefined) return undefined const paths = pathArgumentsFor(exec.name, exec.arguments) - if (paths.length === 0) return undefined + if (paths.length === 0) { + // A tool that names no path is judged against the path it will actually + // use. For most of them that is nothing — a tool with no path argument + // cannot leave the facet. For the ones whose own default is a directory it + // is the session directory, which a narrowed facet does not contain, and + // "no opinion" there would hand the model a read the guard exists to + // refuse. The containment test is the same one; only the path is ours + // rather than the model's. + if (!IMPLICIT_SESSION_PATHS.includes(exec.name)) return undefined + const verdict = await inside(scope, scope.cwd, exec.signal) + if (verdict.inside) return undefined + const kind = mode === 'deny' ? 'deny' : 'ask' + return { kind, reason: implicitSearchReason({ cwd: scope.cwd, root: scope.root }) } + } for (const raw of paths) { const verdict = await inside(scope, raw, exec.signal) diff --git a/lib/scope.js b/lib/scope.js index 072f9f8..db22043 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -54,6 +54,28 @@ export const PATH_ARGUMENTS = Object.freeze({ recall: ['path', 'focus'], }) +/** + * Tools that will search the session directory when the model names no path. + * + * The guard's rule is that a call naming no path is not judged, because a tool + * that names no path cannot leave the facet. That holds for a tool with no path + * argument at all, and fails for these three: their *own* default is the session + * workspace. `grep` and `glob` document both their path arguments as "Defaults to + * the session workspace", and `recall` falls back to a directory its own marker + * search picks — an ancestor of the facet whenever the facet carries no + * `CONTROLLER.md` of its own. + * + * So on any session whose facet is narrower than its directory — every session + * whose focus was set from the panel rather than by starting the conversation in + * the folder — a bare `grep` reads outside the facet. An absent argument is not + * an absent path; it is the default one, and that is the path the guard judges. + * + * `recall` is another package's tool and is named here because the coupling to + * its two path arguments already exists in {@link PATH_ARGUMENTS}: leaving it + * out would keep a read path the table claims to cover. + */ +export const IMPLICIT_SESSION_PATHS = Object.freeze(['grep', 'glob', 'recall']) + /** The tool this plugin registers, and the action on it that widens the scope. */ export const FACET_TOOL = 'facet' export const WIDENING_ACTION = 'add' @@ -66,7 +88,10 @@ export const WIDENING_ACTION = 'add' * Anything that is not a non-empty string is skipped. * * An empty result means "no opinion" — either the tool names no path (so it - * cannot leave the facet) or it names one in a shape we refuse to guess at. + * cannot leave the facet) or it names one in a shape we refuse to guess at. The + * first half of that is not true of every tool: the ones whose own default is a + * directory are listed in {@link IMPLICIT_SESSION_PATHS}, and the guard judges + * them against the directory they will actually search. * * @param {string} toolName - the tool being called. * @param {unknown} args - the raw model arguments. @@ -236,6 +261,29 @@ export function outsideReason(input) { return `${what} is ${verb} — this session's facet is ${root}. Ask the user to grant it (\`facet action=add\`), or work within the facet.` } +/** + * Why a search that named no directory is being questioned. + * + * A separate sentence from {@link outsideReason} because the fault is different. + * There, the model named a path outside the facet and is told which one. Here it + * named nothing, so the path at issue is one it never wrote: the tool's own + * default. Saying so is the whole remedy — the fix is a second argument, not a + * grant — so the reason names the directory the search would have covered and + * the one it had to stay in. + * + * @param {object} input + * @param {string} input.cwd - the session working directory the tool defaults to. + * @param {string} input.root - the facet root. + * @returns {string} the reason. + */ +export function implicitSearchReason(input) { + const { cwd, root } = input + return ( + `This search names no \`path\`, so it would cover this session's directory ${cwd} — and this session's ` + + `facet is ${root}. Name a folder inside the facet, or ask the user to grant the one you need.` + ) +} + /** * Why widening the scope is being put to the user. * diff --git a/test/guard.test.js b/test/guard.test.js index 8729710..28fb36e 100644 --- a/test/guard.test.js +++ b/test/guard.test.js @@ -170,13 +170,59 @@ test('a prefix is not a parent', async () => { assert.equal(decision.kind, 'ask') }) -test('an omitted path means the session directory, which is in scope', async () => { +test('an omitted path means the session directory, which here is the facet', async () => { const { captured } = harness() + // Allowed because the default and the facet are the same folder — not because + // the call named nothing. The next test is the case where they differ. assert.deepEqual(await dispatch(captured, { name: 'glob', args: { pattern: '**/*.ts' }, agent: agent() }), { kind: 'allow', }) }) +test('a search that names no path is judged against the directory it would cover', async () => { + // The session directory is the facet's parent here: the user narrowed this + // conversation to `inner`, while the search tools still default to the whole + // session directory. An absent argument is not an absent path — it is the + // default one, and the default one leaves the facet. + const { captured } = harness({ config: { mode: 'ask', scopes: { 'session-1': { focus: 'inner' } } } }) + + const decision = await dispatch(captured, { name: 'grep', args: { pattern: 'motion_learning' }, agent: agent() }) + assert.equal(decision.kind, 'ask') + assert.match(decision.reason, /names no `path`/) + assert.match(decision.reason, /inner/, 'the reason names the boundary the call crossed') + + // Naming the folder is the whole remedy, and it costs no grant. + assert.deepEqual( + await dispatch(captured, { name: 'grep', args: { pattern: 'x', path: 'inner' }, agent: agent() }), + { kind: 'allow' }, + ) + // The same rule reaches the other two tools whose default is a directory. + assert.equal( + (await dispatch(captured, { name: 'glob', args: { pattern: '**/*.ts' }, agent: agent() })).kind, + 'ask', + ) + assert.equal( + (await dispatch(captured, { name: 'recall', args: { query: 'motion learning' }, agent: agent() })).kind, + 'ask', + ) + // A tool with no path argument at all is still left alone: it has no default + // that could leave the facet. + assert.deepEqual(await dispatch(captured, { name: 'todo_write', args: { todos: [] }, agent: agent() }), { + kind: 'allow', + }) +}) + +test('recall is judged on the scope it names, because that is what it will search', async () => { + const { captured } = harness({ config: { mode: 'deny', scopes: { 'session-1': { focus: 'inner' } } } }) + assert.deepEqual(await dispatch(captured, { name: 'recall', args: { query: 'x', focus: 'inner' }, agent: agent() }), { + kind: 'allow', + }) + assert.equal( + (await dispatch(captured, { name: 'recall', args: { query: 'x', path: '..' }, agent: agent() })).kind, + 'deny', + ) +}) + test('tools the guard cannot judge are left alone, deliberately', async () => { const { captured } = harness() // A shell command cannot be read reliably from argv, so the facet is enforced