Agent Host: dispatchAction does validate not whether clients can dispatch that action
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
Ref #331796 (changes)
## Summary
The AHP protocol server's `dispatchAction` handler decides whether to dispatch an incoming client action by checking which *family* it belongs to (`isSessionAction`, `isChatAction`, `isTerminalAction`, ...), not whether that specific action *type* is client-dispatchable. Server-owned action types that live inside an otherwise-permitted family pass the guard and get dispatched.
## Where
[The guard](https://github.com/microsoft/vscode/blob/584b2dacffdba8b29df943d8d6f99154c9f967e4/src/vs/platform/agentHost/node/protocolServerHandler.ts#L515) in `src/vs/platform/agentHost/node/protocolServerHandler.ts`:
```ts
} else if (isSessionAction(action) || isChatAction(action) || isTerminalAction(action) || isChangesetAction(action) || isAnnotationsAction(action) || action.type === ActionType.RootConfigChanged) {
this._agentService.dispatchAction(channel, action, client.clientId, msg.params.clientSeq, client.telemetryContext);
}
```
The family guards return `true` for every action in the family. The protocol already distinguishes client-vs server-owned actions per type via the [`IS_CLIENT_DISPATCHABLE` map](https://github.com/microsoft/vscode/blob/584b2dacffdba8b29df943d8d6f99154c9f967e4/src/vs/platform/agentHost/common/state/protocol/action-origin.generated.ts#L323) and the [`isClientDispatchable()` helper](https://github.com/microsoft/vscode/blob/584b2dacffdba8b29df943d8d6f99154c9f967e4/src/vs/platform/agentHost/common/state/protocol/common/reducer-helpers.ts#L29-L32), whose doc comment says "Servers SHOULD call this to validate incoming `dispatchAction` requests." The handler consults neither.
## Impact
A client can dispatch server-owned actions the protocol intends only the server to emit, for example `automation/set` and the server-owned `automationRun/*` lifecycle transitions, plus server-owned types in the session family (`session/ready`, `session/creationFailed`, etc). There's no downstream re-check: `agentService.dispatchAction` routes and dispatches without re-validating client-dispatchability.
## Additional context
Noticed while reviewing #331796 (automations AHP migration). That PR switched this guard to `isClientDispatchable`, the correct per-type check, but doing so pulled synced protocol code into the vscode-specific sync script. Looking at the draft predecessor #330463:
- [`cfb12483`](https://github.com/microsoft/vscode/pull/330463/commits/cfb12483dc91821221f7caad4006c0b7ac3456d9) followed the existing family-guard convention.
- [`23915495`](microsoft/vscode/pull/330463/commits/23915495e319560fc2a50ce20c64ee55982769a7) switched to `isClientDispatchable` as a "review findings" fix.
It looks like we should use `isClientDispatchable(action)` per action type and explicitly reject server-owned actions, instead of checking family membership.
Note: `isClientDispatchable` likely needs its type expanded if brought into `dispatchAction`
🤖 AI Assisted Bug Report
Contributor guide
Assessment
This issue has not been assessed yet.