Repository navigation
Bare-id OAuth state for shared playgrounds + reconciliation tests - #35
Conversation
Members log in from the bare-id join cookie, which the callback gate rejected outright. Accepting it naively would let the canonical flow treat a foreign object as the caller's pre-login device (export + clear), so bare-id states on non-shared objects are still refused before any adoption runs. Tests: new tests/sharing-canonical.test.ts pins the sharing x canonical reconciliation (invite id shape, join routing, member-token git ops, member/non-member callback, migration skip, bare-id anonymity).
|
@cubic-dev-ai please post-merge review this PR. It was merged before your review completed (your check showed skipping). The change touches OAuth callback auth and session routing in src/index.ts, plus a reconciliation of shared playgrounds with canonical sessions — security-sensitive, so a retroactive review would be valuable. Thanks! |
@tpaulshippy I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/index.ts">
<violation number="1" location="src/index.ts:1128">
P1: Bare canonical IDs are public, so this state check no longer prevents OAuth login CSRF. An attacker can authorize their own GitHub account and navigate a member's browser to this callback; the member cookie is sent and `setMemberToken` links the attacker's token to that slot. Generate and validate a one-time nonce bound to the member/browser instead of using the bare ID as OAuth state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Members log in from the bare-id join cookie, which the callback gate rejected outright (members could never finish login on a canonical-hosted playground). Accepting it naively would let the canonical flow treat a foreign object as the caller's pre-login device (export + clear), so bare-id states on non-shared objects are still refused before any adoption runs.
New tests/sharing-canonical.test.ts (8 tests) pins the sharing x canonical reconciliation: invite id shape, join routing, member-token git ops, member/non-member callback, migration skip, bare-id anonymity, bare-id callback rejection.
Summary by cubic
Members logging in from a shared playground's bare-id join cookie can now finish GitHub OAuth — the callback previously rejected bare-id states outright. Bare-id states on non-shared objects are still refused before any adoption runs, so the canonical flow never treats a foreign object as the caller's pre-login device.
Tests
tests/sharing-canonical.test.tspinning sharing x canonical reconciliation: invite id shape, join routing, member-token git ops, member/non-member callbacks, migration skip, and bare-id anonymity and rejection.Written for commit f86f03e. Summary will update on new commits.