diff --git a/cli/launcher/dialog_darwin.go b/cli/launcher/dialog_darwin.go index ea680c5..6f9d682 100644 --- a/cli/launcher/dialog_darwin.go +++ b/cli/launcher/dialog_darwin.go @@ -65,43 +65,23 @@ func alert(title, message string) error { return runOsascript(2*time.Minute, script) } -// alertWithCopy is alert() plus a button that puts secret on the clipboard. +// alertWithSecret shows a credential and puts it on the clipboard. // -// AppleScript cannot make a run of text clickable — `display dialog` draws one -// static string — so the click target has to be a button. That is the whole -// reason this is not simply "click the password". -// -// It re-shows the dialog after copying rather than dismissing. A password -// displayed once is exactly the thing someone reaches for a second time, and a -// window that disappears on the first attempt is how people end up reading -// credentials off a screenshot. The loop always terminates: every turn of it -// waits for a click. -func alertWithCopy(title, message, secret, copyLabel string) error { - body := message - for { - script := fmt.Sprintf( - `display dialog %s with title %s %s buttons {%s, "Done"} default button "Done"`, - quoteAS(body), quoteAS(title), iconClause(), quoteAS(copyLabel), - ) - out, err := outputOsascript(5*time.Minute, script) - if err != nil { - // Dismissing the window is a perfectly good way to say "I have it". - if errors.Is(err, errCancelled) { - return nil - } - return err - } - if !strings.Contains(out, copyLabel) { - return nil - } - - if err := copyToClipboard(secret); err != nil { - logf("could not copy to the clipboard: %v", err) - body = message + "\n\nCould not copy to the clipboard." - continue - } - body = message + "\n\nCopied to the clipboard." +// The copying happens before the window opens, not on a button, and the message +// says so. A button was tried and was worse: AppleScript cannot make a run of +// text clickable — `display dialog` is modal and returns only when it closes — +// so "copy" meant closing the window and opening it again, which on screen is +// a flash. Copying up front removes the flash and the click at once, and there +// is nothing to be coy about: this is a password the app generated seconds ago +// and is showing on purpose, and reaching for the clipboard is the next thing +// anyone does with it. +func alertWithSecret(title, message, secret, secretName string) error { + note := fmt.Sprintf("\n\nThe %s is on your clipboard.", secretName) + if err := copyToClipboard(secret); err != nil { + logf("could not copy the %s to the clipboard: %v", secretName, err) + note = fmt.Sprintf("\n\nThe %s could not be copied to your clipboard — select it above.", secretName) } + return alert(title, message+note) } // copyToClipboard pipes a value to pbcopy. diff --git a/cli/launcher/firstrun_darwin.go b/cli/launcher/firstrun_darwin.go index c9f403a..dcb41e8 100644 --- a/cli/launcher/firstrun_darwin.go +++ b/cli/launcher/firstrun_darwin.go @@ -226,11 +226,11 @@ func runFirstRun(u *updater) error { retireBootstrapPassword() } - return alertWithCopy("cix is set up", fmt.Sprintf( + return alertWithSecret("cix is set up", fmt.Sprintf( "Sign in at %s\n\nEmail:\n%s\n\nTemporary password:\n%s\n\n"+ "You will be asked to change this password on first login.%s%s", dashboardURL(vars), email, password, waitNote, cliNote), - password, "Copy Password") + password, "password") } // registerWithCLI adds (or updates) the local server in ~/.cix/config.yaml. diff --git a/cli/launcher/firstrun_darwin_test.go b/cli/launcher/firstrun_darwin_test.go index f78753d..0caa5c9 100644 --- a/cli/launcher/firstrun_darwin_test.go +++ b/cli/launcher/firstrun_darwin_test.go @@ -1,8 +1,10 @@ package main import ( + "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -158,3 +160,57 @@ func TestSetDefaultKeepsExistingChoices(t *testing.T) { t.Errorf("version check = %q, want the default filled in", vars["CIX_VERSION_CHECK_ENABLED"]) } } + +// When the server exits on a configuration it will not accept, its own words +// are the only useful thing to show — the menu otherwise says "Stopped" and +// nothing more, which is how a deleted database looked like a broken button. +func TestLastServerError(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + logs := filepath.Join(home, ".cix", "logs") + if err := os.MkdirAll(logs, 0o700); err != nil { + t.Fatal(err) + } + + t.Run("reports the tail", func(t *testing.T) { + body := "cix-server is ready\n\n\nbootstrap auth: incomplete bootstrap configuration:\n" + + " CIX_BOOTSTRAP_ADMIN_EMAIL is set but CIX_BOOTSTRAP_ADMIN_PASSWORD is empty.\n" + if err := os.WriteFile(filepath.Join(logs, "cix-server.err"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + got := lastServerError() + if !strings.Contains(got, "incomplete bootstrap configuration") { + t.Errorf("lastServerError() = %q, want the refusal in it", got) + } + // Blank lines carry nothing into a dialog. + if strings.Contains(got, "\n\n") { + t.Errorf("lastServerError() kept blank lines: %q", got) + } + }) + + t.Run("bounded", func(t *testing.T) { + var sb strings.Builder + for i := range 500 { + fmt.Fprintf(&sb, "line %d with a good deal of text after it so the cap is reached\n", i) + } + if err := os.WriteFile(filepath.Join(logs, "cix-server.err"), []byte(sb.String()), 0o644); err != nil { + t.Fatal(err) + } + got := lastServerError() + if n := len([]rune(got)); n > 901 { + t.Errorf("lastServerError() returned %d runes, want it capped", n) + } + if !strings.Contains(got, "line 499") { + t.Error("lastServerError() dropped the most recent line") + } + }) + + t.Run("no log at all", func(t *testing.T) { + if err := os.Remove(filepath.Join(logs, "cix-server.err")); err != nil { + t.Fatal(err) + } + if got := lastServerError(); got == "" { + t.Error("lastServerError() = \"\", want something to show the user") + } + }) +} diff --git a/cli/launcher/launchd_darwin.go b/cli/launcher/launchd_darwin.go index 7d92be5..ec79e06 100644 --- a/cli/launcher/launchd_darwin.go +++ b/cli/launcher/launchd_darwin.go @@ -331,3 +331,64 @@ func foreignAgent() bool { } return !strings.Contains(string(data), managedByMarker) } + +// serverDiedOnStart reports why the server is already gone after a Start we +// asked for, or "" when it is still alive. +// +// launchctl answers for having spawned the process, not for the process +// surviving it. A server that rejects its own configuration exits in +// milliseconds and leaves the menu reading "Stopped" with no explanation — +// indistinguishable from a Start button that does nothing. +// +// The wait is two-sided on purpose: a pid takes a moment to appear, and a +// process that is going to die does it almost at once. Neither half is a +// deadline for anything the user waits on — a healthy server returns from here +// as soon as it has a pid. +func serverDiedOnStart() string { + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) && launchdPID() == 0 { + time.Sleep(200 * time.Millisecond) + } + time.Sleep(2 * time.Second) + if launchdPID() != 0 { + return "" + } + return lastServerError() +} + +// lastServerError pulls the tail of cix-server.err, for a dialog. +// +// The server writes its refusals there as plain prose across several lines, so +// the last few non-empty ones are the message. Capped: an unbounded log tail in +// a modal dialog is its own failure. +func lastServerError() string { + dir, err := logDir() + if err != nil { + return "The server exited immediately." + } + data, err := os.ReadFile(filepath.Join(dir, "cix-server.err")) + if err != nil { + return "The server exited immediately." + } + + var lines []string + for line := range strings.SplitSeq(string(data), "\n") { + if strings.TrimSpace(line) != "" { + lines = append(lines, strings.TrimRight(line, " \t")) + } + } + if len(lines) == 0 { + return "The server exited immediately, without logging anything." + } + const keep = 8 + if len(lines) > keep { + lines = lines[len(lines)-keep:] + } + + out := strings.Join(lines, "\n") + const maxRunes = 900 + if r := []rune(out); len(r) > maxRunes { + out = "…" + string(r[len(r)-maxRunes:]) + } + return out +} diff --git a/cli/launcher/menu_darwin.go b/cli/launcher/menu_darwin.go index 76e92ee..cb3deed 100644 --- a/cli/launcher/menu_darwin.go +++ b/cli/launcher/menu_darwin.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" "os/exec" @@ -446,22 +447,55 @@ func (m *menu) toggleServer() { } defer m.endBusy() - var err error if s.State == stateRunning { - err = stopServer() - } else { - // Rewrite the wrapper before starting. It is generated, not edited, and - // something else may have replaced it — install-server.sh most obviously. - if err = writeLaunchdFiles(autostartEnabled()); err == nil { - err = startServer() + if err := stopServer(); err != nil { + _ = alert("cix", fmt.Sprintf("Could not stop the server.\n\n%v", err)) } + m.poll.refresh() + return } - if err != nil { - verb := "start" - if s.State == stateRunning { - verb = "stop" + + // A missing database is not something Start can fix. The server refuses to + // boot without an admin account to create — correctly — and says so in a + // log nobody has open, so the button appears to do nothing at all. Offer + // the thing that would actually help. + if needsFirstRun() { + ok, err := confirm("Set cix up again?", + "There is no cix database. If you deleted it, the server cannot start until an "+ + "administrator account is created again.\n\n"+ + "Setting up again creates a new account and a new, empty index. Anything that "+ + "was indexed before is already gone with the database.", + "Set Up") + if err != nil || !ok { + return } - _ = alert("cix", fmt.Sprintf("Could not %s the server.\n\n%v", verb, err)) + if err := runFirstRun(m.updater); err != nil && !errors.Is(err, errCancelled) { + logf("re-running setup failed: %v", err) + _ = alert("Setup failed", fmt.Sprintf("cix could not set itself up again.\n\n%v", err)) + } + m.poll.refresh() + return + } + + // Rewrite the wrapper before starting. It is generated, not edited, and + // something else may have replaced it — install-server.sh most obviously. + err := writeLaunchdFiles(autostartEnabled()) + if err == nil { + err = startServer() + } + if err != nil { + _ = alert("cix", fmt.Sprintf("Could not start the server.\n\n%v", err)) + return + } + + // launchctl reports success for having spawned the process, not for the + // process surviving. A server that exits on a configuration it cannot + // accept leaves the menu saying "Stopped" and nothing else — which is how + // a deleted database looked like a broken Start button. + if detail := serverDiedOnStart(); detail != "" { + logf("the server exited immediately after Start: %s", detail) + _ = alert("The server stopped straight away", detail+ + "\n\nThe full log is in ~/.cix/logs/cix-server.err.") } m.poll.refresh() } diff --git a/cli/launcher/resetpw_darwin.go b/cli/launcher/resetpw_darwin.go index f37ce3c..578d558 100644 --- a/cli/launcher/resetpw_darwin.go +++ b/cli/launcher/resetpw_darwin.go @@ -70,11 +70,11 @@ func (m *menu) resetPasswordFlow() { return } - _ = alertWithCopy("Password reset", fmt.Sprintf( + _ = alertWithSecret("Password reset", fmt.Sprintf( "Account:\n%s\n\nTemporary password:\n%s\n\n"+ "You will be asked to change it at the next sign-in. Other sessions for this "+ "account have been signed out.", email, password), - password, "Copy Password") + password, "password") } // runResetPassword executes the reset and returns the generated password.