mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-02 16:26:56 +00:00
fd8e4b0bde701551d813fb77ed66a92c3d2c3db6
591
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  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
|
||
|
Evan JarrettandClaude Opus 5
|
3263156067 |
billing: key entitlements on the Stripe product, not the price
Tier resolution matched a subscription's price ID against the configured stripe_price_monthly/stripe_price_yearly, which inverts what a price change is supposed to do. Stripe prices are immutable, so changing what a tier costs means creating a new price, and Stripe never migrates existing subscribers off the old one. Updating the config to the new price IDs therefore un-tiers precisely the subscribers a price change is meant to leave alone. They did not even drop cleanly to free. An unresolved tier logs a warning, returns nil, and the event is recorded in stripe_processed_events -- so Stripe answers 200, never redelivers, and a later dashboard Resend is swallowed by the idempotency check. Reproduced against the sandbox: a subscription on a price the config does not list granted nothing, and the event could not be replayed afterwards. A tier has one product and many prices over its life, so the product is the durable key for an entitlement. Tiers gain a stripe_product field, and resolution tries the product first, falling back to the price IDs so configs without it keep working unchanged. Checkout still keys on price -- that direction has to name a specific price to charge. Verified live: a subscription on a price created outside the config, under the Pro product, resolved to tierName=Pro tierRank=2 and landed on the hold. The same shape with an unknown product produced the silent no-op before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB |
||
|
Evan JarrettandClaude Opus 5
|
4b9d4bcbeb |
billing: retry a failed customer lookup instead of dropping the subscription
getCustomerDID returned "" for a FAILED customer.Get exactly as it does for a
customer carrying no user_did. handleSubscriptionChange read that as "not our
customer" and returned nil, so HandleWebhook recorded the event as processed
and answered 200. Stripe never redelivered. A transient Stripe API error
therefore dropped a paid upgrade permanently — the precise "paid but never
received tier" hole
|
||
|
Evan JarrettandClaude Opus 5
|
894dd243da |
hold/admin: cover the top-users panel 7d9de7c fixed
|
||
|
Evan JarrettandClaude Opus 5
|
f4d0c8bf05 |
auth: verify the hold before caching a captain record on the third path
|
||
|
Evan JarrettandClaude Opus 5
|
5112425673 |
appview: make the footer Bluesky link configurable
The link was hardcoded before |
||
|
Evan JarrettandClaude Opus 5
|
34b4516aa7 |
test/e2e: correct the claim that headed Chromium cannot launch here
It can, and normally in well under a second. Two launches hung for the full 180s handshake timeout under heavy concurrent docker and test load, and I wrote that up as "headed is impossible from an agent shell" and moved everything to headless. That was wrong, and wrong in a way that would have quietly degraded every future browser check. The display is reachable: DISPLAY=:0, XAUTHORITY set to the mutter XWayland cookie, both the Wayland socket and /tmp/.X11-unix/X0 present, xdpyinfo happy. The README now says to check xdpyinfo and retry rather than conclude anything, and batch10-anonpull.mjs defaults to headed again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh |
||
|
Evan JarrettandClaude Opus 5
|
17e25a4df2 |
appview: guard the tag-listing paging that gates a shared-record delete
The delete path decides whether an io.atcr.manifest record is still wanted by
enumerating the DID's tag records. That enumeration is load-bearing in a way a
tag listing usually is not: records for every one of a DID's repositories share
one collection, so a single page is a per-account budget rather than a per-repo
one. Past it a live tag falls off the end, the digest reads as unreferenced,
and the shared record is deleted out from under a repository nobody touched —
along with its layers on the hold.
Two cases, both mutation-verified:
* the tag that keeps the digest alive sits on page 3. Stopping after the
first page deletes the record. cleanupUntaggedManifest has carried this
hazard in a comment since
|
||
|
Evan JarrettandClaude Opus 5
|
b17ebb69a5 |
test: cover the nested-repo tag rkey on the delete paths
|
||
|
Evan JarrettandClaude Opus 5
|
0b212a527f |
appview: guard the two UI delete paths against cross-repo destruction
Same defect as the OCI path, two more places. The io.atcr.manifest record is
keyed by digest alone, so one record backs every repository of a DID holding
identical content, and both of these deleted it without asking whether another
repository still wants it — then purged the layers on the hold.
DeleteManifestHandler already removes this repository's tags before deleting
the record, so a tag remaining at that point can only belong to another
repository. It now checks IsManifestTaggedAnyRepo there and keeps the shared
record when one does, reporting sharedRecordKept so the caller can tell the
difference between "deleted" and "deliberately left alone".
DeleteUntaggedManifestsHandler is the subtler one. Its digest list comes from
GetAllUntaggedManifestDigests, whose tag join is scoped to one repository
(m.repository = t.repository), so a digest tagged only in a DIFFERENT
repository is reported as untagged and swept. The query is a reasonable
per-repository view and a dangerous delete list; the guard goes at the delete,
not in the query, matching how DeleteTagHandler already works. Skips are
counted separately from failures, because a skip is the guard working and
folding it into "failed" would make a correct run look broken.
Both fail closed. Leaving a manifest behind is recoverable; deleting one
another repository is still serving is not.
The new db test pins both halves of the interaction: that the query really does
report a cross-repo-tagged digest as untagged, so a change there is noticed,
and that IsManifestTaggedAnyRepo answers DID-wide, which is the thing actually
standing between the sweep and another repo's live image.
DeleteTagHandler needed no change —
|
||
|
Evan JarrettandClaude Opus 5
|
701c866723 |
appview: stop an OCI manifest delete destroying another repo's image
The io.atcr.manifest record is keyed by digest alone (digestToRKey), so one
record backs every repository of a DID holding identical content.
ManifestStore.Delete removed it unconditionally and then called
purgeDeletedManifest, which asks the hold to drop the layer records and free
the blobs. Deleting through one repository therefore stripped the manifest out
from under every other repository sharing that digest and took their bytes with
it — reachable from the public OCI API with `crane delete`, and not recoverable.
Reproduced end to end before fixing: push identical content to shared-a and
shared-b, delete shared-a by digest, and shared-b:v1 answers 404.
The codebase already had the right guard and the right policy written down.
|
||
|
Evan JarrettandClaude Opus 5
|
3dcb4b5c02 |
test/e2e: document the batch-10 traps
Five things that each produced a confident wrong answer during val/10-anonpull, so the next batch does not rediscover them: the repo page route, tags living in a <select>, headed Chromium not launching from an agent shell, logged-out checks needing their own browser profile, and the dev hold defaulting to public:false with a propagation delay after the flip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh |
||
|
Evan JarrettandClaude Opus 5
|
43bf79c71f |
test/integration: make every pull client actually read a blob
Two of the three matrix clients never fetched one. orasClient.Pull stopped at
repo.Resolve (a HEAD on the manifest) and regclient's stopped at ManifestHead,
so their pull rows — including anonymous_pull_denied, stranger_pull and
crew_read_only_pull on the private-hold matrix — were passing without ever
exercising blob authorization. They passed on the manifest denial alone.
That is invisible from the outside because it produces the right verdicts for
the wrong reason. The full suite is still green after the fix, so no
authorization bug was hiding behind it; what was hiding was the coverage.
craneClient.Pull was already correct:
|
||
|
Evan JarrettandClaude Opus 5
|
00e2897c30 |
test/e2e: drive the anonymous repo page, public and private
The API half of val/10-anonpull is covered by the auth matrix. This covers what
Go tests structurally cannot see: whether a logged-out repo page renders, 500s,
or comes back as a blank panel.
Public hold, logged out: 200, all four tags render, and the digest on the page
matches what the registry serves for the selected tag. That last check compares
against whichever tag is selected rather than a hardcoded one, because the page
renders the selected tag's digest and a hardcoded comparison silently fails
whenever the default changes.
Three instrument bugs are baked into the script as comments, because each of
them produced a confident wrong answer first:
* The repo page is /r/{handle}/*, not /{handle}/{repo}. The latter is a 404
"Lost at Sea" page, which reads exactly like a denial if you don't check.
* Tags are <option>s in a <select>. Scraping a,td,span finds nothing and
reports "no tags" on a page that is rendering them correctly.
* Logged-out checks use a throwaway persistent profile. A plain
chromium.launch() does not complete its handshake here, and clearing the
shared profile's cookies would cost an interactive re-login.
Private hold, logged out: the plan expects a denial. It is not what happens —
the page returns 200 with every tag listed while /v2/ refuses the same repo
with 401 for the same caller. Recorded as a documented divergence rather than
asserted as a failure: the manifest records are world-readable in the user's
PDS by design, so nothing secret is exposed, and /r/ has used OptionalAuth
since before validate-base with the page never consulting captain.Public. The
two read paths simply disagree, and that predates this range.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh
|
||
|
Evan JarrettandClaude Opus 5
|
ae1d7ba626 |
auth: test NarrowToPullOnly, the function that gates the anonymous path
NarrowToPullOnly had no test. IsPullOnlyScope has a thorough one, but it only answers yes/no — NarrowToPullOnly rewrites the access list, so what it emits is what gets signed, and anonymous tokens skip the authgate entirely. Nothing downstream re-authorizes what this function decides to hand out. The load-bearing property is the allowlist: "pull" is the only action that can survive. Beyond the per-case assertions, every case re-checks that no other action reached the output, so a new case cannot accidentally assert its way past the property the function exists to hold. The wildcard cases are the point. A wildcard action means "any action" to distribution's actionSet.contains, so expanding "*" into "pull" is the single rewrite that would turn a wildcard request into a grant. Mutation-verified: * treat "*" as pull -> the three wildcard cases fail * stop narrowing the actions -> the four narrowing cases fail * trim the action slice in place -> DoesNotMutateInput fails That last one initially did NOT fail, and the fixture is why. The input had "pull" first, so an in-place trim writing "pull" into index 0 changed nothing observable and the test passed against the exact defect it was written for. "pull" is now deliberately not first, with a comment saying so, because the ordering is the whole instrument here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh |
||
|
Evan JarrettandClaude Opus 5
|
915d9adcb2 |
test/e2e: cover the /auth/token surface, and correct 9d4ad84's provenance
batch09-token.sh drives the request shapes against a running stack, which is where the interesting part of |
||
|
Evan JarrettandClaude Opus 5
|
85a07d660a |
appview: give the O(n) bcrypt scan a guard that can actually fail
TestDeviceStore_ValidateDoesNotScanIndexedRows is documented as "the
regression guard for the O(n) bcrypt scan", but it does not observe whether a
scan happened. It asserts that every row is indexed and that an unknown secret
errors, and both hold with or without the fix. Deleting the
`WHERE secret_lookup IS NULL` filter from the fallback query — which is the
defect
|
||
|
Evan JarrettandClaude Opus 5
|
ce01e47ba6 |
auth: cover the two batch-09 commits that shipped without tests
|
||
|
Evan JarrettandClaude Opus 5
|
576a6b9e35 |
scanner: test grype DB freshness, backoff and reload fallback
scanner/internal/scan had no test file at all, which is why
|
||
|
Evan JarrettandClaude Opus 5
|
2984331f0c |
hold/gc: test the blob sweep, and give the S3 mock object ages
deleteOrphanedBlobs is the only part of GC that removes bytes and it had no test. It could not have had one: object age decides whether a blob is deletable, the in-process harness stamps every object with time.Now(), and MockS3Client's ListObjectsV2 set no LastModified at all. Neither could express "this blob is nine days old", so both halves of the grace rule went unexercised — including the half that protects a push still in flight. MockS3Client gains ObjectTimes, a per-key LastModified consulted by ListObjectsV2. Keys with no entry list without a timestamp exactly as before, so existing tests are unaffected. It also gains DeleteObjectError, matching the error injection the other operations already had. Four cases, three of which are reasons NOT to delete: an old unreferenced blob goes; a young unreferenced blob stays; a referenced old blob stays; a /link object is never treated as a blob. Plus a failure case pinning that one undeletable object does not abort the walk, and that a blob which never left storage is not counted as deleted or reported as reclaimed space. Verified by mutation. Disabling the grace check deletes the young blob; disabling the referenced check deletes the live one; both fail. Disabling the /data suffix check changes nothing, because extractDigestFromPath anchors on /data$ and rejects everything else — so that check is a redundant early-out rather than a guard, and the test says so rather than implying otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Evan JarrettandClaude Opus 5
|
5f74299bd7 |
test/e2e: exercise the stale-preview refusal and the real GC sweep
Three scripts, split by what they cost to run.
batch07-stale-preview.mjs stages a preview and waits out the 30-minute
maxPreviewAgeForDelete constant. batch07-stale-click.mjs is the resumable
half: it re-renders whatever preview the hold already holds and clicks delete
on it. GET /admin/api/gc/status re-renders lastPreview WITHOUT touching
lastPreviewAt, so showing an old preview does not reset its age — which is
what makes a failed run cost seconds instead of another 31 minutes.
Result against the dev hold: "preview is 34m0s old (limit 30m0s) — run Scan
again before deleting" rendered through the progress-to-error fragment chain,
with all 387 records still there afterwards. That chain is the point; the
refusal logic itself already has a Go test, but a refusal that renders as
nothing is indistinguishable from "there was nothing to delete".
batch07-sweep.mjs then runs the destructive path for real: 387 records
deleted of 387 staged, orphaned count to zero, referenced blobs unchanged at
15. Safe only against the dev hold on Storj; production is Bunny + UpCloud
and is not reachable from here.
page.on('dialog') did not reliably intercept hx-confirm on this page, and an
unaccepted native dialog blocks every later evaluate() and innerText(), so
the script hangs rather than fails — the worst failure mode for an unattended
check. Both scripts now strip the hx-confirm attribute before clicking. The
confirm is not what is under test.
Two findings worth carrying, neither introduced by this range:
* deleteOrphanedBlobs is still unexercised. The bucket holds 19 objects,
of which 8 are past the 7-day blob grace, and none are unreferenced — so
there is nothing for it to collect. More pushes cannot help: fresh blobs
are inside the grace window by definition.
* Storage accounting is derived from layer records, so this sweep moved the
dashboard from 1.3 GB to 1.1 KB while the bucket held 147 MB throughout.
It was overstating by ~9x before (records for blobs held by another hold)
and understates now (referenced blobs with no layer records). Quotas and
billing read the same number. Belongs to batch 12.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Evan JarrettandClaude Opus 5
|
0071528b8f |
test/e2e: check the GC preview panel and hand-check its orphan claim
Drives "Scan for Orphans" through the admin panel and asserts the wiring the
Go tests structurally cannot: that the progress fragment swaps into
#gc-results and hands off to the preview fragment, that every advertised stat
renders a value, and that each table's row count agrees with the stat above
it. A GC that decides correctly and renders a blank panel still gets someone
to click delete on the wrong thing.
Then it hand-checks the claim itself, which is the part that matters: for
each distinct manifest behind an orphaned record, resolve the owner's PDS and
ask whether that manifest is really gone.
Two instrument bugs were found writing this, both in the script rather than
the product, and both worth keeping as comments:
* The three tables overlap on a Digest column, so classifying by "has
Digest but no RKey" swallowed Missing Records as orphaned blobs and
reported 3 blobs against a stat of 0. Classification is now by exact
header set.
* Asserting the manifest is ABSENT from the PDS is too strong. A manifest
can be alive and name a different hold, which is exactly what happens
when defaultHold is repointed and the image re-pushed. Those records are
legitimately orphaned here. The only state that means GC is staged to
destroy live data is a manifest that exists AND still names this hold.
Against the dev hold: 387 orphaned records over 68 distinct manifests, 25
sampled — 17 gone, 8 alive but now pointing at the production hold, 0 still
naming this hold. Orphaned blobs 0, referenced 15, and the counts agree with
the tables.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Evan JarrettandClaude Opus 5
|
4542897f08 |
hold/gc: escape the DID in listRecords/getRecord repo parameters
fetchUserManifestsFromEndpoint, fetchUserTags and fetchUserProfile
interpolated the user DID straight into a query string. A did:web carrying a
port spells that port as a literal %3A, so the PDS received the parameter
decoded back to ":" — a different DID, matching no repo. listRecords then
answers 200 with an empty list and GC reads it as "this user has no
manifests": every blob they own drops out of the referenced set and is
deleted once past the seven-day blob grace.
There is no error and no status code to notice, which is the same soft
failure
|
||
|
Evan JarrettandClaude Opus 5
|
891ad01de3 |
hold/gc: require a predecessor's successor to name this hold
|
||
|
Evan JarrettandClaude Opus 5
|
48eee49ef9 |
hold/gc: cover checkPredecessorAt and the unresolved-holds reset
|
||
|
Evan JarrettandClaude Opus 5
|
bb45a80d51 |
hold/pds: cover the scanner-disconnect teardown 05b856b fixed
newTestScanBroadcaster builds the struct with only a database, so nothing in
the suite ever reached handleWriter, handleReader, or the Unsubscribe teardown
they share — which is precisely what
|
||
|
Evan JarrettandClaude Opus 5
|
8cd59a61f1 |
appview: stop a UI session outliving the OAuth session behind it
Only one oauth_sessions row is kept per account, so signing in again — on a
second device, or simply a second time — replaces it and leaves every earlier
ui_sessions row pointing at an oauth_session_id that no longer exists. Get
checked only expiry, so those still read back as usable.
Found on a live appview: four ui_sessions rows, three orphaned, and requesting
/settings/user with an orphaned cookie returned 200 with the account's handle
rendered throughout, where an anonymous request gets a 302. The browser looks
signed in while the credential behind it is gone, so every PDS-backed action
fails against a UI insisting the session is fine. It now fails closed and sends
the user back through login.
Get also never checked ownership. oauth_sessions is unique on
(account_did, session_id), so the existence check is scoped by both; matching
session_id alone would let one account's live OAuth session validate another
account's dangling reference. That has its own test.
An empty oauth_session_id stays valid, since Create makes sessions that never
had one, and a test pins that so the check cannot start rejecting them.
TestSessionStore_CreateWithOAuth referenced an OAuth session it never inserted,
which is an orphan by definition, so it now creates the row. Its intent was
that CreateWithOAuth persists the ID; it relied on the orphan behaviour only
incidentally. Its not-found branch used t.Error and then dereferenced the nil
session, so that is now t.Fatal.
Pre-existing at
|
||
|
Evan JarrettandClaude Opus 5
|
0a6f20fa74 |
test/e2e: prove a browser session survives an OAuth refresh
oauth-refresh-e2e.sh covers the refresh mechanics; this covers the symptom the
range exists to stop — a signed-in browser being thrown out when the access
token rotates underneath it.
The test is only meaningful because ui_sessions is a table carrying an
oauth_session_id rather than an in-memory map, so restarting the appview to
clear the refresher's cache does not by itself log the browser out. Verified
against the row the browser actually uses: one oauth_sessions row, and the
ui_sessions row created by the login points at it. rev advances 1 to 2 while
/settings/user keeps rendering.
Drives the login itself. Given a handle it fills the field and clicks through
consent, which is the whole flow whenever the PDS already has a session. Two
things it must not do, both learned by doing them:
* Never navigate while waiting for a human. The first version re-issued
goto() every two seconds and wiped the login form out from under whoever
was typing into it.
* Never bail permanently at a password field. Returning there left the flow
stranded on the Authorize screen once the password had been submitted,
because nothing was left to click it. It now pauses and resumes.
ATCR_E2E_FRESH clears only the appview's cookies. Clearing all of them takes
the PDS session with it, which turns a handle-and-consent flow into a password
prompt and makes the login impossible to drive unattended.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Evan JarrettandClaude Opus 5
|
30caf43146 |
test/e2e: drive the OAuth refresh path against a real PDS
client_test.go reproduces the cancellation precisely, but in-process against an httptest PDS. The failure this range fixes — mass sign-outs — happened against a real one, through the real refresher and the real oauth_sessions row, so the unit tests alone are a thinner sign-off than the batch deserves. Staling the access token in the live row and restarting the appview (the refresher caches sessions in memory, so editing the DB alone changes nothing) forces the real refresh path. Four concurrent pulls then advance rev 1 to 3 rather than 1 to 4: the compare-and-swap collapses four racing refreshes into two rotations, with the losers adopting the winner's token instead of each burning one. Killing a pull 150ms into that window and retrying still succeeds. Backs ui.db up first, since a burned refresh token would otherwise leave the account signed out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Evan JarrettandClaude Opus 5
|
c9f8b4178c |
credhelper: stop dev builds nagging about an update to themselves
isNewerVersion split versions on "." and ran each component through
strconv.Atoi, discarding the error and substituting 0. For a git-describe
build the last component is "4-18-g8f70cce", which does not parse, so it
became 0 and every published release compared as newer. Running
v0.1.4-18-g8f70cce printed "Update available: v0.1.4" on every single
invocation, naming a version the binary was already 18 commits past.
Versions are now parsed properly: the "-<commits>-g<sha>" tail is recognised
and kept as a count of commits past the tag, and a version that cannot be
read in full returns false rather than being silently treated as 0. That
second part is the actual root cause — the comparison could not distinguish
"this component is zero" from "I could not read this component".
Ordering for a git-describe build is deliberately not semver, where a
prerelease sorts below its release. Such a build is commits AHEAD of its tag,
so v0.1.4-18-g8f70cce is newer than v0.1.4 and older than v0.1.4-20-gabc1234.
The function had no tests. Both failing cases are pinned along with the
ordinary release comparisons, so the git-describe handling cannot regress the
normal upgrade path.
Pre-existing at
|
||
|
Evan JarrettandClaude Opus 5
|
724e22a978 |
test/e2e: only reset the DB when moving backward through the stack
Migrations are forward-only and the appview applies whatever is missing on boot, so moving to the next batch does not need a reset at all — Air rebuilds into the new code and the live DB migrates in place. Verified moving onto val/04-oauth: 0031 appeared in schema_migrations on its own, on top of a level-27 database, with the appview healthy afterwards. That matters more than it sounds. ui.db holds the OAuth sessions and the appview's signing keys, so the old wipe-on-every-switch cost an interactive `docker-credential-atcr login` per batch, which is most of what made the stack awkward to hand to an agent. Validating in stack order is all forward motion, so in the normal case there is now no login at all. A reset is still done when the live DB carries migrations the branch's code has never heard of, which is what going backward means, and the per-set snapshot is still banked so that case can restore rather than start empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Evan JarrettandClaude Opus 5
|
c1604b9a04 |
test/e2e: key DB snapshots on the migration set, not the highest version
Batching reorders migrations. val/04-oauth carries
|
||
|
Evan JarrettandClaude Opus 5
|
f8d9ad7fe9 |
appview: let a registry domain keep its port, and its /v2
DomainRoutingMiddleware normalized the request Host to a bare hostname but
matched server.registry_domains verbatim, so any configured domain carrying a
port could never match. config-appview.example.yaml ships
`registry_domains: [127.0.0.1:5000, atcr.io]`, which means that entry has been
inert since it was written.
It fails closed in the worst way. That same host is also the auto-detected UI
host, and `host == uiHost` was evaluated first, so /v2/* was answered with
"registry API is not available on this domain, use 127.0.0.1:5000" — naming the
exact host the client had just used. The registry API is unreachable on the dev
stack, and any single-host deployment hits the same wall: listing a host in
registry_domains does nothing if it is also the UI host.
Both sides are now normalized through hostWithoutPort, and a registry domain
takes /v2/* even when it doubles as the UI host, which is a legitimate
single-domain deployment. Everything else is unchanged: a UI-only host still
refuses /v2/, registry domains still redirect non-/v2 traffic to the UI, and
/auth/token and /auth/device/* are still served directly so a cross-host 307
cannot strip the Authorization header.
hostWithoutPort uses net.SplitHostPort instead of the previous LastIndex(":")
scan, which mangled bracketed IPv6 literals into "[::1" and could never match
the "::1" that url.URL.Hostname() yields for the UI host.
The middleware had no tests at all. The two failing cases are pinned first, and
the four pre-existing behaviours are pinned alongside them so the reorder
cannot quietly widen what /v2/ is served on.
Pre-existing at
|
||
|
Evan JarrettandClaude Opus 5
|
50e77ac7a4 |
test/e2e: snapshot the appview DB per migration level
Wiping ui.db on every switch also wipes the OAuth sessions and the appview's
oauth_p256/jwt_rsa keys, so each batch cost an interactive
`docker-credential-atcr login`. There are only five distinct migration levels
across the stack (27 for batches 00-08, 28 for 09-11, 29 for 12-13, 32 for 14,
34 for 15), so the DB is snapshotted per level and restored instead of
re-migrated. One login now serves every batch sharing a level.
Also skip the compose pin from val/01 onward:
|
||
|
Evan JarrettandClaude Opus 5
|
5aefa85048 |
test/e2e: cover the admin job wiring ab4a4eb changed
jobs_test.go covers the job framework thoroughly, but nothing covers the
wiring: whether the kickoff handler renders the progress fragment into the
right hx-target, and whether the loop actually outlives the request it was
started from. Both are what
|
||
|
Evan JarrettandClaude Opus 5
|
4c04983e23 |
appview: stop the backfill claiming every user was just active
last_seen means "this user did something recently". The backfill walks every historical record in the network, so stamping it there recorded when the backfill ran, not when the user was active — for every user at once, on every run. That destroys the only signal the column carries, and it is the one column in users that nothing upstream can rebuild. It is now written on the two paths that represent real activity: an interactive login, and a live commit event on the firehose, which does mean the user just wrote a record. The backfill still corrects handle, PDS endpoint and avatar, which is why it re-resolves rather than trusting a cache; it just no longer claims the user was present. UpsertUser grows an options form rather than a fourth named variant, since the avatar and last_seen decisions are independent and all four combinations occur. Anyone computing MAU from this column should know it was unreliable for every backfill run before this change. Also corrects docs/HORIZONTAL_SCALING.md, which claimed oci_client and registry_domain were local-only preferences. They are fields on io.atcr.sailor.profile: settings writes them to the user's PDS and ProcessSailorProfile refreshes the local cache. users is fully derived apart from last_seen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Evan JarrettandClaude Opus 5
|
13edb7184d |
appview: stop writing last_seen and last_used on every event
Neither is a correctness problem; both are round trips on hot paths for timestamps nothing reads at that resolution. UpdateUserLastSeen ran per Jetstream event for cached users, so once per indexed record. DeviceStore.UpdateLastUsed ran per /auth/token call, so once per docker push and pull including each layer's re-auth. Cheap against a local file, a network round trip each against a remote primary, and the second sat on the authentication path. Both are now throttled to once per five minutes per subject. The MAU queries and the admin views work in hours or days, so nothing loses meaning. The throttle state is per-process and lost on restart, costing at most one extra write per subject per boot; only the lease holder runs the consumer, so exactly one process is doing the first of these at a time. UpdateLastUsed stamps the throttle before writing rather than after, so a slow or failing write cannot let every concurrent layer upload through to pile on more of them. Verified by disabling the throttle: 50 back-to-back calls then rewrite the timestamp every time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Evan JarrettandClaude Opus 5
|
e9d43aa767 |
db: cover the recency change on all four surfaces, not one
The MAX(id) replacement touched four queries that each carry their own copy of the same CTE: SearchRepositories, GetRepoCards, GetUserRepoCards and GetStarredRepoCards. Only the third had a test. Fixing one and missing another would leave the UI disagreeing with itself about which manifest is current, depending on which page you were looking at. All four now assert that recency follows created_at rather than insert order, and that a tie between manifests pushed in the same second resolves the same way on every surface. Verified by regressing the CTEs back to rowid ordering: each of the four fails independently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |