gabriel / musehub public
Open #131 Bug
filed by gabriel human · 101 days ago

Wire-protocol push has no repo-specific write-authorization check -- plus a general sweep

0 Anchors
— Blast radius
— Churn 30d
0 Proposals

Wire-protocol push has no repo-specific write-authorization check — plus a general sweep for the same class of bug

Background

While verifying aaronrene's collaborator access (write permission just granted on muse and musehub), traced the actual wire-protocol push handler end to end and found it has no repo-specific authorization check at all — a materially different, more serious problem than anything being checked at the time.

Confirmed, traced in code — not assumed

musehub/api/routes/wire.py::push_unpack_mpack (the real push handler) depends only on:

claims: TokenClaims = Depends(require_valid_token),

require_valid_token (= require_signed_request, musehub/auth/request_signing.py:289) verifies only that the request carries a valid MSign signature from some registered identity — it has no concept of which repo is being written to.

pusher_id/claims.handle is passed into the service function (wire_push_unpack_mpack, musehub/services/musehub_wire_push.py:539) but is never referenced again anywhere else in that 1707-line file — accepted as a parameter, never checked against anything.

There is no _assert_writable-style helper anywhere in wire.py — only _assert_readable (line 90), used for fetch/read paths. _guard_write_access — the exact per-repo collaborator/owner check musehub/api/routes/musehub/ proposals.py:116 and issues.py correctly call before allowing a write — appears in zero wire-protocol routes (confirmed via grep -rln _guard_write_access musehub/).

In plain terms: any authenticated MuseHub identity can currently push to any repo, public or private, regardless of collaborator status. This is unrelated to the collaborator-invite work that surfaced it — collaborator invites are still correct and necessary for MP creation and other REST-gated actions (which do call _guard_write_access correctly); push simply doesn't do what anyone would reasonably assume.

Why a sweep, not just a point fix

The REST entity routes (issues.py, proposals.py, labels.py, releases.py, repos.py) went through an explicit auth-hardening pass (Phase 3 of the "Quality Roadmap," 2026-04-07 — see ~/ecosystem/ QUALITY_ROADMAP.md) that added require_scope/_guard_write_access enforcement everywhere. wire.py is a separate, older, custom binary protocol router — it's plausible it was simply never included in that sweep, which raises the real question: are there other routers/endpoints with the same gap? This ticket exists to answer that definitively, not just patch the one confirmed hole.

