Skip to content

feat(desktop): authorize client capabilities in managed sessions - #4143

Open
YayoiNanoka wants to merge 26 commits into
apache:mainfrom
YayoiNanoka:feat/client-capability-session-grants
Open

feat(desktop): authorize client capabilities in managed sessions#4143
YayoiNanoka wants to merge 26 commits into
apache:mainfrom
YayoiNanoka:feat/client-capability-session-grants

Conversation

@YayoiNanoka

Copy link
Copy Markdown
Contributor

Summary

Allow trusted macOS Desktop Client Capabilities to cross the managed execution boundary without switching the Session to Full Access.

  • separate provider acceptance from Host admission so approval completes before T1 and execution
  • persist Session Grants and reuse them for matching Browser Origins
  • attest Browser Origins before admission and limit cross-Origin results
  • keep Desktop Settings on its existing read/update-confirmation path
  • present a narrow Client Capability approval in the existing composer interaction slot

This is the first stacked slice for #4012. Computer Use and Desktop MCP are intentionally left to follow-up PRs.

Refs #4012

Verification

  • Focused Node test suites passed for the Client Capability broker/coordinator/protocol, Interaction coordinator/projector, Session Grant storage, Desktop Browser/session/native capability paths, permission IPC, and Workbar adapter.
  • Relevant Core, Runtime Host, preload, and renderer type checks passed.
  • git diff --check main...HEAD passed.
  • Full root lint/build/typecheck was not run; the current checkout has unrelated existing Runtime/UI type errors and missing sigstore type packages in the Desktop main configuration.

Draft follow-up

  • Attach a screenshot of the Client Capability approval card.
  • Confirm the required repository CI on this exact head.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Over 1000 readable lines label Aug 29, 2026
@YayoiNanoka
YayoiNanoka force-pushed the feat/client-capability-session-grants branch 7 times, most recently from 35facba to b7dd148 Compare August 30, 2026 07:42
@YayoiNanoka
YayoiNanoka marked this pull request as ready for review August 30, 2026 07:49

function navigationResult(url: string, requiresApproval: boolean): string {
return (
`Navigated to ${sanitizedPageUrl(url)}.` +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@YayoiNanoka
YayoiNanoka force-pushed the feat/client-capability-session-grants branch 2 times, most recently from 3bf37f5 to 2ba3ae1 Compare August 30, 2026 11:12
@YayoiNanoka
YayoiNanoka force-pushed the feat/client-capability-session-grants branch from 2ba3ae1 to f6d014a Compare August 30, 2026 11:34

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Over 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants