Skip to content

fix(official): require verified ownership for newly allowlisted official org handles#2481

Open
BunsDev wants to merge 4 commits into
mainfrom
codex/fix-official-publisher-handle-squatting
Open

fix(official): require verified ownership for newly allowlisted official org handles#2481
BunsDev wants to merge 4 commits into
mainfrom
codex/fix-official-publisher-handle-squatting

Conversation

@BunsDev

@BunsDev BunsDev commented Jun 3, 2026

Copy link
Copy Markdown
Member

Summary

  • What changed: Added reserved-owner verification and a dedicated org-level check by introducing isReservedOwnerVerifiedOfficialOrgHandle and isOfficialOrgPublisher and consulting them from isOfficialPublisher in convex/lib/officialPublishers.ts. Blocked self-serve org creation for reserved-owner-verified official handles in createOrgPublisherForUser unless the handle is already reserved for the actor. Exposed a DbCtx-friendly getLatestActiveReservedHandle helper in convex/lib/reservedHandles.ts. Added regression tests in convex/lib/officialPublishers.test.ts covering org-level and personal-publisher membership cases for nvidia, and added a spec update in specs/official-publishers.md.
  • Why: A new allowlist entry (e.g. nvidia) made Official status rely solely on the mutable publisher handle, which allowed handle-squatting and unauthorized Official package publishing. The change binds Official status to a verified owner/reservation so unclaimed or attacker-created orgs cannot acquire Official privileges by registering the handle.

Linked Issue

Screenshots

  • Screenshots/recordings attached, or N/A

Behavioural Proof

  • Require an active reservedHandles record whose rightfulOwnerUserId owns the org before treating a newly allowlisted handle (e.g. nvidia) as Official, both for org publishers and for personal publishers deriving Official status via org membership.

  • Block self-serve org creation for reserved-owner-verified official handles unless the actor is the rightful owner.

  • Regression tests confirm: unreserved nvidia org → not official; reserved+rightful-owner-owned nvidia org → official; personal publisher member of unreserved nvidia org → not official; personal publisher member of reserved+rightful-owner-owned nvidia org → official.

  • Behavioural proof included, or N/A

Security / Trust Impact

  • Security/trust impact explained

Prevents handle-squatting on newly allowlisted official handles (e.g. nvidia). Official status for these handles is now gated on an admin-created reservedHandles record binding the handle to a specific rightfulOwnerUserId, and self-serve org creation for those handles is blocked for non-rightful-owners.

Data / Deploy Impact

  • No data/deploy impact

Verification

  • bun run ci:static
  • Focused tests for touched behavior: bunx vitest run convex/lib/officialPublishers.test.ts convex/publishers.test.ts --reporter verbose — all passed
  • bun run ci:unit or N/A for docs/config-only: bun run ci:unit passed
  • Broader gate when required (ci:types-build, ci:packages, ci:e2e-http, ci:playwright-smoke, test:pw:local-auth, proof:ui): bun run ci:types-build passed
  • Other: bun run format:check && bun run lint passed; bun run ci:static and bun run deadcode:ci could not fully run in this environment due to bun audit and knip/registry access issues (environment/tooling limits, not regressions)

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
clawhub Error Error Jun 3, 2026 12:13pm

@clawsweeper

clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 24, 2026, 9:25 PM ET / 01:25 UTC.

Summary
The PR adds reservation-based Official publisher checks for nvidia, blocks unreserved official-handle org creation, expands route reservations, updates tests/specs, and adds package-lock.json.

Reproducibility: not applicable. as a user issue reproduction path because this is a PR review. The blockers are source-reproducible from the PR head diff, current main policy/spec, and live dirty merge state.

Review metrics: 3 noteworthy metrics.

  • Diff size: +11528/-26 across 8 files. Most changed lines are lockfile churn rather than the authorization fix under review.
  • Npm lockfile: 1 added file, 391057 bytes and 11154 lines. A second package-manager lockfile can change install and preview deployment behavior in this Bun-only repository.
  • Live merge state: dirty/not mergeable. Maintainers need a resolved branch before judging the real merge result against current main.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase onto current main and preserve the exact-row Official publisher model unless maintainers explicitly approve a policy replacement.
  • Remove package-lock.json and rerun deploy/static-relevant checks.
  • Resolve route-reservation conflicts while keeping current clawhub, creators, and OpenClaw extension alias protections.

Risk before merge

  • [P1] Conflict resolution could replace current main's exact-row officialPublishers authorization model with handle/reservation-derived org and personal publisher Official status.
  • [P1] The added package-lock.json changes package-manager and deployment surface in a Bun-only repository and is associated with failed preview deployment feedback.
  • [P1] The route-reservation change predates current main and must preserve clawhub, creators, and OpenClaw extension alias reservations.
  • [P1] The PR is currently dirty, so the actual three-way merge result cannot be reviewed yet.

