Skip to content

Commit 2485cf7

Browse files
committed
feat(cred): select credential ref per invocation (--ref flag + env)
Read/query commands always used the single active credential_ref from config.yml, so concurrent processes targeting different accounts raced on that shared file — a clobbered ref could leave gro pointing at a tokenless profile ("no OAuth token found") or a half-written value. Add a per-invocation credential-ref selector honored by all commands, mirroring the existing --backend precedence: --ref <service>/<profile> > GOOGLE_READONLY_CREDENTIAL_REF env > config credential_ref - root: persistent --ref flag recorded via keychain.SetCredentialRefOverride in a new WireCredentialRefSelection hook that validates <service>/<profile> up front. set-credential keeps its own local --ref write-target (shadow). - keychain: effectiveRef() applies flag > env > config at the single open() resolution site; an explicit override suppresses the one-time legacy migration (it only ever targets the configured ref, same rationale as OpenRef). - env-var name derived from the service so it tracks the <SERVICE>_ prefix. Tests cover precedence + empty-flag fall-through, flag validation, the PersistentPreRunE shadowing guard, and set-credential's local-shadow vs a read command inheriting the persistent flag. Docs updated (README multi-account section + docs/development.md). Closes #166
1 parent 0533076 commit 2485cf7

8 files changed

Lines changed: 319 additions & 1 deletion

File tree

README.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,30 @@ config > auto). With the default ref, `set-credential`
213213
runs the one-time legacy migration first so a pre-existing `token.json` cannot
214214
later collide; an explicit `--ref` never migrates.
215215

216+
### Selecting an account per invocation (concurrent multi-account use)
217+
218+
`set-credential` can store tokens for several accounts under distinct credential
219+
refs (`<service>/<profile>`), but by default every command reads the single
220+
active `credential_ref` from `config.yml`. To let concurrent processes each
221+
target a *different* account without racing on that shared file, **any** command
222+
accepts a per-invocation credential ref, resolved with the same precedence shape
223+
as `--backend`:
224+
225+
`--ref <service>/<profile>` flag > `GOOGLE_READONLY_CREDENTIAL_REF` env > `config.yml` `credential_ref`
226+
227+
```bash
228+
# Two mailboxes in parallel, each fully self-contained — no config.yml rewrite:
229+
gro mail search "is:unread" --ref google-readonly/work &
230+
gro mail search "is:unread" --ref google-readonly/personal &
231+
232+
# Or via the environment, to wrap a whole process:
233+
GOOGLE_READONLY_CREDENTIAL_REF=google-readonly/work gro calendar today
234+
```
235+
236+
An explicit `--ref`/env override targets an already-provisioned profile, so it
237+
never runs the one-time legacy `token.json` migration (that only ever applies to
238+
the configured/default ref).
239+
216240
## Commands
217241

218242
### Configuration Commands

