Workers best-practices audit: remaining findings across 12 packages

[davidwkeith]

Summary

Follow-up from a Workers-best-practices audit of every mountable worker (16 catalog.json packages + @dwk/conformance-target + examples/deploy-to-cloudflare), reviewed by dedicated subagents against compatibility_date/nodejs_compat, streaming/waitUntil, floating-promise, crypto.subtle.timingSafeEqual, SSRF (@dwk/safe-fetch), Durable Object (extends/this.env/Hibernation API), and type-integrity rules from the workers-best-practices skill.

Three findings from that audit were fixed directly (not tracked here):

• [solid-pod] #patch buffered the full PATCH body before the WAC authorization check ran (pod.ts)
• [activitypub] deliverActivity's outbound POST bypassed @dwk/safe-fetch's redirect re-validation (delivery.ts)
• [webfinger] resolveHandle read a remote response body with no size cap (lookup.ts)

Everything below is the remainder — not urgent enough to block on, but real and worth tracking. @dwk/webmention, @dwk/websub, and @dwk/host-meta came back clean and aren't listed.

Status: all HIGH and MEDIUM findings were closed by #477. All LOW findings are closed by the follow-up PR from this branch.

HIGH

• webauthn — cbor.ts (Reader.readItem) recurses into nested arrays/maps with no depth limit; a crafted attestationObject of a few KB can stack-overflow the Worker (DoS). Add a depth counter, throw CborError past a small max (~16-32).
• webauthn — no try/catch anywhere in the package (verify.ts, rp.ts, handler.ts); a non-CborError parse failure (including the recursion issue above) escapes as an unhandled exception instead of the package's structured {error} JSON contract.
• solid-pod — WebSocket upgrade (pod.ts) is handled before any auth check, and #broadcast sends every resource-change notification (including private-resource IRIs) to every connected socket with no per-subscriber WAC filtering. An unauthenticated client can passively enumerate pod contents. Documented in-code as a "deliberate simplification" — worth deciding if that's still acceptable.
• mastodon-api — route dispatcher (handler.ts) has no top-level try/catch; a D1 failure or internal invariant throw bypasses the documented Mastodon JSON error shape (errors.ts isn't actually the single mapping point it's meant to be).
• micropub — fediverse syndication (handler.ts, syndicateEntry) is awaited inline inside the create-post response path; ctx is discarded as _ctx, so there's no way to background it despite a code comment implying it should be non-blocking.

MEDIUM

• solid-oidc — token-endpoint.ts buffers form bodies with no size cap before PKCE/code validation runs (public, unauthenticated endpoint).
• solid-oidc — CodeStore (handler.ts) rebuilt fresh per-request instead of once, causing a redundant D1 schema-check round trip on every /authorize//token call.
• solid-oidc — config.ts wires an injected logger/metrics seam that's never called at any security-relevant rejection point (PKCE mismatch, invalid/replayed code, rejected DPoP proof).
• indieauth — encoding.ts timingSafeEqual short-circuits on length mismatch before comparing bytes, instead of using the native crypto.subtle.timingSafeEqual. Used for PKCE challenge and HMAC signature checks.
• microsub — store.ts generateUid() uses Math.random() for part of a channel ID.
• activitypub — object.ts (#processVerifications, #resolveInbox) — two remote-fetch call sites skip the readBodyCapped discipline the rest of the file (correctly) uses.
• mastodon-api — encoding.ts timingSafeEqualHex hand-rolls a constant-time comparison instead of crypto.subtle.timingSafeEqual; used for client-secret/token comparisons.
• webauthn — verify.ts compares the WebAuthn challenge with !== on strings instead of a timing-safe byte comparison.
• vc — did-web.ts findVerificationMethod blind-casts most fields of an attacker-reachable DID document (entry as unknown as VerificationMethod), only id is validated. Fails safe downstream today, but still worth a real type guard.
• webdav — webdav.ts top-level handler has no try/catch; an unexpected backend exception escapes as a non-DAV response instead of a well-formed 5xx.
• remotestorage — storage.ts catch-all rethrows unexpected storage errors with no logging, breaking the package's otherwise-careful Logger/Metrics seam.
• atproto-pds — object.ts #importRepo buffers the entire migration CAR into memory with no size check, unlike the sibling #uploadBlob path which checks content-length first.
• atproto-pds — xrpc.ts errorResponse never logs the underlying error for a non-XrpcError; combined with ForwardedConfig dropping logger/metrics across the DO boundary, unexpected internal errors are invisible even in console.error.
• conformance-target — timing-safe-equal.ts loops to Math.min(a.length, b.length) instead of hashing to a fixed size first, leaking length via timing.
• conformance-target — index.ts fetch() has no top-level try/catch around routeRequest().
• examples/deploy-to-cloudflare — index.ts types the handler as satisfies ExportedHandler without unioning in HostMetaEnv, even though both packages are mounted. Harmless today (both fragments are empty) but a copy-paste trap for the next package added — this file is a template users clone.
• micropub — handler.ts returns a raw D1 error message verbatim to the client as a 500 error_description when a media metadata insert fails, with no server-side log of the failure.

LOW

• indieauth — handler.ts router has no top-level try/catch.
• indieauth — handler.ts/store.ts blind-cast DB-stored JSON (parseProfile, rowToRecord) without a shape guard.
• microsub — handler.ts handleFollow sends a non-essential queue message inline instead of via ctx.waitUntil.
• vc — handler.ts module-level signerCache is shared across every createVc() instance in the isolate, cutting against the package's "instantiate multiple times, tested in isolation" contract.
• conformance-target — index.ts one floating promise (request.body.cancel().catch(...) not wrapped in ctx.waitUntil).
• conformance-target — wrangler.jsonc compatibility_date is ~19 months stale.
• examples/deploy-to-cloudflare — index.ts no try/catch around routing dispatch (starter template code that gets copied into real projects).

Notes

• File:line references are as of the audit; re-check against current main before fixing, code may have moved.
• None of these are release-blocking on their own; triage by package priority (per RELEASING.md) when picking these up.