mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-20 01:04:15 +00:00
0ea0e5494ca29b3971ba12256fc18286c890bee3
628
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Evan JarrettandClaude Opus 5
|
0ea0e5494c |
deploy: bound the scanner's memory and yield the box to the hold
The scanner shares a 1 GiB host with the hold, and nothing stopped it taking the whole box down with it. max_image_size was absent from the template, so production ran the 2 GiB default. Peak RSS tracks image size and a 389 MiB compressed image already reaches 561 MiB, so 512 MiB is the ceiling that fits. Sizing matters in this order: max_image_size rejects before a byte is downloaded, while MemoryMax kills mid-scan, and the hold then returns the row to pending and the restarted scanner is handed the same job again, which is a permanent crash loop on one image. If the cgroup cap is ever seen firing, lower max_image_size rather than raising it. GOMEMLIMIT is soft: the runtime collects harder to respect it and never fails an allocation to honour it. With no cgroup cap the kernel picks its own OOM victim, and the other large process on that host is the hold, so the scanner's overshoot could kill the service it reports to. CPUWeight and IOWeight, because no unit here set any scheduling directive: a scan finishing a minute later costs nothing, a hold that cannot answer a pull costs a user. Weights apply only under contention, so the scanner still uses the whole box when nothing else wants it. workers drops to 1. Proactive dispatch was depth-one hold-wide until this release so the second worker never received anything, and now that it can, two concurrent scans of a node:22-class image need 687 MiB against a host this size. Raise it with MemoryMax and the host together. Note the cgroup ceiling still wants rechecking: it was sized from measurements taken with Grype disabled, and the vulnerability database's resident pages count against MemoryMax even though GOMEMLIMIT cannot see them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U1Km3N3uUmeGaj7VbaM8PF |
||
|
Evan JarrettandClaude Opus 5
|
a63f668de0 |
scanner: fix five crash and halt classes found by a pipeline audit
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 |
||
|
Evan JarrettandClaude Opus 5
|
f16a8eaa82 |
appview: warn about a deprecated chart on the tab people actually land on
|
||
|
Evan JarrettandClaude Opus 5
|
e542dd12f6 |
appview: keep the port in the registry host printed to users
On the dev stack (registry_domains: ["127.0.0.1:5000"]) the /auth/token failure guidance said `docker login 127.0.0.1`, dropping the port docker needs. The message exists to tell a stuck developer what to run, so a command that cannot work is the whole defect. Production is unaffected, both registry domains being port-free. The stripping itself is deliberate and stays: NormalizeService removes the port so JWT audiences match the port-stripped routing host. Removing it would desynchronize the service key from the routing key. The unstripped form turned out to survive in only one place. resolveService returns a key from h.services, which SetServices normalizes, and cfg.Auth.Services is normalized too by deriveServices, so neither holds the original. Only cfg.Server.RegistryDomains, straight off the YAML, does, and it was never reaching the token package. Adds a display-only map keyed by the same NormalizeService function the other two key on, so a resolved service always maps back to the entry it came from, and a multi-domain deployment prints the domain the client is authenticating against rather than the primary. Collisions take the first configured entry, matching deriveServices' own first-wins dedupe. A miss falls through to the normalized name, which is today's behaviour. Audiences and routing are untouched, proven by a test that mints a real token, parses the JWT and asserts aud is still the normalized host. The wiring line landed in the previous commit, both changes having been made in server.go at the same time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
3589473feb |
appview: run the hold purge on a worker pool, not the request context
Deleting a tag while over quota could leave the user in the worst available state. DeleteTagHandler deleted the tag and manifest rows first, then called PurgeOnHold, which bounded itself at 10s against the *request* context. The UpCloud load balancer in front of the appview cuts at its default backend timeout at about the same moment, wins the race, hands the client a 504 and cancels that context, killing the purge partway. The appview logged a warning and returned 200. So: gateway error, nothing freed, still locked out, the image gone from the UI so the purge cannot be retried through it, blobs orphaned until the hold's GC, and the appview considering it a success. Measured on production at 10.002367218s. Purges now go to a fixed pool of 4 workers rooted at context.Background(), so they survive the request ending. Following the shape of the hold's startJob helper, minus the progress fragment, since nobody is watching a purge. The buffer is bounded at 256 and sheds with an ERROR rather than growing: an unbounded queue turns a slow hold into an appview memory leak. Submissions are deduplicated on holdDID|manifestURI so a double-clicked delete does one purge and one service-token fetch. The channel send happens under the mutex that guards close, so a concurrent drain cannot send on a closed channel, and the drain is wired into both exit paths before logging shuts down. Failures are now classified and surfaced instead of swallowed: transient ones retry three times under a 90s budget (the hold's purge is idempotent), an unauthorized third-party hold logs at DEBUG since it is expected, and anything else that exhausts its retries logs at ERROR naming the manifest and hold, which is enough to re-drive by hand. Deliberately not reordered. Purge-first-then-delete requires waiting for the purge to know whether to delete, which puts the 10s call straight back on the request. So the orphaned-blob window remains, materially narrower but real: a purge that exhausts its retries still leaves blobs referenced by nothing until the hold's GC, and there is no row left to say so. Closing that needs a durable pending-purge record, which was judged out of proportion here. server.go in this commit also carries one line belonging to the next one, the token handler's display-name wiring, since the two changes landed in the same file concurrently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
b4fccce4d1 |
hold: actually send subject, so the attestation scan guard can fire
The hold declines to enqueue a scan when a pushed manifest has a subject, which
is how it means to skip attestations, signatures and other referrer artifacts.
The AppView never sent one: notifyHoldAboutManifest built mediaType, config,
layers and manifests, and #manifestInfo defined only those four. So the
condition was always true, the guard never fired, and every referrer artifact
was enqueued for scanning.
The AppView already parsed subject. NewManifestRecord unmarshals it into
ManifestRecord.Subject, and Put hands that same pointer to
notifyHoldAboutManifest. The value was in scope and simply never serialized, so
this is one missing marshal step rather than a missing parse.
Adds subject to the lexicon as a #blobInfo ref, and mediaType to #blobInfo,
which config has always sent and the hold has always parsed. That only makes the
schema honest about what is already on the wire.
Hoists the hold's anonymous request struct to a named type with IsMultiArch,
IsReferrer and HasScannableContent, so the predicate is written once and
testable without standing up a HoldPDS.
Both directions degrade safely. An older hold ignores the unknown key and
behaves exactly as today, so shipping the appview alone is harmless but achieves
nothing until the hold catches up. An older appview sends no subject, leaving
the manifest enqueued as before.
Complements
|
||
|
Evan JarrettandClaude Opus 5
|
9566201377 |
appview: give the confirm modal an accessible name and a focus trap
The modal guards tag deletion, device revocation and webhook deletion, so it is the wrong control to leave unlabelled and escapable. Note the original report was half wrong: role="dialog" and aria-modal="true" were already set on the outer element, and are in the deployed bundle. The observation was most likely taken against the inner .modal-box. Neither was added here. What was actually missing: the h2 existed but nothing pointed at it, so a screen reader announced a dialog with no name; and the only focus management was focusing Cancel on open, so Tab walked straight out into the skip link and page header behind the backdrop. Adds aria-labelledby and aria-describedby with sequence-suffixed ids so two modals cannot cross-reference. aria-describedby matters more than usual here: none of the three call sites' messages contain ". ", so the title is always the generic "Are you sure?" and the specific text is the body. The trap re-queries on each keypress rather than caching at open, and handles three cases: focus escaping to body gets pulled back, first plus Shift+Tab wraps to last, last plus Tab wraps to first. Focus is restored to the opener on every close path, guarded by document.contains, because confirming a tag deletion fires an htmx swap that can remove the button that opened the modal. Restoration happens before onConfirm so htmx sees a sane focus state. Native <dialog> would give the trap and Escape for free, but it renders in the top layer while this is styled entirely with daisyUI .modal classes that assume a positioned div, so converting means CSS work plus the seamark theme fork. Worth doing as its own change, not smuggled into this one. Includes the bundle rebuild, since nothing in the dev loop keeps that artifact current. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
dbb195a4ab |
appview: process daily stats, and drop the dead README branch fallback
io.atcr.hold.stats.daily was handled in the backfill collection list and in processor.go's dispatch, but missing from isRelevantCollection, which gates events before ProcessRecord ever sees them. So daily stats records arrived over the socket and were discarded at the worker, and the trend charts that read them got nothing live. This is the "present in one list, missing from the other" shape CLAUDE.md's firehose checklist warns about. Checked the whole class: this was the only gap. LayerCollection and ImageConfigCollection are absent deliberately, having no processor handler, and the test now pins that intent. Note this is currently masked by the relay outage, so fixing the relay alone would not have restored the charts. Separately, the README resolution tried "main" and fell back to "master", but DeriveReadmeURL never fetches: it parses the source URL and interpolates the branch, returning empty only for an unsupported platform, which is branch-independent. So if the main call returned empty the master call returned empty for the same reason, and the fallback could never fire. Removed, with a comment recording that a branch fallback has to happen at fetch time after a 404. The other two call sites already do exactly that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
a95c89aaef |
docs: record how a hold falls off the relay and stays off
Around 2026-08-20 the Bluesky relay marked every ATCR hold offline and stopped dialing. Nobody noticed for two weeks, and it surfaced only indirectly as "pull and push counts are up to 24 h stale". The stats were a symptom; the fleet was simply disconnected. It cannot recover on its own. Indigo's relay gives up on a host after 16 consecutive dial failures and returns from the redialer, and only a fresh requestCrawl revives it. The hold sends requestCrawl exactly once, at boot (server.go:400), with no ticker and no check that any relay is subscribed. So a dropped hold is invisible until its process restarts, silently. Documents the current mechanics, the failure mode, how to diagnose it with getHostStatus and a frozen repo rev, and how to recover. The automatic re-crawl is described as a deferred proposal and explicitly NOT implemented, by decision: a jittered ticker guarded on subscriber liveness, plus surfacing the subscriber count, since the deeper problem is that this was silent. Two things found while writing it, both recorded. The proposal needs plumbing that does not exist: EventBroadcaster has no exported subscriber count, and Subscriber does not retain the userAgent, so "is a relay listening" cannot currently be answered. And ResubscribeAllHosts selects only active hosts, so an offline host is not recovered even by a relay restart. Carries a replay warning. |
||
|
Evan JarrettandClaude Opus 5
|
25e2aa0228 |
docs: design note for artifact type classification
Finding 32: pushing an artifact with an unrecognised config media type classifies as "unknown", and every template branches two ways on "helm-chart" with the container-image page in the else. So an in-toto attestation is served a docker pull command, Layers/Vulnerabilities/SBOM tabs, "Image layer history", and a promise that scans run shortly after push, which for that artifact will never be true. This is a design note rather than a fix, since the change is larger than the symptom. The inventory is the part worth keeping. The classification rule exists in four places, keyed off three different inputs (appview config media type, hold config media type with no unknown case, scanner config map plus layer shape, hold layer substrings), and the appview's artifact_type feeds none of the scan decisions. Manifest-level artifactType is discarded at parse time on every push: it is absent from the record struct, the constructor and the lexicon, surviving only inside the unindexed manifest blob. Two corrections to the framing this started from, both verified rather than assumed. The repo does not have referrers support: the pinned distribution version has no referrers code and ATCR registers no such route, so what exists is subject_digest persistence plus an attestation badge. And GetTopLevelManifests filters artifact_type != 'unknown', so an untagged unknown artifact is invisible while a tagged one renders as an image, which the finding did not mention. Proposes a type set, spec precedence (manifest artifactType, then config media type, then structural signals), a UI contract stating what such a page must not show, and a six stage plan. Only stage 2 needs a migration, for the raw string column; new slug values need no DDL and no data migration, since jetstream upserts artifact_type on every record it sees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
7a0769d8e4 |
appview: three small UI fixes, empty state, icon sizing, aria-selected
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 |
||
|
Evan JarrettandClaude Opus 5
|
9d8bd513da |
appview: render the install scripts from config instead of shipping ATCR's
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
|
||
|
Evan JarrettandClaude Opus 5
|
2743445e65 |
appview: fix two /auth/token defects, wrong login host and dropped scopes
Both are pre-existing and were found while working on finding 27.
sendAuthError built its "docker login <host>" line from r.Host. /auth/token is
served on the UI domain as well as on every registry domain, and the
WWW-Authenticate realm points at the UI domain's copy, so a client following
the realm was told to run "docker login seamark.dev" - the one host that
deliberately refuses /v2/* with an OCI UNSUPPORTED error pointing at
seamark.cr. It now uses the service resolved for the token, which is the
registry domain, falling back to the deployment's primary rather than to
r.Host. The single-domain case still prints a host that serves /v2/, and an
unconfigured service supplied by the client cannot steer it.
Separately, the scope parameter was read with .Get, taking the first value
only. The Docker token spec allows scope to be repeated, so a client asking for
two repositories was issued a token covering one and got a 401 on the other.
Both wire forms are now flattened, on the GET query string and on the OAuth2
POST body, which had the same defect via PostFormValue.
Empty and whitespace-only values are dropped. Exact duplicate scope strings
collapse, but two entries naming the same repository with different actions are
left alone: merging them would union the action sets, and every gate downstream
is written only to narrow.
More entries now reach the anonymous gate added in
|
||
|
Evan JarrettandClaude Opus 5
|
84872580a3 |
appview: show a chart's deprecation where people actually look
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 |
||
|
Evan JarrettandClaude Opus 5
|
0490278fb8 |
appview: don't report a failed README guess as an error
Four of the twelve home-page repos showed "We couldn't load the README, it may be rate-limited or private". The URL in those cases was not configured by anyone: it was derived from org.opencontainers.image.source, a label images inherit from their base image, so the raw URL named an unrelated project and 404ed. A 404 on a URL the appview guessed is an expected outcome the owner cannot act on. The failure flag is now set only when the owner actually pointed us at the URL, via the io.atcr.readme annotation. A derived URL that fails renders as if there were no README. Both paths keep their debug log, now carrying an "explicit" field so the two cases stay distinguishable. Render failures are suppressed for derived URLs too. The panel's copy and its "Edit README" action address an owner who configured a source; on a derived URL there is no configured source, and content that failed to render is very likely another project's README anyway. This is the alarming half of the finding. Rendering the wrong project's README when the fetch succeeds is the larger half and is untouched: there is no reliable way to detect an inherited label, since the only signal is org.opencontainers.image.base.name, which is not always set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
44a17cbcdc |
appview: stop the upgrade banner inventing an improvement across arch mismatch
On a public repo, to anonymous visitors, the digest banner read "2 fixes, 12 Critical / 27 High / 26 Medium vulns, -22 layers, -53.6 MB" while the same page showed 244 vulnerabilities and Layers (22), and its own "View diff" link landed on "Layers 22 -> 22, every layer Unchanged". Platform matching only ran when both sides were manifest lists. The comment after that block said the mismatched case would "fall through and show a basic banner without layer/vuln details", but no such branch was ever written and nothing guarded the fallthrough, so execution continued into the layer and vuln computation with the unresolved originals still in place. The newer side was the multi-arch index, which carries no layers of its own and is not scanned, so all 22 layers of the other side read as removed and the vuln delta was computed against an absent scan. Returns 204 for either mismatch direction, as the no-common-platform path already does. The promised "basic banner" is not implementable as the function stands, which is presumably why it never appeared: the template renders only NewerTag, DiffURL and Summary, and DiffSummary is nothing but deltas, so a delta-less banner collapses to the tag name and would be suppressed by the existing "nothing meaningful changed" guard anyway. The comment is replaced with one that says what is actually true. Showing a real banner here would mean resolving the index to the child matching the single-arch side's platform, and that side's os/arch is not in the appview DB at all: Platforms is populated only for manifest lists and the manifests table has no os/arch columns. It would need either a config fetch from the hold at render time or denormalising os/arch during ingest. Not attempted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
39919cc832 |
appview: stop webhooks reaching private addresses
The URL check accepted http:// while telling the user "must be https", and guarded no addresses at all. POST /api/webhooks with http://127.0.0.1:9/hook returned 200 and created the webhook, so both scheduled deliveries and the synchronous Test button would dial arbitrary destinations from the appview host, on demand, for any authenticated user. Loopback, link-local (including the cloud metadata endpoint at 169.254.169.254) and RFC1918 were all reachable. Enforces https, and refuses non-public destinations. The load-bearing half is the dial-time check, not the creation-time one. An attacker controls their own DNS, so a hostname that resolves publicly when the webhook is created can resolve to loopback when it is delivered, and a creation-time check cannot see a redirect either. The guard is therefore a net.Dialer Control hook on the delivery client, which inspects the resolved address on every connection attempt. Transport.Proxy is explicitly nil: honouring HTTP(S)_PROXY would route around the Control hook and hand the bypass straight back. Redirects are re-validated per hop and capped at 3. The creation-time check stays so the user gets an immediate, comprehensible error instead of a silent delivery failure later. IPv4-mapped IPv6 is unmapped before every check, so ::ffff:127.0.0.1 and friends hit the IPv4 rules. Ranges with no net.IP helper are listed explicitly: CGNAT, NAT64, ::/96, TEST-NET and reserved space. Both outbound paths are covered, since the scheduled dispatcher and the Test button both funnel through attemptDelivery. The dispatcher's other client is deliberately left unguarded: it fetches quota stats from holds, which legitimately live on private addresses, and those URLs are not user-supplied. Note this removes the ability to point a webhook at a localhost receiver in local development. There is deliberately no environment-variable escape hatch, since a security toggle read from the environment is the same bypass wearing a nicer coat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
dfd604b106 |
hold/scanner: stop one undispatchable job freezing all scanning
Vulnerability scanning produced nothing across the whole deployment for nine days, from 2026-08-25 01:20:48 until a scanner restart on 2026-09-03. The scanner was connected and idle, the hold's discovery pass kept reporting unscannedFound=15 every four hours, and no scan_jobs row was created in that entire window. hasActiveJobs counted pending, assigned and processing rows globally with no age bound, and waitForCapacity spins while it is true. dispatchLoop calls it before popping any candidate, so a single pending row that never reached a terminal state reported "busy" forever: discovery kept pushing candidates into unscannedQueue and nothing ever popped them. That is why the symptom was an empty queue rather than a growing one. Nothing papered over it because push-triggered enqueue only fires for owner or a tier with scan_on_push, which in production means pro alone. All 210 manifests pushed to this hold in that window came from free, supporter, or accounts with no crew row, so the frozen proactive loop was the only source of jobs. Nor could it recover on its own. Only Enqueue and drainPendingJobs dispatch a pending row, and drainPendingJobs runs only when a scanner newly connects; reDispatchTimedOut considered assigned rows only. The hold had been up since Aug 14 and the scanner since Aug 21 on the same websocket, so the drain path had not run since the row appeared. So bound the capacity gate to pending rows younger than pendingStaleAfter, give reDispatchTimedOut a pending reclaim, and check RowsAffected on the assign UPDATE now that two dispatchers can race for a row. waitForCapacity warns and names the blocking jobs after ten minutes without capacity, because the failure mode above was completely silent. Two adjacent fixes for the same outage. The scanner never called InitLogger, so log_level and log_shipper were dead config and an idle scanner was mute, which is what made nine days invisible. And skipReason now also skips a job whose layers contain nothing tar-shaped: the job that wedged this queue was an in-toto attestation whose config mediaType is an ordinary image config, so the existing config-type check missed it and buildOCILayout would have handed Syft an empty image. The regression tests were verified against the old logic first: three of them fail on it and pass on the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPWkeCKcbtGoyXyyeMhSps |
||
|
Evan JarrettandClaude Opus 5
|
af7522b154 |
appview: stop /auth/token granting anonymous pull the registry will refuse
An unauthenticated token request for a repo on a private hold came back with a signed token granting pull, and /v2/ then 401ed that exact token. The authorization server and the resource server disagreed about the same request. The old behaviour was deliberate — "minting a pull-only token is not a grant", with the hold owning the decision via captain.Public — but a token spec expects the server to issue the subset it will authorize, so granting pull and then refusing it is the wrong shape. Adds an optional AnonymousAuthorizer, kept separate from Authorizer because the anonymous path has no DID and no auth method (three of Authorize's four arguments are meaningless) and because it must drop whole entries rather than narrow actions in place, where entries can belong to different owners. Denied entries are dropped; if nothing granting survives, the caller gets the standard 401 challenge rather than a token with an empty access list, so docker prompts for credentials instead of proceeding to a second 401. The scope-less /v2/ ping and the actionless entry NarrowToPullOnly preserves on purpose both bypass the gate entirely — no identity resolution, no hold lookup — since anonymous discovery depends on them. Fails open on any lookup error, matching the /v2/ check, which states the reason: the hold is the enforcing authority and a transient failure must not break anonymous pulls of public images. /v2/ still enforces; this is a correctness and UX fix, not a security fix, and nothing was exposed. Also closes the successor asymmetry documented under finding 3: /v2/ applies a single-hop migration redirect before checking read access, so judging the pre-migration identity here would have reintroduced the disagreement this gate removes. It was one extra local read of hold_captain_records. Tests pin both directions and prove the chain is not followed past one hop. Costs one directory-cached identity resolution plus two local SQL reads per granting entry, and no call to the hold. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
4caeb25031 |
appview: rebuild the JS bundle so the committed asset matches src
Picks up the two JS changes in this batch: the crane destination argument in
updatePullCommand (app.js) and syncDiffMenu (repository.js).
The bundle is a committed build artifact, and nothing in the dev loop keeps it
current on its own. Air's pre_cmd is go generate and its cmd is a Go build; it
never invokes esbuild. Bundling happens in npm run js:watch, a separate
process. So a src-only change leaves the committed bundle stale until someone
runs npm run js:build by hand, which is how
|
||
|
Evan JarrettandClaude Opus 5
|
85f312f953 |
appview: refuse /v2/_catalog with UNSUPPORTED, and send Allow on every 405
/v2/_catalog answered a bare request with 200 {"repositories":[]} but 400ed
any n, because buildDistributionConfig leaves Catalog.MaxEntries at 0 and the
library rejects n > max. crane catalog sends n=1000, so it always failed. The
same endpoint both worked and rejected a legal parameter.
Setting MaxEntries was the obvious fix and is the wrong one. This registry has
no global catalog and will not grow one: repositories live in per-user ATProto
PDS namespaces, and buildStorageConfig hands the library a placeholder
inmemory driver, so its enumeration is empty by construction. An empty 200
asserts that this registry contains no repositories, which is false and
silently so. UNSUPPORTED says the true thing, and matches the vocabulary
DomainRoutingMiddleware already uses to refuse /v2/* on the UI domain.
There is no conformance cost: _catalog is not in the OCI distribution spec at
all. It is a Docker Registry HTTP API V2 extension, and the spec places
repository discovery out of scope. Docker Hub and GHCR both refuse it outright
and Quay returns an unconditional empty list; none of them 400s a legal n.
The path had three distinct behaviours, not two, and all three now collapse to
one: GET varied by parameter, HEAD was answered by gorilla's MethodHandler
with a bare 405, and the trailing-slash form 301-redirected because
distribution sets StrictSlash(true). Both path forms are registered, and chi
prefers a static pattern over the /v2/* wildcard regardless of declaration
order (verified against the pinned chi version, and pinned by a test that
fails if a request reaches the distribution stand-in).
Also adds the Allow header that RFC 9110 requires on any 405 — a MUST in both
15.5.6 and 10.2.1, not a SHOULD. The value is empty, which 10.2.1 defines as
"the resource allows no methods": true for the catalog, and true for /v2/* on
the UI domain, so the pre-existing gap in DomainRoutingMiddleware is closed
too. Naming a method there would advertise something that does not work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
|
||
|
Evan JarrettandClaude Opus 5
|
8bc6d65e1e |
appview: emit crane's mandatory destination in the pull command switcher
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 |
||
|
Evan JarrettandClaude Opus 5
|
3158460298 |
appview: hide the current tag from the repository Diff dropdown
The Diff menu listed every tag including the one being viewed, and clicking
that entry did nothing: diffToTag returns early on to === currentTag, with no
navigation, no toast, no feedback.
Fixed in JS rather than in the template, which is the part that is easy to
get wrong. #diff-dropdown sits outside #tag-content, and the tag selector
block is marked "stays in DOM, never swapped" — so a {{ range }} filter would
be correct on first paint and stale after the first htmx tag swap, omitting
the originally loaded tag and re-including the newly current one. Same dead
entry, harder to see.
syncDiffMenu() reads the live value from #tag-selector and is called from
initTabs(), which already runs on load and again from the htmx:afterSettle
handler for #tag-content, so it stays correct across swaps.
Hidden rather than disabled: a disabled row still takes space and still reads
as an item to a screen reader, and "diff against the tag you are already on"
is meaningless rather than temporarily unavailable. Uses style.display to
match filterTags() in the same file, since daisyUI's .menu li rules outrank
Tailwind's .hidden.
The template guards the dropdown with {{ if gt (len .AllTags) 1 }} and tag
names are unique per repo (tags PK is did+repository+tag), so exactly one
entry is ever hidden and the menu can never end up empty. The early return in
diffToTag stays as a backstop.
Pre-existing, not a deploy regression. The baseline missed it because the
test repo had one tag, so the check skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
|
||
|
Evan JarrettandClaude Opus 5
|
1631898005 |
appview: give every Repository() error an OCI code instead of 500 {}
A user with an unresolvable defaultHold (did:web:localhost%3A8080) got HTTP
500 with a body of literally {} on every request in their namespace, and
crane retried it three times because 500 is retryable.
The cause is in the distribution library. handlers/app.go:755 switches on the
error type returned by Repository() with cases for ErrRepositoryUnknown,
ErrRepositoryNameInvalid and errcode.Error, and no default. A bare fmt.Errorf
matches none of them, so context.Errors stays empty; ServeJSON then finds no
ErrorCoder, leaves sc == 0, falls through to 500, and Errors.MarshalJSON
renders the nil slice as {} via omitempty.
So every error leaving Repository() must be coded. Four were not:
hold URL unresolvable -> 404 NAME_UNKNOWN when errors.Is
atproto.ErrHoldDIDPermanent, else 503 UNAVAILABLE
no hold DID configured -> 500 UNKNOWN, but with a body and a log line
invalid image name -> 400 NAME_INVALID
name missing an owner -> 400 NAME_INVALID
The permanent/transient split is the point: a DNS blip must stay retryable,
but a did:web that can never resolve must not be retried at all. Stored user
data that cannot resolve is a 4xx condition, not a server fault, and the
NAME_UNKNOWN message now names the hold so the owner can fix their profile.
The hold DID is already world-readable in their sailor profile record.
Tests assert through a helper that replays distribution's exact type switch,
so an uncoded error still surfaces as 500 {} and the assertions bind to the
real behaviour rather than to the constructors.
Does not address the logrus line at app.go:757, which fires unconditionally
before the type switch and cannot be avoided by any returned error type.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
|
||
|
Evan JarrettandClaude Opus 5
|
2ee5a35525 |
appview: stop the schema-drift check reporting phantom index differences
reportSchemaDrift logged 76 differences on every boot, all false positives: 38 indexes each reported twice, once as missing from the database and once as present but undeclared. The only difference was a single space before the column list. The two sides of the comparison are built differently. referenceSnapshot() applies schema.sql to a local in-memory libsql, which stores the CREATE text verbatim as "ON t(col)". Production is an embedded replica syncing to Bunny, whose parser re-emits normalized DDL as "ON t (col)". describeIndexes collapsed whitespace runs but could not normalize a space that exists on one side only, and SchemaDrift compares by exact string. Normalize whitespace adjacent to ( ) and , so both spellings converge. Space after ) is deliberately left alone, and the space before DESC is untouched, so column order and direction still have to match. This mattered because the check exists to catch a migration recorded but not executed (0004 was, which is why 0009 exists). At 76 phantom findings, real drift would have been one line among 77, under a hint telling the operator to write a corrective migration that is not needed. The first boot after this lands is the first honest reading of that warning. The existing tests all passed because they run local-only, which is exactly how this shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 |
||
|
Evan JarrettandClaude Opus 5
|
a63b839613 |
appview: gzip UI and static responses, leaving /v2 alone
Go's net/http compresses nothing by default and the appview is served by a
raw Go server behind a load balancer that does not compress either, so
everything went out uncompressed: style.css at 180 KB, bundle.min.js at
105 KB, and the homepage HTML at 95 KB. Lighthouse put style.css alone at
1,768 ms of blocked first paint, and mobile Performance measured 77 against
98 on desktop, entirely on paint metrics (TBT 10ms, CLS 0).
klauspost/compress is already a direct dependency and ships gzhttp, so this
costs no new one. Brotli would save roughly 11 KB more across the three
largest assets in exchange for a runtime dependency, which is not worth it.
The /v2 skip is the part that needs care. The OCI registry API is mounted on
the same chi router as the UI (server.go:561), and container layers are
already gzipped tarballs, so compressing that path burns CPU for no gain.
The content-type allowlist would catch most of it, but /v2/* also serves
application/json for tag listings and errors, so the path check keeps the
registry out of the compression path entirely. The predicate matches the one
the domain-routing middleware already uses.
Measured against a local build, gzip vs identity:
/ 14,526 -> 4,298 71%
/css/style.css 183,990 -> 31,596 83%
/js/bundle.min.js 107,781 -> 32,648 70%
/icons.svg 26,360 -> 8,403 69%
total 332,657 -> 76,945 77%
Verified end to end against a running binary: UI and static responses carry
Content-Encoding: gzip with Vary: Accept-Encoding, /v2/ and /v2/*/tags/list
carry neither, and woff2 stays untouched. Tests cover both directions plus
the under-1 KB and no-Accept-Encoding cases.
HTTP/2 is the remaining half and cannot be fixed here: the load balancer
terminates TLS and negotiates no ALPN at all, which pins every request to
HTTP/1.1. That is an LB setting.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0124r73LT4qoFE82TqwH2Gu9
|
||
|
Evan JarrettandClaude Opus 5
|
74778bdd05 |
appview: apply the mockup-code contrast fix to the seamark theme
|
||
|
Evan JarrettandClaude Opus 5
|
cbd0c5f05c |
appview: rebuild the JS bundle so the committed asset matches src
The tracked bundle predated
|
||
|
Evan JarrettandClaude Opus 5
|
27fab41c1c |
appview: make the webhook cap agree between creation and delivery
The UI and the dispatcher asked the same question and got different
answers. getWebhookLimits short-circuited on a disabled billing manager and
returned unlimited without consulting it, while server.go hands
BillingManager.GetWebhookLimits straight to NewDispatcher, bypassing that
short-circuit entirely. With billing compiled out the stub answers
non-captains with (1, false).
So a non-captain saw "N / unlimited webhooks configured", could create as
many as they liked, and only the oldest was ever delivered. allTriggers was
false on that same path, so even the surviving one was restricted to
FreeTriggerMask; a webhook set to a scan trigger fired nothing at all, with
no message anywhere and only an INFO line server-side.
Route both paths through the manager so they cannot drift. The intended
non-billing policy is one webhook with TriggerFirst | TriggerPush |
TriggerQuota, which is what the stub already returned and what the shipped
config's Free tier specifies (max_webhooks: 1, webhook_all_triggers: false),
so enabling billing leaves free users exactly where they were and only
unlocks upward. That also removes a downgrade cliff: nobody can accumulate
webhooks under a phantom unlimited and lose them when billing turns on.
Drop the dead webhookLimits{Max: 1}, overwritten on the following line, and
give a nil manager the same policy rather than a third answer.
Latent, not live: production has zero webhooks configured today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk
|
||
|
Evan JarrettandClaude Opus 5
|
62aea5f3f0 |
credhelper: print the verification URL that carries the code
The interactive prompt named codeResp.VerificationURI while openBrowser was handed verificationURL, the one with ?user_code= appended. Pressing Enter therefore always worked, which is why this went unnoticed; copying the printed URL instead landed on /device with no code. That matters more than it looks, because the branch tests the wrong thing. isTerminal(os.Stdin) asks whether stdin is a TTY, not whether a browser exists, so an SSH session on a headless box takes the headed path and is told to press Enter to open a browser it does not have. The non-interactive branch, which already printed the full URL, is only reached by piping stdin. Printing the code-carrying URL in both branches makes that mismatch moot rather than requiring a smarter predicate. Also give /device without a code the styled device-error page instead of bare text/plain, matching the expired-code path beside it, and stop renderError panicking when Templates is nil. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk |
||
|
Evan JarrettandClaude Opus 5
|
a219df9545 |
appview/js: fix webhook test result, 400 toasts, and Layers tab init
Three unrelated client-side defects found while baselining production. The webhook Test button always reported success. renderAlert writes no status code, so both outcomes are HTTP 200 and the result lives in the markup, which partials/alert.html emits as "alert alert-error". testWebhook looked for class="error", which that string does not contain, and resp.ok is always true, so the failure branch was unreachable. A webhook pointed at a dead URL was reported as delivered. Match alert-error instead. Avatar upload rejections lost the server's reason. The htmx:responseError handler maps status codes to fixed strings and had no 400 case, so "File too large (max 3MB)" and "Invalid file type" both surfaced as "Something went wrong". Surface the body when it is short plain text; a rendered error page or a long trace is not toast material. The repository page's Layers tab was never initialised. initLayersTables runs from DOMContentLoaded, when the panel is still a spinner, and from htmx:afterSettle, which htmx.process() does not emit. So empty-layer hiding and no-history run collapsing never ran there, and the checkbox claimed rows were hidden while all of them were on screen. Export it and call it from the tab controller. It now also re-seeds checkboxes within the loaded scope, which fixes the digest page contradicting itself: the stored preference was honoured for the rows and ignored for the control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk |
||
|
Evan JarrettandClaude Opus 5
|
1253ca15ec |
appview: report a never-scanned image as unscanned, not as a failure
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 |
||
|
Evan JarrettandClaude Opus 5
|
27ce122db0 |
auth: stop logging an unresolvable hold DID at ERROR
A hold DID that can never resolve is a property of stored user data, not a fault on our side. The value comes from a user's own sailor profile defaultHold, so any account can choose the appview's ERROR volume, and nothing is cached on the failure path, so it re-logs on every request for that user. On production this was not a rounding error: two accounts pointing at did:web:localhost%3A8080 produced 2956 of 2958 ERROR lines over seven days, 99.9%. The genuine rate underneath was about two a day, which made level=ERROR useless as a signal or an alert threshold. Classify at the resolution boundary instead of string-matching prose. ErrHoldDIDPermanent marks a malformed identifier or a missing DID document; those log at DEBUG while everything an operator could act on stays at ERROR. didWebHostUnusable is conservative on purpose: it only claims the cases we are sure about (percent-encoded ports, bare IPs, localhost), so an unfamiliar failure stays loud rather than being quietly swallowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk |
||
|
Evan JarrettandClaude Opus 5
|
a4aedbd2c8 |
gc: retry transient PDS failures instead of pinning a user's storage
Both paginated walks bailed on the first error, and the caller treats a failed walk as "assume everything is referenced", so one blip skipped that DID's storage for the whole run. Measured before changing anything: every DID GC had classified unreachable but healthy failed only one or two runs out of six, and replaying the exact same listRecords calls afterwards returned 200 in 45-680 ms with no rate limiting. Ordinary blips on small self-hosted PDSes, amplified into a full-DID skip. Share one listRecordsPage helper between fetchUserTags and fetchUserManifestsFromEndpoint. The retry decision splits deliberately: timeouts, connection reset, 5xx and 429 get another attempt, while DNS failure, TLS failure, connection refused and any 4xx do not. Those are stable facts about an endpoint, and retrying them would only slow the run and keep a dead PDS looking alive longer. Unrecognised errors stay permanent, so an unfamiliar failure degrades to today's behaviour rather than hammering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk |
||
|
Evan JarrettandClaude Opus 5
|
c36e90f6b7 |
hold/admin: swap out the deleted crew row instead of sending 204
The delete handler returned 204 No Content for htmx requests, on the theory that an empty body plus hx-swap="outerHTML" would make the row disappear. htmx's default responseHandling maps 204 to swap:false, so it never swapped at all: the record was gone from the PDS but the row stayed on screen until a manual refresh. Return an empty 200, which htmx does swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ai43R3s33cBGMybGp2gUcG |
||
|
Evan JarrettandClaude Opus 5
|
2a58ccebd8 |
hold/gc: name the third ownership state instead of encoding it as a lie
|
||
|
Evan JarrettandClaude Opus 5
|
264d332bbd |
hold/gc: stop an unreachable hold's manifests from being adopted
|
||
|
Evan JarrettandClaude Opus 5
|
fd8e4b0bde |
docs: correct stale credential-helper and workspace layout in CLAUDE.md
The documented build command `go build -o bin/docker-credential-atcr ./cmd/credential-helper` has not worked since the helper was split into per-brand modules: that path holds no Go files, so the command fails with "no Go files in .../cmd/credential-helper". Point it at cmd/credential-helper/atcr, grouped with the scanner under a "separate modules" heading since both need the cd-and-build form, and note that `make build-credential-helper` is the same build with version/commit ldflags stamped. The workspace section claimed two modules; there are five. The two credential helpers and deploy/upcloud were missing, and the main module no longer contains the credential helper. Added why the helpers are split out at all: `go install atcr.io/cmd/credential-helper/atcr@latest` has to resolve without the main module's dependency tree, which is what the `require atcr.io vX.Y.Z` pin in each is for. Both documented commands verified against the current tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KWoKzpgtBJ33sCyGxJGR7x |
||
|
Evan JarrettandClaude Opus 5
|
15871ad188 |
deps: update all modules, bump go and builder images to 1.26.7
Update every direct dependency across all five workspace modules to latest. Notable jumps: syft v1.43.0 -> v1.51.1, grype v0.111.1 -> v0.118.0, stereoscope v0.1.23 -> v0.3.1, indigo -> 2026-09-01, aws-sdk-go-v2/service/s3 v1.99.1 -> v1.110.0, grpc v1.80.0 -> v1.83.2, x/crypto v0.50.0 -> v0.55.0. Three deps needed more than a version bump: go-libipfs could not be updated at all. The repo was renamed to boxo, so every tag past v0.7.0 declares `module github.com/ipfs/boxo` and cannot be required under the old path. sqlite_store.go already imported go-block-format alongside it and used the archived package exactly once, inside a function already returning blockformat.Block, so it was relying on structural interface satisfaction. Collapsing to the native type drops the archived dependency entirely. go-didplc moved its package from the repo root into a didplc/ subdir in v0.2.2. Package name is unchanged and every symbol we use (RegularOp, OpEnum, OpService, Client.DirectoryURL, Submit) is intact, so this is an import path change only. The go-diskfs replace in scanner/go.mod had inverted. It pinned v1.7.0 because syft v1.43 passed diskfs entries as os.FileInfo; syft v1.51.1 fixed that upstream and now requires v1.9.4, so the workaround had become the thing breaking the build. Removed per its own "Remove when syft ships a fix" note, closing anchore/syft#4796 for us. The indigo bump needed no code changes: of the 21 packages we import only 5 changed, and the repo/MST/CAR-store core is byte-identical. It does bring a util/ssrf fix blocking 6to4 addresses (2002::/16), which we inherit through atproto/auth/oauth. Go 1.26.7 across go.work, all five go.mod files, the four Dockerfiles, the three tangled workflows, and the stale references in docs/DEVELOPMENT.md. Verified golang:1.26.7-trixie resolves on mirror.gcr.io, which is what the Dockerfiles actually pull from. Makefile's TRIXIE_BUILDER_IMAGE stays on the floating golang:1-trixie. make test, make lint, and make test-race all pass, as do the scanner module's tests and the integration-tagged build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KWoKzpgtBJ33sCyGxJGR7x |
||
|
Evan JarrettandClaude Opus 5
|
606338b33e |
appview: fix mockup-code contrast in both themes
daisyUI dims the gutter prefix with `opacity: .5`, which multiplies against whatever opacity the line's own text color carries. On bg-base-300 a plain `$` measured 3.20:1 in light and 4.19:1 in dark, and the `#` on a line already dimmed to /70 compounded to 2.15:1. The text needs 4.5:1, so this failed in dark mode too, just less visibly than the light-mode report that surfaced it. Give the prefix an absolute muted color instead of a multiplying opacity. The override has to sit unlayered: daisyUI ships this selector in `@layer daisyui`, declared after `@layer components`, so a rule in components loses on layer order however specific it is. A first attempt inside components left daisyUI's `opacity: .5` live on top of the new 70% color, which made the `$` worse (0.5 -> 0.35 effective) rather than better. The hero tagline drops its /70 and now reads at the same weight as the docker commands above it. install.html's two comment lines were at /50, which failed on the text itself (3.20:1 light), and move to /70 where they still read as comments. Measured in Chromium against the built stylesheet, compositing each pseudo-element color over its real background on a canvas: every prefix and comment is now 5.84:1 light / 6.71:1 dark, against 14.03:1 / 12.02:1 for the command text, so the gutter stays visibly de-emphasized. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpqkSRqpcnAjsSZgFTUN5z |
||
|
Evan JarrettandClaude Opus 5
|
ee9ccc1fb8 |
appview: drop crossorigin from the imgs.blue preconnect
Connections are keyed by credentials mode. The crossorigin attribute opened the imgs.blue socket in anonymous-CORS mode, but every request to that origin is a plain <img src> avatar fetch (BlobCDNURL / resizeImage) in no-cors mode, so nothing could reuse it. The browser opened a second connection anyway and Lighthouse flagged the hint as unused while still listing imgs.blue as a preconnect candidate worth ~300ms of LCP. The attribute is correct for the font preloads below, which is where it was likely copied from; comment the distinction so it stays put. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LpqkSRqpcnAjsSZgFTUN5z |
||
|
Evan JarrettandClaude Opus 5
|
777bd15149 |
build: race-check the billing package too
test-race runs `go test -race ./...`, which is untagged, so pkg/billing prints "no test files" and the money path has never been through the race detector. This is the same gap batch 13 found in `test`, where the only -tags billing line in the Makefile was a build line and gate_test.go had never executed in CI. That one was fixed by adding test-billing; test-race was left behind. It is not a theoretical gap. UpdateCrewTierOnAllHolds fans out to every managed hold concurrently and joins the errors, and RefreshHoldTiers reads and writes holdTierCache under a mutex from a background worker while request handlers read it. Those are the two places in the package where a race would actually live. Passes: 2.059s, no races reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rmjvU2gSRL9wFnmqVsWaF |
||
|
Evan JarrettandClaude Opus 5
|
521cf143e5 |
appview: cover the half of 13edb71 that had no tests
That commit throttles two writes and states a statement order. Only one of the three claims was defended. touchLastSeen is the hotter of the two writes — it ran once per indexed record, so a busy firehose meant a database round trip per event for a timestamp read in hours or days. Deleting its throttle outright left every existing test green, as did keying it globally instead of per DID, which would let one busy account suppress every other account's first write. Both now fail. The statement order is the third claim: UpdateLastUsed stamps the throttle before the write rather than after, so a slow or failing write cannot let every concurrent caller through to queue another attempt behind it. That matters because this runs on the authentication path, once per layer during a push, and the pile-up is worst exactly when the database is least able to absorb it. A failing write makes the ordering observable without timing anything: with the stamp after the write every call retries, with it before only the first does. Dropping the table leaves no row to inspect, so the attempts are counted through the warning the function already logs. Under the reordering it reports 10 attempts across 10 calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rmjvU2gSRL9wFnmqVsWaF |
||
|
Evan JarrettandClaude Opus 5
|
ff0942196e |
db: cover the orphan drop in 0034, the case production actually presents
TestMigration0034PreservesLayersAndReferences seeds no orphan and says why: the live foreign key refuses to create one, so the migration's join through manifest_id is "insurance for a database whose foreign keys were off at some point, not for anything reachable now". Production is that database. It carries 160 layers and 22 manifest_references pointing at manifests.id values that no longer exist, left by deletes performed under mattn/go-sqlite3, where the constraint the DDL declared was not enforced. libSQL turns foreign keys on by default and mattn did not, so the rows predate the driver swap. The insurance is load-bearing on the only database that matters, and nothing tested it. The new case rebuilds the pre-0034 shape with the child foreign keys absent, which is what that era's schema behaved like, and seeds three orphaned layers and two orphaned references beside live ones. It asserts the orphans are gone, the live rows survive attached to the right key, nothing lands keyless, and foreign_key_check is clean afterwards. Verified against the defect: with the child manifest_key made nullable and the joins turned into LEFT JOINs, all five orphans survive and the test fails on both counts. Recorded honestly, the two guards are redundant with each other — LEFT JOIN alone still drops them, because INSERT OR IGNORE swallows the NOT NULL violation. Only removing both carries an orphan forward, and such a row counts, selects, and joins to no manifest ever again. Confirmed on a copy of the production database: layers 18803 -> 18643 and manifest_references 2175 -> 2153, exactly the rows the join excludes, with manifests unchanged at 3864. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rmjvU2gSRL9wFnmqVsWaF |
||
|
Evan JarrettandClaude Opus 5
|
8c9a85826d |
appview: stop leasing the billing tier refresh, and let it be cancelled
|
||
|
Evan JarrettandClaude Opus 5
|
f0b28c04f5 |
db: cover the DID guard on the batched tag delete
TestDeleteTagsNotInListScopedToDID passes a nil keep list, which takes the
early return and deletes with `DELETE FROM tags WHERE did = ?`. So the scoping
it proves is the shortcut's, not the one inside the chunk loop that
|
||
|
Evan JarrettandClaude Opus 5
|
18c77ace28 |
test/e2e: drive the billing Drive list against the Stripe sandbox
Three scripts covering what only a browser can see, run end to end against the sandbox with a real checkout and a real portal cancellation. batch13-billing-drive.mjs runs the same account either side of one config change -- whether its default hold appears in server.managed_holds. Managed: the billing tab offers real tiers, checkout 302s to Stripe, the portal is reachable. Self-hosted: checkout 403s, the portal still 302s, and the advisor answers managed_hold_required rather than upgrade_required, which matters because telling a paying subscriber to "upgrade" would sell them a tier they already hold. batch13-portal-cancel.mjs walks into Stripe's portal instead of asserting the redirect, because the batch card calls a subscriber who cannot cancel the worst outcome here and a 302 does not prove a cancel control exists at the far end. batch13-webhook-downgrade.mjs creates three webhooks under an allowance of ten, then reads the page back after the downgrade. Every one of these is invisible on a hold owner's account: GetSubscriptionInfo returns a synthetic "Captain" tier before any Stripe lookup, and GetWebhookLimits / HasAIAdvisor / GetSupporterBadge bypass on the same first line. The first run used the shared e2e profile, which still had the owner signed in, and reported a clean pass built entirely on that bypass. The scripts now assert the page does not render "Captain", and take a separate profile. Two traps worth keeping: a bare button[type=submit] matches the nav's hidden logout button before the form's own submit, and hx-confirm here renders a custom modal whose backdrop swallows clicks -- strip the attribute rather than trying to dismiss a dialog that never fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB |
||
|
Evan JarrettandClaude Opus 5
|
fa6473a896 |
appview/holdclient: cover the tier fan-out itself, and its failure path
The existing tests covered updateCrewTierWithRetry and UpdateCrewTierOnHold. UpdateCrewTierOnAllHolds -- the function the Stripe webhook actually calls, and whose error decides whether a paid upgrade is retried or dropped -- had none. Three cases: the joined error names every failing hold and not the one that succeeded; a hold that accepts and never answers does not starve the holds after it (mutation-verified by making the fan-out serial, which leaves the healthy hold contacted zero times); and a context deadline aborts the retry loop rather than running to tierUpdateMaxAttempts. That last one records a real mismatch rather than an intent. Three attempts at a 5s client timeout need ~15s, and the webhook allows the whole fan-out 10s, so under a hang the budget funds two attempts and never three -- confirmed against a blackholed hold on the dev stack, which failed at exactly 10.0s with a bare context error rather than the "after N attempts" wrapper. If either constant or the deadline moves, that test is where the arithmetic gets re-checked. Also covers the other half in pkg/billing: a fan-out failure has to reach Stripe as a 5xx and leave stripe_processed_events empty. A hold that is briefly down otherwise costs the customer their tier permanently -- the same shape of loss as the customer-lookup hole, one layer further out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB |
||
|
Evan JarrettandClaude Opus 5
|
4dd473bbf1 |
hold/pds: cover HandleUpdateCrewTier, which had no test
The hold end of the billing fan-out had no test at all -- only the ErrCrewMemberNotFound sentinel was covered. Its answer decides whether the Stripe webhook records an event as processed or retries it, so each status it can return means something different upstream and is covered separately: the applied path (asserting the stored crew record actually changed, not just the response body), not-crew as a successful no-op, the 403 on a body userDid that disagrees with the signed subject, an empty body userDid falling back to the token subject, 401 unsigned, 400 with no tiers configured, and rank clamping. Each was mutation-verified. One of them corrected the test's own comment: removing the 403 guard does not let a body retarget a grant, because every step after it keys off the token's sub claim and req.UserDID is read nowhere else. The guard makes a disagreeing body loud rather than silently ignored, and the stored-tier assertion is the regression guard for the day something reaches for that unsigned field when it needs "which user". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB |
||
|
Evan JarrettandClaude Opus 5
|
2d30f6abb7 |
test/stripe-integration: give the suite a real database
The suite built its Manager with a nil database, which switches off every
`m.db != nil` branch in HandleWebhook: the idempotency check, the per-customer
ordering guard, and the processed-event record. It ran against real Stripe and
exercised none of the code those guards live in, so it read as far broader
coverage than it was.
It now opens a libsql file under t.TempDir (":memory:" is per-connection, so
the pool's second connection would see no tables), and two new tests cover the
branches that were dead: a redelivery is skipped, and a stale out-of-order
event is ignored. Both assert that the second delivery did not reach a handler
rather than counting rows -- RecordStripeEvent is an idempotent upsert, so
deleting either guard leaves the table identical. Both were mutation-verified
against the guard they cover.
Two fixes fall out of turning the database on:
buildEventPayload never set `created`, which unmarshals as 0. Harmless with no
database; with one, the ordering guard reads every later event for a customer
as older than what it already applied, so the second event silently becomes a
no-op. It is stamped now, with buildEventPayloadAt for the ordering test.
TestHandleWebhookAllSubscribedEvents was failing on this branch and nothing
caught it, because stripe-integration-test is not part of `make test`. It
posted events for the literal "cus_fake" and expected "No user DID found" --
which was true only while a failed customer.Get collapsed to an empty DID.
Since that became a retryable error, the fixture reached the error branch
instead. It now uses a real sandbox customer carrying no user_did, so the
assertion tests the branch it names.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB
|