feat(mastodon-api,activitypub): opt-in owner-scoped status write surface - #426
Conversation
Add POST /api/v1/statuses behind config.allowWrites (default off, so every write route stays 404 and the plain-bearer exception remains strictly read-only). When enabled, the exception widens to owner-scoped write: the route requires an owner-bound bearer (422 for app-level tokens) carrying write / write:statuses scope (403 otherwise), enforces a 500-char ceiling, renders the plain-text status to Note HTML, and publishes through @dwk/activitypub's outbox path via a new internal __client/publish DO route that shares #storePost with the AS2 /publish endpoint. New MastodonBackend.publishStatus seam and tokenHasScope helper. Delete, interaction verbs, follow, and reply-on-create are follow-ups. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XT45PcE3pgXgsQffQDWdhW
|
The Generated by Claude Code |
davidwkeith
left a comment
There was a problem hiding this comment.
Automated review pass.
CONTRIBUTING.md conformance: checked out the branch and ran the local CI gate — pnpm install, pnpm build (all 30 packages), pnpm lint, pnpm format:check, pnpm --filter @dwk/mastodon-api typecheck, pnpm --filter @dwk/activitypub typecheck, pnpm test --project @dwk/mastodon-api (164/164), pnpm test --project @dwk/activitypub (288/288). All clean. PR title (feat(mastodon-api,activitypub): opt-in owner-scoped status write surface) is correct Conventional Commits form; PR body keeps the template's Summary/Packages affected/Checklist headings verbatim and leaves the one inapplicable checklist item (catalog/conformance status) unchecked with a valid one-line reason rather than deleting it. Changeset covers both affected packages with a minor bump, which fits (new opt-in surface, no breaking change to the default). Spec (spec/packages/mastodon-api.md § Write surface) and spec/non-functional-requirements.md's DPoP exception note are both updated in the same PR to match the new behavior, per the "specs are the requirements" ground rule. Colocated tests are present for the new code (auth.test.ts for tokenHasScope, statuses-write.test.ts for the route).
Correctness: the read-only default, owner-account gating, scope enforcement (write vs write:statuses hierarchy), and 500-char/blank validation all check out against the tests and manual tracing. One real (but currently latent) issue found — see the inline comment on object.ts's #clientPublish: its response omits the relayedBy field that every other __client/* row producer sets explicitly, silently turning a declared string | null into undefined. It doesn't break anything today (the one consumer uses a truthy check), but it's inconsistent with the rest of the surface and worth a one-line fix. Also left a minor style nit on a duplicate ./entities.js import in statuses-write.ts.
Note: the github-advanced-security check is currently failing, but that's GitHub's own Copilot code-scanning agent erroring out on model_not_supported (an infra-side issue on their end, unrelated to this diff) — not something to fix in this PR. build-test/integration were still in progress at review time.
Generated by Claude Code
… import Address review on #426: - #clientPublish's response omitted relayedBy, so toBackendEntry read it as undefined against the declared string | null. Set relayedBy: null explicitly, matching every other __client/* row producer. - Merge the duplicate ./entities.js import in statuses-write.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XT45PcE3pgXgsQffQDWdhW
davidwkeith
left a comment
There was a problem hiding this comment.
One finding on the write-surface diff: spoilerText isn't HTML-escaped before being federated as the Note's summary, unlike content two lines above (see inline comment). Everything else — the allowWrites/scope/account gating, tokenHasScope, the #storePost refactor sharing logic between /publish and __client/publish, and the CONTRIBUTING.md checklist (changeset present for both touched packages, spec updated, tests added, conventional-commit-styled title/scope) — looks solid.
Generated by Claude Code
…summary publishStatus escaped the status body via plainTextToHtml but assigned spoilerText to the Note's summary raw. Since the Note is fanned out to followers, an unescaped </&/> in the owner's CW became literal markup on receiving instances. Extract escapeHtml and apply it to the summary too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XT45PcE3pgXgsQffQDWdhW
davidwkeith
left a comment
There was a problem hiding this comment.
Overview
Adds an opt-in, owner-scoped write surface to @dwk/mastodon-api: POST /api/v1/statuses, gated behind config.allowWrites (default off → 404), requiring an owner-bound bearer with write/write:statuses scope. Publishes through a new internal POST <actor>/__client/publish DO route that shares #storePost with the existing AS2 /publish path.
Code quality / correctness
- Default-safe posture:
allowWritesabsent/falseand/or a backend withoutpublishStatusboth fall through to404inhandleCreateStatus(statuses-write.ts), so "no write endpoint exists" genuinely still holds unless explicitly opted in. Good. - Auth checks are layered correctly:
invalidToken()(no/invalid bearer) →accountRequired()(app-levelclient_credentialstoken, noaccountId) →insufficientScope()(read-only token). Verified against the tests (statuses-write.test.ts) and matches the read-endpoint precedent (422 for app tokens). tokenHasScopecorrectly implements the Mastodon scope hierarchy (broadwritecoverswrite:statuses, but not the reverse) with clean unit tests covering the asymmetry.- HTML escaping:
plainTextToHtml/escapeHtmlinmastodon-api.tsescape&,<,>in bothcontentand the CWsummarybefore they're embedded in the federatedNote— correctly closes the "typed<script>into a status" federation-injection concern, and it's tested (escapes HTML metacharacters in both content and the CW summary). #storePostextraction inobject.tsis a clean, behavior-preserving refactor shared by#publishPostand the new#clientPublish— same outbox write, fan-out, and alarm arming either way.- Minor/non-blocking:
#storePostdoes anINSERT OR IGNOREthen a separateSELECT seq FROM outbox WHERE id = ?to recover the assignedseq. Since this is a single-threaded DO the extra round trip is harmless, butINSERT … RETURNING seqwould fold it into one statement if you ever revisit this path.
CONTRIBUTING.md conformance
- ✅ PR title
feat(mastodon-api,activitypub): opt-in owner-scoped status write surface— correct Conventional Commits scope/format. - ✅ PR body keeps
Summary/Packages affected/Checklistheadings verbatim. - ✅ Changeset present (
.changeset/mastodon-write-surface.md), bumping both affected packages minor. - ✅
catalog.json/conformance/status.jsoncheckbox left unchecked with a one-line reason ("not applicable, no new worker") rather than deleted — matches the CONTRIBUTING requirement, and is correct: this is an additive route on the existing worker, not a new one. - ✅ Spec updated: new
spec/packages/mastodon-api.md§ Write surface,spec/non-functional-requirements.md's DPoP exception note updated to describe the widened (opt-in) blast radius. - ✅ Colocated tests:
statuses-write.test.ts(404-when-off, 401, 403, 422×2, 200, app-token 422),auth.test.ts(tokenHasScope), plus a DO-level adapter test inmastodon-api.test.ts. - Noted your comment on the PR already explaining the
github-advanced-securityfailure as a recurring GitHub-side infra issue unrelated to this diff (consistent with #423/#424) — the repo's own gates (release-gate,integration,Analyze,CodeQL) are green.
Security
Accepted blast radius is explicitly documented (a leaked write token can author as the owner until revoked) and matches how any Mastodon instance treats an access token — reasonable for an opt-in, single-owner deployment. No issues found beyond what's already called out in the spec.
No blocking issues. Nice, tightly-scoped increment.
Generated by Claude Code
…-surface # Conflicts: # packages/activitypub/src/mastodon-api.test.ts
davidwkeith
left a comment
There was a problem hiding this comment.
Reviewed the full diff (mastodon-api.ts, object.ts, statuses-write.ts, auth.ts, backend.ts, config.ts, errors.ts, plus the spec/NFR updates) and traced the write path end-to-end: handleCreateStatus → MastodonBackend.publishStatus → ActivityPubObject#clientPublish/#storePost → parsePostInput/buildPostActivity.
Prior review threads: all three open threads (missing relayedBy: null in #clientPublish, duplicate entities.js import, unescaped spoilerText in the federated summary) are already fixed and resolved (1c3c86d, 89b3b35), with a regression test added for the HTML-escaping fix. No new instances of these issues found elsewhere in the diff.
Correctness spot-checks:
tokenHasScope's hierarchy is correct and matches its tests: a broadwritegrant coverswrite:statuses, but not the reverse, andreadnever grantswrite.- The opt-in gating is airtight —
handleCreateStatus404s unless bothconfig.allowWritesandbackend.publishStatusare present, so the documented "default is read-only, no write endpoint exists" claim in the spec holds even if a deployment forgets to wire a write-capable backend. #storePost's extraction out of#publishPostis a faithful refactor — same SQL, same fan-out/audience delivery, same alarm arming — just returning the row'sseq/activityfor the new caller.#clientPublishrequires the same internal+publish header markers as/publish, so it isn't reachable without the mastodon-api layer's own auth/scope checks upstream.plainTextToHtml/escapeHtmlcorrectly escape bothcontentandsummarynow (verified via the newmastodon-api.test.tscase), andparsePostInput/PostInputalready supportsummary/sensitive, so nothing is silently dropped between the Mastodon-shaped input and the AS2Note.
CI note: the github-advanced-security check shows failure, but the job log shows it's the GitHub Copilot autofind reviewer erroring out on its own (CAPIError: 400 model_not_supported) before it could analyze anything — not a finding against this diff. CodeQL itself (the actual static-analysis check) is green. Nothing to act on there; may just need a re-run.
CONTRIBUTING.md conformance:
- PR title
feat(mastodon-api,activitypub): opt-in owner-scoped status write surface— correct form (lowercase type, comma-separated scopes minus the@dwk/prefix, uncapitalized subject). - Spec updated in the same PR (
spec/packages/mastodon-api.md§ Write surface,spec/non-functional-requirements.md's DPoP exception note) alongside the behavior change, as required. - Changeset added (
@dwk/mastodon-api+@dwk/activitypub, both minor) — correct bump for an additive, backward-compatible (default-off) feature. - Colocated tests added for every new code path (route-level 404/401/403/422/200 matrix,
tokenHasScopeunit tests, DO-level adapter + escaping test). - Template headings kept verbatim; the one inapplicable checklist item (catalog/conformance status) is left unchecked with a reason instead of deleted.
No changes requested.
Generated by Claude Code
Summary
Adds the first slice of a Mastodon write surface, so off-the-shelf clients (Tusky, Pixelfed) can post from this deployment — implemented under the "widen exception, owner-only" posture confirmed for this work.
The tension this resolves: real Mastodon clients can't do DPoP, so any write route they use inherently extends the documented plain-bearer DPoP-everywhere exception from read-only to writes. This PR makes that extension opt-in and owner-scoped, never on by default:
config.allowWritesabsent/false, every write route answers404— the token exception stays strictly read-only, exactly as documented, and "no write endpoint exists" still holds for the default config.client_credentialstokens are422, as on the read account endpoints) and (b) carrieswriteorwrite:statusesscope (readalone →403 insufficient_scope). Every other mitigation is unchanged: tokens stay opaque, hashed at rest, isolated to this package, RFC 7009 revocable. Accepted blast radius: a leaked write token can author as the owner until revoked — how any Mastodon instance treats an access token.v1 endpoint:
POST /api/v1/statuses(create). Plain-textstatus→NoteHTML (\n\n→paragraph,\n→<br>, escaped),spoiler_text→summary,sensitivecarried through; a 500-char ceiling and blank body are422. Published through@dwk/activitypub's existing outbox/fan-out path via a new internalPOST <actor>/__client/publishDO route that shares its store/fan-out logic (#storePost) with the AS2/publishendpoint and returns the stored row's snowflake coordinates, so the response renders as the owner-attributedStatus.Deferred to follow-ups (noted in the spec): delete, the interaction verbs (favourite/reblog/bookmark + undos), follow/unfollow, and
in_reply_to_idon create.New backend seam
MastodonBackend.publishStatus?(optional — a backend without it leaves the route404even whenallowWritesis set) and atokenHasScopehelper.Packages affected
@dwk/mastodon-api, @dwk/activitypub
Checklist
spec/packages/and updated them ifbehaviour changed — added
spec/packages/mastodon-api.md§ Write surface and updatedspec/non-functional-requirements.md's DPoP exception notesrc/*.test.ts) — route tests (statuses-write.test.ts: 404-when-off, 401, 403 read-only, 422 blank/over-limit, 200 create, app-token 422),tokenHasScopeunit tests, and a DO-level adapter publish testpnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test(2,809 tests green; release + catalog gates pass)pnpm changeset) if this touches a publishablepackage
catalog.json/conformance/status.jsonif this adds a newmountable worker or changes conformance status — not applicable, no new worker; the route is an additive endpoint on the existing
mastodon-apiworker🤖 Generated with Claude Code
https://claude.ai/code/session_01XT45PcE3pgXgsQffQDWdhW
Generated by Claude Code