Skip to content
Merged
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
101 changes: 101 additions & 0 deletions .qwen/design/2026-06-12-session-shell-permission-policy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
---
title: 'Session Shell Permission Policy'
date: '2026-06-12'
status: 'implemented'
---

# Session Shell Permission Policy

## Problem

`POST /session/:id/shell` executes a shell command directly through the daemon,
without an LLM tool call or the normal agent permission mediation flow. Before
this change, the endpoint was a non-strict mutation and could be reached with a
daemon token plus a session id, or on the tokenless loopback developer default.

That is too much authority for a direct shell surface. A caller should not be
able to execute shell commands unless the daemon operator explicitly enables
the surface and the caller proves it is attached to the target session.

## Goals

- Disable direct session shell by default.
- Require explicit operator opt-in with `qwen serve --enable-session-shell`.
- Require bearer-token configuration before the opt-in becomes effective.
- Require a client id that is registered on the addressed session.
- Apply the same policy at the REST route, ACP HTTP dispatcher, and bridge
execution sink.
- Keep normal agent shell tool approvals and permission mediation unchanged.

## Non-Goals

- Do not route direct shell through `PermissionMediator`.
- Do not change prompt submission, prompt queueing, or SDK pending prompt
behavior.
- Do not add a shell-specific rate limiter.
- Do not add an environment-variable alias for the opt-in flag.

## Design

`runQwenServe` resolves and trims the bearer token once. After that it computes
one effective boolean:

```ts
sessionShellCommandEnabled =
opts.enableSessionShell === true && token !== undefined;
```

That value is threaded into the bridge, REST app, and ACP dispatcher. Embedded
callers that invoke `createServeApp` directly compute token presence using a
non-empty string check so `token: ''` behaves like no token for both strict
mutation gating and shell capability advertisement.

The REST route uses `mutate({ strict: true })`. On a tokenless loopback daemon,
the strict gate returns `401 token_required` before the handler runs. When a
token is configured, the handler rejects disabled shell with
`session_shell_disabled`, then requires `X-Qwen-Client-Id`, then validates the
command body, and finally delegates to the bridge.

The ACP dispatcher keeps `_qwen/session/shell` dispatchable for old clients, but
does not advertise it in the initialize `_qwen.methods` list unless the
effective policy is enabled. Disabled ACP calls return a stable
`session_shell_disabled` JSON-RPC error without logging the command or calling
the bridge. Enabled calls still require the connection to own the session and
must use the bridge-stamped session binding client id.

The bridge enforces the final defense-in-depth check at
`executeShellCommand()`: disabled, missing client id, unknown session, then
unbound client id. Only after those checks pass does it publish shell events,
execute the command, or write shell history.

## Error Contract

REST:

- no token: `401`, `code: token_required`
- disabled: `403`, `code/errorKind: session_shell_disabled`
- missing client id: `403`, `code/errorKind: client_id_required`
- malformed or unbound client id: existing `400 invalid_client_id`
- unknown session: existing `404 SessionNotFoundError` mapping

ACP:

- disabled: `RPC.INVALID_REQUEST`, `data.errorKind: session_shell_disabled`
- missing session binding client id: `RPC.INVALID_REQUEST`,
`data.errorKind: client_id_required`
- unowned session and invalid client id keep existing JSON-RPC mappings

## Compatibility

`DaemonSessionClient.shellCommand()` continues to work when the daemon is
explicitly enabled and authenticated because the session client carries the
session-bound client id. Bare `DaemonClient.shellCommand(sessionId, command)`
must pass `opts.clientId`, otherwise it receives `client_id_required`.

## Test Coverage

The implementation is covered by focused bridge, REST, ACP transport, serve
boot, and command-parser tests. The highest-value checks are default-disabled
behavior, tokenless strict gating, capability advertisement, ACP initialize
method filtering, bridge sink enforcement, and propagation of the session-bound
client id.
60 changes: 60 additions & 0 deletions .qwen/e2e-tests/session-shell-permission-policy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Session Shell Permission Policy E2E

## Problem

Direct session shell is a user-visible daemon capability. It must stay disabled
by default and only become visible and callable when the operator enables it on
an authenticated daemon.

## Scenarios

1. Start `qwen serve` on loopback without `--token` or
`QWEN_SERVER_TOKEN`.
- `/capabilities.features` must not include `session_shell_command`.
- ACP initialize `_meta.qwen.methods` must not include
`_qwen/session/shell`.
- `POST /session/:id/shell` must return `401 token_required`.