Maintainer options:

  1. Rebase as a row-model patch (recommended)
    Resolve conflicts on current main, drop the lockfile, and preserve exact officialPublishers rows as the source of truth while keeping only compatible cleanup.
  2. Split route reservations only
    If current main already covers the Official hardening, pause this branch and land a narrow route-reservation PR that preserves current protected handles and tests.
  3. Approve a policy replacement explicitly
    Maintainers can intentionally continue the reservation-derived Official model only after documenting why it should replace or supplement exact rows and how the trust boundary remains safe.

Next step before merge

  • [P2] Member-authored security-boundary work is dirty and requires maintainer-owned policy/conflict resolution rather than an automated repair lane.

Security
Needs attention: The intended hardening is valuable, but the current diff has authorization-policy, route-reservation, and package-manager/deploy-surface concerns before merge.

Review findings

  • [P1] Rebase onto the row-scoped Official model — convex/lib/officialPublishers.ts:74-83
  • [P1] Remove the npm lockfile — package-lock.json:1
  • [P2] Preserve current route reservations during rebase — convex/lib/publicRouteReservations.ts:10-45
Review details

Best possible solution:

Preserve current main's exact-row officialPublishers model, remove package-lock.json, and either rebase only the compatible route-reservation cleanup or get explicit maintainer approval for any Official policy replacement.

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

Not applicable as a user issue reproduction path because this is a PR review. The blockers are source-reproducible from the PR head diff, current main policy/spec, and live dirty merge state.

Is this the best way to solve the issue?

No. Current main's exact-row officialPublishers model is the narrower maintainable security policy, so this branch needs rebase/split work or explicit maintainer approval for a policy replacement.

Full review comments:

  • [P1] Rebase onto the row-scoped Official model — convex/lib/officialPublishers.ts:74-83
    Current main derives Official status only from exact officialPublishers rows, and the spec says org membership does not confer Official status. This branch reintroduces handle/reservation-derived Official status and personal publisher inheritance, so preserve the row-scoped model or get explicit maintainer approval for changing that policy.
    Confidence: 0.93
  • [P1] Remove the npm lockfile — package-lock.json:1
    This repository tracks bun.lock and enforces Bun with preinstall; adding package-lock.json introduces a second package-manager lockfile and can change install/deploy behavior. Drop this file before merge.
    Confidence: 0.97
  • [P2] Preserve current route reservations during rebase — convex/lib/publicRouteReservations.ts:10-45
    Current main reserves handles such as clawhub and creators and also protects OpenClaw extension aliases, but the branch's replacement route-reservation list predates those entries. Resolve the conflict so current main's protected handles remain reserved.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3ef5f14d84a7.

Label changes

Label justifications:

  • P1: The PR touches the Official publisher trust boundary and has merge-blocking compatibility, security-boundary, and deploy-surface concerns.
  • merge-risk: 🚨 compatibility: Conflict resolution could replace current main's exact-row Official model or lose current public-route owner reservations.
  • merge-risk: 🚨 security-boundary: Official publisher status is a trust signal and package publishing boundary, so changing how it is derived needs explicit maintainer approval.
  • merge-risk: 🚨 automation: The added npm lockfile changes install/deploy behavior in a Bun-only repo and coincides with failed preview deployment feedback.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Not applicable because the PR author is a repository member; regression tests are useful but the external-contributor proof gate does not apply.
Evidence reviewed

Security concerns:

  • [high] Official status policy conflicts with current main — convex/lib/officialPublishers.ts:74
    The branch derives Official status from handle/reservation checks and org membership while current main uses exact officialPublishers rows, so accepting it without explicit approval could weaken or confuse the publishing trust boundary.
    Confidence: 0.91
  • [medium] Accidental npm lockfile changes install surface — package-lock.json:1
    package-lock.json adds npm registry resolution data and a second package-manager lockfile in a Bun-only repo, which can change install and preview deployment behavior.
    Confidence: 0.95
  • [medium] Route reservation drift could reopen protected handles — convex/lib/publicRouteReservations.ts:10
    The branch route-reservation list predates current main's clawhub, creators, and extension-alias reservations, so conflict resolution must preserve those protections.
    Confidence: 0.84

What I checked:

  • Repository policy read: AGENTS.md was read in full, including Convex/spec/security guidance and the protected maintainer-authored item rule. (AGENTS.md:1, 3ef5f14d84a7)
  • Convex guidance read: The required Convex AI guidelines were read before reviewing Convex code. (convex/_generated/ai/guidelines.md:1, 3ef5f14d84a7)
  • Current main Official source of truth: Current main derives Official status by querying officialPublishers for the exact publisher id, not by handle allowlists or org membership. (convex/lib/officialPublishers.ts:9, 3ef5f14d84a7)
  • Current main Official policy spec: The spec says Official is publisher-scoped and not inherited by users, personal publishers, org members, GitHub identities, OIDC trust, or trustedPublisher. (specs/official-publishers.md:3, 3ef5f14d84a7)
  • Current main admin row API: Admin-only mutations add exact officialPublishers rows by publisher id and audit the change, which is the supported marking path on current main. (convex/publishers.ts:2855, 3ef5f14d84a7)
  • PR branch reintroduces handle-derived Official status: The PR head defines legacy/reserved Official handle sets and makes org and personal publisher Official status depend on handle, reservation, and membership checks. (convex/lib/officialPublishers.ts:69, 4a4ad1634c9e)

