Repository navigation
feat: connect a person's own account for assistant turns that act for them - #8044
Conversation
🦋 Changeset detectedLatest commit: 2a2cfc3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Demo
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 12 files
This PR changes authentication, authorization, or input validation. Ultrareviews find 2.4x more serious bugs than standard reviews. Comment @cubic-dev-ai ultrareview to run one.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Reviewed incremental diff against #8043 at f141723b220d5e2c51153b3cc35b17e3763f7df8; tested combined head in an isolated local stack with synthetic credentials/local OAuth stub. No novel confirmed defect to annotate inline; not repeating resolved comments. COMMENT, not blanket approval: private-delivery behavior remains unverified end to end.
| Check | PASS/FAIL |
|---|---|
| Recovery security: U/org/registration binding, membership removal, malformed/expired state, wrong-user/support/impersonation refusal, concurrent single-use completion | PASS — 21 focused test entries; real PostgreSQL/Redis, mocked browser authentication and permission decisions |
| Personal subject resolution: direct/gateway select U, never owner/agent; attribution-only context cannot substitute for verified credential | PASS — 8 final new test entries including HTTP checks below |
| Post-authentication HTTP gate: missing/expired U → 412 without challenge; permission denial → 403 without link; userless rejection → 401 | PASS — production selection/rejection + HTTP serializer; injected selection/permitter, not full public-router admission |
| Existing credential/policy/resolver regressions | PASS — separate focused runs: 17 and 88 entries (overlap; not a unique-test total) |
| Assistant adapter/composition/history regressions and outbound-tool characterization | PASS — 49 entries, including fake-transport actual Slack tool probes; no Slack network calls |
| Standalone recovery routes: invalid/expired/future/zero-date state; signed-out redirect with exact return path; wrong user, missing membership, support, impersonation | PASS — real server/session checks; refused starts preserve token |
| Standalone authorized start → local OAuth exchange → persisted session → completion; start/callback replay | PASS — 302 start, 303 callback, exactly one token exchange and persisted session, 200 completion; replay 404/401 |
| Recovery-route privacy headers | PASS — all 11 tested recovery-route responses assert no-store/no-referrer |
| Browser invalid-link rendering and real local dev-IDP login | PASS — 2 scenarios; no external requests attempted |
Standalone total: 14 HTTP + 2 browser scenarios. Recovery cache entries were synthetically seeded, so this bypasses issuance; ordinary login and callback exchange/persistence were real. Support/impersonation sessions were synthetic clones. Mandatory enforcement mode was not independently confirmed in the standalone run. Browser login and HTTP return-path assertions were separate, not a continuous browser recovery journey.
Not verified (not counted as PASS): model-driven private Slack delivery, trusted-U recipient mapping, ephemeral→DM fallback, no-private-route suppression, dashboard stream/transcript access, full issuance-to-retry flow, or selected-profile immutability. Adapter changes are prompt instructions; actual outbound Slack tools pass supplied URLs/recipients unchanged. Direct arbitrary-tool-argument probes do not demonstrate that the revised model violates those instructions, so this is a coverage limitation, not a new reproduced finding.
No production edits, commits, pushes, external OAuth providers, or real Slack calls. Cleanup verified: isolated stack, volumes/networks, scoped Pub/Sub resources, browser session, disposable worktree, temporary tests/caches/logs, and local review refs removed. Shared emulator preserved; main checkout clean.
This comment has been minimized.
This comment has been minimized.
… them Person-backed turns resolve the authorizing user's connection. A missing or expired one answers with a single-use link bound to that user, organization and client; starting and completing it verify the signed-in user and their access, and a denial shows No access.
Issue links on the canonical dashboard origin and relay only links with that prefix, route them to the person the turn acts for, keep the link out of logs, validate a login's callback before using it up, and accept a callback on another host when the person's access still checks out.
654edfc to
cff2976
Compare
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Require the starting browser cookie (via the remote login hop when the callback host differs) before storing a connection, re-resolve the server auth client at login start and completion, mount the link routes with browser error pages, and give worker turns the site URL.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
…cp:auth check Principal-backed API keys name their validated authorizer as the connection owner for typed auth clients and read that user connection; the mandatory check still decides with the agent grants. Also satisfy lint, use a generated query for the callback-origin fixture, bound browser hops in tests, and drop a stale-attachment case the check reports as an error.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
ResolveAuthClientAccessTokens sent principal-backed keys to the attachment path before the authorizer branch; treat a key asking for its authorizer as unattached. Cover the token retrieval, assert the shadow decision, and reuse the consent registration helper for a callback origin.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…nnection Keep the agent subject so an explicit attachment still binds its pinned connection; without one, a principal-backed agent key names its validated authorizer as the connection owner for the agent mcp:auth check and reads that user connection. Align the reconnect check with token retrieval.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
… handback paths Under enforcement, a legacy agent gateway member and a handback to the upstream client choice read an unattached agent key from its validated authorizer own connection when the agent mcp:auth check on a catalog client names that owner. Explicit attachments still serve first, pinned; before enforcement the key is still denied.
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…zer fallback An agent key falls back to its authorizer own connection only when the agent was never bound for the sign-in and registration. A revoked binding or one pinned to a replaced source keeps failing closed. Also cover a connect-only credential ceiling, the policy agent sessions store.
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…their authorizer Remove the binding-history query and Bound flag: a revoked attachment never serves, and the authorizer path stands on its own mcp:auth decision. Admitted agent sessions take the same authorizer path as agent keys; issued v4 sessions keep their admission-derived auth ceiling.
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
AIM-598 (2 of 2)
Stacked on #8043.
Summary
AuthorizerUserIDU) now resolves personal auth clients against U's own connection. Before, it used the agent subject, which owns no connection.connectionSubjectfeeds both the mandatorymcp:authcheck and the token lookup, for direct servers and gateway members.connection_owner = $represented_userrule now matches U. The agent's, owner's and U's policies all still apply, and U's owner blocks keep working.Connection requiredorReconnect required(a 412 with a readable message) instead of the 401 that sent the runtime into the owner-only OAuth flow. The message carries a link and tells the assistant to deliver it privately or not at all.<site>/mcp/connection_recovery/{token}, always on the canonical dashboard origin. It is an opaque, single-use cache entry bound to U, the organization and the auth client (with its registration and server), expiring after 15 minutes. The message is not logged, because it carries the link.mcp:authcheck for the client.mcp:auth. If the browser presents a session, it must be U. If the callback lands on a host without the session cookie, the server-held, single-use state bound to U stands in. The login is consumed only after these checks, so a stray callback cannot spend it, and a recovery login whose entry is missing fails closed.assistant_mcp_auth_requiredis unchanged.Motivation
RFC sections 7.4, 7.5 and 7.7: an agent uses only its authorizing user's connection and never borrows the owner's. When that connection is missing, the person gets a connect link bound to them, which completion verifies. A denial stays "No access".