diff --git a/.air.hold.toml b/.air.hold.toml index 0b8a723..8450205 100644 --- a/.air.hold.toml +++ b/.air.hold.toml @@ -3,7 +3,8 @@ tmp_dir = "tmp" [build] pre_cmd = ["go generate ./pkg/hold/..."] -cmd = "go build -buildvcs=false -o ./tmp/atcr-hold ./cmd/hold" +# GO_TAGS (set by Dockerfile.dev / `make dev`) supplies build tags, e.g. testmode. +cmd = "go build ${GO_TAGS:+-tags $GO_TAGS} -buildvcs=false -o ./tmp/atcr-hold ./cmd/hold" entrypoint = ["./tmp/atcr-hold", "serve", "--config", "config-hold.example.yaml"] include_ext = ["go", "html", "css", "js"] exclude_dir = ["bin", "tmp", "vendor", "deploy", "docs", ".git", "dist", "node_modules", "scanner", "pkg/appview", "pkg/labeler"] diff --git a/.air.labeler.toml b/.air.labeler.toml index ed460a7..b0646df 100644 --- a/.air.labeler.toml +++ b/.air.labeler.toml @@ -2,7 +2,8 @@ root = "." tmp_dir = "tmp" [build] -cmd = "go build -buildvcs=false -o ./tmp/atcr-labeler ./cmd/labeler" +# GO_TAGS (set by Dockerfile.dev / `make dev`) supplies build tags, e.g. testmode. +cmd = "go build ${GO_TAGS:+-tags $GO_TAGS} -buildvcs=false -o ./tmp/atcr-labeler ./cmd/labeler" entrypoint = ["./tmp/atcr-labeler", "serve", "--config", "config-labeler.example.yaml"] include_ext = ["go", "html", "css", "js"] exclude_dir = ["bin", "tmp", "vendor", "deploy", "docs", ".git", "dist", "node_modules", "scanner", "pkg/appview", "pkg/hold"] diff --git a/.air.toml b/.air.toml index af8f69c..fba3ff4 100644 --- a/.air.toml +++ b/.air.toml @@ -7,7 +7,8 @@ poll = true poll_interval = 500 # Pre-build: generate assets if missing (each string is a shell command) pre_cmd = ["go generate ./pkg/appview/..."] -cmd = "go build -tags billing -buildvcs=false -o ./tmp/atcr-appview ./cmd/appview" +# GO_TAGS (set by Dockerfile.dev / `make dev`) appends build tags, e.g. testmode. +cmd = "go build -tags billing${GO_TAGS:+,$GO_TAGS} -buildvcs=false -o ./tmp/atcr-appview ./cmd/appview" entrypoint = ["./tmp/atcr-appview", "serve", "--config", "config-appview.example.yaml"] include_ext = ["go", "html", "css", "js"] exclude_dir = ["bin", "tmp", "vendor", "deploy", "docs", ".git", "dist", "node_modules", "scanner", "pkg/hold", "pkg/labeler"] diff --git a/CLAUDE.md b/CLAUDE.md index 03a49c8..8a015bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,11 +39,16 @@ cd cmd/credential-helper/atcr && go build -o ../../../bin/docker-credential-atcr # Build appview with billing support (optional build tag; billing lives in pkg/billing/, appview-side only) go build -tags billing -o bin/atcr-appview ./cmd/appview -# Tests -go test ./... # all tests -go test ./pkg/atproto/... # specific package -go test -run TestManifestStore ./pkg/atproto/... # specific test -go test -race ./... # race detector +# Tests (run as a `testmode` build, see below) +go test -tags testmode ./... # all tests +go test -tags testmode ./pkg/atproto/... # specific package +go test -tags testmode -run TestManifestStore ./pkg/atproto/... # specific test +go test -tags testmode -race ./... # race detector +make test # same, every workspace module + +# Local run against a loopback hold / PDS: build with the testmode tag +go build -tags testmode -o bin/atcr-appview ./cmd/appview +go build -tags testmode -o bin/atcr-hold ./cmd/hold # Docker docker build -f Dockerfile.appview -t atcr.io/appview:latest . @@ -71,6 +76,27 @@ go run ./cmd/s3-test # S3 connectivity test go run ./cmd/healthcheck # HTTP health check (for Docker) ``` +### The `testmode` build tag + +`pkg/atproto/indigo_prod.go` (`!testmode`) and `pkg/atproto/indigo_local.go` +(`testmode`) are the only place ATCR departs from indigo's defaults. Both define +`NewDirectory()`, `NewOAuthClientApp()`, and the constant `TestModeBuild`; all +identity and OAuth construction in the repo goes through them. Production builds +get indigo's hardened directory and HTTP clients (SSRF-guarded, HTTPS-only +did:web). A `-tags testmode` build wraps the directory so a did:web naming an IP, +localhost, or any host with a port resolves over plain HTTP, and gives the OAuth +client plain HTTP clients so it can reach a PDS on loopback. + +- Tests, the integration harness, `make dev`, and docker-compose all build with + the tag (`make test`, `TEST_TAGS` in the Makefile, `GO_TAGS` in Air/Dockerfile.dev). +- Production Dockerfiles and `make build-trixie` never set it. There is no + runtime switch: a production binary cannot be configured to resolve local DIDs. +- Tests that need loopback did:web resolution call `t.Fatal` naming the tag + when `atproto.TestModeBuild` is false, so an untagged `go test` says why. +- `server.test_mode` (runtime) still gates the remaining behavioral branches + (hold issuer-mismatch tolerance, backfill warning suppression, registry + fall-back-to-default-hold); it no longer affects DID resolution. + ## Architecture Overview ATCR uses **distribution/distribution** as a library, extending it via middleware to route content to different backends: diff --git a/Dockerfile.dev b/Dockerfile.dev index 48d25cb..e9cf7aa 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -4,9 +4,15 @@ FROM mirror.gcr.io/library/golang:1.26.7-trixie ARG AIR_CONFIG=.air.toml +# Extra Go build tags for Air's build command (comma-separated). docker-compose +# passes `testmode` so the dev stack can resolve its loopback did:web +# identities. Production images (Dockerfile.appview, Dockerfile.hold) never +# set this and never carry the tag. +ARG GO_TAGS="" ENV DEBIAN_FRONTEND=noninteractive ENV AIR_CONFIG=${AIR_CONFIG} +ENV GO_TAGS=${GO_TAGS} RUN apt-get update && \ apt-get install -y --no-install-recommends sqlite3 libsqlite3-dev curl nodejs npm && \ diff --git a/Makefile b/Makefile index ed1209f..91487fe 100644 --- a/Makefile +++ b/Makefile @@ -143,33 +143,40 @@ define test-submodules done endef +# Tests run as a `testmode` build. The tag compiles pkg/atproto/indigo_local.go +# in place of indigo_prod.go, which is what lets a did:web on a loopback port +# (every test server, the integration harness, docker-compose) resolve over +# plain HTTP. Production binaries never carry the tag; there is no runtime +# switch. Tests that need it fail fast with a message naming the tag. +TEST_TAGS := testmode + test: test-billing ## Run all tests (every workspace module) @echo "→ Running tests..." - go test -cover ./... - $(call test-submodules,-cover) + go test -tags $(TEST_TAGS) -cover ./... + $(call test-submodules,-tags $(TEST_TAGS) -cover) # pkg/billing is behind the `billing` build tag, so `go test ./...` never # compiles it, let alone runs it. Its tests covered the money path and had # never executed in this target or in CI. test-billing: ## Run the billing-tagged tests (skipped by plain `go test ./...`) @echo "→ Running billing-tagged tests..." - go test -tags billing -cover ./pkg/billing/... + go test -tags billing,$(TEST_TAGS) -cover ./pkg/billing/... test-race: ## Run tests with race detector (every workspace module) @echo "→ Running tests with race detector..." - go test -race ./... + go test -race -tags $(TEST_TAGS) ./... @echo "→ Running billing-tagged tests with race detector..." - go test -race -tags billing ./pkg/billing/... - $(call test-submodules,-race) + go test -race -tags billing,$(TEST_TAGS) ./pkg/billing/... + $(call test-submodules,-race -tags $(TEST_TAGS)) test-verbose: ## Run tests with verbose output (every workspace module) @echo "→ Running tests with verbose output..." - go test -v ./... - $(call test-submodules,-v) + go test -v -tags $(TEST_TAGS) ./... + $(call test-submodules,-v -tags $(TEST_TAGS)) integration-test: ## Run in-process smoke test (no docker, fake PDS + gofakes3 + hold + appview) @echo "→ Running integration smoke test..." - go test -tags=integration -count=1 -race -timeout=120s ./test/integration/... + go test -tags=integration,$(TEST_TAGS) -count=1 -race -timeout=120s ./test/integration/... stripe-integration-test: ## Run Stripe sandbox-backed billing tests (needs STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, STRIPE_TEST_PRICE_MONTHLY, STRIPE_TEST_PRICE_YEARLY) @echo "→ Running Stripe sandbox integration tests..." @@ -202,6 +209,8 @@ lint: check-golangci-lint ## Run golangci-lint golangci-lint run ./... @echo "→ Running golangci-lint (billing tag)..." golangci-lint run --build-tags=billing ./pkg/billing/... + @echo "→ Running golangci-lint (testmode tag)..." + golangci-lint run --build-tags=testmode ./pkg/atproto/... lex-lint: ## Lint ATProto lexicon schemas goat lex lint ./lexicons/ @@ -215,7 +224,10 @@ install-credential-helper: build-credential-helper ## Install credential helper ##@ Development Targets -dev: $(GENERATED_ASSETS) ## Run AppView locally with Air hot reload +# Air's build command appends $GO_TAGS to its -tags list; a local run needs +# `testmode` for the same reason the tests do (see TEST_TAGS above). +dev: export GO_TAGS ?= testmode +dev: $(GENERATED_ASSETS) ## Run AppView locally with Air hot reload (testmode build) @which air > /dev/null || (echo "→ Installing Air..." && go install github.com/air-verse/air@latest) air -c .air.toml diff --git a/cmd/relay-compare/main.go b/cmd/relay-compare/main.go index f1b5c38..f0d2e7d 100644 --- a/cmd/relay-compare/main.go +++ b/cmd/relay-compare/main.go @@ -19,6 +19,7 @@ import ( "sync" "time" + "atcr.io/pkg/atproto" "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/syntax" "github.com/bluesky-social/indigo/xrpc" @@ -137,7 +138,7 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), *timeout) defer cancel() - dir = identity.DefaultDirectory() + dir = atproto.NewDirectory() // Short display names for each relay names := make([]string, len(relays)) diff --git a/docker-compose.yml b/docker-compose.yml index 6665b7c..4f34c34 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,6 +3,8 @@ services: build: context: . dockerfile: Dockerfile.dev + args: + GO_TAGS: testmode image: atcr-appview-dev:latest container_name: atcr-appview # Option B: share the hold's network namespace so "localhost:8080" reaches the @@ -98,6 +100,7 @@ services: dockerfile: Dockerfile.dev args: AIR_CONFIG: .air.hold.toml + GO_TAGS: testmode image: atcr-hold-dev:latest container_name: atcr-hold ports: @@ -146,6 +149,7 @@ services: dockerfile: Dockerfile.dev args: AIR_CONFIG: .air.labeler.toml + GO_TAGS: testmode image: atcr-labeler-dev:latest container_name: atcr-labeler ports: diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 85bf320..0ddd948 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -128,7 +128,8 @@ poll = true poll_interval = 500 # Pre-build: generate assets if missing (each string is a shell command) pre_cmd = ["go generate ./pkg/appview/..."] -cmd = "go build -tags billing -buildvcs=false -o ./tmp/atcr-appview ./cmd/appview" +# GO_TAGS (set by Dockerfile.dev / `make dev`) appends build tags, e.g. testmode. +cmd = "go build -tags billing${GO_TAGS:+,$GO_TAGS} -buildvcs=false -o ./tmp/atcr-appview ./cmd/appview" entrypoint = ["./tmp/atcr-appview", "serve", "--config", "config-appview.example.yaml"] include_ext = ["go", "html", "css", "js"] exclude_dir = ["bin", "tmp", "vendor", "deploy", "docs", ".git", "dist", "node_modules", "scanner", "pkg/hold", "pkg/labeler"] @@ -145,7 +146,15 @@ Key points that differ from a naive config: - `pre_cmd` runs `go generate ./pkg/appview/...`, which regenerates and re-embeds CSS/JS/icons before the build. - `cmd` builds with `-tags billing` (AppView dev runs with billing support) and - `-buildvcs=false`. + `-buildvcs=false`. Air runs the command through `sh -c`, so `${GO_TAGS:+,$GO_TAGS}` + appends whatever `GO_TAGS` holds. docker-compose passes `GO_TAGS: testmode` as a + build arg to `Dockerfile.dev`, and `make dev` exports the same, so every dev + build is a **testmode build**: `pkg/atproto/indigo_local.go` replaces + `indigo_prod.go`, letting a did:web on an IP, `localhost`, or any port (the + hold's `did:web:localhost%3A8080`, the appview's `did:web:127.0.0.1%3A5000`, + the labeler's `did:web:172.28.0.4%3A5002`) resolve over plain HTTP, and letting + the OAuth client reach a PDS on loopback. Production images never set the tag + and cannot be configured to resolve local DIDs at runtime. - `entrypoint` is the full argv for the built binary: it runs `serve --config config-appview.example.yaml`. (The example config is the dev base config; env vars in `docker-compose.yml` override it.) @@ -273,14 +282,21 @@ builds the generated assets, and runs `air -c .air.toml`. You can also run Air directly, or skip hot reload entirely: ```bash -# Air, AppView config -air -c .air.toml +# Air, AppView config (GO_TAGS makes it a testmode build, as `make dev` does) +GO_TAGS=testmode air -c .air.toml # No hot reload — build and run once -go build -tags billing -o bin/atcr-appview ./cmd/appview +go build -tags billing,testmode -o bin/atcr-appview ./cmd/appview ./bin/atcr-appview serve --config config-appview.example.yaml ``` +Leave `testmode` off only when the appview talks exclusively to public +identities (a real PDS, a hold on a public HTTPS hostname); with it off, any +did:web naming an IP, `localhost`, or a port fails to resolve, exactly as in +production. Tests need the tag too: `make test` sets it, and a bare +`go test ./...` fails fast in the tests that depend on it with a message naming +the tag. + Running on the host requires a working toolchain for the build: Go 1.26.7 (see `go.work`), Node/npm (for the `go generate` asset step), and SQLite headers. Override config values with the `ATCR_*` env vars listed above, diff --git a/internal/testharness/harness.go b/internal/testharness/harness.go index 2c5f734..0b77724 100644 --- a/internal/testharness/harness.go +++ b/internal/testharness/harness.go @@ -117,6 +117,14 @@ func New(t *testing.T, opts ...Option) *Harness { } h := &Harness{t: t} + // The stack's holds and appview identify themselves by did:web on loopback + // ports. Only a `-tags testmode` build resolves those (pkg/atproto's + // indigo_local.go); a production build would fail every lookup with an + // opaque dial error, so say why up front. + if !atproto.TestModeBuild { + t.Fatal("the integration harness needs a `-tags testmode` build: go test -tags integration,testmode ./test/integration/...") + } + // 1. Fake PDS. h.PDS = testpds.New(t) atproto.SetDirectory(h.PDS.Directory()) diff --git a/pkg/appview/authgate/testhelpers_test.go b/pkg/appview/authgate/testhelpers_test.go index 5f2e7a9..7e85134 100644 --- a/pkg/appview/authgate/testhelpers_test.go +++ b/pkg/appview/authgate/testhelpers_test.go @@ -5,12 +5,12 @@ import ( "database/sql" "net/http" "net/http/httptest" - "strings" "testing" "atcr.io/pkg/appview/db" "atcr.io/pkg/atproto" "atcr.io/pkg/auth" + "atcr.io/pkg/testpds" ) // newTestDB returns an in-memory libsql DB with the full appview schema @@ -72,26 +72,33 @@ type quotaServerResult struct { // quotaServer spins up an httptest.Server that responds to every request // with the given status + body, records hit count + last URL, and returns -// both the server URL and the did:web:HOST form that resolves to it under -// atproto.SetTestMode(true). +// both the server URL and the did:web:HOST form that resolves to it. The +// server also serves its own did:web document (not counted in hits) so the +// test-mode identity directory can resolve that DID back to the server. func quotaServer(t *testing.T, status int, body string) *quotaServerResult { t.Helper() + if !atproto.TestModeBuild { + t.Fatal("this test resolves a did:web on 127.0.0.1 and needs a `-tags testmode` build") + } res := "aServerResult{} - res.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + base := "http://" + r.Host + testpds.HoldDIDDocumentHandler(testpds.DIDWebForURL(base), base)(w, r) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { res.hits++ res.lastURL = r.URL.String() w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) _, _ = w.Write([]byte(body)) - })) + }) + res.server = httptest.NewServer(mux) t.Cleanup(res.server.Close) - // httptest.Server.URL has the form "http://127.0.0.1:PORT". The - // did:web equivalent percent-encodes the colon; didWebToURL reverses - // the encoding and re-derives the http://host:port form, which is - // what we need for atproto.SetTestMode(true) to route requests. - host := strings.TrimPrefix(res.server.URL, "http://") - res.holdDID = "did:web:" + strings.Replace(host, ":", "%3A", 1) + // httptest.Server.URL has the form "http://127.0.0.1:PORT"; did:web + // percent-encodes the port colon. + res.holdDID = testpds.DIDWebForURL(res.server.URL) return res } diff --git a/pkg/appview/holdclient/tier_update_test.go b/pkg/appview/holdclient/tier_update_test.go index 134a5fa..184e206 100644 --- a/pkg/appview/holdclient/tier_update_test.go +++ b/pkg/appview/holdclient/tier_update_test.go @@ -10,6 +10,7 @@ import ( "time" "atcr.io/pkg/atproto" + "atcr.io/pkg/testpds" "github.com/bluesky-social/indigo/atproto/atcrypto" ) @@ -89,14 +90,25 @@ func TestUpdateCrewTierOnHold_PostsToEndpoint(t *testing.T) { // function the Stripe webhook actually calls, and the one whose error decides // whether a paid upgrade is retried or dropped — had no test at all. -// didFor turns an httptest server URL into a did:web that ResolveHoldDIDToURL -// maps straight back to it. The port makes DIDWebToURL choose http, and test -// mode is what lets a did:web that no directory can resolve fall back to being -// decoded from the DID itself. -func didFor(t *testing.T, serverURL string) string { +// holdServer starts an httptest server standing in for a hold and returns it +// with the did:web that ResolveHoldDIDToURL maps back to it. The server serves +// its own DID document at /.well-known/did.json (always promptly, whatever the +// hold handler does), and handler gets every other request. Resolving a +// loopback did:web is what a `-tags testmode` build provides. +func holdServer(t *testing.T, handler http.HandlerFunc) (*httptest.Server, string) { t.Helper() - host := strings.TrimPrefix(serverURL, "http://") - return "did:web:" + strings.ReplaceAll(host, ":", "%3A") + if !atproto.TestModeBuild { + t.Fatal("this test resolves a did:web on 127.0.0.1 and needs a `-tags testmode` build") + } + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + base := "http://" + r.Host + testpds.HoldDIDDocumentHandler(testpds.DIDWebForURL(base), base)(w, r) + }) + mux.Handle("/", handler) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv, testpds.DIDWebForURL(srv.URL) } func testKey(t *testing.T) *atcrypto.PrivateKeyP256 { @@ -116,20 +128,15 @@ func TestUpdateCrewTierOnAllHolds_JoinedErrorNamesEveryFailingHold(t *testing.T) atproto.SetTestMode(true) t.Cleanup(func() { atproto.SetTestMode(false) }) - ok := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, okDID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - })) - defer ok.Close() - bad1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }) + _, bad1DID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { http.Error(w, "down", http.StatusServiceUnavailable) - })) - defer bad1.Close() - bad2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }) + _, bad2DID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { http.Error(w, "broken", http.StatusInternalServerError) - })) - defer bad2.Close() - - okDID, bad1DID, bad2DID := didFor(t, ok.URL), didFor(t, bad1.URL), didFor(t, bad2.URL) + }) err := UpdateCrewTierOnAllHolds(context.Background(), []string{okDID, bad1DID, bad2DID}, "did:plc:user", 1, testKey(t), "did:web:appview") @@ -159,25 +166,23 @@ func TestUpdateCrewTierOnAllHolds_SlowHoldDoesNotStarveOthers(t *testing.T) { t.Cleanup(func() { atproto.SetTestMode(false) }) release := make(chan struct{}) - slow := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, slowDID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { <-release - })) - defer slow.Close() + }) defer close(release) var healthyHits atomic.Int32 - healthy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, healthyDID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { healthyHits.Add(1) w.WriteHeader(http.StatusOK) - })) - defer healthy.Close() + }) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() // Slow hold first: serial contact would spend the entire budget on it. err := UpdateCrewTierOnAllHolds(ctx, - []string{didFor(t, slow.URL), didFor(t, healthy.URL)}, + []string{slowDID, healthyDID}, "did:plc:user", 1, testKey(t), "did:web:appview") if err == nil { @@ -203,11 +208,10 @@ func TestUpdateCrewTierOnAllHolds_DeadlineCutsRetriesShort(t *testing.T) { release := make(chan struct{}) var attempts atomic.Int32 - hung := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, hungDID := holdServer(t, func(w http.ResponseWriter, r *http.Request) { attempts.Add(1) <-release - })) - defer hung.Close() + }) defer close(release) // Deadline deliberately shorter than tierUpdateMaxAttempts would need. @@ -215,7 +219,7 @@ func TestUpdateCrewTierOnAllHolds_DeadlineCutsRetriesShort(t *testing.T) { defer cancel() start := time.Now() - err := UpdateCrewTierOnAllHolds(ctx, []string{didFor(t, hung.URL)}, + err := UpdateCrewTierOnAllHolds(ctx, []string{hungDID}, "did:plc:user", 1, testKey(t), "did:web:appview") elapsed := time.Since(start) diff --git a/pkg/appview/jetstream/processor.go b/pkg/appview/jetstream/processor.go index 93ffcec..f129bb7 100644 --- a/pkg/appview/jetstream/processor.go +++ b/pkg/appview/jetstream/processor.go @@ -11,7 +11,6 @@ import ( "atcr.io/pkg/appview/db" "atcr.io/pkg/atproto" atpdata "github.com/bluesky-social/indigo/atproto/atdata" - "github.com/bluesky-social/indigo/atproto/identity" "github.com/bluesky-social/indigo/atproto/lexicon" ) @@ -43,7 +42,7 @@ func (p *Processor) SetWebhookDispatcher(d WebhookDispatcher) { // statsCache: shared stats cache for aggregating across holds (nil to skip stats processing) func NewProcessor(database db.DBTX, useCache bool, statsCache *StatsCache) *Processor { // Create lexicon catalog for debug validation logging - dir := identity.DefaultDirectory() + dir := atproto.NewDirectory() catalog := lexicon.NewResolvingCatalog() catalog.Directory = dir diff --git a/pkg/appview/middleware/registry_test.go b/pkg/appview/middleware/registry_test.go index d19afe1..4df30e9 100644 --- a/pkg/appview/middleware/registry_test.go +++ b/pkg/appview/middleware/registry_test.go @@ -405,14 +405,11 @@ func TestHoldResolutionError_PermanentVsTransient(t *testing.T) { // errors that atproto.ResolveHoldURL actually produces, not hand-built ones, // so the permanent/transient split can't silently drift from the resolver. func TestHoldResolutionError_FromRealResolver(t *testing.T) { - // testMode short-circuits did:web resolution to a derived URL, which would - // hide the permanent case entirely. - prevTestMode := atproto.IsTestMode() - atproto.SetTestMode(false) - t.Cleanup(func() { atproto.SetTestMode(prevTestMode) }) - t.Run("stale defaultHold is permanent", func(t *testing.T) { - const holdDID = "did:web:localhost%3A8080" + // Port 1 (tcpmux) rather than 8080: a `-tags testmode` build really + // does try http://localhost:PORT/.well-known/did.json, and a dev hold + // listening on 8080 would turn this into a successful resolution. + const holdDID = "did:web:localhost%3A1" _, err := atproto.ResolveHoldURL(context.Background(), holdDID) require.Error(t, err) diff --git a/pkg/atproto/directory.go b/pkg/atproto/directory.go index 9ad06e3..d2c9660 100644 --- a/pkg/atproto/directory.go +++ b/pkg/atproto/directory.go @@ -39,8 +39,11 @@ func SetDirectory(d identity.Directory) { } // GetDirectory returns the shared identity.Directory. On first call (and if -// SetDirectory has not been used), it constructs an indigo cached directory -// with a 24h TTL backed by Jetstream event-driven invalidation. +// SetDirectory has not been used), it constructs the directory via +// NewDirectory: indigo's cached directory (24h TTL, Jetstream-driven +// invalidation) in production builds, and the same wrapped with plain-HTTP +// resolution of local did:web identifiers in `-tags testmode` builds. See +// indigo_prod.go and indigo_local.go. // // Using a shared instance ensures all identity lookups across the application // use the same cache, which is more memory-efficient and provides better cache @@ -49,7 +52,7 @@ func GetDirectory() identity.Directory { directoryMu.Lock() defer directoryMu.Unlock() if sharedDirectory == nil { - sharedDirectory = identity.DefaultDirectory() + sharedDirectory = NewDirectory() } return sharedDirectory } diff --git a/pkg/atproto/indigo_local.go b/pkg/atproto/indigo_local.go new file mode 100644 index 0000000..888a09c --- /dev/null +++ b/pkg/atproto/indigo_local.go @@ -0,0 +1,181 @@ +//go:build testmode + +package atproto + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "strings" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// TestModeBuild reports whether this binary was compiled with `-tags testmode`. +// +// This file is the only place ATCR departs from indigo's defaults, and it is +// compiled out of production binaries entirely. A test-mode build resolves +// did:web identifiers that name loopback, private, or port-qualified hosts over +// plain HTTP, and lets the OAuth client talk to a PDS on such an address. Every +// other lookup goes through indigo exactly as it would in production. +const TestModeBuild = true + +// maxLocalDIDDocSize bounds a local did:web document read. Real documents are +// a few hundred bytes. +const maxLocalDIDDocSize = 1 << 20 + +// NewDirectory returns the identity directory used for every DID and handle +// lookup in this process. Test-mode builds wrap indigo's directory so that +// local did:web identifiers (an IP literal, localhost, or any host with a port) +// resolve by fetching http://host[:port]/.well-known/did.json directly. Those +// lookups are not cached: a dev hold or appview restarts often and its +// document is one loopback GET away. +func NewDirectory() identity.Directory { + base := identity.BaseDirectory{ + PLCURL: identity.DefaultPLCURL, + // No Transport means http.DefaultTransport: no SSRF dialer, so a + // test-mode build can also resolve well-known handles served locally. + HTTPClient: http.Client{Timeout: 10 * time.Second}, + PLCClient: &http.Client{Timeout: 10 * time.Second}, + Resolver: net.Resolver{ + Dial: func(ctx context.Context, network, address string) (net.Conn, error) { + d := net.Dialer{Timeout: 3 * time.Second} + return d.DialContext(ctx, network, address) + }, + }, + TryAuthoritativeDNS: true, + SkipDNSDomainSuffixes: []string{".bsky.social"}, + UserAgent: "atcr-identity/testmode", + } + inner := identity.NewCacheDirectory(&base, 250_000, 24*time.Hour, 2*time.Minute, 5*time.Minute) + return &localDirectory{ + inner: inner, + client: &http.Client{Timeout: 10 * time.Second}, + } +} + +// NewOAuthClientApp constructs an indigo OAuth client app for ATCR. Test-mode +// builds replace the HTTP clients indigo uses for PAR, token, and auth-server +// metadata requests with plain timeout-only clients, so the flow can reach a +// PDS on loopback or a private address. The identity directory is the shared +// process-wide one, which in this build already understands local did:web. +func NewOAuthClientApp(cfg *oauth.ClientConfig, store oauth.ClientAuthStore) *oauth.ClientApp { + app := oauth.NewClientApp(cfg, store) + app.Client = &http.Client{Timeout: 30 * time.Second} + app.Resolver.Client = &http.Client{Timeout: 10 * time.Second} + app.Dir = GetDirectory() + return app +} + +// localDirectory implements identity.Directory. It handles did:web identifiers +// that name a local host itself and delegates everything else to inner. +type localDirectory struct { + inner identity.Directory + client *http.Client +} + +var _ identity.Directory = (*localDirectory)(nil) + +func (d *localDirectory) LookupDID(ctx context.Context, did syntax.DID) (*identity.Identity, error) { + if docURL, ok := localDIDWebURL(did); ok { + return d.resolveLocalDIDWeb(ctx, did, docURL) + } + return d.inner.LookupDID(ctx, did) +} + +func (d *localDirectory) LookupHandle(ctx context.Context, handle syntax.Handle) (*identity.Identity, error) { + return d.inner.LookupHandle(ctx, handle) +} + +func (d *localDirectory) Lookup(ctx context.Context, atid syntax.AtIdentifier) (*identity.Identity, error) { + if did, err := atid.AsDID(); err == nil { + return d.LookupDID(ctx, did) + } + return d.inner.Lookup(ctx, atid) +} + +func (d *localDirectory) Purge(ctx context.Context, atid syntax.AtIdentifier) error { + return d.inner.Purge(ctx, atid) +} + +// localDIDWebURL reports whether did is a did:web naming a local host, and if +// so returns the plain-HTTP URL of its DID document. +// +// did:web encodes a port as %3A and uses a bare ':' to separate optional path +// segments, so "did:web:localhost%3A8080" is http://localhost:8080 and +// "did:web:127.0.0.1:hold" would be http://127.0.0.1/hold/did.json. A host is +// local when it is an IP literal, localhost (or a *.localhost name), or carries +// a port at all: indigo rejects every one of those, and none of them can be +// anything but a development endpoint. +func localDIDWebURL(did syntax.DID) (string, bool) { + if did.Method() != "web" { + return "", false + } + segments := strings.Split(did.Identifier(), ":") + hostPort := strings.NewReplacer("%3A", ":", "%3a", ":").Replace(segments[0]) + if hostPort == "" { + return "", false + } + + hostname := hostPort + hasPort := false + if h, _, err := net.SplitHostPort(hostPort); err == nil { + hostname = h + hasPort = true + } + local := hasPort || + hostname == "localhost" || + strings.HasSuffix(hostname, ".localhost") || + net.ParseIP(hostname) != nil + if !local { + return "", false + } + + path := "/.well-known/did.json" + if len(segments) > 1 { + path = "/" + strings.Join(segments[1:], "/") + "/did.json" + } + return "http://" + hostPort + path, true +} + +// resolveLocalDIDWeb fetches and parses a local did:web document. The handle +// is left as handle.invalid: nothing on a loopback address has a verifiable +// handle, and no caller in this codebase reads it for a hold or appview DID. +func (d *localDirectory) resolveLocalDIDWeb(ctx context.Context, did syntax.DID, docURL string) (*identity.Identity, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, docURL, nil) + if err != nil { + return nil, fmt.Errorf("%w: local did:web request for %s: %w", identity.ErrDIDResolutionFailed, did, err) + } + resp, err := d.client.Do(req) + if err != nil { + return nil, fmt.Errorf("%w: local did:web fetch %s: %w", identity.ErrDIDResolutionFailed, docURL, err) + } + defer resp.Body.Close() + + switch { + case resp.StatusCode == http.StatusNotFound: + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxLocalDIDDocSize)) + return nil, fmt.Errorf("%w: local did:web HTTP 404 for %s", identity.ErrDIDNotFound, did) + case resp.StatusCode != http.StatusOK: + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxLocalDIDDocSize)) + return nil, fmt.Errorf("%w: local did:web HTTP %d for %s", identity.ErrDIDResolutionFailed, resp.StatusCode, did) + } + + var doc identity.DIDDocument + if err := json.NewDecoder(io.LimitReader(resp.Body, maxLocalDIDDocSize)).Decode(&doc); err != nil { + return nil, fmt.Errorf("%w: local did:web document for %s: %w", identity.ErrDIDResolutionFailed, did, err) + } + // ParseIdentity keeps only keys whose controller matches the document's own + // id, so a document that omits both still yields its keys. Either way the + // identity is for the DID that was asked for. + ident := identity.ParseIdentity(&doc) + ident.DID = did + ident.Handle = syntax.HandleInvalid + return &ident, nil +} diff --git a/pkg/atproto/indigo_local_test.go b/pkg/atproto/indigo_local_test.go new file mode 100644 index 0000000..c65cad2 --- /dev/null +++ b/pkg/atproto/indigo_local_test.go @@ -0,0 +1,149 @@ +//go:build testmode + +package atproto + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/bluesky-social/indigo/atproto/atcrypto" + "github.com/bluesky-social/indigo/atproto/identity" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +func TestLocalDIDWebURL(t *testing.T) { + cases := []struct { + did string + want string + local bool + }{ + {"did:web:localhost%3A8080", "http://localhost:8080/.well-known/did.json", true}, + {"did:web:127.0.0.1%3A5000", "http://127.0.0.1:5000/.well-known/did.json", true}, + {"did:web:172.28.0.4%3A5002", "http://172.28.0.4:5002/.well-known/did.json", true}, + {"did:web:localhost", "http://localhost/.well-known/did.json", true}, + {"did:web:hold.localhost", "http://hold.localhost/.well-known/did.json", true}, + {"did:web:10.0.0.7", "http://10.0.0.7/.well-known/did.json", true}, + {"did:web:example.com%3A8443", "http://example.com:8443/.well-known/did.json", true}, + {"did:web:127.0.0.1:hold", "http://127.0.0.1/hold/did.json", true}, + {"did:web:hold01.atcr.io", "", false}, + {"did:web:atcr.io:path", "", false}, + {"did:plc:aaaaaaaaaaaaaaaaaaaaaaaa", "", false}, + } + for _, tc := range cases { + t.Run(tc.did, func(t *testing.T) { + did, err := syntax.ParseDID(tc.did) + if err != nil { + t.Fatalf("ParseDID(%q): %v", tc.did, err) + } + got, local := localDIDWebURL(did) + if local != tc.local { + t.Fatalf("localDIDWebURL(%q) local = %v, want %v", tc.did, local, tc.local) + } + if got != tc.want { + t.Errorf("localDIDWebURL(%q) = %q, want %q", tc.did, got, tc.want) + } + }) + } +} + +// TestLocalDirectoryResolvesLoopbackDIDWeb is the end-to-end check for the +// test-mode seam: a did:web naming a loopback host with a port resolves through +// the shared directory by fetching its document over plain HTTP, and the +// resolver helpers built on the directory need no fallback of their own. +func TestLocalDirectoryResolvesLoopbackDIDWeb(t *testing.T) { + priv, err := atcrypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatalf("generate P-256: %v", err) + } + pub, err := priv.PublicKey() + if err != nil { + t.Fatalf("public key: %v", err) + } + + mux := http.NewServeMux() + var srv *httptest.Server + mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + did := "did:web:" + strings.Replace(strings.TrimPrefix(srv.URL, "http://"), ":", "%3A", 1) + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": did, + "verificationMethod": []map[string]string{{ + "id": did + "#appview", + "type": "Multikey", + "controller": did, + "publicKeyMultibase": pub.Multibase(), + }}, + "service": []map[string]string{ + {"id": "#atproto_pds", "type": "AtprotoPersonalDataServer", "serviceEndpoint": srv.URL}, + {"id": "#atcr_hold", "type": "AtcrHoldService", "serviceEndpoint": srv.URL}, + }, + }) + }) + srv = httptest.NewServer(mux) + t.Cleanup(srv.Close) + + did := "did:web:" + strings.Replace(strings.TrimPrefix(srv.URL, "http://"), ":", "%3A", 1) + + SetDirectory(NewDirectory()) + t.Cleanup(func() { SetDirectory(nil) }) + ctx := context.Background() + + ident, err := GetDirectory().LookupDID(ctx, syntax.DID(did)) + if err != nil { + t.Fatalf("LookupDID(%q): %v", did, err) + } + if ident.DID.String() != did { + t.Errorf("identity DID = %q, want %q", ident.DID, did) + } + if ident.Handle != syntax.HandleInvalid { + t.Errorf("identity handle = %q, want handle.invalid (no verification on loopback)", ident.Handle) + } + if _, err := ident.GetPublicKey("appview"); err != nil { + t.Errorf("GetPublicKey(appview): %v", err) + } + + url, err := ResolveHoldDIDToURL(ctx, did) + if err != nil { + t.Fatalf("ResolveHoldDIDToURL(%q): %v", did, err) + } + if url != srv.URL { + t.Errorf("ResolveHoldDIDToURL = %q, want %q", url, srv.URL) + } + + isHold, err := HasHoldService(ctx, did) + if err != nil { + t.Fatalf("HasHoldService(%q): %v", did, err) + } + if !isHold { + t.Error("HasHoldService = false, want true: document declares #atcr_hold") + } + + // Lookup by at-identifier takes the same path. + atid, _ := syntax.ParseAtIdentifier(did) + if _, err := GetDirectory().Lookup(ctx, atid); err != nil { + t.Errorf("Lookup(%q): %v", did, err) + } +} + +func TestLocalDirectoryMissingDocumentIsNotFound(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + t.Cleanup(srv.Close) + did := "did:web:" + strings.Replace(strings.TrimPrefix(srv.URL, "http://"), ":", "%3A", 1) + + _, err := NewDirectory().LookupDID(context.Background(), syntax.DID(did)) + if !errors.Is(err, identity.ErrDIDNotFound) { + t.Fatalf("LookupDID on a 404 = %v, want ErrDIDNotFound", err) + } +} + +func TestLocalDirectoryUnreachableHostIsResolutionFailure(t *testing.T) { + // Port 1 (tcpmux) is never bound on a developer machine. + _, err := NewDirectory().LookupDID(context.Background(), syntax.DID("did:web:127.0.0.1%3A1")) + if !errors.Is(err, identity.ErrDIDResolutionFailed) { + t.Fatalf("LookupDID on an unreachable host = %v, want ErrDIDResolutionFailed", err) + } +} diff --git a/pkg/atproto/indigo_prod.go b/pkg/atproto/indigo_prod.go new file mode 100644 index 0000000..a5bb16c --- /dev/null +++ b/pkg/atproto/indigo_prod.go @@ -0,0 +1,33 @@ +//go:build !testmode + +package atproto + +import ( + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/identity" +) + +// TestModeBuild reports whether this binary was compiled with `-tags testmode`. +// +// A production build is never a test-mode build. The identity directory and +// OAuth client below are indigo's hardened defaults, which refuse loopback and +// private-network addresses and require HTTPS on port 443 for did:web. There +// is deliberately no runtime switch that can relax them. +const TestModeBuild = false + +// NewDirectory returns the identity directory used for every DID and handle +// lookup in this process. Production builds use indigo's default directory +// unchanged: PLC + did:web over HTTPS with SSRF protection, cached 24h. +func NewDirectory() identity.Directory { + return identity.DefaultDirectory() +} + +// NewOAuthClientApp constructs an indigo OAuth client app for ATCR. Production +// builds keep indigo's hardened HTTP clients for PAR, token, and auth-server +// metadata requests; only the identity directory is swapped for the shared +// process-wide one so OAuth lookups hit the same cache as everything else. +func NewOAuthClientApp(cfg *oauth.ClientConfig, store oauth.ClientAuthStore) *oauth.ClientApp { + app := oauth.NewClientApp(cfg, store) + app.Dir = GetDirectory() + return app +} diff --git a/pkg/atproto/indigo_prod_test.go b/pkg/atproto/indigo_prod_test.go new file mode 100644 index 0000000..a859edb --- /dev/null +++ b/pkg/atproto/indigo_prod_test.go @@ -0,0 +1,25 @@ +//go:build !testmode + +package atproto + +import ( + "context" + "testing" + + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// TestProdDirectoryRejectsLocalDIDWeb pins the production contract: without +// `-tags testmode` there is no plain-HTTP path for a port-qualified or loopback +// did:web, so such an identifier fails before any network request is made +// (indigo rejects the identifier as not a plain hostname). +func TestProdDirectoryRejectsLocalDIDWeb(t *testing.T) { + if TestModeBuild { + t.Fatal("TestModeBuild must be false without -tags testmode") + } + for _, did := range []string{"did:web:localhost%3A8080", "did:web:127.0.0.1%3A5000"} { + if _, err := NewDirectory().LookupDID(context.Background(), syntax.DID(did)); err == nil { + t.Errorf("LookupDID(%q) succeeded in a production build; local did:web must not resolve", did) + } + } +} diff --git a/pkg/atproto/resolver.go b/pkg/atproto/resolver.go index 03447e9..9d35dda 100644 --- a/pkg/atproto/resolver.go +++ b/pkg/atproto/resolver.go @@ -29,6 +29,10 @@ var ErrHoldDIDPermanent = errors.New("hold DID is not resolvable") // error prose, is what lets ResolveHoldDIDToURL classify the failure as // permanent. Conservative by design: anything not clearly hopeless is left to // the transient path, so an unfamiliar failure still surfaces loudly. +// +// A `-tags testmode` build resolves exactly these identifiers over plain HTTP +// (see indigo_local.go), so there a failure means the local service is down +// rather than unresolvable; the quieter classification is harmless in dev. func didWebHostUnusable(did string) bool { host, ok := strings.CutPrefix(did, "did:web:") if !ok { @@ -138,12 +142,6 @@ func ResolveHoldDIDToURL(ctx context.Context, did string) (string, error) { ident, err := directory.LookupDID(ctx, didParsed) if err != nil { - // In test mode, fall back to deriving URL directly from did:web. - // The indigo directory hardcodes HTTPS and rejects IPs/ports, - // so local dev (HTTP, IP:port) always needs this fallback. - if testMode && strings.HasPrefix(did, "did:web:") { - return DIDWebToURL(did), nil - } // A missing DID document or a structurally unusable did:web will fail // identically on every retry, so mark it permanent and let callers pick // a quieter log level. @@ -180,12 +178,6 @@ func HasHoldService(ctx context.Context, did string) (bool, error) { ident, err := GetDirectory().LookupDID(ctx, didParsed) if err != nil { - // In test mode, local did:web identifiers (HTTP, IP:port) are not - // resolvable by the indigo directory at all — trust them, matching - // the ResolveHoldDIDToURL fallback. - if testMode && strings.HasPrefix(did, "did:web:") { - return true, nil - } return false, fmt.Errorf("failed to resolve DID %s: %w", did, err) } @@ -207,19 +199,6 @@ func NormalizeDID(did string) string { return "did:web:" + host } -// DIDWebToURL converts a did:web DID to its base URL. -// did:web:example.com → https://example.com -// did:web:172.28.0.3%3A8080 → http://172.28.0.3:8080 -func DIDWebToURL(did string) string { - host := strings.TrimPrefix(did, "did:web:") - host = strings.ReplaceAll(host, "%3A", ":") - scheme := "https" - if strings.Contains(host, ":") { - scheme = "http" - } - return scheme + "://" + host -} - // ResolveDIDToPDS resolves a DID to its PDS endpoint. // Uses the shared identity directory with cache TTL and event-driven invalidation. func ResolveDIDToPDS(ctx context.Context, did string) (string, error) { diff --git a/pkg/atproto/resolver_test.go b/pkg/atproto/resolver_test.go index 7d4f881..f0fc297 100644 --- a/pkg/atproto/resolver_test.go +++ b/pkg/atproto/resolver_test.go @@ -92,54 +92,13 @@ func TestHasHoldService(t *testing.T) { t.Error("HasHoldService(unresolvable) expected error, got nil") } }) -} -// TestHasHoldServiceTestMode covers the test-mode fallback: local did:web -// identifiers (HTTP, IP:port) that the indigo directory cannot resolve are -// trusted, matching the ResolveHoldDIDToURL fallback. Resolvable DIDs are -// still checked normally, and the fallback never applies to other methods. -func TestHasHoldServiceTestMode(t *testing.T) { - pdsOnlyDID := "did:plc:ordinaryaccount" - - SetDirectory(&stubDirectory{byDID: map[string]*identity.Identity{ - pdsOnlyDID: { - DID: syntax.DID(pdsOnlyDID), - Services: map[string]identity.ServiceEndpoint{ - "atproto_pds": {Type: "AtprotoPersonalDataServer", URL: "https://pds.example.com"}, - }, - }, - }}) - defer SetDirectory(nil) - - SetTestMode(true) - defer SetTestMode(false) - - ctx := context.Background() - - t.Run("trusts unresolvable did:web", func(t *testing.T) { - localDID := "did:web:172.28.0.3%3A8080" - isHold, err := HasHoldService(ctx, localDID) - if err != nil { - t.Fatalf("HasHoldService(%q) unexpected error: %v", localDID, err) - } - if !isHold { - t.Errorf("HasHoldService(%q) = false, want true (test-mode did:web fallback)", localDID) - } - }) - - t.Run("rejects unresolvable did:plc", func(t *testing.T) { - if _, err := HasHoldService(ctx, "did:plc:unknown000000000000"); err == nil { - t.Error("HasHoldService(unresolvable did:plc) expected error even in test mode, got nil") - } - }) - - t.Run("still checks resolvable DIDs", func(t *testing.T) { - isHold, err := HasHoldService(ctx, pdsOnlyDID) - if err != nil { - t.Fatalf("HasHoldService(%q) unexpected error: %v", pdsOnlyDID, err) - } - if isHold { - t.Errorf("HasHoldService(%q) = true, want false (fallback must not bypass a successful lookup)", pdsOnlyDID) + // Local did:web identifiers are no exception: resolving them is the + // identity directory's job (indigo_local.go under -tags testmode), and + // HasHoldService itself never trusts a DID it could not resolve. + t.Run("unresolvable local did:web", func(t *testing.T) { + if _, err := HasHoldService(ctx, "did:web:172.28.0.3%3A8080"); err == nil { + t.Error("HasHoldService(unresolvable did:web) expected error, got nil") } }) } diff --git a/pkg/auth/hold_remote_captain_verify_test.go b/pkg/auth/hold_remote_captain_verify_test.go index d304212..fed1896 100644 --- a/pkg/auth/hold_remote_captain_verify_test.go +++ b/pkg/auth/hold_remote_captain_verify_test.go @@ -10,6 +10,7 @@ import ( "time" "atcr.io/pkg/atproto" + "atcr.io/pkg/testpds" ) // 69307c0 verified captain records against the publishing DID's atcr_hold @@ -23,9 +24,18 @@ import ( // captainServer serves a captain record for any repo, with allowAllCrew set — // the value that makes a row visible to every user rather than just its author. +// +// It also serves its own did:web document so the test-mode identity directory +// can resolve the DID derived from its URL (see didFromServer) back to it. func captainServer(t *testing.T) *httptest.Server { t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requireTestModeBuild(t) + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + base := "http://" + r.Host + testpds.HoldDIDDocumentHandler(testpds.DIDWebForURL(base), base)(w, r) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { _ = json.NewEncoder(w).Encode(map[string]any{ "uri": "at://x/io.atcr.hold.captain/self", "cid": "bafytest", @@ -36,11 +46,22 @@ func captainServer(t *testing.T) *httptest.Server { "allowAllCrew": true, }, }) - })) + }) + srv := httptest.NewServer(mux) t.Cleanup(srv.Close) return srv } +// requireTestModeBuild fails fast, with the reason, when the binary cannot +// resolve a loopback did:web. Without it these tests die on an opaque dial +// error from indigo's hardened directory. +func requireTestModeBuild(t *testing.T) { + t.Helper() + if !atproto.TestModeBuild { + t.Fatal("this test resolves a did:web on 127.0.0.1 and needs a `-tags testmode` build") + } +} + func didFromServer(url string) string { return "did:web:" + strings.ReplaceAll(strings.TrimPrefix(url, "http://"), ":", "%3A") } diff --git a/pkg/auth/oauth/client.go b/pkg/auth/oauth/client.go index 36ddd8b..a4e01d0 100644 --- a/pkg/auth/oauth/client.go +++ b/pkg/auth/oauth/client.go @@ -100,8 +100,7 @@ func NewClientApp(baseURL string, store oauth.ClientAuthStore, scopes []string, slog.Info("Using public OAuth client (localhost development)") } - clientApp := oauth.NewClientApp(&config, store) - clientApp.Dir = atproto.GetDirectory() + clientApp := atproto.NewOAuthClientApp(&config, store) return clientApp, nil } @@ -132,8 +131,7 @@ func NewClientAppWithKey(baseURL string, store oauth.ClientAuthStore, scopes []s slog.Info("Using public OAuth client (localhost development)") } - clientApp := oauth.NewClientApp(&config, store) - clientApp.Dir = atproto.GetDirectory() + clientApp := atproto.NewOAuthClientApp(&config, store) return clientApp, nil } diff --git a/pkg/billing/tier_resolution_test.go b/pkg/billing/tier_resolution_test.go index 1803a3b..921e2d9 100644 --- a/pkg/billing/tier_resolution_test.go +++ b/pkg/billing/tier_resolution_test.go @@ -7,11 +7,11 @@ import ( "fmt" "net/http" "net/http/httptest" - "strings" "testing" "time" "atcr.io/pkg/atproto" + "atcr.io/pkg/testpds" "github.com/bluesky-social/indigo/atproto/atcrypto" "github.com/stripe/stripe-go/v84" ) @@ -100,6 +100,27 @@ func TestResolveTier_UnknownEverythingIsUnresolved(t *testing.T) { } } +// managedHoldServer starts an httptest server standing in for a managed hold +// and returns it with the did:web the tier fan-out resolves back to it. The +// server serves its own DID document at /.well-known/did.json; handler gets +// every other request. Resolving a loopback did:web needs a `-tags testmode` +// build, so the test fails fast with that message otherwise. +func managedHoldServer(t *testing.T, handler http.HandlerFunc) (*httptest.Server, string) { + t.Helper() + if !atproto.TestModeBuild { + t.Fatal("this test resolves a did:web on 127.0.0.1 and needs a `-tags testmode` build") + } + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + base := "http://" + r.Host + testpds.HoldDIDDocumentHandler(testpds.DIDWebForURL(base), base)(w, r) + }) + mux.Handle("/", handler) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv, testpds.DIDWebForURL(srv.URL) +} + // TestHandleSubscriptionChange_GrandfatheredSubscriberKeepsTier runs the same // property through the real webhook path, so it covers the payload shape too: // Stripe sends price.product as a bare ID string, and this fails if that is @@ -126,15 +147,13 @@ func TestHandleSubscriptionChange_GrandfatheredSubscriberKeepsTier(t *testing.T) TierRank int `json:"tierRank"` } pushes := make(chan tierPush, 4) - hold := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, holdDID := managedHoldServer(t, func(w http.ResponseWriter, r *http.Request) { var got tierPush _ = json.NewDecoder(r.Body).Decode(&got) pushes <- got w.WriteHeader(http.StatusOK) - })) - defer hold.Close() - m.managedHolds = []string{"did:web:" + strings.ReplaceAll( - strings.TrimPrefix(hold.URL, "http://"), ":", "%3A")} + }) + m.managedHolds = []string{holdDID} priv, err := atcrypto.GeneratePrivateKeyP256() if err != nil { diff --git a/pkg/billing/webhook_retry_test.go b/pkg/billing/webhook_retry_test.go index 4169f6c..af8c83b 100644 --- a/pkg/billing/webhook_retry_test.go +++ b/pkg/billing/webhook_retry_test.go @@ -307,12 +307,10 @@ func TestHandleSubscriptionChange_HoldFanoutFailureIsRetryable(t *testing.T) { stripeAPIReturning(t, http.StatusOK, `{"id":"cus_fanout","object":"customer","metadata":{"user_did":"did:plc:fanoutuser"}}`) - hold := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, holdDID := managedHoldServer(t, func(w http.ResponseWriter, r *http.Request) { http.Error(w, "hold is down", http.StatusServiceUnavailable) - })) - defer hold.Close() - m.managedHolds = []string{"did:web:" + strings.ReplaceAll( - strings.TrimPrefix(hold.URL, "http://"), ":", "%3A")} + }) + m.managedHolds = []string{holdDID} priv, err := atcrypto.GeneratePrivateKeyP256() if err != nil { diff --git a/pkg/hold/admin/admin.go b/pkg/hold/admin/admin.go index e00f7c6..d302795 100644 --- a/pkg/hold/admin/admin.go +++ b/pkg/hold/admin/admin.go @@ -153,8 +153,7 @@ func NewAdminUI(ctx context.Context, holdPDS *pds.HoldPDS, quotaMgr *quota.Manag "redirect_uri", redirectURI) } - clientApp := indigooauth.NewClientApp(&oauthConfig, oauthStore) - clientApp.Dir = atproto.GetDirectory() + clientApp := atproto.NewOAuthClientApp(&oauthConfig, oauthStore) // Parse templates templates, err := parseTemplates() diff --git a/pkg/hold/pds/appview_token_test.go b/pkg/hold/pds/appview_token_test.go index 66f686e..8b8157b 100644 --- a/pkg/hold/pds/appview_token_test.go +++ b/pkg/hold/pds/appview_token_test.go @@ -4,12 +4,12 @@ import ( "encoding/json" "net/http" "net/http/httptest" - "net/url" "strings" "testing" "atcr.io/pkg/atproto" "atcr.io/pkg/auth" + "atcr.io/pkg/testpds" "github.com/bluesky-social/indigo/atproto/atcrypto" ) @@ -41,33 +41,33 @@ func newAppviewTestEnvWithKey(t *testing.T, priv atcrypto.PrivateKey) *appviewTe t.Fatalf("public key: %v", err) } - doc := map[string]any{ - "@context": []string{"https://www.w3.org/ns/did/v1"}, - "verificationMethod": []map[string]string{{ - "id": "#atproto", - "type": "Multikey", - "publicKeyMultibase": pub.Multibase(), - }}, + // fetchAppviewPublicKey resolves the appview DID through the identity + // directory; only a `-tags testmode` build resolves a loopback did:web. + if !atproto.TestModeBuild { + t.Fatal("this test resolves a did:web on 127.0.0.1 and needs a `-tags testmode` build") } + // The document mirrors what the real appview serves: the key is + // controlled by the document's own DID, so the directory keeps it. mux := http.NewServeMux() mux.HandleFunc("/.well-known/did.json", func(w http.ResponseWriter, r *http.Request) { + did := testpds.DIDWebForURL("http://" + r.Host) w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(doc) + _ = json.NewEncoder(w).Encode(map[string]any{ + "@context": []string{"https://www.w3.org/ns/did/v1"}, + "id": did, + "verificationMethod": []map[string]string{{ + "id": did + "#appview", + "type": "Multikey", + "controller": did, + "publicKeyMultibase": pub.Multibase(), + }}, + }) }) server := httptest.NewServer(mux) t.Cleanup(server.Close) - u, err := url.Parse(server.URL) - if err != nil { - t.Fatalf("parse server URL: %v", err) - } - // did:web requires percent-encoded ":" for ports - appviewDID := "did:web:" + strings.ReplaceAll(u.Host, ":", "%3A") - - // fetchAppviewPublicKey downgrades to http when test mode is on - atproto.SetTestMode(true) - t.Cleanup(func() { atproto.SetTestMode(false) }) + appviewDID := testpds.DIDWebForURL(server.URL) // Reset shared jti cache so other tests don't pollute this one and vice versa. *sharedAppviewJTICache = *newJTIReplayCache() diff --git a/pkg/hold/pds/auth.go b/pkg/hold/pds/auth.go index 0eca3dd..b92a1a3 100644 --- a/pkg/hold/pds/auth.go +++ b/pkg/hold/pds/auth.go @@ -695,7 +695,21 @@ func ValidateAppviewToken(r *http.Request, appviewDID, holdDID string) (string, } if err := pubKey.HashAndVerifyLenient(signedData, signature); err != nil { - return "", fmt.Errorf("signature verification failed: %w", err) + // The identity directory caches the appview's DID document for up to + // 24h. If the appview re-keyed since (a fresh database, a rotated + // OAuth key), the cached key is stale and every token fails until the + // entry expires. Purge and try once more against a fresh document + // before rejecting. + if perr := atproto.InvalidateIdentity(r.Context(), appviewDID); perr != nil { + return "", fmt.Errorf("signature verification failed: %w", err) + } + pubKey, ferr := fetchAppviewPublicKey(r.Context(), appviewDID) + if ferr != nil { + return "", fmt.Errorf("signature verification failed: %w", err) + } + if err := pubKey.HashAndVerifyLenient(signedData, signature); err != nil { + return "", fmt.Errorf("signature verification failed: %w", err) + } } // Replay check: when `jti` is present, refuse to honour the same one @@ -714,60 +728,46 @@ func ValidateAppviewToken(r *http.Request, appviewDID, holdDID string) (string, return subject, nil } -// fetchAppviewPublicKey fetches the appview's verification key from its -// did:web DID document. Accepts any Multikey atcrypto can parse (P-256 or -// K-256). Returns the first one found. +// appviewKeyFragments lists the verification-method fragments an appview may +// publish its signing key under, in preference order. The appview's own DID +// document uses #appview; #atproto is the conventional fragment and what the +// test fixtures serve. +var appviewKeyFragments = []string{"appview", "atproto"} + +// fetchAppviewPublicKey resolves the appview's DID document through the shared +// identity directory and returns its verification key. Accepts any key the +// directory can parse (P-256 or K-256): a known fragment first, then whatever +// else the document declares. +// +// The directory is what makes this work in every build: production resolves +// did:web over HTTPS with indigo's hardening, and a `-tags testmode` build +// resolves a port-qualified or loopback did:web over plain HTTP. func fetchAppviewPublicKey(ctx context.Context, did string) (atcrypto.PublicKey, error) { - if !strings.HasPrefix(did, "did:web:") { - return nil, fmt.Errorf("only did:web is supported for appview DID, got %s", did) - } - - host := strings.TrimPrefix(did, "did:web:") - host = strings.ReplaceAll(host, "%3A", ":") - scheme := "https" - if atproto.IsTestMode() { - scheme = "http" - } - didDocURL := fmt.Sprintf("%s://%s/.well-known/did.json", scheme, host) - - req, err := http.NewRequestWithContext(ctx, "GET", didDocURL, nil) + didParsed, err := syntax.ParseDID(did) if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) + return nil, fmt.Errorf("invalid appview DID %q: %w", did, err) } - resp, err := http.DefaultClient.Do(req) + ident, err := atproto.GetDirectory().LookupDID(ctx, didParsed) if err != nil { - return nil, fmt.Errorf("failed to fetch DID document: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("DID document fetch returned status %d", resp.StatusCode) + return nil, fmt.Errorf("failed to resolve appview DID %s: %w", did, err) } - var doc struct { - VerificationMethod []struct { - ID string `json:"id"` - Type string `json:"type"` - PublicKeyMultibase string `json:"publicKeyMultibase"` - } `json:"verificationMethod"` + for _, fragment := range appviewKeyFragments { + if pubKey, err := ident.GetPublicKey(fragment); err == nil { + return pubKey, nil + } } - if err := json.NewDecoder(resp.Body).Decode(&doc); err != nil { - return nil, fmt.Errorf("failed to decode DID document: %w", err) - } - - for _, vm := range doc.VerificationMethod { - if vm.Type != "Multikey" || vm.PublicKeyMultibase == "" { + for fragment := range ident.Keys { + if slices.Contains(appviewKeyFragments, fragment) { continue } - pubKey, err := atcrypto.ParsePublicMultibase(vm.PublicKeyMultibase) - if err != nil { - continue + if pubKey, err := ident.GetPublicKey(fragment); err == nil { + return pubKey, nil } - return pubKey, nil } - return nil, fmt.Errorf("no Multikey verification method found in DID document for %s", did) + return nil, fmt.Errorf("no usable verification key in DID document for %s", did) } // fetchPublicKeyFromDID fetches the public key from a DID document diff --git a/pkg/labeler/server.go b/pkg/labeler/server.go index 28a7e1b..82cb2dc 100644 --- a/pkg/labeler/server.go +++ b/pkg/labeler/server.go @@ -79,8 +79,7 @@ func NewServer(cfg *Config) (*Server, error) { oauthConfig = indigooauth.NewPublicConfig(clientID, redirectURI, scopes) } - clientApp := indigooauth.NewClientApp(&oauthConfig, oauthStore) - clientApp.Dir = atproto.GetDirectory() + clientApp := atproto.NewOAuthClientApp(&oauthConfig, oauthStore) auth := NewAuth(cfg.Labeler.OwnerDID) diff --git a/pkg/testpds/didweb.go b/pkg/testpds/didweb.go new file mode 100644 index 0000000..6a20b93 --- /dev/null +++ b/pkg/testpds/didweb.go @@ -0,0 +1,33 @@ +package testpds + +import ( + "encoding/json" + "net/http" +) + +// DIDWebForURL returns the did:web identifier for a local http://host:port +// URL, percent-encoding the port colon as did:web requires. It is the DID a +// `-tags testmode` build resolves back to that URL by fetching +// http://host:port/.well-known/did.json. +func DIDWebForURL(rawURL string) string { + return "did:web:" + didWebForHost(rawURL) +} + +// HoldDIDDocumentHandler serves a minimal did:web document for a test server +// standing in for a hold: both the #atproto_pds and #atcr_hold services point +// at baseURL, and no keys are declared. Mount it at /.well-known/did.json so +// the test-mode identity directory can resolve the server's DID. +func HoldDIDDocumentHandler(did, baseURL string) http.HandlerFunc { + doc := map[string]any{ + "@context": []string{"https://www.w3.org/ns/did/v1"}, + "id": did, + "service": []map[string]string{ + {"id": "#atproto_pds", "type": "AtprotoPersonalDataServer", "serviceEndpoint": baseURL}, + {"id": "#atcr_hold", "type": "AtcrHoldService", "serviceEndpoint": baseURL}, + }, + } + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/did+ld+json") + _ = json.NewEncoder(w).Encode(doc) + } +}