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 b25aee3 lives: handler_test.go proves each shape
in-process, but it cannot show which form a real client picks, and that is the
whole reason the commit exists.

What driving real clients turned up, now encoded in the script's comments so a
re-run re-checks it:

  * Docker 29.7.2 and skopeo 1.22.2 use the GET form even holding a credential
    helper secret, and take two token requests for a pull with no 401 retry —
    so neither exercises the POST path at all.
  * containerd 2.3.3 does POST, and gets 200. That is the client b25aee3 was
    written for, and the only one here that would have eaten the old 405.
  * There is no anonymous branch in the handler at this branch; anonymous GET
    is a 401. The anonymous path arrives with val/10-anonpull, so the plan's
    "anonymous pull is GET-only" note describes a later batch.

seed-legacy-devices.go reproduces the day-one production devices table for the
08121f3 check: every row legacy, the real device inserted last so it sits at
the end of the rowid-order scan. Measured here at 200 rows: 7.83s first auth,
backfilled, 0.006s second. It is build-tagged `ignore` so it stays out of
go build ./... while remaining go run-able.

The 9d4ad84 comment claimed the reference PDS and tranquil both answer 403
InsufficientScope for a read-only app password. Only the tranquil half is
supported: the observation is issue #26 on pds.sqrl.systems, and the reference
PDS has no read-only app passwords at all, so the branch cannot be reproduced
against one. The comment now records that provenance and the reason a wrong
guess is harmless — the classification only adds a branch, and an unrecognised
error name falls through to the 503 that shipped before it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh

pkg/auth/servicetoken.go

@@ -58,8 +58,18 @@ var ErrAppPasswordInsufficientScope = errors.New("app-password lacks scope to mi
 
 // isInsufficientScopeError reports whether a PDS error body signals that the
 // presented app-password session is scoped too narrowly to call getServiceAuth.
-// The reference PDS and tranquil both return the atproto error name
-// "InsufficientScope" (paired with 403) for read-only app passwords.
+//
+// The name and status come from one observed case: issue #26, a read-only app
+// password on tranquil (pds.sqrl.systems). createSession succeeded, then
+// getServiceAuth answered 403 InsufficientScope. Read-only app passwords appear
+// to be tranquil-specific — its choices are Full Access, Read Only and Post
+// only — and the reference PDS has no such thing, so this branch cannot be
+// reproduced against one and is untested end to end.
+//
+// It is safe either way, because this only adds a branch. A PDS reporting the
+// same condition under a different name falls through to the generic non-200
+// path and the 503 that shipped before, so a wrong guess costs the better
+// message, not correctness.
 func isInsufficientScopeError(body []byte) bool {
 	return atprotoErrorName(body) == "InsufficientScope"
 }

test/e2e/batch09-token.sh

@@ -0,0 +1,111 @@
+#!/usr/bin/env bash
+# batch09-token.sh — the /auth/token surface that unit tests cannot reach.
+#
+# handler_test.go covers the request shapes in-process. What it cannot show is
+# which form a real client actually picks, and whether the deliberate
+# non-RFC 401 keeps those clients moving instead of looping.
+#
+# Covers:
+#   b25aee3 — serve the OAuth2 POST form; refresh_token refused 401, not 400
+#   08121f3 — indexed device auth is O(1) (the legacy scan is checked separately,
+#             see LEGACY BACKFILL below — it is destructive and opt-in)
+#
+# Findings this script encodes, so a re-run re-checks them:
+#   * Docker (29.7.2) and skopeo (1.22.2) use the GET form even when they hold a
+#     credential. containerd (2.3.3) is the only client here that POSTs, which
+#     is the client b25aee3 was written for — it ate a 405 before this commit.
+#   * There is no anonymous branch in the handler at batch 09. Anonymous GET is
+#     a 401 here; the anonymous path arrives with val/10-anonpull.
+#
+# Read-only app passwords (9d4ad84) are NOT covered: the reference PDS has no
+# such thing, so the 403 InsufficientScope path is unreproducible here. See
+# pkg/auth/servicetoken.go for what is and is not known about it.
+#
+# Usage:  ./test/e2e/batch09-token.sh
+#         ATCR_E2E_DESTRUCTIVE=1 ./test/e2e/batch09-token.sh   # + legacy scan
+set -uo pipefail
+
+APPVIEW=${ATCR_APPVIEW_URL:-http://127.0.0.1:5000}   # never localhost: that 307s
+A="$APPVIEW/auth/token"
+SERVICE=${ATCR_E2E_SERVICE:-127.0.0.1}
+HANDLE=${ATCR_E2E_HANDLE:-evan.jarrett.net}
+REPO=${ATCR_E2E_REPO:-$HANDLE/valtest}
+
+pass=0; fail=0
+check() { # check <label> <want> <got>
+  if [ "$2" = "$3" ]; then printf '  ok    %-52s %s\n' "$1" "$3"; pass=$((pass+1))
+  else printf '  FAIL  %-52s want %s, got %s\n' "$1" "$2" "$3"; fail=$((fail+1)); fi
+}
+
+code() { curl -s -o /dev/null -w '%{http_code}' "$@"; }
+
+secret() {
+  python3 -c "
+import json,os,sys
+p=os.path.expanduser('~/.atcr/device.json')
+d=json.load(open(p))
+r=d['registries'].get('$APPVIEW')
+if not r: sys.exit('no credential for $APPVIEW — run: docker-credential-atcr login ${APPVIEW#http://}')
+print(list(r['accounts'].values())[0]['device_secret'])"
+}
+
+echo "==> unauthenticated shapes"
+# containerd sends the refresh grant only when it has no username, which is the
+# same condition that disables its 405 fallback. A spec-correct 400 hard-fails
+# those clients; 401 is on its retry list and routes it to the GET form.
+check "POST grant_type=refresh_token -> 401 (NOT the RFC's 400)" 401 \
+  "$(code -X POST "$A" -d grant_type=refresh_token -d refresh_token=x)"
+check "POST with no body -> 401" 401 "$(code -X POST "$A")"
+# Empty username is rejected before any anonymous handling, so a credential-less
+# client that starts on POST gets a 401 and retries rather than being told 400.
+check "POST grant_type=password, empty username -> 401" 401 \
+  "$(code -X POST "$A" -d grant_type=password -d username= -d password=x -d service="$SERVICE")"
+check "PUT -> 405" 405 "$(code -X PUT "$A")"
+check "POST password grant, bogus secret -> 401" 401 \
+  "$(code -X POST "$A" -d grant_type=password -d username="$HANDLE" \
+       -d password=atcr_device_bogus -d service="$SERVICE")"
+
+SECRET=$(secret) || exit 1
+SCOPE="repository:$REPO:pull"
+ENC=$(python3 -c "import urllib.parse,sys;print(urllib.parse.quote(sys.argv[1]))" "$SCOPE")
+
+echo "==> authenticated, both forms"
+check "POST password grant -> 200" 200 \
+  "$(code -X POST "$A" -d grant_type=password -d username="$HANDLE" \
+       -d password="$SECRET" -d service="$SERVICE" --data-urlencode "scope=$SCOPE")"
+check "GET basic auth -> 200" 200 \
+  "$(code -u "$HANDLE:$SECRET" "$A?service=$SERVICE&scope=$ENC")"
+
+# The POST form is an OAuth2 endpoint, so it must mirror `token` into
+# `access_token`. No refresh_token is issued on purpose: the registry JWT's
+# lifetime is pinned to the AppView<->hold service-auth, and a refresh token
+# would be a fourth long-lived credential needing its own storage and revocation.
+echo "==> POST response body is OAuth2-shaped"
+curl -s -X POST "$A" -d grant_type=password -d username="$HANDLE" \
+  -d password="$SECRET" -d service="$SERVICE" --data-urlencode "scope=$SCOPE" > /tmp/b09.json
+check "access_token mirrors token" true \
+  "$(python3 -c "import json;d=json.load(open('/tmp/b09.json'));print(str(d.get('token')==d.get('access_token') and bool(d.get('token'))).lower())")"
+check "no refresh_token issued" false \
+  "$(python3 -c "import json;print(str('refresh_token' in json.load(open('/tmp/b09.json'))).lower())")"
+
+echo "==> indexed device auth is O(1)"
+# Sub-second only holds once the row is indexed. A legacy row still pays the
+# scan on its first auth by design; that is the LEGACY BACKFILL check below.
+t0=$(date +%s.%N); code -u "$HANDLE:$SECRET" "$A?service=$SERVICE&scope=$ENC" >/dev/null; t1=$(date +%s.%N)
+el=$(python3 -c "print(f'{$t1-$t0:.3f}')")
+check "indexed auth under 1s (was 15.7-16.0s in production)" true \
+  "$(python3 -c "print(str($el < 1.0).lower())")"
+echo "        measured ${el}s"
+
+if [ "${ATCR_E2E_DESTRUCTIVE:-}" = "1" ]; then
+  echo "==> LEGACY BACKFILL (destructive: rewrites the devices table)"
+  echo "    Reproduce day-one production with test/e2e/seed-legacy-devices.go:"
+  echo "      every row legacy, the real device inserted LAST so it sits at the"
+  echo "      end of the rowid-order scan. Measured here: 7.83s first auth,"
+  echo "      backfilled, then 0.006s. See docs in that file."
+  echo "    Not automated: it needs a bcrypt hash of the live device secret."
+fi
+
+echo
+echo "passed $pass, failed $fail"
+[ "$fail" -eq 0 ]

test/e2e/seed-legacy-devices.go

@@ -0,0 +1,82 @@
+//go:build ignore
+
+// seed-legacy-devices.go — reproduce the day-one production devices table for
+// the 08121f3 (O(n) bcrypt scan) check in batch09-token.sh.
+//
+// Why this has to exist: no backfill of the real table is possible, because the
+// plaintext is not recoverable from a bcrypt hash. So on the day migration 0028
+// ships, all 244 production devices are legacy (secret_lookup NULL) and migrate
+// lazily on their next successful auth. A regression on that path locks out
+// every existing user while newly registered devices keep working, which is the
+// failure mode least likely to surface in a smoke test.
+//
+// The real device is inserted LAST so it sits at the end of the rowid-order
+// scan — the position the commit message calls out as costing the most ("the
+// scan ran in rowid order, so the newest devices paid the most").
+//
+// Only two hashes are generated. The fillers are copies of one hash with a
+// mutated tail: bcrypt reads its cost from the prefix and runs the full key
+// derivation before comparing, so a mutated tail costs a real comparison and
+// then misses — exactly what scanning a non-matching row does.
+//
+// Usage:
+//
+//	go run test/e2e/seed-legacy-devices.go <device-secret> <did> <handle> [n] \
+//	  | docker exec -i atcr-appview sqlite3 /var/lib/atcr/ui.db
+//
+// Then time two authentications. Measured on this dev stack with n=200:
+// first 7.83s (the scan), secret_lookup backfilled, second 0.006s.
+//
+// Clean up afterwards, or later tests inherit 200 fake devices:
+//
+//	docker exec atcr-appview sqlite3 /var/lib/atcr/ui.db \
+//	  "DELETE FROM devices WHERE id LIKE 'legacy-%'"
+package main
+
+import (
+	"fmt"
+	"os"
+	"strconv"
+
+	"golang.org/x/crypto/bcrypt"
+)
+
+func main() {
+	if len(os.Args) < 4 {
+		fmt.Fprintln(os.Stderr, "usage: seed-legacy-devices.go <device-secret> <did> <handle> [n]")
+		os.Exit(2)
+	}
+	secret, did, handle := os.Args[1], os.Args[2], os.Args[3]
+	n := 200
+	if len(os.Args) > 4 {
+		var err error
+		if n, err = strconv.Atoi(os.Args[4]); err != nil {
+			fmt.Fprintf(os.Stderr, "bad count %q: %v\n", os.Args[4], err)
+			os.Exit(2)
+		}
+	}
+
+	filler, err := bcrypt.GenerateFromPassword([]byte("filler"), bcrypt.DefaultCost)
+	if err != nil {
+		panic(err)
+	}
+	real_, err := bcrypt.GenerateFromPassword([]byte(secret), bcrypt.DefaultCost)
+	if err != nil {
+		panic(err)
+	}
+
+	const ins = "INSERT INTO devices (id,did,handle,name,secret_hash,secret_lookup," +
+		"ip_address,user_agent,created_at) VALUES ('%s','%s','%s','%s','%s',NULL," +
+		"'10.0.0.1','seed-legacy',datetime('now'));\n"
+
+	fmt.Println("BEGIN;")
+	fmt.Println("DELETE FROM devices;")
+	for i := 0; i < n; i++ {
+		h := append([]byte(nil), filler...)
+		h[len(h)-1] = byte('a' + (i % 26))
+		h[len(h)-2] = byte('a' + ((i / 26) % 26))
+		fmt.Printf(ins, fmt.Sprintf("legacy-%d", i), did, handle, fmt.Sprintf("Legacy %d", i), string(h))
+	}
+	fmt.Printf(ins, "real-device", did, handle, "Real Device", string(real_))
+	fmt.Println("COMMIT;")
+}