feat(desktop): authorize client capabilities in managed sessions - #4143
feat(desktop): authorize client capabilities in managed sessions#4143YayoiNanoka wants to merge 26 commits into
Conversation
35facba to
b7dd148
Compare
|
|
||
| function navigationResult(url: string, requiresApproval: boolean): string { | ||
| return ( | ||
| `Navigated to ${sanitizedPageUrl(url)}.` + |
There was a problem hiding this comment.
[P1] Redact destination details beyond the Origin before approval
requiresApproval means this navigation crossed into an Origin for which the Session has no Grant, but sanitizedPageUrl() still exposes its pathname. A destination such as https://other.example/reset/<token>?source=...#account therefore reveals <token> to the model before other.example is approved.
Please format the two cases separately: when requiresApproval is true, parse only HTTP(S) URLs and return at most url.origin (or a generic unapproved-site marker), dropping userinfo, pathname, query, and fragment; when it is false, preserve the current origin + pathname result for same-Origin navigation. Invalid or non-HTTP(S) destinations should remain generic.
Please also cover sensitive path segments for redirect and click navigation, including an A→B→A or multi-hop case, so the first violated URL recorded by the monotonic lease cannot leak destination details.
3bf37f5 to
2ba3ae1
Compare
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
2ba3ae1 to
f6d014a
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving managed Client Capability admission into the Runtime Host and keeping the approval/grant commit atomic. One current-main integration boundary still needs a manual merge rather than choosing either side of the conflict. This is a suggestion from an outside review, so please do push back if the final owner-association contract differs from what is now on main.
AI-assisted review disclosure: Codex ran independent authority, security, recovery, and test analysis lanes; Astro-Han is the contributor of record for this review.
| readonly providerId: string; | ||
| readonly principalId: string; | ||
| readonly clientInstanceId: string; | ||
| readonly principalKind: ClientCapabilityConnectionIdentity['principalKind']; |
There was a problem hiding this comment.
[P1] (category ③ — Client/provider trust boundary)
Thanks for making the local Desktop provider trusted for this slice. During rebase, this identity state also needs to preserve the Client-owner binding that current main added in #4187. This head still falls back to the sole global provider when the initiating Client has no same-ID provider, and the state does not carry credentialBoundClientInstanceId / capabilityOwner; a remote owner can therefore inherit an unrelated trusted Desktop provider whenever it is the only candidate. Keeping only the PR side loses remote isolation, while keeping only main leaves the local local_owner untrusted and breaks the intended Auto Browser path. Could the conflict resolve to one authority that preserves #4187’s credential-bound owner association and remote fail-closed behavior while explicitly admitting the authenticated local owner, with local Auto / remote unrelated / remote associated regressions? Please feel free to push back if another final identity seam supersedes this coordinator.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the substantial recovery and ownership work in this update. One cancellation seam still appears able to keep the current Code Mode run alive indefinitely. This is an AI-assisted review; I independently traced the broker, interaction, and drain paths. These are suggestions from an outside perspective, so please do push back if the approval lifetime is intentionally governed by a different invariant.
| turnId: options.context.turnId, | ||
| runId: options.context.runId, | ||
| toolCallId: options.context.toolCallId, | ||
| providerSignal: prepared.providerSignal, |
There was a problem hiding this comment.
Thanks for adding Host approval here. In the category ② cancellation/recovery path, could the caller signal also close this approval? After the provider sends accepted, the invocation broker clears its timer. If Code Mode’s 30s deadline or the caller then aborts, the broker settles/removes the invocation and sends cancel/release, but it does not abort prepared.providerSignal; requestClientCapabilityApproval() listens only to that provider signal. prepareTool() therefore remains blocked on this decision, and executeCodeCell() waits for the host-operation drain before returning. The same pending approval promise can also be reused by later calls in the scope. That turns a bounded caller timeout into an unbounded turn/Code Mode hang, so I believe this is P1. Could approval observe the caller cancellation too, close the durable interaction, clear the pending entry, and add an accepted → approval pending → caller abort regression? Please push back if approvals are deliberately allowed to outlive callers and another owner guarantees the drain still terminates.
Summary
Allow trusted macOS Desktop Client Capabilities to cross the managed execution boundary without switching the Session to Full Access.
This is the first stacked slice for #4012. Computer Use and Desktop MCP are intentionally left to follow-up PRs.
Refs #4012
Verification
git diff --check main...HEADpassed.Draft follow-up
AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex contributed implementation, tests, commit preparation, and this PR description. Each affected commit carries
Generated-by: OpenAI Codex.This draft was prepared and submitted by OpenAI Codex on behalf of the human contributor of record, YayoiNanoka.
Checklist
Does this PR entail a change in behavior?