Goal

  1. Fix the confirmed gap: push_unpack_mpack (and any sibling wire-protocol write endpoint — repair-object/snapshot/commit, force-push, tag push) enforces the same per-repo collaborator/owner check as the REST routes.
  2. Sweep every write-capable route in the codebase (grep for @router.post/ put/patch/delete across all of musehub/api/routes/) and confirm each one either calls _guard_write_access/require_scope appropriately, or is deliberately public-write by design (rare — document any such case explicitly, don't just assume).
  3. A regression test proves the wire push gap specifically: a collaborator with no write access (or no collaborator record at all) attempting to push must be rejected — today it would succeed.

Out of Scope

  • Redesigning the wire protocol's binary format or transport — this is an authorization gap, not a protocol design issue.
  • The org/quorum audit (musehub issue #130) — related in spirit (both are "is our permission model actually complete") but that's the CLI/ functionality-parity angle; this ticket is specifically about finding and closing authorization holes.

Phases

Phase 1 — Red: prove the gap with a real test

  • AUTHZ_01 — A registered identity with no collaborator record on a private repo successfully pushes via push_unpack_mpack today. Must pass (i.e., prove the vulnerability) against current code, then be inverted to a rejection assertion once fixed.
  • AUTHZ_02 — Same setup for a public repo — confirm whether public repos are even more exposed (anyone can push, not just anyone with an account) or whether some other check applies there.

Deliverable: the gap is proven with a real test, not just static analysis, before any fix lands.

Phase 2 — Fix the confirmed wire-protocol gap

  • Add a per-repo write-authorization check to push_unpack_mpack and every other wire-protocol write endpoint (repair-object/snapshot/ commit already partially check this per the earlier grep — verify those are actually correct and consistent, not just present).
  • AUTHZ_01/02 now correctly reject unauthorized pushes.

Deliverable: wire-protocol push is provably no longer bypassable.

Phase 3 — Full sweep

  • Enumerate every write route across musehub/api/routes/ (REST + wire + coord + mcp write-tools) and record, per route: does it check collaborator/owner access, does it use require_scope, or is it deliberately open? Produce this as a table in this issue, not just a pass/fail — a route relying on require_scope alone (recall: require_scope passes humans unconditionally, it's an agent-capability gate, not a repo-ownership gate) needs a second look, not just a checkbox.
  • Any additional gaps found get their own fix + test, following the same Phase 1→Phase 2 pattern used here.

Deliverable: a complete, documented map of every write route's actual authorization behavior — no more "plausible, unverified" routes.

Acceptance Criteria

  • push_unpack_mpack (and siblings) reject a push from a non-collaborator.
  • Every write route in musehub/api/routes/ has a documented, verified authorization story — not assumed correct because it "probably" calls the right dependency.
  • Full regression suite green; no privilege-escalation regressions introduced by the fix itself (e.g. don't accidentally lock out legitimate owners).

Implementation Order

Phase 1 → Phase 2 → Phase 3. Fix the confirmed, live gap first (it's on staging right now); the broader sweep comes after the acute issue is closed.

◎ Activity1
●
gabriel opened this issue 101 days ago
○
aaronrene 36 days ago

musehub#131 follow-on — collaborator write access is unreachable: accepted_at can never be set

Status: DRAFT for review before posting Date: 2026-09-10 Author: aaronrene Severity: blocks all collaborator writes on every MuseHub repo; regression introduced by the #131 authorization fix Relationship to #131: the authorization check added in security/131-wire-write-authz is correct. The gap is that the state it checks for has no setter.

Summary

Every write gate in MuseHub authorizes a non-owner by requiring an accepted collaborator row:

MusehubCollaborator.identity_handle == caller_id,
MusehubCollaborator.accepted_at.isnot(None),
MusehubCollaborator.permission.in_(["write", "admin"]),

accepted_at is documented as "Null until the invited user accepts the invitation."

No code path in MuseHub sets accepted_at. There is no accept route, no MCP write tool, and no CLI subcommand. invite_collaborator creates the row with invited_at=now and omits accepted_at entirely, so it is NULL on creation and stays NULL forever.

Net effect: an invited collaborator — at any permission level, including admin — can never obtain write access to any MuseHub repo. Only repo owners can write.

Observed

aaronrene is an admin collaborator on gabriel/musehub:

$ muse hub collaborator list
  aaronrene                         admin

$ muse hub collaborator list --json
  { "handle": "aaronrene", "permission": "admin", "invitedBy": "gabriel" }

Pushing a new, non-shared backup branch is rejected:

$ muse push origin backup/aaron-local-dev-2026-09-01
[PUSH step 3]   { commits: 55, snapshots: 54, blobs: 26 }
[PUSH step 5] POST /push/mpack-presign { mpack_key, size_bytes }
❌ Push failed: HTTP 403:
   {"detail":"only the repo owner or a write/admin collaborator may write to this repo"}

The 403 is correct behavior given the data: the row's accepted_at is NULL, so the check cannot pass.

Why there is no setter (verified on origin/dev @ 8a1389ae)

The collaborators router defines exactly four routes:

Line Route Handler
112 GET list_collaborators
139 POST invite_collaborator
226 PUT update_collaborator_permission
295 DELETE remove_collaborator

There is no accept route. Invitation creation never sets the field:

new_collab = MusehubCollaborator(
    id=compute_collaborator_id(repo_id, invitee_identity.identity_id, now.isoformat()),
    repo_id=repo_id,
    identity_handle=body.handle,
    permission=body.permission.value,
    invited_by_handle=actor,
    invited_at=now,
)

A tree-wide search for any write to accepted_at (excluding the unrelated MusehubIdentity.tos_accepted_at) finds only two occurrences, both of which are reads or the synthetic owner record:

  • repos.py:1481 — accepted_at=None for the synthesized owner entry
  • repos.py:1502 — accepted_at=collab.accepted_at passthrough on read

muse hub collaborator exposes only list, invite, update-permission, remove — no accept.

Blast radius

Everything gated on accepted-collaborator status is unreachable for non-owners:

Surface File
Wire-protocol push api/routes/wire.py:116, :144
Wire push service (3 gates) services/musehub_wire_push.py:152, :223, :301
Proposal review / merge api/routes/musehub/proposals.py:94
Issue close / reopen / assign / label / delete-comment api/routes/musehub/issues.py:83
Labels api/routes/musehub/labels.py:103
Webhooks api/routes/musehub/webhooks.py:39
Collaborator invite / update / remove api/routes/musehub/collaborators.py:169, :255, :322
Repository writes services/musehub_repository.py:322, :434
MCP write executor services/musehub_mcp_executor.py:1448

What still works, because public repos take a different path: reading, cloning, creating issues, and commenting. issues.py::_guard_write_access only restricts non-owners on private repos, so on a public repo any authenticated identity may create issues, comments, and proposals.

The MP flow is still blocked end to end, because opening a proposal requires first pushing a branch, and push is exactly what 403s.

Regression timeline

This is new as of 2026-09-09. Before the #131 fix the wire push path had no repo-scoped write-authorization check at all — which is the vulnerability #131 correctly identified and closed. My feature branches (feat/f7b-http-coverage @ 82eef2e1, feat/kd-6b @ c04414e7) are on the server precisely because pushes succeeded under the pre-fix behavior.

Adding the check without adding an accept path flipped collaborators from "can push, unauthorized" to "cannot push at all." Both states are wrong; the second is the safer of the two, and it is a one-endpoint fix.

Deadlock worth flagging

I cannot deliver this fix through the normal flow. Opening an MP requires pushing a branch, and pushing requires the write access this bug removes. So the unblock has to come from the owner side.

Proposed fix

Immediate unblock (owner action, one row per collaborator): set accepted_at on the existing aaronrene rows for gabriel/musehub and gabriel/muse. This restores the handover without any code change.

Proper fix (small, and it closes the design gap):

  1. Add POST /repos/{repo_id}/collaborators/accept — callable only by the invited identity itself, setting accepted_at = now() where it is currently NULL. Idempotent.
  2. Add muse hub collaborator accept to the CLI, plus surface pending invitations in collaborator list output (the current JSON omits acceptedAt, which is why an unaccepted admin row looks fully provisioned).
  3. Decide explicitly whether an owner may accept on a collaborator's behalf. I would say no — acceptance should stay an affirmative act by the invitee — with the owner-side unblock above handled as a one-off data fix rather than a permanent endpoint.
  4. Tests: invited-but-unaccepted is denied write; accepted is allowed; accept is idempotent; a third party cannot accept someone else's invitation; accepted_at survives a permission update.

Suggested seven-tier coverage

Tier Coverage
Unit accept sets timestamp only when NULL; idempotent; permission unchanged
Integration invite → accept → push succeeds; invite → push 403
E2E full MP flow as an accepted admin collaborator: push branch, open proposal, merge
Stress concurrent accepts on one invitation produce one row, one timestamp
Data-integrity accepted_at preserved across update-permission; owner synthetic row unaffected
Performance authorization check stays a single indexed lookup
Security only the invitee may accept; no cross-repo or cross-identity acceptance; revoked/removed collaborator cannot re-accept; unaccepted admin has zero write capability on every gate listed above

Evidence index

Claim How verified
aaronrene is admin on gabriel/musehub muse hub collaborator list --json
Push is rejected 403 muse push origin backup/aaron-local-dev-2026-09-01
No accept route exists Route enumeration of collaborators.py at origin/dev 8a1389ae
Invite never sets accepted_at Source of invite_collaborator at same ref
No setter anywhere Tree-wide search for writes to accepted_at
Gates requiring acceptance Enumerated above with file:line
Public repos still allow comments issues.py::_guard_write_access at same ref
gabriel/musehub is public muse hub repo read --json → visibility: public
Pushes worked pre-fix Author's feature branch tips present on server