fix(security): enforce current organization skill ownership

[Patrick-Erichsen]

Former publishers retained authority over organization skills through the historical ownerUserId. Transfer requests and delete/restore now use current publisher authorization, and transfer acceptance rechecks the requester's authority after membership removal or downgrade.

Addresses GHSA-9558-q4f9-324f. The advisory is published with the deployed revision and regression evidence.

Verification

• Before: regression tests against cbfee7343ddc867316dd9b3de6fa8856730f9f41 reproduced successful former-publisher transfer requests, acceptance after revocation/downgrade, and delete/restore.
• After: 40 focused tests pass, including current organization-admin and personal-owner compatibility coverage.
• Real local Convex backend at http://127.0.0.1:3320, using synthetic organization/member/recipient fixtures: transfer request denied; pending acceptance cancelled; delete denied; restore denied; organization ownership preserved. No production data or accounts were exercised.
• bun run ci:static passed.
• bun run ci:unit passed: 6,627 tests; 497 passing files, 1 skipped.
• bun run ci:types-build passed, including root, schema, CLI, admin TypeScript and production build.
• bunx convex dev --once --codegen disable pushed and typechecked the candidate successfully on the disposable local backend.
• .agents/skills/autoreview/scripts/autoreview --mode local --no-web-search: clean, no actionable findings.

Best-fix verdict: best. Reusing the shared publisher authorization keeps organization membership authoritative without changing personal-owner or staff permissions. Changing stored historical user IDs would not revalidate pending requests and would require a data migration.

Combined release validation: ci:static, ci:unit (6,656 passing tests), and ci:types-build passed on f3cd9104981d1875cc2945418172837297afcb5d. The protected Test frontend passed four HTTP checks, two browser checks, and an archive download check.

Deployment verified

Merged and deployed in 8c2de6c506bb4efabe3f0c2ffb8370b9e23d4650. Test deployment and Production deployment succeeded for that exact SHA. Live reads/downloads/image rendering and browser smoke tests pass; forged ingress is rejected. The active Production catalog rollout was restored after backend deployment. The superseded private security-fork PR is closed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
clawhub	Building	Preview	Sep 11, 2026 3:43pm UTC

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 11:47 AM ET / 15:47 UTC.

ClawSweeper review

What this changes

Require current organization ownership permissions for skill transfers, deletion, and restoration, including rechecking pending transfers after membership changes.

Merge readiness

✅ Ready for maintainer review

Keep open: current main still contains the authorization bypass, and this collaborator-authored PR provides a focused repair. No blocking patch findings were identified.

Priority: P0
Reviewed head: d710c4ba210cb196065a8da0d4c9cf757b7f12e3

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused security repair with regression coverage, preserved compatibility paths, and no blocking correctness findings.
Proof confidence	🌊 off-meta tidepool	Not applicable: The collaborator exemption applies. The body reports real local Convex rejection and cancellation results for the changed mutations; source and regression coverage place authorization before final writes, with no material introduced authority uncertainty requiring the exception proof gate.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The collaborator exemption applies. The body reports real local Convex rejection and cancellation results for the changed mutations; source and regression coverage place authorization before final writes, with no material introduced authority uncertainty requiring the exception proof gate.
Evidence reviewed	8 items	Verified review boundary: The complete introduced delta contains two production files, one regression-test file, and one security specification. REST metadata confirms the PR remains open and unmerged with the supplied base and head. Current main still needs the repair: Main permits the historical ownerUserId to bypass organization authorization when requesting a transfer and skips requester reauthorization on acceptance when that historical ID matches. Published-version check: The v0.23.3 transfer implementation also contains the historical-owner shortcut; the latest supplied release does not eliminate the need for this repair.
Findings	None	None.
Security	None	None.

How this fits together

ClawHub’s backend receives authenticated skill-management requests from its website and CLI. Publisher authorization determines whether those requests may change skill ownership or marketplace visibility.

flowchart TD
  A[Authenticated management request] --> B[Resolve skill and actor]
  B --> C[Check current publisher permissions]
  C --> D[Reject unauthorized request]
  C --> E[Create pending transfer]
  E --> F[Recheck requester on acceptance]
  F --> G[Update ownership or cancel transfer]
  C --> H[Apply guarded delete or restore]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch size	+173/-24 across 4 files	The patch stays within ownership authorization, regression coverage, and its security contract.
Production versus tests	Production +20/-24; tests +144/-0; specification +9/-0	Production code shrinks while regression coverage exercises revoked authority and allowed operations.

Technical review

Best possible solution:

Keep current publisher membership authoritative at each mutation boundary while preserving personal-owner compatibility and existing moderation safeguards.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: retain an organization skill’s historical ownerUserId, revoke that user’s membership, then request a transfer or delete/restore the skill; main bypasses the membership check. This review did not execute the reproduction.

Is this the best way to solve the issue?

Yes—best fix: reuse the existing ownership helper at the final mutation boundaries. Rewriting historical owner IDs would require migration and would not itself revalidate pending transfers.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cbfee7343ddc.

Labels

Label changes:

• add P0: Current main permits former organization publishers to bypass authorization for ownership transfers and destructive lifecycle operations.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.

Label justifications:

• P0: Current main permits former organization publishers to bypass authorization for ownership transfers and destructive lifecycle operations.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.

Evidence

What I checked:

• Verified review boundary: The complete introduced delta contains two production files, one regression-test file, and one security specification. REST metadata confirms the PR remains open and unmerged with the supplied base and head. (d710c4ba210c)
• Current main still needs the repair: Main permits the historical ownerUserId to bypass organization authorization when requesting a transfer and skips requester reauthorization on acceptance when that historical ID matches. (convex/skillTransfers.ts:35, cbfee7343ddc)
• Published-version check: The v0.23.3 transfer implementation also contains the historical-owner shortcut; the latest supplied release does not eliminate the need for this repair. (convex/skillTransfers.ts:35, 87ca030c30f3)
• Authorization through final effects: Transfer acceptance checks current requester authority before creating the destination publisher or updating skill, alias, and embedding ownership. Delete/restore checks current authority before visibility, statistics, embedding, and audit writes. HTTP callers derive actors from authenticated API tokens. (convex/skillTransfers.ts:266, d710c4ba210c)
• Existing compatibility contract: The shared ownership helper preserves personal and legacy ownership, ranks organization owners above admins, and retains explicit platform-admin exemptions. Existing publisher tests reject stale organization ownerUserId access and cover linked and legacy personal publishers. (convex/lib/publishers.ts:130, d710c4ba210c)
• Focused regression coverage: Eight parameter-expanded cases cover removed or downgraded requesters, cancellation after revocation with organization ownership preserved, denied former-member delete/restore, and successful current-admin operations. These are convex-test cases, not independently observed live-backend execution. (convex/skillOwnershipAuthorization.runtime.test.ts:46, d710c4ba210c)

Likely related people:

• Patrick-Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• vyctorbrzezowski: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.