docs/development.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ OAuth tokens are per-user access secrets and live only through `cli-common/creds
104104

105105
Non-secret config uses the OS-native config directory via `cli-common/statedir`. Current config fields include `credential_ref`, `oauth_client_path`, `granted_scopes`, and `keyring.backend`.
106106

107+
The active `credential_ref` can be overridden per invocation — precedence `--ref` flag > `GOOGLE_READONLY_CREDENTIAL_REF` env > `config.yml` — so concurrent processes can target different accounts without racing on the shared config file. This mirrors the `--backend` selection machinery: the persistent flag is recorded by `root.WireCredentialRefSelection` and applied at the single `keychain.open` resolution site (which suppresses the one-time legacy migration whenever an override is present, since migration only ever targets the configured ref).
108+
107109
Secret ingress belongs in setup and credential-management commands only. For the shared rules, use `working-with-secrets.md`, `working-with-state.md`, and `scriptability.md` from `cli-common`.
108110

109111
## Testing Notes

internal/cmd/root/backend_wire_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ const serviceName = "google-readonly"
2525
func resetState(t *testing.T) {
2626
t.Helper()
2727
keychain.SetBackendFlagOverride("", false)
28+
keychain.SetCredentialRefOverride("", false)
2829
// rootCmd.SetArgs mutates package-level state; if a test panics before
2930
// the next test calls SetArgs, a stale slice could bleed in (notably
3031
// under `go test -shuffle=on`). Clear it on cleanup.
3132
t.Cleanup(func() {
3233
keychain.SetBackendFlagOverride("", false)
34+
keychain.SetCredentialRefOverride("", false)
3335
rootCmd.SetArgs(nil)
3436
})
3537
}
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package root
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/spf13/cobra"
8+
9+
"github.com/open-cli-collective/google-readonly/internal/keychain"
10+
)
11+
12+
// TestWireCredentialRefSelection_FlagSet proves a --ref on a real command
13+
// path is recorded in the override the keychain.open resolver reads.
14+
func TestWireCredentialRefSelection_FlagSet(t *testing.T) {
15+
resetState(t)
16+
t.Setenv(keychain.CredentialRefEnvVar(), "")
17+
18+
probe := newProbeCmd("probe-ref-flagset")
19+
rootCmd.AddCommand(probe)
20+
defer removeChild(t, probe)
21+
rootCmd.SetArgs([]string{"probe-ref-flagset", "--ref", "google-readonly/acct-a"})
22+
23+
if err := rootCmd.Execute(); err != nil {
24+
t.Fatalf("Execute: %v", err)
25+
}
26+
v, set := keychain.GetCredentialRefOverride()
27+
if !set {
28+
t.Fatalf("override flagSet = false, want true")
29+
}
30+
if v != "google-readonly/acct-a" {
31+
t.Errorf("override value = %q, want %q", v, "google-readonly/acct-a")
32+
}
33+
}
34+
35+
// TestWireCredentialRefSelection_FlagInvalid asserts a malformed --ref fails
36+
// up front with a clear "--ref" error, before any keyring work.
37+
func TestWireCredentialRefSelection_FlagInvalid(t *testing.T) {
38+
resetState(t)
39+
40+
probe := newProbeCmd("probe-ref-invalid")
41+
rootCmd.AddCommand(probe)
42+
defer removeChild(t, probe)
43+
rootCmd.SetArgs([]string{"probe-ref-invalid", "--ref", "no-slash"})
44+
45+
err := rootCmd.Execute()
46+
if err == nil {
47+
t.Fatal("expected error, got nil")
48+
}
49+
if !strings.Contains(err.Error(), "--"+credentialRefFlagName) {
50+
t.Errorf("error should mention --%s: %v", credentialRefFlagName, err)
51+
}
52+
}
53+
54+
// TestWireCredentialRefSelection_ShadowingSubcommand regresses the
55+
// cobra-doesn't-chain-PersistentPreRunE bug for --ref, mirroring the
56+
// --backend guard.
57+
func TestWireCredentialRefSelection_ShadowingSubcommand(t *testing.T) {
58+
resetState(t)
59+
t.Setenv(keychain.CredentialRefEnvVar(), "")
60+
61+
shadow := &cobra.Command{
62+
Use: "shadow-ref",
63+
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
64+
return WireCredentialRefSelection(cmd)
65+
},
66+
}
67+
leaf := newProbeCmd("leaf")
68+
shadow.AddCommand(leaf)
69+
rootCmd.AddCommand(shadow)
70+
defer removeChild(t, shadow)
71+
72+
rootCmd.SetArgs([]string{"shadow-ref", "leaf", "--ref", "google-readonly/acct-b"})
73+
if err := rootCmd.Execute(); err != nil {
74+
t.Fatalf("Execute through shadowing PreRunE: %v", err)
75+
}
76+
v, set := keychain.GetCredentialRefOverride()
77+
if !set || v != "google-readonly/acct-b" {
78+
t.Errorf("override = (%q, %v); want (\"google-readonly/acct-b\", true) — shadower's PreRunE failed to invoke WireCredentialRefSelection", v, set)
79+
}
80+
}
81+
82+
// TestCredentialRef_SetCredentialShadowsPersistent documents the intentional
83+
// exception to the inherit-everywhere rule: `set-credential` keeps its own
84+
// local --ref (the write target), so it must resolve to a DIFFERENT *pflag.Flag
85+
// than the root's persistent selector — while a read command inherits the
86+
// canonical persistent one. A regression that dropped set-credential's local
87+
// flag (or that made a read command shadow --ref) would flip these.
88+
func TestCredentialRef_SetCredentialShadowsPersistent(t *testing.T) {
89+
canonical := rootCmd.PersistentFlags().Lookup(credentialRefFlagName)
90+
if canonical == nil {
91+
t.Fatalf("root persistent flag --%s not registered", credentialRefFlagName)
92+
}
93+
94+
var sc *cobra.Command
95+
for _, c := range rootCmd.Commands() {
96+
if c.Name() == "set-credential" {
97+
sc = c
98+
break
99+
}
100+
}
101+
if sc == nil {
102+
t.Fatal("set-credential command not registered on rootCmd")
103+
}
104+
if got := sc.Flag(credentialRefFlagName); got == nil {
105+
t.Fatalf("set-credential has no --%s", credentialRefFlagName)
106+
} else if got == canonical {
107+
t.Errorf("set-credential --%s resolved to the persistent flag; expected its own local shadow", credentialRefFlagName)
108+
}
109+
110+
// A read command (no local --ref) must inherit the canonical persistent flag.
111+
me := newProbeCmd("probe-ref-inherit")
112+
rootCmd.AddCommand(me)
113+
defer removeChild(t, me)
114+
if got := me.Flag(credentialRefFlagName); got != canonical {
115+
t.Errorf("read command --%s = %p, want canonical %p (unexpected shadow)", credentialRefFlagName, got, canonical)
116+
}
117+
}

internal/cmd/root/root.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ var (
3232
noColor bool
3333
)
3434

35+
// credentialRefFlagName is the global per-invocation credential-ref selector.
36+
// It shares its name with `set-credential`'s own write-target --ref (a local
37+
// flag that shadows this persistent one for that command only).
38+
const credentialRefFlagName = "ref"
39+
3540
var rootCmd = &cobra.Command{
3641
Use: "gro",
3742
Short: "A non-destructive CLI for Google services",
@@ -52,7 +57,10 @@ This will guide you through OAuth setup for Google API access.`,
5257
if noColor {
5358
lipgloss.DefaultRenderer().SetColorProfile(termenv.Ascii)
5459
}
55-
return WireBackendSelection(cmd)
60+
if err := WireBackendSelection(cmd); err != nil {
61+
return err
62+
}
63+
return WireCredentialRefSelection(cmd)
5664
},
5765
}
5866

