Skip to content

Conversation

@mkrokosz
Copy link

@mkrokosz mkrokosz commented Feb 3, 2026

Summary

Fixes a vulnerability in the 7-day GitHub account age check that could be bypassed via username swapping.

The Attack:

  1. Attacker creates new GitHub account "attacker123" (< 7 days old)
  2. Attacker logs into ClaWHub → handle = "attacker123"
  3. Attacker tries to publish → fails (account too new)
  4. Attacker changes GitHub username to "attacker456"
  5. Accomplice (with 7+ day old account) claims "attacker123"
  6. Attacker publishes → ClaWHub fetches github.com/users/attacker123 → gets accomplice's old created_at
  7. Bypass successful

The Fix:

  • Query authAccounts table to get the stored providerAccountId (GitHub's immutable numeric user ID)
  • Compare against the id field in the GitHub API response
  • Reject if they don't match

Changes

  • convex/users.ts: Add getGithubAccountIdInternal query
  • convex/lib/githubAccount.ts: Add ID verification check
  • convex/lib/githubAccount.test.ts: Add tests for attack scenario

Test plan

  • Existing tests pass
  • New tests cover attack scenario (ID mismatch rejected, ID match allowed)
  • Manual test: publish skill with valid account still works

🤖 Generated with Claude Code

Greptile Overview

Greptile Summary

This PR hardens the 7-day GitHub account age gate against username-swap attacks by looking up the immutable GitHub providerAccountId from authAccounts and comparing it to the id returned by GET /users/:username before trusting created_at. It introduces a new internal query (getGithubAccountIdInternal) and adds unit tests covering both the mismatch (reject) and match (allow) scenarios.

Overall the approach fits cleanly into the existing caching flow in requireGitHubAccountAge: on stale cache, it fetches GitHub metadata, verifies identity, then stores githubCreatedAt/githubFetchedAt on the user.

Confidence Score: 4/5

  • This PR is mostly safe to merge and meaningfully improves security, with one fail-open edge case to consider.
  • The core fix (binding username to immutable GitHub ID) and tests are straightforward, but the current mismatch logic skips verification when payload.id is missing, which could undermine the intended protection in some scenarios.
  • convex/lib/githubAccount.ts

(4/5) You can add custom instructions or style guidelines for the agent here!

Context used:

  • Context from dashboard - AGENTS.md (source)

The 7-day account age check used the GitHub username (handle) to fetch
account creation date from the GitHub API. Since GitHub usernames can be
changed and immediately claimed by others, an attacker could:

1. Create new GitHub account "attacker123" (< 7 days old)
2. Log into ClaWHub → handle = "attacker123"
3. Change GitHub username to "attacker456"
4. Have accomplice (with old account) claim "attacker123"
5. Bypass 7-day check using accomplice's account age

Fix: Verify the GitHub user ID from the API response matches the
providerAccountId stored in authAccounts during OAuth. This ID is
immutable and tied to the actual GitHub account.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vercel
Copy link
Contributor

vercel bot commented Feb 3, 2026

@mkrokosz is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Address code review feedback:
- If storedGithubId exists but GitHub API doesn't return id, reject
  rather than silently allowing (fail closed vs fail open)
- Add afterEach for timer cleanup in tests
- Add test case for fail-closed behavior

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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