2. Start `qwen serve --token <token>` without `--enable-session-shell`.
- `/capabilities.features` must not include `session_shell_command`.
- ACP initialize must not advertise `_qwen/session/shell`.
- Authenticated REST shell calls must return
`session_shell_disabled`.

3. Start `qwen serve --token <token> --enable-session-shell`.
- `/capabilities.features` must include `session_shell_command`.
- ACP initialize must advertise `_qwen/session/shell`.
- REST shell without `X-Qwen-Client-Id` must return
`client_id_required`.
- REST shell with the session-bound client id must execute and stream
shell output through the session events.

## Commands

Focused automated checks:

```bash
cd packages/acp-bridge && npx vitest run src/bridge.test.ts
cd packages/cli && npx vitest run src/serve/server.test.ts src/serve/acpHttp/transport.test.ts src/commands/serve.test.ts
```

Final verification:

```bash
npm run build
npm run typecheck
```

## What This Proves

- The default daemon does not expose direct session shell.
- Operator opt-in without bearer auth is ineffective.
- Authenticated opt-in advertises the capability consistently across REST and
ACP.
- Calls still need a client id bound to the target session.

## What This Does Not Prove

- It does not validate prompt queue backpressure.
- It does not validate normal agent-originated shell tool approval behavior.
- It does not add or validate shell-specific rate limiting.
115 changes: 114 additions & 1 deletion packages/acp-bridge/src/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
InvalidSessionScopeError,
NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE,
RestoreInProgressError,
SessionShellClientRequiredError,
SessionShellDisabledError,
SessionNotFoundError,
WorkspaceMismatchError,
} from './bridgeErrors.js';
Expand All @@ -40,7 +42,7 @@ import type { ChannelFactory } from './channel.js';
import type { BridgeTelemetry } from './bridgeOptions.js';
import { createInMemoryChannel } from './inMemoryChannel.js';
import type { BridgeEvent } from './eventBus.js';
import { ApprovalMode } from '@qwen-code/qwen-code-core';
import { ApprovalMode, ShellExecutionService } from '@qwen-code/qwen-code-core';
import {
FakeAgent,
type ChannelHandle,
Expand Down Expand Up @@ -4767,6 +4769,117 @@ describe('createAcpSessionBridge', () => {
});
});

describe('executeShellCommand permission policy', () => {
function mockShellExecute(output = 'ok') {
return vi.spyOn(ShellExecutionService, 'execute').mockResolvedValue({
pid: 123,
result: Promise.resolve({
rawOutput: Buffer.from(output),
output,
exitCode: 0,
signal: null,
error: null,
aborted: false,
pid: 123,
executionMethod: 'none',
}),
});
}

async function setupShellSession() {
const handle = makeChannel();
const bridge = makeBridge({
sessionShellCommandEnabled: true,
channelFactory: async () => handle.channel,
});
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });
return { bridge, session, handle };
}

it('rejects direct shell by default before executing the command', async () => {
const shellSpy = mockShellExecute();
const { bridge, session } = await setupShellSession();
const disabledBridge = makeBridge({
channelFactory: async () => {
throw new Error('disabled shell should not spawn a channel');
},
});

await expect(
disabledBridge.executeShellCommand(session.sessionId, 'echo hi'),
).rejects.toBeInstanceOf(SessionShellDisabledError);
expect(shellSpy).not.toHaveBeenCalled();

await bridge.shutdown();
await disabledBridge.shutdown();
shellSpy.mockRestore();
});

it('requires a client id before checking whether the session exists', async () => {
const shellSpy = mockShellExecute();
const bridge = makeBridge({
sessionShellCommandEnabled: true,
channelFactory: async () => {
throw new Error('missing client id should not spawn a channel');
},
});

await expect(
bridge.executeShellCommand('unknown-session', 'echo hi'),
).rejects.toBeInstanceOf(SessionShellClientRequiredError);
expect(shellSpy).not.toHaveBeenCalled();

await bridge.shutdown();
shellSpy.mockRestore();
});

it('rejects unregistered client ids when direct shell is enabled', async () => {
const shellSpy = mockShellExecute();
const { bridge, session } = await setupShellSession();

await expect(
bridge.executeShellCommand(session.sessionId, 'echo hi', undefined, {
clientId: 'client-not-issued',
}),
).rejects.toBeInstanceOf(InvalidClientIdError);
expect(shellSpy).not.toHaveBeenCalled();

await bridge.shutdown();
shellSpy.mockRestore();
});

