openclaw/clawhub
1
0
Fork 0
mirror of https://github.com/openclaw/clawhub.git synced 2026-07-22 03:35:29 -04:00
Code Issues 183 Packages Projects Releases 1 Wiki Activity

[PR #116] [MERGED] fix: gate publish by immutable GitHub account ID #263

New Issue
Closed
opened 2026-02-15 17:16:33 -05:00 by yindo · 0 comments
yindo commented 2026-02-15 17:16:33 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/openclaw/clawhub/pull/116
Author: @mkrokosz
Created: 2/3/2026
Status: ✅ Merged
Merged: 2/14/2026
Merged by: @steipete

Base: main ← Head: security/github-username-swap-attack

📝 Commits (4)

  • 3e5ee01 security: prevent GitHub username swap attack on 7-day account age check
  • 82e8a93 fix: fail closed when GitHub API omits user ID
  • 116e8e4 chore: merge origin/main
  • d5ddb2c docs: changelog for upload gate fix

📊 Changes

9 files changed (+159 additions, -72 deletions)

View changed files

📝 CHANGELOG.md (+1 -0)
📝 convex/_generated/api.d.ts (+6 -0)
📝 convex/httpApiV1.ts (+2 -2)
📝 convex/lib/githubAccount.test.ts (+120 -54)
📝 convex/lib/githubAccount.ts (+14 -8)
📝 convex/schema.ts (+1 -3)
📝 convex/tsconfig.json (+1 -1)
📝 convex/users.ts (+11 -1)
📝 docs/security.md (+3 -3)

📄 Description

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)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/openclaw/clawhub/pull/116 **Author:** [@mkrokosz](https://github.com/mkrokosz) **Created:** 2/3/2026 **Status:** ✅ Merged **Merged:** 2/14/2026 **Merged by:** [@steipete](https://github.com/steipete) **Base:** `main` ← **Head:** `security/github-username-swap-attack` --- ### 📝 Commits (4) - [`3e5ee01`](https://github.com/openclaw/clawhub/commit/3e5ee015df1bcd56b25f690a0a7efb7926e6e01d) security: prevent GitHub username swap attack on 7-day account age check - [`82e8a93`](https://github.com/openclaw/clawhub/commit/82e8a93d94e934cff7123045d2dad8203ef5a740) fix: fail closed when GitHub API omits user ID - [`116e8e4`](https://github.com/openclaw/clawhub/commit/116e8e448a683d5745e83818cbddaba54f29b6ab) chore: merge origin/main - [`d5ddb2c`](https://github.com/openclaw/clawhub/commit/d5ddb2cd4f34b356691a7e0b7daee2867fff6c1e) docs: changelog for upload gate fix ### 📊 Changes **9 files changed** (+159 additions, -72 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+1 -0) 📝 `convex/_generated/api.d.ts` (+6 -0) 📝 `convex/httpApiV1.ts` (+2 -2) 📝 `convex/lib/githubAccount.test.ts` (+120 -54) 📝 `convex/lib/githubAccount.ts` (+14 -8) 📝 `convex/schema.ts` (+1 -3) 📝 `convex/tsconfig.json` (+1 -1) 📝 `convex/users.ts` (+11 -1) 📝 `docs/security.md` (+3 -3) </details> ### 📄 Description ## 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](https://claude.com/claude-code) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> 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. <h3>Confidence Score: 4/5</h3> - 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 <!-- greptile_other_comments_section --> <sub>(4/5) You can add custom instructions or style guidelines for the agent [here](https://app.greptile.com/review/github)!</sub> **Context used:** - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=a1d58d20-b4dd-4cbb-973a-9fd7824e1921)) <!-- /greptile_comment --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-02-15 17:16:33 -05:00
yindo closed this issue 2026-02-15 17:16:33 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
pull-request
Mirrored from GitHub Pull Request
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: openclaw/clawhub#263
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?