Repository navigation
fix(server): pairing grants use SQLite-compatible scope flags - #16786
lastobelus wants to merge 1 commit into
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The implementation is a narrow SQLite compatibility fix with focused regression tests, but it changes the production authentication path that validates requested scopes and consumes pairing credentials. Authentication-sensitive behavior requires human review even when the diff is small and the intended behavior is preserved. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe pairing grant scope predicate now uses a numeric SQL condition. Tests cover rejected scope requests and one-time grant consumption. ChangesPairing grant scope matching
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The scope checks preserve rejection of empty or ungranted requests while allowing a valid unscoped one-time consume. No material merge-blocking risk is evident; proceed with normal checks. Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
Full details: ApprovabilityExplanation This pull request changes pairing credential consumption in
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Note Grok responding on behalf of Julius. Closing as superseded by #16730 (merged in 4daec10), which lands the same |
Pairing credential consumption binds
requestedScopes === undefineddirectly into an SQLite query. On Node 24.13.1, native SQLite rejects boolean bind values, so valid pairing requests fail withBootstrapCredentialConsumeAvailableErrorbefore the intended scope checks run. This is a small, focused fix for an obvious parameter-type bug; it preserves the existing pairing and scope behavior and qualifies for the no-prior-discussion exception.Bind the scope-omission flag as
1or0. Add regression cases showing that empty and ungranted scope requests receive the intended scope error without consuming the token; a subsequent request omitting scopes still receives its original grant and consumes it exactly once.Verification on Node 24.13.1:
vp test run apps/server/src/auth/PairingGrantStore.test.ts apps/server/src/auth/EnvironmentAuth.test.ts apps/server/src/auth/SessionStore.test.ts apps/server/src/auth/http.test.ts: 56 tests pass across four files, covering omitted and requested scopes, proof binding, and concurrent one-time consumption.Model: GPT-6.1 Sol. Harness: Codex.
Independent downstream delivery: https://redirect.github.com/lastobelus/lastCode/pull/318.