a7569a7 added credential-less pulls of public images. Three things about it
were wrong, all of them in how the appview handled the decision that belongs
to the hold.
**Scope handling was all-or-nothing.** IsPullOnlyScope required every
requested action to already be "pull", but clients routinely ask for more
than the operation needs — pull,push is common for a plain read, and some
ask for pull,push,delete up front. Those were rejected and challenged,
leaving a credential-less client no way to pull even a public image, which
is the entire feature. NarrowToPullOnly drops the write actions and issues a
token carrying "pull" and nothing else. Granting a subset is what the
distribution token spec expects. The allowlist property is preserved: "pull"
is the only action that survives, and "*" is deliberately not expanded into
it, since a wildcard request is not evidence the caller wants a read.
**The appview-side read gate was inert.** checkReadAccess passed p.ctx.DID,
the DID of the repository *owner*, not the requester. Any non-empty DID
satisfies a private hold's check, and the owner's is never empty, so it asked
"may the owner read their own hold", answered yes, and admitted everyone.
Worse, CheckReadAccessWithCaptain admitted any authenticated DID to a private
hold at all, on an explicitly-MVP assumption that holding a DID was close
enough to being a sailor. Every doc says otherwise (docs/hold.md:109 "Crew
with blob:read", CLAUDE.md:140, docs/BYOS.md:280) and so does the hold
(ValidateBlobReadAccess: owner, or crew carrying blob:read/blob:write). It
now takes isCrew and requires owner-or-crew, and callers only pay for the
crew lookup when it can change the answer — a public hold or an anonymous
caller is decided by the captain record alone. Nothing here loosens access;
it brings the local gate into agreement with the authority.
**Denials could not reach the client.** distribution's blobHandler.GetBlob
maps everything except ErrBlobUnknown to ErrorCodeUnknown, so a 401 raised
in the blob store left as a 500 — misreporting an auth failure as a server
fault, and giving BearerChallenge no 401 to attach WWW-Authenticate to, so
Docker was told "server error" instead of being prompted for credentials.
Clients that retry 5xx looped: 4.1s per case in the matrix, now 0.01s. The
check moves to Repository(), where an errcode.Error is passed through
verbatim by the registry app — the same mechanism a7569a7 used for
NAME_UNKNOWN. It fails open on a lookup error, since the hold is the
authority and a transient failure should not break public pulls.
Removes auth.allow_anonymous_pull. It could only ever withhold — captain.Public
is what grants — so it was a second flag for a decision the hold already owns,
and gating it appview-side was never the intent. Layer bytes 307 straight to
S3, so the appview is not even in the path whose cost might have justified an
operator-side lever.
Tests: TestAuthMatrix only ever ran against a public hold, and its pull cases
never fetched a layer — crane.Pull is lazy and img.Digest() needs only the
manifest, which ATCR serves from the user's PDS where it is world-readable, so
no pull row in the matrix touched blob authorization at all. Pulls now
materialize layer bytes, and testharness.WithPrivateHold plus
TestAuthMatrixPrivateHold cover public:false + allow_all_crew:true — the
production shape, where anyone with an account pulls and pushes and anonymous
gets nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
io.atcr.manifest rkeys are the digest alone (digestToRKey), so a single
record backs every repository of a user holding identical content. Both
paths that cascade-delete that record checked for remaining tags scoped to
one repository, which asks the wrong question: a tag in another repo keeps
the shared record alive just as much as a tag in this one.
With me/a:v1 and me/b:v1 at the same digest, deleting me/a:v1 saw no
remaining tags in repo a, deleted the shared PDS record, and purged the
layers on the hold. me/b:v1 was left pointing at content that no longer
exists, and the firehose delete handler then cleared the rows for every
repo (DeleteManifest with an empty repository argument).
The collision predates this, but it was reachable only behind the opt-in
AutoRemoveUntagged profile flag. 1b91768 enabled OCI manifest DELETE and
made TagStore.Untag cascade unconditionally, which turned a latent metadata
collision into blob-level data loss on an ordinary skopeo/crane delete.
- cleanupUntaggedManifest no longer filters candidate tags to
rctx.Repository. The surrounding comment already noted that the tag
collection is account-wide; the filter contradicted it.
- ShouldCascadeDeleteManifest takes the tag question DID-wide via a new
IsManifestTaggedAnyRepo, and drops its now-meaningless repository
parameter.
- IsManifestTagged stays repository-scoped and keeps its caller: the
delete-manifest confirmation prompt is genuinely asking about the one
repo whose tags the user is about to remove.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two migration files shipped as version 28: 0028_add_device_secret_lookup
(08121f3) and 0028_create_stripe_processed_events (12c55ed).
runMigrations keys applied migrations by the integer parsed from the
filename and skips any version already present in schema_migrations, so
the first file to load records 28 and the second is skipped in silence —
no error, no log. loadMigrations enumerates via fs.Glob, which sorts
lexically, so add_device_secret_lookup won and stripe_processed_events
never ran.
On an existing database that leaves stripe_processed_events missing, and
StripeEventSeen then fails closed: the error wraps into ErrWebhookProcessing,
the webhook returns 500, and Stripe redelivers into the same missing table
forever. No subscription, tier, or dispute event is ever applied — defeating
the exact idempotency 12c55ed was written to add.
Fresh installs were unaffected, which is why no test caught it: they take
the applySchema path where schema.sql already has the table and both
version-28 rows are merely recorded as applied.
Renumbered to 0029 rather than renumbering the device migration, so a
database that already ran this build (28 recorded, devices.secret_lookup
present, stripe table missing) picks the migration up on next boot instead
of staying broken. Renumbering the other file would have left that database
with the table still missing and re-run its ALTER on a column that exists.
Adds two guards: one asserting migration versions are distinct, and one
exercising the upgrade path where a duplicate manifests as a missing
schema_migrations row. Both fail on a planted duplicate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Credential-less pulls of public images. /auth/token issues a pull-only
token with an empty subject when no Basic auth is present; the
destination hold still enforces captain.Public, and push or delete always
challenges.
- token.IsPullOnlyScope and AuthMethodAnonymous;
Handler.issueAnonymousToken skips the authorizer gate and the
service-auth pre-mint, since there is no identity to reconcile and no
AppView-to-hold service token to bind. The token is still stamped
with the resolved registry domain, so anonymous pull works on
secondary front doors whose access controller demands their own
audience.
- auth.allow_anonymous_pull (default true) turns it fully off, restoring
the previous always-challenge behavior. Mirrored into the deploy
template, since the default means existing deploys pick this up.
- RegistryContext.Anonymous is plumbed from the middleware.
- ProxyBlobStore sends no Authorization header when the service token is
empty, and returns 401 rather than 403 for anonymous denials so Docker
prompts for credentials, including when a stale captain cache lets the
request through and the hold says private.
- BearerChallenge wraps the /v2/ subtree so a 401 raised deep in the
stack via errcode.ServeJSON still carries WWW-Authenticate.
Distribution's own scoped challenges are left alone.
IsPullOnlyScope allowlists the pull action instead of denylisting push and
delete. Distribution's actionSet.contains treats "*" as *every* action, so
a scope of `repository:victim/img:*` names neither denied string and would
have handed an unauthenticated caller a token valid for push and delete on
someone else's repository — clearing the authgate entirely, since anonymous
tokens deliberately skip it. Writes would still have failed further down
(no PDS credential), but the gate itself was bypassable. Now every
requested action must be exactly "pull". Covered by new claims tests.
Unresolvable identities return NAME_UNKNOWN instead of a bare error that
distribution renders as 500. This path was previously unreachable without
credentials; anonymous pull opens it to the internet, and a 5xx on
arbitrary input both misreports a bad request as a server fault and sends
clients that retry 5xx into a retry loop. That loop was real: in the auth
matrix, regclient spent 83s on a single case before this fix, and the
suite now runs in 5s.
Stat preserves an authorization verdict from getPresignedURL rather than
flattening it to ErrBlobUnknown. Distribution calls Stat before ServeBlob
on GET and HEAD, so without this an anonymous pull from a private hold
answered 404 and BearerChallenge had no 401 to annotate — the 401 path
above could never actually reach a client.
The auth matrix is updated to match: anonymous pull of the seeded public
repo now succeeds, anonymous push is denied against a real identity's
namespace (rather than an unresolvable one, which was testing name
resolution rather than authorization), and a new case pins the
NAME_UNKNOWN behavior for an unknown identity.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The webhook limit was only checked at creation, so losing entitlement (a
hold switch or a plan downgrade) left previously-created webhooks firing
paid behavior forever.
- Dispatcher takes a WebhookLimiter, consulted on every dispatch. It
caps the list to the current allowance, keeping the oldest N to match
what the creation gate would have permitted, and masks paid trigger
bits.
- GetWebhooksForUser orders by created_at ASC, id ASC so that cap is
deterministic. ListWebhooks gets the same tiebreak: it feeds the
settings UI, and without it the list a user sees could disagree with
the one the dispatcher truncates.
- webhooks.FreeTriggerMask is shared by the creation gate and the
dispatch backstop so the two cannot drift.
Capping is logged when it actually truncates. The webhooks stay visible in
settings, so from the user's side delivery would otherwise just stop with
no signal — and the same line is the only evidence if the limiter itself
degraded, since a billing lookup failure falls back to free-tier limits
and would quietly demote a paying user mid-dispatch.
Two cost fixes, both because this puts the entitlement lookup on a hot
path it was never on before:
findCustomerByDID now consults the customer cache instead of always
issuing a Stripe customer search. GetWebhookLimits reaches it via
GetSubscriptionInfo on every delivery, so uncached it meant a
rate-limited Search API call for every push and every scan record of
every user with a webhook configured.
DispatchForQuota checks whether the user has any quota webhook at all
before fetching the allowance. The original code filtered first precisely
so the common path (no quota webhooks) did no work; taking the allowance
up front would have spent the expensive lookup on every push. The cap
itself is still computed over the full list, since the count limit spans
all webhook types.
Note DeliverTest is deliberately not capped: it is an explicit,
user-initiated "send test" from the settings page, not automatic delivery.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entitlements were keyed on the Stripe subscription alone, so a subscriber
who switched to a self-hosted hold kept paying for features the appview
cannot deliver, and could still reach checkout.
- billing.ActiveHoldChecker and Manager.onManagedHold gate every
entitlement. An empty default hold counts as managed: the user has no
explicit preference and falls back to the operator's primary managed
hold.
- The checker reads the primary DB, not the read replica. A hold switch
writes default_hold_did to the primary, and replica lag would keep
paid features alive after a switch away.
- db.GetUserDefaultHoldDID is the clean default-hold signal, unlike
GetUserHoldDID which falls back to a manifest hold_endpoint (a URL,
not a DID).
- Jetstream fails closed: an unresolvable hold reference is cached raw
rather than left empty, since an empty value reads as managed.
- UI: the billing tab is hidden on self-hosted, a cancel/manage banner
appears when a self-hosted user still has an active plan, the image
advisor returns managed_hold_required instead of upgrade_required,
and the checkout route returns 403. The portal stays open so existing
subscribers can still cancel.
Two consistency fixes fall out of wiring this up:
The settings UI reads the resolved default_hold_did rather than the raw
profile.DefaultHold. The profile field is the record value as written and
may be a URL-form reference; jetstream resolves it to a DID on the way
into the DB, and the server-side gate reads that resolved value. Comparing
the raw form against managed DIDs would show the "you are self-hosted"
banner and hide billing from a user whose entitlements say otherwise.
HasAIAdvisor falls back to the free tier's AIAdvisor setting when
off-managed instead of a hard false, matching GetWebhookLimits. Losing a
managed hold should drop a user to free-tier entitlements, not below them.
BEHAVIOR CHANGE for existing paying users on self-hosted holds: they lose
the AI advisor, supporter badge and paid webhook limits as soon as this
deploys, while Stripe keeps charging them. The only notice is the banner
on /settings/storage, which they have to visit to see. Decide on a
migration (notification, or a one-time reconciliation over active
subscriptions) before shipping this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Webhook delivery was neither idempotent nor order-safe, and every failure
returned 400, which Stripe does not retry. A transient DB or hold error
therefore dropped a subscription change silently and permanently.
- New stripe_processed_events table: event_id as primary key dedups
redelivery, and event_created per customer drops stale out-of-order
deliveries.
- HandleWebhook distinguishes ErrWebhookSignature (400, no retry) from
ErrWebhookProcessing (500, Stripe redelivers). The event handlers
return errors instead of swallowing them. ErrBillingDisabled maps to
400: the route is mounted but billing is off, so redelivery can never
succeed and Stripe should stop rather than retry to exhaustion.
- Refuse to boot when billing is enabled with an empty
STRIPE_WEBHOOK_SECRET. Stripe HMACs with the empty key, so an
attacker can reproduce the signature and the endpoint is forgeable.
- UpdateCrewTierOnAllHolds retries each hold (3 attempts, linear
backoff, 5s per request) and returns a joined error so the webhook
can fail and let Stripe redeliver.
The fan-out contacts holds concurrently rather than in sequence. Serially,
one unreachable hold burns the caller's entire 10s budget on its own
retries (3 x 5s plus backoff) and the holds after it are never contacted;
because Stripe redelivers in the same order, a persistently-down first
hold means the rest are never updated at all.
On the hold, the signature-validated sub claim is now the source of truth
for updateCrewTier: a mismatched body userDid is rejected with 403 rather
than retargeting the grant to another DID. "Not crew on this hold" is a
200 no-op, since the appview fans updates out to every managed hold and a
subscriber is not crew everywhere.
That no-op has to be told apart from a storage failure. GetCrewMember
collapsed both into one generic error, so a CAR-store failure read as
"not a member", answered 200, and let the appview record the event as
processed — losing the tier grant permanently, which is exactly the
failure mode this commit exists to prevent. Missing records now carry an
ErrCrewMemberNotFound sentinel, and anything else returns 500 so Stripe
redelivers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DELETE /v2/<name>/manifests/<ref> answered UNSUPPORTED before ever
reaching the ATProto-backed stores: distribution v3.1.1's DeleteManifest
handler short-circuits unless app.deleteEnabled is set, which comes from
storage.delete.enabled. Set it (mirrored in the test harness). The
companion storage.EnableDelete option it appends only affects
distribution's built-in store, which RoutingRepository replaces, so it is
a no-op for us.
With the route reachable, make the stores do the right thing:
- ManifestStore.Delete purges the hold's per-layer, scan and image
config records on a detached context, since the DELETE handler
returns immediately and cancels the request context.
- TagStore.Untag resolves the digest before deleting the tag record,
then deletes the manifest if that was its last tag and it is not a
manifest list child, matching the web UI's delete-tag behavior so
deleting an only-tagged image doesn't orphan the manifest.
- cleanupUntaggedManifest becomes package-level over *RegistryContext
so both stores share one implementation.
- purgeOnHold moves out of handlers into pkg/appview/holdpurge so the
storage layer can call it: handlers already depends on storage via
middleware, so storage to handlers would be an import cycle.
- ProxyBlobStore.Delete returns distribution.ErrUnsupported, so the
always-registered blob DELETE route gives a clean OCI UNSUPPORTED
error instead of a generic 500. Layer bytes are reclaimed by the
hold's refcounted GC.
The cascade's still-tagged re-check pages through the tag records rather
than reading a single capped page. Tags for all of a user's repositories
share one collection, so one page is a per-account budget: past ~100 tags
a live tag fell off the end and the manifest was deleted while still
referenced. Incomplete enumeration now skips the delete, since an
orphaned manifest is recoverable and a deleted live one is not.
This also makes the over-quota delete grant added in 6e426dc load-bearing:
it hands out pull,delete tokens, which could not do anything while
distribution rejected every DELETE.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A read-only app password authenticates fine via createSession but cannot
call com.atproto.server.getServiceAuth, which is privileged — the PDS
answers 403 InsufficientScope. That fell through to the generic non-200
path and became a 503, which invites the client to retry a request that
can never succeed, with no indication of what is actually wrong.
Classify it with a sentinel error and map it to a 403 at /auth/token,
carrying text that names the fix: use a full-access app password.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
createSession, the app-password getServiceAuth call, and the hold's
/.well-known/atproto-did resolution all used http.DefaultClient, which
has no timeout. All three run on the /auth/token path, so a slow or
unreachable PDS or hold could hold the request open indefinitely, well
past Docker's own token-fetch timeout.
Give each a bounded client: 15s for createSession, 10s for the
app-password getServiceAuth, 10s for the hold DID fetch. All three are
safe to cut off — the two GETs are idempotent, and a timed-out
createSession only orphans an unused server-side session.
The OAuth refresh path deliberately keeps no overall timeout: its POSTs
run through refreshDetachTransport and must not be cancelled mid-rotation,
which would strand a rotated refresh token.
Note holdDIDResolveClient is package-level in pkg/atproto, so the 10s cap
applies to every ResolveHoldDID caller, including the GC, Jetstream
backfill and hold-health background workers, not only the token path.
That is intended (none of them want an unbounded fetch either), but it is
a wider blast radius than the token path alone.
This bounds three hops, not the whole request: the OAuth getServiceAuth
GET and identity resolution are still unbounded, so /auth/token is not
yet fully time-boxed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
findCustomerByDID built its search query with fmt.Sprintf, so the DID was
interpolated into a quoted Stripe search string unescaped. DIDs arriving
here are OAuth-validated and the DID grammar forbids quotes, so this is
not currently exploitable, but the query's safety depended on a validator
several layers away rather than on anything visible at the call site.
Escape backslash and single quote the way Stripe's search grammar
specifies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Freshness was measured from load time, so a load that fell back to a
stale-but-valid on-disk DB earned a fresh cache lease and could ride past
Grype's MaxAllowedBuiltAge cliff. Measure from the DB's own build
timestamp instead, and throttle reload attempts with a 30m backoff so a
down upstream doesn't make every worker pay its own download timeout.
Two locking fixes come with it, both reachable only once the DB is stale
and so newly relevant now that staleness is tracked honestly:
- FindMatches ran on a provider fetched outside the lock while a reload
could Close() it under the write lock. The scan now holds the read
lock across matching and reads the provider under it, so a reload
waits for in-flight scans instead of closing a store mid-scan.
- The retry backoff is also tested on the read-lock fast path. Once the
DB is stale the freshness test never passes again, so every scan was
taking the exclusive lock purely to reach the backoff return.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hold needs did:web:localhost%3A8080 so that the aud it presents in
service tokens is accepted: atproto only allows a port-bearing did:web on
localhost, and did:web:<ip>%3A<port> is rejected by real PDSes. Joining
the hold's netns (network_mode: service:atcr-hold) lets both reach each
other on localhost. The appview's 5000 is published on the hold service,
which owns the namespace.
Dev compose only; nothing in deploy/ or CI references this file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Design for moving push bandwidth off the appview and onto the hold,
mirroring the pull path's 307 redirect. Proposal only, not implemented.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Handles can change; the DID cannot. Linking the DID keeps the footer
correct if the account's handle is ever reassigned.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dashboard's top-users panel resolved a handle for every user with a
quota record, then sorted and truncated to ten. On a hold with ~500 crew
that is ~500 serial identity lookups to render ten rows.
Each lookup goes through the shared identity directory, whose HTTP client
allows 10s per request. One stalled lookup consumed the entire reverse
proxy budget, so the panel returned a partial body and the client hung up
mid-render:
admin/auth.go:161 "Failed to render template"
template=partials/top_users.html
error="write: broken pipe"
"GET /admin/api/top-users?limit=10" - 200 4096B in 10.005s
Sort and truncate first, then resolve only the surviving rows, so the
count is bounded by the limit rather than by hold size. Resolve those
concurrently under a 3s deadline: a slow lookup now degrades to a bare
DID instead of taking the whole request down with it.
The crew tab has the same underlying problem in a different shape, one
lazy-load request per row, and is not addressed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The quota gate ran on any scope containing "push" and denied the entire
token request, so "quota exceeded ... Delete images to free space" named
a remedy the gate itself blocked: docker and crane both request
pull,push,delete for a manifest delete, and manifest DELETE is
bearer-only, so there was no path left to free space.
When the request also asks for delete, drop push from the repository
entries and issue the reduced token instead of denying. A plain
pull,push is still denied so the quota message reaches the client that
needs to see it; granting a pushless token there would turn a clear
error into an opaque 401 on the first blob upload.
The narrowing happens in place on the access slice the handler hands to
the issuer, so document that on token.Authorizer along with the ordering
the gate goroutine depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DeleteManifestHandler built the tag rkey as "repo:tag" while the write
path uses RepositoryTagToRKey, which is "repo_tag" with "/" encoded as
"~". For a nested repo like stream/cache the two never match, so the
cascade leaves the tag record on the PDS while removing the local cache
row, and the tag reappears on the next backfill. Depending on the
variant it either no-ops (deleteRecord is idempotent) or fails outright
on an rkey containing a slash.
Every other io.atcr.tag call site already routes through the helper.
This was the last hand-built one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
checkPredecessor returned a bare false on every failure path (DNS failure,
dial error, non-200, 5s timeout, unparseable body), indistinguishable from a
hold affirmatively answering "I have no successor". isPredecessorHold then
cached that false in predecessorCache, which lives for the life of the
process and is never reset, so one blip during a single GC run permanently
unreferenced that hold's manifests. Those blobs are long past the 7-day
grace period that protects recent content, so the next run deleted them
outright with nothing to fall back on.
checkPredecessor now reports whether its answer is definitive, and only
definitive answers are cached. An inconclusive check keeps the hold's
manifests referenced and is recorded in predecessorUnresolved, which bounds
the cost to one timeout per run rather than one per manifest and is cleared
at the start of every analysis so a hold that was down once is re-checked
next time instead of written off.
This matches the convention the rest of the package already follows: a user
whose PDS cannot be reached has their records treated as referenced, never
as garbage. An outage must not be the reason content becomes deletable.
Non-200 counts as inconclusive on the same reasoning. A reachable service
that cannot produce its own captain record is malfunctioning, not answering,
and over-protecting an unrelated hold merely leaves some blobs unreclaimed.
Splits the fetch-and-parse half into checkPredecessorAt so it can be tested
against a local server without depending on DNS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An expired app-password token could wedge an account permanently. The 401
branch clears the cached token, but some PDSes report the same condition as
400 with an atproto error name in the body, which fell through to the generic
non-200 branch. That clears only the derived service token, so the dead
bearer token stayed in the cache and every subsequent request replayed it.
Observed on one account against at.hexlab.foo: 16,110 of these errors and
4,254 retryable 503s over 33 hours, with no recovery path. The cache is
in-memory, so it only cleared on process restart.
Now the non-200 branch classifies the atproto error name and evicts on the
ones that mean the presented token is unusable, matching what the 401 branch
already does. For app-passwords that is the equivalent of a refresh: the next
authentication re-mints via createSession.
Deliberately not routed through oauth.IsSessionInvalidError, which excludes
ExpiredToken on purpose — there it would delete a recoverable OAuth session
and sign the user out everywhere, whereas here the only thing discarded is a
cache entry that will be repopulated.
Not addressed here: the failure still surfaces as a 503, which is retryable
and so keeps clients looping. Returning 401 with the re-auth hint would be
the better signal, but it spans the token handler and is a separate change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ValidateDeviceSecret ran bcrypt.CompareHashAndPassword against every row in
the devices table until one matched — no WHERE clause. At bcrypt cost 10
(~65ms on the single-core production host) and 244 registered devices, a
device near the end of the scan cost ~15.8s of pure CPU per /auth/token,
which is past Docker's client deadline. Measured on production: 15.7-16.0s
steady state with the appview pinned at 100% CPU for the duration, while
anonymous requests on the same box served in 20ms.
The cost grew linearly with every device registered, and the scan ran in
rowid order, so the newest devices — the ones most likely to be in active
use — paid the most. This is the timeout users were reporting.
Devices now carry secret_lookup = hex(sha256(secret)), indexed, and
authentication fetches the single matching row.
SHA-256 is the verifier here, not merely an index. Device secrets are 32
bytes from crypto/rand, so presenting a value that hashes to a stored digest
requires a preimage or a 2^256 search. bcrypt's work factor only helps when
the input space is small enough to enumerate, which does not apply to a
random 256-bit token, and a database leak exposes no more than before.
The plaintext is not recoverable from a bcrypt hash, so existing rows cannot
be backfilled directly. They are migrated lazily on their next successful
authentication, which any push, pull or login triggers, and the legacy scan
is filtered to un-migrated rows so its cost decays as devices migrate. The
backfill runs after the cursor is closed: issuing it inside the rows loop
deadlocks, because the open cursor holds the connection the write needs.
bcrypt now exists solely to carry legacy rows across and can be deleted once
the table is fully migrated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The route was registered GET-only, so containerd and Docker, which try the
OAuth2 POST endpoint first whenever they hold a secret, ate a 405 and retried
on the GET form. Every authenticated pull paid two auth round trips, and in the
production logs the POST share of token traffic grew from 0.5% to 38% over six
weeks as more clients pulled from k8s with basic-auth imagePullSecrets.
Serve both specs on the same path. After credentials and scope are extracted
the two paths are identical, so this is an extraction branch plus a form-shaped
error writer.
Only grant_type=password is supported and no refresh token is issued: the
registry JWT's lifetime is pinned to the AppView<->hold service-auth, so a
refresh token would be a fourth long-lived credential with its own storage and
revocation. Clients handle its absence by continuing to use the credential they
already hold.
The refresh grant is refused with 401 rather than the 400 that RFC 6749 5.2
prescribes. containerd sends that grant only when it has no username, which is
the same condition that disables its 405 fallback, so a 400 would hard-fail
those clients. 401 is on its retry list and routes them to the GET form, where
a device secret authenticates off the password alone. That shape previously had
no working path at all.
resolveService now takes the requested service as an argument, since it arrives
in the query string on GET and in the form body on POST.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same class of bug as the firehose backfill, plus a second one found alongside
it. Both take down the whole hold process.
1. send on closed channel. Subscribe spawns drainPendingJobs in its own
goroutine, and it sends to sub.send without holding sb.mu. Unsubscribe
closed sub.send under the lock, so a scanner disconnecting during the drain
closed the channel out from under an in-flight send. The existing
`case <-sub.done` guard did not help: done meant "writer goroutine exited"
and was closed by handleWriter, which is a different event from
unsubscribing.
2. close of closed channel. Unsubscribe closed sub.send unconditionally, but
it is called from two places — handleWriter on write error, and
handleReader in its defer. A scanner dropping mid-write hits both, and the
slice-removal loop had no guard, so the second call fell straight through
to the close. The unassign UPDATE ran twice for the same reason, which
could return jobs a replacement scanner had already been handed.
sub.send is now never closed. done is repurposed to mean "this subscriber is
gone", closed only by Unsubscribe and guarded on whether the subscriber was
actually still registered. That makes drainPendingJobs' existing done case
correct, and handleWriter selects on done rather than ranging over send.
dispatchJob was already safe — it sends under sb.mu, which excludes
Unsubscribe.
hold01 is unaffected in practice (scanner disabled, no shared secret), but
seamark-hold runs the scanner continuously and is exposed on any scanner
restart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A relay that connected with a stale cursor and then dropped mid-backfill took
the whole hold process down:
panic: send on closed channel
pds.sendBackfillMsg events.go:869
pds.backfillFromDatabase events.go:809
pds.backfillSubscriber events.go:716
Subscribe spawns backfillSubscriber in its own goroutine, and that goroutine
writes to sub.send without holding b.mu. Unsubscribe closed sub.send under the
lock, so a disconnect during backfill closed the channel out from under an
in-flight send. select cannot guard that — a send on a closed channel panics
unconditionally.
sub.send is now never closed. Subscriber gains a done channel that Unsubscribe
closes instead, and every sender that runs unlocked selects on it. The map
check in Unsubscribe keeps the close single-shot, which matters because both
readPump and handleSubscriber call it on the way out. handleSubscriber selects
on done rather than ranging over send, since nothing closes send any more.
Broadcast and BroadcastIdentity were already safe (they send under b.mu, which
excludes Unsubscribe). backfillFromMemory was safe too via b.mu.RLock, but now
routes through the shared helper so a disconnect aborts immediately instead of
stalling up to 5s per event while holding the read lock and blocking every
broadcast.
The bug dates to 2025-10, so every build since is affected. It only fires when
a backfill goroutine exists, which Subscribe skips when cursor == currentSeq —
that is why caught-up relays never triggered it and a hold whose relays are all
behind is exposed on every reconnect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The namespace no longer depends on the personal PDS at jarrett.app being
online. lexicons/ is compiled by Corpora into a signed AT Protocol
repository and served from a Cloudflare Worker that holds no key, no
state and no database. Signing happens offline; nothing at serving time
signs.
This is the failure that prompted it: when that PDS returned HTTP 500,
OAuth logins failed with "invalid_scope: Failed to resolve requested
permission set", and only for users whose Auth Server met the NSID on a
cold cache — which is why it looked user-specific rather than global.
lexicons/ stays the source of truth. dist/ is compiled output and is
gitignored, as is any signing key, which lives outside both repositories
and is needed for every publish: any record change moves the MST root and
re-signs the whole repo.
The README carries the cutover runbook and the distinction that is
easiest to get wrong — lexicon.atcr.io is the host the Worker serves on
and is created once by wrangler, while the six _lexicon.* TXT records are
the authority pointers and are flipped separately.
Cutover completed 2026-08-04. All six authority domains name
did:web:lexicon.atcr.io. Verified by corpora verify (101 checks), goat,
@atproto/lexicon-resolver returning our exact record CIDs, and a cold
OAuth PAR against two unrelated Auth Servers. Two lexicons resolve for the
first time: io.atcr.hold.image.getConfig and io.atcr.hold.stats.daily.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to f12d5e0. Extending the orphan sweep to io.atcr.hold.scan and
io.atcr.hold.image.config brought those collections under a delete path built
for layer records, and the two are not interchangeable. Layer rkeys are
generated per write, so a remembered rkey is a stable handle on one record.
Scan and image-config rkeys are atproto.ScanRecordKey(manifestDigest) — the bare
digest hex, with no DID — which is both reused across re-pushes and shared
between users who push identical images. "Delete rkey K" therefore stopped
meaning "delete the record I judged", and the sweep could destroy live data two
ways.
Both were confirmed by probe before being fixed, and each guard below has a
regression test that fails when that guard alone is reverted.
Shared records: two users pushing the same image collapse onto ONE record whose
body names only the last writer. When that user deleted their manifest the
record looked orphaned, and deleting it stripped the vulnerability scan and the
layer history/env/entrypoint from every co-owner's still-live image.
- knownDigests keeps a record while any successfully fetched user still holds
a manifest at that digest, not just the one the record names.
- knownDigests alone is not enough: it is built only from users we reached, so
a co-owner whose PDS was down this run is indistinguishable from one who
deleted their image. digestOwners closes that. It maps each digest to every
DID that pushed it here, derived from the hold's own layer records during
the walk analyzeRecords already performs, so it costs nothing extra and
needs no network. If any owner was unreachable, the record stays. This is
the sweep's existing principle — an unreachable PDS never causes a deletion —
extended from the record's named user to everyone the record serves.
Layer records are the right source because they are load-bearing for storage
accounting and billing, so they exist for anything a user is charged for.
Reused rkeys: a re-push upserts into the same digest-derived slot, so a record
marked orphaned by an earlier scan can be replaced by a LIVE one before the
delete runs. The admin delete button makes this wide — lastPreview is in-memory
for the life of the process, so an open tab keeps a stale scan actionable — and
doRun has the same race across its analyze-to-delete gap.
- Records now carry the CID they had when judged, and the delete re-reads the
slot to confirm it still holds that revision. Anything else (rewritten,
missing, unreadable, or a ref with no recorded CID) is skipped, not deleted.
- The CID check is record identity, not orphanhood, and identity alone is not
enough either. A re-push does not necessarily rewrite the SCAN record:
scan-on-push is tier-gated, and the broadcaster's discovery loop skips any
manifest that already has a scan record, so its CID survives a re-push
unchanged. manifestsWithLayerRecords covers that — a push writes layer
records to this hold, so their presence proves the manifest is back
regardless of what the scan concluded. Local, no PDS round trip. A manifest
whose layer records are themselves orphaned but not yet collected also lands
in the set, which only defers its aux record one cycle; one more round of a
leak beats deleting a live user's records.
- maxPreviewAgeForDelete (30m) refuses a stale preview outright. The
per-record checks are the real safety net; this stops the admin acting on a
picture that is hours old. Refusal surfaces through startBackground ->
lastError -> the polling progress fragment, so it is visible rather than a
silent no-op.
The four state maps auxRecordOrphaned consults are grouped into auxOrphanState.
Three are same-shaped maps that were trivial to transpose positionally, and
swapping knownDigests with fetchedUsers would have silently widened what the
sweep deletes.
Not addressed, all failure-to-collect rather than data loss: maxPreviewItems
caps the admin button's list while doRun uses the uncapped one, so a hold with
10000+ orphaned layer records surfaces no aux orphans in the preview;
DeleteManifestAuxRecord cannot distinguish "already gone" from "write failed";
and discoverUserDIDs never consults the aux collections, so a record whose owner
has no remaining layer records is kept indefinitely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user who deletes a manifest record directly on their PDS was invisible
to the hold. purgeManifest only fires on appview-driven deletes, and the
GC orphan sweep only walked io.atcr.hold.layer, so scan and image config
records survived forever and the user's storage number never dropped.
analyzeRecords now also walks io.atcr.hold.scan and
io.atcr.hold.image.config, applying the same orphan test the layer sweep
uses: a record dies only when its owning PDS was reachable and
demonstrably lacks the manifest. Unreachable PDS, unparseable URI,
unparseable timestamp, and immature records all keep.
Splitting the grace period was required to make this useful, not
cosmetic. Records only need to outlast a push (blobs and layer records
are written before the manifest reaches the user's PDS), so they age out
at 24h. Blobs stay on the 7 day window, but since records now age out
faster than the blobs they name, a record can no longer serve as its
blob's clock. Blob age comes from S3 LastModified instead, which
WalkBlobs already received from ListObjectsV2 and was discarding.
Net effect: pruning an old manifest frees the user's storage on the next
nightly run rather than never, while the bytes are still reclaimed on
the same best-effort schedule as before.
DeleteManifestAuxRecord is deliberately narrow, accepting only the scan
and image config collections, so a bug in the sweep can't reach captain,
crew, or layer records.
Not addressed: the SBOM and vuln report blobs those scan records point
at. They share the /repos/{holdDID}/blobs/ prefix with the hold's avatar
and OG images, so collecting them needs a referenced-CID set built from
the hold's own records rather than a prefix walk.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An AppView can front several registry domains that all reach the same
backend (seamark.dev serving buoy.cr, seamark.cr, and soon atcr.io).
Distribution's token access controller holds `service` as a single string
and uses it twice: as the value advertised in the WWW-Authenticate
challenge, and as the sole accepted JWT audience. So it announced one
domain's name on every domain, and honoured one domain's tokens
everywhere. A push to seamark.cr was challenged with service="buoy.cr".
Both uses sit inside Authorized, which already has the request, but the
value is fixed at construction and reachable through no hook — autoredirect
only templates the realm. So register an "atcr-token" controller that
builds one upstream controller per domain and dispatches on r.Host. Each
front door now advertises its own name and demands its own audience. All
signature, certificate and claim verification stays in upstream code; this
only routes.
The token handler stops discarding ?service= and stamps the audience with
the front door the client used, allowlist-checked against the configured
domains so the value stays server-determined despite arriving from the
client. It has to come from the query param because the realm lives on the
UI host, where r.Host names no registry domain.
This is token hygiene and spec conformance, not a privilege boundary: every
domain fronts the same backend, so a client can obtain a token for any of
them just by handshaking there. What it buys is a truthful challenge and
the decoupling needed to later split a domain onto its own AppView.
Also unify the domain list. DomainRoutingMiddleware keyed its map on the
raw config while matching a port-stripped host, so a domain configured with
a port could never match its own requests. It now shares the normalized
cfg.Auth.Services, so routing and authorization agree on one set of names.
cfg.Auth.ServiceName was an exact alias for Services[0] and is replaced by
PrimaryService(), which also removes an empty-slice index.
Rollout: the audience for seamark.cr and bouy.cr changes, so a token minted
just before the restart draws one 401 and Docker re-handshakes into a valid
one. buoy.cr is unchanged (it stays primary), and atcr.io keeps the service
name it already has today. The challenge and the accepted audience come
from the same delegate, so the retry converges by construction. Deploy as a
single flip, not a canary: an old instance ignores ?service= and would keep
minting the primary audience while a new one rejects it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 6e77311. Listing the operated holds on /privacy put
resolveHoldDisplayName in the render path, where it makes up to two sequential
network calls per DID: the DID document fetch, then handle verification. /privacy
is public and unauthenticated, and the server sets no HTTP read or write timeout,
so an unreachable hold or a plc.directory outage stalled the page for every
visitor. The identity directory's negative-cache TTL is short, so the stall
recurred rather than settling after the first hit.
The DIDs come from config and never change while the process runs, so resolution
happens once in a background goroutine and lands in an atomic.Pointer. The
handler is constructed once at route registration, so the cache is process-wide.
Until resolution completes the page renders offline names, which are already
correct for did:web holds since those decode straight from the DID.
Dropped the did:plc truncation. resolveHoldDisplayName's last fallback cut a DID
to 24 characters plus an ellipsis, which is shorter than a did:plc, so the result
could not be resolved back to a hold. That is tolerable in a settings dropdown
and actively misleading in a privacy policy naming the services we operate.
Shortening for display belongs in the template. The non-network fallbacks are now
in holdDisplayNameOffline so the background resolver and the render path share
them.
Also fixed the surrounding copy, which scoped coverage to *.<site> domains while
the list immediately above it could contain holds on other domains — the two
sentences contradicted each other. It now refers to the listed holds, and only
makes that claim when there is a real list; with no managed holds configured the
template still shows an illustrative placeholder, which must not be presented as
fact in a legal document.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 37bab32. That commit stopped deleting OAuth sessions on transient
errors, which fixed spurious sign-outs but overshot on one path: a genuinely dead
session stopped being evicted at all, turning a forced re-login into a permanent
failure loop.
GetOrFetchServiceToken flattened every non-200 from getServiceAuth into
fmt.Errorf("service auth failed with status %d: %s"). IsSessionInvalidError then
had nothing structured to inspect, and its string fallback could not help: it
looks for the OAuth 2.0 code invalid_token, while atproto emits the XRPC name
InvalidToken. The difference is the underscore, not the case, so lowercasing
never bridged it. A revoked session came back 401 InvalidToken and was classified
transient, so /auth/token returned 503 forever and the user was never prompted to
re-authenticate.
The non-200 branch now wraps an *atclient.APIError carrying the status and the
parsed atproto error name, which is what the existing structured checks in
IsSessionInvalidError already know how to read. Transient shapes stay transient:
atprotoErrorName returns "" for a non-JSON body, so 500s with HTML, 502s, and
429s do not evict.
ExpiredToken is deliberately not treated as a dead session. It means "refresh
me", and deleting on it would sign the user out of every UI session over an
ordinary access-token expiry a refresh would have fixed. isAuthError omits it for
the same reason; the two classifiers have to agree about the same condition.
The comment on the string fallback claimed it was a looser spelling of the
structured check. It is not — it handles a different error family. indigo's
RefreshTokens returns OAuth token-endpoint failures as a bare fmt.Errorf carrying
the auth server's snake_case code verbatim ("token refresh failed (HTTP 400):
invalid_grant"), never a typed error, so a string match is the only thing that
can classify a refresh failure, which is the invalid_grant replay case 37bab32
exists to detect. Both comments now say which family they cover.
Two hardening items on the same theme:
use_dpop_nonce no longer counts as an auth error in the appview's isOAuthError.
It is a routine handshake step indigo retries with the server-supplied nonce, and
treating it as fatal signed users out over ordinary nonce rotation. It can still
escape when a server sends that error with no DPoP-Nonce header, leaving indigo
nothing to retry with; a stuck session there is preferable to signing everyone
out in the common case, and the comment says so rather than claiming it cannot
happen.
Detached session deletes are bounded by SessionDeleteTimeout. They run on
context.WithoutCancel so a canceled request cannot leave the cleanup half-done,
which also stripped the only deadline they had — a wedged database write blocked
the goroutine with no way to shed it. Matches the bound already on the detached
persist callback. The unparseable-token-endpoint warning is now deduped per
endpoint rather than once per process, since that path fails open by returning
the client unwrapped, silently reinstating the refresh burn.
The refreshDetachTimeout comment now notes the cap is per-POST: the DPoP-nonce
retry means one refresh can issue two, holding the per-DID lock for up to twice
the stated value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 8a556a5. v3 keys accounts by DID and, unlike v2, rewrites the
config during ordinary `docker pull` traffic (migration on first read, DID
backfill after a successful get) rather than only on explicit login. That moved
three latent ways to lose a device.json onto the hot path. A user cannot recover
from any of them: once the file is gone they no longer know which accounts they
had.
save() is now atomic. It writes a temp file in the same directory, fsyncs, and
renames, so the config always holds either the previous contents or the complete
new contents. The truncate-then-write it replaced left a zero-length window that
Ctrl-C, Docker reaping the helper, or a suspend could land in. The fsync matters
on its own: on ext4/XFS a rename can become durable while the new file's data
blocks are not, which resurrects the empty-file case. The rename also replaces
the destination's mode, which incidentally repairs a config restored from a
backup as world-readable (os.WriteFile's perm argument only applies at creation).
An unreadable config no longer degrades into an empty one that the next write
commits. loadConfig returns a usable empty config so read paths can still print
something helpful, but it now marks the unrecoverable cases with
errConfigUnusable, and loadConfigForWrite refuses on that sentinel. This is the
downgrade path: the previous binary hard-gates on Version == 2, so without the
guard a reinstall of an older helper would wipe a v3 file on the first
`docker login`. loadConfig also stops explicitly on a version newer than it
understands instead of falling through the legacy probes to the same empty
config. All six write paths take the guard — get, store, erase, login, logout,
switch. login had the same warn-and-continue-then-save shape as store.
The guard keys on the sentinel rather than on any error, deliberately. The
v2 -> v3 migration returns a fully populated config alongside a "saving migrated
config" error when the directory is not writable, and every pre-v3 user passes
through that path on their next invocation; refusing there would hard-break
`docker pull` for exactly the population that is migrating.
migrateV2toV3 merges DID collisions deterministically. Two v2 entries collapse
onto one v3 key when a handle was renamed and the old entry was never cleaned up,
which is the case v3 exists to fix. The previous loop wrote both into the same
map slot in randomized iteration order, so which account survived varied run to
run, and when the stale one won, get would fail validation and remove it — the
user ended up with no account at all. Iteration is now sorted and collisions
resolve through v2EntryBeats (active entry, then the one holding a secret, then
the smaller key), with the loser donating its secret if the winner has none. The
"No account is dropped" comment was false and is now accurate.
Also deterministic: find() and upsert() resolve their scans through a shared
scanFor helper that prefers DID matches and breaks ties on the smallest key, so
two entries sharing a handle can no longer hand Docker a different secret on each
invocation. upsert additionally refuses to match a handle that already belongs to
a different DID, which previously let it overwrite an unrelated account's DID and
destroy that account's credentials. Nil map entries are tolerated throughout
rather than panicking on Docker's credential path.
resolveHandleDNS is bounded at 2s. It runs synchronously on every store and on
every get for a DID-less account, and net.LookupTXT applies no deadline of its
own, so a blackholing resolver (captive portal, split-horizon VPN) stalled
docker login and docker pull on each invocation.
Tests cover the paths that can lose credentials. Each was checked against the
pre-fix code: the collision, null-entry, DID-theft, find-determinism,
permissions, and out-of-place-write tests all fail or panic without their fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The credential helper stored accounts keyed by handle, which broke on
handle renames and let the Docker `store` path (username+secret, no DID)
overwrite a good account with a DID-less one — how evan.jarrett.net on
buoy.cr ended up active with a blank DID.
Re-key everything by the stable DID, treating handle as a mutable display
label. DID is recovered client-side via standard AT-proto handle
resolution (DNS TXT _atproto.<handle> + HTTPS .well-known/atproto-did) —
no server change, no JWT, no auth, and no indigo pulled into the helper.
- resolve.go: stdlib handle->DID resolver
- config.go: v3 DID-keyed schema; find/activeAccount/upsert/rekey helpers;
upsert never blanks a known DID; migrateV2toV3 re-keys existing files in
place (no login lost; DID-less accounts stay provisional and self-heal)
- protocol.go: store resolves+preserves DID; get lazily backfills+re-keys;
list reports the active account's handle (was arbitrary map iteration)
- status/switch/logout: display handle, key/compare by DID
- config_test.go: migration, upsert-never-blanks-DID, rekey, find, list,
resolver parse helpers
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a Docker client canceled a slow /auth/token request mid-refresh, the
token-refresh POST was aborted client-side but completed on the PDS, which
rotated the refresh token. The rotated token was never received or persisted,
so the next refresh replayed the consumed token, got invalid_grant, and the
session (OAuth + UI) was deleted, signing the user out everywhere.
- Detach refresh POSTs from the inbound request context via a per-session
RoundTripper (WithoutCancel + 30s cap); once a refresh starts it completes
- Persist session updates (rotated tokens, DPoP nonces) on a detached context
- Gate session deletion on IsSessionInvalidError: cancellation, timeouts, and
transport errors no longer delete sessions; genuine invalid_grant still does
- Add phase timing to /auth/token and per-DID lock wait warnings to attribute
the ~14s pre-refresh stalls that push requests past Docker's deadline
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
checkPredecessor returned true for any hold with a non-empty successor field,
without verifying the successor was actually this hold.
This caused every hold running proactive scan discovery to queue manifests from
every other migrated hold on the network, producing 404 errors when the scanner
tried to fetch foreign blobs from its own S3.
Signed-off-by: Maarten Rijke <did:plc:fy4lwkc4hrd776vfkcrbzr5a>
- diff view gains a Packages tab with added/removed/changed/unchanged
package tables and purl-derived type/license/upstream links
- captain records verified against the DID's atcr_hold service before
caching (processor + batch backfill), preventing forged holds
- fix empty-handle updates clobbering cached handles and colliding on
the UNIQUE constraint
- move fillPrevCIDs into repo.go; DirectRepoOperator is now canonical,
repomgr kept as a test oracle
- surface read-only crew status in hold selector
- reconcile docs
1. Multiple registry domains + per-user domain preference
The biggest feature. The appview can serve several registry domains (e.g. buoy.cr, atcr.io),
and users can now pick which one shows up in their pull/push commands.
- Lexicon/record: adds registryDomain (and documents ociClient)
to the sailor profile (lexicons/.../profile.json, pkg/atproto/lexicon.go).
- DB: new registry_domain column on users (schema.sql + migration 0027),
with GetUserByDID/Handle reads, UpdateUserRegistryDomain writer,
and Jetstream caching it on profile updates (writes unconditionally so clearing propagates).
- UI/handlers: new UpdateRegistryDomainHandler + /api/profile/registry-domain route,
a <select> in the user settings panel (only shown when >1 domain configured), and resolveRegistryURL()
which falls back to the primary domain if the user's pref is stale/removed. Tests added for all of it.
2. default_hold_did removed → first managed_holds entry is the default
Consolidates two overlapping config fields into one. ServerConfig.DefaultHoldDID is gone;
PrimaryHoldDID() now returns managed_holds[0]. managed_holds is now REQUIRED.
Updated in config, validation, server wiring, test harness, example YAML, and the deploy template.
3. Admin long-running operations → generic background-job framework
New pkg/hold/admin/jobs.go introduces a reusable startJob/jobRegistry pattern
(a detached context.Background() job + a /admin/api/jobs/{key}/status polling endpoint).
This replaces the bespoke scan-backfill goroutine state machine, and now also wraps crew tier remap and crew import
all three previously looped synchronously on the request context and got 504'd/cancelled mid-run by the reverse proxy.
Forms switched from POST-redirect to htmx fragments (job_progress.html, job_result.html, crew_import_results.html)
the old crew_import_results.html page and scan_backfill_progress.html partial were deleted.
This is also captured as a new rule in CLAUDE.md.
4. Cascade-delete manifest on last-tag deletion
DeleteTagHandler now, after removing the last tag pointing to a digest, cascade-deletes the manifest itself
(PDS + DB + hold blob purge) — but only if it's not a child of a manifest list (multi-arch parent).
New GetTagDigest and ShouldCascadeDeleteManifest queries back it, plus cascade_delete_test.go.
Also switches tag rkey computation to the atproto.RepositoryTagToRKey helper.
5. Billing simplification
Drops the OwnerBadge config option (hold-owner supporter badge).
The user-profile template no longer special-cases an "owner" badge value (only "Captain").
Example tiers renamed to the nautical scheme (deckhand/bosun/quartermaster).
6. Build/deploy: go generate always runs via Make
make generate is now a phony target that always runs go generate ./... (regenerating cbor_gen, icon sprites, etc.),
and build-trixie depends on it. The deploy tooling (provision.go/update.go)
drops its own runGenerate calls since the Makefile handles it.
7. New cmd/firehose-tap tool (untracked)
A standalone CLI that subscribes to a com.atproto.sync.subscribeRepos endpoint and pretty-prints events,
with emphasis on Sync 1.1 compliance fields (per-op prev CIDs, commit prevData) and a --validate CI mode.
Fits with the recent "more sync1.1 compliant" commit.
Evan JarrettandClaude Opus 5
Maarten RijkeandTangled
lime360andTangled
Anhgelus MorhtuuzhandTangled