Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions docs/design/daemon-git-worktree-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Daemon Git worktree guard

## Context

A daemon ACP session is owned by one bound workspace. The model shell tool
already rejects an explicit `directory` outside its effective workspace, but a
Git command can relocate itself with `-C`, `--work-tree`, or `--git-dir` while
the shell process still starts inside the workspace. This can let a daemon
agent mutate another checkout or worktree after the direct directory form was
rejected.

## Scope

The guard applies only to model tool execution through the managed daemon ACP
path. It does not change CLI or TUI shell validation, Git safety classification,
permission rules, confirmation behavior, or direct user shell execution.

The daemon enables its managed tool guard for every ACP child. The host owns the
bound workspace and adds it to the validated guard request before applying the
built-in policy. An optional external tool guard remains an additional policy
and receives the same request only after the built-in policy allows it.

## Policy

The built-in guard inspects `run_shell_command` calls only. It recognizes Git
invocations whose repository location is changed by literal forms of:

- `git -C <path>` and `git -C<path>`
- `git --work-tree <path>` and `git --work-tree=<path>`
- `git --git-dir <path>` and `git --git-dir=<path>`

Relative targets resolve from the command's effective starting directory:
`arguments.directory` when present, otherwise the session's current effective
working directory. The bridge supplies both that current directory and the
immutable bound workspace from trusted session state. The current effective
working directory is the allowed execution boundary so a session moved through
the controlled daemon `/cd` flow can operate in its selected worktree without
being mistaken for an escape from the original storage owner.

A statically resolved Git relocation is denied when both of the following
hold:

1. its target is outside the session's effective working directory after
canonical path resolution;
2. its Git subcommand is mutating or cannot be classified as read-only.

Read-only relocated Git commands remain allowed. Commands with no recognized
Git relocation retain existing behavior. Dynamic relocation targets are denied
for mutating or unknown subcommands because the daemon cannot prove that the
target remains inside the effective working directory.

`--git-dir` is evaluated by its repository directory. A target ending in
`.git` uses its parent as the repository target; linked-worktree administrative
paths are still outside the bound workspace and are denied for mutations.

## Failure semantics

Malformed managed guard requests, stale session or prompt ownership, missing
trusted workspace context, policy exceptions, and malformed external-provider
responses fail closed before execution. A built-in denial is final and is not
sent to the optional provider.

## Non-goals

- No changes to core `ShellTool`, `ShellToolInvocation`, shell AST parsing,
`PermissionManager`, `evaluatePermissionFlow`, or `CoreToolScheduler`.
- No new confirmation flow or linked-worktree exception.
- No restriction on direct user-entered daemon shell commands.
- No general shell interpreter or environment-variable analysis.
- No attempt to correlate a denial with a previous tool call.
6 changes: 6 additions & 0 deletions packages/acp-bridge/src/bridgeClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,14 @@ describe('BridgeClient — managed external tool guard', () => {
});
const entry: {
sessionId: string;
workspaceCwd: string;
effectiveCwd: string;
promptActive: boolean;
activePromptId?: string;
} = {
sessionId: 'session-1',
workspaceCwd: '/workspace',
effectiveCwd: '/workspace/worktree',
promptActive: true,
activePromptId: 'prompt-1',
};
Expand All @@ -275,6 +279,8 @@ describe('BridgeClient — managed external tool guard', () => {
toolCallId: 'call-1',
toolName: 'write_file',
arguments: { path: 'README.md' },
workspaceCwd: '/workspace',
effectiveCwd: '/workspace/worktree',
});
});

Expand Down
4 changes: 4 additions & 0 deletions packages/acp-bridge/src/bridgeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,8 @@ function sliceLineRange(
*/
export interface BridgeClientSessionEntry {
sessionId: string;
workspaceCwd: string;
effectiveCwd: string;
events: EventBus;
artifacts: SessionArtifactStore;
recordingDegraded: boolean;
Expand Down Expand Up @@ -1245,6 +1247,8 @@ export class BridgeClient implements Client {
toolCallId,
toolName,
arguments: args,
workspaceCwd: entry.workspaceCwd,
effectiveCwd: entry.effectiveCwd,
});
const currentEntry = this.resolveEntry(sessionId);
if (
Expand Down
4 changes: 4 additions & 0 deletions packages/acp-bridge/src/bridgeOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ export interface ExternalToolGuardPrepareRequest {
readonly toolCallId: string;
readonly toolName: string;
readonly arguments: Readonly<Record<string, unknown>>;
/** Daemon-owned workspace identity. Never accepted from the ACP child. */
readonly workspaceCwd?: string;
/** Daemon-owned current session working directory. */
readonly effectiveCwd?: string;
}

export type ExternalToolGuardPrepareResult =
Expand Down
52 changes: 20 additions & 32 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18286,39 +18286,27 @@ describe('createManagedExternalToolGuard', () => {
expect(extMethod).not.toHaveBeenCalled();
});

it.each([
ToolNames.AGENT,
ToolNames.WORKFLOW,
ToolNames.CREATE_SUB_SESSION,
ToolNames.SEND_MESSAGE,
])(
'rejects unsupported nested executor %s without contacting the provider',
async (toolName) => {
const extMethod = vi.fn();
const guard = createManagedExternalToolGuard({
extMethod,
} as unknown as AgentSideConnection);
it('forwards nested executors to the daemon host guard', async () => {
const extMethod = vi.fn().mockResolvedValue({ allowed: true });
const guard = createManagedExternalToolGuard({
extMethod,
} as unknown as AgentSideConnection);

await expect(
guard({
callId: 'call-1',
toolName,
args: {},
signal: new AbortController().signal,
invocationContext: {
version: 1,
sessionId: 'session-1',
promptId: 'prompt-1',
},
}),
).resolves.toEqual({
allowed: false,
reason:
'Managed external tool guard v1 does not support nested or delegated agent execution.',
});
expect(extMethod).not.toHaveBeenCalled();
},
);
await expect(
guard({
callId: 'call-1',
toolName: ToolNames.AGENT,
args: {},
signal: new AbortController().signal,
invocationContext: {
version: 1,
sessionId: 'session-1',
promptId: 'prompt-1',
},
}),
).resolves.toEqual({ allowed: true });
expect(extMethod).toHaveBeenCalledOnce();
});

it('stops waiting when the tool invocation is cancelled', async () => {
const extMethod = vi.fn(
Expand Down
13 changes: 0 additions & 13 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2807,19 +2807,6 @@ export function createManagedExternalToolGuard(
if (context.signal.aborted) {
throw new DOMException('Tool invocation aborted', 'AbortError');
}
if (
context.toolName === ToolNames.AGENT ||
context.toolName === ToolNames.WORKFLOW ||
context.toolName === ToolNames.CREATE_SUB_SESSION ||
context.toolName === ToolNames.SEND_MESSAGE
) {
return {
allowed: false,
reason:
'Managed external tool guard v1 does not support nested or delegated agent execution.',
};
}

let rejectOnAbort: ((error: Error) => void) | undefined;
const aborted = new Promise<never>((_resolve, reject) => {
rejectOnAbort = reject;
Expand Down
Loading
Loading