Skip to content

Fix P1s: shared-save null bypass, push cleanup, invite secret leak - #40

Merged
tpaulshippy merged 2 commits into
mainfrom
fix/p1-share-guards
Oct 20, 2026
Merged

tpaulshippy merged 2 commits into
mainfrom
fix/p1-share-guards

Conversation

@tpaulshippy

@tpaulshippy tpaulshippy commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Addresses the two cubic P1s from #39 plus the invite credential leak:

  1. Shared-save null bypass (routes/files.ts, routes/github-repos.ts): an explicit baseVersion: null skipped both the shared 409 guard (=== undefined check) and the atomic compare (checkBaseVersion treats null as no-check) — a silent teammate overwrite. Null is now normalized to absent at both save endpoints, so it hits the same 409 reload prompt as an omitted version.
  2. Push cleanup deletes possible commits (github.ts repoHasCommits + create/promote catch): on push failure the shell is probed first (commits endpoint; 409 = confirmed empty). Only a confirmed-empty shell is deleted; a repo that may hold commits or can't be verified is kept, with a message saying exactly which.
  3. Invite leaks memberSecret (routes/share.ts): the JSON body spread the full createInvite result including the owner's member credential (readable by page JS despite the HttpOnly cookie). The body now carries only code + expiresAt + playgroundId; the credential still travels in the cookie, which the client never needed from JSON.

Tests: null-baseVersion 409s for both save endpoints, invite body has no secret, repoHasCommits unit cases (commits/empty/denied/network-down), and a new tests/repo-create.test.ts covering create ×3 probe outcomes + promote kept-commits. 358/358 pass, tsc clean.


Summary by cubic

Fixes three P1 issues: a shared-save null bypass, unsafe repo cleanup on failed push, and a leaked invite credential.

  • Explicit baseVersion: null now behaves like an omitted version on both /api/files and /api/file, so shared saves hit the same stale-version 409 instead of silently overwriting a teammate's edits.
  • Failed initial pushes (create and promote) delete the new repo only when the commits endpoint returns the exact empty-repo 409. Other 409s, anomalous 200s, and failed probes keep the repo; a confirmed-empty repo whose deletion fails is reported explicitly instead of claimed unverifiable.
  • POST /api/share/invite no longer spreads the full invite record into the JSON body; it returns only code, expiresAt, and playgroundId. The owner's member credential travels only in the HttpOnly cookie.

Written for commit 0f923be. Summary will update on new commits.

View guided diff Turn on auto-fix

- Treat explicit baseVersion:null as absent in /api/files and /api/file:
  it previously bypassed both the shared 409 guard and the atomic
  compare (checkBaseVersion treats null as no-check), allowing silent
  teammate overwrites
- Only delete a just-created repo after confirming it is empty
  (repoHasCommits probe; 409 means fresh shell). A push accepted
  before failing, or an unverifiable probe, keeps the repo with an
  explicit message instead of deleting possible commits
- Stop spreading memberSecret in the /api/share/invite JSON body;
  the credential travels only in the HttpOnly cookie, the client
  uses just code + playgroundId

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/github.ts Resolved
Comment thread src/github.ts Resolved
Comment thread src/routes/github-repos.ts Resolved
- 409 counts as empty only with the known empty-repo message;
  other 409s and anomalous 200 payloads stay unknown (fail safe)
- Confirmed-empty but failed delete reports explicitly instead of
  claiming an unverifiable state
@tpaulshippy
tpaulshippy merged commit e0b7fb0 into main Oct 20, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant