diff --git a/docs/superpowers/plans/2026-07-26-provider-probing-p1-automated-confined.md b/docs/superpowers/plans/2026-07-26-provider-probing-p1-automated-confined.md new file mode 100644 index 00000000..87839117 --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-provider-probing-p1-automated-confined.md @@ -0,0 +1,515 @@ +# Provider Probing P1 — Automated, Confined Probing + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Turn the operator's one-shot geolocation prober into an automated worker that runs confined, picks its work from durable server-side state, and refuses to start unless its confinement is actually in place. + +**Architecture:** The prober stays an unprivileged process that assumes nothing about how it is confined. Each deployment supplies confinement natively — a restricted Docker network on beta, systemd `IPAddressDeny`/`IPAddressAllow` upstream — and the prober verifies it at startup by attempting a direct connection to a geolocation address and exiting if it succeeds. Due-provider selection moves from the prober's in-memory TTL cache to a server endpoint backed by `provider_egress_location` freshness, so a restart no longer re-probes everything. + +**Tech Stack:** Go 1.26.5, PostgreSQL 16, Redis, Docker Compose (beta only), systemd (mainstream). + +**Source spec:** `docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md` +**Predecessor:** `docs/superpowers/plans/2026-07-26-provider-probing-p0-prerequisites.md` (complete) + +## Global Constraints + +- **The hard constraint:** the operator's server must never send a request directly to a geolocation API. Every lookup egresses through a provider. Nothing in this plan may add a code path that could contact `ip.pn`, `free.freeipapi.com` or `ipinfo.io` other than through a provider tunnel. +- `geolocate/` in the operator-proxy repo must remain **standard-library-only**. No `golang.org/x/...`. +- The prober must require **no Linux capabilities** and must not create namespaces or firewall rules itself. Beta runs Docker Compose; the mainstream deployment does not use Docker at all. Anything that only works under one is wrong. +- **Two branches per server-side change:** a feature branch off `beta/self-contained-env` PR'd to `Ryanmello07/server`, plus a cherry-picked branch PR'd to `urnetwork/server` `main`. +- Remotes differ per checkout — run `git remote -v` before pushing. `/root/urnetwork/server`: `origin` = `Ryanmello07/server`, `upstream` = `urnetwork/server`. `/tmp/sandbox/server` has these **inverted**. Never push a feature branch to an upstream repo. +- Do not alter `PassesMinimums`, scoring weights, or thresholds. +- P1 is **direct probing only** — no identity rotation, no multi-hop, no verdict logic. Those are P3 and P2. Results land exactly as they do today. +- Server tests need the local stack: `local/run-local.sh`, then `./test.sh -run TestName`. A whole-package `go test ./model` is not achievable in this environment (pre-existing missing `subsidy.yml`); say so explicitly rather than claiming it passed. +- A **live beta deployment** runs on the dev machine via `docker-compose.beta.yml`. Never stop, recreate or `down` it. + +--- + +## File Structure + +| File | Responsibility | Change | +| --- | --- | --- | +| `model/network_client_location_model.go` (server) | `loadLocationStables` hardcodes `forceMinimum=false` | Modify — thread the flag through | +| `model/provider_egress_location_model.go` (server) | Egress location storage | Modify — add due-provider query | +| `api/handlers/provider_egress_location_handlers.go` (server) | Operator-authenticated ingest | Modify — add the due-list handler | +| `api/api.go` (server) | Route table | Modify — register the route | +| `confinement/confinement.go` (operator-proxy) | **New.** Startup self-check | Create | +| `cmd/egress-prober/main.go` (operator-proxy) | Prober CLI | Modify — self-check + server-driven due list | +| `docker-compose.beta.yml` + `BETA.md` (server) | Beta deployment | Modify — confined prober service | +| `docs/operator/prober-systemd.md` (server) | **New.** Non-Docker deployment | Create | + +--- + +## Task 1: Location enumeration honours `force_minimum` + +**Repo:** `/root/urnetwork/server`. Two branches: `feat/probe-location-force-minimum` and `-upstream`. + +**Context:** P0 made the prober send `force_minimum: true` to `find-providers2`, which bypasses `PassesMinimums` for *provider* selection. But the prober gets its **locations** from `GET /network/provider-locations`, which only emits a location if `loadLocationStables` has an entry — and that reads `clientScoreLocationFilterKey(false, rankMode, ...)` at `model/network_client_location_model.go:1877` with `forceMinimum` **hardcoded false**. + +Consequence: a location where every provider fails minimums is invisible to the prober, so those providers are never probed and can never graduate probation. `force_minimum` on the second call cannot recover what the first call never listed. This was the Important finding from the P0 final review. + +**Files:** +- Modify: `model/network_client_location_model.go:1858-1890` (`loadLocationStables`) and its callers +- Test: `model/network_client_location_model_test.go` + +**Interfaces:** +- Produces: `loadLocationStables(ctx, rankMode, forceMinimum bool, locationIds, ...)` — exact parameter order to be matched to the existing signature; add `forceMinimum` adjacent to `rankMode`. + +- [ ] **Step 1: Create the branch** + +```bash +cd /root/urnetwork/server +git checkout beta/self-contained-env && git pull --ff-only origin beta/self-contained-env +git checkout -b feat/probe-location-force-minimum +``` + +- [ ] **Step 2: Read before writing** + +Read `loadLocationStables` in full (`model/network_client_location_model.go:1858` onward) and find every caller with `grep -n 'loadLocationStables' model/*.go controller/*.go`. `GetProviderLocations` is the caller that matters. Note whether the writer side (`UpdateClientScores`, around `:2848`) already writes **both** the `forceMinimum=false` and `forceMinimum=true` key families — it does, since `exportClientScores` is invoked in a `for _, forceMinimum := range []bool{false, true}` loop. This task only fixes the read side. + +- [ ] **Step 3: Write the failing test** + +Add to `model/network_client_location_model_test.go`. One connected+valid Public provider whose score falls below the minimums, asserting it is invisible with `forceMinimum=false` and visible with `true`: + +```go +func TestLoadLocationStablesHonoursForceMinimum(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + handlerId := CreateNetworkClientHandler(ctx) + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, "0.0.0.1:0", handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + err = UpdateClientScores(ctx) + connect.AssertEqual(t, err, nil) + + locationIds := map[server.Id]bool{city.CountryLocationId: true} + + // this provider has no latency or speed test, so scoring penalises it + // past the minimums gate -- exactly the population the prober needs to + // reach and currently cannot see + strict, err := loadLocationStables(ctx, RankModeQuality, false, locationIds, server.Id{}) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, len(strict), 0) + + forced, err := loadLocationStables(ctx, RankModeQuality, true, locationIds, server.Id{}) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, len(forced), 1) + }) +} +``` + +Adapt the argument list to `loadLocationStables`'s real signature — read it first; do not guess the parameter order. If the function returns a map keyed differently, assert on presence of `city.CountryLocationId` rather than length. + +- [ ] **Step 4: Run it and confirm it fails** + +```bash +cd /root/urnetwork/server && ./local/run-local.sh --keep-up +./test.sh -run TestLoadLocationStablesHonoursForceMinimum +``` + +Expected: a compile failure (`too many arguments`) because `loadLocationStables` does not yet take `forceMinimum`. That is the correct first failure. + +- [ ] **Step 5: Thread the flag through** + +Add a `forceMinimum bool` parameter to `loadLocationStables` and pass it to `clientScoreLocationFilterKey` in place of the literal `false` at `:1877`. Update every caller. `GetProviderLocations` must pass `false` — user-facing location listing keeps today's behaviour and must not change. + +Add this comment above the parameter: + +```go + // forceMinimum selects which pre-computed key family to read. The writer + // (UpdateClientScores) populates both, so this only chooses between them. + // User-facing listing passes false and keeps today's behaviour; an operator + // census passes true, because a location where every provider fails the + // minimums gate is otherwise invisible and its providers can never be + // probed or graduate probation. +``` + +- [ ] **Step 6: Run the test and the regressions** + +```bash +./test.sh -run TestLoadLocationStablesHonoursForceMinimum +./test.sh -run TestUpdateClientScores +./test.sh -run TestUpdateClientLocations +go build ./... && go vet ./... +``` + +Expected: the new test passes; no previously-passing test regresses. + +- [ ] **Step 7: Commit, push, open both PRs** + +```bash +git add model/network_client_location_model.go model/network_client_location_model_test.go +git commit -m "fix(model): let loadLocationStables honour forceMinimum + +The read side hardcoded forceMinimum=false, so a location where every +provider fails the minimums gate was invisible even to callers that +explicitly bypass minimums. Providers there could never be probed and so +could never graduate probation. The writer already populates both key +families; this only chooses between them. User-facing listing still +passes false." +git push -u origin feat/probe-location-force-minimum +gh pr create --repo Ryanmello07/server --base beta/self-contained-env \ + --head feat/probe-location-force-minimum \ + --title "fix(model): let loadLocationStables honour forceMinimum" \ + --body "loadLocationStables hardcoded forceMinimum=false on the read side, hiding locations where every provider fails minimums. The writer already populates both key families. User-facing listing is unchanged." + +git fetch upstream main +git checkout -b feat/probe-location-force-minimum-upstream upstream/main +git cherry-pick feat/probe-location-force-minimum +go build ./... && go vet ./... +git push -u origin feat/probe-location-force-minimum-upstream +gh pr create --repo urnetwork/server --base main \ + --head Ryanmello07:feat/probe-location-force-minimum-upstream \ + --title "fix(model): let loadLocationStables honour forceMinimum" \ + --body "The read side hardcoded forceMinimum=false, hiding locations where every provider fails the minimums gate from callers that explicitly bypass minimums." +``` + +If the cherry-pick conflicts, keep upstream's surrounding code and carry only the parameter and its use. + +--- + +## Task 2: Server-side due-provider selection + +**Repo:** `/root/urnetwork/server`. Two branches: `feat/probe-due-endpoint` and `-upstream`. + +**Context:** the prober currently decides what to probe from an in-memory TTL cache, so a restart re-probes everything and there is no durable record of what is due. Freshness already lives server-side in `provider_egress_location.observed_at`. Expose it, so selection is durable and the server owns the schedule. + +The endpoint is operator-to-server, authenticated by the same shared secret as the ingest endpoint — not a network JWT. + +**Files:** +- Modify: `model/provider_egress_location_model.go` — add the query +- Modify: `api/handlers/provider_egress_location_handlers.go` — add the handler +- Modify: `api/api.go:57` area — register the route +- Test: `model/provider_egress_location_model_test.go`, `api/handlers/provider_egress_location_handlers_test.go` + +**Interfaces:** +- Produces: `model.GetProviderEgressLocationDue(ctx context.Context, minObservedAt time.Time, limit int) []server.Id` — client ids of Public providers whose newest probe is older than `minObservedAt` or absent. +- Produces: `GET /network/provider-egress-due?limit=N` returning `{"client_ids":["..."]}`. + +- [ ] **Step 1: Create the branch** + +```bash +cd /root/urnetwork/server +git checkout beta/self-contained-env && git pull --ff-only origin beta/self-contained-env +git checkout -b feat/probe-due-endpoint +``` + +- [ ] **Step 2: Write the failing model test** + +Add to `model/provider_egress_location_model_test.go`: + +```go +func TestGetProviderEgressLocationDue(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + now := server.NowUtc() + + fresh := server.NewId() + stale := server.NewId() + never := server.NewId() + + location := &Location{ + LocationType: LocationTypeCountry, + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, location) + + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: fresh, LocationId: location.LocationId, + CountryCode: "us", ObservedAt: now.Add(-1 * time.Hour), + }) + SetProviderEgressLocation(ctx, &ProviderEgressLocation{ + ClientId: stale, LocationId: location.LocationId, + CountryCode: "us", ObservedAt: now.Add(-72 * time.Hour), + }) + // `never` deliberately gets no row at all + + due := GetProviderEgressLocationDue(ctx, now.Add(-24*time.Hour), 100) + + // a provider probed an hour ago must not be re-probed; one probed three + // days ago must be; one never probed must be + connect.AssertEqual(t, slices.Contains(due, fresh), false) + connect.AssertEqual(t, slices.Contains(due, stale), true) + connect.AssertEqual(t, slices.Contains(due, never), true) + }) +} +``` + +Note: `never` only appears if the query sources candidates from connected Public providers rather than from `provider_egress_location` alone. That is the point — the set of things needing a probe is mostly things with no row yet. The test will need `never` set up as a connected client with a Public provide key, the same way Task 2 of the P0 plan did (`Testing_CreateDevice` + `ConnectNetworkClient` + `SetConnectionLocation` + `SetProvide`). Do that for all three ids so the only variable is probe freshness. + +- [ ] **Step 3: Run it and confirm it fails** + +Run: `./test.sh -run TestGetProviderEgressLocationDue` +Expected: compile failure, `undefined: GetProviderEgressLocationDue`. + +- [ ] **Step 4: Implement the query** + +Add to `model/provider_egress_location_model.go`. Source candidates from connected+valid Public providers, LEFT JOIN their egress row, and select those whose `observed_at` is missing or older than the cutoff. Mirror the P0 filter exactly — a provider without `provide_mode = 3` is not probeable and must not be returned. Order oldest-first so the longest-unprobed go first, and honour `limit`. + +Compute the cutoff in Go and pass it as an argument. **Do not** compare a naive `timestamp` column against SQL `now()` — this codebase has shipped that bug before; the comparison casts through the session timezone and silently skips a window. + +- [ ] **Step 5: Run the model test** + +Run: `./test.sh -run TestGetProviderEgressLocationDue` +Expected: PASS. + +- [ ] **Step 6: Write the failing handler test** + +Add to `api/handlers/provider_egress_location_handlers_test.go`, following the existing auth tests in that file. Cover: no secret → 401; wrong secret → 401; correct secret → 200 with a JSON body containing `client_ids`. The existing tests already show how `operatorIngestSecret` is stubbed — reuse that, and confirm your accept-case would fail against an always-401 handler (an earlier review caught exactly that gap in this file). + +- [ ] **Step 7: Implement the handler and route** + +Add `ProviderEgressLocationDue` to `api/handlers/provider_egress_location_handlers.go`, reusing `operatorSecretHeader`, the `hmac.Equal` comparison and the fail-closed `operatorIngestSecret()` already in that file. Parse `limit` from the query string, clamp it to a sane maximum (500) and default it (100). Register in `api/api.go` beside the existing route: + +```go + router.NewRoute("GET", "/network/provider-egress-due", handlers.ProviderEgressLocationDue), +``` + +- [ ] **Step 8: Verify and ship both branches** + +```bash +./test.sh -run TestGetProviderEgressLocationDue +./test.sh -run TestProviderEgressLocation +go build ./... && go vet ./... +``` + +Then commit, push, open the beta PR, cherry-pick onto `feat/probe-due-endpoint-upstream` based on `upstream/main`, and open the upstream PR — same shape as Task 1 Step 7. + +--- + +## Task 3: Prober confinement self-check and server-driven work + +**Repo:** `/tmp/sandbox/urnetwork-operator-proxy`, branch `main`. Single repo, no cherry-pick. + +**Context:** the prober must refuse to run unless its confinement is real. Because beta uses Docker and mainstream does not, the check cannot inspect namespaces or firewall rules — it must test the property directly: try to reach a geolocation API without a tunnel, and exit if that works. + +**Files:** +- Create: `confinement/confinement.go`, `confinement/confinement_test.go` +- Modify: `cmd/egress-prober/main.go` + +**Interfaces:** +- Consumes: `GET /network/provider-egress-due` from Task 2. +- Produces: `confinement.Verify(ctx context.Context, dial func(context.Context, string, string) (net.Conn, error), addrs []string, timeout time.Duration) error` + +- [ ] **Step 1: Write the failing test** + +Create `confinement/confinement_test.go`. The dialer is injected so the test never touches the real network: + +```go +func TestVerifyFailsWhenDirectConnectionSucceeds(t *testing.T) { + // a dialer that connects means nothing is stopping the prober reaching a + // geolocation api directly -- the confinement is absent and the whole + // guarantee is void, so Verify must refuse + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + c1, _ := net.Pipe() + return c1, nil + } + err := Verify(context.Background(), dial, []string{"34.117.59.81:443"}, time.Second) + if err == nil { + t.Fatal("Verify returned nil when a direct connection succeeded; it must refuse to run") + } +} + +func TestVerifyPassesWhenDirectConnectionRefused(t *testing.T) { + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("connect: network is unreachable") + } + if err := Verify(context.Background(), dial, []string{"34.117.59.81:443"}, time.Second); err != nil { + t.Fatalf("Verify errored when the connection was refused: %s", err) + } +} + +func TestVerifyRequiresAtLeastOneAddress(t *testing.T) { + // an empty address list would vacuously "pass" and silently disable the + // entire check + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("unreachable") + } + if err := Verify(context.Background(), dial, nil, time.Second); err == nil { + t.Fatal("Verify accepted an empty address list; that would disable the check") + } +} +``` + +- [ ] **Step 2: Run and confirm failure** + +Run: `cd /tmp/sandbox/urnetwork-operator-proxy && go test ./confinement/ -v` +Expected: build failure, `undefined: Verify`. + +- [ ] **Step 3: Implement** + +Create `confinement/confinement.go`: + +```go +// Package confinement verifies at startup that this process cannot reach a +// geolocation api directly. +// +// The prober's entire guarantee is that every geolocation lookup egresses +// through a provider, so the api reports the provider's address and never the +// operator's. That is enforced outside this process -- a restricted network +// under docker compose, systemd IPAddressDeny/IPAddressAllow otherwise -- and +// the mechanism differs per deployment. +// +// Rather than inspect a mechanism it cannot portably know, the prober tests the +// property: it attempts a direct connection and refuses to run if one succeeds. +// Operator configuration therefore stops being an assumption and becomes a +// precondition. +package confinement + +import ( + "context" + "errors" + "fmt" + "net" + "time" +) + +// ErrNotConfined reports that a direct connection succeeded. +var ErrNotConfined = errors.New("confinement: a direct connection to a geolocation address succeeded; this process is not confined") + +// ErrNoAddresses reports an empty address list, which would make the check +// vacuous. +var ErrNoAddresses = errors.New("confinement: at least one address is required") + +// DialFunc matches net.Dialer.DialContext. +type DialFunc func(ctx context.Context, network, addr string) (net.Conn, error) + +// Verify returns nil only when every address refuses a direct connection. +// +// A dial error is the expected, healthy outcome. A successful connection means +// the confinement is missing and returns ErrNotConfined. A timeout counts as +// refused: a dropped packet is what a deny rule looks like from inside. +func Verify(ctx context.Context, dial DialFunc, addrs []string, timeout time.Duration) error { + if len(addrs) == 0 { + return ErrNoAddresses + } + for _, addr := range addrs { + attemptCtx, cancel := context.WithTimeout(ctx, timeout) + conn, err := dial(attemptCtx, "tcp", addr) + cancel() + if err == nil { + if conn != nil { + conn.Close() + } + return fmt.Errorf("%w: %s", ErrNotConfined, addr) + } + } + return nil +} +``` + +- [ ] **Step 4: Run the tests** + +Run: `go test ./confinement/ -v` +Expected: all three PASS. + +- [ ] **Step 5: Wire the check into the prober** + +In `cmd/egress-prober/main.go`, immediately after flag parsing and before any tunnel or API work, call `confinement.Verify` with a real `net.Dialer{}.DialContext`, the geolocation IPs, and a short timeout (3s). On error, log and `os.Exit(1)`. + +Add a `--skip-confinement-check` flag defaulting to **false**, for the operator running the tool interactively as a one-shot diagnostic (which is how the manual probe is used). It must log loudly when set. Do not let it default true — a check that is off by default is not a check. + +Source the addresses from the same host list `geolocate` already knows, resolved to IPs at startup; do not hardcode a second copy of the endpoint list that can drift. + +- [ ] **Step 6: Replace the in-memory due cache with the server list** + +Add a `--due-url` flag (default: `/network/provider-egress-due`). When the operator secret is present, fetch the due client ids from Task 2's endpoint and probe exactly those, instead of enumerating everything and filtering by the in-memory TTL. Keep the existing enumeration path as the fallback when the endpoint returns 404, so the prober still works against a server that has not deployed Task 2. + +- [ ] **Step 7: Verify** + +```bash +go build ./... && go vet ./... && gofmt -l . && go test ./... -race -count=1 +``` + +Expected: all packages ok, `gofmt -l` silent. + +- [ ] **Step 8: Commit and push** + +```bash +git add confinement/ cmd/egress-prober/main.go +git commit -m "feat(confinement): refuse to run unless direct geolocation egress is blocked" +git push origin main +``` + +--- + +## Task 4: Deployment confinement for both environments + +**Repo:** `/root/urnetwork/server`. Two branches: `feat/probe-deployment-confinement` and `-upstream`. + +**Context:** the prober is only as confined as its deployment makes it. Beta gets a Compose service on a restricted network; mainstream gets a documented systemd unit. Task 3's self-check is what proves either one actually works. + +**Files:** +- Modify: `docker-compose.beta.yml` +- Modify: `BETA.md` +- Create: `docs/operator/prober-systemd.md` + +- [ ] **Step 1: Add the beta service** + +Add an `egress-prober` service to `docker-compose.beta.yml` on a **dedicated internal network** that reaches `api` and `connect` only. Give it the operator secret and prober JWT via the vault mount, `restart: unless-stopped`, and no added capabilities. + +The service must NOT be attached to the default network that reaches the public internet — that attachment is the confinement, and Task 3's self-check will fail the container at startup if it is wrong. That is the intended feedback loop: a misconfigured network stops the prober instead of silently leaking. + +- [ ] **Step 2: Verify the confinement actually holds** + +Bring up only the new service against the running beta stack, without recreating anything else: + +```bash +cd /root/urnetwork/server +docker compose -f docker-compose.beta.yml up -d --no-deps egress-prober +docker compose -f docker-compose.beta.yml logs --tail=30 egress-prober +``` + +Expected: the service starts and the self-check passes. Then prove the check has teeth by attaching it to the default network temporarily and confirming it **exits non-zero** with `ErrNotConfined`. Report both outcomes. + +- [ ] **Step 3: Document the non-Docker deployment** + +Create `docs/operator/prober-systemd.md` with a complete unit file using `IPAddressDeny=any` plus `IPAddressAllow=` entries for the platform api/connect addresses and localhost, `DynamicUser=yes`, `NoNewPrivileges=yes`, and no capabilities. State explicitly that `IPAddressAllow` takes addresses, not hostnames, so the operator must list the platform's addresses and update them if they change — and that Task 3's self-check is what catches it if they forget. + +- [ ] **Step 4: Update BETA.md** + +Document the new service, the secret it needs, and how to confirm the self-check passed. + +- [ ] **Step 5: Ship both branches** + +Commit, push, open the beta PR, cherry-pick onto `-upstream` based on `upstream/main`, open the upstream PR. Note that `docker-compose.beta.yml` is beta-only; if it does not exist upstream, carry only `docs/operator/prober-systemd.md` in the upstream cherry-pick and say so in the PR body. + +--- + +## Verification Summary + +| Task | Gate | +| --- | --- | +| 1 | New test proves a below-minimums location is hidden with `false` and visible with `true`; `GetProviderLocations` behaviour unchanged | +| 2 | Model test covers fresh / stale / never-probed; handler tests cover 401, 401, 200 and would fail against an always-401 handler | +| 3 | Three `confinement` tests including the empty-address case; full race suite green | +| 4 | Beta service starts confined; attaching it to the default network makes it exit non-zero | +| All | Both PRs open per server-side change | + +## Out of Scope + +- Identity rotation and multi-hop probing (P3). +- Verdict logic, RTT corroboration, the schema extension (P2). +- Bandwidth measurement and its byte budget (P2/P3). +- Letting measured bandwidth satisfy `PassesMinimums`, and whether providers should be penalised for tests their transport version cannot run. diff --git a/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md b/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md index bd61f8e1..8a912a96 100644 --- a/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md +++ b/docs/superpowers/specs/2026-07-25-enforced-provider-geo-probing-design.md @@ -31,7 +31,7 @@ code-level discipline alone (see *The jail*). | Question | Decision | | --- | --- | | Consequence of no fresh probe | **Advisory.** Provider stays listed, falls back to its mmdb location, and is flagged unverified. | -| Where the prober runs | **Taskworker, in a network-namespace jail** (a separate child process). | +| Where the prober runs | **A separate unprivileged process, confined by the deployment** (Docker network on beta, systemd `IPAddressAllow` upstream), self-verifying its confinement at startup. | | Tamper resistance | **Rotating identity + corroboration + multi-hop probing.** | | Which providers | **Public providers only** (`provide_mode = 3`). | @@ -64,9 +64,11 @@ Four units with clean boundaries: ### `egressprober` (new binary, operator-proxy repo) Wraps the existing `providertunnel`, `geolocate` and `ingest` libraries in a -long-lived worker that reads probe jobs and reports results. It is a separate -process specifically so it can be network-jailed; Go cannot confine a subset of -goroutines to a namespace. +worker that reads probe jobs and reports results. It is a separate process so +that the deployment can confine it as a whole — a process is the smallest unit +either Docker or systemd can apply an egress restriction to, and Go cannot +confine a subset of goroutines. Keeping it out of the taskworker also means the +component holding geolocation code is not the one holding database credentials. ### `provider_egress_probe_work.go` (new taskworker job, server repo) @@ -99,16 +101,39 @@ Extended, not replaced. ## The jail -The prober process runs in a network namespace whose only route is the platform -websocket endpoint. Not firewall rules inside a shared namespace — a distinct -netns, so no route to a geolocation API exists at all. +The prober runs confined so that no route to a geolocation API exists at all, +rather than relying on application code to decline to use one. -The Go-level fail-closed behaviour stays as defence in depth, but correctness of -the hard constraint stops depending on it. A future refactor that introduces a -default `http.Client` then fails loudly instead of silently leaking. +**The prober itself requires no privileges and assumes nothing about how it is +confined.** This matters because the deployments differ fundamentally: beta runs +under Docker Compose, and the mainstream deployment does not use Docker at all. +A design that depended on Docker networking, or on the process creating its own +namespace (which would need `CAP_NET_ADMIN` on a component that today runs +completely unprivileged), would fit one environment and break the other. -The namespace is asserted by an integration test (see *Testing*), so the -constraint is verified as infrastructure rather than trusted as code. +Confinement is therefore supplied by the deployment, in whatever way is native +to it: + +| Deployment | Mechanism | +| --- | --- | +| beta (Docker Compose) | a service attached only to a network that reaches the platform | +| mainstream (no Docker) | systemd `IPAddressDeny=any` + `IPAddressAllow=`, kernel-enforced via eBPF | + +Neither requires the prober to hold a capability, and neither is code we have to +keep correct. + +**The prober verifies its own confinement at startup.** Before doing any work it +attempts a *direct* connection — not through a tunnel — to a known geolocation +API address. If that connection succeeds, the confinement is not in place, and +the prober exits non-zero without probing anything. + +This is the part that makes the arrangement trustworthy across environments. The +operator's configuration is no longer an assumption the design rests on; it is a +runtime precondition the prober refuses to run without, and it holds identically +under Docker, systemd, or a future deployment that resembles neither. + +The Go-level fail-closed behaviour stays as defence in depth, so a refactor that +introduced a default `http.Client` would fail loudly rather than leak silently. ## Identity rotation @@ -294,9 +319,11 @@ project's probe population would also give an honest count. - **`probeverdict`**: table-driven over every rule, including RTT-impossible cases with real distances, and an explicit case asserting mmdb divergence does **not** produce `Suspect`. -- **The jail**: an integration test asserting the namespace has *no route* to a - geolocation IP — the constraint verified as infrastructure, not trusted as - code. Complemented by a tcpdump check in staging. +- **Confinement**: a test asserting the startup self-check fails closed — that + the prober exits non-zero when a direct connection to a geolocation address + succeeds, and proceeds when it is refused. This is the constraint verified as + a runtime precondition rather than trusted as configuration. Complemented by a + tcpdump check in staging. - **Multi-hop**: assert the target observes the intermediary's `source_id`, not the prober's. - **Pool guard**: assert that below `min_intermediary_pool` the probe runs direct @@ -480,8 +507,8 @@ measured bandwidth survive a provider restart. The second is a user-visible bug today. The third gates everything else — without it P1's results and the lifecycle's probation both evaporate on restart. -**P1 — Automated probing in a jail.** The `egressprober` binary, the network -namespace, and the taskworker job. Direct probing only, single fixed identity, +**P1 — Automated probing, confined.** The `egressprober` worker, its startup +self-check, the deployment confinement for both environments, and the taskworker job. Direct probing only, single fixed identity, no verdict logic — results land exactly as they do today. Delivers the automation and, critically, makes the hard constraint structural. diff --git a/model/network_client_location_model.go b/model/network_client_location_model.go index 3693df74..c0ff7316 100644 --- a/model/network_client_location_model.go +++ b/model/network_client_location_model.go @@ -1476,6 +1476,35 @@ func initialClientLocationsKey() string { return fmt.Sprintf("{cl}i") } +// returns the given ids with nils dropped and duplicates removed, preserving +// order. +// +// A client's city/region/country location ids are not guaranteed to differ: a +// country-only geo resolution stores the country id in all three columns and a +// region-only one stores it in two, because SetConnectionLocation falls back to +// the coarsest granularity available rather than writing a NULL into the NOT +// NULL city/region columns. Anything that fans a client out across the three +// columns has to treat them as a set, or the client is counted once per column +// instead of once per location it is in. +// +// Scope: that coarsest-granularity fallback is a beta change. upstream/main has +// no country-only fallback -- it passes the NULL through and the insert raises +// -- so on main today no row with city = region = country exists and this +// dedupe changes nothing there. It becomes load-bearing upstream when the +// fallback lands with PR #407, which carries its own copy of this helper. +func distinctIds(ids ...*server.Id) []server.Id { + distinct := make([]server.Id, 0, len(ids)) + for _, id := range ids { + if id == nil { + continue + } + if !slices.Contains(distinct, *id) { + distinct = append(distinct, *id) + } + } + return distinct +} + func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr error) { topCitiesPerRegion := 20 topCitiesPerCountry := 10 @@ -1519,8 +1548,37 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er WHERE network_client_location_reliability.connected = true AND - network_client_location_reliability.valid = true + network_client_location_reliability.valid = true AND + -- this is the number shown to everyone, so count only providers + -- a stranger can actually reach. GetProvideRelationship returns + -- ProvideModePublic for a cross-network pair, so a Public + -- provide key is what makes a provider generally reachable; + -- without one it advertises supply nobody outside its own + -- network can use. + -- + -- Note this is deliberately narrower than the candidate pool + -- UpdateClientScores builds. That pool also carries + -- ProvideModeNetwork providers, which are real usable supply + -- for sources inside their own network, and FindProviders2 + -- filters them per request against the caller's network. They + -- do not belong in a public count. + -- + -- Every other mode is excluded, not just Stream. In particular + -- resolveNonCompanionProvideMode + -- (controller/connect_controller.go) lets a Stream-only + -- destination be resolved as a *companion* stream, but that + -- dead-ends at CreateCompanionTransferEscrow, which requires a + -- pre-existing reverse-direction origin contract -- so a + -- Stream-only destination can never bootstrap a session and is + -- correctly absent from both the count and the pool. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) `, + ProvideModePublic, ) server.WithPgResult(result, err, func() { for result.Next() { @@ -1533,9 +1591,25 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er &countryLocationId, )) - locationClientCounts[cityLocationId] += 1 - locationClientCounts[regionLocationId] += 1 - locationClientCounts[countryLocationId] += 1 + // count each client at most once per distinct location id. A + // client whose geo lookup resolved neither a city nor a region + // is stored with city = region = country (see + // SetConnectionLocation's country fallback), so incrementing + // all three unconditionally counted that one client three + // times in its own country. Distinct ids -- a real + // city-granular client -- still roll up into their region and + // country exactly as before. + // + // This is a live fix on beta, where that fallback exists, and a + // forward guard against upstream/main, where it does not yet -- + // see distinctIds. + for _, locationId := range distinctIds( + &cityLocationId, + ®ionLocationId, + &countryLocationId, + ) { + locationClientCounts[locationId] += 1 + } } }) @@ -2194,6 +2268,16 @@ type ClientScore struct { HasLatencyTest bool HasSpeedTest bool + // true when the provider holds a ProvideModeNetwork provide key but no + // ProvideModePublic one, i.e. it can only settle a contract with a source + // in its own network. FindProviders2 keeps such a provider only for callers + // in NetworkId. It is stored negated on purpose: the score cache is gob + // encoded with a 5h ttl, so entries written before this field existed decode + // with the zero value, and the zero value has to mean "publicly usable" -- + // the pre-existing behaviour -- or every provider would be treated as + // network-only until the cache turned over. + NetworkOnly bool + LookbackIndex int LookbackClientScores map[int]*ClientScore @@ -2284,6 +2368,7 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r clientScore = &ClientScore{ ClientId: lookbackClientScore.ClientId, NetworkId: lookbackClientScore.NetworkId, + NetworkOnly: lookbackClientScore.NetworkOnly, LookbackClientScores: map[int]*ClientScore{}, } m[lookbackClientScore.ClientId] = clientScore @@ -2385,6 +2470,7 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r var lookbackIndex int var reliabilityWeight float64 var independentReliabilityWeight float64 + var publiclyUsable bool server.Raise(result.Scan( &cityLocationXId, ®ionLocationXId, @@ -2400,11 +2486,13 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r &lookbackIndex, &reliabilityWeight, &independentReliabilityWeight, + &publiclyUsable, )) lookbackClientScore = &ClientScore{ ClientId: clientId, LookbackIndex: lookbackIndex, NetworkId: networkId, + NetworkOnly: !publiclyUsable, ReliabilityWeight: reliabilityWeight, IndependentReliabilityWeight: independentReliabilityWeight, MinRelativeLatencyMillis: minRelativeLatencyMillis, @@ -2451,7 +2539,17 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r -- fix(beta): see the LEFT JOIN comment below COALESCE(client_connection_reliability_score.lookback_index, 0), COALESCE(client_connection_reliability_score.reliability_weight, 1), - COALESCE(client_connection_reliability_score.independent_reliability_weight, 1) + COALESCE(client_connection_reliability_score.independent_reliability_weight, 1), + -- publicly usable: holds a Public provide key, so a caller from + -- any network can settle a contract with it. A provider that is + -- in the pool without this is Network-only, and FindProviders2 + -- hands it out to callers in its own network only. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) FROM network_client_location_reliability @@ -2466,33 +2564,62 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r client_connection_reliability_score.client_id = network_client_location_reliability.client_id WHERE network_client_location_reliability.connected = true AND - network_client_location_reliability.valid = true + network_client_location_reliability.valid = true AND + -- the candidate pool, unlike the public count in + -- UpdateClientLocations, carries every provider that can settle + -- a contract with *someone*: Public for any caller, Network for + -- callers in the provider's own network. GetProvideRelationship + -- only ever returns one of those two, so this pair is exactly + -- the set CreateContract can accept. FindProviders2 then decides + -- eligibility per request -- it has to be done there and not + -- here, because the score cache is keyed by (forceMinimum, + -- rankMode, locationId, callerLocationId) and is not + -- network-scoped. + -- + -- Restricting this to Public would remove Network-only + -- providers from their own network's discovery, which works + -- today via CreateContractNoEscrow. Stream is still excluded: + -- resolveNonCompanionProvideMode can resolve a Stream-only + -- destination as a companion, but that dead-ends at + -- CreateCompanionTransferEscrow, which needs a pre-existing + -- reverse origin contract, so it can never bootstrap a session. + -- + -- GetProviderLocations gates on loadLocationStables, populated + -- from here; see exportClientScores for why the ClientFilter it + -- reads stays Public-only. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode IN ($1, $2) + ) `, + ProvideModePublic, + ProvideModeNetwork, ) server.WithPgResult(result, err, func() { for result.Next() { lookbackClientScore, cityLocationId, regionLocationId, countryLocationId := loadClientScore(result) - clientScores, ok := locationClientScores[*cityLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*cityLocationId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - - clientScores, ok = locationClientScores[*regionLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*regionLocationId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - - clientScores, ok = locationClientScores[*countryLocationId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationClientScores[*countryLocationId] = clientScores + // once per distinct location id: a country-only client stores + // its country id in all three columns (see + // SetConnectionLocation), and a client belongs in a location's + // pool once. The per-location map is keyed by client id so a + // repeat is already absorbed, but going through the set makes + // the intent explicit and keeps this loop in step with the + // counting loop in UpdateClientLocations. + for _, locationId := range distinctIds( + cityLocationId, + regionLocationId, + countryLocationId, + ) { + clientScores, ok := locationClientScores[locationId] + if !ok { + clientScores = map[server.Id]*ClientScore{} + locationClientScores[locationId] = clientScores + } + addClientScore(lookbackClientScore, clientScores) } - addClientScore(lookbackClientScore, clientScores) } }) @@ -2513,7 +2640,14 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r network_client_location_reliability.has_speed_test, COALESCE(client_connection_reliability_score.lookback_index, 0), -- fix(beta): see LEFT JOIN comment below COALESCE(client_connection_reliability_score.reliability_weight, 1), - COALESCE(client_connection_reliability_score.independent_reliability_weight, 1) + COALESCE(client_connection_reliability_score.independent_reliability_weight, 1), + -- publicly usable; see the per-location query above + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) FROM network_client_location_reliability @@ -2535,36 +2669,40 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r WHERE network_client_location_reliability.connected = true AND - network_client_location_reliability.valid = true + network_client_location_reliability.valid = true AND + -- same rule as the per-location query above: Public or + -- Network. This one fills locationGroupClientScores -> the + -- clientScoreLocationGroup* redis keys -> loadClientScores + -- -> FindProviders2 whenever a spec carries a + -- LocationGroupId, so a user who picks a promoted group + -- (e.g. "Strong Privacy Laws") must be filtered by the same + -- request-time network check as a plain location. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode IN ($1, $2) + ) `, + ProvideModePublic, + ProvideModeNetwork, ) server.WithPgResult(result, err, func() { for result.Next() { lookbackClientScore, cityLocationGroupId, regionLocationGroupId, countryLocationGroupId := loadClientScore(result) - if cityLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*cityLocationGroupId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*cityLocationGroupId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - } - - if regionLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*regionLocationGroupId] + // once per distinct group id. The three location columns can + // be the same id (a country-only client), in which case all + // three group joins resolve to the same membership rows. + for _, locationGroupId := range distinctIds( + cityLocationGroupId, + regionLocationGroupId, + countryLocationGroupId, + ) { + clientScores, ok := locationGroupClientScores[locationGroupId] if !ok { clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*regionLocationGroupId] = clientScores - } - addClientScore(lookbackClientScore, clientScores) - } - - if countryLocationGroupId != nil { - clientScores, ok := locationGroupClientScores[*countryLocationGroupId] - if !ok { - clientScores = map[server.Id]*ClientScore{} - locationGroupClientScores[*countryLocationGroupId] = clientScores + locationGroupClientScores[locationGroupId] = clientScores } addClientScore(lookbackClientScore, clientScores) } @@ -2675,17 +2813,29 @@ func UpdateClientScores(ctx context.Context, ttl time.Duration, parallel int) (r filter *ClientFilter, ) { clientScores := []*ClientScore{} - netReliabilityWeight := float64(0) + publicCount := 0 + publicNetReliabilityWeight := float64(0) for _, clientScore := range s { if clientScore.PassesMinimums[rankMode] || forceMinimum { - netReliabilityWeight += clientScore.ReliabilityWeight clientScores = append(clientScores, clientScore) + if !clientScore.NetworkOnly { + publicCount += 1 + publicNetReliabilityWeight += clientScore.ReliabilityWeight + } } } + // the samples above carry Network-only providers too -- FindProviders2 + // filters them per request against the caller's network -- but the + // ClientFilter does not. It is read only by loadLocationStables, which + // decides the `Stable` flag GetProviderLocations publishes to every + // user. That is a public surface, so like the provider count in + // UpdateClientLocations it counts only providers a stranger can reach: + // a location whose only supply is network-only is not stable, and with + // zero public providers it reports no providers at all. filter = &ClientFilter{ - Count: len(clientScores), - NetReliabilityWeight: netReliabilityWeight, + Count: publicCount, + NetReliabilityWeight: publicNetReliabilityWeight, } mathrand.Shuffle(len(clientScores), func(i int, j int) { @@ -3076,6 +3226,34 @@ func FindProviders2( glog.Infof("[nclm]findproviders2 load %.2fms (%d)\n", loadMillis, len(clientScores)) } + // drop providers this caller cannot contract with. + // + // UpdateClientScores puts both Public and Network-only providers in the + // pool. A Network-only provider can only settle a contract with a + // source in its own network (GetProvideRelationship -> + // ProvideModeNetwork -> CreateContractNoEscrow); it is real, usable + // supply for those users and has always been discoverable to them, so + // it stays. For anyone else CreateContract would reject with + // NoPermission, so it goes. + // + // This must happen here, after loadClientScores, and must never be + // baked into the cached set: the client score redis entries are keyed + // by (forceMinimum, rankMode, locationId, callerLocationId) with no + // network component, so a single cached set is shared by callers from + // every network. + // + // A session with no jwt yields the zero network id, which matches no + // provider -- fail closed rather than leak. + var callerNetworkId server.Id + if session.ByJwt != nil { + callerNetworkId = session.ByJwt.NetworkId + } + for clientId, clientScore := range clientScores { + if clientScore.NetworkOnly && clientScore.NetworkId != callerNetworkId { + delete(clientScores, clientId) + } + } + for clientId, _ := range excludeFinalDestinations() { delete(clientScores, clientId) } diff --git a/model/network_client_location_model_test.go b/model/network_client_location_model_test.go index 3dcc5b26..1d644399 100644 --- a/model/network_client_location_model_test.go +++ b/model/network_client_location_model_test.go @@ -1233,6 +1233,14 @@ func TestUpdateClientLocationsCountsClientsWithoutReliabilityScores(t *testing.T err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) connect.AssertEqual(t, err, nil) + // only clients holding a Public provide key are counted (see + // TestUpdateClientLocationsCountsOnlyPublicProviders); the + // reliability-score join is what is under test here, so satisfy that + // precondition explicitly + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + }) + // populates network_client_location_reliability straight from the // live connection tables -- independent of, and deliberately without // ever touching, the reliability-scoring pipeline below @@ -1326,6 +1334,14 @@ func TestUpdateClientScoresCountsClientsWithoutReliabilityScores(t *testing.T) { err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) connect.AssertEqual(t, err, nil) + // only clients holding a Public provide key are counted (see + // TestUpdateClientLocationsCountsOnlyPublicProviders); the + // reliability-score join is what is under test here, so satisfy that + // precondition explicitly + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + }) + // good latency and speed tests so the quality score gate passes and // the reliability-score join is what's actually being tested here server.Tx(ctx, func(tx server.PgTx) { @@ -1425,3 +1441,910 @@ func TestSetConnectionLocationToleratesCountryOnlyLocation(t *testing.T) { connect.AssertEqual(t, *cty, country.CountryLocationId) }) } + +func TestUpdateClientLocationsCountsOnlyPublicProviders(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + handlerId := CreateNetworkClientHandler(ctx) + + // connect a client and give it the provide modes supplied + connectOne := func(modes map[ProvideMode][]byte) server.Id { + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, "0.0.0.1:0", handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + if modes != nil { + SetProvide(ctx, clientId, modes) + } + return clientId + } + + // serves strangers -- must be counted + connectOne(map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + // own network only -- must NOT be counted, it cannot accept a + // contract from a user outside its network + connectOne(map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + // no provide key at all -- must NOT be counted + connectOne(nil) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + err := UpdateClientLocations(ctx, time.Hour) + connect.AssertEqual(t, err, nil) + + initialClientLocations, err := loadInitialClientLocations(ctx) + connect.AssertEqual(t, err, nil) + if initialClientLocations == nil { + t.Fatal("expected a populated client locations cache, got nil") + } + + found := false + for _, clientLocation := range initialClientLocations.Locations { + if clientLocation.LocationId == city.CountryLocationId { + found = true + // exactly one of the three connected clients holds a Public + // key; counting the other two advertises supply no user can + // reach (39 advertised vs 2 reachable, observed on beta) + connect.AssertEqual(t, clientLocation.ClientCount, 1) + } + } + connect.AssertEqual(t, found, true) + }) +} + +// `UpdateClientScores` writes two things, and the provide-mode rule differs +// between them. +// +// The `ClientFilter` -- read by `loadLocationStables`, which +// `GetProviderLocations` gates the public `Stable` flag on -- counts only +// providers a stranger can reach, the same rule as the provider count in +// `UpdateClientLocations`. A location whose only supply is network-only is not +// stable and reports no providers at all. +// +// The sampled candidate pool is wider: it also carries `ProvideModeNetwork` +// providers, which are real usable supply for sources in their own network. +// `FindProviders2` filters those per request (see +// TestFindProviders2NetworkOnlyProviderVisibleOnlyToItsOwnNetwork); the pool +// itself is shared by every caller and cannot make that decision. +// +// Two connected+valid providers with identical latency/speed data in two +// different countries, differing ONLY in whether they hold a Public provide +// key. +func TestUpdateClientScoresCountsOnlyPublicProviders(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + publicCity := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, publicCity) + + networkOnlyCity := &Location{ + LocationType: LocationTypeCity, + City: "Toronto", + Region: "Ontario", + Country: "Canada", + CountryCode: "ca", + } + CreateLocation(ctx, networkOnlyCity) + + publicClientId, networkOnlyClientId := connectPublicAndNetworkOnlyProviders( + ctx, + t, + publicCity, + networkOnlyCity, + ) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + locationStables, err := loadLocationStables( + ctx, + []server.Id{publicCity.CountryLocationId, networkOnlyCity.CountryLocationId}, + RankModeQuality, + server.Id{}, + ) + connect.AssertEqual(t, err, nil) + + // the Public provider's country has a provider a stranger can use + _, ok := locationStables[publicCity.CountryLocationId] + connect.AssertEqual(t, ok, true) + // the network-only provider's country has none. a missing entry is how + // loadLocationStables says "no providers", so it must be absent + _, ok = locationStables[networkOnlyCity.CountryLocationId] + connect.AssertEqual(t, ok, false) + + // the pool FindProviders2 consumes carries both, each tagged with + // whether a caller outside its network may use it + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{ + publicCity.LocationId: true, + networkOnlyCity.LocationId: true, + }, + map[server.Id]bool{}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, len(clientScores), 2) + publicClientScore, ok := clientScores[publicClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, publicClientScore.NetworkOnly, false) + networkOnlyClientScore, ok := clientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) + }) +} + +// the location *group* source query in `UpdateClientScores` follows the same +// Public-or-Network rule as the per-location one. It fills +// `locationGroupClientScores` -> the `clientScoreLocationGroup*` redis keys -> +// `loadClientScores` -> `FindProviders2` whenever the spec carries a +// `LocationGroupId`, so a user who selects a promoted group (e.g. "Strong +// Privacy Laws") has to be filtered by the same request-time network check as +// a user who selects a plain location -- and the group cache has to carry the +// flag that check reads. +// +// Dropping either `provide_mode` from the group query's `EXISTS` breaks this: +// without Network the group's own-network supply disappears, and without +// Public nothing is reachable at all. Dropping the `EXISTS` entirely would let +// in modes `CreateContract` can never accept. +// +// Same two providers, differing only in the Public provide key, each in its +// own location group. +func TestUpdateClientScoresGroupCarriesNetworkProvidersTagged(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + publicCity := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, publicCity) + + networkOnlyCity := &Location{ + LocationType: LocationTypeCity, + City: "Toronto", + Region: "Ontario", + Country: "Canada", + CountryCode: "ca", + } + CreateLocation(ctx, networkOnlyCity) + + publicGroup := &LocationGroup{ + Name: "Test Group Public", + Promoted: true, + MemberLocationIds: []server.Id{ + publicCity.CityLocationId, + publicCity.RegionLocationId, + publicCity.CountryLocationId, + }, + } + CreateLocationGroup(ctx, publicGroup) + + networkOnlyGroup := &LocationGroup{ + Name: "Test Group Network Only", + Promoted: true, + MemberLocationIds: []server.Id{ + networkOnlyCity.CityLocationId, + networkOnlyCity.RegionLocationId, + networkOnlyCity.CountryLocationId, + }, + } + CreateLocationGroup(ctx, networkOnlyGroup) + + publicClientId, networkOnlyClientId := connectPublicAndNetworkOnlyProviders( + ctx, + t, + publicCity, + networkOnlyCity, + ) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + // read the group cache only -- no location ids -- so this asserts on + // the group query alone + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{}, + map[server.Id]bool{ + publicGroup.LocationGroupId: true, + networkOnlyGroup.LocationGroupId: true, + }, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, len(clientScores), 2) + publicClientScore, ok := clientScores[publicClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, publicClientScore.NetworkOnly, false) + networkOnlyClientScore, ok := clientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) + + // the network-only group on its own exports exactly its one provider, + // tagged network-only so FindProviders2 hands it to that provider's + // own network and nobody else + networkOnlyGroupClientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{}, + map[server.Id]bool{networkOnlyGroup.LocationGroupId: true}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + connect.AssertEqual(t, len(networkOnlyGroupClientScores), 1) + networkOnlyClientScore, ok = networkOnlyGroupClientScores[networkOnlyClientId] + connect.AssertEqual(t, ok, true) + connect.AssertEqual(t, networkOnlyClientScore.NetworkOnly, true) + }) +} + +// connects one provider per location that is identical in every respect the +// scoring pipeline looks at -- connected, valid, same latency and speed +// samples so both clear the strict minimums -- except that the first holds a +// Public provide key and the second holds only a Network one. Anything that +// separates the two downstream is the provide-mode filter and nothing else. +func connectPublicAndNetworkOnlyProviders( + ctx context.Context, + t testing.TB, + publicLocation *Location, + networkOnlyLocation *Location, +) (publicClientId server.Id, networkOnlyClientId server.Id) { + handlerId := CreateNetworkClientHandler(ctx) + + connectProvider := func(location *Location, modes map[ProvideMode][]byte) server.Id { + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, "0.0.0.1:0", handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, location.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, modes) + + // good latency and speed samples so the client passes the strict + // minimums; loadLocationStables reads the force-minimum-false filter + // key, which is empty for a client that fails them + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + `INSERT INTO network_client_latency (connection_id, latency_ms, sample_count) VALUES ($1, $2, $3)`, + connectionId, 30, 1, + )) + server.RaisePgResult(tx.Exec( + ctx, + `INSERT INTO network_client_speed (connection_id, bytes_per_second, sample_count) VALUES ($1, $2, $3)`, + connectionId, 100*1024*1024, 1, + )) + }) + return clientId + } + + // serves strangers + publicClientId = connectProvider(publicLocation, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + // own network only -- cannot accept a contract from a user outside it + networkOnlyClientId = connectProvider(networkOnlyLocation, map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + return +} + +// A client whose geo lookup resolved no city and no region is stored with +// city_location_id = region_location_id = country_location_id -- the coarsest +// granularity available is written into all three columns. Both fan-out loops +// then walked city, region and country unconditionally, so one such client +// added 3 to its own country: `/network/provider-locations` advertised three +// times the supply that exists, and the inflation is worst exactly where geo +// resolution is coarsest (datacenter, mobile and VPN egress). +// +// That inflation is real on beta, which has the coarsest-granularity fallback +// in `SetConnectionLocation`. It is NOT reachable on `upstream/main`, which has +// no country-only fallback and raises instead, so against main this is a +// forward guard; the fallback arrives with PR #407. The fixture builds the +// collapsed row with a direct UPDATE for exactly that reason -- it does not +// depend on which branch's write path could produce it. +// +// The second half of this test is the guard on the fix: a genuinely +// city-granular client must still roll up into its region and its country. A +// dedupe keyed on the client alone rather than on (client, location) would +// silently stop city clients counting toward their country, which is a far +// worse regression than the one being fixed. +func TestUpdateClientLocationsCountsEachClientOncePerLocation(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + countryOnlyCity, city, _, _ := createCountryOnlyAndCityProviders(ctx, t) + + err := UpdateClientLocations(ctx, time.Hour) + connect.AssertEqual(t, err, nil) + + clientLocations, err := loadClientLocations(ctx, map[server.Id]bool{ + countryOnlyCity.CountryLocationId: true, + city.CityLocationId: true, + city.RegionLocationId: true, + city.CountryLocationId: true, + }) + connect.AssertEqual(t, err, nil) + + countOf := func(locationId server.Id) int { + clientLocation, ok := clientLocations[locationId] + if !ok { + t.Fatalf("expected a cached client location for %s", locationId) + } + return clientLocation.ClientCount + } + + // one client, counted once -- not once per city/region/country column + connect.AssertEqual(t, countOf(countryOnlyCity.CountryLocationId), 1) + + // and the roll-up for a real city client is untouched + connect.AssertEqual(t, countOf(city.CityLocationId), 1) + connect.AssertEqual(t, countOf(city.RegionLocationId), 1) + connect.AssertEqual(t, countOf(city.CountryLocationId), 1) + }) +} + +// The `UpdateClientScores` half of the fan-out. This deliberately does NOT +// assert "counted once": the per-location and per-group accumulators are maps +// keyed by client id, so a repeated client is absorbed whether or not the loop +// dedupes, and an earlier version of this test asserting a pool size of 1 for +// the country-only provider returned 1 both before and after the dedupe. It +// could not fail, so it was not testing anything. +// +// What the code here CAN get wrong is the shape of the dedupe. Keying it on the +// client rather than on (client, location) -- the tempting simplification, since +// the map already absorbs repeats -- would stop a city-granular provider +// appearing in its region's and its country's pools, silently emptying every +// coarse-grained search. So the assertions are on membership at each +// granularity, by client id rather than by count, which also catches a provider +// leaking into another country's pool. +func TestUpdateClientScoresRollsCityProviderUpToRegionAndCountry(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + countryOnlyCity, city, countryOnlyClientId, cityClientId := createCountryOnlyAndCityProviders(ctx, t) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + poolAt := func(locationId server.Id) map[server.Id]*ClientScore { + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{locationId: true}, + map[server.Id]bool{}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + return clientScores + } + assertPoolIs := func(label string, locationId server.Id, wantClientId server.Id) { + clientScores := poolAt(locationId) + if _, ok := clientScores[wantClientId]; !ok { + t.Errorf("%s pool is missing provider %s", label, wantClientId) + } + if len(clientScores) != 1 { + t.Errorf("%s pool holds %d clients, want only %s", label, len(clientScores), wantClientId) + } + } + + // the city provider must reach every granularity above it + assertPoolIs("city", city.CityLocationId, cityClientId) + assertPoolIs("region", city.RegionLocationId, cityClientId) + assertPoolIs("country", city.CountryLocationId, cityClientId) + + // and the country-only provider stays in its own country, not the + // city provider's + assertPoolIs("country-only country", countryOnlyCity.CountryLocationId, countryOnlyClientId) + }) +} + +// connects two Public providers in two different countries and rolls the +// reliability table forward: +// +// - one whose `network_client_location` row has all three location columns +// collapsed to its country id, which is what a country-granularity geo +// lookup produces (`SetConnectionLocation` writes the coarsest available id +// into the NOT NULL city/region columns), and +// - one at genuine city granularity, with three distinct ids. +// +// The collapse is applied with a direct UPDATE rather than by handing +// `SetConnectionLocation` a country-only `location` row, so the fixture depends +// only on the shape of the stored row and not on which branch's fallback +// produced it. +func createCountryOnlyAndCityProviders(ctx context.Context, t testing.TB) ( + countryOnlyCity *Location, + city *Location, + countryOnlyClientId server.Id, + cityClientId server.Id, +) { + countryOnlyCity = &Location{ + LocationType: LocationTypeCity, + City: "Toronto", + Region: "Ontario", + Country: "Canada", + CountryCode: "ca", + } + CreateLocation(ctx, countryOnlyCity) + + city = &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + handlerId := CreateNetworkClientHandler(ctx) + connectPublicProvider := func(location *Location, ip string) server.Id { + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, ip, handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, location.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + }) + + // UpdateClientScores joins client_connection_reliability_score, so + // every provider needs a score row or the join, not the logic under + // test, is what excludes it. The address hash is shared across the + // fixture on purpose: validity is derived from + // network_client_connection, not from these stats. + AddClientReliabilityStats( + ctx, + networkId, + clientId, + [32]byte{}, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + return clientId + } + + countryOnlyClientId = connectPublicProvider(countryOnlyCity, "0.0.0.1:0") + cityClientId = connectPublicProvider(city, "0.0.0.2:0") + + // collapse the first provider to country granularity + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + ` + UPDATE network_client_location + SET + city_location_id = $2, + region_location_id = $2 + WHERE client_id = $1 + `, + countryOnlyClientId, + countryOnlyCity.CountryLocationId, + )) + }) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + return +} + +// `UpdateClientLocations` counts only providers holding a Public provide key, +// because that count is shown to everyone and a stranger genuinely cannot use a +// `Network`-only provider. The candidate pool is a different question: a +// `Network`-only provider serves sources inside its own network today (via +// `CreateContractNoEscrow`) and is discoverable to them. Restricting the pool +// to Public would make those providers invisible to the very users they exist +// for -- a live regression for anyone running providers for their own +// organisation. +// +// So the pool carries both, and eligibility is decided per request against the +// caller's network. All four cases: +// +// same-network caller other-network caller +// Public provider returned returned +// Network-only provider returned NOT returned +// +// This has to be a request-time filter, not a cache-time one: the client-score +// redis entries are keyed by (forceMinimum, rankMode, locationId, +// callerLocationId) and are not network-scoped, so one cached set is shared by +// callers from every network. +func TestFindProviders2NetworkOnlyProviderVisibleOnlyToItsOwnNetwork(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + handlerId := CreateNetworkClientHandler(ctx) + + connectProvider := func(networkId server.Id, ip string, modes map[ProvideMode][]byte) server.Id { + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, _, err := ConnectNetworkClient(ctx, clientId, ip, handlerId) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, city.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + SetProvide(ctx, clientId, modes) + + // upstream joins client_connection_reliability_score with an INNER + // JOIN, so a provider with no reliability row never reaches the + // pool at all. Give every provider one so this test asserts the + // provide-mode rule and nothing else. + AddClientReliabilityStats( + ctx, + networkId, + clientId, + [32]byte{}, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + return clientId + } + + publicNetworkId := server.NewId() + publicClientId := connectProvider(publicNetworkId, "0.0.0.1:0", map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + + networkOnlyNetworkId := server.NewId() + networkOnlyClientId := connectProvider(networkOnlyNetworkId, "0.0.0.2:0", map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + findFrom := func(networkId server.Id, name string) map[server.Id]bool { + clientSession := session.Testing_CreateClientSession( + ctx, + jwt.NewByJwt(networkId, server.NewId(), name, false, false), + ) + res, err := FindProviders2( + &FindProviders2Args{ + Specs: []*ProviderSpec{ + {LocationId: &city.LocationId}, + }, + Count: 100, + ForceMinimum: true, + }, + clientSession, + ) + connect.AssertEqual(t, err, nil) + found := map[server.Id]bool{} + for _, provider := range res.Providers { + found[provider.ClientId] = true + } + return found + } + + // the network-only provider's own network sees both + sameNetwork := findFrom(networkOnlyNetworkId, "same-network") + connect.AssertEqual(t, sameNetwork[publicClientId], true) + connect.AssertEqual(t, sameNetwork[networkOnlyClientId], true) + + // a stranger sees only the Public one. handing them the network-only + // provider would produce a `CreateContract` NoPermission rejection. + otherNetwork := findFrom(server.NewId(), "other-network") + connect.AssertEqual(t, otherNetwork[publicClientId], true) + connect.AssertEqual(t, otherNetwork[networkOnlyClientId], false) + }) +} + +// The pool predicate in `UpdateClientScores` is the whole reason this work +// exists, and until now nothing failed when it was deleted. +// +// Both source queries carry `EXISTS (... provide_mode IN (Public, Network))`. +// Deleting either one refills the candidate pool with the entire consumer +// fleet, because **every** client registers `ProvideMode_Stream` on connect +// (`connect/transfer_contract_manager.go`) whether or not it provides anything. +// Those clients cannot settle a contract -- `resolveNonCompanionProvideMode` +// can resolve a Stream-only destination as a companion, but that dead-ends at +// `CreateCompanionTransferEscrow`, which needs a pre-existing reverse origin +// contract and so can never bootstrap a session. That is the +// 39-providers-advertised-against-2-usable failure this filter fixes. +// +// The existing provide-mode tests do not catch a deletion: their fixtures are +// Public and Network-only, both of which the predicate admits, so removing it +// changes nothing they look at. These build the populations that would flood +// back in -- a Stream-only client and a keyless one -- and assert the pool +// still holds exactly the provider that can serve a contract. +// +// connectProvidersOfEveryProvideMode puts four providers in one location: +// Public, Network-only, Stream-only, and keyless. The first two belong in the +// pool (Network-only is filtered per request by `FindProviders2`); the last two +// never do. +func connectProvidersOfEveryProvideMode(ctx context.Context, t testing.TB, location *Location) ( + publicClientId server.Id, + networkOnlyClientId server.Id, + streamOnlyClientId server.Id, + keylessClientId server.Id, +) { + handlerId := CreateNetworkClientHandler(ctx) + + connectProvider := func(i int, modes map[ProvideMode][]byte) server.Id { + networkId := server.NewId() + clientId := server.NewId() + Testing_CreateDevice(ctx, networkId, server.NewId(), clientId, "", "") + connectionId, _, _, clientAddressHash, err := ConnectNetworkClient( + ctx, + clientId, + fmt.Sprintf("0.0.0.%d:0", i), + handlerId, + ) + connect.AssertEqual(t, err, nil) + err = SetConnectionLocation(ctx, connectionId, location.LocationId, &ConnectionLocationScores{}) + connect.AssertEqual(t, err, nil) + if modes != nil { + SetProvide(ctx, clientId, modes) + } + + // a reliability score row for every client. The per-location query + // joins client_connection_reliability_score, so without one the join -- + // not the provide-mode predicate under test -- is what decides who is + // in the pool. + AddClientReliabilityStats( + ctx, + networkId, + clientId, + clientAddressHash, + server.NowUtc(), + &ClientReliabilityStats{ + ConnectionEstablishedCount: 1, + ProvideEnabledCount: 1, + ReceiveMessageCount: 1, + ReceiveByteCount: 1024, + SendMessageCount: 1, + SendByteCount: 1024, + }, + ) + + // identical latency and speed samples for every client, so all four + // clear the strict minimums and the provide mode is the only thing + // that can separate them + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + `INSERT INTO network_client_latency (connection_id, latency_ms, sample_count) VALUES ($1, $2, $3)`, + connectionId, 30, 1, + )) + server.RaisePgResult(tx.Exec( + ctx, + `INSERT INTO network_client_speed (connection_id, bytes_per_second, sample_count) VALUES ($1, $2, $3)`, + connectionId, 100*1024*1024, 1, + )) + }) + return clientId + } + + publicClientId = connectProvider(1, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + networkOnlyClientId = connectProvider(2, map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + // the population the deleted predicate would readmit: every consumer + // client registers Stream on connect, and Stream alone can never settle a + // contract + streamOnlyClientId = connectProvider(3, map[ProvideMode][]byte{ + ProvideModeStream: []byte("stream-secret"), + }) + keylessClientId = connectProvider(4, nil) + + UpdateClientLocationReliabilities(ctx, server.NowUtc().Add(-time.Hour), server.NowUtc()) + UpdateClientReliabilityScores(ctx, server.NowUtc(), true) + return +} + +// The per-location source query. Fails against a deletion of its +// `WHERE EXISTS`, which is what leaves the pool full of Stream-only consumers. +func TestUpdateClientScoresPoolExcludesStreamOnlyAndKeylessProviders(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + publicClientId, networkOnlyClientId, streamOnlyClientId, keylessClientId := + connectProvidersOfEveryProvideMode(ctx, t, city) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{city.LocationId: true}, + map[server.Id]bool{}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + + assertPoolAdmitsOnlyContractableProviders( + t, + clientScores, + publicClientId, + networkOnlyClientId, + streamOnlyClientId, + keylessClientId, + ) + }) +} + +// The location-*group* source query, which is a separate statement with its own +// copy of the predicate and its own redis keys +// (`clientScoreLocationGroup*` -> `loadClientScores` -> `FindProviders2` +// whenever the spec carries a `LocationGroupId`). A user who picks a promoted +// group must get the same pool a user who picks a plain location gets; deleting +// this query's `WHERE EXISTS` alone leaves the per-location test above green. +// +// Only group ids are passed to `loadClientScores`, so this reads the group +// cache and nothing else. +func TestUpdateClientScoresGroupPoolExcludesStreamOnlyAndKeylessProviders(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + city := &Location{ + LocationType: LocationTypeCity, + City: "Palo Alto", + Region: "California", + Country: "United States", + CountryCode: "us", + } + CreateLocation(ctx, city) + + group := &LocationGroup{ + Name: "Test Group Provide Modes", + Promoted: true, + MemberLocationIds: []server.Id{ + city.CityLocationId, + city.RegionLocationId, + city.CountryLocationId, + }, + } + CreateLocationGroup(ctx, group) + + publicClientId, networkOnlyClientId, streamOnlyClientId, keylessClientId := + connectProvidersOfEveryProvideMode(ctx, t, city) + + err := UpdateClientScores(ctx, time.Hour, 1) + connect.AssertEqual(t, err, nil) + + clientScores, err := loadClientScores( + true, + RankModeQuality, + ctx, + map[server.Id]bool{}, + map[server.Id]bool{group.LocationGroupId: true}, + server.Id{}, + 100, + ) + connect.AssertEqual(t, err, nil) + + assertPoolAdmitsOnlyContractableProviders( + t, + clientScores, + publicClientId, + networkOnlyClientId, + streamOnlyClientId, + keylessClientId, + ) + }) +} + +// Errorf rather than Fatalf throughout, so one run reports every provider the +// pool got wrong instead of stopping at the first. +func assertPoolAdmitsOnlyContractableProviders( + t testing.TB, + clientScores map[server.Id]*ClientScore, + publicClientId server.Id, + networkOnlyClientId server.Id, + streamOnlyClientId server.Id, + keylessClientId server.Id, +) { + if _, ok := clientScores[streamOnlyClientId]; ok { + t.Errorf( + "Stream-only client %s is in the candidate pool; every consumer client registers ProvideMode_Stream, so this is the whole consumer fleet advertised as supply", + streamOnlyClientId, + ) + } + if _, ok := clientScores[keylessClientId]; ok { + t.Errorf("client %s holds no provide key at all and is in the candidate pool", keylessClientId) + } + + // and the providers that CAN settle a contract are still there -- a filter + // that excluded them would "pass" the assertions above for the wrong reason + publicClientScore, ok := clientScores[publicClientId] + if !ok { + t.Errorf("Public client %s is missing from the candidate pool", publicClientId) + } else if publicClientScore.NetworkOnly { + t.Errorf("Public client %s is tagged network-only", publicClientId) + } + networkOnlyClientScore, ok := clientScores[networkOnlyClientId] + if !ok { + t.Errorf("Network-only client %s is missing from the candidate pool; it serves its own network today", networkOnlyClientId) + } else if !networkOnlyClientScore.NetworkOnly { + t.Errorf("Network-only client %s is not tagged network-only, so FindProviders2 would hand it to strangers", networkOnlyClientId) + } + + if len(clientScores) != 2 { + t.Errorf("candidate pool holds %d clients, want exactly the 2 that can settle a contract", len(clientScores)) + } +} diff --git a/model/network_stats_model.go b/model/network_stats_model.go index 2df1a885..95a85fe3 100644 --- a/model/network_stats_model.go +++ b/model/network_stats_model.go @@ -71,9 +71,19 @@ func GetBlockUsersSnapshot(ctx context.Context, block int) (int64, bool) { } // CountProviderCountries returns the number of countries with a connected, -// valid provider — the same population the public providers map draws from. +// valid provider holding a Public provide key — the same population the public +// providers map and /network/provider-locations draw from. // The (connected, valid, country_location_id) index covers the scan; joining // location dedupes any non-canonical country location rows by country code. +// +// The Public-only predicate is the same one UpdateClientLocations applies +// (network_client_location_model.go). This is a public stat, so it must answer +// the same question the public provider list does: how many countries a +// stranger can actually pick a provider in. GetProvideRelationship returns +// ProvideModePublic for a cross-network pair, so only a Public provide key +// makes a provider generally reachable — a ProvideModeNetwork provider is +// usable only inside its own network and is effectively private. Counting +// those here would report countries with no pickable supply at all. func CountProviderCountries(ctx context.Context) int64 { var count int64 server.ReplicaDb(ctx, func(conn server.PgConn) { @@ -86,8 +96,15 @@ func CountProviderCountries(ctx context.Context) int64 { location.location_id = network_client_location_reliability.country_location_id WHERE network_client_location_reliability.connected = true AND - network_client_location_reliability.valid = true + network_client_location_reliability.valid = true AND + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) `, + ProvideModePublic, ) server.WithPgResult(result, err, func() { if result.Next() { diff --git a/model/providers_map_model.go b/model/providers_map_model.go index 9e6d2800..f54af623 100644 --- a/model/providers_map_model.go +++ b/model/providers_map_model.go @@ -125,10 +125,27 @@ func GetProvidersMap(ctx context.Context) (map[string]map[string]*RegionProvider WHERE network_client_location_reliability.connected = true AND - network_client_location_reliability.valid = true + network_client_location_reliability.valid = true AND + -- same Public-only rule as UpdateClientLocations + -- (network_client_location_model.go): this map is public, + -- so it must count only providers a stranger can actually + -- reach. GetProvideRelationship returns ProvideModePublic + -- for a cross-network pair, so a Public provide key is + -- what makes a provider generally reachable; a + -- ProvideModeNetwork provider is real supply only to its + -- own network and is effectively private. Without this the + -- map reports more providers than /network/provider-locations + -- will let anyone pick from. + EXISTS ( + SELECT 1 FROM provide_key + WHERE + provide_key.client_id = network_client_location_reliability.client_id AND + provide_key.provide_mode = $1 + ) GROUP BY location.country_code, location.location_name `, + ProvideModePublic, ) server.WithPgResult(result, err, func() { for result.Next() { diff --git a/model/providers_map_model_test.go b/model/providers_map_model_test.go index 84f2eb76..b38feda4 100644 --- a/model/providers_map_model_test.go +++ b/model/providers_map_model_test.go @@ -155,6 +155,12 @@ func TestGetProvidersMapAggregatesRegions(t *testing.T) { )) } }) + // the map counts only providers holding a Public provide key (see + // TestProviderStatsCountOnlyPublicProviders); every provider this + // helper builds is meant to count, so give them all one + SetProvide(ctx, clientId, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + }) } addProvider(nsw, true, true) @@ -184,3 +190,129 @@ func TestGetProvidersMapAggregatesRegions(t *testing.T) { connect.AssertEqual(t, true, au["New South Wales"].Lon > 140) }) } + +// Both public provider numbers must answer the same question the public +// provider *list* answers: how much supply a stranger can actually pick. +// +// `/network/provider-locations` (UpdateClientLocations) counts only providers +// holding a Public provide key -- GetProvideRelationship returns +// ProvideModePublic for a cross-network pair, so without a Public key a +// provider can only ever serve its own network and is effectively private. +// `/stats/providers-map` and CountProviderCountries applied no provide-mode +// filter at all, so they reported supply that no user outside the provider's +// own network could select: the map and the country count would both exceed +// the list, in the same deployment, for the same population. +// +// Deleting either EXISTS predicate must fail this test. +func TestProviderStatsCountOnlyPublicProviders(t *testing.T) { + server.DefaultTestEnv().Run(t, func(t testing.TB) { + ctx := context.Background() + + // two countries so the country count distinguishes them: the Public + // provider is the only one in Australia, the Network-only provider the + // only one in Japan. A correct count is 1 country, not 2. + nsw := &Location{ + LocationType: LocationTypeRegion, + Region: "New South Wales", + Country: "Australia", + CountryCode: "au", + } + CreateLocation(ctx, nsw) + tokyo := &Location{ + LocationType: LocationTypeRegion, + Region: "Tokyo", + Country: "Japan", + CountryCode: "jp", + } + CreateLocation(ctx, tokyo) + + // a connected, valid, scored provider in `region` holding exactly the + // provide modes given (nil for none at all) + addProvider := func(region *Location, modes map[ProvideMode][]byte) { + clientId := server.NewId() + networkId := server.NewId() + server.Tx(ctx, func(tx server.PgTx) { + server.RaisePgResult(tx.Exec( + ctx, + ` + INSERT INTO network_client_location_reliability ( + client_id, + network_id, + update_block_number, + region_location_id, + country_location_id, + client_address_hash_count, + location_count, + connected + ) + VALUES ($1, $2, 1, $3, $4, 1, 1, true) + `, + clientId, + networkId, + region.LocationId, + region.CountryLocationId, + )) + server.RaisePgResult(tx.Exec( + ctx, + ` + INSERT INTO client_connection_reliability_score ( + client_id, + lookback_index, + independent_reliability_score, + independent_reliability_weight, + reliability_score, + reliability_weight, + min_block_number, + max_block_number, + region_location_id, + country_location_id + ) + VALUES ($1, 0, 1, 1, 1, 1, 1, 1, $2, $3) + `, + clientId, + region.LocationId, + region.CountryLocationId, + )) + }) + if modes != nil { + SetProvide(ctx, clientId, modes) + } + } + + // serves strangers -- must be counted by both surfaces + addProvider(nsw, map[ProvideMode][]byte{ + ProvideModePublic: []byte("public-secret"), + ProvideModeNetwork: []byte("network-secret"), + }) + // own network only -- must NOT be counted by either surface + addProvider(tokyo, map[ProvideMode][]byte{ + ProvideModeNetwork: []byte("network-secret"), + }) + // no provide key at all -- must NOT be counted by either surface + addProvider(tokyo, nil) + + // surface 1: /stats/providers-map + providersMap, err := GetProvidersMap(ctx) + connect.AssertEqual(t, err, nil) + + au := providersMap["au"] + connect.AssertNotEqual(t, au, nil) + connect.AssertEqual(t, au["New South Wales"].ProviderCount, 1) + + // Japan's only supply is network-only/keyless, so it must not appear on + // the public map at all -- not as a region with a zero count, and not + // as a country + // Errorf, not Fatalf: the country count below is the *other* surface + // this test covers, and a run that stopped here would never exercise + // it -- both predicates need to be seen failing independently. + if jp, found := providersMap["jp"]; found { + t.Errorf("providers map contains jp = %v; its only providers are network-only/keyless and unreachable to any user outside their own network", jp) + } + + // surface 2: the /stats country count + countries := CountProviderCountries(ctx) + if countries != 1 { + t.Fatalf("CountProviderCountries() = %d, want 1 (only au has a Public provider; jp's are network-only/keyless)", countries) + } + }) +}