mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-07 16:11:46 +02:00
ea8e686197
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Apps use OAuth connection grants to let agents reach external services. > - Paperclip stored a requested scope as a granted scope when a provider omitted `scope` from its token response. > - A live MCP connection showed why that matters: the provider reported scopes beyond the requested scope. Scope names alone do not establish write capability. > - This PR records whether a scope came from the provider or from Paperclip's request, and flags asserted extra scopes. > - The result is an honest scope record across authorization and refresh, including for self-hosted and Cloud instances. **Review order:** shared scope resolver and OAuth write paths in `server/src/services/tool-access.ts` → optional schema/types/validator fields → focused tests in `server/src/__tests__/tool-access-service.test.ts`. This PR changes the shared OAuth record. The Enterpret catalog definition is separate in **#13906**. ## Linked Issues or Issue Description No public issue covers this bug. Related: **#13906** adds the official read-only Enterpret connector with OAuth and organization tokens. **What happened?** The OAuth callback stored `normalizeOauthScopes(token.scope ?? requestedScopes)`. When the provider omitted `scope`, Paperclip recorded the request as though it were a verified grant. In the live case: ```text Paperclip requested mcp:read Provider token reply no scope field Paperclip recorded ["mcp:read"] Token introspection openid email profile mcp:read mcp:write ``` The access token is opaque. A negative introspection control returned `active: false` with no scope, confirming the wider scope belonged to the live token. The grants API could therefore present requested scopes as though the provider had asserted them. This observation did not establish access to write tools. **Expected behavior** Store the provider's asserted scope when present. When absent, identify the value as an inference from the request. Preserve that provenance through refresh and report extra scopes only when the provider actually asserts them. **Steps to reproduce** 1. Use an OAuth provider that omits `scope` from its token response. 2. Complete consent after requesting `mcp:read`. 3. Read the connection or grant: before this fix, the stored `scopes` looked like an asserted read-only grant. 4. The focused fixture tests reproduce the callback and refresh record without needing a live provider. **Deployment mode** The shared OAuth path affects self-hosted and Cloud. The live provider evidence came from an isolated self-hosted runtime. ## What Changed - Added `resolveGrantedOauthScopes`. It records `scopeSource: provider` when the token response asserts scope, or `requested_fallback` when it does not. `unrequestedScopes` contains only provider-asserted scopes outside the request. - Applied the resolver at initial authorization and refresh. A refresh without `scope` retains a previous provider assertion and warning; a fresh assertion can replace them. - Stored the actual authorization request per grant as `providerTenant.oauth.requestedScopes`. This keeps a multi-user connection's refresh baseline tied to the right grant. Generic MCP OAuth also records discovered scopes when it sends them; curated apps retain their reviewed request. - Carried provenance onto the connection and default organization grant. The organization grant does not receive token expiry, so its existing refresh/reconnect behavior remains intact. - Added optional fields to database JSONB schema types, shared types, and validation. Existing grants need no migration or backfill. This PR **does not change authorization decisions**, reject tokens, introspect providers, or show a new UI warning. If a provider hides an over-grant by omitting `scope`, Paperclip still cannot discover it. `unrequestedScopes: []` with `requested_fallback` means **unknown**, not least privilege. ## Verification **Current head:** `7fde922a8`. Reconciled with master `88ff98b83`; the final diff is six OAuth implementation, contract, and test files. Preserved master's generic `offline_access` consent and legacy callback baseline, and verified requested versus provider-asserted scopes on each grant. Removed four duplicated GitHub token-method fixture properties after fresh review. All 504 focused tests in seven suites pass on the final head. Local workspace build, recursive typecheck, build-gap typecheck, module boundaries, node-version, token gates, and runtime push-policy checks pass. The full local Vitest run completed with 15,610 passing tests, 91 skipped, and six failures in heartbeat/workspace suites; all six failures reproduced on unchanged master `88ff98b83` in an isolated baseline worktree. These suites and their runtime code are unchanged by this PR. Greptile is 5/5 on this exact head with no actionable findings. All current-head GitHub checks are green: 53 successful check runs, two intentional Storybook skips, and the successful Snyk status. The initially failed adapter-access and signoff-heartbeat shards both passed their single rerun. [CI run](https://github.com/paperclipai/paperclip/actions/runs/37521588330). The regression coverage includes omitted and explicit scopes, asserted extra scopes, refresh preservation, per-user request baselines, generic MCP discovery, and organization grants. No live provider token is needed to reproduce these recordkeeping cases. ## Risks - Existing records keep their scope list but have no source marker. Historical provenance cannot be recovered from them. - A refresh that omits `scope` preserves the last known assertion. A provider that silently narrows a grant will not update the record until it asserts a new scope. - A provider that omits `scope` leaves the actual scope unasserted. This PR cannot discover scopes omitted from the response or establish endpoint capabilities. Enterpret’s provider fixes are separate follow-ups. - Scope provenance is additive metadata. No policy path uses these new fields to allow or deny an action. ## Model Used Implementation and tests: Claude Opus 5 (`claude-opus-5`) through Claude Code with extended thinking, tools, and code execution. PR text cleanup: Codex GPT-6 (exact host model ID and context window were not exposed; tool use). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] Focused tests pass locally; full-suite baseline failures are documented above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All current-head CI gates are green - [x] Greptile is 5/5 on the current head with no actionable follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge This shared scope-recording fix can be reviewed and merged independently of the Enterpret connector. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing>