From 915d9adcb20940cc6952179860989b6fb80959e5 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Mon, 24 Aug 2026 12:49:03 -0500 Subject: [PATCH] test/e2e: cover the /auth/token surface, and correct 9d4ad84's provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01SeaUS5AFPX9gqCahoLRMRh --- pkg/auth/servicetoken.go | 14 +++- test/e2e/batch09-token.sh | 111 ++++++++++++++++++++++++++++++++ test/e2e/seed-legacy-devices.go | 82 +++++++++++++++++++++++ 3 files changed, 205 insertions(+), 2 deletions(-) create mode 100755 test/e2e/batch09-token.sh create mode 100644 test/e2e/seed-legacy-devices.go diff --git a/pkg/auth/servicetoken.go b/pkg/auth/servicetoken.go index dc2a8e2..ecce7d3 100644 --- a/pkg/auth/servicetoken.go +++ b/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" } diff --git a/test/e2e/batch09-token.sh b/test/e2e/batch09-token.sh new file mode 100755 index 0000000..3fc5344 --- /dev/null +++ b/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