Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
47 changes: 47 additions & 0 deletions components/configuration/filemode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package configuration_test
import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/opf/openproject-cli/components/configuration"
Expand All @@ -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")
}
}
26 changes: 26 additions & 0 deletions components/configuration/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/url"
"os"
"regexp"
"runtime"
"sort"
"strings"

Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions components/printer/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package printer

import (
"encoding/json"
"fmt"

"github.com/opf/openproject-cli/components/errors"
)
Expand All @@ -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"))
}
Expand Down
9 changes: 8 additions & 1 deletion components/printer/console_printer.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package printer

import "fmt"
import (
"fmt"
"os"
)

type ConsolePrinter struct{}

Expand All @@ -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...)
}
3 changes: 3 additions & 0 deletions components/printer/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions components/printer/testing_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
}
21 changes: 21 additions & 0 deletions components/printer/warning_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}