From 2d03c0724fe14899955d577fa6bab0ab8e752884 Mon Sep 17 00:00:00 2001 From: Po-Han Shih Date: Fri, 2 Oct 2026 16:44:56 +0800 Subject: [PATCH 1/2] fix(auth): fail fast on non-TTY login and tighten cli.yaml to 0600 Browser OpenURL can return nil on headless Linux (DISPLAY set, Chromium starts, dbus dies) while WaitForToken blocks forever. ModeCharDevice is the wrong test because /dev/null is a char device; use term.IsTerminal. PersistentPreRunE now requires ZEABUR_TOKEN/--token when JSON, -i=false, or stdin is not a terminal, instead of starting the browser callback. GenerateToken always prints the URL and bounds WaitForToken. Also create ~/.config/zeabur/cli.yaml as 0600 (and chmod existing files) so the access token is not world-readable under a default umask. Related to #280 (survey hang) but a different bug path. --- go.mod | 2 +- internal/cmd/auth/login/login.go | 2 +- internal/cmd/root/root.go | 8 +++--- pkg/auth/browser.go | 18 ++++++++++++++ pkg/auth/browser_test.go | 15 ++++++++++++ pkg/auth/client.go | 16 +++++++++++- pkg/config/init.go | 18 +++++++++++--- pkg/config/init_test.go | 42 ++++++++++++++++++++++++++++++++ 8 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 pkg/auth/browser.go create mode 100644 pkg/auth/browser_test.go create mode 100644 pkg/config/init_test.go diff --git a/go.mod b/go.mod index 24acd81..3214652 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( go.uber.org/zap v1.28.0 golang.org/x/crypto v0.52.0 golang.org/x/oauth2 v0.36.0 + golang.org/x/term v0.43.0 ) require ( @@ -62,7 +63,6 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/net v0.54.0 // indirect golang.org/x/sys v0.45.0 // indirect - golang.org/x/term v0.43.0 // indirect golang.org/x/text v0.37.0 // indirect golang.org/x/tools v0.44.0 // indirect gopkg.in/yaml.v3 v3.0.1 diff --git a/internal/cmd/auth/login/login.go b/internal/cmd/auth/login/login.go index f6b9934..878699c 100644 --- a/internal/cmd/auth/login/login.go +++ b/internal/cmd/auth/login/login.go @@ -74,7 +74,7 @@ func RunLogin(f *cmdutil.Factory, opts *Options) error { err error ) - if f.Interactive { + if f.Interactive && auth.CanCompleteBrowserLogin() { f.Log.Info("A browser window will be opened for you to login, please confirm") // get token from web token, err := f.AuthClient.GenerateToken(context.Background()) diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index 581049b..8d963e7 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -79,9 +79,11 @@ func NewCmdRoot(f *cmdutil.Factory, version, commit, date string) (*cobra.Comman // require that the user is authenticated before running most commands if cmdutil.IsAuthCheckEnabled(cmd) { - // in JSON mode, fail fast if not authenticated instead of opening a browser - if f.JSON && !f.LoggedIn() { - return fmt.Errorf("not authenticated: run `zeabur auth login` before using --json") + // JSON, -i=false, non-TTY, and headless Linux cannot finish + // the browser callback. OpenURL can still "succeed" and then + // WaitForToken blocks forever. + if !f.LoggedIn() && (f.JSON || !f.Interactive || !auth.CanCompleteBrowserLogin()) { + return fmt.Errorf("not authenticated: set ZEABUR_TOKEN or use --token, or run `zeabur auth login` from a terminal with a browser") } // do not return error, guide user to login instead diff --git a/pkg/auth/browser.go b/pkg/auth/browser.go new file mode 100644 index 0000000..5f6fd4d --- /dev/null +++ b/pkg/auth/browser.go @@ -0,0 +1,18 @@ +package auth + +import ( + "os" + + "golang.org/x/term" +) + +// CanCompleteBrowserLogin reports whether we should start the implicit +// browser callback. /dev/null is a char device, so ModeCharDevice is the +// wrong test; term.IsTerminal is false for pipes and /dev/null. +// +// OpenURL can still return nil on this host (DISPLAY=:1, Chromium starts, +// dbus dies). Combined with a non-terminal stdin that is the hang: +// WaitForToken blocks forever. +func CanCompleteBrowserLogin() bool { + return term.IsTerminal(int(os.Stdin.Fd())) +} \ No newline at end of file diff --git a/pkg/auth/browser_test.go b/pkg/auth/browser_test.go new file mode 100644 index 0000000..b0007ab --- /dev/null +++ b/pkg/auth/browser_test.go @@ -0,0 +1,15 @@ +package auth + +import ( + "os" + "testing" + + "golang.org/x/term" +) + +func TestCanCompleteBrowserLogin_MatchesIsTerminal(t *testing.T) { + want := term.IsTerminal(int(os.Stdin.Fd())) + if got := CanCompleteBrowserLogin(); got != want { + t.Fatalf("CanCompleteBrowserLogin() = %v, term.IsTerminal(stdin) = %v", got, want) + } +} \ No newline at end of file diff --git a/pkg/auth/client.go b/pkg/auth/client.go index 1916ed0..3447b1f 100644 --- a/pkg/auth/client.go +++ b/pkg/auth/client.go @@ -7,6 +7,8 @@ import ( "encoding/hex" "fmt" "net/url" + "os" + "time" "github.com/cli/browser" ) @@ -60,11 +62,23 @@ func (c *ImplicitFlowClient) GenerateToken(ctx context.Context) (token string, e endpoint.RawQuery = query.Encode() + // Always print the URL. On a headless Linux box OpenURL can return nil + // after launching a broken Chromium, then WaitForToken blocks forever. + fmt.Fprintf(os.Stderr, "Open this URL to log in: %s\n", endpoint.String()) + + if !CanCompleteBrowserLogin() { + return "", fmt.Errorf("cannot complete browser login (stdin is not a terminal); set ZEABUR_TOKEN or use --token (url=%s)", endpoint.String()) + } + // Open the browser if err := browser.OpenURL(endpoint.String()); err != nil { return "", fmt.Errorf("failed to open browser (url=%s): %w", endpoint.String(), err) } + // Bound the wait so a "successful" OpenURL on a broken display cannot hang. + ctx, stopWait := context.WithTimeout(ctx, 2*time.Minute) + defer stopWait() + // Wait for the token tokenResponse, err := c.callbackServer.WaitForToken(ctx) if err != nil { @@ -85,4 +99,4 @@ func randomString(length int) (string, error) { return "", err } return hex.EncodeToString(b), nil -} +} \ No newline at end of file diff --git a/pkg/config/init.go b/pkg/config/init.go index 6e57896..175206e 100644 --- a/pkg/config/init.go +++ b/pkg/config/init.go @@ -41,11 +41,23 @@ func initViper(configPath string) { func createConfigFile(configPath string) { if _, err := os.Stat(configPath); os.IsNotExist(err) { - if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(configPath), 0o700); err != nil { panic(fmt.Errorf("could not create config directory: %w", err)) } - if _, err := os.Create(configPath); err != nil { + f, err := os.OpenFile(configPath, os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { panic(fmt.Errorf("could not create config file: %w", err)) } + if cerr := f.Close(); cerr != nil { + panic(fmt.Errorf("could not close config file: %w", cerr)) + } + return + } else if err != nil { + panic(fmt.Errorf("could not stat config file: %w", err)) } -} + // Existing files were created with os.Create (0666 & umask = 0644). + // The YAML holds the access token; tighten it on every start. + if err := os.Chmod(configPath, 0o600); err != nil { + panic(fmt.Errorf("could not restrict config file mode: %w", err)) + } +} \ No newline at end of file diff --git a/pkg/config/init_test.go b/pkg/config/init_test.go new file mode 100644 index 0000000..f25be69 --- /dev/null +++ b/pkg/config/init_test.go @@ -0,0 +1,42 @@ +package config + +import ( + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestCreateConfigFile_Mode600(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "zeabur", "cli.yaml") + if err := os.Chmod(dir, 0o755); err != nil { + t.Fatal(err) + } + createConfigFile(path) + st, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + // Windows ACLs ignore Unix mode bits; the OpenFile/Chmod path is still + // exercised above. Assert modes on POSIX only. + return + } + if got := st.Mode().Perm(); got != 0o600 { + t.Fatalf("new file mode = %o, want 0600", got) + } + + // Simulate the old os.Create umask hole, then start again. + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + createConfigFile(path) + st, err = os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := st.Mode().Perm(); got != 0o600 { + t.Fatalf("tightened file mode = %o, want 0600", got) + } +} \ No newline at end of file From 57ecb533af9e558e583a1df81c29fe98aadfae53 Mon Sep 17 00:00:00 2001 From: stantheman0128 Date: Sat, 3 Oct 2026 22:13:24 +0800 Subject: [PATCH 2/2] test: move auth and config checks into an external package --- pkg/auth/browser_test.go | 7 +++--- pkg/config/init_test.go | 51 ++++++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/pkg/auth/browser_test.go b/pkg/auth/browser_test.go index b0007ab..d970847 100644 --- a/pkg/auth/browser_test.go +++ b/pkg/auth/browser_test.go @@ -1,15 +1,16 @@ -package auth +package auth_test import ( "os" "testing" + "github.com/zeabur/cli/pkg/auth" "golang.org/x/term" ) func TestCanCompleteBrowserLogin_MatchesIsTerminal(t *testing.T) { want := term.IsTerminal(int(os.Stdin.Fd())) - if got := CanCompleteBrowserLogin(); got != want { + if got := auth.CanCompleteBrowserLogin(); got != want { t.Fatalf("CanCompleteBrowserLogin() = %v, term.IsTerminal(stdin) = %v", got, want) } -} \ No newline at end of file +} diff --git a/pkg/config/init_test.go b/pkg/config/init_test.go index f25be69..35161b2 100644 --- a/pkg/config/init_test.go +++ b/pkg/config/init_test.go @@ -1,42 +1,47 @@ -package config +package config_test import ( "os" "path/filepath" "runtime" "testing" + + "github.com/zeabur/cli/pkg/config" ) -func TestCreateConfigFile_Mode600(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "zeabur", "cli.yaml") - if err := os.Chmod(dir, 0o755); err != nil { - t.Fatal(err) +func assertUnixMode(t *testing.T, path string, want os.FileMode) { + t.Helper() + if runtime.GOOS == "windows" { + return } - createConfigFile(path) st, err := os.Stat(path) if err != nil { t.Fatal(err) } - if runtime.GOOS == "windows" { - // Windows ACLs ignore Unix mode bits; the OpenFile/Chmod path is still - // exercised above. Assert modes on POSIX only. - return - } - if got := st.Mode().Perm(); got != 0o600 { - t.Fatalf("new file mode = %o, want 0600", got) + if got := st.Mode().Perm(); got != want { + t.Fatalf("%s mode = %o, want %o", path, got, want) } +} + +func TestNew_ConfigFileMode600(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "zeabur", "cli.yaml") + cfg := config.New(path) + assertUnixMode(t, path, 0o600) - // Simulate the old os.Create umask hole, then start again. - if err := os.Chmod(path, 0o644); err != nil { + cfg.SetTokenString("secret-token") + if err := cfg.Write(); err != nil { t.Fatal(err) } - createConfigFile(path) - st, err = os.Stat(path) - if err != nil { + assertUnixMode(t, path, 0o600) +} + +func TestNew_TightensExisting0644(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "cli.yaml") + if err := os.WriteFile(path, []byte("token: old\n"), 0o644); err != nil { t.Fatal(err) } - if got := st.Mode().Perm(); got != 0o600 { - t.Fatalf("tightened file mode = %o, want 0600", got) - } -} \ No newline at end of file + _ = config.New(path) + assertUnixMode(t, path, 0o600) +}