fix: judge a search that names no path against the directory it would use
The guard's rule was that a call naming no path is not judged, because a tool that names no path cannot leave the facet. That is true of a tool with no path argument and false of three that have one with a default: `grep` and `glob` document theirs as "Defaults to the session workspace", and `recall` falls back to a directory its own marker search picks. 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` searches outside the facet and returns what it finds. An absent argument is not an absent path; it is the default one. The three tools are now judged against the directory they would have searched, through the same containment test that decides every other call, and the refusal is its own sentence rather than the "that path is outside" one: the path at issue is one the model never wrote, so the remedy is a second argument, not a grant. A tool with no path argument at all is still left alone. `bash` is still the documented way out, and the first deployment running this plugin confirmed it is a real one: an agent scoped to one module used a shell command to parse a config file outside its facet, having been told in the same prompt not to. That is what this plugin is for, and what it cannot do is stated in the README rather than implied.
This commit is contained in:
1 parent
b0902017fd
commit
a9a25490db
4 files changed
+137
-13
No files matched your search
@@ -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
|
So an out-of-scope file call is put to you, and saying yes records the grant for
|
||||||
the rest of the conversation.
|
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
|
### The `facet` tool
|
||||||
|
|
||||||
| action | effect |
|
| 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
|
- **`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
|
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
|
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** —
|
- **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
|
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.
|
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:
|
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
|
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
|
(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
|
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
|
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
|
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
|
narrowed past is dropped rather than honoured, that `reset` lands on the
|
||||||
declared scope rather than the session directory, and that every `facet` result
|
declared scope rather than the session directory, that a search naming no path
|
||||||
satisfies the tool's own declared output schema.
|
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
|
- The bundle's patch layers parse, merge and land correctly against the real
|
||||||
profile: `dsh --profile web --patch ./cordis.patch.yml --patch
|
profile: `dsh --profile web --patch ./cordis.patch.yml --patch
|
||||||
./presets/facet.patch.yml --dump-config` shows `facet-policy`, `preset-facet`,
|
./presets/facet.patch.yml --dump-config` shows `facet-policy`, `preset-facet`,
|
||||||
the `facet-guard` row and the compaction config, with `preset-standard`
|
the `facet-guard` row and the compaction config, with `preset-standard`
|
||||||
untouched. The optional `standard-compaction` overlay was checked the same way.
|
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
|
**Verified live.** The bundle is installed in a `web` profile and composing
|
||||||
harness, because that needs the bundle installed. The first thing to check after
|
sessions: the runtime binding is in the running session's context, the crosshair
|
||||||
installing is that `preset-facet` is not `broken` — a preset whose child row
|
panel writes the declaration, and the guard decides the calls it names. That is
|
||||||
fails to resolve still appears on the roster, with its diagnostic, and cannot
|
also where the `bash` boundary above was confirmed the hard way. The first thing
|
||||||
compose a session:
|
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
|
```sh
|
||||||
dsh --profile web --dump-config | grep -A 3 preset-facet
|
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
|
lib/guard.js preset row: rule, binding, tool, pre-execute guard
|
||||||
presets/facet.patch.yml the `facet` agent preset
|
presets/facet.patch.yml the `facet` agent preset
|
||||||
presets/standard-compaction.patch.yml optional: the same budget for `standard`
|
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
|
`client/client.js` is a browser half rather than a module the host imports, and
|
||||||
|
|||||||
+16
-1
@@ -37,6 +37,8 @@ import {
|
|||||||
FACET_TOOL,
|
FACET_TOOL,
|
||||||
facetContextText,
|
facetContextText,
|
||||||
facetRuleText,
|
facetRuleText,
|
||||||
|
IMPLICIT_SESSION_PATHS,
|
||||||
|
implicitSearchReason,
|
||||||
isWideningCall,
|
isWideningCall,
|
||||||
outsideReason,
|
outsideReason,
|
||||||
pathArgumentsFor,
|
pathArgumentsFor,
|
||||||
@@ -181,7 +183,20 @@ export function apply(ctx, config) {
|
|||||||
if (scope === undefined) return undefined
|
if (scope === undefined) return undefined
|
||||||
|
|
||||||
const paths = pathArgumentsFor(exec.name, exec.arguments)
|
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) {
|
for (const raw of paths) {
|
||||||
const verdict = await inside(scope, raw, exec.signal)
|
const verdict = await inside(scope, raw, exec.signal)
|
||||||
|
|||||||
+49
-1
@@ -54,6 +54,28 @@ export const PATH_ARGUMENTS = Object.freeze({
|
|||||||
recall: ['path', 'focus'],
|
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. */
|
/** The tool this plugin registers, and the action on it that widens the scope. */
|
||||||
export const FACET_TOOL = 'facet'
|
export const FACET_TOOL = 'facet'
|
||||||
export const WIDENING_ACTION = 'add'
|
export const WIDENING_ACTION = 'add'
|
||||||
@@ -66,7 +88,10 @@ export const WIDENING_ACTION = 'add'
|
|||||||
* Anything that is not a non-empty string is skipped.
|
* Anything that is not a non-empty string is skipped.
|
||||||
*
|
*
|
||||||
* An empty result means "no opinion" — either the tool names no path (so it
|
* 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 {string} toolName - the tool being called.
|
||||||
* @param {unknown} args - the raw model arguments.
|
* @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.`
|
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.
|
* Why widening the scope is being put to the user.
|
||||||
*
|
*
|
||||||
|
|||||||
+47
-1
@@ -170,13 +170,59 @@ test('a prefix is not a parent', async () => {
|
|||||||
assert.equal(decision.kind, 'ask')
|
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()
|
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() }), {
|
assert.deepEqual(await dispatch(captured, { name: 'glob', args: { pattern: '**/*.ts' }, agent: agent() }), {
|
||||||
kind: 'allow',
|
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 () => {
|
test('tools the guard cannot judge are left alone, deliberately', async () => {
|
||||||
const { captured } = harness()
|
const { captured } = harness()
|
||||||
// A shell command cannot be read reliably from argv, so the facet is enforced
|
// A shell command cannot be read reliably from argv, so the facet is enforced
|
||||||
|
|||||||
Reference in new issue
Block a user