An audit of the scan pipeline and the hold side of scanning found several
ways scanning stops without saying so. Each fix here was written test-first:
a test expressing the wanted behaviour, confirmed failing for the right
reason, then the change.
A summary-less result crash-looped both processes. worker.go dereferenced
result.Summary unconditionally, but processJob only sets it when Grype runs,
and SendResult puts the nil on the wire before the scanner dies on it, so
handleResult's unguarded log killed the hold too. A nil Summary now means
"not scanned for vulnerabilities", deliberately distinct from "scanned, found
zero" — inventing a zeroed summary would report every image as clean when
Grype never ran. The hold writes a record rather than orphaning the uploaded
SBOM, and the appview renders an "SBOM only" state instead of a green Clean
badge.
The Grype database could wedge with no way back short of a restart. All three
throttles in loadVulnDatabase were guarded by vulnDB != nil, so a scanner
holding no provider retried a full download on every scan under the exclusive
lock. Two earlier attempts at this bug each added one more condition to the
same chain; this replaces the chain with a single decision function over a
state snapshot, consulted by both call sites so they cannot disagree. That
disagreement was itself a bug: the 50-scan reload had never once executed.
Two independent halts. An unparseable frame was dropped in silence, stranding
a row that held the hold's only dispatch slot forever; it is now answered
"skipped" on first delivery. The 10-minute sweep leaked the in-flight digest
and wrote no record, permanently retiring one image per timeout.
A digest went unvalidated into filepath.Join and os.Create, so a layer digest
of sha256:../../../x wrote outside the scan directory, and nothing verified
that downloaded bytes hashed to the digest naming them. Digests come from
records in a user's own PDS. Both are fixed together: verification is what
makes an escaping write self-defeating.
Concurrency did not work on either axis. The proactive capacity gate was
depth-one hold-wide, so neither extra workers nor extra scanner processes
received work. Depth is now the sum of the worker counts scanners advertise on
connect, the gate is scoped to proactive work, and dispatch prefers the
least-loaded scanner. Disconnects no longer hand a running scan to someone
else: a scanner keeps a stable per-process identity and reclaims its own rows
within a grace window, while a process that truly restarted returns with a new
identity and has its work reclaimed, which is correct because the restart did
lose it.
The hold's scanning deadline measured queueing rather than scanning, because
the scanner acks on receipt and handleAck never refreshed assigned_at. A new
"started" message, sent by the worker that dequeues the job, separates the two
budgets. An older scanner never sends it and falls under the queueing budget,
which is more forgiving than the deadline it gets today.
Adds an in-process mock hold and an e2e harness that runs the real client,
queue and worker pool, seeded with 84 real manifest records fetched from a
live PDS. Real image layouts and the Grype database are fetched by scripts and
gitignored; suites needing them skip cleanly, so the default run stays offline
and fast.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1Km3N3uUmeGaj7VbaM8PF
8487258 added the deprecation notice to the shared helm metadata partial, on
the understanding that both the Overview and Chart tabs rendered it. Only the
Chart tab does. The Overview panel renders the README and never touches chart
metadata, so a deprecated chart carried no warning on the default landing tab.
The notice is extracted into a shared helm-deprecation-notice block, so the
copy lives in one place, and the Overview panel renders it server-side as the
first element in the panel. Deprecation decides whether you should use the
chart at all, so it belongs in the first paint rather than arriving a beat
later from a lazy fetch.
Getting the data there costs nothing extra. The page already made a blocking
hold call for layer count, and for a chart that call was wasted: a helm config
blob is Chart.yaml, which has no history key, so the count always came back 0
and fell through to the database. That call is now FetchHelmChartMeta instead,
against the same XRPC endpoint, so a chart page makes one hold call rather than
two and the displayed layer count is unchanged. A container image never fetches
chart metadata and its path is byte-for-byte the old code.
Failure follows the layer-count precedent: log at warn, leave the metadata nil,
render the page. An unreachable hold means no notice, not a broken repository
page. The tradeoff against the lazy version is that a slow-but-up hold now
delays the whole page, bounded by the same 10s the page already accepted.
A container image emits no element at all rather than an empty one, so the
space-y-4 stack spacing is untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
Artifacts filter had no empty state: filtering to zero matches left a blank
panel, indistinguishable from something being broken. It now uses the existing
state-empty partial, with no CTA since the filter input is right there. Rows
appended by Load More now also obey the active filter, which is a change to
existing behaviour but is required for the empty state to be truthful.
The tag icon beside the tag selector computed to 5.44px by 24px on every repo
page: the class was right, the flex parent was shrinking it. Adds shrink-0 to
that instance only. 31 other icons are direct flex children without shrink-0
and are left alone, since a site-wide sweep found only this one squeezed.
Digest-page scan tabs set role="tab" but never aria-selected, while the
repo-page tabs do. Both are now set server-side so first paint is correct, with
JS keeping them in sync afterwards, following switchRepoTab's pattern. The
diff-content tabs had the identical defect and are fixed too, since the handler
is generic over radio tabs and leaving them out would have meant attributes set
by JS but never by the server.
Includes the bundle rebuild for these and the two /auth/token-adjacent JS
changes, since nothing in the dev loop keeps that artifact current on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
seamark.dev's /install and /settings/devices told users to pipe
seamark.dev/static/install.sh into bash. That file was the unmodified ATCR
script: it announced itself as the "ATCR Credential Helper Installer",
installed docker-credential-atcr, and finished by telling the user to configure
credHelpers for atcr.io, the wrong registry for that deployment. Anyone
following the documented setup ended up pointed at another service. The
templates hardcoded docker-credential-atcr, "atcr" and ~/.atcr/device.json
alongside a correctly themed {{ .RegistryURL }}.
The scripts are now rendered from config by a handler, rather than forked per
brand. A theme overlay was the alternative and was worse: it needed a full copy
of both install.sh and install.ps1 per brand, four scripts to keep in sync, and
the operator asked for these values to come from config.
credential_helper.name is the single knob. Docker resolves a credHelpers value
x by exec'ing docker-credential-x, so the credHelpers value, the binary suffix
and the config directory are genuinely one word, not three that can drift. It
is validated against a strict pattern because it is interpolated into a shell
script.
install.sh renders byte-identical to the deleted static file under the atcr
default, so existing installs are unaffected. install.ps1 differs by one line,
where a stale usage comment named a path the script is not served at.
Two behaviour changes worth noting: these two URLs drop from a one-year
Cache-Control to five minutes, since the body now depends on deployment config;
and credential_helper.tangled_repo becomes a real overridable default. It was
previously assigned over unconditionally and read by nothing, while the shipped
script used a different URL form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
digest.html rendered a Deprecated chip, but the shared helm-metadata partial
never read .Deprecated, so the repo page carried no deprecation signal at all.
The data was already on the struct; this was a display omission.
The notice leads the metadata card, above the description: deprecation decides
whether you use the chart at all, so it has to be read before the prose that
sells it. It uses the alert/alert-warning callout the codebase already uses for
this kind of thing, including in the adjacent helm-digest-content partial,
rather than a bare badge. A lone small badge reads as a stray tag once it is
outside the digest page's row of status chips, and leaves no room to say why it
matters.
The digest page now shows deprecation twice, deliberately: its header chip is
the scannable signal above the fold, and this carries the explanation further
down. Removing the chip would push the only signal below the install command,
which is the burial this fix is meant to undo.
Note the repo page's Overview tab, which is where most people land, still shows
nothing: it renders the README and never touches chart metadata. Only the Chart
tab gains the notice here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
The client switcher built every command as "<client> pull <ref>". That is
valid for docker, podman, buildah and nerdctl, but crane requires a
destination:
$ crane pull seamark.cr/user/bench8x1:v2
Error: requires at least 2 arg(s), only received 1
So selecting crane handed the user a command that cannot run. pullPrefix is a
prefix-only helper, which is precisely why it could not express this; add a
matching pullPostfix that returns " <image>.tar" for crane and "" for
everything else, including "none" (image reference only), which must get
neither prefix nor postfix.
Both render paths change together, since fixing one leaves the bug visible in
the other: the Go template helper paints first, and updatePullCommand in
app.js re-renders when the dropdown changes.
Repository names may contain slashes, so only the last path segment is used —
otherwise the destination would name a subdirectory that does not exist. A
name ending in "/" yields no destination at all rather than a bare ".tar",
on the grounds that a visibly wrong-arity command beats silently writing a
hidden file. That input is not reachable through the real repo-name path.
The test asserts the whole command string rather than just the postfix, so it
covers the prefix/postfix interaction and the "none" case where both vanish.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
digest_content.go branched on Error == "never-scanned" to pick the
"not scanned yet" copy, but nothing anywhere produced that string: both
FetchVulnDetails and FetchSbomDetails returned the human sentence
"No scan record found" for a missing record. So vulnReason and sbomReason
could never be "not-scanned", the friendly branches in vulns-section.html
and sbom-section.html were dead code, and every unscanned image fell
through to fetch-failed.
Free-tier accounts have scan_on_push off, so this was every image they
push, told "Scan data couldn't be loaded... try refreshing in a minute"
about something that had never been scanned and never would be by
refreshing. The digest page showed the raw internal string instead.
Replace the prose sentinel with a NotScanned bool the 404 path actually
sets, and give other non-200 statuses a distinct message so a 500 from the
hold stops being indistinguishable from an absent record. The detail
templates branch on it before Error, so nothing leaks the internal value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk
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>
- 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.
1. Removing distribution/distribution from the Hold Service (biggest change)
The hold service previously used distribution's StorageDriver interface for all blob operations. This replaces it with direct AWS SDK v2 calls through ATCR's own pkg/s3.S3Service:
- New S3Service methods: Stat(), PutBytes(), Move(), Delete(), WalkBlobs(), ListPrefix() added to pkg/s3/types.go
- Pull zone fix: Presigned URLs are now generated against the real S3 endpoint, then the host is swapped to the CDN URL post-signing (previously the CDN URL was set as the endpoint, which
broke SigV4 signatures)
- All hold subsystems migrated: GC, OCI uploads, XRPC handlers, profile uploads, scan broadcaster, manifest posts — all now use *s3.S3Service instead of storagedriver.StorageDriver
- Config simplified: Removed configuration.Storage type and buildStorageConfigFromFields(); replaced with a simple S3Params() method
- Mock expanded: MockS3Client gains an in-memory object store + 5 new methods, replacing duplicate mockStorageDriver implementations in tests (~160 lines deleted from each test file)
2. Vulnerability Scan UI in AppView (new feature)
Displays scan results from the hold's PDS on the repository page:
- New lexicon: io/atcr/hold/scan.json with vulnReportBlob field for storing full Grype reports
- Two new HTMX endpoints: /api/scan-result (badge) and /api/vuln-details (modal with CVE table)
- New templates: vuln-badge.html (severity count chips) and vuln-details.html (full CVE table with NVD/GHSA links)
- Repository page: Lazy-loads scan badges per manifest via HTMX
- Tests: ~590 lines of test coverage for both handlers
3. S3 Diagnostic Tool
New cmd/s3-test/main.go (418 lines) — tests S3 connectivity with both SDK v1 and v2, including presigned URL generation, pull zone host swapping, and verbose signing debug output.
4. Deployment Tooling
- New syncServiceUnit() for comparing/updating systemd units on servers
- Update command now syncs config keys (adds missing keys from template) and service units with daemon-reload
5. DB Migration
0011_fix_captain_successor_column.yaml — rebuilds hold_captain_records to add the successor column that was missed in a previous migration.
6. Documentation
- APPVIEW-UI-FUTURE.md rewritten as a status-tracked feature inventory
- DISTRIBUTION.md renamed to CREDENTIAL_HELPER.md
- New REMOVING_DISTRIBUTION.md — 480-line analysis of fully removing distribution from the appview side
7. go.mod
aws-sdk-go v1 moved from indirect to direct (needed by cmd/s3-test).
Evan JarrettandClaude Opus 5