diff --git a/cmd/waired-agent/login.go b/cmd/waired-agent/login.go index beffb3ab7..a1a72cdd3 100644 --- a/cmd/waired-agent/login.go +++ b/cmd/waired-agent/login.go @@ -29,8 +29,13 @@ type enrollFunc func(ctx context.Context, opts setup.EnrollOptions) (*setup.Enro type loginController struct { sb *switchboard activate func(parent context.Context) error - enroll enrollFunc - rootCtx context.Context + // reactivate tears the live session down and rebuilds it from the + // state a re-auth just rewrote. activate alone cannot do this: it + // refuses to publish over a session that is already current, which is + // precisely the state every re-auth starts from. + reactivate func(parent context.Context) error + enroll enrollFunc + rootCtx context.Context // enrollHTTPFor builds the HTTP client enrollment talks to the control // plane with, given the control URL that run() resolved. It is a // factory, not a client: under --bypass-cp-iam the transport mints a @@ -63,12 +68,17 @@ type loginControllerConfig struct { Endpoint string RootCtx context.Context Activate func(parent context.Context) error - Logger *slog.Logger + // Reactivate replaces a live session with one built from the state on + // disk. Required for re-auth; optional otherwise (a nil one makes a + // re-auth fail loudly rather than half-succeed with stale tokens in + // the running session). + Reactivate func(parent context.Context) error // EnrollHTTPFor is optional; nil enrolls with the default client. Set // it when the control plane is behind something that needs a // per-request credential — the IAM-gated Cloud Run service the testnet // runs against answers an unauthenticated POST with a 403 HTML page. EnrollHTTPFor func(ctx context.Context, controlURL string) *http.Client + Logger *slog.Logger // Enroll is optional; nil uses setup.Enroll. Enroll enrollFunc } @@ -81,6 +91,7 @@ func newLoginController(sb *switchboard, cfg loginControllerConfig) *loginContro return &loginController{ sb: sb, activate: cfg.Activate, + reactivate: cfg.Reactivate, enroll: enroll, enrollHTTPFor: cfg.EnrollHTTPFor, rootCtx: cfg.RootCtx, @@ -95,10 +106,26 @@ func (lc *loginController) Start(ctx context.Context, req management.LoginStartR lc.mu.Lock() defer lc.mu.Unlock() - // Already enrolled + active: idempotent no-op. - if lc.sb.current() != nil { + // Already enrolled + active: idempotent no-op. `waired init` run twice + // must not re-enrol, and the tray's start-on-click must not either. + // + // Reauth is the one caller that means it. It is how an enrolled device + // renews credentials the refresh loop can no longer renew for itself + // (#175) — the control plane matches the machine key and renews the + // same device row, so this replaces tokens, it does not add a device. + // Read the session once: lc.mu does not cover the switchboard, and the + // node-key rotator can swap it underneath. Two reads could disagree and + // leave this taking the no-op branch with reauth already true. + live := lc.sb.current() != nil + reauth := req.Reauth && live + if live && !req.Reauth { return management.LoginStatus{Phase: management.LoginPhaseActive}, nil } + if reauth && lc.reactivate == nil { + // Enrolling would rewrite the tokens on disk while the live session + // kept using the old ones — worse than refusing, and invisible. + return management.LoginStatus{}, errors.New("login: this daemon cannot re-authenticate a live session") + } // A login is already in flight: single-flight — return its status // rather than spawning a second browser OAuth. if lc.session != nil { @@ -131,7 +158,7 @@ func (lc *loginController) Start(ctx context.Context, req management.LoginStartR phase: management.LoginPhaseLoggingIn, cancel: cancel, } - go lc.run(loginCtx, sessID, controlURL, deviceName, req.AuthKey) + go lc.run(loginCtx, sessID, controlURL, deviceName, req.AuthKey, reauth) return lc.snapshotLocked(), nil } @@ -152,8 +179,10 @@ func (lc *loginController) Status(ctx context.Context, sessionID string) (manage } // run executes enrollment then live activation on a background -// goroutine, advancing the session's phase as it goes. -func (lc *loginController) run(ctx context.Context, sessID, controlURL, deviceName, authKey string) { +// goroutine, advancing the session's phase as it goes. reauth selects +// the activation that replaces a live session instead of publishing a +// first one. +func (lc *loginController) run(ctx context.Context, sessID, controlURL, deviceName, authKey string, reauth bool) { // Resolve a port-0 login endpoint (default "udp4:127.0.0.1:0") to a // concrete free UDP port before enrolling. The endpoint is persisted into // identity.json and later parsed by udpListenPortFromEndpoint (which @@ -203,7 +232,15 @@ func (lc *loginController) run(ctx context.Context, sessID, controlURL, deviceNa // Live activation. Runs on rootCtx (process lifetime): the resulting // session must outlive both this goroutine and the login context. - if err := lc.activate(lc.rootCtx); err != nil { + // A re-auth has a session already running on the credentials we just + // replaced, so it tears that down first — activate refuses to publish + // over a live one, and leaving it up would mean the daemon kept using + // tokens the control plane has already rotated away from. + activate := lc.activate + if reauth { + activate = lc.reactivate + } + if err := activate(lc.rootCtx); err != nil { lc.fail(sessID, err) return } diff --git a/cmd/waired-agent/login_test.go b/cmd/waired-agent/login_test.go index 0e82d62ae..e73d8de38 100644 --- a/cmd/waired-agent/login_test.go +++ b/cmd/waired-agent/login_test.go @@ -65,6 +65,22 @@ func newTestLoginController(sb *switchboard, enroll enrollFunc, activate func(co }) } +// newTestReauthController is the same, plus the reactivate hook a re-auth +// needs. Separate so the tests above keep proving that a controller +// WITHOUT one still serves every non-reauth login (#175). +func newTestReauthController(sb *switchboard, enroll enrollFunc, activate, reactivate func(context.Context) error) *loginController { + return newLoginController(sb, loginControllerConfig{ + StateDir: "/tmp/does-not-matter", + DefaultControlURL: "https://cp.example", + Endpoint: "udp4:127.0.0.1:0", + RootCtx: context.Background(), + Activate: activate, + Reactivate: reactivate, + Logger: testLogger(), + Enroll: enroll, + }) +} + func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } func waitPhase(t *testing.T, lc *loginController, sessID string, want management.LoginPhase) management.LoginStatus { @@ -304,6 +320,92 @@ func TestLoginIdempotentWhenAlreadyActive(t *testing.T) { } } +// PRODUCT CONTRACT (#175): Reauth is the one request that means "yes, I +// know this device is enrolled — enrol it again anyway". It is what +// replaced the standalone re-auth path, so if this stops working there is +// no other way for an enrolled device to renew credentials its refresh +// loop can no longer renew. +// +// This does not invert TestLoginIdempotentWhenAlreadyActive above: that +// pins the no-op for a request that did NOT ask, and still does. +func TestLoginReauthReenrollsALiveDevice(t *testing.T) { + sb := &switchboard{} + sb.publish(&session{provider: &agentProvider{id: &identity.Identity{DeviceID: "d1"}}}) + fe := &fakeEnroll{result: &setup.EnrollResult{DeviceID: "d1", AccountEmail: "ops@example.com"}} + + var activates, reactivates int32 + lc := newTestReauthController(sb, fe.fn, + func(context.Context) error { atomic.AddInt32(&activates, 1); return nil }, + func(context.Context) error { atomic.AddInt32(&reactivates, 1); return nil }) + + st, err := lc.Start(context.Background(), management.LoginStartRequest{Reauth: true}) + if err != nil { + t.Fatal(err) + } + if st.SessionID == "" { + t.Fatal("a re-auth must open a real session, not report the no-op status") + } + got := waitPhase(t, lc, st.SessionID, management.LoginPhaseActive) + if got.AccountEmail != "ops@example.com" { + t.Errorf("account email = %q, want the one the re-enrolment returned", got.AccountEmail) + } + if n := atomic.LoadInt32(&fe.calls); n != 1 { + t.Errorf("enroll ran %d times, want exactly 1", n) + } + // The live session is running on the credentials that were just + // replaced, so it has to be rebuilt — activate alone refuses to + // publish over a current session, and leaving it up would keep the + // daemon on tokens the control plane has rotated away from. + if atomic.LoadInt32(&reactivates) != 1 { + t.Errorf("reactivate ran %d times, want 1", reactivates) + } + if atomic.LoadInt32(&activates) != 0 { + t.Errorf("plain activate ran %d times on a re-auth, want 0", activates) + } +} + +// A fresh daemon has no session to tear down, so a Reauth request there is +// just a login. Worth pinning because the CLI sets the flag from "does +// identity.json exist", which the daemon has no reason to agree with — a +// state dir restored from backup, or an identity the daemon failed to +// activate, both land here. +func TestLoginReauthOnUnenrolledDaemonIsAPlainLogin(t *testing.T) { + sb := &switchboard{} + fe := &fakeEnroll{result: &setup.EnrollResult{DeviceID: "d1"}} + + var activates, reactivates int32 + lc := newTestReauthController(sb, fe.fn, + func(context.Context) error { atomic.AddInt32(&activates, 1); return nil }, + func(context.Context) error { atomic.AddInt32(&reactivates, 1); return nil }) + + st, err := lc.Start(context.Background(), management.LoginStartRequest{Reauth: true}) + if err != nil { + t.Fatal(err) + } + waitPhase(t, lc, st.SessionID, management.LoginPhaseActive) + if atomic.LoadInt32(&activates) != 1 || atomic.LoadInt32(&reactivates) != 0 { + t.Errorf("activate=%d reactivate=%d; an unenrolled daemon has nothing to rebuild", + activates, reactivates) + } +} + +// Refusing beats half-succeeding: with no way to rebuild the session, a +// re-auth would rewrite the tokens on disk and leave the running session +// using the old ones — an inconsistency nothing would report. +func TestLoginReauthRefusedWithoutARebuildHook(t *testing.T) { + sb := &switchboard{} + sb.publish(&session{provider: &agentProvider{id: &identity.Identity{DeviceID: "d1"}}}) + fe := &fakeEnroll{} + lc := newTestLoginController(sb, fe.fn, func(context.Context) error { return nil }) + + if _, err := lc.Start(context.Background(), management.LoginStartRequest{Reauth: true}); err == nil { + t.Fatal("want an error when the controller cannot rebuild the session") + } + if atomic.LoadInt32(&fe.calls) != 0 { + t.Error("enroll must not run when the rebuild it depends on is impossible") + } +} + func TestLoginStatusUnknownSessionResting(t *testing.T) { sb := &switchboard{} lc := newTestLoginController(sb, (&fakeEnroll{}).fn, func(context.Context) error { return nil }) diff --git a/cmd/waired-agent/main.go b/cmd/waired-agent/main.go index 5caf84cab..c53347ee3 100644 --- a/cmd/waired-agent/main.go +++ b/cmd/waired-agent/main.go @@ -1398,23 +1398,28 @@ func run(ctx context.Context, args []string) error { return nil } - // reactivate rebuilds the live session from the (now rotated) node key - // on disk: it tears down the current session and re-runs activate, - // which re-loads node.key and reconstructs the engine / multiplex-bind - // / relay factory / disco around it (#228). Serialised by its own mutex - // so a rotation cannot race a second rotation; the once-per-~150d - // cadence makes a race with a concurrent login implausible. Runs on a - // detached goroutine (the rotator triggers it via `go reactivate()`) - // because teardown cancels the rotator's own context. + // rebuildSession replaces the live session with one built from the + // state now on disk: it tears the current one down and re-runs + // activate, which re-loads node.key and reconstructs the engine / + // multiplex-bind / relay factory / disco around it (#228). Two callers + // need it — node-key rotation and a re-auth through the login + // controller (#175) — and both must be serialised against each other, + // so they share one mutex rather than each holding their own. var reactivateMu sync.Mutex - reactivate = func() { + rebuildSession := func(parent context.Context) error { reactivateMu.Lock() defer reactivateMu.Unlock() if s := sb.current(); s != nil { s.teardown() } sb.reset() - if err := activate(ctx); err != nil { + return activate(parent) + } + // The rotator's entry point: same rebuild, error logged rather than + // returned. Runs on a detached goroutine (the rotator triggers it via + // `go reactivate()`) because teardown cancels the rotator's own context. + reactivate = func() { + if err := rebuildSession(ctx); err != nil { logger.Error("re-activate after node-key rotation failed; device unenrolled until restart", "err", err) } } @@ -1442,6 +1447,7 @@ func run(ctx context.Context, args []string) error { Endpoint: "udp4:" + *loginListen, RootCtx: ctx, Activate: activate, + Reactivate: rebuildSession, EnrollHTTPFor: enrollHTTPFor, Logger: logger, }) diff --git a/cmd/waired/init_benchmark.go b/cmd/waired/init_benchmark.go index c3830994d..1d3ce2fea 100644 --- a/cmd/waired/init_benchmark.go +++ b/cmd/waired/init_benchmark.go @@ -327,6 +327,22 @@ func waitForBenchmark(mgmtURL string, out io.Writer) (resp *management.Benchmark case "pull_failed": writePrompt(out, "Model download failed; skipping the interactive-performance check.") return nil, false + case "disabled", "stopped": + // Terminal, the same way waitForBundledModel already treats + // them (init_pull.go): a subsystem that is off or parked will + // never report a ready model, so waiting is waiting for + // something nobody has asked to happen. + // + // This used to fall into the default arm below, which reads + // any unrecognised state as "engine is up, download in + // flight" — so `waired init --inference-enabled=false` sat on + // "Waiting for the model to finish downloading…" for the full + // ten-minute deadline and then reported it had given up, on a + // host with no model and no intention of getting one. Found + // by #175's installtest migration, which made this the path + // CI takes: ten minutes per leg, three legs, every PR. + writePrompt(out, "Local inference is off on this device; skipping the performance check.") + return nil, false case "no_engine": // On a fresh bundled install the engine is still being // brought up at the first polls, so `no_engine` is transient diff --git a/cmd/waired/init_benchmark_test.go b/cmd/waired/init_benchmark_test.go index c8866861a..d52a72b08 100644 --- a/cmd/waired/init_benchmark_test.go +++ b/cmd/waired/init_benchmark_test.go @@ -341,6 +341,56 @@ func TestPromptBenchmark_TerminalStateSkips(t *testing.T) { } } +// PRODUCT CONTRACT: a subsystem that is switched off or parked never +// produces a ready model, so the wait must end at the first poll — the +// same call waitForBundledModel already makes (init_pull.go). +// +// These two states used to fall through to "engine is up, a download must +// be in flight", so `waired init --inference-enabled=false` on the daemon +// path printed "Waiting for the model to finish downloading…" and held the +// terminal for the full ten-minute deadline before reporting that it had +// given up — on a host with no model and no intention of getting one. +// Measured: three installtest legs, ten minutes each, every PR. +func TestPromptBenchmark_OffSubsystemSkipsImmediately(t *testing.T) { + for _, state := range []string{"disabled", "stopped"} { + t.Run(state, func(t *testing.T) { + stub := &benchStub{ready: false, state: state} + srv := stub.server() + defer srv.Close() + + var out strings.Builder + done := make(chan error, 1) + go func() { + done <- promptBenchmarkRecommendation(srv.URL, false, &out, + bufio.NewScanner(strings.NewReader("")), false) + }() + select { + case err := <-done: + if err != nil { + t.Fatalf("prompt: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("still waiting: an off subsystem must not be waited out") + } + + if got := out.String(); !strings.Contains(got, "Local inference is off") { + t.Errorf("expected the off-subsystem skip notice, got: %q", got) + } + if got := out.String(); strings.Contains(got, "finish downloading") { + t.Errorf("announced a model download for state %q: %q", state, got) + } + // One probe of each endpoint is enough to decide; anything more + // means the loop kept going after a terminal answer. + stub.mu.Lock() + calls := stub.benchCalls + stub.mu.Unlock() + if calls != 1 { + t.Errorf("/benchmark polled %d times, want 1", calls) + } + }) + } +} + // When the only lighter step-down is the tiny 0.5B, declining (default No) // disables local inference rather than switching / dismissing. func TestPromptBenchmark_TinyDeclineDisables(t *testing.T) { diff --git a/cmd/waired/init_route_daemon.go b/cmd/waired/init_route_daemon.go index 77b41e450..7df8175bf 100644 --- a/cmd/waired/init_route_daemon.go +++ b/cmd/waired/init_route_daemon.go @@ -21,11 +21,15 @@ import ( // failure into a silent, remote, unfixable one. // // So the probe no longer decides *which implementation runs*; it decides -// whether the agent is there at all. Local enrollment runs only when it is -// explicitly selected, and an agent that is not answering is an error. -// Tailscale has no fallback either: `tailscale up` fails when tailscaled -// is not answering, and even its container image starts tailscaled first -// and authenticates it with an auth key. +// whether the agent is there at all. Tailscale has no fallback either: +// `tailscale up` fails when tailscaled is not answering, and even its +// container image starts tailscaled first and authenticates it with an +// auth key. +// +// Since the daemon can re-authenticate an already-enrolled device (#175), +// there is no second implementation left to choose between: every +// enrollment — first run, unattended with an auth key, or re-auth — runs +// in the daemon. The only remaining question is whether it is there. type enrollRoute int @@ -34,11 +38,6 @@ const ( // Drive its login endpoints; it owns enrollment, the state dir and the // live tunnel. routeDaemon enrollRoute = iota - // routeLocal: local enrollment, explicitly selected. Only re-auth - // reaches it now — the daemon login refreshes no tokens yet. Once it - // can, this route and the local enrollment implementation behind it go - // away entirely (#175). - routeLocal // routeAgentDown: a service is registered but the management API never // answered inside the wait window. This is exactly the state the silent // fallback used to hide, so it fails loudly instead. @@ -52,36 +51,23 @@ const ( // itself is a pure function that can be table-tested over every // combination (CLAUDE.md §Test discipline). type enrollFacts struct { - // The one remaining explicit selector for local enrollment: re-auth. - // The daemon login cannot refresh tokens for an already-enrolled device - // yet; when it can, this route and the local enrollment implementation - // behind it both go away (#175). - renewing bool - // authKey is set when the operator passed --auth-key (or - // $WAIRED_AUTH_KEY). It is a credential for the DAEMON's enrollment, - // never a selector for the local one: the local path has no way to - // redeem it, and silently ignoring it would enrol the host - // capability-less — the exact failure #175 exists to remove. - authKey bool // serviceInstalled reports whether an OS service is registered // (systemd unit / LaunchDaemon plist / SCM entry) — i.e. whether an // agent is *supposed* to be running here. serviceInstalled bool } -// chooseEnrollRoute picks the journey. probe is invoked only when an -// explicit selector has not already settled the answer, so a re-auth run -// never pays the wait window; it receives serviceInstalled -// because the probe waits longer for a service that is registered (and is -// therefore probably still starting). +// chooseEnrollRoute picks the journey. It receives serviceInstalled twice +// over — once as a fact and once through probe — because the probe waits +// longer for a service that is registered (and is therefore probably +// still starting), while the answer to "is this host missing an install or +// missing a running process" is the fact itself. +// +// PRODUCT CONTRACT (#175): no input selects an enrollment that bypasses +// the daemon, because there is no longer one to select. Whatever the run +// carries — an auth key, an existing identity to renew, neither — it goes +// to the daemon or it fails saying why. func chooseEnrollRoute(f enrollFacts, probe func(serviceInstalled bool) bool) enrollRoute { - // An auth key outranks the local selectors. Only the daemon can - // redeem one, so a run that carries a key must reach the daemon or - // fail saying why — never quietly fall back to a local enrollment - // that would drop the credential on the floor. - if !f.authKey && f.renewing { - return routeLocal - } if probe(f.serviceInstalled) { return routeDaemon } diff --git a/cmd/waired/init_route_daemon_test.go b/cmd/waired/init_route_daemon_test.go index e593a9269..d439a5ca5 100644 --- a/cmd/waired/init_route_daemon_test.go +++ b/cmd/waired/init_route_daemon_test.go @@ -12,87 +12,44 @@ import ( // daemon-driven login and a second, local enrollment implementation from a // single 1-second probe, with no user-visible signal — so a host whose // service failed to start enrolled "successfully" into a permanently -// dead-ended setup. These pin the replacement: local enrollment runs ONLY -// when explicitly selected, and an agent that is not answering is an error, +// dead-ended setup. Now that the daemon can also re-authenticate an +// enrolled device, there is no second implementation to choose: every run +// goes to the daemon, and an agent that is not answering is an error, // never a silent downgrade. +// +// The table is therefore the full input space (2 facts × 2 probe results), +// not a sample of it — which is the point: if a later change reintroduces +// an input that steers somewhere else, there is nowhere for it to steer to. func TestChooseEnrollRoute(t *testing.T) { tests := []struct { - name string - facts enrollFacts - daemonUp bool - want enrollRoute - wantProbed bool + name string + facts enrollFacts + daemonUp bool + want enrollRoute }{ { - name: "daemon answers, service registered", - facts: enrollFacts{serviceInstalled: true}, - daemonUp: true, - want: routeDaemon, - wantProbed: true, + name: "daemon answers, service registered", + facts: enrollFacts{serviceInstalled: true}, + daemonUp: true, + want: routeDaemon, }, { - name: "daemon answers, no registered service (raw-binary dev run)", - facts: enrollFacts{}, - daemonUp: true, - want: routeDaemon, - wantProbed: true, + name: "daemon answers, no registered service (raw-binary dev run)", + facts: enrollFacts{}, + daemonUp: true, + want: routeDaemon, }, { - name: "service registered but never answers: loud failure, not local enroll", - facts: enrollFacts{serviceInstalled: true}, - daemonUp: false, - want: routeAgentDown, - wantProbed: true, + name: "service registered but never answers: loud failure, not local enroll", + facts: enrollFacts{serviceInstalled: true}, + daemonUp: false, + want: routeAgentDown, }, { - name: "nothing registered and nothing answering", - facts: enrollFacts{}, - daemonUp: false, - want: routeAgentAbsent, - wantProbed: true, - }, - { - name: "re-auth selects local enrollment without probing", - facts: enrollFacts{renewing: true, serviceInstalled: true}, - daemonUp: false, - want: routeLocal, - wantProbed: false, - }, - - // PRODUCT CONTRACT (#175): an auth key is a credential for the - // DAEMON's enrollment. Only the daemon can redeem one, so a run - // carrying a key must reach the daemon or fail saying why — it - // must NEVER fall back to a local enrollment that would drop the - // credential and register the host capability-less, which is the - // exact failure this issue exists to remove. The key therefore - // outranks every local selector. - { - name: "--auth-key takes the daemon route", - facts: enrollFacts{authKey: true, serviceInstalled: true}, - daemonUp: true, - want: routeDaemon, - wantProbed: true, - }, - { - name: "--auth-key with a dead service fails, never local", - facts: enrollFacts{authKey: true, serviceInstalled: true}, - daemonUp: false, - want: routeAgentDown, - wantProbed: true, - }, - { - name: "--auth-key with no agent at all fails, never local", - facts: enrollFacts{authKey: true}, - daemonUp: false, - want: routeAgentAbsent, - wantProbed: true, - }, - { - name: "--auth-key overrides re-auth", - facts: enrollFacts{authKey: true, renewing: true, serviceInstalled: true}, - daemonUp: true, - want: routeDaemon, - wantProbed: true, + name: "nothing registered and nothing answering", + facts: enrollFacts{}, + daemonUp: false, + want: routeAgentAbsent, }, } @@ -108,12 +65,15 @@ func TestChooseEnrollRoute(t *testing.T) { if got != tt.want { t.Errorf("chooseEnrollRoute() = %v, want %v", got, tt.want) } - if probed != tt.wantProbed { - t.Errorf("probe called = %v, want %v", probed, tt.wantProbed) + // Every route is decided by the probe now; nothing short-circuits + // it. A fact that skipped the probe would be a fact that picked an + // implementation without asking whether the agent is there. + if !probed { + t.Error("probe was not called; every route decision must consult the agent") } // The probe waits longer for a registered service, so it must // receive that fact verbatim rather than re-deriving it. - if probed && sawInstalled != tt.facts.serviceInstalled { + if sawInstalled != tt.facts.serviceInstalled { t.Errorf("probe got serviceInstalled=%v, want %v", sawInstalled, tt.facts.serviceInstalled) } }) @@ -258,13 +218,11 @@ func TestDaemonRequiredError(t *testing.T) { } } -// The successful routes carry no error — the caller switches on the route -// and only asks for an error on the two failing ones. +// The one successful route carries no error — the caller switches on the +// route and only asks for an error on the two failing ones. func TestDaemonRequiredErrorNilForSuccessfulRoutes(t *testing.T) { - for _, route := range []enrollRoute{routeDaemon, routeLocal} { - if err := daemonRequiredError(route, "linux", "hint"); err != nil { - t.Errorf("route %v: expected no error, got %v", route, err) - } + if err := daemonRequiredError(routeDaemon, "linux", "hint"); err != nil { + t.Errorf("route %v: expected no error, got %v", routeDaemon, err) } } diff --git a/cmd/waired/login_client.go b/cmd/waired/login_client.go index 5ad600535..fcfb27983 100644 --- a/cmd/waired/login_client.go +++ b/cmd/waired/login_client.go @@ -41,11 +41,17 @@ var daemonReachable = func(mgmtURL string) bool { // authKey is appended rather than slotted next to control/deviceName on // purpose: three adjacent string parameters invite a silent swap, and a // trailing argument leaves every existing call site's positions untouched. -func runInitViaDaemon(mgmtURL, control, deviceName string, noBrowser, nonInteractive, skipIntegration bool, gatewayBaseURL string, owner *stdinReader, inf daemonInitInference, authKey string) error { +// +// reauth is set when this host already has an identity. Without it the +// daemon treats a Start on an active session as an idempotent no-op and +// this function would print a successful sign-in for a run that renewed +// nothing (#175). +func runInitViaDaemon(mgmtURL, control, deviceName string, noBrowser, nonInteractive, skipIntegration bool, gatewayBaseURL string, owner *stdinReader, inf daemonInitInference, authKey string, reauth bool) error { reqBody, _ := json.Marshal(management.LoginStartRequest{ ControlURL: control, DeviceName: deviceName, AuthKey: authKey, + Reauth: reauth, }) out, err := httpPost(mgmtURL+"/waired/v1/login/start", reqBody) if err != nil { @@ -59,6 +65,14 @@ func runInitViaDaemon(mgmtURL, control, deviceName string, noBrowser, nonInterac return fmt.Errorf("decode login start: %w", err) } if st.SessionID == "" { + // An agent too old to know about `reauth` ignores the field and + // answers with its idempotent no-op: active, no session. Saying + // "no session id" there would send the operator looking for a bug + // in a daemon that is working exactly as it was built to (#175). + if reauth && st.Phase == management.LoginPhaseActive { + return errors.New("this device is signed in, but the background service is too old to renew that sign-in.\n" + + " Update Waired, then run `waired init` again") + } return errors.New("daemon did not return a login session id") } diff --git a/cmd/waired/login_client_inference_test.go b/cmd/waired/login_client_inference_test.go index c23cce41e..6d1f15f69 100644 --- a/cmd/waired/login_client_inference_test.go +++ b/cmd/waired/login_client_inference_test.go @@ -85,7 +85,7 @@ func runViaDaemonGuarded(t *testing.T, url string) string { go func() { done <- runInitViaDaemon(url, "https://cp.example", "dev-1", true /*noBrowser*/, true /*nonInteractive*/, true /*skipIntegration*/, "http://127.0.0.1:9473", - nil /*no terminal*/, daemonInitInference{}, "") + nil /*no terminal*/, daemonInitInference{}, "", false /*reauth*/) }() select { case runErr = <-done: diff --git a/cmd/waired/login_client_prompts_test.go b/cmd/waired/login_client_prompts_test.go index 3872bb571..95a4688f0 100644 --- a/cmd/waired/login_client_prompts_test.go +++ b/cmd/waired/login_client_prompts_test.go @@ -177,7 +177,7 @@ func runDaemonInit(t *testing.T, url string, owner *stdinReader, o daemonInitOpt go func() { done <- runInitViaDaemon(url, "https://cp.example", "dev-1", o.noBrowser, o.nonInteractive, o.skipIntegration, - "http://127.0.0.1:9473", owner, daemonInitInference{}, "") + "http://127.0.0.1:9473", owner, daemonInitInference{}, "", false /*reauth*/) }() select { case runErr = <-done: diff --git a/cmd/waired/login_client_test.go b/cmd/waired/login_client_test.go index afabc144d..dd48d0b46 100644 --- a/cmd/waired/login_client_test.go +++ b/cmd/waired/login_client_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "sync/atomic" "testing" "time" @@ -63,7 +64,7 @@ func TestRunInitViaDaemonPollsToActive(t *testing.T) { // nonInteractive=true so the post-login #133 prompt never reads stdin. if err := runInitViaDaemon(srv.URL, "https://cp.example", "dev-1", true, true, true, /* skipIntegration: keep the test hermetic (no home-dir writes) */ - "http://127.0.0.1:9473", nil /*no terminal*/, daemonInitInference{}, ""); err != nil { + "http://127.0.0.1:9473", nil /*no terminal*/, daemonInitInference{}, "", false /*reauth*/); err != nil { t.Fatalf("runInitViaDaemon: %v", err) } if atomic.LoadInt32(&polls) < 2 { @@ -71,6 +72,88 @@ func TestRunInitViaDaemonPollsToActive(t *testing.T) { } } +// PRODUCT CONTRACT (#175): the re-auth intent must reach the daemon, and +// only when this host is actually re-authenticating. A request that always +// carried it would turn every first-run login into a re-enrolment attempt; +// one that never carried it would leave `waired init` on an enrolled host +// printing a successful sign-in for a run that renewed nothing. +func TestRunInitViaDaemonSendsReauthOnlyWhenRenewing(t *testing.T) { + setBenchTiming(t, time.Millisecond, 5*time.Second, time.Minute) + for _, tc := range []struct{ reauth bool }{{false}, {true}} { + name := "fresh" + if tc.reauth { + name = "renewing" + } + t.Run(name, func(t *testing.T) { + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/waired/v1/login/start": + _ = json.NewDecoder(r.Body).Decode(&body) + _ = json.NewEncoder(w).Encode(management.LoginStatus{ + SessionID: "s1", Phase: management.LoginPhaseActive, + AccountEmail: "user@example.com", + }) + case "/waired/v1/status": + _, _ = w.Write([]byte(`{}`)) + case "/waired/v1/inference/status": + _ = json.NewEncoder(w).Encode(management.InferenceStatus{SubsystemState: "disabled"}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + if err := runInitViaDaemon(srv.URL, "https://cp.example", "dev-1", true, true, + true, "http://127.0.0.1:9473", nil, daemonInitInference{}, "", tc.reauth); err != nil { + t.Fatalf("runInitViaDaemon: %v", err) + } + + got, present := body["reauth"].(bool) + if tc.reauth && (!present || !got) { + t.Errorf("renewing run sent reauth=%v (present=%v), want true", got, present) + } + // Omitted rather than false: the body a first run sends is the + // body it has always sent. + if !tc.reauth && present { + t.Errorf("fresh run sent a reauth field (%v); it must be omitted", got) + } + }) + } +} + +// An agent predating `reauth` ignores it and answers with its idempotent +// no-op — active, no session id. Reading that as success is the silent +// "renewed nothing" this whole change exists to remove, so it must be a +// named failure instead. +func TestRunInitViaDaemonNamesAnAgentTooOldToReauth(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/waired/v1/login/start" { + // No SessionID: exactly what loginController.Start returns for + // an already-active daemon that never saw the field. + _ = json.NewEncoder(w).Encode(management.LoginStatus{Phase: management.LoginPhaseActive}) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + err := runInitViaDaemon(srv.URL, "https://cp.example", "dev-1", true, true, + true, "http://127.0.0.1:9473", nil, daemonInitInference{}, "", true /*reauth*/) + if err == nil { + t.Fatal("want an error when the agent cannot re-authenticate") + } + if !strings.Contains(err.Error(), "too old") { + t.Errorf("error should say the service is too old, got: %v", err) + } + // The copy is for a person who has never heard of a daemon. + if strings.Contains(strings.ToLower(err.Error()), "session id") { + t.Errorf("error leaks the protocol-level symptom: %v", err) + } +} + func TestRunInitViaDaemonSurfacesError(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -93,7 +176,7 @@ func TestRunInitViaDaemonSurfacesError(t *testing.T) { err := runInitViaDaemon(srv.URL, "https://cp.example", "dev-1", true, true, true, /* skipIntegration: keep the test hermetic (no home-dir writes) */ - "http://127.0.0.1:9473", nil /*no terminal*/, daemonInitInference{}, "") + "http://127.0.0.1:9473", nil /*no terminal*/, daemonInitInference{}, "", false /*reauth*/) if err == nil { t.Fatal("expected error from error phase") } diff --git a/cmd/waired/main.go b/cmd/waired/main.go index 81f6f2a98..2fe263945 100644 --- a/cmd/waired/main.go +++ b/cmd/waired/main.go @@ -274,17 +274,15 @@ func runInitBody(o *initFlags) error { // Which enrollment journey this run takes. Enrollment is daemon-owned // (the Tailscale model): the running waired-agent performs it and this - // process is a thin client over its login API. There is no implicit - // fallback to the standalone path below — a probe that failed because - // the service never started used to silently produce a registered but + // process is a thin client over its login API. There is no fallback to + // the standalone path below — a probe that failed because the service + // never started used to silently produce a registered but // capability-less device (#175). See init_route_daemon.go. authKey, err := authKeyFromFlags(*authKeyFlag) if err != nil { return err } route := chooseEnrollRoute(enrollFacts{ - renewing: renewing, - authKey: authKey != "", serviceInstalled: serviceInstalledFn(), }, func(serviceInstalled bool) bool { return waitForDaemonStartup(*mgmtURL, serviceInstalled, os.Stdout) @@ -299,14 +297,16 @@ func runInitBody(o *initFlags) error { Enabled: *inferenceEnabled, Share: *inferenceShare, ModelID: *bundledModelID, - }, authKey) + }, authKey, renewing) case routeAgentDown, routeAgentAbsent: return daemonRequiredError(route, runtime.GOOS, serviceStartHintFn()) } - // routeLocal: explicitly-selected local enrollment (--bypass-mode / - // --google-sa-login / re-auth). Removed once the daemon login covers - // all three (#175). + // UNREACHABLE from here down: chooseEnrollRoute returns only the three + // routes above and every one of them returns. What follows is the + // standalone enrollment implementation, kept for one more PR so this + // one is only the behaviour change; #175's deletion PR removes it + // together with the `waired init` flags that exist solely to steer it. listenAddr, err := chooseListenAddr(*listen) if err != nil { return err diff --git a/docs-site/src/content/docs/ja/reference/cli.md b/docs-site/src/content/docs/ja/reference/cli.md index 79a917340..784ef7b55 100644 --- a/docs-site/src/content/docs/ja/reference/cli.md +++ b/docs-site/src/content/docs/ja/reference/cli.md @@ -5,7 +5,7 @@ meta: audience: ターミナルで作業する人、画面のないマシンを扱う人 needs: Waired がインストール済みであること time: 索引を眺めて、必要な節だけ読む -sourceHash: 994b51f9d79a3316 +sourceHash: 01fe115fc4a94fc9 --- このページの内容は、注記のあるもの以外すべて @@ -108,6 +108,12 @@ waired doctor --fix # 確認なしで修復(スクリプト・SSH サインインの状態と期限を表示し、更新が必要なら `init` の再実行を促します。 サービス用インストールでは `status` と同様に管理者権限が必要です。 +更新は最初に実行したのと同じ `waired init` です。このパソコンがすでにサインイン +済みであることを認識し、確認を取ったうえで、サインインだけを入れ替えます。 +設定も AI ソフトウェアも、ネットワーク上でのこのパソコンの位置づけもそのままで、 +端末一覧でも同じ端末のままです。サインインを保持しているのはバックグラウンドの +サービスなので、更新には Waired がバックグラウンドで動いている必要があります。 + ### `waired logout` このパソコンの識別情報と秘密を削除し、次の `waired init` が diff --git a/docs-site/src/content/docs/reference/cli.md b/docs-site/src/content/docs/reference/cli.md index 2e4afc28d..d872f94a6 100644 --- a/docs-site/src/content/docs/reference/cli.md +++ b/docs-site/src/content/docs/reference/cli.md @@ -109,6 +109,13 @@ waired doctor --fix # repair without asking (scripts, SSH) Shows the sign-in state and when it expires, and tells you to re-run `init` if it needs renewing. Needs elevation on a service install, like `status`. +Renewing is the same `waired init` you ran the first time — it recognises that +this computer is already signed in, confirms before continuing, and replaces +only the sign-in. Your settings, your AI software and this computer's place in +your network all stay as they are; it stays the same device on your device +list. Waired has to be running in the background for it to work, because the +background service is what holds the sign-in. + ### `waired logout` Removes this computer's identity and secrets, so the next `waired init` enrolls diff --git a/docs/decisions/20260727/2030-daemon-owns-reauthentication.md b/docs/decisions/20260727/2030-daemon-owns-reauthentication.md new file mode 100644 index 000000000..1a6e95340 --- /dev/null +++ b/docs/decisions/20260727/2030-daemon-owns-reauthentication.md @@ -0,0 +1,60 @@ +--- +status: accepted +--- + +# 再認証を daemon に移し、enrollment の分岐をなくす (20260727 20:30) + +## Status +Accepted + +## Context + +#175 は「daemon を経由しない enrollment が、capability を宣言しないデバイスを +作る」欠陥を消す作業。PR-1 (#271) が暗黙のフォールバックを、PR-2 (#290/#291) が +`--google-sa-login` と `--bypass-mode` を消した結果、`routeLocal` に到達する入口は +**再認証だけ**になっていた。 + +再認証とは、identity.json を持つホストで `waired init` を実行すること +(`waired auth status` が期限切れのときに案内する操作)。daemon 側の +`tokenRefresher` は refresh token が生きている間しかトークンを延長できず、 +`ReauthRequiredAt` が立った時点でループを止める。そこから先に進む手段が +local enrollment しかなかった。 + +Control Plane 側は調査の結果、既に対応済みだった: `EnrollDevice` は +machine key で既存デバイスを引き当てて `renewDeviceTx` で更新し +(`#115 Phase C`)、デバイス数上限からも再認証は除外されている。 +つまり必要なのは agent 側の配線だけで、proto 変更も CP のデプロイも要らない。 + +## Decision + +`LoginStartRequest` に `reauth` を足し、daemon の `loginController.Start` が +それを見たときだけ「登録済みなら何もしない」早期 return を迂回する。 +再認証後は live session が古い資格情報のまま動いているので、 +node key rotation が使っていた teardown→activate の再構築を関数として切り出し +(`rebuildSession`)、rotation と re-auth で同じ mutex を共有させる。 + +これで `chooseEnrollRoute` から `routeLocal` が消え、判断は +「agent がいるか」だけになった。`enrollFacts` も `serviceInstalled` 一つになる。 + +古い agent は `reauth` を無視して no-op ステータス(active・session id なし)を +返すため、CLI はそれを成功と読まずに「サービスが古いので更新できない」と +名指しで失敗する。 + +あわせて、この移行が CI で初めて踏んだ既存欠陥を直した: +`waitForBenchmark` が `disabled` / `stopped` を「エンジン起動済み」と誤読し、 +来ないモデルを 10 分待っていた(`waitForBundledModel` は同じ状態を既に終端として +扱っている)。installtest の各レグが 10 分ずつ無駄にしていた。 + +## Consequences + +- **daemon が動いていないホストでは再認証できなくなる。** これは #175 の設計判断 + そのもので、`routeAgentDown` / `routeAgentAbsent` がサービスの起動方法を案内する。 +- local enrollment の実装(`cmd/waired/main.go` の後半、`internal/setup/init.go`、 + `deploy.go` の一部、`waired init` のフラグ 6 本)は到達不能になった。 + 削除は次の PR で行い、この PR は振る舞いの変更だけに保つ。 +- `--auth-key` を登録済みホストで実行したときの「黙って成功を出す」挙動も + 同時に直った。 + +## Refs +- https://github.com/waired-ai/waired-agent/issues/175 +- docs/decisions/20260727/1900-auth-key-headless-enrollment.md diff --git a/internal/management/login.go b/internal/management/login.go index c89df91bf..b3e6bb12f 100644 --- a/internal/management/login.go +++ b/internal/management/login.go @@ -38,6 +38,18 @@ type LoginStartRequest struct { // guard already confines it to the local IPC socket (unix socket / // named pipe) — the key never crosses the TCP listener. AuthKey string `json:"auth_key,omitempty"` + // Reauth re-runs enrollment for a device that is ALREADY enrolled and + // active, replacing its tokens and certificate while keeping the same + // device (the control plane matches on the machine key and renews the + // row). Without it Start is an idempotent no-op on an active daemon, + // which is right for a repeated `waired init` but wrong for the one + // case that needs to reach the control plane again: re-auth (#175). + // + // An agent predating this field IGNORES it (handleLoginStart decodes + // leniently) and answers with the no-op status: active, and no session + // id. That is a version skew the CLI has to name rather than read as + // success — see runInitViaDaemon. + Reauth bool `json:"reauth,omitempty"` } // LoginStatus is returned by both /waired/v1/login/start and @@ -58,7 +70,9 @@ type LoginStatus struct { // (Tailscale-model: the daemon, not a spawned CLI, drives enrollment), // so the tray/CLI never need polkit elevation. Start is idempotent / // single-flight: a second Start while a login is in flight returns the -// existing session rather than spawning a second OAuth. +// existing session rather than spawning a second OAuth, and a Start on an +// already-active daemon reports active without re-enrolling — unless +// LoginStartRequest.Reauth asks for exactly that. type LoginController interface { // Start kicks off (or rejoins) a login session and returns its // current status. It returns quickly; the browser OAuth + device