diff --git a/README.md b/README.md index e6b054aa1..6069ba0ff 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,7 @@ All install paths provide `subrouter`, `sr`, and `cx`. The npm and Python wrappe ### Local macOS daemon -On macOS, install Subrouter as a localhost-only LaunchAgent: +On macOS, install Subrouter as a loopback-only LaunchAgent: ```bash make build @@ -136,6 +136,8 @@ Transcript recording is off by default. Enable it explicitly with `subrouter ins The 10 minute `sr` auto-switch interval is the default. Override it with `subrouter install-daemon --sr-switch-interval 5m`, or disable it with `--sr-switch-interval 0`. The old `--cx-switch-interval` flag remains a compatibility alias. +To serve other machines from this Mac instead, pass a tailnet address and an admin token — `install-daemon` refuses a non-loopback bind without one. See [docs/tailscale.md](docs/tailscale.md). + ### Linux systemd service On a Linux server, install the binary and service: @@ -183,6 +185,8 @@ sudo sr install-systemd --addr 0.0.0.0:31415 --admin-token "$TOKEN" sr server add team --url http://100.64.0.1:31415 --admin-token "$TOKEN" --default ``` +The macOS equivalent is `install-daemon --addr :31415 --admin-token-file `; [docs/tailscale.md](docs/tailscale.md) covers that topology, including why tailnet membership is not itself authorization. + When `SUBROUTER_ADMIN_TOKEN` or `--admin-token` is set, non-loopback requests to sensitive `/_subrouter/*` endpoints must send `Authorization: Bearer ` or `X-Subrouter-Admin-Token: `. Loopback stays trusted for ordinary admin endpoints. Account onboarding uses a distinct `SUBROUTER_ACCOUNT_IMPORT_TOKEN`; that token authorizes only `GET` and `POST /_subrouter/account-import` and cannot access admin APIs or proxy traffic. A server with neither credential configured rejects every account import, including `sr add`. That state is reported as `"account_import": "disabled"` by `/_subrouter/health` and logged as a warning at startup, and `sr doctor` runs the same preflight `sr add` runs against the selected server. diff --git a/cmd/subrouter/install_daemon.go b/cmd/subrouter/install_daemon.go index 5eb40f6a1..1baf9fdcb 100644 --- a/cmd/subrouter/install_daemon.go +++ b/cmd/subrouter/install_daemon.go @@ -23,6 +23,8 @@ const defaultDaemonLabel = "ai.manaflow.subrouter" type daemonConfig struct { Label string Addr string + AdminToken string + AdminTokenFile string InstallPath string TranscriptsDir string LogDir string @@ -51,7 +53,9 @@ func installDaemon(args []string) error { flags := flag.NewFlagSet("install-daemon", flag.ContinueOnError) config := daemonConfig{} flags.StringVar(&config.Label, "label", defaultDaemonLabel, "launchd label") - flags.StringVar(&config.Addr, "addr", "127.0.0.1:31415", "daemon listen address") + flags.StringVar(&config.Addr, "addr", "127.0.0.1:31415", "daemon listen address; a non-loopback address (e.g. a tailnet IP or MagicDNS name) requires --admin-token or --admin-token-file") + flags.StringVar(&config.AdminToken, "admin-token", "", "admin token for a non-loopback bind, written into the plist as SUBROUTER_ADMIN_TOKEN; the plist is then chmod 0600. Prefer --admin-token-file") + flags.StringVar(&config.AdminTokenFile, "admin-token-file", "", "path to a file holding the admin token, written into the plist as SUBROUTER_ADMIN_TOKEN_FILE; keeps the secret out of the plist entirely") flags.StringVar(&config.InstallPath, "install-path", defaultInstallPath, "subrouter binary install path") flags.StringVar(&config.TranscriptsDir, "transcripts", "", "local transcript directory; empty disables transcript recording") flags.StringVar(&config.LogDir, "log-dir", filepath.Join(home, "Library", "Logs"), "daemon log directory") @@ -78,6 +82,20 @@ func installDaemonWithConfig(config daemonConfig, home string, runner commandRun if err := validateDaemonConfig(config); err != nil { return err } + // The plist must carry the absolute path: launchd resolves relative paths + // against WorkingDirectory, not the installing shell's cwd. + resolvedTokenFile, err := resolveAdminTokenFile(config.AdminTokenFile) + if err != nil { + return err + } + if resolvedTokenFile != "" { + config.AdminTokenFile = resolvedTokenFile + if info, statErr := os.Stat(resolvedTokenFile); statErr == nil { + if warning := adminTokenFileWarning(resolvedTokenFile, info.Mode()); warning != "" { + fmt.Fprintln(os.Stderr, warning) + } + } + } plist, err := launchAgentPlist(config, home) if err != nil { return err @@ -116,7 +134,7 @@ func installDaemonWithConfig(config daemonConfig, home string, runner commandRun } plistPath := launchAgentPath(home, config.Label) - if err := os.WriteFile(plistPath, []byte(plist), 0o644); err != nil { + if err := os.WriteFile(plistPath, []byte(plist), launchAgentMode(config)); err != nil { return err } if config.Start { @@ -139,6 +157,65 @@ func installDaemonWithConfig(config daemonConfig, home string, runner commandRun return nil } +// launchAgentMode mirrors systemdDefaultsMode: a plist carrying the admin token +// inline is readable by every local user at the default 0644, which would hand +// the credential to exactly the audience the token exists to gate. --admin-token-file +// keeps the secret in a separate file, so the plist itself stays 0644. +// resolveAdminTokenFile turns --admin-token-file into the absolute path the +// plist should carry, and proves the daemon will be able to read it. +// +// Both halves have to happen at install time rather than at boot. The plist +// sets KeepAlive, so a token file the daemon cannot read is not one clear +// failure: serve exits, launchd restarts it, and the only signal an operator +// gets is churn in subrouter.err.log. And a relative path would be resolved +// against the LaunchAgent's WorkingDirectory rather than the shell the +// installer ran in, so it would quietly find a different file, or none. +// +// The readability check runs through the same readSecretFile the daemon uses, +// so "install succeeded" implies "serve can start". +func resolveAdminTokenFile(path string) (string, error) { + trimmed := strings.TrimSpace(path) + if trimmed == "" { + return "", nil + } + absolute, err := filepath.Abs(trimmed) + if err != nil { + return "", fmt.Errorf("admin-token-file %q: %w", trimmed, err) + } + info, err := os.Stat(absolute) + if err != nil { + return "", fmt.Errorf("admin-token-file %s: %w", absolute, err) + } + if info.IsDir() { + return "", fmt.Errorf("admin-token-file %s is a directory", absolute) + } + if _, err := readSecretFile(absolute, "admin-token-file"); err != nil { + return "", err + } + return absolute, nil +} + +// adminTokenFileWarning reports a token file other local users can read. That +// is not fatal -- the operator may have deliberate reasons -- but it defeats +// the only thing --admin-token-file buys over --admin-token, so it should not +// pass silently. +func adminTokenFileWarning(path string, mode os.FileMode) string { + if mode.Perm()&0o077 == 0 { + return "" + } + return fmt.Sprintf( + "warning: %s is mode %04o, so other local users can read the admin token; chmod 600 it", + path, mode.Perm(), + ) +} + +func launchAgentMode(config daemonConfig) os.FileMode { + if strings.TrimSpace(config.AdminToken) != "" { + return 0o600 + } + return 0o644 +} + func validateDaemonConfig(config daemonConfig) error { if strings.TrimSpace(config.Label) == "" { return errors.New("label is required") @@ -146,12 +223,28 @@ func validateDaemonConfig(config daemonConfig) error { if strings.TrimSpace(config.Addr) == "" { return errors.New("addr is required") } - host, _, err := net.SplitHostPort(config.Addr) - if err != nil { + if _, _, err := net.SplitHostPort(config.Addr); err != nil { return fmt.Errorf("addr must include host and port: %w", err) } - if host != "127.0.0.1" && host != "localhost" && host != "::1" { - return fmt.Errorf("install-daemon only supports localhost addresses, got %q", config.Addr) + // A non-loopback bind is allowed — it is what tailnet sharing needs — but it + // carries the same exposure `serve` refuses to arm silently: every host the + // ACLs admit reaches the admin surface (accounts, transcripts, + // account-import, drain). Reuse serve's predicate rather than a second + // spelling of "is this loopback", so the installed daemon can never be armed + // in a state serve itself would reject at startup. + if bindAddrRequiresAdminToken(config.Addr) && + strings.TrimSpace(config.AdminToken) == "" && + strings.TrimSpace(config.AdminTokenFile) == "" { + return fmt.Errorf( + "refusing to install a daemon bound to %s without an admin token: every host that can reach this address would get the admin endpoints (accounts, transcripts, account-import, drain). Pass --admin-token-file (preferred; keeps the secret out of the plist), --admin-token, or bind a loopback address such as 127.0.0.1:31415", + config.Addr, + ) + } + if strings.TrimSpace(config.AdminToken) != "" && strings.TrimSpace(config.AdminTokenFile) != "" { + return errors.New("pass only one of --admin-token or --admin-token-file") + } + if _, err := resolveAdminTokenFile(config.AdminTokenFile); err != nil { + return err } if strings.TrimSpace(config.InstallPath) == "" { return errors.New("install-path is required") @@ -309,31 +402,39 @@ func launchAgentPlist(config daemonConfig, home string) (string, error) { return "", err } data := struct { - Label string - Home string - Path string - InstallPath string - Addr string - TranscriptsDir string - HasTranscripts bool - SRSwitchInterval string - LogPath string - ErrorLogPath string - WorkingDirectory string - CloudConfigPath string + Label string + Home string + Path string + InstallPath string + Addr string + AdminToken string + HasAdminToken bool + AdminTokenFile string + HasAdminTokenFile bool + TranscriptsDir string + HasTranscripts bool + SRSwitchInterval string + LogPath string + ErrorLogPath string + WorkingDirectory string + CloudConfigPath string }{ - Label: escapeXMLString(config.Label), - Home: escapeXMLString(home), - Path: escapeXMLString(config.Path), - InstallPath: escapeXMLString(config.InstallPath), - Addr: escapeXMLString(config.Addr), - TranscriptsDir: escapeXMLString(config.TranscriptsDir), - HasTranscripts: strings.TrimSpace(config.TranscriptsDir) != "", - SRSwitchInterval: escapeXMLString(config.SRSwitchInterval), - LogPath: escapeXMLString(filepath.Join(config.LogDir, "subrouter.log")), - ErrorLogPath: escapeXMLString(filepath.Join(config.LogDir, "subrouter.err.log")), - WorkingDirectory: escapeXMLString(config.WorkingDirectory), - CloudConfigPath: escapeXMLString(cloudConfigPath), + Label: escapeXMLString(config.Label), + Home: escapeXMLString(home), + Path: escapeXMLString(config.Path), + InstallPath: escapeXMLString(config.InstallPath), + Addr: escapeXMLString(config.Addr), + AdminToken: escapeXMLString(config.AdminToken), + HasAdminToken: strings.TrimSpace(config.AdminToken) != "", + AdminTokenFile: escapeXMLString(config.AdminTokenFile), + HasAdminTokenFile: strings.TrimSpace(config.AdminTokenFile) != "", + TranscriptsDir: escapeXMLString(config.TranscriptsDir), + HasTranscripts: strings.TrimSpace(config.TranscriptsDir) != "", + SRSwitchInterval: escapeXMLString(config.SRSwitchInterval), + LogPath: escapeXMLString(filepath.Join(config.LogDir, "subrouter.log")), + ErrorLogPath: escapeXMLString(filepath.Join(config.LogDir, "subrouter.err.log")), + WorkingDirectory: escapeXMLString(config.WorkingDirectory), + CloudConfigPath: escapeXMLString(cloudConfigPath), } var out bytes.Buffer @@ -359,7 +460,11 @@ var launchAgentTemplate = template.Must(template.New("launch-agent").Parse(`{{.Home}} PATH {{.Path}} - + {{if .HasAdminTokenFile}}SUBROUTER_ADMIN_TOKEN_FILE + {{.AdminTokenFile}} + {{end}}{{if .HasAdminToken}}SUBROUTER_ADMIN_TOKEN + {{.AdminToken}} + {{end}} KeepAlive Label diff --git a/cmd/subrouter/install_daemon_test.go b/cmd/subrouter/install_daemon_test.go index 6aa84a259..96175c81a 100644 --- a/cmd/subrouter/install_daemon_test.go +++ b/cmd/subrouter/install_daemon_test.go @@ -128,3 +128,225 @@ func TestInstallDaemonWritesPlistAndInstallsExecutableWithoutStarting(t *testing t.Fatalf("cx alias target = %q, want %q", target, config.InstallPath) } } + +// A tailnet bind is the whole point of team sharing, so it must be installable — +// but only in a state `serve` would itself accept at startup. install-daemon used +// to refuse every non-loopback address outright, which made the chosen topology +// unreachable through the supported install path. +func TestValidateDaemonConfigRefusesNonLoopbackBindWithoutAdminToken(t *testing.T) { + base := daemonConfig{ + Label: defaultDaemonLabel, + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + } + for _, addr := range []string{ + "100.101.102.103:31415", // tailnet IPv4 + "mac.tail1234.ts.net:31415", // MagicDNS: unresolvable at validation time + ":31415", // every interface + } { + config := base + config.Addr = addr + err := validateDaemonConfig(config) + if err == nil { + t.Fatalf("addr %q: expected refusal without an admin token", addr) + } + if !strings.Contains(err.Error(), "admin token") { + t.Fatalf("addr %q: error should name the missing admin token, got %v", addr, err) + } + } +} + +// writeAdminTokenFile creates a token file the daemon could actually read, so +// validation exercises the real reader rather than a path that happens to +// look plausible. +func writeAdminTokenFile(t *testing.T, mode os.FileMode) string { + t.Helper() + path := filepath.Join(t.TempDir(), "admin-token") + if err := os.WriteFile(path, []byte("secret-admin-token\n"), mode); err != nil { + t.Fatalf("write admin token file: %v", err) + } + return path +} + +func TestValidateDaemonConfigAllowsLoopbackAndTokenedTailnetBinds(t *testing.T) { + tokenFile := writeAdminTokenFile(t, 0o600) + base := daemonConfig{ + Label: defaultDaemonLabel, + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + } + cases := []struct { + name string + mutate func(*daemonConfig) + }{ + {"loopback needs no token", func(c *daemonConfig) { c.Addr = "127.0.0.1:31415" }}, + {"localhost needs no token", func(c *daemonConfig) { c.Addr = "localhost:31415" }}, + {"tailnet with token file", func(c *daemonConfig) { + c.Addr = "100.101.102.103:31415" + c.AdminTokenFile = tokenFile + }}, + {"tailnet with inline token", func(c *daemonConfig) { + c.Addr = "100.101.102.103:31415" + c.AdminToken = "secret-admin-token" + }}, + } + for _, tc := range cases { + config := base + tc.mutate(&config) + if err := validateDaemonConfig(config); err != nil { + t.Fatalf("%s: unexpected error %v", tc.name, err) + } + } +} + +func TestValidateDaemonConfigRejectsBothTokenForms(t *testing.T) { + config := daemonConfig{ + Label: defaultDaemonLabel, + Addr: "100.101.102.103:31415", + AdminToken: "secret-admin-token", + AdminTokenFile: writeAdminTokenFile(t, 0o600), + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + } + if err := validateDaemonConfig(config); err == nil { + t.Fatal("expected an error when both token forms are passed") + } +} + +// The security property that motivates the env-var plumbing: launchd +// ProgramArguments are visible in `ps` to every local user, so an admin token +// must never be an argv element. +func TestLaunchAgentPlistKeepsAdminTokenOutOfProgramArguments(t *testing.T) { + home := "/Users/alice" + config := daemonConfig{ + Label: defaultDaemonLabel, + Addr: "100.101.102.103:31415", + AdminToken: "super-secret-admin-token", + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + Path: defaultDaemonPath("/Users/alice/bin/subrouter"), + } + plist, err := launchAgentPlist(config, home) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(plist, "SUBROUTER_ADMIN_TOKEN") { + t.Fatal("expected the token to be exported as an environment variable") + } + args := plist[strings.Index(plist, "ProgramArguments"):] + args = args[:strings.Index(args, "")] + if strings.Contains(args, config.AdminToken) { + t.Fatal("admin token leaked into ProgramArguments, where ps exposes it to every local user") + } + if mode := launchAgentMode(config); mode != 0o600 { + t.Fatalf("a plist carrying an inline token must be 0600, got %#o", mode) + } +} + +func TestLaunchAgentPlistPrefersTokenFileAndLeavesPlistReadable(t *testing.T) { + home := "/Users/alice" + tokenFile := writeAdminTokenFile(t, 0o600) + config := daemonConfig{ + Label: defaultDaemonLabel, + Addr: "mac.tail1234.ts.net:31415", + AdminTokenFile: tokenFile, + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + Path: defaultDaemonPath("/Users/alice/bin/subrouter"), + } + plist, err := launchAgentPlist(config, home) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(plist, "SUBROUTER_ADMIN_TOKEN_FILE") || + !strings.Contains(plist, ""+tokenFile+"") { + t.Fatal("expected SUBROUTER_ADMIN_TOKEN_FILE in the plist environment") + } + if strings.Contains(plist, "SUBROUTER_ADMIN_TOKEN") { + t.Fatal("token-file mode must not also emit an inline SUBROUTER_ADMIN_TOKEN") + } + // No secret in the plist, so it does not need tightening. + if mode := launchAgentMode(config); mode != 0o644 { + t.Fatalf("token-file mode should leave the plist at 0644, got %#o", mode) + } +} + +// An admin token file the daemon cannot read has to fail here, not at boot. +// The plist sets KeepAlive, so serve would exit, launchd would restart it, and +// the operator's only signal would be churn in subrouter.err.log. +func TestValidateDaemonConfigRejectsUnusableAdminTokenFile(t *testing.T) { + dir := t.TempDir() + + empty := filepath.Join(dir, "empty-token") + if err := os.WriteFile(empty, []byte(" \n"), 0o600); err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + path string + }{ + {"missing", filepath.Join(dir, "does-not-exist")}, + {"directory", dir}, + {"empty once trimmed", empty}, + } + for _, tc := range cases { + config := daemonConfig{ + Label: defaultDaemonLabel, + Addr: "100.101.102.103:31415", + AdminTokenFile: tc.path, + InstallPath: "/Users/alice/bin/subrouter", + LogDir: "/Users/alice/Library/Logs", + WorkingDirectory: "/Users/alice/fun/subrouter", + SRSwitchInterval: "10m", + } + if err := validateDaemonConfig(config); err == nil { + t.Fatalf("%s: expected install to refuse an unusable admin token file", tc.name) + } + } +} + +// launchd resolves relative paths against WorkingDirectory, not the shell the +// installer ran in, so the plist has to carry an absolute path or it silently +// reads a different file — or none. +func TestResolveAdminTokenFileReturnsAbsolutePath(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "admin-token"), []byte("secret\n"), 0o600); err != nil { + t.Fatal(err) + } + t.Chdir(dir) + + resolved, err := resolveAdminTokenFile("admin-token") + if err != nil { + t.Fatal(err) + } + if !filepath.IsAbs(resolved) { + t.Fatalf("resolved token path %q is not absolute", resolved) + } + if filepath.Base(resolved) != "admin-token" { + t.Fatalf("resolved token path %q lost the file name", resolved) + } +} + +// --admin-token-file exists to keep the secret out of the plist. A token file +// every other local user can read gives that back, so it must not pass quietly. +func TestAdminTokenFileWarningFlagsGroupAndWorldReadableModes(t *testing.T) { + if adminTokenFileWarning("/tmp/admin-token", 0o600) != "" { + t.Fatal("0600 should not warn") + } + for _, mode := range []os.FileMode{0o640, 0o604, 0o644, 0o666} { + if adminTokenFileWarning("/tmp/admin-token", mode) == "" { + t.Fatalf("mode %04o should warn", mode) + } + } +} diff --git a/cmd/subrouter/main.go b/cmd/subrouter/main.go index 05bb3e8c5..a25e0f988 100644 --- a/cmd/subrouter/main.go +++ b/cmd/subrouter/main.go @@ -98,21 +98,33 @@ func secretFromEnvironment(valueName, fileName string) (string, error) { if path == "" { return direct, nil } + return readSecretFile(path, fileName) +} + +// readSecretFile applies the rules a secret file has to satisfy for the daemon +// to start: readable, within maxSecretFileBytes, and non-empty once trimmed. +// +// It is shared with the installers on purpose. An installer that checked a +// token file its own way could accept a file the daemon then rejects, which +// under launchd's KeepAlive is not a single clear failure but an endless +// restart loop. Two gates answering one question from separate code is the +// shape of the tokenless-remote-admin bug; one reader is the fix. +func readSecretFile(path, label string) (string, error) { file, err := os.Open(path) if err != nil { - return "", fmt.Errorf("read %s: %w", fileName, err) + return "", fmt.Errorf("read %s: %w", label, err) } defer file.Close() body, err := io.ReadAll(io.LimitReader(file, maxSecretFileBytes+1)) if err != nil { - return "", fmt.Errorf("read %s: %w", fileName, err) + return "", fmt.Errorf("read %s: %w", label, err) } if len(body) > maxSecretFileBytes { - return "", fmt.Errorf("read %s: secret exceeds %d bytes", fileName, maxSecretFileBytes) + return "", fmt.Errorf("read %s: secret exceeds %d bytes", label, maxSecretFileBytes) } secret := strings.TrimSpace(string(body)) if secret == "" { - return "", fmt.Errorf("read %s: secret is empty", fileName) + return "", fmt.Errorf("read %s: secret is empty", label) } return secret, nil } @@ -216,6 +228,30 @@ func probe(args []string) error { return nil } +// bindAddrRequiresAdminToken reports whether listening on addr exposes the +// admin surface beyond loopback, which the startup gate refuses without an +// admin token. An unparseable addr never binds, so it is left for net.Listen +// to report; an empty host (":31415") binds every interface; a hostname that +// is not "localhost" (e.g. tailnet MagicDNS) is treated as non-loopback +// because its resolution cannot be trusted at validation time. +func bindAddrRequiresAdminToken(addr string) bool { + host, _, err := net.SplitHostPort(strings.TrimSpace(addr)) + if err != nil { + return false + } + if host == "" { + return true + } + if strings.EqualFold(host, "localhost") { + return false + } + ip := net.ParseIP(host) + if ip == nil { + return true + } + return !ip.IsLoopback() +} + var directSRCommands = map[string]struct{}{ "add": {}, "add-admin-key": {}, @@ -294,6 +330,7 @@ func serve(args []string) error { usageScoreTTL := flags.Duration("usage-score-ttl", 30*time.Second, "maximum age for usage scores before account selection refreshes them; 0 disables") shutdownTimeout := flags.Duration("shutdown-timeout", 10*time.Minute, "maximum time to drain in-flight proxy requests after SIGTERM/SIGINT") adminToken := flags.String("admin-token", "", "admin token required for non-loopback _subrouter endpoints; defaults to SUBROUTER_ADMIN_TOKEN") + allowUnauthenticatedAdmin := flags.Bool("allow-unauthenticated-admin", false, "serve the admin endpoints to REMOTE callers without any admin token; only sane for a deliberately unsecured deployment") accountImportToken := flags.String("account-import-token", "", "token limited to protected account import; defaults to SUBROUTER_ACCOUNT_IMPORT_TOKEN") requireSessionLeases := flags.Bool("require-session-leases", false, "reject proxy requests without a valid session lease; defaults to SUBROUTER_REQUIRE_SESSION_LEASES") maxBodyBytes := flags.Int64("max-body-bytes", 1<<20, "max JSON request body bytes to inspect for session IDs") @@ -333,6 +370,19 @@ func serve(args []string) error { if err != nil { return err } + // Refuse a non-loopback bind with no admin token. authorizeAdmin grants + // loopback callers admin unconditionally; on any wider bind, the admin + // surface (accounts, transcripts, account-import, drain) is reachable by + // every host that can reach the port — on a tailnet, every device the ACLs + // admit, over the plaintext HTTP this daemon deliberately allows there. + // Failing loudly at startup with the flag names beats failing open at + // request time. + if !*allowUnauthenticatedAdmin && strings.TrimSpace(*adminToken) == "" && bindAddrRequiresAdminToken(*addr) { + return fmt.Errorf( + "refusing to bind %s without an admin token: every host that can reach this address would get the admin endpoints (accounts, transcripts, account-import, drain). Set --admin-token (or SUBROUTER_ADMIN_TOKEN / SUBROUTER_ADMIN_TOKEN_FILE), bind a loopback address, or pass --allow-unauthenticated-admin to accept an unsecured deployment", + *addr, + ) + } if *publicURL == "" { *publicURL = strings.TrimSpace(os.Getenv("SUBROUTER_PUBLIC_URL")) } @@ -553,32 +603,33 @@ func serve(args []string) error { } server := proxy.Server{ - StreamDrops: &proxy.StreamDropStats{}, - Upstream: upstream, - CodexUpstream: codexUpstream, - APIUpstream: apiUpstream, - ClaudeUpstream: claudeUpstream, - KimiUpstream: kimiUpstream, - ZAIUpstream: zaiUpstream, - Accounts: nil, - AccountRef: accountRef, - CredentialBroker: credentialBroker, - Sessions: store, - SchedulerRef: schedulerRef, - UsageScoreTTL: usageScoreTTLForServe(*fetchUsage, *usageScoreTTL), - Transport: outboundTransport, - Logger: slog.Default(), - Lifecycle: proxy.NewLifecycle(), - AdminToken: *adminToken, - AccountImportToken: *accountImportToken, - RequireSessionLease: *requireSessionLeases || envTrue("SUBROUTER_REQUIRE_SESSION_LEASES"), - ForwardSessionHeaders: envTrue("SUBROUTER_FORWARD_SESSION_HEADERS"), - LocalProxyToken: localProxyToken, - MaxBodyBytes: *maxBodyBytes, - Bedrock: bedrockConfig, - ClaudeFableAPIKey: fableAPIKey, - FableBedrockPrimary: fableBedrockEnabled, - Transcripts: transcript.NewRecorder(*transcriptDir), + StreamDrops: &proxy.StreamDropStats{}, + Upstream: upstream, + CodexUpstream: codexUpstream, + APIUpstream: apiUpstream, + ClaudeUpstream: claudeUpstream, + KimiUpstream: kimiUpstream, + ZAIUpstream: zaiUpstream, + Accounts: nil, + AccountRef: accountRef, + CredentialBroker: credentialBroker, + Sessions: store, + SchedulerRef: schedulerRef, + UsageScoreTTL: usageScoreTTLForServe(*fetchUsage, *usageScoreTTL), + Transport: outboundTransport, + Logger: slog.Default(), + Lifecycle: proxy.NewLifecycle(), + AdminToken: *adminToken, + AllowUnauthenticatedAdmin: *allowUnauthenticatedAdmin, + AccountImportToken: *accountImportToken, + RequireSessionLease: *requireSessionLeases || envTrue("SUBROUTER_REQUIRE_SESSION_LEASES"), + ForwardSessionHeaders: envTrue("SUBROUTER_FORWARD_SESSION_HEADERS"), + LocalProxyToken: localProxyToken, + MaxBodyBytes: *maxBodyBytes, + Bedrock: bedrockConfig, + ClaudeFableAPIKey: fableAPIKey, + FableBedrockPrimary: fableBedrockEnabled, + Transcripts: transcript.NewRecorder(*transcriptDir), } transcriptGCSSyncer := transcript.NewGCSSyncer(transcript.GCSSyncerConfig{ SourceDir: *transcriptDir, diff --git a/cmd/subrouter/serve_admin_gate_test.go b/cmd/subrouter/serve_admin_gate_test.go new file mode 100644 index 000000000..4077cda60 --- /dev/null +++ b/cmd/subrouter/serve_admin_gate_test.go @@ -0,0 +1,108 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestBindAddrRequiresAdminToken(t *testing.T) { + cases := []struct { + addr string + want bool + }{ + {"127.0.0.1:31415", false}, + {"127.0.0.99:31415", false}, + {"localhost:31415", false}, + {"[::1]:31415", false}, + {"0.0.0.0:31415", true}, + {"[::]:31415", true}, + {":31415", true}, + {"100.64.1.2:31415", true}, + {"192.168.1.10:31415", true}, + {"my-machine.tail1234.ts.net:31415", true}, + // Unparseable addresses never bind; net.Listen reports them with a + // better error than the admin gate could. + {"invalid:::", false}, + } + for _, tc := range cases { + if got := bindAddrRequiresAdminToken(tc.addr); got != tc.want { + t.Errorf("bindAddrRequiresAdminToken(%q) = %v, want %v", tc.addr, got, tc.want) + } + } +} + +// clearControlSecretEnv makes the test immune to ambient token configuration — +// an exported SUBROUTER_ADMIN_TOKEN on the host would silently satisfy the +// gate and turn these tests vacuous. +func clearControlSecretEnv(t *testing.T) { + t.Helper() + for _, name := range []string{ + "SUBROUTER_ADMIN_TOKEN", "SUBROUTER_ADMIN_TOKEN_FILE", + "SUBROUTER_ACCOUNT_IMPORT_TOKEN", "SUBROUTER_ACCOUNT_IMPORT_TOKEN_FILE", + "SUBROUTER_PROXY_TOKEN", "SUBROUTER_PROXY_TOKEN_FILE", + } { + t.Setenv(name, "") + } +} + +// The tailnet trap: authorizeAdmin grants loopback callers admin +// unconditionally, so the moment serve binds a wider address, the admin +// surface (accounts, transcripts, account-import, drain) is one HTTP request +// away for every host that can reach the port. A tokenless non-loopback bind +// must therefore die at startup, naming the flags that fix it. +func TestServeRefusesNonLoopbackBindWithoutAdminToken(t *testing.T) { + for _, addr := range []string{"100.64.0.5:31415", "0.0.0.0:31415", ":31415"} { + t.Run(addr, func(t *testing.T) { + clearControlSecretEnv(t) + tempDir := t.TempDir() + t.Setenv("SUBROUTER_STATE_DIR", tempDir) + t.Setenv("SUBROUTER_CLOUD_CONFIG", filepath.Join(tempDir, "cloud.json")) + + err := serve([]string{ + "--addr", addr, + "--fetch-usage=false", + "--sr-switch-interval=0", + }) + if err == nil { + t.Fatal("serve accepted a tokenless non-loopback bind") + } + if !strings.Contains(err.Error(), "refusing to bind") || + !strings.Contains(err.Error(), "--admin-token") || + !strings.Contains(err.Error(), "--allow-unauthenticated-admin") { + t.Fatalf("gate error %q does not name the fix", err) + } + }) + } +} + +// Both sanctioned ways past the gate must actually get past it. The fixture +// points SUBROUTER_CLOUD_CONFIG at a directory so serve fails deterministically +// at the cloud-config load — a step that comes after the gate — proving the +// gate itself let the configuration through without ever binding a socket. +func TestServeAdminGateHonorsTokenAndExplicitOptIn(t *testing.T) { + for name, extraArgs := range map[string][]string{ + "admin-token": {"--admin-token", "gate-test-token"}, + "allow-unauthenticated-admin": {"--allow-unauthenticated-admin"}, + } { + t.Run(name, func(t *testing.T) { + clearControlSecretEnv(t) + tempDir := t.TempDir() + t.Setenv("SUBROUTER_STATE_DIR", tempDir) + t.Setenv("SUBROUTER_CLOUD_CONFIG", tempDir) // a directory: read fails past the gate + + args := append([]string{ + "--addr", "100.64.0.5:31415", + "--fetch-usage=false", + "--sr-switch-interval=0", + }, extraArgs...) + err := serve(args) + if err == nil { + t.Fatal("expected the deliberate cloud-config failure") + } + if strings.Contains(err.Error(), "refusing to bind") { + t.Fatalf("gate fired despite %s: %v", name, err) + } + }) + } +} diff --git a/docs/tailscale.md b/docs/tailscale.md new file mode 100644 index 000000000..84b56e055 --- /dev/null +++ b/docs/tailscale.md @@ -0,0 +1,80 @@ +# Serving Subrouter over a tailnet + +One machine runs the daemon and holds the accounts. Every other machine on the tailnet points its agents at that daemon instead of running its own. This is the shape to use when a pool of provider accounts should be shared by a few people, or by one person across a laptop, a desktop, and a VM, without copying provider credentials onto each of them. + +The daemon it installs is the same `subrouter serve` that a loopback install runs. The only difference is the bind address and the admin token that a non-loopback bind requires. + +## The tailnet is not the security boundary + +Tailscale decides which hosts can *reach* the port. It does not decide what they can *do* once they reach it, and Subrouter does not treat tailnet membership as authorization. + +`authorizeAdmin` trusts exactly one thing implicitly: a loopback peer. Every other caller — including one that is unambiguously on your tailnet, with a valid node key, admitted by your ACLs — must present the admin token to touch `/_subrouter/accounts`, `/_subrouter/transcripts`, `/_subrouter/account-import`, or `/_subrouter/drain`. + +That is deliberate, and it is a repaired bug rather than an original design. Tokenless remote admin used to be the silent default: a daemon bound to a tailnet address with no `--admin-token` handed every reachable host the full admin surface, including transcripts and account import. So `serve` now refuses to start on a non-loopback address without a token, and as of this change `install-daemon` refuses to install one. There is an `--allow-unauthenticated-admin` escape hatch on `serve`; treat it as a debugging tool, not a deployment option. + +Concretely: your ACLs are the second lock, not the first. A tailnet with a permissive `autogroup:member` rule is a normal, sensible tailnet and is still not sufficient on its own. + +## Install on macOS + +Generate a token and put it in its own file. Prefer this form — the secret then never enters the plist at all. + +```bash +mkdir -p ~/.subrouter +openssl rand -hex 32 > ~/.subrouter/admin-token +chmod 600 ~/.subrouter/admin-token + +make build +./bin/subrouter install-daemon \ + --addr 100.x.y.z:31415 \ + --admin-token-file ~/.subrouter/admin-token +``` + +Bind the **tailnet IP**, not the MagicDNS name. The address is resolved once, at bind time, and a LaunchAgent can start before `tailscaled` has MagicDNS answering; the 100.x address on the interface is ready earlier and does not change. MagicDNS names are the right thing for *clients* to dial, just not for the listener. + +The alternative form puts the token inline in the plist's `EnvironmentVariables`, and the installer then writes the plist `0600` instead of `0644`: + +```bash +./bin/subrouter install-daemon --addr 100.x.y.z:31415 --admin-token "$TOKEN" +``` + +Pass one or the other, never both — `serve` treats `SUBROUTER_ADMIN_TOKEN` and `SUBROUTER_ADMIN_TOKEN_FILE` both being set as a fatal misconfiguration, so accepting both flags would install a daemon that validates cleanly and then fails at boot. + +Either way the token stays out of `ProgramArguments`. launchd argv is world-readable through `ps`, so a token passed there would be legible to every local user on the machine — the exact audience the token exists to gate. + +## Install on Linux + +`install-systemd` has taken `--admin-token` for a while and is unchanged: + +```bash +sudo sr install-systemd --addr 100.x.y.z:31415 --admin-token-stdin <<<"$TOKEN" +``` + +`--admin-token-stdin` is the systemd equivalent of `--admin-token-file`: it keeps the secret out of the process arguments and out of your shell history. There is no `--admin-token-stdin` on macOS; use `--admin-token-file`. + +## Point the clients at it + +On each other machine: + +```bash +sr server add pool --url http://host.tailnet-name.ts.net:31415 --admin-token "$TOKEN" --default +``` + +MagicDNS is fine here. The token is the same one the daemon was installed with; a client without it can still proxy provider traffic but cannot read accounts, sessions, the dashboard, or transcripts. + +Account onboarding is a separate, narrower credential: `SUBROUTER_ACCOUNT_IMPORT_TOKEN` authorizes `GET` and `POST /_subrouter/account-import` and nothing else. Give that to a machine that only needs to hand accounts in, and it cannot read admin APIs or proxy traffic with it. + +## What remains loopback-only + +`/_subrouter/drain` is refused from any non-loopback peer regardless of token. Draining the pool is something you do on the host itself, over SSH or at the keyboard — a correct admin token does not buy it remotely. + +`/_subrouter/health` stays unauthenticated in both directions so liveness probes work. `/_subrouter/ready` likewise, returning 503 while draining. + +## Operating it + +Logs are `~/Library/Logs/subrouter.log` and `~/Library/Logs/subrouter.err.log`. The plist sets `KeepAlive`, so a bind that loses the race with `tailscaled` at login does not leave the daemon dead — launchd restarts it until the address is available. The tell is a burst of start/exit pairs in the error log that stops on its own. A burst that *doesn't* stop is a real bind failure: wrong IP, or the Tailscale node changed address. + +Rotating the token means rewriting the token file (or reinstalling with a new `--admin-token`), restarting the daemon, and running `sr server add` again on every client with the new value. Clients fail closed and loudly on a stale token — a 401 on the admin endpoints, not a silent downgrade to unauthenticated. + +## Before you expose one + +[docs/production.md](production.md) is the checklist — listener, tokens, credential transfer, drain behaviour. This document covers the topology and the install; that one covers whether the result is fit to leave running. diff --git a/internal/proxy/admin_gate_test.go b/internal/proxy/admin_gate_test.go new file mode 100644 index 000000000..a4f20c053 --- /dev/null +++ b/internal/proxy/admin_gate_test.go @@ -0,0 +1,82 @@ +package proxy + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" +) + +// The open-remote-admin regression: with no AdminToken and no +// AccountImportToken configured, authorizeAdmin used to grant every REMOTE +// caller the full admin surface — accounts, transcripts, account-import, +// drain — the moment the daemon was bound beyond loopback. Tokenless remote +// admin now requires the explicit AllowUnauthenticatedAdmin opt-in. +func TestTokenlessRemoteAdminRequiresExplicitOptIn(t *testing.T) { + cases := []struct { + name string + server Server + remoteAddr string + forwarded string + want bool + }{ + { + name: "tokenless remote is denied by default", + server: Server{}, + remoteAddr: "100.64.0.9:52011", + want: false, + }, + { + name: "explicit opt-in restores the legacy open behavior", + server: Server{AllowUnauthenticatedAdmin: true}, + remoteAddr: "100.64.0.9:52011", + want: true, + }, + { + name: "a scoped import token is never the only barrier, even opted in", + server: Server{ + AllowUnauthenticatedAdmin: true, + AccountImportToken: "import-only", + }, + remoteAddr: "100.64.0.9:52011", + want: false, + }, + { + name: "loopback keeps unconditional admin without any opt-in", + server: Server{}, + remoteAddr: "127.0.0.1:40000", + want: true, + }, + { + name: "spoofed forwarded header cannot claim loopback", + server: Server{}, + remoteAddr: "100.64.0.9:52011", + forwarded: "127.0.0.1", + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest("GET", "http://example/_subrouter/accounts", nil) + req.RemoteAddr = tc.remoteAddr + if tc.forwarded != "" { + req.Header.Set("X-Forwarded-For", tc.forwarded) + req.Header.Set("X-Real-IP", tc.forwarded) + } + if got := tc.server.authorizeAdmin(req); got != tc.want { + t.Fatalf("authorizeAdmin = %v, want %v", got, tc.want) + } + }) + } +} + +// loopbackAdminRequest builds a request whose peer is the daemon host itself, +// which is how the operator-facing admin endpoints are reached in the +// deployments these tests model. httptest.NewRequest's default RemoteAddr +// (192.0.2.1) is a REMOTE peer, and tokenless remote admin now fails closed — +// tests exercising admin endpoint logic must say which peer they mean. +func loopbackAdminRequest(method, target string, body io.Reader) *http.Request { + req := httptest.NewRequest(method, target, body) + req.RemoteAddr = "127.0.0.1:54321" + return req +} diff --git a/internal/proxy/dashboard_test.go b/internal/proxy/dashboard_test.go index c3b71f8fd..4b50ccee8 100644 --- a/internal/proxy/dashboard_test.go +++ b/internal/proxy/dashboard_test.go @@ -33,7 +33,7 @@ func TestDashboardAndTranscriptEndpoints(t *testing.T) { handler := server.Handler() dashboard := httptest.NewRecorder() - handler.ServeHTTP(dashboard, httptest.NewRequest(http.MethodGet, "/_subrouter/dashboard", nil)) + handler.ServeHTTP(dashboard, loopbackAdminRequest(http.MethodGet, "/_subrouter/dashboard", nil)) if dashboard.Code != http.StatusOK { t.Fatalf("dashboard status = %d", dashboard.Code) } @@ -45,7 +45,7 @@ func TestDashboardAndTranscriptEndpoints(t *testing.T) { } list := httptest.NewRecorder() - handler.ServeHTTP(list, httptest.NewRequest(http.MethodGet, "/_subrouter/transcripts", nil)) + handler.ServeHTTP(list, loopbackAdminRequest(http.MethodGet, "/_subrouter/transcripts", nil)) if list.Code != http.StatusOK { t.Fatalf("list status = %d", list.Code) } @@ -61,7 +61,7 @@ func TestDashboardAndTranscriptEndpoints(t *testing.T) { } detail := httptest.NewRecorder() - handler.ServeHTTP(detail, httptest.NewRequest(http.MethodGet, "/_subrouter/transcripts/codex/session-1", nil)) + handler.ServeHTTP(detail, loopbackAdminRequest(http.MethodGet, "/_subrouter/transcripts/codex/session-1", nil)) if detail.Code != http.StatusOK { t.Fatalf("detail status = %d", detail.Code) } @@ -73,7 +73,7 @@ func TestDashboardAndTranscriptEndpoints(t *testing.T) { } raw := httptest.NewRecorder() - handler.ServeHTTP(raw, httptest.NewRequest(http.MethodGet, "/_subrouter/transcripts/codex/session-1/raw", nil)) + handler.ServeHTTP(raw, loopbackAdminRequest(http.MethodGet, "/_subrouter/transcripts/codex/session-1/raw", nil)) if raw.Code != http.StatusOK { t.Fatalf("raw status = %d", raw.Code) } diff --git a/internal/proxy/multitenant.go b/internal/proxy/multitenant.go index 3ea0e875c..5a617312c 100644 --- a/internal/proxy/multitenant.go +++ b/internal/proxy/multitenant.go @@ -341,9 +341,12 @@ func (m *MultiTenant) newTenantServer(ctx context.Context, t tenant.Tenant) (*Se server.CacheFlight = newSingleFlight() // Reaching a tenant handler already proves possession of the tenant key, // so the tenant-visible _subrouter read endpoints need no admin token. + // tenantControlAuthorized carries that delegation into authorizeAdmin, + // which otherwise refuses tokenless remote callers. server.AdminToken = "" server.AccountImportToken = "" server.tenantAccountImportAuthorized = true + server.tenantControlAuthorized = true server.Transcripts = nil if m.TranscriptDir != "" { server.Transcripts = transcript.NewRecorder(filepath.Join(m.TranscriptDir, "tenants", t.ID)) diff --git a/internal/proxy/multitenant_test.go b/internal/proxy/multitenant_test.go index 7bb466ac3..c6eafe175 100644 --- a/internal/proxy/multitenant_test.go +++ b/internal/proxy/multitenant_test.go @@ -281,7 +281,7 @@ func TestMultiTenantKeyRevocationAndRotation(t *testing.T) { } // Mint a second key through the admin API, then revoke the first. - keysReq := httptest.NewRequest(http.MethodPost, "/_subrouter/tenants/"+created.ID+"/keys", nil) + keysReq := loopbackAdminRequest(http.MethodPost, "/_subrouter/tenants/"+created.ID+"/keys", nil) keysResp := httptest.NewRecorder() handler.ServeHTTP(keysResp, keysReq) if keysResp.Code != http.StatusOK { @@ -294,7 +294,7 @@ func TestMultiTenantKeyRevocationAndRotation(t *testing.T) { t.Fatal(err) } - revokeReq := httptest.NewRequest(http.MethodDelete, "/_subrouter/tenants/"+created.ID+"/keys/"+created.Keys[0].Prefix, nil) + revokeReq := loopbackAdminRequest(http.MethodDelete, "/_subrouter/tenants/"+created.ID+"/keys/"+created.Keys[0].Prefix, nil) revokeResp := httptest.NewRecorder() handler.ServeHTTP(revokeResp, revokeReq) if revokeResp.Code != http.StatusOK { @@ -1841,7 +1841,7 @@ func TestAccountListDoesNotRefreshOrRewriteOAuthCredentials(t *testing.T) { response := httptest.NewRecorder() server.Handler().ServeHTTP( response, - httptest.NewRequest(http.MethodGet, "/_subrouter/accounts", nil), + loopbackAdminRequest(http.MethodGet, "/_subrouter/accounts", nil), ) if response.Code != http.StatusOK { t.Fatalf("response = %d: %s", response.Code, response.Body.String()) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index 123b4a626..9cc8ec3c1 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -80,6 +80,13 @@ type Server struct { StreamDrops *StreamDropStats Lifecycle *Lifecycle AdminToken string + // AllowUnauthenticatedAdmin preserves the legacy open-admin fallback: with + // no AdminToken and no AccountImportToken configured, REMOTE callers get + // the full admin surface (accounts, transcripts, account-import, drain). + // That is only sane for a deliberately unsecured single-user deployment, + // so it now requires this explicit opt-in; without it, tokenless admin is + // loopback-only. + AllowUnauthenticatedAdmin bool // AccountImportToken authorizes only the protected account-import endpoint. // It is intentionally distinct from AdminToken, which can read operational // state and transcripts. @@ -111,6 +118,13 @@ type Server struct { // MultiTenant router has validated its tenant key. Global remote imports // require a configured admin token instead. tenantAccountImportAuthorized bool + // tenantControlAuthorized is likewise set only on a tenant-scoped server: + // the MultiTenant router routes only tenantControlPaths here, and reaching + // any of them already proved possession of the tenant key, so the scoped + // admin reads need no further token. This is authentication DELEGATED + // upstream — distinct from AllowUnauthenticatedAdmin, which is the + // explicit opt-in for a deployment with no authentication at all. + tenantControlAuthorized bool } type ActiveSessions struct { @@ -2169,13 +2183,22 @@ func matchesConfiguredBearerToken(r *http.Request, configuredToken, dedicatedHea } func (s Server) authorizeAdmin(r *http.Request) bool { + if s.tenantControlAuthorized { + // The MultiTenant router validated the tenant key before routing here, + // and only the tenant-visible control paths are routed here at all. + return true + } if isLoopbackRemote(r.RemoteAddr) { return true } if strings.TrimSpace(s.AdminToken) == "" { - // Preserve explicitly unsecured legacy/local configurations, but never let - // a scoped import token become the only barrier around remote admin APIs. - return strings.TrimSpace(s.AccountImportToken) == "" + // Tokenless remote admin used to be the silent default here, which + // meant a daemon bound to a tailnet address without --admin-token + // handed every reachable host the accounts, transcripts, + // account-import, and drain endpoints. It now requires the explicit + // AllowUnauthenticatedAdmin opt-in — and even then a scoped import + // token must never become the only barrier around remote admin APIs. + return s.AllowUnauthenticatedAdmin && strings.TrimSpace(s.AccountImportToken) == "" } return s.matchesConfiguredAdminToken(r) } diff --git a/internal/proxy/proxy_websocket_test.go b/internal/proxy/proxy_websocket_test.go index a05327307..6cc401545 100644 --- a/internal/proxy/proxy_websocket_test.go +++ b/internal/proxy/proxy_websocket_test.go @@ -1546,7 +1546,7 @@ func TestAccountStatusEndpointValidatesRefreshToken(t *testing.T) { }.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/_subrouter/account-status", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodPost, "/_subrouter/account-status", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } @@ -1612,7 +1612,7 @@ func TestUsageStatusEndpointFetchesUsageWithoutForcingFreshRefresh(t *testing.T) }.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/_subrouter/usage-status", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodGet, "/_subrouter/usage-status", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } @@ -1643,7 +1643,7 @@ func TestUsageStatusEndpointIncludesClaudeRequestTimeExhaustion(t *testing.T) { }.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/_subrouter/usage-status", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodGet, "/_subrouter/usage-status", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } diff --git a/internal/proxy/rate_limit_reset_test.go b/internal/proxy/rate_limit_reset_test.go index e9a86fda3..5257ef86a 100644 --- a/internal/proxy/rate_limit_reset_test.go +++ b/internal/proxy/rate_limit_reset_test.go @@ -98,7 +98,7 @@ func TestRateLimitResetEndpointRedeemsCookedAccountWithCredit(t *testing.T) { handler := Server{AccountRef: NewAccountRef(store, nil, client), MaxBodyBytes: 1024}.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/_subrouter/rate-limit-reset?email=cooked@example.com", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodPost, "/_subrouter/rate-limit-reset?email=cooked@example.com", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } @@ -129,7 +129,7 @@ func TestRateLimitResetEndpointRedeemsCookedAccountWithCredit(t *testing.T) { func TestRateLimitResetEndpointRejectsMissingTarget(t *testing.T) { handler := Server{AccountRef: NewAccountRef(accounts.CodexStore{Dir: t.TempDir()}, nil, nil), MaxBodyBytes: 1024}.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/_subrouter/rate-limit-reset", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodPost, "/_subrouter/rate-limit-reset", nil)) if recorder.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", recorder.Code) } @@ -149,7 +149,7 @@ func TestRateLimitResetEndpointDryRunDoesNotConsume(t *testing.T) { handler := Server{AccountRef: NewAccountRef(store, nil, client), MaxBodyBytes: 1024}.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/_subrouter/rate-limit-reset?email=cooked@example.com&dry_run=true", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodPost, "/_subrouter/rate-limit-reset?email=cooked@example.com&dry_run=true", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) } @@ -218,7 +218,7 @@ func TestRateLimitResetEndpointAllSkipsHealthy(t *testing.T) { handler := Server{AccountRef: NewAccountRef(store, nil, client), MaxBodyBytes: 1024}.Handler() recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodPost, "/_subrouter/rate-limit-reset?all=true", nil)) + handler.ServeHTTP(recorder, loopbackAdminRequest(http.MethodPost, "/_subrouter/rate-limit-reset?all=true", nil)) if recorder.Code != http.StatusOK { t.Fatalf("status = %d, body = %s", recorder.Code, recorder.Body.String()) }