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

[PR #180] [MERGED] fix: handle duplicate Convex Auth user records in publish ownership check #284

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

📋 Pull Request Information

Original PR: https://github.com/openclaw/clawhub/pull/180
Author: @emmet-bot
Created: 2/8/2026
Status: ✅ Merged
Merged: 2/14/2026
Merged by: @steipete

Base: main ← Head: fix/publish-owner-check

📝 Commits (3)

  • ad42a5a fix: handle duplicate user records in publish ownership check
  • 64abafe fix: heal publish ownership via GitHub auth identity
  • 56623bb Merge remote-tracking branch 'origin/main' into fix/publish-owner-check

📊 Changes

4 files changed (+89 additions, -21 deletions)

View changed files

➕ convex/lib/githubIdentity.test.ts (+19 -0)
➕ convex/lib/githubIdentity.ts (+22 -0)
📝 convex/skills.ts (+47 -20)
📝 packages/clawdhub/src/skills.test.ts (+1 -1)

📄 Description

Problem

The CLI publish command fails with "Only the owner can publish updates" even when the authenticated user IS the owner.

Root Cause

In convex/skills.ts around line 2394, the insertVersion mutation compares skill.ownerUserId !== userId using Convex document IDs.

When Convex Auth creates duplicate user records for the same GitHub account (e.g., after session expiry and re-auth), the API token's userId (from the old user record) won't match the skill's ownerUserId (potentially updated to a new user record), or vice versa.

This results in legitimate owners being blocked from publishing updates to their own skills.

Solution

This PR adds a fallback ownership check that compares GitHub handles (usernames) instead of Convex user IDs:

  1. If the direct ID comparison fails, fetch both user records
  2. Compare their handle fields (GitHub username = stable identity)
  3. If handles match, allow the operation and heal the data by updating ownerUserId to the current active user ID
  4. If handles don't match (or users not found), reject as before

Benefits

  • Fixes the immediate bug: Owners can now publish updates even with duplicate user records
  • Self-healing: Automatically migrates ownership to the current active user ID
  • Maintains security: Still blocks unauthorized users (different GitHub accounts)
  • Backward compatible: Direct ID matches still work instantly (no extra queries)

Testing

This fix handles the edge case where Convex Auth has created duplicate user records for the same GitHub account, while maintaining the existing security model for all other cases.

Greptile Overview

Greptile Summary

This PR updates the insertVersion internal mutation in convex/skills.ts to tolerate duplicate Convex Auth user records during ownership checks. When skill.ownerUserId !== args.userId, it now falls back to loading both user docs and comparing their handle fields (GitHub username). If the handles match, it allows publishing and “self-heals” by patching the skill’s ownerUserId to the caller’s current user ID; otherwise it preserves the existing rejection (“Only the owner can publish updates”).

Confidence Score: 4/5

  • Mostly safe to merge, but the fallback ownership path should preserve the existing deleted-user invariants.
  • Change is small and scoped to the ownership check, but the new handle-based fallback bypasses the earlier deletedAt guard and can reassign ownership based on a deleted duplicate record if handles match. Fixing that check would align behavior with the existing security model.
  • convex/skills.ts

(2/5) Greptile learns from your feedback when you react with thumbs up/down!

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/180 **Author:** [@emmet-bot](https://github.com/emmet-bot) **Created:** 2/8/2026 **Status:** ✅ Merged **Merged:** 2/14/2026 **Merged by:** [@steipete](https://github.com/steipete) **Base:** `main` ← **Head:** `fix/publish-owner-check` --- ### 📝 Commits (3) - [`ad42a5a`](https://github.com/openclaw/clawhub/commit/ad42a5a7af3e8ab0015e1f0bae9754fd3811239c) fix: handle duplicate user records in publish ownership check - [`64abafe`](https://github.com/openclaw/clawhub/commit/64abafe2a0ef4993202c6badfaa14ff329636f82) fix: heal publish ownership via GitHub auth identity - [`56623bb`](https://github.com/openclaw/clawhub/commit/56623bb3f73e5cc861943b80f10f45ef12334712) Merge remote-tracking branch 'origin/main' into fix/publish-owner-check ### 📊 Changes **4 files changed** (+89 additions, -21 deletions) <details> <summary>View changed files</summary> ➕ `convex/lib/githubIdentity.test.ts` (+19 -0) ➕ `convex/lib/githubIdentity.ts` (+22 -0) 📝 `convex/skills.ts` (+47 -20) 📝 `packages/clawdhub/src/skills.test.ts` (+1 -1) </details> ### 📄 Description ## Problem The CLI publish command fails with "Only the owner can publish updates" even when the authenticated user IS the owner. ## Root Cause In `convex/skills.ts` around line 2394, the `insertVersion` mutation compares `skill.ownerUserId !== userId` using Convex document IDs. When Convex Auth creates duplicate user records for the same GitHub account (e.g., after session expiry and re-auth), the API token's `userId` (from the old user record) won't match the skill's `ownerUserId` (potentially updated to a new user record), or vice versa. This results in legitimate owners being blocked from publishing updates to their own skills. ## Solution This PR adds a fallback ownership check that compares GitHub handles (usernames) instead of Convex user IDs: 1. If the direct ID comparison fails, fetch both user records 2. Compare their `handle` fields (GitHub username = stable identity) 3. If handles match, allow the operation and heal the data by updating `ownerUserId` to the current active user ID 4. If handles don't match (or users not found), reject as before ## Benefits - **Fixes the immediate bug**: Owners can now publish updates even with duplicate user records - **Self-healing**: Automatically migrates ownership to the current active user ID - **Maintains security**: Still blocks unauthorized users (different GitHub accounts) - **Backward compatible**: Direct ID matches still work instantly (no extra queries) ## Testing This fix handles the edge case where Convex Auth has created duplicate user records for the same GitHub account, while maintaining the existing security model for all other cases. <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR updates the `insertVersion` internal mutation in `convex/skills.ts` to tolerate duplicate Convex Auth user records during ownership checks. When `skill.ownerUserId !== args.userId`, it now falls back to loading both user docs and comparing their `handle` fields (GitHub username). If the handles match, it allows publishing and “self-heals” by patching the skill’s `ownerUserId` to the caller’s current user ID; otherwise it preserves the existing rejection (“Only the owner can publish updates”). <h3>Confidence Score: 4/5</h3> - Mostly safe to merge, but the fallback ownership path should preserve the existing deleted-user invariants. - Change is small and scoped to the ownership check, but the new handle-based fallback bypasses the earlier `deletedAt` guard and can reassign ownership based on a deleted duplicate record if handles match. Fixing that check would align behavior with the existing security model. - convex/skills.ts <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</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:40 -05:00
yindo closed this issue 2026-02-15 17:16:40 -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#284
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?