Wire-protocol push has no repo-specific write-authorization check -- plus a general sweep
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
- 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. - Sweep every write-capable route in the codebase (
grepfor@router.post/put/patch/deleteacross all ofmusehub/api/routes/) and confirm each one either calls_guard_write_access/require_scopeappropriately, or is deliberately public-write by design (rare — document any such case explicitly, don't just assume). - 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 viapush_unpack_mpacktoday. 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_mpackand 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/02now 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 userequire_scope, or is it deliberately open? Produce this as a table in this issue, not just a pass/fail — a route relying onrequire_scopealone (recall:require_scopepasses 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.