fix: allow provider-subtree path-token scopes#56
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesscopeWithinPaths Fix and Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the scopeWithinPaths function in packages/server/src/routes/tokens.ts to support provider-subtree scopes. It extracts the operation (read/write) from the scope and validates it against the requested paths by checking if the scope matches any path grant, or if all path grants match the scope. Additionally, new test cases have been added in packages/server/src/__tests__/tokens-route.test.ts to verify that provider-subtree scopes covering requested writeback paths are allowed, while non-covering scopes and whole-tree path-token scopes are rejected. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Duplicate of #57 (same provider-subtree path-token scope fix; opened 31s apart in the session churn). Consolidating on #57 — relayauth-expert's verified branch (full 364-test suite). My review applies verbatim: bidirectional coverage with .every for cross-provider safety, whole-tree rejected, parent grant intact. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ad69b3f9
ℹ️ 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".
| } | ||
| }); | ||
|
|
||
| return pathGrants.every((grant) => matchScope(grant, [scope])); |
There was a problem hiding this comment.
Reject normalized root wildcard scopes
When a requested path-token scope uses duplicated slashes such as relayfile:fs:write://* or relayfile:fs:write:////, normalizePathTokenPath collapses it to /* after the explicit whole-tree checks. This new coverage check then accepts it because /* covers the requested path grant, so callers with a broad parent grant can mint a token scoped to the whole relayfile tree even though *, /, /*, and /** are intended to be rejected.
Useful? React with 👍 / 👎.
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Working tree is clean. The artifact was tracked (not ignored) and I restored it so no stray change gets pushed. PR #56 Review —
|
Review: PR #56 —
|
Summary
*,/,/*,/**) rejectedSecurity / Merge Hold
This intentionally widens path-token minting from exact/narrower-than-path scopes to named-subtree scopes that cover the requested paths, e.g.
relayfile:fs:write:/linear/**for/linear/issues/123.json. The widening is still bounded by the caller parent grant viascopesWithinGrantand does not permit whole-tree scopes.Merge is held pending @Human explicit approval of this provider-subtree write tradeoff.
Caller-Side Requirement
This PR does not make global fallback scopes valid. Cloud/relayfile callers must request a specific
/<provider>/**scope for path-token mints, or avoid the path-token endpoint for genuine whole-tree needs. Sendingrelayfile:fs:write:/**will continue to returninvalid_scope.Tests
node --test --import tsx packages/server/src/__tests__/tokens-route.test.tsnpm --workspace @relayauth/server run typecheckCloses #55