Likely related people:

  • Patrick Erichsen: Recent history includes NVIDIA Official publisher work, exact-row Official policy follow-ups, publisher abuse integration, and route-reservation updates in the central files. (role: recent area contributor; confidence: high; commits: 9a20795b5432, 9cf9e12bdd95, 9cbf98297c9d; files: convex/lib/officialPublishers.ts, specs/official-publishers.md, convex/publishers.ts)
  • Jesse Merhi: History shows earlier work simplifying the Official publisher policy that this branch now conflicts with. (role: introduced behavior; confidence: medium; commits: 90de729fe135; files: convex/lib/officialPublishers.ts, specs/official-publishers.md)
  • Vincent Koc: Recent route-alias protection work touched publicRouteReservations, which this PR must preserve during conflict resolution. (role: recent adjacent area contributor; confidence: medium; commits: 9b352d7f9900; files: convex/lib/publicRouteReservations.ts, convex/lib/publicRouteReservations.test.ts, packages/schema/src/openClawExtensionSlugs.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

Copilot AI 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.

Pull request overview

This PR hardens how ClawHub derives the official publisher flag by ensuring newly-allowlisted official org handles can’t become Official solely by claiming a mutable handle; instead they must be tied to an active reserved-handle record and verified ownership.

Changes:

  • Split official org handles into legacy vs reserved-owner-verified lists and require reservation + rightful-owner org ownership for reserved-owner-verified handles.
  • Block self-serve org creation for reserved-owner-verified official handles unless the actor holds the active reservation.
  • Expose a DbCtx-friendly getLatestActiveReservedHandle helper and add/adjust regression tests + spec wording.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
specs/official-publishers.md Updates the policy spec to distinguish legacy official orgs vs reservation/ownership-verified official orgs.
convex/publishers.ts Blocks self-serve creation of reserved-owner-verified official org handles unless reserved for the actor.
convex/publishers.test.ts Adds regression tests for reserved-owner-verified handle creation behavior.
convex/lib/reservedHandles.ts Adds a DbCtx-compatible helper for fetching the latest active reservation.
convex/lib/officialPublishers.ts Enforces reservation + rightful-owner ownership checks for reserved-owner-verified official org handles and personal derivation.
convex/lib/officialPublishers.test.ts Adds org-level tests for reserved-owner verification logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread convex/lib/officialPublishers.ts Outdated
).resolves.toBe(false);
});

it("treats personal publishers for openclaw org members as official", async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added in commit $(git -C /tmp/workspace/openclaw/clawhub rev-parse --short HEAD): two new tests cover the personal-publisher membership path for nvidia:

  • "does not treat personal publisher of unreserved nvidia org member as official" — member of an org that has no reservedHandles record resolves to false.
  • "treats personal publisher of reserved-owner-controlled nvidia org member as official" — member of an org whose rightfulOwnerUserId holds the "owner" role resolves to true.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added in commit cb53d4ab: two new tests cover the personal-publisher membership path for nvidia:

  • "does not treat personal publisher of unreserved nvidia org member as official" — member of an org with no reservedHandles record resolves to false.
  • "treats personal publisher of reserved-owner-controlled nvidia org member as official" — member of an org whose rightfulOwnerUserId holds the "owner" role resolves to true.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 3, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 3, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 3, 2026
Comment thread package-lock.json
@@ -0,0 +1,11154 @@
{

@vercel vercel Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Accidentally committed package-lock.json causes Vercel to switch from bun to npm, failing the preinstall check

Fix on Vercel

Previously only 'plugins' and 'skills' were protected. Any user could
register @admin, @settings, @dashboard, @search, etc. as a publisher
handle, shadowing those platform routes via the $owner catch-all.

Add all current top-level src/routes/ segments to
RESERVED_PUBLIC_OWNER_HANDLES, grouped by purpose:
- Content browsing: skills, souls, plugins, packages, publishers, orgs
- Profile shortlinks: p, u
- User flows: search, import, upload, publish-skill, publish-plugin,
  stars, dashboard, settings
- Admin/internal: admin, management, audits
- Informational: docs, cli
- Auth/account: user, users

Also add a doc comment noting to update this set when new top-level
routes are added.
@clawsweeper clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jun 3, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants