fix: gate MCP user tokens on owner-scoped personal RBAC - #27945
Conversation
ibetitsmike
changed the title
d1fcb77 to
463b7df
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
463b7df to
733f8fb
Compare
733f8fb to
5c286be
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c286be1d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5c286be to
a5f9914
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
a5f9914 to
9ae868b
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
9ae868b to
30677a1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30677a1bea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
30677a1 to
5897a6b
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
mcp_server_user_tokens rows are per-user credentials, but every dbauthz method was gated on ResourceDeploymentConfig, forcing all call sites to elevate with AsSystemRestricted. Re-gate them on ActionReadPersonal and ActionUpdatePersonal against the owning user, mirroring the external_auth_links pattern, and drop the token-path elevations in coderd/mcp.go and chatd. The id-keyed refresh mutations resolve the owner by prefetching the row via a new GetMCPServerUserTokenByID query. The chatd subject gains ActionUpdatePersonal so background token refresh and failure persistence pass without elevation.
…l authorizer The MethodTestSuite entries assert call shape against a fake authorizer, so they cannot prove real policy outcomes. Add a strict-authorizer test over a real token row (owner passes get, upsert, and delete; a stranger and an org admin of another org fail) and pin the chatd subject's complete ResourceUser action set so a future broad grant fails the test.
Both production writes fetch the token row through the underlying store and authorize only ActionUpdatePersonal, so the subject never needs ActionReadPersonal. Pin the write-only shape in the real-authorizer test and use test contexts instead of context.Background.
2 participants
Re-gates every
mcp_server_user_tokensdbauthz method fromrbac.ResourceDeploymentConfigonto owner-scoped personal actions, mirroring theexternal_auth_linksprior art. These are per-user credential rows, not deployment config, and the old gate forced every call site to elevate withdbauthz.AsSystemRestricted.Summary
ActionReadPersonaland writes onActionUpdatePersonalagainst the token owner (MCPServerUserToken.RBACObject()derives fromuser_id). The id-keyed refresh and refresh-failure mutations resolve the owner via aGetMCPServerUserTokenByIDprefetch inside dbauthz; the mutation queries cannot retarget a row's owner, so there is no fetch-authorize race.AsSystemRestrictedelevations incoderd/mcp.goand all four in chatd are removed along with their//nolint:gocriticmarkers; handlers use the request actor.ActionReadPersonalonResourceUser. Background token refresh and failure persistence use a new per-userAsChatdTokenOwnersubject holding only user-levelActionUpdatePersonalscoped to the token owner (both mutations resolve the row via the underlying store, so the subject needs no read grant), and chatd never holds site-wide personal-write access.TestAsChatdpins the completeResourceUseraction set andTestMCPServerUserTokensAuthexercises both subjects against the real authorizer.rbac.NewAuthorizeragainst real rows): owner passes get/upsert/delete, a stranger fails, and an org admin of another org fails.No user-visible behavior change. Stacked on #27944. Completes the MCP org-separation stack.
Closes https://linear.app/codercom/issue/CODAGT-806
UAT: regression pass on a dogfood instance: OAuth2 connect redirect, member list/status with redaction, self-scoped disconnect (cannot touch another user's token), end-to-end chat MCP tool invocation, and lower-stack audit/ACL smoke checks.