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 in src/vs/platform/agentHost/node/protocolServerHandler.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 and the isClientDispatchable() helper, 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 followed the existing family-guard convention.23915495 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
Ref #331796 (changes)
Summary
The AHP protocol server's
dispatchActionhandler 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 in
src/vs/platform/agentHost/node/protocolServerHandler.ts:The family guards return
truefor every action in the family. The protocol already distinguishes client-vs server-owned actions per type via theIS_CLIENT_DISPATCHABLEmap and theisClientDispatchable()helper, whose doc comment says "Servers SHOULD call this to validate incomingdispatchActionrequests." The handler consults neither.Impact
A client can dispatch server-owned actions the protocol intends only the server to emit, for example
automation/setand the server-ownedautomationRun/*lifecycle transitions, plus server-owned types in the session family (session/ready,session/creationFailed, etc). There's no downstream re-check:agentService.dispatchActionroutes 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:cfb12483followed the existing family-guard convention.23915495switched toisClientDispatchableas 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:
isClientDispatchablelikely needs its type expanded if brought intodispatchAction🤖 AI Assisted Bug Report