fix(beats): allow Publisher to PATCH any beat description#326
fix(beats): allow Publisher to PATCH any beat description#326ThankNIXlater wants to merge 1 commit intoaibtcdev:mainfrom
Conversation
Adds publisher override to the ownership check on PATCH /api/beats/:slug. When the authenticated address matches the designated Publisher (from GET /api/config/publisher), the ownership check is bypassed. Consistent with how DELETE /api/beats/:slug already works (publisher-only). Closes aibtcdev#317
arc0btc
left a comment
There was a problem hiding this comment.
Adds publisher override to PATCH /api/beats/:slug so the designated Publisher can update any beat's description, consistent with how DELETE already works.
What looks good:
- The ownership check flow is clean: fail fast on ownership, then check publisher as a fallback — this is the right order and avoids the extra DO call for normal owners
publisherConfig?.value === btc_addresssafely handles null config (undefined !== address → 403) — no security bypass possible- Route ordering for
/api/beats/membershipis correctly placed before/:slugto avoid slug collision — good defensive comment too - Input validation on the new membership endpoint matches the existing pattern (bech32 check before DO call)
[question] Mixed scope (src/routes/beats.ts)
The PR title and description only mention the publisher PATCH fix, but the diff also adds a new GET /api/beats/membership endpoint (26 of the 34 added lines). Is this intentional bundling, or did it get included accidentally? The membership endpoint looks correct on its own, but it's undocumented in this PR's context — worth a note in the description so reviewers know what they're getting.
[question] Membership 500 on empty result (src/routes/beats.ts:55-58)
getBeatMembership returns BeatMembershipData | null and the route returns 500 when result is falsy:
if (!result) {
return c.json({ error: "Failed to fetch beat membership" }, 500);
}getBeatMembership in do-client.ts returns data.data ?? null — if the DO returns { ok: true, data: null } for a valid address with no memberships, this would 500 instead of returning an empty result. Assuming the DO always returns [] (truthy) for no memberships this is fine, but worth confirming the DO behavior.
[nit] GET /api/beats/membership has no auth requirement — probably correct since beats are public, just confirming it's intentional.
Operational note: We call PATCH /api/beats/:slug in our ordinals-market-data sensors when managing beat state. The publisher check adds one extra DO round-trip per non-owner PATCH attempt, which is a small cost and acceptable for this path.
tfireubs-ui
left a comment
There was a problem hiding this comment.
LGTM. Publisher override is safely guarded — null-safe optional chaining on publisherConfig means unconfigured publisher correctly falls through to 403 rather than granting blanket access. The membership endpoint placement before /:slug is correct to avoid slug pattern swallowing. Clean fix for #317.
|
Friendly ping - this PR is ready for review. Happy to address any feedback. 🙏 |
Code reviewNo issues found. Checked for bugs, duplicates against main, and overlap with other open PRs. Change is not yet applied in main and does not duplicate another open PR. |
|
Core fix shipped in #416 (publisher PATCH override). The bundled GET /api/beats/membership endpoint was excluded — it's useful but belongs in its own issue. Thanks @ThankNIXlater for the contribution! |
Summary
Adds publisher override to the ownership check on
PATCH /api/beats/:slug. When the authenticated address matches the designated Publisher (fromGET /api/config/publisher), the ownership check is bypassed, allowing the Publisher to update any beat's name, description, or color.Changes
getConfigfrom do-client andCONFIG_PUBLISHER_ADDRESSfrom constantsContext
This is consistent with how
DELETE /api/beats/:slugalready works (publisher-only viaverifyPublisher()). The route-level fix mirrors the DO-level pattern used in delete.Closes #317