it('executes and stamps events when the client id belongs to the session', async () => {
const shellSpy = mockShellExecute('hello\n');
const { bridge, session } = await setupShellSession();
const abort = new AbortController();
const events = bridge.subscribeEvents(session.sessionId, {
signal: abort.signal,
});

const result = await bridge.executeShellCommand(
session.sessionId,
'echo hello',
undefined,
{ clientId: session.clientId },
);

expect(result).toEqual({
exitCode: 0,
output: 'hello\n',
aborted: false,
});
expect(shellSpy).toHaveBeenCalledTimes(1);
const it = events[Symbol.asyncIterator]();
const first = await it.next();
expect(first.value?.type).toBe('user_shell_command');
expect(first.value?.originatorClientId).toBe(session.clientId);

abort.abort();
await bridge.shutdown();
shellSpy.mockRestore();
});
});

describe('setSessionApprovalMode (#4175 Wave 4 PR 17)', () => {
/**
* #4282 fold-in 4 (qwen-latest C1). Build a channel factory whose
Expand Down
10 changes: 9 additions & 1 deletion packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import {
SessionLimitExceededError,
WorkspaceMismatchError,
InvalidClientIdError,
SessionShellClientRequiredError,
SessionShellDisabledError,
// Mediator's `vote()` validates `optionId in allowedOptionIds`,
// but the bridge ALSO throws `InvalidPermissionOptionError`
// pre-mediator when a wire client tries to inject the cancel
Expand Down Expand Up @@ -4005,11 +4007,17 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
`qwen serve: bridge executeShellCommand for session=${sessionId}`,
'info',
);
if (opts.sessionShellCommandEnabled !== true) {
throw new SessionShellDisabledError();
}
if (context?.clientId === undefined) {
throw new SessionShellClientRequiredError();
}
const entry = byId.get(sessionId);
if (!entry) throw new SessionNotFoundError(sessionId);
const originatorClientId = resolveTrustedClientId(
entry,
context?.clientId,
context.clientId,
);

if (signal?.aborted) {
Expand Down
27 changes: 24 additions & 3 deletions packages/acp-bridge/src/bridgeErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,29 @@ export class InvalidClientIdError extends Error {
}
}

/**
* Thrown when a direct daemon shell command is attempted without the operator
* explicitly enabling the high-risk session shell surface.
*/
export class SessionShellDisabledError extends Error {
constructor() {
super('Direct session shell is disabled for this daemon');
this.name = 'SessionShellDisabledError';
}
}

/**
* Thrown when a direct daemon shell command has no client id bound to the
* addressed session. The bearer token authenticates the caller to the daemon;
* this error means the caller has not proven ownership of the session.
*/
export class SessionShellClientRequiredError extends Error {
constructor() {
super('Direct session shell requires a session-bound client id');
this.name = 'SessionShellClientRequiredError';
}
}

/**
* Thrown by `bridge.respondToPermission` when the voter's
* `optionId` isn't in the set of options the agent originally
Expand Down Expand Up @@ -449,9 +472,7 @@ export class InvalidRewindTargetError extends Error {
export class BranchWhilePromptActiveError extends Error {
readonly sessionId: string;
constructor(sessionId: string) {
super(
`Cannot branch session ${sessionId}: a prompt is currently active`,
);
super(`Cannot branch session ${sessionId}: a prompt is currently active`);
this.name = 'BranchWhilePromptActiveError';
this.sessionId = sessionId;
}
Expand Down
6 changes: 6 additions & 0 deletions packages/acp-bridge/src/bridgeOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ export interface BridgeOptions {
* legacy behavior, NOT recommended).
*/
permissionResponseTimeoutMs?: number;
/**
* Enables direct daemon shell execution through session shell APIs.
* Defaults to false. Callers should turn this on only after the daemon has
* bearer auth configured and route layers require a session-bound client id.
*/
sessionShellCommandEnabled?: boolean;
/**
* Per-session cap on pending permissions in flight. New
* `requestPermission` calls past this cap resolve as cancelled with
Expand Down
5 changes: 4 additions & 1 deletion packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,10 @@ export interface AcpSessionBridge {
* Execute a shell command directly on the daemon (no LLM involvement).
* Streams output through the session's SSE bus and injects the
* command+result into the LLM's chat history via extMethod.
* Throws `SessionNotFoundError` for unknown ids.
* Throws `SessionShellDisabledError` when direct shell is not enabled,
* `SessionShellClientRequiredError` when no session-bound client id is
* provided, `InvalidClientIdError` when the client id is not bound to the
* session, and `SessionNotFoundError` for unknown ids.
*/
executeShellCommand(
sessionId: string,
Expand Down
Loading
Loading