From ce95835ea3e904ce23b02dfbc7a322d5e26216d8 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 13:15:58 -0700 Subject: [PATCH 01/12] Add credstore package for keychain-backed credential storage Introduces internal/credstore, a credential storage abstraction for the LFX CLI backed by github.com/99designs/keyring: - Credentials (refresh token, cached access token + expiry) are stored in the system keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet) by default, with keyring's own passphrase-encrypted file backend used automatically when no system keychain is available. - When --insecure-storage is passed, the system keychain is bypassed entirely in favor of a plain (unencrypted), owner-only (0600) JSON file, for headless/CI use where a passphrase prompt is unacceptable. - Non-sensitive DeviceState (device ID, IdP domain) is always stored as plain JSON under the XDG state directory (~/.local/state/lfx-cli/ by default), per XDG Base Directory conventions. Wires the shared --insecure-storage flag onto the `lfx auth` command group; the login/token/status/logout actions remain stubs pending LFXV2-2515 and LFXV2-2516. Verified locally on macOS: real Keychain round-trip (save/load/delete, no passphrase prompt) and plain-file fallback round-trip both pass. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- go.mod | 9 + go.sum | 33 ++++ internal/commands/auth.go | 23 ++- internal/credstore/credstore.go | 329 ++++++++++++++++++++++++++++++++ 4 files changed, 391 insertions(+), 3 deletions(-) create mode 100644 internal/credstore/credstore.go diff --git a/go.mod b/go.mod index 991cf0b..b1ede7c 100644 --- a/go.mod +++ b/go.mod @@ -5,11 +5,20 @@ module github.com/linuxfoundation/lfx-cli go 1.26.5 require ( + github.com/99designs/keyring v1.2.2 github.com/urfave/cli-docs/v3 v3.1.0 github.com/urfave/cli/v3 v3.10.1 ) require ( + github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect + github.com/danieljoos/wincred v1.1.2 // indirect + github.com/dvsekhvalnov/jose2go v1.5.0 // indirect + github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect + github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect + github.com/mtibben/percent v0.2.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect + golang.org/x/sys v0.3.0 // indirect + golang.org/x/term v0.3.0 // indirect ) diff --git a/go.sum b/go.sum index b0ce4fa..51198c7 100644 --- a/go.sum +++ b/go.sum @@ -1,16 +1,49 @@ +github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMbk2FiG/kXiLl8BRyzTWDw7gX/Hz7Dd5eDMs= +github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= +github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= +github.com/99designs/keyring v1.2.2/go.mod h1:wes/FrByc8j7lFOAGLGSNEg8f/PaI3cgTBqhFkHUrPk= github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0= +github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dvsekhvalnov/jose2go v1.5.0 h1:3j8ya4Z4kMCwT5nXIKFSV84YS+HdqSSO0VsTQxaLAeM= +github.com/dvsekhvalnov/jose2go v1.5.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= +github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= +github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= +github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c h1:6rhixN/i8ZofjG1Y75iExal34USq5p+wiN1tpie8IrU= +github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c/go.mod h1:NMPJylDgVpX0MLRlPy15sqSwOFv/U1GZ2m21JhFfek0= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/mtibben/percent v0.2.1 h1:5gssi8Nqo8QU/r2pynCm+hBQHpkB/uNK7BJCFogWdzs= +github.com/mtibben/percent v0.2.1/go.mod h1:KG9uO+SZkUp+VkRHsCdYQV3XSZrrSpR3O9ibNBTZrns= +github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= +github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.3.0 h1:NGXK3lHquSN08v5vWalVI/L8XU9hdzE/G6xsrze47As= +github.com/stretchr/objx v0.3.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli-docs/v3 v3.1.0 h1:Sa5xm19IpE5gpm6tZzXdfjdFxn67PnEsE4dpXF7vsKw= github.com/urfave/cli-docs/v3 v3.1.0/go.mod h1:59d+5Hz1h6GSGJ10cvcEkbIe3j233t4XDqI72UIx7to= github.com/urfave/cli/v3 v3.10.1 h1:7Kx9H50hrHbRbyxgO1KP6/BcbiGRz0uYh5YyQ30JEEY= github.com/urfave/cli/v3 v3.10.1/go.mod h1:ysVLtOEmg2tOy6PknnYVhDoouyC/6N42TMeoMzskhso= +golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= +golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI= +golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= +gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 2092839..407629c 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -11,15 +11,32 @@ import ( "github.com/urfave/cli/v3" ) +// insecureStorageFlagName is the auth command group's flag controlling +// whether credentials bypass the system keychain in favor of a plain, +// unencrypted file. +const insecureStorageFlagName = "insecure-storage" + // NewAuthCommand builds the `lfx auth` command group with its subcommands. // -// All subcommands are currently stubs; real implementations land in -// LFXV2-2513 (Auth0 client), LFXV2-2514 (keychain storage), LFXV2-2515 -// (login flow), and LFXV2-2516 (token command). +// The --insecure-storage flag is shared by all subcommands and controls +// whether credentials bypass the system keychain in favor of credstore's +// plain (unencrypted) file fallback, e.g. for headless/CI use. +// +// The login, token, status, and logout actions are currently stubs; real +// implementations land in LFXV2-2515 (login flow) and LFXV2-2516 (token +// command), at which point they'll build a credstore.Store via +// credstore.New(credstore.Options{Insecure: +// cmd.Bool(insecureStorageFlagName)}). func NewAuthCommand() *cli.Command { return &cli.Command{ Name: "auth", Usage: "Manage authentication with the LFX platform", + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: insecureStorageFlagName, + Usage: "Store credentials in a plain (unencrypted) file instead of the system keychain", + }, + }, Commands: []*cli.Command{ newAuthLoginCommand(), newAuthTokenCommand(), diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go new file mode 100644 index 0000000..87b77ed --- /dev/null +++ b/internal/credstore/credstore.go @@ -0,0 +1,329 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +// Package credstore provides secure storage for LFX CLI credentials and +// non-sensitive device state. +// +// Secrets (refresh token, cached access token and its expiry) are stored in +// the operating system's credential store via github.com/99designs/keyring +// (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet). +// When no system keychain is available, keyring automatically falls back to +// its own passphrase-encrypted file backend. +// +// When --insecure-storage is passed explicitly, the system keychain is +// bypassed entirely in favor of a plain (unencrypted), owner-only (0600) +// JSON file under the state directory. This is intended for headless/CI use +// where no passphrase prompt is acceptable, and is deliberately less secure +// than the keyring-backed storage. +// +// Non-sensitive state (device ID, IdP domain used at login) is always stored +// as plain JSON under the XDG state directory (~/.local/state/lfx-cli/ by +// default), per XDG Base Directory conventions for mutable runtime state. +package credstore + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/99designs/keyring" +) + +// ErrNotFound is returned by Load methods when no value has been stored yet. +var ErrNotFound = errors.New("credstore: not found") + +// serviceName identifies this application's entries within shared credential +// stores (e.g. the macOS Keychain "service" attribute). +const serviceName = "lfx-cli" + +// credentialsKey is the keyring item key under which Credentials are stored. +const credentialsKey = "credentials" + +// stateFileName is the name of the plain-JSON file holding non-secret device +// state within the state directory. +const stateFileName = "state.json" + +// insecureCredentialsFileName is the name of the plain (unencrypted) JSON +// file used to store credentials when --insecure-storage bypasses the +// system keychain. +const insecureCredentialsFileName = "credentials.json" + +// Credentials holds the secrets needed to authenticate with the LFX +// platform: the long-lived Auth0 refresh token, and an optional cached +// access token with its expiry. +type Credentials struct { + RefreshToken string `json:"refresh_token"` + AccessToken string `json:"access_token,omitempty"` + AccessTokenExpiry time.Time `json:"access_token_expiry,omitempty"` +} + +// ValidAccessToken reports whether the cached access token is present and +// not yet expired, allowing for a small clock-skew buffer. +func (c Credentials) ValidAccessToken() bool { + const skew = 30 * time.Second + return c.AccessToken != "" && time.Now().Add(skew).Before(c.AccessTokenExpiry) +} + +// DeviceState holds non-sensitive information persisted between CLI +// invocations so that commands like `lfx auth token` don't need to +// re-specify the IdP domain used at login. +type DeviceState struct { + DeviceID string `json:"device_id"` + IDPDomain string `json:"idp_domain,omitempty"` +} + +// Store is the credential storage abstraction used by the auth commands. +type Store interface { + // SaveCredentials persists secrets to the system keychain (or the file + // fallback). + SaveCredentials(creds Credentials) error + // LoadCredentials returns the persisted secrets, or ErrNotFound if none + // have been saved. + LoadCredentials() (Credentials, error) + // DeleteCredentials removes any persisted secrets. It is a no-op if + // none exist. + DeleteCredentials() error + + // SaveDeviceState persists non-sensitive device state as plain JSON. + SaveDeviceState(state DeviceState) error + // LoadDeviceState returns the persisted device state, or ErrNotFound if + // none has been saved. + LoadDeviceState() (DeviceState, error) +} + +// Options configures a Store returned by New. +type Options struct { + // Insecure bypasses the system keychain (and its encrypted-file + // fallback) in favor of a plain, owner-only (0600) JSON file. Intended + // for headless/CI use where a passphrase prompt is unacceptable. + // Corresponds to the CLI's --insecure-storage flag. + Insecure bool + + // StateDir overrides the computed state directory. Intended for tests; + // leave empty to use $XDG_STATE_HOME/lfx-cli (or ~/.local/state/lfx-cli + // if $XDG_STATE_HOME is unset). + StateDir string +} + +// secretsBackend abstracts over the two ways Credentials can be persisted: +// the system keychain (via keyring.Keyring), or a plain file when insecure +// storage is requested. +type secretsBackend interface { + Save(creds Credentials) error + Load() (Credentials, error) + Delete() error +} + +// store is the default Store implementation, backed by a secretsBackend for +// credentials and a plain JSON file for non-secret device state. +type store struct { + secrets secretsBackend + stateDir string +} + +// New builds a Store using the given Options. +func New(opts Options) (Store, error) { + stateDir, err := resolveStateDir(opts.StateDir) + if err != nil { + return nil, fmt.Errorf("credstore: resolve state dir: %w", err) + } + + var secrets secretsBackend + if opts.Insecure { + secrets = &plainFileSecrets{ + path: filepath.Join(stateDir, insecureCredentialsFileName), + } + } else { + kr, err := keyring.Open(keyring.Config{ + ServiceName: serviceName, + // Encrypted-file fallback used automatically by keyring when no + // system keychain is available; the file is protected by a + // passphrase collected via a terminal prompt. + FileDir: filepath.Join(stateDir, "credentials"), + FilePasswordFunc: keyring.TerminalPrompt, + KeychainTrustApplication: true, + }) + if err != nil { + return nil, fmt.Errorf("credstore: open keyring: %w", err) + } + secrets = &keyringSecrets{keyring: kr} + } + + return &store{secrets: secrets, stateDir: stateDir}, nil +} + +// resolveStateDir determines the directory used for non-secret device state +// and the file-backend fallback, creating it if necessary. +func resolveStateDir(override string) (string, error) { + dir := override + if dir == "" { + if xdgState := os.Getenv("XDG_STATE_HOME"); xdgState != "" { + dir = filepath.Join(xdgState, "lfx-cli") + } else { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("determine home directory: %w", err) + } + dir = filepath.Join(home, ".local", "state", "lfx-cli") + } + } + + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", fmt.Errorf("create state directory %q: %w", dir, err) + } + + return dir, nil +} + +// SaveCredentials implements Store. +func (s *store) SaveCredentials(creds Credentials) error { + return s.secrets.Save(creds) +} + +// LoadCredentials implements Store. +func (s *store) LoadCredentials() (Credentials, error) { + return s.secrets.Load() +} + +// DeleteCredentials implements Store. +func (s *store) DeleteCredentials() error { + return s.secrets.Delete() +} + +// SaveDeviceState implements Store. +func (s *store) SaveDeviceState(state DeviceState) error { + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return fmt.Errorf("credstore: marshal device state: %w", err) + } + + path := filepath.Join(s.stateDir, stateFileName) + if err := os.WriteFile(path, data, 0o600); err != nil { + return fmt.Errorf("credstore: write device state: %w", err) + } + + return nil +} + +// LoadDeviceState implements Store. +func (s *store) LoadDeviceState() (DeviceState, error) { + path := filepath.Join(s.stateDir, stateFileName) + + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return DeviceState{}, ErrNotFound + } + if err != nil { + return DeviceState{}, fmt.Errorf("credstore: read device state: %w", err) + } + + var state DeviceState + if err := json.Unmarshal(data, &state); err != nil { + return DeviceState{}, fmt.Errorf("credstore: unmarshal device state: %w", err) + } + + return state, nil +} + +// keyringSecrets is a secretsBackend that stores Credentials in the system +// keychain via keyring.Keyring, falling back to keyring's own +// passphrase-encrypted file backend when no system keychain is available. +type keyringSecrets struct { + keyring keyring.Keyring +} + +func (k *keyringSecrets) Save(creds Credentials) error { + data, err := json.Marshal(creds) + if err != nil { + return fmt.Errorf("credstore: marshal credentials: %w", err) + } + + err = k.keyring.Set(keyring.Item{ + Key: credentialsKey, + Data: data, + Label: "LFX CLI credentials", + Description: "Refresh and access tokens for the LFX platform", + }) + if err != nil { + return fmt.Errorf("credstore: save credentials: %w", err) + } + + return nil +} + +func (k *keyringSecrets) Load() (Credentials, error) { + item, err := k.keyring.Get(credentialsKey) + if errors.Is(err, keyring.ErrKeyNotFound) { + return Credentials{}, ErrNotFound + } + if err != nil { + return Credentials{}, fmt.Errorf("credstore: load credentials: %w", err) + } + + var creds Credentials + if err := json.Unmarshal(item.Data, &creds); err != nil { + return Credentials{}, fmt.Errorf("credstore: unmarshal credentials: %w", err) + } + + return creds, nil +} + +func (k *keyringSecrets) Delete() error { + err := k.keyring.Remove(credentialsKey) + if err != nil && !errors.Is(err, keyring.ErrKeyNotFound) { + return fmt.Errorf("credstore: delete credentials: %w", err) + } + + return nil +} + +// plainFileSecrets is a secretsBackend that stores Credentials as a plain +// (unencrypted), owner-only (0600) JSON file. Used when --insecure-storage +// is passed, bypassing the system keychain entirely so that no passphrase +// prompt is ever required (e.g. for headless/CI use). This is deliberately +// less secure than keyringSecrets. +type plainFileSecrets struct { + path string +} + +func (p *plainFileSecrets) Save(creds Credentials) error { + data, err := json.MarshalIndent(creds, "", " ") + if err != nil { + return fmt.Errorf("credstore: marshal credentials: %w", err) + } + + if err := os.WriteFile(p.path, data, 0o600); err != nil { + return fmt.Errorf("credstore: write credentials: %w", err) + } + + return nil +} + +func (p *plainFileSecrets) Load() (Credentials, error) { + data, err := os.ReadFile(p.path) + if errors.Is(err, os.ErrNotExist) { + return Credentials{}, ErrNotFound + } + if err != nil { + return Credentials{}, fmt.Errorf("credstore: read credentials: %w", err) + } + + var creds Credentials + if err := json.Unmarshal(data, &creds); err != nil { + return Credentials{}, fmt.Errorf("credstore: unmarshal credentials: %w", err) + } + + return creds, nil +} + +func (p *plainFileSecrets) Delete() error { + err := os.Remove(p.path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("credstore: delete credentials: %w", err) + } + + return nil +} From 257e2acc69ad232e83669753b2f5dba6eb287394 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 14:11:54 -0700 Subject: [PATCH 02/12] Address review feedback: drop automatic file-backend fallback - Restrict keyring.Open to real system credential stores (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet/pass), dropping keyring's automatic passphrase-encrypted file fallback. If none of those backends are available, New now returns an error directing the user to --insecure-storage instead of silently falling back to a file. - Add defensive os.ErrNotExist handling in keyringSecrets.Delete, since some backends (e.g. pass) may surface that instead of keyring.ErrKeyNotFound for a missing key. - Reject relative $XDG_STATE_HOME per the XDG Base Directory Specification, falling back to ~/.local/state/lfx-cli instead of resolving state paths relative to the process's working directory. - Document --insecure-storage usage in the README. Addresses Copilot review comments on PR #3. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- README.md | 11 +++++++ internal/credstore/credstore.go | 54 ++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index feff76f..7a420a2 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,17 @@ lfx auth logout lfx api ``` +Credentials (refresh token, cached access token) are stored in your +operating system's credential store by default (macOS Keychain, Windows +Credential Manager, Linux Secret Service/KWallet/`pass`). Pass +`--insecure-storage` to any `auth` subcommand to instead store credentials +in a plain, unencrypted, owner-only file, at the cost of weaker protection +for the stored tokens: + +```bash +lfx auth login --insecure-storage +``` + Run `lfx --help` or `lfx --help` for full details on any command. > **Note:** This project is under active development. Authentication and API diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 87b77ed..c14c30f 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -5,10 +5,13 @@ // non-sensitive device state. // // Secrets (refresh token, cached access token and its expiry) are stored in -// the operating system's credential store via github.com/99designs/keyring -// (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet). -// When no system keychain is available, keyring automatically falls back to -// its own passphrase-encrypted file backend. +// the operating system's credential store via github.com/99designs/keyring: +// macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet, +// or the `pass` password store. keyring's own encrypted-file backend is +// deliberately excluded from this list: it isn't a real system keychain, and +// its Remove behavior doesn't match keyring.ErrKeyNotFound (see +// keyringSecrets.Delete). If none of the allowed backends are available, +// New returns an error rather than silently falling back to a file. // // When --insecure-storage is passed explicitly, the system keychain is // bypassed entirely in favor of a plain (unencrypted), owner-only (0600) @@ -51,6 +54,21 @@ const stateFileName = "state.json" // system keychain. const insecureCredentialsFileName = "credentials.json" +// systemBackends is the set of keyring backends considered real system +// credential stores. Notably excludes keyring.FileBackend: it isn't backed +// by a real system keychain, and its Remove behavior on a missing key +// doesn't match keyring.ErrKeyNotFound (see keyringSecrets.Delete). Explicit +// file-based storage is only available via --insecure-storage +// (plainFileSecrets), never as an automatic fallback. +var systemBackends = []keyring.BackendType{ + keyring.SecretServiceBackend, + keyring.KeychainBackend, + keyring.KeyCtlBackend, + keyring.KWalletBackend, + keyring.WinCredBackend, + keyring.PassBackend, +} + // Credentials holds the secrets needed to authenticate with the LFX // platform: the long-lived Auth0 refresh token, and an optional cached // access token with its expiry. @@ -138,16 +156,12 @@ func New(opts Options) (Store, error) { } } else { kr, err := keyring.Open(keyring.Config{ - ServiceName: serviceName, - // Encrypted-file fallback used automatically by keyring when no - // system keychain is available; the file is protected by a - // passphrase collected via a terminal prompt. - FileDir: filepath.Join(stateDir, "credentials"), - FilePasswordFunc: keyring.TerminalPrompt, + ServiceName: serviceName, + AllowedBackends: systemBackends, KeychainTrustApplication: true, }) if err != nil { - return nil, fmt.Errorf("credstore: open keyring: %w", err) + return nil, fmt.Errorf("credstore: open keyring (use --insecure-storage as a fallback): %w", err) } secrets = &keyringSecrets{keyring: kr} } @@ -156,11 +170,14 @@ func New(opts Options) (Store, error) { } // resolveStateDir determines the directory used for non-secret device state -// and the file-backend fallback, creating it if necessary. +// and the insecure-storage credentials file, creating it if necessary. func resolveStateDir(override string) (string, error) { dir := override if dir == "" { - if xdgState := os.Getenv("XDG_STATE_HOME"); xdgState != "" { + // XDG Base Directory Specification requires $XDG_STATE_HOME to be an + // absolute path; per spec, relative values (and thus the variable) + // must be ignored, falling back to the documented default. + if xdgState := os.Getenv("XDG_STATE_HOME"); filepath.IsAbs(xdgState) { dir = filepath.Join(xdgState, "lfx-cli") } else { home, err := os.UserHomeDir() @@ -228,9 +245,8 @@ func (s *store) LoadDeviceState() (DeviceState, error) { return state, nil } -// keyringSecrets is a secretsBackend that stores Credentials in the system -// keychain via keyring.Keyring, falling back to keyring's own -// passphrase-encrypted file backend when no system keychain is available. +// keyringSecrets is a secretsBackend that stores Credentials in a real +// system credential store via keyring.Keyring (see systemBackends). type keyringSecrets struct { keyring keyring.Keyring } @@ -273,7 +289,11 @@ func (k *keyringSecrets) Load() (Credentials, error) { func (k *keyringSecrets) Delete() error { err := k.keyring.Remove(credentialsKey) - if err != nil && !errors.Is(err, keyring.ErrKeyNotFound) { + // Most backends return keyring.ErrKeyNotFound for a missing key, but + // some (e.g. the pass backend, which shells out to files on disk) may + // instead surface an os.ErrNotExist-wrapping error; treat both as a + // successful no-op per the Store.DeleteCredentials contract. + if err != nil && !errors.Is(err, keyring.ErrKeyNotFound) && !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("credstore: delete credentials: %w", err) } From 52dd4101017df9ad6d9fed76ff0efe431d3a15b4 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 14:11:57 -0700 Subject: [PATCH 03/12] Document intentional use of fmt over slog for CLI output Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- AGENTS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 9755211..224c9c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,6 +20,11 @@ CLI (`lfx auth login` → `lfx auth token`). for LLM/agent-friendly Markdown reference docs (`lfx docs`) - **Release automation**: [GoReleaser](https://goreleaser.com/) for multi-arch binary builds published to GitHub Releases +- **Output**: user-facing command output uses plain `fmt.Println`/ + `fmt.Fprintln`, not `log/slog`. This is intentional: unlike the + JSON-structured `slog` logging convention used by LFX's long-running + services, `lfx` is an interactive CLI with no log aggregator consuming + its output. ## Architecture Overview From 98aa85796f1b7df10cfe14418f48dd8865795cd6 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 14:17:05 -0700 Subject: [PATCH 04/12] Update dependencies and document go mod tidy as a PR prerequisite Run go get -u ./... && go mod tidy, picking up minor version bumps for transitive keyring dependencies (wincred, jose2go, x/sys, x/term, go-md2man). Also documents this as a required step before every PR in AGENTS.md's Contributing Guidelines. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- AGENTS.md | 6 ++++-- go.mod | 10 +++++----- go.sum | 30 ++++++++++++------------------ 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 224c9c5..2bdcda0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -211,5 +211,7 @@ release binaries may be missing even though the GitHub Release exists. the established pattern 2. **Package Comments**: Every new `*.go` file must include the same `// Package ...` doc comment as the rest of its package -3. **Code Quality**: Run `make check` before commits -4. **Documentation**: Update README.md for user-facing changes +3. **Dependencies**: Run `go get -u ./... && go mod tidy` before every PR to + keep dependencies current +4. **Code Quality**: Run `make check` before commits +5. **Documentation**: Update README.md for user-facing changes diff --git a/go.mod b/go.mod index b1ede7c..7199b68 100644 --- a/go.mod +++ b/go.mod @@ -12,13 +12,13 @@ require ( require ( github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect - github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect - github.com/danieljoos/wincred v1.1.2 // indirect - github.com/dvsekhvalnov/jose2go v1.5.0 // indirect + github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect + github.com/danieljoos/wincred v1.2.3 // indirect + github.com/dvsekhvalnov/jose2go v1.8.0 // indirect github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect github.com/mtibben/percent v0.2.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect - golang.org/x/sys v0.3.0 // indirect - golang.org/x/term v0.3.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/term v0.45.0 // indirect ) diff --git a/go.sum b/go.sum index 51198c7..77c7dc3 100644 --- a/go.sum +++ b/go.sum @@ -2,15 +2,14 @@ github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMb github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= github.com/99designs/keyring v1.2.2/go.mod h1:wes/FrByc8j7lFOAGLGSNEg8f/PaI3cgTBqhFkHUrPk= -github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0= -github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo= +github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= +github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dvsekhvalnov/jose2go v1.5.0 h1:3j8ya4Z4kMCwT5nXIKFSV84YS+HdqSSO0VsTQxaLAeM= -github.com/dvsekhvalnov/jose2go v1.5.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= +github.com/dvsekhvalnov/jose2go v1.8.0 h1:LqkkVKAlHFfH9LOEl5fe4p/zL02OhWE7pCufMBG2jLA= +github.com/dvsekhvalnov/jose2go v1.8.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c h1:6rhixN/i8ZofjG1Y75iExal34USq5p+wiN1tpie8IrU= @@ -26,24 +25,19 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.3.0 h1:NGXK3lHquSN08v5vWalVI/L8XU9hdzE/G6xsrze47As= -github.com/stretchr/objx v0.3.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/urfave/cli-docs/v3 v3.1.0 h1:Sa5xm19IpE5gpm6tZzXdfjdFxn67PnEsE4dpXF7vsKw= github.com/urfave/cli-docs/v3 v3.1.0/go.mod h1:59d+5Hz1h6GSGJ10cvcEkbIe3j233t4XDqI72UIx7to= github.com/urfave/cli/v3 v3.10.1 h1:7Kx9H50hrHbRbyxgO1KP6/BcbiGRz0uYh5YyQ30JEEY= github.com/urfave/cli/v3 v3.10.1/go.mod h1:ysVLtOEmg2tOy6PknnYVhDoouyC/6N42TMeoMzskhso= -golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI= -golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U= gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 57a78443de5d2ec73a568fed80d763ae6da0854c Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 14:49:30 -0700 Subject: [PATCH 05/12] Allowlist recently introduced words in cspell config Fixes cspell warnings from make megalinter for words introduced by the credstore package and existing Makefile content: credstore, fprintln, gobin, gopath, mgechev, esac. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- .cspell.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.cspell.json b/.cspell.json index 9a578de..8f1edcf 100644 --- a/.cspell.json +++ b/.cspell.json @@ -8,10 +8,15 @@ "clidocs", "coverprofile", "cpuprof", + "credstore", "ehthumbs", + "esac", + "fprintln", "goarch", + "gobin", "gofmt", "golangci", + "gopath", "goreleaser", "kics", "ldflags", @@ -19,6 +24,7 @@ "lfxv", "linuxfoundation", "memprof", + "mgechev", "techdocs", "urfave", "zizmor" From 693a5eca6856d7e97be72616f943440222cd90ee Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 14:50:14 -0700 Subject: [PATCH 06/12] Fix stale docs, permission preservation, and backend config issues Independently verified each of the following before fixing: - os.WriteFile only applies its mode argument when creating a new file, silently preserving the mode of one that already exists. A pre-existing 0644 credentials.json (or state.json) therefore stayed world-readable even after writing "owner-only" data. Added writeOwnerOnlyFile, which opens with O_TRUNC and explicitly chmods 0600 regardless of prior state, and use it in both plainFileSecrets.Save and SaveDeviceState. Verified with a standalone repro: a pre-existing 0644 file now ends up 0600 after a write. - keyring.KeyCtlBackend's opener calls getKeyringForScope(cfg.KeyCtlScope); with the zero-value scope this package leaves unconfigured, that always errors, and keyring.Open silently skips the backend on error. Confirmed against the vendored keyring v1.2.2 source. Removed it from systemBackends since it can never actually open. - keyring's pass backend namespaces entries via Config.PassPrefix, not ServiceName (confirmed against pass.go). Without it, our credentialsKey would be stored as a generic top-level "credentials" entry in the user's password store, risking collisions with unrelated pass entries. Set PassPrefix to serviceName. - Fixed stale doc comments in Store.SaveCredentials and Options.Insecure still describing an encrypted-file fallback that no longer exists. Re-verified the real macOS Keychain round-trip locally after these changes. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/credstore/credstore.go | 63 +++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index c14c30f..7ff67f1 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -55,15 +55,19 @@ const stateFileName = "state.json" const insecureCredentialsFileName = "credentials.json" // systemBackends is the set of keyring backends considered real system -// credential stores. Notably excludes keyring.FileBackend: it isn't backed -// by a real system keychain, and its Remove behavior on a missing key -// doesn't match keyring.ErrKeyNotFound (see keyringSecrets.Delete). Explicit -// file-based storage is only available via --insecure-storage -// (plainFileSecrets), never as an automatic fallback. +// credential stores. Notably excludes: +// - keyring.FileBackend: it isn't backed by a real system keychain, and +// its Remove behavior on a missing key doesn't match +// keyring.ErrKeyNotFound (see keyringSecrets.Delete). Explicit +// file-based storage is only available via --insecure-storage +// (plainFileSecrets), never as an automatic fallback. +// - keyring.KeyCtlBackend: its opener requires a non-empty Config.KeyCtlScope +// ("user", "session", "process", or "thread"); with the zero value we +// leave it at, Open always fails to open this backend and silently +// skips it, so including it here would be a no-op. var systemBackends = []keyring.BackendType{ keyring.SecretServiceBackend, keyring.KeychainBackend, - keyring.KeyCtlBackend, keyring.KWalletBackend, keyring.WinCredBackend, keyring.PassBackend, @@ -95,8 +99,8 @@ type DeviceState struct { // Store is the credential storage abstraction used by the auth commands. type Store interface { - // SaveCredentials persists secrets to the system keychain (or the file - // fallback). + // SaveCredentials persists secrets to the system keychain (or, with + // Options.Insecure, the plain-file backend). SaveCredentials(creds Credentials) error // LoadCredentials returns the persisted secrets, or ErrNotFound if none // have been saved. @@ -114,10 +118,10 @@ type Store interface { // Options configures a Store returned by New. type Options struct { - // Insecure bypasses the system keychain (and its encrypted-file - // fallback) in favor of a plain, owner-only (0600) JSON file. Intended - // for headless/CI use where a passphrase prompt is unacceptable. - // Corresponds to the CLI's --insecure-storage flag. + // Insecure bypasses the system keychain in favor of a plain, owner-only + // (0600) JSON file. Intended for headless/CI use where a passphrase + // prompt is unacceptable. Corresponds to the CLI's --insecure-storage + // flag. Insecure bool // StateDir overrides the computed state directory. Intended for tests; @@ -156,7 +160,12 @@ func New(opts Options) (Store, error) { } } else { kr, err := keyring.Open(keyring.Config{ - ServiceName: serviceName, + ServiceName: serviceName, + // The pass backend namespaces entries via PassPrefix, not + // ServiceName; without it, our generic credentialsKey would be + // stored as a top-level "credentials" entry in the user's + // password store, risking collisions with unrelated tools. + PassPrefix: serviceName, AllowedBackends: systemBackends, KeychainTrustApplication: true, }) @@ -195,6 +204,30 @@ func resolveStateDir(override string) (string, error) { return dir, nil } +// writeOwnerOnlyFile writes data to path as an owner-only (0600) file. +// Unlike a bare os.WriteFile, this guarantees the 0600 mode is enforced even +// if a file already exists at path with looser permissions: os.WriteFile +// only applies its mode argument when creating a new file, silently +// preserving the mode of one that already exists. +func writeOwnerOnlyFile(path string, data []byte) error { + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) + if err != nil { + return err + } + + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + return err + } + + if _, err := f.Write(data); err != nil { + _ = f.Close() + return err + } + + return f.Close() +} + // SaveCredentials implements Store. func (s *store) SaveCredentials(creds Credentials) error { return s.secrets.Save(creds) @@ -218,7 +251,7 @@ func (s *store) SaveDeviceState(state DeviceState) error { } path := filepath.Join(s.stateDir, stateFileName) - if err := os.WriteFile(path, data, 0o600); err != nil { + if err := writeOwnerOnlyFile(path, data); err != nil { return fmt.Errorf("credstore: write device state: %w", err) } @@ -315,7 +348,7 @@ func (p *plainFileSecrets) Save(creds Credentials) error { return fmt.Errorf("credstore: marshal credentials: %w", err) } - if err := os.WriteFile(p.path, data, 0o600); err != nil { + if err := writeOwnerOnlyFile(p.path, data); err != nil { return fmt.Errorf("credstore: write credentials: %w", err) } From a0df0ec9e11d3add8495884de9af649e152c14eb Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 16:03:29 -0700 Subject: [PATCH 07/12] fix(review): address PR #3 review feedback Address review comments from copilot-pull-request-reviewer[bot]: - .goreleaser.yaml, .github/workflows/release-tag.yml: build darwin binaries natively on macos-latest with CGO_ENABLED=1 instead of cross-compiling from ubuntu-latest with CGO_ENABLED=0. keyring's KeychainBackend is compiled only under the "darwin && cgo" build tag, so released macOS binaries previously had no working Keychain backend. Darwin archives and checksums are uploaded as additional release assets after the main GoReleaser job. - internal/credstore/credstore.go: made writeOwnerOnlyFile atomic by writing to a temp file in the same directory and renaming it into place, instead of truncating the destination in place. A failed write can no longer erase a previously valid stored credentials file. - internal/credstore/credstore.go: corrected doc comments to note that Chmod(0600) does not enforce owner-only access on Windows (Go maps it to the read-only attribute, not a real ACL), rather than overpromising confidentiality there. - .cspell.json: added binpath, mktemp, pipefail to the word list for the new release-tag.yml darwin job. Resolves 3 review threads. Signed-off-by: Eric Searcy --- .cspell.json | 3 ++ .github/workflows/release-tag.yml | 62 +++++++++++++++++++++++++++++++ .goreleaser.yaml | 5 ++- internal/credstore/credstore.go | 61 +++++++++++++++++++++--------- 4 files changed, 112 insertions(+), 19 deletions(-) diff --git a/.cspell.json b/.cspell.json index 8f1edcf..014ff98 100644 --- a/.cspell.json +++ b/.cspell.json @@ -4,6 +4,7 @@ "words": [ "aquasecurity", "artipacked", + "binpath", "cimd", "clidocs", "coverprofile", @@ -25,6 +26,8 @@ "linuxfoundation", "memprof", "mgechev", + "mktemp", + "pipefail", "techdocs", "urfave", "zizmor" diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 05dc550..19f4dbb 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -43,3 +43,65 @@ jobs: args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + darwin: + name: Darwin (native, cgo-enabled) + # Built natively on macos-latest so KeychainBackend (requires + # "darwin && cgo") is actually compiled in. + needs: goreleaser + runs-on: macos-latest + + permissions: + contents: write + + env: + CGO_ENABLED: "1" + + steps: + - name: Checkout + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Set up Go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 + with: + go-version-file: go.mod + # Disable caching: publishes runtime artifacts on tag push. + cache: false + + - name: Build and archive darwin binaries + run: | + set -euo pipefail + + version="${GITHUB_REF_NAME#v}" + name="lfx-cli" + workdir="$(mktemp -d)" + + for arch in amd64 arm64; do + binpath="${workdir}/lfx" + GOOS=darwin GOARCH="${arch}" go build \ + -ldflags="-s -w -X main.version=${version}" \ + -o "${binpath}" ./cmd/lfx + + archive="${name}_darwin_${arch}.tar.gz" + tar -czf "${archive}" -C "${workdir}" lfx -C "${GITHUB_WORKSPACE}" LICENSE LICENSE-docs README.md + rm -f "${binpath}" + + shasum -a 256 "${archive}" >> checksums-darwin.txt + done + + - name: Upload darwin archives and merge checksums + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + + tag="${GITHUB_REF_NAME}" + gh release upload "${tag}" lfx-cli_darwin_amd64.tar.gz lfx-cli_darwin_arm64.tar.gz \ + --repo "${GITHUB_REPOSITORY}" + + gh release download "${tag}" --repo "${GITHUB_REPOSITORY}" --pattern checksums.txt --output checksums.txt + cat checksums-darwin.txt >> checksums.txt + gh release upload "${tag}" checksums.txt --repo "${GITHUB_REPOSITORY}" --clobber diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 81533d8..33bd00f 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -8,6 +8,10 @@ before: - go mod tidy builds: + # darwin is excluded here: KeychainBackend needs the "darwin && cgo" build + # tag, which a CGO_ENABLED=0 cross-compile from Linux can't satisfy. + # Darwin binaries are built natively by the darwin job in + # release-tag.yml and uploaded as additional release assets. - id: lfx main: ./cmd/lfx binary: lfx @@ -15,7 +19,6 @@ builds: - CGO_ENABLED=0 goos: - linux - - darwin - windows goarch: - amd64 diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 7ff67f1..06f8c69 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -14,10 +14,13 @@ // New returns an error rather than silently falling back to a file. // // When --insecure-storage is passed explicitly, the system keychain is -// bypassed entirely in favor of a plain (unencrypted), owner-only (0600) -// JSON file under the state directory. This is intended for headless/CI use -// where no passphrase prompt is acceptable, and is deliberately less secure -// than the keyring-backed storage. +// bypassed entirely in favor of a plain (unencrypted), owner-only (0600 on +// POSIX; on Windows, Go's Chmod maps 0600 to the read-only attribute +// instead of a real ACL, so confidentiality there relies on the file's +// inherited directory permissions) JSON file under the state directory. +// This is intended for headless/CI use where no passphrase prompt is +// acceptable, and is deliberately less secure than the keyring-backed +// storage. // // Non-sensitive state (device ID, IdP domain used at login) is always stored // as plain JSON under the XDG state directory (~/.local/state/lfx-cli/ by @@ -119,8 +122,9 @@ type Store interface { // Options configures a Store returned by New. type Options struct { // Insecure bypasses the system keychain in favor of a plain, owner-only - // (0600) JSON file. Intended for headless/CI use where a passphrase - // prompt is unacceptable. Corresponds to the CLI's --insecure-storage + // (0600; see writeOwnerOnlyFile for the Windows caveat) JSON file. + // Intended for headless/CI use where a passphrase prompt is + // unacceptable. Corresponds to the CLI's --insecure-storage // flag. Insecure bool @@ -204,28 +208,49 @@ func resolveStateDir(override string) (string, error) { return dir, nil } -// writeOwnerOnlyFile writes data to path as an owner-only (0600) file. -// Unlike a bare os.WriteFile, this guarantees the 0600 mode is enforced even -// if a file already exists at path with looser permissions: os.WriteFile -// only applies its mode argument when creating a new file, silently -// preserving the mode of one that already exists. -func writeOwnerOnlyFile(path string, data []byte) error { - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) +// writeOwnerOnlyFile writes data to path as an owner-only (0600) file, +// atomically: it writes to a temporary file in the same directory first, +// then renames it over path only after the write fully succeeds. This +// avoids truncating (and potentially losing) an existing valid file if the +// write is interrupted partway through. +// +// On Windows, 0600 does not enforce owner-only access: Go maps it to the +// read-only attribute rather than a real ACL, so confidentiality there +// depends on the file's inherited directory permissions. +func writeOwnerOnlyFile(path string, data []byte) (err error) { + dir := filepath.Dir(path) + + tmp, err := os.CreateTemp(dir, ".tmp-*") if err != nil { return err } + tmpPath := tmp.Name() + defer func() { + if err != nil { + _ = os.Remove(tmpPath) + } + }() + + if err = tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return err + } + + if _, err = tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } - if err := f.Chmod(0o600); err != nil { - _ = f.Close() + if err = tmp.Sync(); err != nil { + _ = tmp.Close() return err } - if _, err := f.Write(data); err != nil { - _ = f.Close() + if err = tmp.Close(); err != nil { return err } - return f.Close() + return os.Rename(tmpPath, path) } // SaveCredentials implements Store. From 6b363b90bcd19d052cef559e997377400a76ad8c Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 16:10:21 -0700 Subject: [PATCH 08/12] fix(review): address further PR #3 review feedback Address review comments from copilot-pull-request-reviewer[bot]: - README.md: documented that the owner-only insecure-storage file's 0600 permissions don't translate to a real ACL on Windows, so users there shouldn't assume the plaintext tokens are protected by file mode alone. - .github/workflows/release-tag.yml: made the darwin job's release asset upload retry-safe. Added --clobber to the archive upload so a rerun doesn't fail on already-existing assets, and filter out any previously-appended darwin checksum lines before re-appending them so reruns don't duplicate entries in checksums.txt. Resolves 2 review threads. Signed-off-by: Eric Searcy --- .github/workflows/release-tag.yml | 7 ++++++- README.md | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 19f4dbb..062f1be 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -100,8 +100,13 @@ jobs: tag="${GITHUB_REF_NAME}" gh release upload "${tag}" lfx-cli_darwin_amd64.tar.gz lfx-cli_darwin_arm64.tar.gz \ - --repo "${GITHUB_REPOSITORY}" + --repo "${GITHUB_REPOSITORY}" --clobber gh release download "${tag}" --repo "${GITHUB_REPOSITORY}" --pattern checksums.txt --output checksums.txt + # Filter out any darwin lines already present (e.g. from a prior, + # partially-failed run of this job) before appending, so reruns + # don't duplicate entries in checksums.txt. + grep -v -F -f checksums-darwin.txt checksums.txt > checksums-filtered.txt || true + mv checksums-filtered.txt checksums.txt cat checksums-darwin.txt >> checksums.txt gh release upload "${tag}" checksums.txt --repo "${GITHUB_REPOSITORY}" --clobber diff --git a/README.md b/README.md index 7a420a2..b63f889 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,10 @@ operating system's credential store by default (macOS Keychain, Windows Credential Manager, Linux Secret Service/KWallet/`pass`). Pass `--insecure-storage` to any `auth` subcommand to instead store credentials in a plain, unencrypted, owner-only file, at the cost of weaker protection -for the stored tokens: +for the stored tokens. On Windows, this owner-only mode relies on inherited +directory permissions rather than a real ACL, since Go's `Chmod(0600)` maps +to the read-only attribute there rather than restricting access to the +current user. ```bash lfx auth login --insecure-storage From 506d8fce8aac2cba5b85f5ea7f7a289c0341d630 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 16:57:23 -0700 Subject: [PATCH 09/12] fix(review): filter darwin checksums by filename, not full line Address review comment from copilot-pull-request-reviewer[bot]: - .github/workflows/release-tag.yml: the previous full-line grep filter only removed a darwin checksums.txt entry if its hash exactly matched the newly built archive. Since rebuilt archives get new hashes on every run (from embedded timestamps), a rerun would leave the stale entry in place and append a conflicting one for the same filename. Filter by filename instead, so reruns always replace prior darwin entries rather than duplicating them. Resolves 1 review thread. Signed-off-by: Eric Searcy --- .github/workflows/release-tag.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 062f1be..9127af1 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -103,10 +103,12 @@ jobs: --repo "${GITHUB_REPOSITORY}" --clobber gh release download "${tag}" --repo "${GITHUB_REPOSITORY}" --pattern checksums.txt --output checksums.txt - # Filter out any darwin lines already present (e.g. from a prior, - # partially-failed run of this job) before appending, so reruns - # don't duplicate entries in checksums.txt. - grep -v -F -f checksums-darwin.txt checksums.txt > checksums-filtered.txt || true + # Remove any existing entries for the darwin archives by filename + # (not full-line match, rebuilt archives have new hashes each run + # from embedded timestamps) before appending, so reruns don't + # leave stale or duplicate checksums.txt entries. + awk '{print $2}' checksums-darwin.txt > darwin-filenames.txt + grep -v -F -f darwin-filenames.txt checksums.txt > checksums-filtered.txt || true mv checksums-filtered.txt checksums.txt cat checksums-darwin.txt >> checksums.txt gh release upload "${tag}" checksums.txt --repo "${GITHUB_REPOSITORY}" --clobber From 7486df64b91cd88fdcd859586c586a245058e5cc Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 17:20:50 -0700 Subject: [PATCH 10/12] refactor(release): drop GoReleaser, build all platforms in one job GoReleaser's only remaining value in this pipeline (archive naming, checksums.txt, GitHub release asset upload) was already being hand-rolled for the darwin-native job added to fix the cgo/Keychain issue, plus a cross-job checksums.txt merge step. Its changelog generation was dead code: gh release create --generate-notes already sets the release body before the tag-push workflow runs, and GoReleaser's default keep-existing mode won't overwrite it. CGO_ENABLED=0 cross-compilation works from any host OS, so linux and windows binaries don't need ubuntu-latest specifically. This collapses the previous two-job split (ubuntu-latest running GoReleaser for linux/windows, macos-latest natively building darwin with a separate checksum-merge step) into a single macos-latest job that builds and archives all 5 targets and uploads them in one pass, with one checksums.txt and no merge logic needed. Also carries forward GoReleaser's own recommended reproducible-builds recipe (pin mtime to commit timestamp, -trimpath) for all 5 binaries, and extends it to the non-binary archived files (LICENSE, README): actions/checkout sets file mtimes to checkout time since git doesn't track per-file mtimes at all, so those need pinning too for consistent archives across runs. - .goreleaser.yaml: removed. - .github/workflows/release-tag.yml: single `release` job replacing the `goreleaser` + `darwin` jobs. - Makefile, AGENTS.md: removed GoReleaser references. - .cspell.json: updated word list for the new script's vocabulary. Signed-off-by: Eric Searcy --- .cspell.json | 4 +- .github/workflows/release-tag.yml | 128 ++++++++++++++---------------- .goreleaser.yaml | 55 ------------- AGENTS.md | 12 +-- Makefile | 17 +--- 5 files changed, 74 insertions(+), 142 deletions(-) delete mode 100644 .goreleaser.yaml diff --git a/.cspell.json b/.cspell.json index 014ff98..4de95cc 100644 --- a/.cspell.json +++ b/.cspell.json @@ -4,6 +4,7 @@ "words": [ "aquasecurity", "artipacked", + "binname", "binpath", "cimd", "clidocs", @@ -17,8 +18,8 @@ "gobin", "gofmt", "golangci", + "goos", "gopath", - "goreleaser", "kics", "ldflags", "lfx", @@ -29,6 +30,7 @@ "mktemp", "pipefail", "techdocs", + "trimpath", "urfave", "zizmor" ], diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 9127af1..8532930 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -11,9 +11,15 @@ name: Publish Tagged Release permissions: {} jobs: - goreleaser: - name: GoReleaser - runs-on: ubuntu-latest + release: + name: Build and upload release artifacts + runs-on: macos-latest + # Runs on macos-latest (rather than ubuntu-latest) so darwin binaries + # can be built natively with CGO_ENABLED=1: keyring's KeychainBackend + # is compiled only under the "darwin && cgo" build tag, which a + # cross-compiled, CGO_ENABLED=0 darwin build can't satisfy. linux and + # windows binaries are still pure cross-compiles (CGO_ENABLED=0, no + # cgo-only backends there), which works the same from any host OS. permissions: contents: write @@ -34,81 +40,69 @@ jobs: # into a release build (zizmor cache-poisoning audit). cache: false - - name: Run GoReleaser - uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 - with: - # Constrained to v2 (the action's documented default) so a - # future GoReleaser v3 release doesn't silently break old tags. - version: "~> v2" - args: release --clean - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - darwin: - name: Darwin (native, cgo-enabled) - # Built natively on macos-latest so KeychainBackend (requires - # "darwin && cgo") is actually compiled in. - needs: goreleaser - runs-on: macos-latest - - permissions: - contents: write - - env: - CGO_ENABLED: "1" - - steps: - - name: Checkout - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - with: - fetch-depth: 0 - persist-credentials: false - - - name: Set up Go - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 - with: - go-version-file: go.mod - # Disable caching: publishes runtime artifacts on tag push. - cache: false - - - name: Build and archive darwin binaries + - name: Build and archive release binaries run: | set -euo pipefail - version="${GITHUB_REF_NAME#v}" name="lfx-cli" - workdir="$(mktemp -d)" - - for arch in amd64 arm64; do - binpath="${workdir}/lfx" - GOOS=darwin GOARCH="${arch}" go build \ + version="${GITHUB_REF_NAME#v}" + # Reproducible builds: pin every archived file's mtime to the + # commit timestamp (rather than build/checkout time), and strip + # build-path information from the binary. actions/checkout sets + # file mtimes to checkout time, not the original commit time (git + # doesn't track per-file mtimes at all), so LICENSE/README need + # pinning here too, not just the binary. + # https://reproducible-builds.org/docs/archives/ + commit_epoch="$(git log -1 --format=%ct)" + mtime="$(date -u -r "${commit_epoch}" +%Y%m%d%H%M.%S)" + touch -t "${mtime}" LICENSE LICENSE-docs README.md + + # goos, goarch, cgo_enabled + targets=( + "linux amd64 0" + "linux arm64 0" + "windows amd64 0" + "darwin amd64 1" + "darwin arm64 1" + ) + + for target in "${targets[@]}"; do + read -r goos goarch cgo_enabled <<< "${target}" + + workdir="$(mktemp -d)" + binname="lfx" + [ "${goos}" = "windows" ] && binname="lfx.exe" + binpath="${workdir}/${binname}" + + GOOS="${goos}" GOARCH="${goarch}" CGO_ENABLED="${cgo_enabled}" go build \ + -trimpath \ -ldflags="-s -w -X main.version=${version}" \ -o "${binpath}" ./cmd/lfx - - archive="${name}_darwin_${arch}.tar.gz" - tar -czf "${archive}" -C "${workdir}" lfx -C "${GITHUB_WORKSPACE}" LICENSE LICENSE-docs README.md - rm -f "${binpath}" - - shasum -a 256 "${archive}" >> checksums-darwin.txt + touch -t "${mtime}" "${binpath}" + + if [ "${goos}" = "windows" ]; then + archive="${name}_${goos}_${goarch}.zip" + zip -X -j "${archive}" "${binpath}" LICENSE LICENSE-docs README.md + else + archive="${name}_${goos}_${goarch}.tar.gz" + tar -czf "${archive}" -C "${workdir}" "${binname}" -C "${GITHUB_WORKSPACE}" LICENSE LICENSE-docs README.md + fi + + shasum -a 256 "${archive}" >> checksums.txt + rm -rf "${workdir}" done - - name: Upload darwin archives and merge checksums + - name: Upload release artifacts env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail - tag="${GITHUB_REF_NAME}" - gh release upload "${tag}" lfx-cli_darwin_amd64.tar.gz lfx-cli_darwin_arm64.tar.gz \ + gh release upload "${GITHUB_REF_NAME}" \ + lfx-cli_linux_amd64.tar.gz \ + lfx-cli_linux_arm64.tar.gz \ + lfx-cli_windows_amd64.zip \ + lfx-cli_darwin_amd64.tar.gz \ + lfx-cli_darwin_arm64.tar.gz \ + checksums.txt \ --repo "${GITHUB_REPOSITORY}" --clobber - - gh release download "${tag}" --repo "${GITHUB_REPOSITORY}" --pattern checksums.txt --output checksums.txt - # Remove any existing entries for the darwin archives by filename - # (not full-line match, rebuilt archives have new hashes each run - # from embedded timestamps) before appending, so reruns don't - # leave stale or duplicate checksums.txt entries. - awk '{print $2}' checksums-darwin.txt > darwin-filenames.txt - grep -v -F -f darwin-filenames.txt checksums.txt > checksums-filtered.txt || true - mv checksums-filtered.txt checksums.txt - cat checksums-darwin.txt >> checksums.txt - gh release upload "${tag}" checksums.txt --repo "${GITHUB_REPOSITORY}" --clobber diff --git a/.goreleaser.yaml b/.goreleaser.yaml deleted file mode 100644 index 33bd00f..0000000 --- a/.goreleaser.yaml +++ /dev/null @@ -1,55 +0,0 @@ -# Copyright The Linux Foundation and each contributor to LFX. -# SPDX-License-Identifier: MIT - -version: 2 - -before: - hooks: - - go mod tidy - -builds: - # darwin is excluded here: KeychainBackend needs the "darwin && cgo" build - # tag, which a CGO_ENABLED=0 cross-compile from Linux can't satisfy. - # Darwin binaries are built natively by the darwin job in - # release-tag.yml and uploaded as additional release assets. - - id: lfx - main: ./cmd/lfx - binary: lfx - env: - - CGO_ENABLED=0 - goos: - - linux - - windows - goarch: - - amd64 - - arm64 - ignore: - - goos: windows - goarch: arm64 - ldflags: - - -s -w -X main.version={{.Version}} - -archives: - - id: lfx - formats: [tar.gz] - format_overrides: - - goos: windows - formats: [zip] - name_template: >- - {{ .ProjectName }}_{{ .Os }}_{{ .Arch }} - -checksum: - name_template: "checksums.txt" - -changelog: - sort: asc - filters: - exclude: - - "^docs:" - - "^test:" - - "^ci:" - -release: - github: - owner: linuxfoundation - name: lfx-cli diff --git a/AGENTS.md b/AGENTS.md index 2bdcda0..264d221 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,8 +18,10 @@ CLI (`lfx auth login` → `lfx auth token`). subcommand routing - **Docs generation**: [`urfave/cli-docs/v3`](https://github.com/urfave/cli-docs) for LLM/agent-friendly Markdown reference docs (`lfx docs`) -- **Release automation**: [GoReleaser](https://goreleaser.com/) for - multi-arch binary builds published to GitHub Releases +- **Release automation**: a single `release-tag.yml` GitHub Actions job + (running on `macos-latest`) cross-compiles linux/windows binaries and + natively builds cgo-enabled darwin binaries, then uploads all archives + to the GitHub Release - **Output**: user-facing command output uses plain `fmt.Println`/ `fmt.Fprintln`, not `log/slog`. This is intentional: unlike the JSON-structured `slog` logging convention used by LFX's long-running @@ -34,7 +36,8 @@ lfx-cli/ │ └── lfx/ # Main application entry point ├── internal/ │ └── commands/ # CLI subcommand implementations -├── .goreleaser.yaml # Multi-arch release build configuration +├── .github/workflows/ +│ └── release-tag.yml # Multi-arch release build/upload ├── go.mod # Go module definition ├── Makefile # Build automation ├── README.md # User documentation @@ -89,7 +92,6 @@ make fmt # Format code make vet # Run go vet make lint # Run golangci-lint (if installed) make revive # Run revive (if installed) -make goreleaser-check # Validate .goreleaser.yaml (if goreleaser installed) make check # Run all of the above ``` @@ -188,7 +190,7 @@ explicitly instructed. Do **not** create or push git tags manually. Instead, use the GitHub Releases UI (or `gh` CLI) to create a release; GitHub will create the tag automatically, and the **Publish Tagged Release** GitHub Actions workflow -will run GoReleaser to build and publish multi-arch binaries. +will build and upload multi-arch binaries to it. ```bash # Determine the next version by inspecting the latest tag. diff --git a/Makefile b/Makefile index 5586ab1..2ad8f2c 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # Copyright The Linux Foundation and each contributor to LFX. # SPDX-License-Identifier: MIT -.PHONY: all build clean check fmt vet lint revive goreleaser-check test test-coverage run deps install-tools megalinter help +.PHONY: all build clean check fmt vet lint revive test test-coverage run deps install-tools megalinter help # Build variables BINARY_NAME=lfx @@ -41,7 +41,7 @@ clean: # the read-only checks run; those are safe to parallelize with each other. check: $(MAKE) fmt - $(MAKE) vet lint revive goreleaser-check + $(MAKE) vet lint revive # Format Go code fmt: @@ -71,15 +71,6 @@ revive: echo "revive not installed, skipping..."; \ fi -# Validate the GoReleaser config (if available) -goreleaser-check: - @echo "Validating .goreleaser.yaml..." - @if command -v goreleaser >/dev/null 2>&1; then \ - goreleaser check; \ - else \ - echo "goreleaser not installed, skipping..."; \ - fi - # Run tests test: @echo "Running tests..." @@ -106,12 +97,11 @@ deps: install-tools: @echo "Installing development tools..." go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest - go install github.com/goreleaser/goreleaser/v2@latest go install github.com/mgechev/revive@latest @gobin="$$(go env GOPATH)/bin"; \ case ":$$PATH:" in \ *":$$gobin:"*) ;; \ - *) echo "Warning: $$gobin is not in your PATH; installed tools (golangci-lint, goreleaser, revive) won't be found. Add it to your PATH, e.g.: export PATH=\"$$PATH:$$gobin\"" ;; \ + *) echo "Warning: $$gobin is not in your PATH; installed tools (golangci-lint, revive) won't be found. Add it to your PATH, e.g.: export PATH=\"$$PATH:$$gobin\"" ;; \ esac # Run MegaLinter locally via Docker (matches CI Go flavor at v9.6.0). @@ -131,7 +121,6 @@ help: @echo " vet - Run go vet" @echo " lint - Run golangci-lint" @echo " revive - Run revive" - @echo " goreleaser-check - Validate .goreleaser.yaml" @echo " test - Run tests" @echo " test-coverage - Run tests with coverage report" @echo " run - Build and run the CLI (pass args via ARGS=...)" From 550250d48c5be89e6bb213825af056afbfce9270 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Fri, 17 Jul 2026 17:28:12 -0700 Subject: [PATCH 11/12] fix(release): make tar.gz archives reproducible bsdtar's built-in -z (gzip) writer embeds the current wall-clock time in the gzip header, independent of the archived files' own mtimes. Even with every archived file's mtime pinned to the commit timestamp, this made the tar.gz bytes differ on every run. Fixed by piping an uncompressed tar stream through `gzip -n`, which omits the timestamp (and original filename) from the gzip header. Verified locally: two separate builds from separate git clones (with a 2-second gap) now produce byte-identical tar.gz archives; zip archives were already reproducible without this change, since zip timestamps are stored per-entry rather than in a container-level header. Signed-off-by: Eric Searcy --- .github/workflows/release-tag.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 8532930..f1edbbf 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -85,7 +85,13 @@ jobs: zip -X -j "${archive}" "${binpath}" LICENSE LICENSE-docs README.md else archive="${name}_${goos}_${goarch}.tar.gz" - tar -czf "${archive}" -C "${workdir}" "${binname}" -C "${GITHUB_WORKSPACE}" LICENSE LICENSE-docs README.md + # Pipe through gzip -n rather than tar's built-in -z: bsdtar's + # gzip writer embeds the current wall-clock time in the gzip + # header regardless of the archived files' mtimes, which + # would otherwise make the archive bytes differ on every run + # despite the pinned mtimes above. + tar -cf - -C "${workdir}" "${binname}" -C "${GITHUB_WORKSPACE}" LICENSE LICENSE-docs README.md \ + | gzip -n > "${archive}" fi shasum -a 256 "${archive}" >> checksums.txt From 2ead8ccbe58dcaadf4b640ebc3e610aa75eb8e34 Mon Sep 17 00:00:00 2001 From: Eric Searcy Date: Mon, 20 Jul 2026 10:02:59 -0700 Subject: [PATCH 12/12] fix(review): write credentials.json in place instead of temp+rename Replaces the temp-file-plus-rename write with a direct in-place open, truncate, and write. path is exclusively created and managed by this CLI, so there's no concurrent writer to guard against, and the rename swap was introducing problems of its own: os.Rename isn't guaranteed atomic on Windows, and on SELinux systems a renamed-in file can pick up the temp file's security context instead of the destination's. 0600 is now requested only at file creation (matching os.OpenFile semantics); an existing file's permissions are left as-is rather than tightened, since it's the user's file to manage once it exists. Addresses PR #3 review feedback. Assisted-by: github-copilot:claude-sonnet-5 Signed-off-by: Eric Searcy --- internal/credstore/credstore.go | 50 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/internal/credstore/credstore.go b/internal/credstore/credstore.go index 06f8c69..660c015 100644 --- a/internal/credstore/credstore.go +++ b/internal/credstore/credstore.go @@ -208,49 +208,43 @@ func resolveStateDir(override string) (string, error) { return dir, nil } -// writeOwnerOnlyFile writes data to path as an owner-only (0600) file, -// atomically: it writes to a temporary file in the same directory first, -// then renames it over path only after the write fully succeeds. This -// avoids truncating (and potentially losing) an existing valid file if the -// write is interrupted partway through. +// writeOwnerOnlyFile writes data to path in place, creating it with +// owner-only (0600) permissions if it doesn't already exist. path is +// exclusively created and managed by this CLI, so there's no other writer +// to race against and no need for a temp-file-plus-rename swap: that would +// only introduce problems here, since os.Rename isn't guaranteed atomic on +// Windows, and on SELinux systems a renamed-in file can pick up the temp +// file's security context instead of the destination's. The accepted +// tradeoff is that a crash or write error between truncation and the write +// completing can leave the file empty or partially written, requiring the +// user to re-authenticate; for a local single-writer state file, that's a +// simpler and more predictable failure mode than the complexity a +// crash-safe rename would add. +// +// The 0600 mode is requested only at creation time (per os.OpenFile +// semantics); if path already exists with looser permissions (e.g. a user +// deliberately loosened them), this does not tighten them back. It's the +// user's file to manage once it exists. // // On Windows, 0600 does not enforce owner-only access: Go maps it to the // read-only attribute rather than a real ACL, so confidentiality there // depends on the file's inherited directory permissions. func writeOwnerOnlyFile(path string, data []byte) (err error) { - dir := filepath.Dir(path) - - tmp, err := os.CreateTemp(dir, ".tmp-*") + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) if err != nil { return err } - tmpPath := tmp.Name() defer func() { - if err != nil { - _ = os.Remove(tmpPath) + if closeErr := f.Close(); err == nil { + err = closeErr } }() - if err = tmp.Chmod(0o600); err != nil { - _ = tmp.Close() - return err - } - - if _, err = tmp.Write(data); err != nil { - _ = tmp.Close() - return err - } - - if err = tmp.Sync(); err != nil { - _ = tmp.Close() - return err - } - - if err = tmp.Close(); err != nil { + if _, err = f.Write(data); err != nil { return err } - return os.Rename(tmpPath, path) + return f.Sync() } // SaveCredentials implements Store.