@@ -82,6 +90,32 @@ func WireBackendSelection(cmd *cobra.Command) error {
8290
return nil
8391
}
8492

93+
// WireCredentialRefSelection records the user-supplied --ref flag for the next
94+
// keychain.Open* call and validates its <service>/<profile> shape up front so a
95+
// bad value fails with a clear "--ref" error before any keyring work. The
96+
// resolved precedence (--ref flag > <SERVICE>_CREDENTIAL_REF env > config
97+
// credential_ref) is applied at keychain.open; this hook only records the flag.
98+
//
99+
// Like WireBackendSelection, it is exported because cobra does NOT chain
100+
// PersistentPreRunE — a subcommand that defines its own must call this
101+
// explicitly. Reads via cmd.Flag() so persistent-flag inheritance works from
102+
// any subcommand path.
103+
func WireCredentialRefSelection(cmd *cobra.Command) error {
104+
f := cmd.Flag(credentialRefFlagName)
105+
if f == nil {
106+
return nil
107+
}
108+
value := f.Value.String()
109+
changed := f.Changed
110+
if changed && value != "" {
111+
if _, _, err := cccredstore.ParseRef(value); err != nil {
112+
return fmt.Errorf("--%s: %w", credentialRefFlagName, err)
113+
}
114+
}
115+
keychain.SetCredentialRefOverride(value, changed)
116+
return nil
117+
}
118+
85119
// Execute runs the root command with a background context
86120
func Execute() {
87121
ExecuteContext(context.Background())
@@ -117,6 +151,11 @@ func init() {
117151
rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Enable verbose output for debugging")
118152
rootCmd.PersistentFlags().BoolVar(&noColor, "no-color", false, "Disable colored output")
119153
rootCmd.PersistentFlags().String(cccredstore.BackendFlagName, "", cccredstore.BackendFlagUsage())
154+
rootCmd.PersistentFlags().String(credentialRefFlagName, "", fmt.Sprintf(
155+
"Credential ref <service>/<profile> for this invocation, so concurrent commands "+
156+
"can target different accounts without racing on config.yml "+
157+
"(precedence: --%s flag > %s env > config credential_ref)",
158+
credentialRefFlagName, keychain.CredentialRefEnvVar()))
120159

121160
// Register commands
122161
rootCmd.AddCommand(initcmd.NewCommand())

internal/keychain/credref_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package keychain
2+
3+
import "testing"
4+
5+
// resetCredRefOverride keeps the package-level --ref override clean across
6+
// tests so a leaked value can't tilt the next.
7+
func resetCredRefOverride(t *testing.T) {
8+
t.Helper()
9+
SetCredentialRefOverride("", false)
10+
t.Cleanup(func() { SetCredentialRefOverride("", false) })
11+
}
12+
13+
// TestCredentialRefEnvVar pins the derived env-var name to the documented
14+
// <SERVICE>_CREDENTIAL_REF, tracking the same prefix as the backend env var.
15+
func TestCredentialRefEnvVar(t *testing.T) {
16+
if got := CredentialRefEnvVar(); got != "GOOGLE_READONLY_CREDENTIAL_REF" {
17+
t.Errorf("CredentialRefEnvVar() = %q, want %q", got, "GOOGLE_READONLY_CREDENTIAL_REF")
18+
}
19+
}
20+
21+
// TestEffectiveRef_Precedence proves --ref flag > env > config, and that an
22+
// override is reported (so the caller suppresses the one-time migration).
23+
func TestEffectiveRef_Precedence(t *testing.T) {
24+
const cfgRef = "google-readonly/cfg"
25+
26+
t.Run("config only", func(t *testing.T) {
27+
resetCredRefOverride(t)
28+
t.Setenv(CredentialRefEnvVar(), "")
29+
ref, ov := effectiveRef(cfgRef)
30+
if ref != cfgRef || ov {
31+
t.Errorf("got (%q,%v), want (%q,false)", ref, ov, cfgRef)
32+
}
33+
})
34+
35+
t.Run("env overrides config", func(t *testing.T) {
36+
resetCredRefOverride(t)
37+
t.Setenv(CredentialRefEnvVar(), "google-readonly/env")
38+
ref, ov := effectiveRef(cfgRef)
39+
if ref != "google-readonly/env" || !ov {
40+
t.Errorf("got (%q,%v), want (google-readonly/env,true)", ref, ov)
41+
}
42+
})
43+
44+
t.Run("flag overrides env and config", func(t *testing.T) {
45+
resetCredRefOverride(t)
46+
t.Setenv(CredentialRefEnvVar(), "google-readonly/env")
47+
SetCredentialRefOverride("google-readonly/flag", true)
48+
ref, ov := effectiveRef(cfgRef)
49+
if ref != "google-readonly/flag" || !ov {
50+
t.Errorf("got (%q,%v), want (google-readonly/flag,true)", ref, ov)
51+
}
52+
})
53+
54+
t.Run("flag set but empty does not override", func(t *testing.T) {
55+
resetCredRefOverride(t)
56+
t.Setenv(CredentialRefEnvVar(), "")
57+
SetCredentialRefOverride("", true) // Changed=true but no value
58+
ref, ov := effectiveRef(cfgRef)
59+
if ref != cfgRef || ov {
60+
t.Errorf("got (%q,%v), want (%q,false) — empty --ref must fall through", ref, ov, cfgRef)
61+
}
62+
})
63+
}

internal/keychain/keychain.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import (
1717
"encoding/json"
1818
"errors"
1919
"fmt"
20+
"os"
21+
"strings"
2022

2123
"github.com/open-cli-collective/cli-common/credstore"
2224
"golang.org/x/oauth2"
@@ -71,9 +73,49 @@ func open(overwrite, runMigration bool) (*Store, error) {
7173
if err != nil {
7274
return nil, err
7375
}
76+
// A per-invocation --ref flag or <SERVICE>_CREDENTIAL_REF env selects the
77+
// credential ref for this process (precedence: flag > env > config), so
78+
// concurrent processes can each target a different account without racing
79+
// on the shared config.yml. An explicit override also suppresses the
80+
// one-time §1.8 migration: it only ever targets the configured/default ref
81+
// (running it against an arbitrary ref could write the default's legacy
82+
// data under the wrong service/profile — the same reason OpenRef skips it).
83+
if ref, overridden := effectiveRef(cfg.CredentialRef); overridden {
84+
cfg.CredentialRef = ref
85+
runMigration = false
86+
}
7487
return openWith(cfg, overwrite, runMigration)
7588
}
7689

90+
// CredentialRefEnvVar is the per-invocation credential-ref override env var
91+
// (e.g. "GOOGLE_READONLY_CREDENTIAL_REF"). It is derived from gro's service so
92+
// it always tracks the same <SERVICE>_ prefix credstore uses for the backend
93+
// env var, and is never hard-coded (§1.3). Exported so the cobra layer can name
94+
// it in the --ref flag's help text.
95+
func CredentialRefEnvVar() string {
96+
service, _, err := credstore.ParseRef(config.DefaultCredentialRef)
97+
if err != nil {
98+
// DefaultCredentialRef is a compile-time-valid ref; unreachable.
99+
return ""
100+
}
101+
return strings.TrimSuffix(credstore.BackendEnvVar(service), "_KEYRING_BACKEND") + "_CREDENTIAL_REF"
102+
}
103+
104+
// effectiveRef applies the per-invocation credential-ref precedence
105+
// (--ref flag > <SERVICE>_CREDENTIAL_REF env > config credential_ref) and
106+
// reports whether an explicit override was supplied. Mirrors the --backend
107+
// precedence chain. When overridden is true, the caller must skip the one-time
108+
// §1.8 migration (see open / OpenRef).
109+
func effectiveRef(configRef string) (ref string, overridden bool) {
110+
if v, set := GetCredentialRefOverride(); set && v != "" {
111+
return v, true
112+
}
113+
if v := os.Getenv(CredentialRefEnvVar()); v != "" {
114+
return v, true
115+
}
116+
return configRef, false
117+
}
118+
77119
// OpenRef opens a store against an explicit ref instead of config.yml's
78120
// credential_ref — used by `gro set-credential --ref` and the refresh
79121
// persister. An empty ref falls back to the configured/default ref.

internal/keychain/wire.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,32 @@ func GetBackendFlagOverride() (value string, flagSet bool) {
2828
defer backendMu.RUnlock()
2929
return backendFlagValue, backendFlagWasSet
3030
}
31+
32+
var (
33+
credRefMu sync.RWMutex
34+
credRefFlagValue string
35+
credRefFlagWasSet bool
36+
)
37+
38+
// SetCredentialRefOverride records the user-supplied --ref flag for the next
39+
// keychain.Open* call. Called by root.WireCredentialRefSelection at
40+
// PersistentPreRunE time. Mirrors SetBackendFlagOverride: a persistent flag
41+
// can't be threaded through the parameterless keychain.Open() the read
42+
// commands call, so it is recorded here and read back at the single
43+
// resolution site (open). flagSet matches cobra's pflag.Flag.Changed — true
44+
// when the user passed --ref on the command line, regardless of value.
45+
func SetCredentialRefOverride(value string, flagSet bool) {
46+
credRefMu.Lock()
47+
defer credRefMu.Unlock()
48+
credRefFlagValue = value
49+
credRefFlagWasSet = flagSet
50+
}
51+
52+
// GetCredentialRefOverride returns the current --ref override and whether it
53+
// was set. The flag-set vs unset distinction lets an explicit empty value be
54+
// told apart from "no flag".
55+
func GetCredentialRefOverride() (value string, flagSet bool) {
56+
credRefMu.RLock()
57+
defer credRefMu.RUnlock()
58+
return credRefFlagValue, credRefFlagWasSet
59+
}

0 commit comments

Comments
 (0)