diff --git a/cmd/root.go b/cmd/root.go index e61c8a6..58ad97f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -60,6 +60,14 @@ projects of your OpenProject instance.`, os.Exit(1) } + if insecure, mode := configuration.InsecureConfigPermissions(); insecure { + path := configuration.ConfigFilePath() + printer.Warning(fmt.Sprintf( + "config file %s is accessible by other users (mode %#o); it stores your API token. Run 'chmod 600 \"%s\"' to restrict access.", + path, mode, path, + )) + } + if host == "" && explicit { printer.Error(openerrors.Custom(fmt.Sprintf( "Profile %q not found. Run 'op login --profile %s' to create it.", diff --git a/components/configuration/filemode_test.go b/components/configuration/filemode_test.go index 89c02f6..7ebfa47 100644 --- a/components/configuration/filemode_test.go +++ b/components/configuration/filemode_test.go @@ -6,6 +6,7 @@ package configuration_test import ( "os" "path/filepath" + "runtime" "testing" "github.com/opf/openproject-cli/components/configuration" @@ -27,3 +28,49 @@ func TestWriteConfigForProfile_FileModeIs0600(t *testing.T) { t.Errorf("config file mode = %04o, want 0600 (token must not be world-readable)", mode) } } + +// A config written by the CLI (mode 0600) must not be reported as insecure. +func TestInsecureConfigPermissions_SecureFile(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "secret-token"); err != nil { + t.Fatal(err) + } + + if insecure, mode := configuration.InsecureConfigPermissions(); insecure { + t.Errorf("0600 config reported insecure (mode %#o)", mode) + } +} + +// A config readable by group or other users must be reported as insecure so the +// CLI can warn that the API token may leak. +func TestInsecureConfigPermissions_WorldReadableFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix permission bits are not meaningful on Windows") + } + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "secret-token"); err != nil { + t.Fatal(err) + } + if err := os.Chmod(configuration.ConfigFilePath(), 0644); err != nil { + t.Fatal(err) + } + + insecure, mode := configuration.InsecureConfigPermissions() + if !insecure { + t.Errorf("0644 config not reported insecure (mode %#o)", mode) + } + if mode.Perm() != 0644 { + t.Errorf("reported mode = %#o, want 0644", mode.Perm()) + } +} + +// A missing config file is not insecure. +func TestInsecureConfigPermissions_MissingFile(t *testing.T) { + setupTempConfig(t) + + if insecure, _ := configuration.InsecureConfigPermissions(); insecure { + t.Error("missing config file reported insecure") + } +} diff --git a/components/configuration/profiles.go b/components/configuration/profiles.go index 9bff0f1..fac9ef4 100644 --- a/components/configuration/profiles.go +++ b/components/configuration/profiles.go @@ -5,6 +5,7 @@ import ( "net/url" "os" "regexp" + "runtime" "sort" "strings" @@ -84,6 +85,31 @@ func DeleteProfile(profile string) error { return deleteProfile(profile) } +// ConfigFilePath returns the path of the config file. It honours +// $XDG_CONFIG_HOME/$HOME, so it is absolute only when those are. Exposed so +// callers can name the file in user-facing messages. +func ConfigFilePath() string { + return configFile() +} + +// InsecureConfigPermissions reports whether the config file exists with +// permissions that let group or other users access it. The file stores API +// tokens (and is written with mode 0600), so callers should warn the user when +// this returns true. The second value is the file's permission bits. Unix +// permission bits are not meaningful on Windows, so it always reports false +// there; a missing or unreadable file is likewise not reported as insecure. +func InsecureConfigPermissions() (insecure bool, mode os.FileMode) { + if runtime.GOOS == "windows" { + return false, 0 + } + info, err := os.Stat(configFile()) + if err != nil { + return false, 0 + } + mode = info.Mode().Perm() + return mode&0077 != 0, mode +} + // AllProfiles returns every profile stored in the config file. func AllProfiles() ([]*Profile, error) { if err := ensureConfigDir(); err != nil { diff --git a/components/printer/common.go b/components/printer/common.go index 7b3b33f..e3ef4d3 100644 --- a/components/printer/common.go +++ b/components/printer/common.go @@ -2,6 +2,7 @@ package printer import ( "encoding/json" + "fmt" "github.com/opf/openproject-cli/components/errors" ) @@ -20,6 +21,12 @@ func Input(prompt string) { activePrinter.Printf(prompt) } +// Warning writes a non-fatal diagnostic to standard error so it never corrupts +// machine-readable output (e.g. JSON) written to standard out. +func Warning(msg string) { + activePrinter.Eprintln(fmt.Sprintf("%s %s", Yellow("[WARNING]"), msg)) +} + func Done() { activePrinter.Println(Green("DONE")) } diff --git a/components/printer/console_printer.go b/components/printer/console_printer.go index a60002d..bf7c8e7 100644 --- a/components/printer/console_printer.go +++ b/components/printer/console_printer.go @@ -1,6 +1,9 @@ package printer -import "fmt" +import ( + "fmt" + "os" +) type ConsolePrinter struct{} @@ -11,3 +14,7 @@ func (printer *ConsolePrinter) Printf(format string, a ...any) (n int, err error func (printer *ConsolePrinter) Println(a ...any) (n int, err error) { return fmt.Println(a...) } + +func (printer *ConsolePrinter) Eprintln(a ...any) (n int, err error) { + return fmt.Fprintln(os.Stderr, a...) +} diff --git a/components/printer/printer.go b/components/printer/printer.go index 00f0c24..776acae 100644 --- a/components/printer/printer.go +++ b/components/printer/printer.go @@ -3,6 +3,9 @@ package printer type Printer interface { Printf(format string, a ...any) (n int, err error) Println(a ...any) (n int, err error) + // Eprintln writes a line to standard error, keeping diagnostics out of the + // machine-readable output written to standard out. + Eprintln(a ...any) (n int, err error) } var activePrinter Printer diff --git a/components/printer/testing_printer.go b/components/printer/testing_printer.go index faea265..82f2c31 100644 --- a/components/printer/testing_printer.go +++ b/components/printer/testing_printer.go @@ -18,6 +18,12 @@ func (printer *TestingPrinter) Println(a ...any) (n int, err error) { return len(printer.Result), nil } +func (printer *TestingPrinter) Eprintln(a ...any) (n int, err error) { + printer.Result += fmt.Sprintln(a...) + + return len(printer.Result), nil +} + func (printer *TestingPrinter) Reset() { printer.Result = "" } diff --git a/components/printer/warning_test.go b/components/printer/warning_test.go new file mode 100644 index 0000000..327b51d --- /dev/null +++ b/components/printer/warning_test.go @@ -0,0 +1,21 @@ +package printer_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" +) + +func TestWarning(t *testing.T) { + testingPrinter.Reset() + + printer.Warning("watch out") + + if !strings.Contains(testingPrinter.Result, "[WARNING]") { + t.Errorf("warning missing [WARNING] tag: %q", testingPrinter.Result) + } + if !strings.Contains(testingPrinter.Result, "watch out") { + t.Errorf("warning missing message: %q", testingPrinter.Result) + } +}