From 3f4ffd942b8e44b78680cb4475a0aa1e469e4e24 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 25 Sep 2026 14:20:06 -0500 Subject: [PATCH 1/4] feat: Run one command as another session Every command ran as the active session, so reaching another account or environment meant running 'auth switch' before and after. Scripts, CI jobs and agents that touch production and staging in one run had to change shared state, and anything running at the same time saw the switch. A global --session flag and the DATUM_SESSION environment variable now pick the session for one process. The value is a session name (email@api-host) or an email signed in on one endpoint; an ambiguous or unknown value fails with the valid choices. The flag wins over the variable. The command uses that session's last context unless a scope flag or DATUM_PROJECT/DATUM_ORGANIZATION names one. The override is held in memory in datumconfig and consulted by ActiveSessionEntry and CurrentContextEntry, which every reader of the active session already goes through, so no caller can miss it. It is not a config field, so saving the config never persists it; the console's context switcher keeps its pick in memory under an override. login, logout, auth switch and ctx use change the active session, so they reject --session and ignore DATUM_SESSION. auth get-token keeps its own --session flag, which shadows the global one, so kubeconfig exec entries resolve exactly as before. Plugin forwarding strips a leading --session before the plugin name and resolves the session first, so plugins get its name and API host and a datumctl they call back into inherits it. Fixes #302 Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 11 +- internal/client/factory.go | 9 +- internal/client/session_override_test.go | 132 ++++++ internal/cmd/auth/auth.go | 2 +- internal/cmd/auth/get_token.go | 30 +- internal/cmd/auth/list.go | 2 +- internal/cmd/auth/switch.go | 9 +- internal/cmd/ctx/list.go | 4 +- internal/cmd/ctx/use.go | 9 +- internal/cmd/landing.go | 1 + internal/cmd/login/login.go | 8 +- internal/cmd/logout/logout.go | 8 +- internal/cmd/root.go | 22 +- internal/cmd/session_override.go | 66 +++ internal/cmd/session_override_test.go | 436 ++++++++++++++++++ internal/cmd/whoami/whoami.go | 78 ++-- internal/console/components/ctxswitcher.go | 9 +- internal/console/model.go | 6 +- internal/datumconfig/config_v1beta1.go | 12 +- internal/datumconfig/session_override.go | 196 ++++++++ internal/datumconfig/session_override_test.go | 159 +++++++ internal/picker/picker.go | 4 +- internal/plugindispatch/forward.go | 52 ++- .../plugindispatch/session_override_test.go | 173 +++++++ 24 files changed, 1377 insertions(+), 61 deletions(-) create mode 100644 internal/client/session_override_test.go create mode 100644 internal/cmd/session_override.go create mode 100644 internal/cmd/session_override_test.go create mode 100644 internal/datumconfig/session_override.go create mode 100644 internal/datumconfig/session_override_test.go create mode 100644 internal/plugindispatch/session_override_test.go diff --git a/README.md b/README.md index 8f4fc08..2ed8ed2 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,16 @@ DATUM_PROJECT=my-project datumctl get dnszones DATUM_ORGANIZATION=my-org datumctl get projects ``` -`--project` and `--organization` flags work too. For machine-to-machine auth, see `datumctl login --credentials` for the machine-account flow. +`--project` and `--organization` flags work too. + +To run one command as another signed-in account without switching the active one, name its session with `--session` or `DATUM_SESSION`. The value is a session name (`email@api-host`) or an email signed in on only one endpoint. The command uses that session's last context unless you pass a scope: + +```bash +datumctl get dnszones --session alice@example.com@api.staging.env.datum.net +DATUM_SESSION=alice@example.com datumctl get projects +``` + +For machine-to-machine auth, see `datumctl login --credentials` for the machine-account flow. ## Agent Skills diff --git a/internal/client/factory.go b/internal/client/factory.go index 1089788..c5aed04 100644 --- a/internal/client/factory.go +++ b/internal/client/factory.go @@ -270,6 +270,11 @@ func (c *CustomConfigFlags) loadDatumContext() (*datumconfig.DiscoveredContext, } ctxEntry := cfg.CurrentContextEntry() if ctxEntry == nil { + // Under a session override with no context, still hand back the + // overriding session so its endpoint and TLS settings apply. + if datumconfig.HasSessionOverride() { + return nil, cfg.ActiveSessionEntry(), nil + } return nil, nil, nil } session := cfg.SessionByName(ctxEntry.Session) @@ -375,8 +380,8 @@ func (c *CustomConfigFlags) ensureOnboardingComplete( sessionName := "" if ctxEntry != nil { sessionName = ctxEntry.Session - } else if cfg.ActiveSession != "" { - sessionName = cfg.ActiveSession + } else { + sessionName = cfg.ActiveSessionName() } orgDisplayName = cfg.OrgDisplayName(sessionName, orgID) } diff --git a/internal/client/session_override_test.go b/internal/client/session_override_test.go new file mode 100644 index 0000000..9f23f85 --- /dev/null +++ b/internal/client/session_override_test.go @@ -0,0 +1,132 @@ +package client + +import ( + "context" + "encoding/json" + "testing" + "time" + + "golang.org/x/oauth2" + + "go.datum.net/datumctl/internal/authutil" + "go.datum.net/datumctl/internal/datumconfig" + "go.datum.net/datumctl/internal/keyring" + "go.datum.net/datumctl/internal/miloapi" +) + +const ( + ovProd = "swells@datum.net@api.datum.net" + ovStaging = "swells@datum.net@api.staging.env.datum.net" + ovSolo = "solo@example.com@api.datum.net" +) + +func setupFactoryOverrideEnv(t *testing.T) *DatumCloudFactory { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("DATUM_PROJECT", "") + t.Setenv("DATUM_ORGANIZATION", "") + keyring.MockInit() + t.Cleanup(datumconfig.ClearSessionOverride) + + for key, subject := range map[string]string{"key-prod": "u-prod", "key-staging": "u-staging", "key-solo": "u-solo"} { + blob, _ := json.Marshal(authutil.StoredCredentials{ + Hostname: "auth.datum.net", + Subject: subject, + Token: &oauth2.Token{AccessToken: "tok", Expiry: time.Now().Add(time.Hour)}, + }) + if err := keyring.Set(authutil.ServiceName, key, string(blob)); err != nil { + t.Fatal(err) + } + } + + prodCtx := datumconfig.QualifiedContextName(ovProd, "org-prod") + stagingCtx := datumconfig.QualifiedContextName(ovStaging, "org-staging/proj-staging") + cfg := datumconfig.NewV1Beta1() + cfg.Sessions = []datumconfig.Session{ + {Name: ovProd, UserKey: "key-prod", UserEmail: "swells@datum.net", + Endpoint: datumconfig.Endpoint{Server: "https://api.datum.net"}, LastContext: prodCtx}, + {Name: ovStaging, UserKey: "key-staging", UserEmail: "swells@datum.net", + Endpoint: datumconfig.Endpoint{Server: "https://api.staging.env.datum.net", TLSServerName: "staging.sni"}, + LastContext: stagingCtx}, + {Name: ovSolo, UserKey: "key-solo", UserEmail: "solo@example.com", + Endpoint: datumconfig.Endpoint{Server: "https://api.solo.example", TLSServerName: "solo.sni"}}, + } + cfg.Contexts = []datumconfig.DiscoveredContext{ + {Name: prodCtx, Session: ovProd, OrganizationID: "org-prod"}, + {Name: stagingCtx, Session: ovStaging, OrganizationID: "org-staging", ProjectID: "proj-staging"}, + } + cfg.ActiveSession = ovProd + cfg.CurrentContext = prodCtx + if err := datumconfig.SaveV1Beta1(cfg); err != nil { + t.Fatal(err) + } + + f, err := NewDatumFactory(context.Background()) + if err != nil { + t.Fatal(err) + } + f.ConfigFlags.SkipOnboardingCheck = true + return f +} + +// Requests go to the overriding session's endpoint with its credentials and +// its last context, while --project and DATUM_ORGANIZATION still pick scope. +func TestToRESTConfig_SessionOverride(t *testing.T) { + const staging = "https://api.staging.env.datum.net" + tests := []struct { + name string + override string + project string + envOrg string + wantHost string + wantServer string + }{ + {name: "no override", wantHost: miloapi.OrgControlPlaneURL("https://api.datum.net", "org-prod")}, + {name: "override uses its last context", override: ovStaging, + wantHost: miloapi.ProjectControlPlaneURL(staging, "proj-staging"), wantServer: "staging.sni"}, + {name: "--project beats the override's context", override: ovStaging, project: "other", + wantHost: miloapi.ProjectControlPlaneURL(staging, "other"), wantServer: "staging.sni"}, + {name: "DATUM_ORGANIZATION beats the override's context", override: ovStaging, envOrg: "env-org", + wantHost: miloapi.OrgControlPlaneURL(staging, "env-org"), wantServer: "staging.sni"}, + {name: "override without a context uses its user control plane on its endpoint", override: ovSolo, + wantHost: miloapi.UserControlPlaneURL("https://api.solo.example", "u-solo"), wantServer: "solo.sni"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := setupFactoryOverrideEnv(t) + t.Setenv("DATUM_ORGANIZATION", tt.envOrg) + *f.ConfigFlags.Project = tt.project + if tt.override != "" { + datumconfig.SetSessionOverride(tt.override, datumconfig.SessionOverrideFromFlag) + } + rc, err := f.ConfigFlags.ToRESTConfig() + if err != nil { + t.Fatalf("ToRESTConfig: %v", err) + } + if rc.Host != tt.wantHost { + t.Errorf("Host = %q, want %q", rc.Host, tt.wantHost) + } + if rc.ServerName != tt.wantServer { + t.Errorf("ServerName = %q, want %q", rc.ServerName, tt.wantServer) + } + }) + } +} + +func TestResolveSessionEndpoint_SessionOverride(t *testing.T) { + setupFactoryOverrideEnv(t) + datumconfig.SetSessionOverride(ovStaging, datumconfig.SessionOverrideFromEnv) + cfg, err := datumconfig.LoadAuto() + if err != nil { + t.Fatal(err) + } + session, ep, err := ResolveSessionEndpoint(cfg, "") + if err != nil { + t.Fatal(err) + } + if session.Name != ovStaging || ep.BaseServer != "https://api.staging.env.datum.net" || ep.UserKey != "key-staging" { + t.Errorf("got session %q, server %q, key %q; want the staging session", session.Name, ep.BaseServer, ep.UserKey) + } +} diff --git a/internal/cmd/auth/auth.go b/internal/cmd/auth/auth.go index b8c9190..99af6f7 100644 --- a/internal/cmd/auth/auth.go +++ b/internal/cmd/auth/auth.go @@ -56,7 +56,7 @@ Advanced — kubectl integration: } cmd.AddCommand( - getTokenCmd, + getTokenCmd(), listCmd, switchCmd(), updateKubeconfigCmd(), diff --git a/internal/cmd/auth/get_token.go b/internal/cmd/auth/get_token.go index 33aa37e..2fdf7af 100644 --- a/internal/cmd/auth/get_token.go +++ b/internal/cmd/auth/get_token.go @@ -21,10 +21,11 @@ const ( ) // getTokenCmd retrieves tokens based on the --output flag. -var getTokenCmd = &cobra.Command{ - Use: "get-token", - Short: "Print an access token (kubectl and plugin credential helper)", - Long: `Print the current access token for the active Datum Cloud user. +func getTokenCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "get-token", + Short: "Print an access token (kubectl and plugin credential helper)", + Long: `Print the current access token for the active Datum Cloud user. Most datumctl users do not need this command — datumctl handles authentication automatically for all its own commands. @@ -43,6 +44,9 @@ This command exists for two advanced use cases: $DATUM_CREDENTIALS_HELPER auth get-token --session $DATUM_SESSION When DATUM_SESSION is empty, omit the --session flag. +Without --session, the token is for the active session, or for the session +named by DATUM_SESSION when it is set. + If the stored token is expired, datumctl automatically uses the stored refresh token to obtain a new one before printing. @@ -50,19 +54,21 @@ Output formats (--output / -o): token Print the raw access token (default). client.authentication.k8s.io/v1 Print a Kubernetes ExecCredential JSON object for kubectl credential plugin use.`, - Example: ` # Get a raw token for use in a script or direct API call + Example: ` # Get a raw token for use in a script or direct API call datumctl auth get-token # Get a Kubernetes ExecCredential JSON object (used by kubectl automatically) datumctl auth get-token --output=client.authentication.k8s.io/v1`, - Args: cobra.NoArgs, - RunE: runGetToken, // Use single function -} + Args: cobra.NoArgs, + RunE: runGetToken, // Use single function + } -func init() { - // Add flags for direct execution mode - getTokenCmd.Flags().StringP("output", "o", outputFormatToken, fmt.Sprintf("Output format. One of: %s|%s", outputFormatToken, outputFormatK8sV1Creds)) - getTokenCmd.Flags().String("session", "", "Look up a specific session by name (defaults to the active session). Used by the kubectl exec plugin path so each kubeconfig entry pins to its own datumctl session.") + cmd.Flags().StringP("output", "o", outputFormatToken, fmt.Sprintf("Output format. One of: %s|%s", outputFormatToken, outputFormatK8sV1Creds)) + // This local --session shadows the global one on purpose: kubeconfig exec + // entries written by update-kubeconfig pass an exact session name here, and + // its lookup and errors must not change under them. + cmd.Flags().String("session", "", "Look up a specific session by name (defaults to the active session). Used by the kubectl exec plugin path so each kubeconfig entry pins to its own datumctl session.") + return cmd } // runGetToken implements the logic based on the --output flag. diff --git a/internal/cmd/auth/list.go b/internal/cmd/auth/list.go index 297ad4b..730e1e5 100644 --- a/internal/cmd/auth/list.go +++ b/internal/cmd/auth/list.go @@ -55,7 +55,7 @@ func runList(_ *cobra.Command, _ []string) error { for _, s := range cfg.Sessions { status := "" - if s.Name == cfg.ActiveSession { + if s.Name == cfg.ActiveSessionName() { status = "Active" } if showEndpoint { diff --git a/internal/cmd/auth/switch.go b/internal/cmd/auth/switch.go index 3daf498..47fe469 100644 --- a/internal/cmd/auth/switch.go +++ b/internal/cmd/auth/switch.go @@ -34,7 +34,14 @@ func switchCmd() *cobra.Command { without one, datumctl prints the command to run for each match. Each session remembers the last context you used, so switching users - also restores the context. To add a new account, run 'datumctl login'.`), + also restores the context. To add a new account, run 'datumctl login'. + + This command changes the active session, so it rejects the global + --session flag and ignores DATUM_SESSION. To run a single command as + another session without switching, pass --session to that command.`), + Annotations: map[string]string{ + datumconfig.SessionOverrideAnnotation: datumconfig.SessionOverrideRejected, + }, Example: templates.Examples(` # Interactive session picker datumctl auth switch diff --git a/internal/cmd/ctx/list.go b/internal/cmd/ctx/list.go index 9bab305..ea50ba6 100644 --- a/internal/cmd/ctx/list.go +++ b/internal/cmd/ctx/list.go @@ -118,7 +118,7 @@ func printContextTree(w io.Writer, cfg *datumconfig.ConfigV1Beta1, sessionName s if g.orgCtx != nil { current := "" - if cfg.CurrentContext == g.orgCtx.Name { + if cfg.CurrentContextName() == g.orgCtx.Name { current = "*" } tbl.AddRow(cfg.OrgDisplayName(sessionName, orgID), orgID, "org", current) @@ -126,7 +126,7 @@ func printContextTree(w io.Writer, cfg *datumconfig.ConfigV1Beta1, sessionName s for _, p := range g.projects { current := "" - if cfg.CurrentContext == p.Name { + if cfg.CurrentContextName() == p.Name { current = "*" } tbl.AddRow(" "+cfg.ProjectDisplayName(sessionName, p.ProjectID), p.Ref(), "project", current) diff --git a/internal/cmd/ctx/use.go b/internal/cmd/ctx/use.go index 3bd4238..225e194 100644 --- a/internal/cmd/ctx/use.go +++ b/internal/cmd/ctx/use.go @@ -17,7 +17,14 @@ func useCmd() *cobra.Command { Long: `Switch the active context to an organization or project. If no argument is provided, an interactive picker is shown. -Use the format 'org/project' to select a project context, or just 'org' for an org context.`, +Use the format 'org/project' to select a project context, or just 'org' for an org context. + +Switching context can change the active session, so this command rejects the +global --session flag and ignores DATUM_SESSION. To use another session's +context for one command, pass --session to that command instead.`, + Annotations: map[string]string{ + datumconfig.SessionOverrideAnnotation: datumconfig.SessionOverrideRejected, + }, Args: cobra.MaximumNArgs(1), RunE: runUse, } diff --git a/internal/cmd/landing.go b/internal/cmd/landing.go index 966229d..787ede1 100644 --- a/internal/cmd/landing.go +++ b/internal/cmd/landing.go @@ -337,6 +337,7 @@ var landingTips = []string{ "'datumctl plugin index add ' registers a team or community catalog.", // Power-user tips "Set DATUM_PROJECT or DATUM_ORGANIZATION to override context for a single command.", + "Pass --session or set DATUM_SESSION to run a command as another signed-in account without switching.", "'datumctl describe ' shows status conditions — handy for debugging.", "JSON and YAML both work with -f; mix them freely in a single directory.", "'datumctl version --client' prints the local version without hitting the server.", diff --git a/internal/cmd/login/login.go b/internal/cmd/login/login.go index ca5951b..65f6759 100644 --- a/internal/cmd/login/login.go +++ b/internal/cmd/login/login.go @@ -40,7 +40,13 @@ By default, opens your browser for OAuth2 PKCE authentication. Use --no-browser in headless environments (SSH, CI, containers) to authenticate via a device-code flow that does not need a browser on this machine. -Use --credentials to authenticate as a service account (non-interactive).`, +Use --credentials to authenticate as a service account (non-interactive). + +Login makes the new session active, so it rejects the global --session flag +and ignores DATUM_SESSION.`, + Annotations: map[string]string{ + datumconfig.SessionOverrideAnnotation: datumconfig.SessionOverrideRejected, + }, Example: ` # Log in (opens browser, then picks a context) datumctl login diff --git a/internal/cmd/logout/logout.go b/internal/cmd/logout/logout.go index 59fc898..3f29f12 100644 --- a/internal/cmd/logout/logout.go +++ b/internal/cmd/logout/logout.go @@ -24,7 +24,13 @@ func Command() *cobra.Command { Long: `Remove local authentication credentials. Specify an email to log out sessions for that user. -Use --all to log out all users.`, +Use --all to log out all users. + +Logout changes which sessions exist, so it rejects the global --session flag +and ignores DATUM_SESSION.`, + Annotations: map[string]string{ + datumconfig.SessionOverrideAnnotation: datumconfig.SessionOverrideRejected, + }, Args: func(cmd *cobra.Command, args []string) error { allFlag, _ := cmd.Flags().GetBool("all") if allFlag && len(args) > 0 { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 3c592ab..59438d7 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -103,7 +103,11 @@ terminal. No knowledge of Kubernetes or kubectl required. Get started: datumctl login datumctl get organizations - datumctl get dnszones`, + datumctl get dnszones + +Run one command as another signed-in account, leaving the active one as is: + datumctl get dnszones --session user@example.com@api.staging.env.datum.net + DATUM_SESSION=user@example.com datumctl get dnszones`, // ArbitraryArgs allows unknown subcommand names to reach RunE so the // plugin dispatch logic can handle them before Cobra rejects them. Args: cobra.ArbitraryArgs, @@ -122,6 +126,9 @@ Get started: "Allowed values: human, json, yaml.", ) } + if err := applySessionOverride(cmd); err != nil { + return err + } startUpdateCheck(cmd) handleUpdateCheck(cmd) return nil @@ -229,6 +236,8 @@ Get started: rootCmd.PersistentFlags().Bool("warnings-as-errors", false, "Treat warnings as errors") rootCmd.PersistentFlags().String("error-format", customerrors.FormatHuman, "Error output format on failure. One of: human, json, yaml.") + rootCmd.PersistentFlags().String(sessionFlag, "", + "Run this command as another signed-in session without changing the active one: a session name (email@api-host) or an email signed in on one endpoint. Overrides DATUM_SESSION.") ioStreams := genericclioptions.IOStreams{ In: rootCmd.InOrStdin(), Out: rootCmd.OutOrStdout(), @@ -772,7 +781,16 @@ Specify the resource type and name to view its history.` // ForwardPlugin replaces the process for managed plugins before cobra // parses flags. If it fails (e.g. integrity check), execution falls // through to cobra and the RunE path re-enforces the same checks. - _ = plugindispatch.ForwardPlugin(earlyPluginsDir, rootCmd, factory) + // + // A --session or DATUM_SESSION value that names no single session is the + // one failure reported here: falling through would run cobra over the + // plugin's own flags and bury the session error under "unknown flag". + if err := plugindispatch.ForwardPlugin(earlyPluginsDir, rootCmd, factory); err != nil { + if _, ok := customerrors.IsUserError(err); ok { + customerrors.Format(os.Stderr, err, customerrors.FormatHuman, 0) + os.Exit(1) + } + } return rootCmd } diff --git a/internal/cmd/session_override.go b/internal/cmd/session_override.go new file mode 100644 index 0000000..22ed825 --- /dev/null +++ b/internal/cmd/session_override.go @@ -0,0 +1,66 @@ +package cmd + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" +) + +// sessionFlag is the global flag that runs one command as a named session. +const sessionFlag = "session" + +// applySessionOverride installs the session named by --session, or else by +// DATUM_SESSION, as this process's session. Every lookup of the active session +// and current context goes through datumconfig, so installing it here reaches +// every command without touching the config file. +// +// Commands annotated with datumconfig.SessionOverrideAnnotation change the +// active session themselves: they reject --session and ignore DATUM_SESSION. +// A command that defines its own local --session flag (auth get-token) handles +// that flag itself; DATUM_SESSION still applies when the flag is not given. +func applySessionOverride(cmd *cobra.Command) error { + datumconfig.ClearSessionOverride() + + flag := cmd.Flags().Lookup(sessionFlag) + flagSet := flag != nil && flag.Changed + + if rejectsSessionOverride(cmd) { + if flagSet { + return customerrors.NewUserErrorWithHint( + fmt.Sprintf("'%s' changes the active session, so it does not accept --session.", cmd.CommandPath()), + "Run it without --session. It also ignores DATUM_SESSION.", + ) + } + return nil + } + + ownsFlag := flag != nil && flag != cmd.Root().PersistentFlags().Lookup(sessionFlag) + if flagSet { + if ownsFlag { + return nil + } + _, err := datumconfig.ApplySessionOverride(flag.Value.String(), datumconfig.SessionOverrideFromFlag) + return err + } + + if v := os.Getenv(datumconfig.SessionEnvVar); v != "" { + _, err := datumconfig.ApplySessionOverride(v, datumconfig.SessionOverrideFromEnv) + return err + } + return nil +} + +// rejectsSessionOverride reports whether cmd, or a parent of it, is annotated +// as changing the active session. +func rejectsSessionOverride(cmd *cobra.Command) bool { + for c := cmd; c != nil; c = c.Parent() { + if c.Annotations[datumconfig.SessionOverrideAnnotation] == datumconfig.SessionOverrideRejected { + return true + } + } + return false +} diff --git a/internal/cmd/session_override_test.go b/internal/cmd/session_override_test.go new file mode 100644 index 0000000..34323bb --- /dev/null +++ b/internal/cmd/session_override_test.go @@ -0,0 +1,436 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "golang.org/x/oauth2" + + "go.datum.net/datumctl/internal/authutil" + "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" + "go.datum.net/datumctl/internal/keyring" + "go.datum.net/datumctl/internal/updatecheck" +) + +const ( + ovSharedEmail = "swells@datum.net" + ovProdHost = "api.datum.net" + ovStagingHost = "api.staging.env.datum.net" + ovSoloEmail = "solo@example.com" +) + +var ( + ovProd = datumconfig.SessionName(ovSharedEmail, ovProdHost) + ovStaging = datumconfig.SessionName(ovSharedEmail, ovStagingHost) + ovSolo = datumconfig.SessionName(ovSoloEmail, ovProdHost) + ovProdCtx = datumconfig.QualifiedContextName(ovProd, "org-prod") + ovStagingCtx = datumconfig.QualifiedContextName(ovStaging, "org-staging/proj-staging") +) + +// setupOverrideEnv points HOME at a temp dir, mocks the keyring, and writes a +// config with one email signed in on prod and staging plus a second email +// signed in once. Prod is active. Staging remembers a project context; the solo +// session has no context. It returns the config path. +func setupOverrideEnv(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv(updatecheck.EnvDisable, "1") + t.Setenv(datumconfig.SessionEnvVar, "") + t.Setenv("DATUM_PROJECT", "") + t.Setenv("DATUM_ORGANIZATION", "") + keyring.MockInit() + t.Cleanup(datumconfig.ClearSessionOverride) + + cfg := datumconfig.NewV1Beta1() + cfg.Sessions = []datumconfig.Session{ + {Name: ovProd, UserKey: "key-prod", UserEmail: ovSharedEmail, UserName: "Scot Prod", + Endpoint: datumconfig.Endpoint{Server: "https://" + ovProdHost}, LastContext: ovProdCtx}, + {Name: ovStaging, UserKey: "key-staging", UserEmail: ovSharedEmail, UserName: "Scot Staging", + Endpoint: datumconfig.Endpoint{Server: "https://" + ovStagingHost}, LastContext: ovStagingCtx}, + {Name: ovSolo, UserKey: "key-solo", UserEmail: ovSoloEmail, UserName: "Solo", + Endpoint: datumconfig.Endpoint{Server: "https://" + ovProdHost}}, + } + cfg.Contexts = []datumconfig.DiscoveredContext{ + {Name: ovProdCtx, Session: ovProd, OrganizationID: "org-prod"}, + {Name: ovStagingCtx, Session: ovStaging, OrganizationID: "org-staging", ProjectID: "proj-staging"}, + } + cfg.ActiveSession = ovProd + cfg.CurrentContext = ovProdCtx + if err := datumconfig.SaveV1Beta1(cfg); err != nil { + t.Fatalf("save config: %v", err) + } + path, err := datumconfig.DefaultPath() + if err != nil { + t.Fatalf("config path: %v", err) + } + return path +} + +// runRoot executes a fresh root command and returns what the user sees: the +// command's output plus the formatted error, if any. +func runRoot(t *testing.T, args ...string) (string, error) { + t.Helper() + root := RootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs(args) + err := root.Execute() + if err != nil { + customerrors.Format(&out, err, customerrors.FormatHuman, 0) + } + return out.String(), err +} + +func readFile(t *testing.T, path string) []byte { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return b +} + +func TestSessionOverrideWhoami(t *testing.T) { + tests := []struct { + name string + env string + args []string + want []string + notWant []string + wantErr bool + }{ + { + name: "no override acts as the active session", + args: []string{"whoami"}, + want: []string{"User: Scot Prod (swells@datum.net)", "Context: org-prod"}, + notWant: []string{"Session:"}, + }, + { + name: "flag by session name uses that session and its last context", + args: []string{"whoami", "--session", ovStaging}, + want: []string{ + "User: Scot Staging (swells@datum.net)", + "Session: " + ovStaging + " (from --session; the active session is unchanged)", + "Endpoint: " + ovStagingHost, + "Context: org-staging/proj-staging", + "Project: proj-staging", + }, + }, + { + name: "flag before the command works too", + args: []string{"--session", ovStaging, "whoami"}, + want: []string{"User: Scot Staging (swells@datum.net)"}, + }, + { + name: "flag by unique email; session without a context has none", + args: []string{"whoami", "--session", ovSoloEmail}, + want: []string{ + "User: Solo (solo@example.com)", + "Session: " + ovSolo + " (from --session", + "Context: (none)", + "Pass --project or --organization", + }, + notWant: []string{"org-prod"}, + }, + { + name: "env var selects the session", + env: ovStaging, + args: []string{"whoami"}, + want: []string{ + "User: Scot Staging (swells@datum.net)", + "(from DATUM_SESSION; the active session is unchanged)", + }, + }, + { + name: "env var by unique email", + env: ovSoloEmail, + args: []string{"whoami"}, + want: []string{"User: Solo (solo@example.com)"}, + }, + { + name: "flag beats env var", + env: ovStaging, + args: []string{"whoami", "--session", ovSoloEmail}, + want: []string{"User: Solo (solo@example.com)", "(from --session;"}, + notWant: []string{"Scot Staging"}, + }, + { + name: "shared email fails and names each session", + args: []string{"whoami", "--session", ovSharedEmail}, + wantErr: true, + want: []string{ + "swells@datum.net is signed in on more than one endpoint, so --session swells@datum.net is ambiguous.", + "--session " + ovProd, + "--session " + ovStaging, + }, + }, + { + name: "shared email in env var fails the same way", + env: ovSharedEmail, + args: []string{"whoami"}, + wantErr: true, + want: []string{"so DATUM_SESSION swells@datum.net is ambiguous", "--session " + ovStaging}, + }, + { + name: "unknown value fails and lists valid sessions", + args: []string{"whoami", "--session", "nobody@example.com"}, + wantErr: true, + want: []string{ + "No session matches --session nobody@example.com.", + "Valid sessions:", + ovProd, ovStaging, ovSolo, + }, + }, + { + name: "unknown env var fails even when the flag is absent", + env: "nobody@example.com", + args: []string{"whoami"}, + wantErr: true, + want: []string{"No session matches DATUM_SESSION nobody@example.com."}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := setupOverrideEnv(t) + before := readFile(t, path) + t.Setenv(datumconfig.SessionEnvVar, tt.env) + + out, err := runRoot(t, tt.args...) + if (err != nil) != tt.wantErr { + t.Fatalf("err = %v, wantErr %v\noutput:\n%s", err, tt.wantErr, out) + } + if tt.wantErr { + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + } + for _, w := range tt.want { + if !strings.Contains(out, w) { + t.Errorf("output missing %q\noutput:\n%s", w, out) + } + } + for _, nw := range tt.notWant { + if strings.Contains(out, nw) { + t.Errorf("output unexpectedly contains %q\noutput:\n%s", nw, out) + } + } + if after := readFile(t, path); !bytes.Equal(before, after) { + t.Errorf("config file changed:\nbefore:\n%s\nafter:\n%s", before, after) + } + }) + } +} + +// Each process picks its own session; an override must not leak into the next +// command, and a command with no override acts as the active session again. +func TestSessionOverrideSequence(t *testing.T) { + path := setupOverrideEnv(t) + before := readFile(t, path) + + steps := []struct { + env string + args []string + wantUser string + }{ + {args: []string{"whoami", "--session", ovStaging}, wantUser: "Scot Staging"}, + {args: []string{"whoami", "--session", ovSoloEmail}, wantUser: "Solo"}, + {env: ovStaging, args: []string{"whoami"}, wantUser: "Scot Staging"}, + {args: []string{"whoami"}, wantUser: "Scot Prod"}, + {args: []string{"whoami", "--session", "nobody@example.com"}}, + {args: []string{"whoami"}, wantUser: "Scot Prod"}, + } + for i, s := range steps { + t.Setenv(datumconfig.SessionEnvVar, s.env) + out, err := runRoot(t, s.args...) + if s.wantUser == "" { + if err == nil { + t.Fatalf("step %d: expected an error\n%s", i, out) + } + continue + } + if err != nil { + t.Fatalf("step %d: %v\n%s", i, err, out) + } + if !strings.Contains(out, "User: "+s.wantUser+" (") { + t.Errorf("step %d (%v): want user %q\noutput:\n%s", i, s.args, s.wantUser, out) + } + if s.wantUser == "Scot Prod" && strings.Contains(out, "Session:") { + t.Errorf("step %d: override leaked into a command without one\n%s", i, out) + } + } + if after := readFile(t, path); !bytes.Equal(before, after) { + t.Errorf("config file changed across the sequence") + } +} + +func TestSessionOverrideRejectedByActiveSessionCommands(t *testing.T) { + tests := []struct { + name string + args []string + cmd string + }{ + {"login", []string{"login", "--session", ovStaging}, "datumctl login"}, + {"logout", []string{"logout", ovSoloEmail, "--session", ovStaging}, "datumctl logout"}, + {"auth switch", []string{"auth", "switch", ovSoloEmail, "--session", ovStaging}, "datumctl auth switch"}, + {"ctx use", []string{"ctx", "use", "org-prod", "--session", ovStaging}, "datumctl ctx use"}, + {"flag before the command", []string{"--session", ovStaging, "auth", "switch", ovSoloEmail}, "datumctl auth switch"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := setupOverrideEnv(t) + before := readFile(t, path) + + out, err := runRoot(t, tt.args...) + if err == nil { + t.Fatalf("expected an error\noutput:\n%s", out) + } + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + want := "'" + tt.cmd + "' changes the active session, so it does not accept --session." + if !strings.Contains(out, want) { + t.Errorf("output missing %q\noutput:\n%s", want, out) + } + if after := readFile(t, path); !bytes.Equal(before, after) { + t.Errorf("config file changed after a rejected command") + } + }) + } +} + +// Commands that change the active session ignore DATUM_SESSION, even when it +// is unresolvable, and act on the stored active session as before. +func TestSessionOverrideEnvIgnoredByActiveSessionCommands(t *testing.T) { + for _, env := range []string{ovStaging, "nobody@example.com"} { + t.Run(env, func(t *testing.T) { + setupOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, env) + + if out, err := runRoot(t, "auth", "switch", ovSoloEmail); err != nil { + t.Fatalf("auth switch: %v\n%s", err, out) + } + cfg, err := datumconfig.LoadAuto() + if err != nil { + t.Fatal(err) + } + if cfg.ActiveSession != ovSolo { + t.Errorf("active session = %q, want %q", cfg.ActiveSession, ovSolo) + } + + // ctx use resolves in the stored active session (solo now has no + // contexts), not in the DATUM_SESSION session. + out, err := runRoot(t, "ctx", "use", "org-staging/proj-staging") + if err == nil { + t.Fatalf("ctx use should resolve in the active session, not DATUM_SESSION\n%s", out) + } + if strings.Contains(out, "No session matches") { + t.Errorf("ctx use read DATUM_SESSION:\n%s", out) + } + }) + } +} + +// captureStdout runs fn and returns what it wrote to os.Stdout. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + orig := os.Stdout + os.Stdout = w + defer func() { os.Stdout = orig }() + fn() + w.Close() + b, _ := io.ReadAll(r) + return string(b) +} + +func seedToken(t *testing.T, userKey, token string) { + t.Helper() + creds := authutil.StoredCredentials{ + Hostname: "auth.datum.net", + Token: &oauth2.Token{AccessToken: token, Expiry: time.Now().Add(time.Hour)}, + } + blob, err := json.Marshal(creds) + if err != nil { + t.Fatal(err) + } + if err := keyring.Set(authutil.ServiceName, userKey, string(blob)); err != nil { + t.Fatal(err) + } +} + +// Kubeconfig exec entries call "auth get-token --session "; that must +// keep returning that session's token, whatever DATUM_SESSION says. +func TestGetTokenSessionFlagUnchanged(t *testing.T) { + tests := []struct { + name string + env string + args []string + want string + }{ + {"session flag by name", "", []string{"auth", "get-token", "--session", ovStaging}, "tok-staging"}, + {"session flag beats env var", ovSoloEmail, []string{"auth", "get-token", "--session", ovStaging}, "tok-staging"}, + {"no flag uses the active session", "", []string{"auth", "get-token"}, "tok-prod"}, + {"no flag honors DATUM_SESSION", ovSoloEmail, []string{"auth", "get-token"}, "tok-solo"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := setupOverrideEnv(t) + seedToken(t, "key-prod", "tok-prod") + seedToken(t, "key-staging", "tok-staging") + seedToken(t, "key-solo", "tok-solo") + before := readFile(t, path) + t.Setenv(datumconfig.SessionEnvVar, tt.env) + + var err error + var out string + got := captureStdout(t, func() { out, err = runRoot(t, tt.args...) }) + if err != nil { + t.Fatalf("get-token: %v\n%s", err, out) + } + if got != tt.want { + t.Errorf("token = %q, want %q", got, tt.want) + } + if after := readFile(t, path); !bytes.Equal(before, after) { + t.Errorf("config file changed") + } + }) + } + + // The local flag keeps its old, exact-name-only error. + setupOverrideEnv(t) + _, err := runRoot(t, "auth", "get-token", "--session", ovSoloEmail) + if err == nil || !strings.Contains(err.Error(), `no session named "`+ovSoloEmail+`"`) { + t.Errorf("get-token --session error = %v, want the unchanged exact-name error", err) + } +} + +// The overriding session's context never leaks into the config file, even +// when a command saves the config for other reasons. +func TestSessionOverrideLeavesStoredSessionAlone(t *testing.T) { + path := setupOverrideEnv(t) + if _, err := runRoot(t, "ctx", "list", "--session", ovStaging); err != nil { + t.Fatalf("ctx list: %v", err) + } + cfg, err := datumconfig.LoadAutoFromPath(filepath.Clean(path)) + if err != nil { + t.Fatal(err) + } + if cfg.ActiveSession != ovProd || cfg.CurrentContext != ovProdCtx { + t.Errorf("stored active session/context = %q/%q, want %q/%q", + cfg.ActiveSession, cfg.CurrentContext, ovProd, ovProdCtx) + } +} diff --git a/internal/cmd/whoami/whoami.go b/internal/cmd/whoami/whoami.go index 9dff208..1809604 100644 --- a/internal/cmd/whoami/whoami.go +++ b/internal/cmd/whoami/whoami.go @@ -3,9 +3,11 @@ package whoami import ( "context" "fmt" + "io" "os" "github.com/spf13/cobra" + "k8s.io/kubectl/pkg/util/templates" "go.datum.net/datumctl/internal/authutil" "go.datum.net/datumctl/internal/datumconfig" @@ -17,12 +19,25 @@ func Command() *cobra.Command { return &cobra.Command{ Use: "whoami", Short: "Show the current user and context", - Args: cobra.NoArgs, - RunE: runWhoami, + Long: templates.LongDesc(` + Show the account and context datumctl commands run as. + + With --session or DATUM_SESSION, shows the account and context that + session gives, and notes that the active session is unchanged.`), + Example: templates.Examples(` + # Show the active account and context + datumctl whoami + + # Show what a command run as another session would use + datumctl whoami --session user@example.com@api.staging.env.datum.net`), + Args: cobra.NoArgs, + RunE: runWhoami, } } func runWhoami(cmd *cobra.Command, _ []string) error { + out := cmd.OutOrStdout() + cfg, err := datumconfig.LoadAuto() if err != nil { return err @@ -36,58 +51,64 @@ func runWhoami(cmd *cobra.Command, _ []string) error { return authutil.ErrNoActiveUser } - // Get user info from stored credentials for freshest data. - creds, err := authutil.GetStoredCredentials(session.UserKey) - if err != nil { - return fmt.Errorf("get credentials: %w", err) + // Prefer stored credentials for the freshest name and email; the session + // record carries both too, so missing credentials are not fatal here. + userName, userEmail := session.UserName, session.UserEmail + if creds, err := authutil.GetStoredCredentials(session.UserKey); err == nil { + if creds.UserName != "" { + userName = creds.UserName + } + if creds.UserEmail != "" { + userEmail = creds.UserEmail + } } - userName := creds.UserName - if userName == "" { - userName = session.UserName - } - userEmail := creds.UserEmail - if userEmail == "" { - userEmail = session.UserEmail - } + fmt.Fprintf(out, "User: %s (%s)\n", userName, userEmail) - fmt.Printf("User: %s (%s)\n", userName, userEmail) + overrideName, overrideSource := datumconfig.SessionOverride() + if overrideName != "" { + fmt.Fprintf(out, "Session: %s (from %s; the active session is unchanged)\n", overrideName, overrideSource) + } - printOnboardingStatus(cmd.Context(), cfg, session) + printOnboardingStatus(cmd.Context(), out, cfg, session) // Show endpoint only when multiple endpoints are in use. if cfg.HasMultipleEndpoints() { - fmt.Printf("Endpoint: %s\n", datumconfig.StripScheme(session.Endpoint.Server)) + fmt.Fprintf(out, "Endpoint: %s\n", datumconfig.StripScheme(session.Endpoint.Server)) } ctxEntry := cfg.CurrentContextEntry() if ctxEntry != nil { - fmt.Printf("Context: %s\n", ctxEntry.Ref()) + fmt.Fprintf(out, "Context: %s\n", ctxEntry.Ref()) - fmt.Printf("Organization: %s\n", datumconfig.FormatWithID( + fmt.Fprintf(out, "Organization: %s\n", datumconfig.FormatWithID( cfg.OrgDisplayName(ctxEntry.Session, ctxEntry.OrganizationID), ctxEntry.OrganizationID)) if ctxEntry.ProjectID != "" { - fmt.Printf("Project: %s\n", datumconfig.FormatWithID( + fmt.Fprintf(out, "Project: %s\n", datumconfig.FormatWithID( cfg.ProjectDisplayName(ctxEntry.Session, ctxEntry.ProjectID), ctxEntry.ProjectID)) } } else { - fmt.Println("Context: (none)") - fmt.Println(" Run 'datumctl ctx use' to select a context.") + fmt.Fprintln(out, "Context: (none)") + if overrideName != "" { + fmt.Fprintln(out, " Pass --project or --organization to choose a scope for this session.") + } else { + fmt.Fprintln(out, " Run 'datumctl ctx use' to select a context.") + } } // Surface env-var overrides — these silently override the active context. if v := os.Getenv("DATUM_PROJECT"); v != "" { - fmt.Printf("\nOverride: DATUM_PROJECT=%s (overrides context project)\n", v) + fmt.Fprintf(out, "\nOverride: DATUM_PROJECT=%s (overrides context project)\n", v) } if v := os.Getenv("DATUM_ORGANIZATION"); v != "" { - fmt.Printf("\nOverride: DATUM_ORGANIZATION=%s (overrides context organization)\n", v) + fmt.Fprintf(out, "\nOverride: DATUM_ORGANIZATION=%s (overrides context organization)\n", v) } return nil } -func printOnboardingStatus(ctx context.Context, cfg *datumconfig.ConfigV1Beta1, session *datumconfig.Session) { +func printOnboardingStatus(ctx context.Context, out io.Writer, cfg *datumconfig.ConfigV1Beta1, session *datumconfig.Session) { orgID := onboarding.ResolveEffectiveOrgID(cfg, os.Getenv("DATUM_PROJECT"), os.Getenv("DATUM_ORGANIZATION")) if orgID == "" { return @@ -108,13 +129,12 @@ func printOnboardingStatus(ctx context.Context, cfg *datumconfig.ConfigV1Beta1, result, err := onboarding.CheckOrg(ctx, apiHostname, tknSrc, userID, orgID, cfg.OrgDisplayName(session.Name, orgID)) if err != nil { - fmt.Println("Onboarding: couldn't check") + fmt.Fprintln(out, "Onboarding: couldn't check") return } - fmt.Printf("Onboarding: %s\n", onboarding.StatusLabel(result)) + fmt.Fprintf(out, "Onboarding: %s\n", onboarding.StatusLabel(result)) if result.State != onboarding.Complete { - fmt.Printf(" Finish setup at %s\n", result.ActionURL) + fmt.Fprintf(out, " Finish setup at %s\n", result.ActionURL) } } - diff --git a/internal/console/components/ctxswitcher.go b/internal/console/components/ctxswitcher.go index d77a057..1b27188 100644 --- a/internal/console/components/ctxswitcher.go +++ b/internal/console/components/ctxswitcher.go @@ -124,6 +124,13 @@ func (m CtxSwitcherModel) Update(msg tea.Msg) (CtxSwitcherModel, tea.Cmd) { if e.isHeader || e.ctx == nil { return m, nil } + // Under --session or DATUM_SESSION the switch lasts only for this + // console; the stored current context stays unchanged. + if datumconfig.HasSessionOverride() { + datumconfig.SetOverrideContext(e.ctx.Name) + newCtx := tuictx.FromConfig(m.cfg) + return m, func() tea.Msg { return ContextSwitchedMsg{Ctx: newCtx} } + } m.cfg.CurrentContext = e.ctx.Name if err := datumconfig.SaveV1Beta1(m.cfg); err != nil { return m, func() tea.Msg { @@ -180,7 +187,7 @@ func (m CtxSwitcherModel) View() string { lines = append(lines, headerStyle.Render("▾ "+e.label)) continue } - isCurrent := m.cfg != nil && e.ctx != nil && e.ctx.Name == m.cfg.CurrentContext + isCurrent := m.cfg != nil && e.ctx != nil && e.ctx.Name == m.cfg.CurrentContextName() indent := " " var line string switch { diff --git a/internal/console/model.go b/internal/console/model.go index d39cb1e..adfda06 100644 --- a/internal/console/model.go +++ b/internal/console/model.go @@ -122,7 +122,11 @@ func finishDeviceAuthCmd(ctx context.Context, session *authutil.DeviceAuthSessio } s := authutil.BuildSession(result, session.AuthHostname) cfg.UpsertSession(s) - cfg.ActiveSession = s.Name + // A console started with --session or DATUM_SESSION keeps the stored + // active session unchanged, even across a re-login. + if !datumconfig.HasSessionOverride() { + cfg.ActiveSession = s.Name + } tknSrc, tErr := authutil.GetTokenSourceForUser(ctx, result.UserKey) if tErr == nil { orgs, projects, _ := discovery.FetchOrgsAndProjects(ctx, result.APIHostname, tknSrc, result.Subject) diff --git a/internal/datumconfig/config_v1beta1.go b/internal/datumconfig/config_v1beta1.go index e6fff28..40ae4cb 100644 --- a/internal/datumconfig/config_v1beta1.go +++ b/internal/datumconfig/config_v1beta1.go @@ -434,7 +434,13 @@ func appendUnique(s []string, v string) []string { } // CurrentContextEntry returns the active context, or nil if none is set. +// Under a session override (see SetSessionOverride) it returns a context owned +// by the overriding session, falling back to that session's last-used context, +// so a command never pairs one session's credentials with another's scope. func (c *ConfigV1Beta1) CurrentContextEntry() *DiscoveredContext { + if name, _ := SessionOverride(); name != "" { + return c.overrideContextEntry(name) + } if c.CurrentContext == "" { return nil } @@ -445,8 +451,12 @@ func (c *ConfigV1Beta1) CurrentContextEntry() *DiscoveredContext { // is authoritative: whatever context is selected determines which environment // is active, so whoami and every request agree. The stored ActiveSession is // only a fallback for when no current context resolves (e.g. right after login -// before a context is picked). +// before a context is picked). A session override (--session or DATUM_SESSION) +// takes precedence over both for the life of the process. func (c *ConfigV1Beta1) ActiveSessionEntry() *Session { + if name, _ := SessionOverride(); name != "" { + return c.SessionByName(name) + } if ctx := c.CurrentContextEntry(); ctx != nil { if s := c.SessionByName(ctx.Session); s != nil { return s diff --git a/internal/datumconfig/session_override.go b/internal/datumconfig/session_override.go new file mode 100644 index 0000000..764efbf --- /dev/null +++ b/internal/datumconfig/session_override.go @@ -0,0 +1,196 @@ +package datumconfig + +import ( + "fmt" + "strings" + "sync" + + customerrors "go.datum.net/datumctl/internal/errors" +) + +// SessionEnvVar names the environment variable that runs one datumctl process +// as a given session. The --session flag takes precedence over it. +const SessionEnvVar = "DATUM_SESSION" + +// SessionOverrideAnnotation marks a command that changes the active session or +// its login. Such commands reject --session and ignore DATUM_SESSION, since +// acting "as another session" while changing which session is active is +// contradictory. +const SessionOverrideAnnotation = "datumctl.session-override" + +// SessionOverrideRejected is the SessionOverrideAnnotation value for commands +// that reject a session override. +const SessionOverrideRejected = "reject" + +// SessionOverrideSource records where a session override came from, so output +// can tell the user why the command is not running as the active session. +type SessionOverrideSource string + +const ( + SessionOverrideFromFlag SessionOverrideSource = "--session" + SessionOverrideFromEnv SessionOverrideSource = SessionEnvVar +) + +// sessionOverride is the process-wide session override. It lives only in +// memory: ActiveSessionEntry and CurrentContextEntry consult it, but it is +// never a field of ConfigV1Beta1, so saving the config can never persist it. +var sessionOverride struct { + mu sync.RWMutex + name string + source SessionOverrideSource + context string // context picked in this process (console switcher) +} + +// SetSessionOverride makes every lookup of the active session in this process +// return the named session, without changing the stored active session or +// current context. name must be a session name; resolve user input first with +// ResolveSessionSelector. +func SetSessionOverride(name string, source SessionOverrideSource) { + sessionOverride.mu.Lock() + defer sessionOverride.mu.Unlock() + sessionOverride.name = name + sessionOverride.source = source + sessionOverride.context = "" +} + +// ClearSessionOverride removes the process-wide session override. +func ClearSessionOverride() { + SetSessionOverride("", "") +} + +// SessionOverride returns the overriding session name and where it came from, +// or ("", "") when the process runs as the stored active session. +func SessionOverride() (string, SessionOverrideSource) { + sessionOverride.mu.RLock() + defer sessionOverride.mu.RUnlock() + return sessionOverride.name, sessionOverride.source +} + +// HasSessionOverride reports whether this process runs as an overriding session. +func HasSessionOverride() bool { + name, _ := SessionOverride() + return name != "" +} + +// SetOverrideContext selects a context for the rest of this process while a +// session override is active, in place of writing current-context to disk. It +// is ignored when no override is active or the context belongs to another +// session. +func SetOverrideContext(name string) { + sessionOverride.mu.Lock() + defer sessionOverride.mu.Unlock() + if sessionOverride.name != "" { + sessionOverride.context = name + } +} + +func overrideContext() string { + sessionOverride.mu.RLock() + defer sessionOverride.mu.RUnlock() + return sessionOverride.context +} + +// ActiveSessionName returns the name of the session this process acts as: the +// session override when one is set, else the stored active session. +func (c *ConfigV1Beta1) ActiveSessionName() string { + if s := c.ActiveSessionEntry(); s != nil { + return s.Name + } + if name, _ := SessionOverride(); name != "" { + return name + } + return c.ActiveSession +} + +// CurrentContextName returns the name of the context this process acts in, or +// "" when there is none. Callers comparing against the current context use it +// instead of the stored CurrentContext so a session override is honored. +func (c *ConfigV1Beta1) CurrentContextName() string { + if ctx := c.CurrentContextEntry(); ctx != nil { + return ctx.Name + } + return "" +} + +// overrideContextEntry returns the context for the overriding session: a +// context picked earlier in this process, else the stored current context when +// it belongs to that session, else the session's last-used context, else nil. +func (c *ConfigV1Beta1) overrideContextEntry(sessionName string) *DiscoveredContext { + candidates := []string{overrideContext(), c.CurrentContext} + if s := c.SessionByName(sessionName); s != nil { + candidates = append(candidates, s.LastContext) + } + for _, name := range candidates { + if name == "" { + continue + } + if ctx := c.ContextByName(name); ctx != nil && ctx.Session == sessionName { + return ctx + } + } + return nil +} + +// ResolveSessionSelector finds the session a --session flag or DATUM_SESSION +// value names: an exact session name (email@api-host), or an email signed in +// on exactly one endpoint. An email signed in on several endpoints, or a value +// matching nothing, returns a UserError listing the valid choices. +func (c *ConfigV1Beta1) ResolveSessionSelector(value string, source SessionOverrideSource) (*Session, error) { + value = strings.TrimSpace(value) + if s := c.SessionByName(value); s != nil { + return s, nil + } + matches := c.SessionByEmail(value) + if len(matches) == 1 { + return matches[0], nil + } + + if len(matches) > 1 { + var b strings.Builder + b.WriteString("Name the session instead:") + for _, s := range matches { + fmt.Fprintf(&b, "\n --session %s", s.Name) + } + return nil, customerrors.NewUserErrorWithHint( + fmt.Sprintf("%s is signed in on more than one endpoint, so %s %s is ambiguous.", value, source, value), + b.String(), + ) + } + + if len(c.Sessions) == 0 { + return nil, customerrors.NewUserErrorWithHint( + fmt.Sprintf("No session matches %s %s: no accounts are signed in.", source, value), + "Run 'datumctl login' to authenticate.", + ) + } + var b strings.Builder + b.WriteString("Valid sessions:") + for _, s := range c.Sessions { + fmt.Fprintf(&b, "\n %s", s.Name) + } + b.WriteString("\nPass a session name, or an email signed in on one endpoint. Run 'datumctl auth list' to see accounts.") + return nil, customerrors.NewUserErrorWithHint( + fmt.Sprintf("No session matches %s %s.", source, value), + b.String(), + ) +} + +// ApplySessionOverride resolves value against the config at the default path +// and, when it names a session, installs it as the process-wide override. An +// empty value clears any override. It returns the selected session. +func ApplySessionOverride(value string, source SessionOverrideSource) (*Session, error) { + if strings.TrimSpace(value) == "" { + ClearSessionOverride() + return nil, nil + } + cfg, err := LoadAuto() + if err != nil { + return nil, err + } + s, err := cfg.ResolveSessionSelector(value, source) + if err != nil { + return nil, err + } + SetSessionOverride(s.Name, source) + return s, nil +} diff --git a/internal/datumconfig/session_override_test.go b/internal/datumconfig/session_override_test.go new file mode 100644 index 0000000..4f79225 --- /dev/null +++ b/internal/datumconfig/session_override_test.go @@ -0,0 +1,159 @@ +package datumconfig + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +const ( + soProd = "swells@datum.net@api.datum.net" + soStaging = "swells@datum.net@api.staging.env.datum.net" + soSolo = "solo@example.com@api.datum.net" +) + +var ( + soProdCtx = QualifiedContextName(soProd, "org-prod") + soStagingCtx = QualifiedContextName(soStaging, "org-staging") + soStagingProj = QualifiedContextName(soStaging, "org-staging/proj") +) + +func overrideTestConfig() *ConfigV1Beta1 { + cfg := NewV1Beta1() + cfg.Sessions = []Session{ + {Name: soProd, UserEmail: "swells@datum.net", LastContext: soProdCtx}, + {Name: soStaging, UserEmail: "swells@datum.net", LastContext: soStagingCtx}, + {Name: soSolo, UserEmail: "solo@example.com"}, + } + cfg.Contexts = []DiscoveredContext{ + {Name: soProdCtx, Session: soProd, OrganizationID: "org-prod"}, + {Name: soStagingCtx, Session: soStaging, OrganizationID: "org-staging"}, + {Name: soStagingProj, Session: soStaging, OrganizationID: "org-staging", ProjectID: "proj"}, + } + cfg.ActiveSession = soProd + cfg.CurrentContext = soProdCtx + return cfg +} + +func TestResolveSessionSelector(t *testing.T) { + tests := []struct { + name string + value string + source SessionOverrideSource + want string + wantErr []string + }{ + {name: "exact session name", value: soStaging, want: soStaging}, + {name: "surrounding space is ignored", value: " " + soStaging + " ", want: soStaging}, + {name: "unique email", value: "solo@example.com", want: soSolo}, + {name: "shared email lists each --session", value: "swells@datum.net", source: SessionOverrideFromFlag, + wantErr: []string{"--session swells@datum.net is ambiguous", "--session " + soProd, "--session " + soStaging}}, + {name: "unknown value lists every session", value: "nobody@example.com", source: SessionOverrideFromEnv, + wantErr: []string{"No session matches DATUM_SESSION nobody@example.com.", soProd, soStaging, soSolo}}, + {name: "email suffix is not a match", value: "datum.net", source: SessionOverrideFromFlag, + wantErr: []string{"No session matches --session datum.net."}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, err := overrideTestConfig().ResolveSessionSelector(tt.value, tt.source) + if len(tt.wantErr) > 0 { + if err == nil { + t.Fatalf("expected an error, got session %q", s.Name) + } + for _, w := range tt.wantErr { + if !strings.Contains(err.Error(), w) { + t.Errorf("error missing %q:\n%s", w, err) + } + } + return + } + if err != nil { + t.Fatal(err) + } + if s.Name != tt.want { + t.Errorf("session = %q, want %q", s.Name, tt.want) + } + }) + } + + _, err := NewV1Beta1().ResolveSessionSelector("x", SessionOverrideFromFlag) + if err == nil || !strings.Contains(err.Error(), "datumctl login") { + t.Errorf("no sessions: err = %v, want a hint to log in", err) + } +} + +func TestSessionOverrideLookups(t *testing.T) { + tests := []struct { + name string + override string + current string // stored current-context, when not the default + pick string // SetOverrideContext + wantSession string + wantContext string + }{ + {name: "no override", wantSession: soProd, wantContext: soProdCtx}, + {name: "override uses its last context", override: soStaging, wantSession: soStaging, wantContext: soStagingCtx}, + {name: "override without a context has none", override: soSolo, wantSession: soSolo, wantContext: ""}, + {name: "override matching the active session keeps the current context", + override: soStaging, current: soStagingProj, wantSession: soStaging, wantContext: soStagingProj}, + {name: "context picked in this process wins", + override: soStaging, pick: soStagingProj, wantSession: soStaging, wantContext: soStagingProj}, + {name: "context picked from another session is ignored", + override: soStaging, pick: soProdCtx, wantSession: soStaging, wantContext: soStagingCtx}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Cleanup(ClearSessionOverride) + cfg := overrideTestConfig() + if tt.current != "" { + cfg.CurrentContext = tt.current + } + if tt.override != "" { + SetSessionOverride(tt.override, SessionOverrideFromFlag) + } + if tt.pick != "" { + SetOverrideContext(tt.pick) + } + if got := cfg.ActiveSessionEntry(); got == nil || got.Name != tt.wantSession { + t.Errorf("ActiveSessionEntry = %v, want %q", got, tt.wantSession) + } + if got := cfg.ActiveSessionName(); got != tt.wantSession { + t.Errorf("ActiveSessionName = %q, want %q", got, tt.wantSession) + } + if got := cfg.CurrentContextName(); got != tt.wantContext { + t.Errorf("CurrentContextName = %q, want %q", got, tt.wantContext) + } + }) + } +} + +// Saving a config while an override is active writes the stored active +// session and current context, never the override. +func TestSessionOverrideIsNeverSaved(t *testing.T) { + t.Cleanup(ClearSessionOverride) + path := filepath.Join(t.TempDir(), "config") + cfg := overrideTestConfig() + if err := SaveV1Beta1ToPath(cfg, path); err != nil { + t.Fatal(err) + } + before, _ := os.ReadFile(path) + + SetSessionOverride(soStaging, SessionOverrideFromEnv) + SetOverrideContext(soStagingProj) + loaded, err := LoadV1Beta1FromPath(path) + if err != nil { + t.Fatal(err) + } + if loaded.CurrentContextName() != soStagingProj { + t.Fatalf("override not in effect") + } + if err := SaveV1Beta1ToPath(loaded, path); err != nil { + t.Fatal(err) + } + after, _ := os.ReadFile(path) + if !bytes.Equal(before, after) { + t.Errorf("config changed by saving under an override:\nbefore:\n%s\nafter:\n%s", before, after) + } +} diff --git a/internal/picker/picker.go b/internal/picker/picker.go index 516bdf4..3be3458 100644 --- a/internal/picker/picker.go +++ b/internal/picker/picker.go @@ -104,7 +104,7 @@ func buildContextOptions(contexts []datumconfig.DiscoveredContext, cfg *datumcon // Org entry — show display name with resource name when they differ. if g.orgCtx != nil { label := datumconfig.FormatWithID(cfg.OrgDisplayName(g.orgCtx.Session, orgID), orgID) - if cfg.CurrentContext == g.orgCtx.Name { + if cfg.CurrentContextName() == g.orgCtx.Name { label += " *" } options = append(options, huh.NewOption(label, g.orgCtx.Name)) @@ -113,7 +113,7 @@ func buildContextOptions(contexts []datumconfig.DiscoveredContext, cfg *datumcon // Project entries, indented under their org. for _, p := range g.projects { label := " " + datumconfig.FormatWithID(cfg.ProjectDisplayName(p.Session, p.ProjectID), p.ProjectID) - if cfg.CurrentContext == p.Name { + if cfg.CurrentContextName() == p.Name { label += " *" } options = append(options, huh.NewOption(label, p.Name)) diff --git a/internal/plugindispatch/forward.go b/internal/plugindispatch/forward.go index 41be381..2afc25b 100644 --- a/internal/plugindispatch/forward.go +++ b/internal/plugindispatch/forward.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "go.datum.net/datumctl/internal/client" + "go.datum.net/datumctl/internal/datumconfig" "go.datum.net/datumctl/internal/pluginstore" ) @@ -33,8 +34,13 @@ import ( // Must be called after the cobra command tree is fully built so that IsBuiltIn // can correctly distinguish plugin names from registered subcommands. // On success this function does not return (process is replaced). +// +// A leading global --session flag ("datumctl --session X ...") or the +// DATUM_SESSION environment variable runs the plugin as that session. When the +// session cannot be resolved, ForwardPlugin returns the UserError so the caller +// can report it; the plugin is not run. func ForwardPlugin(pluginsDir string, root *cobra.Command, factory *client.DatumCloudFactory) error { - args := os.Args[1:] + sessionValue, args, hasSessionFlag := splitLeadingSessionFlag(os.Args[1:]) if len(args) == 0 || strings.HasPrefix(args[0], "-") { return nil } @@ -83,9 +89,46 @@ func ForwardPlugin(pluginsDir string, root *cobra.Command, factory *client.Datum } } + if err := applyPluginSessionOverride(sessionValue, hasSessionFlag); err != nil { + return err + } + return Exec(binaryPath, args[1:], factory) } +// splitLeadingSessionFlag removes global --session flags that appear before +// the plugin name ("--session X" or "--session=X") and returns the last value +// given, the remaining arguments, and whether the flag was present. Arguments +// after the plugin name belong to the plugin and are left alone. +func splitLeadingSessionFlag(args []string) (value string, rest []string, found bool) { + for len(args) > 0 { + switch { + case args[0] == "--session" && len(args) > 1: + value, found, args = args[1], true, args[2:] + case strings.HasPrefix(args[0], "--session="): + value, found, args = strings.TrimPrefix(args[0], "--session="), true, args[1:] + default: + return value, args, found + } + } + return value, args, found +} + +// applyPluginSessionOverride installs the session a plugin runs as: the +// --session flag when given, else DATUM_SESSION. BuildEnv then reports that +// session's name and API host to the plugin. +func applyPluginSessionOverride(flagValue string, hasFlag bool) error { + if hasFlag { + _, err := datumconfig.ApplySessionOverride(flagValue, datumconfig.SessionOverrideFromFlag) + return err + } + if v := os.Getenv(datumconfig.SessionEnvVar); v != "" { + _, err := datumconfig.ApplySessionOverride(v, datumconfig.SessionOverrideFromEnv) + return err + } + return nil +} + // VerifyManagedPluginIntegrity loads plugins.json, finds the entry for name, // hashes the binary at binaryPath, and compares against InstalledPlugin.SHA256. // @@ -215,6 +258,9 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err // completion. Non-fatal: if env construction fails we proceed without injection // and the plugin will return empty candidates instead of erroring. if factory != nil { + // Honor DATUM_SESSION so completion lists what the plugin will act on. + // An unresolvable value is ignored here; running the plugin reports it. + _ = applyPluginSessionOverride("", false) if env, buildErr := BuildEnv(factory); buildErr == nil { cmd.Env = overlayEnv(os.Environ(), env) } @@ -242,7 +288,9 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err // Cobra intercepts --help before RunE fires, so this must be called before // cobra.Execute(), mirroring the ForwardCompletion pattern. func ForwardHelp(pluginsDir string) error { - args := os.Args[1:] // strip "datumctl" + // Strip "datumctl" and any leading --session, so help for + // "datumctl --session X --help" reaches the plugin too. + _, args, _ := splitLeadingSessionFlag(os.Args[1:]) if len(args) == 0 { return nil } diff --git a/internal/plugindispatch/session_override_test.go b/internal/plugindispatch/session_override_test.go new file mode 100644 index 0000000..4b60897 --- /dev/null +++ b/internal/plugindispatch/session_override_test.go @@ -0,0 +1,173 @@ +package plugindispatch + +import ( + "context" + "slices" + "strings" + "testing" + + "go.datum.net/datumctl/internal/client" + "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" + "go.datum.net/datumctl/internal/keyring" +) + +const ( + ovProd = "swells@datum.net@api.datum.net" + ovStaging = "swells@datum.net@api.staging.env.datum.net" + ovSolo = "solo@example.com@api.datum.net" +) + +// setupPluginOverrideEnv writes a config where prod is active and staging +// remembers a project context. +func setupPluginOverrideEnv(t *testing.T) *client.DatumCloudFactory { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv(datumconfig.SessionEnvVar, "") + t.Setenv("DATUM_PROJECT", "") + t.Setenv("DATUM_ORGANIZATION", "") + keyring.MockInit() + t.Cleanup(datumconfig.ClearSessionOverride) + + prodCtx := datumconfig.QualifiedContextName(ovProd, "org-prod") + stagingCtx := datumconfig.QualifiedContextName(ovStaging, "org-staging/proj-staging") + cfg := datumconfig.NewV1Beta1() + cfg.Sessions = []datumconfig.Session{ + {Name: ovProd, UserKey: "key-prod", UserEmail: "swells@datum.net", + Endpoint: datumconfig.Endpoint{Server: "https://api.datum.net"}, LastContext: prodCtx}, + {Name: ovStaging, UserKey: "key-staging", UserEmail: "swells@datum.net", + Endpoint: datumconfig.Endpoint{Server: "https://api.staging.env.datum.net"}, LastContext: stagingCtx}, + {Name: ovSolo, UserKey: "key-solo", UserEmail: "solo@example.com", + Endpoint: datumconfig.Endpoint{Server: "https://api.datum.net"}}, + } + cfg.Contexts = []datumconfig.DiscoveredContext{ + {Name: prodCtx, Session: ovProd, OrganizationID: "org-prod"}, + {Name: stagingCtx, Session: ovStaging, OrganizationID: "org-staging", ProjectID: "proj-staging"}, + } + cfg.ActiveSession = ovProd + cfg.CurrentContext = prodCtx + if err := datumconfig.SaveV1Beta1(cfg); err != nil { + t.Fatalf("save config: %v", err) + } + + f, err := client.NewDatumFactory(context.Background()) + if err != nil { + t.Fatalf("NewDatumFactory: %v", err) + } + return f +} + +// A plugin run with a session override receives that session's name, API +// host, and scope, so a datumctl it calls back into acts as the same session. +func TestBuildEnv_SessionOverride(t *testing.T) { + tests := []struct { + name string + flag string + hasFlag bool + env string + project string + wantSession string + wantHost string + wantOrg string + wantProject string + }{ + {name: "no override", wantSession: ovProd, wantHost: "api.datum.net", wantOrg: "org-prod"}, + {name: "flag by name", flag: ovStaging, hasFlag: true, + wantSession: ovStaging, wantHost: "api.staging.env.datum.net", wantProject: "proj-staging"}, + {name: "flag by unique email, no context", flag: "solo@example.com", hasFlag: true, + wantSession: ovSolo, wantHost: "api.datum.net"}, + {name: "env var", env: ovStaging, + wantSession: ovStaging, wantHost: "api.staging.env.datum.net", wantProject: "proj-staging"}, + {name: "flag beats env var", flag: ovSolo, hasFlag: true, env: ovStaging, + wantSession: ovSolo, wantHost: "api.datum.net"}, + {name: "--project beats the override's context", flag: ovStaging, hasFlag: true, project: "other-proj", + wantSession: ovStaging, wantHost: "api.staging.env.datum.net", wantProject: "other-proj"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := setupPluginOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, tt.env) + *f.ConfigFlags.Project = tt.project + + if err := applyPluginSessionOverride(tt.flag, tt.hasFlag); err != nil { + t.Fatalf("apply override: %v", err) + } + env, err := BuildEnv(f) + if err != nil { + t.Fatalf("BuildEnv: %v", err) + } + for key, want := range map[string]string{ + "DATUM_SESSION": tt.wantSession, + "DATUM_API_HOST": tt.wantHost, + "DATUM_ORG": tt.wantOrg, + "DATUM_PROJECT": tt.wantProject, + } { + if got := envValue(env, key); got != want { + t.Errorf("%s = %q, want %q", key, got, want) + } + } + }) + } +} + +func TestApplyPluginSessionOverride_Errors(t *testing.T) { + tests := []struct { + name string + flag string + env string + hasFlag bool + want []string + }{ + {name: "shared email", flag: "swells@datum.net", hasFlag: true, + want: []string{"ambiguous", "--session " + ovProd, "--session " + ovStaging}}, + {name: "unknown env value", env: "nobody@example.com", + want: []string{"No session matches DATUM_SESSION nobody@example.com.", ovSolo}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupPluginOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, tt.env) + err := applyPluginSessionOverride(tt.flag, tt.hasFlag) + if err == nil { + t.Fatal("expected an error") + } + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + // Error() carries the message and the hint the user sees. + for _, w := range tt.want { + if !strings.Contains(err.Error(), w) { + t.Errorf("error missing %q:\n%s", w, err) + } + } + if datumconfig.HasSessionOverride() { + t.Error("a failed override must not leave one installed") + } + }) + } +} + +func TestSplitLeadingSessionFlag(t *testing.T) { + tests := []struct { + in []string + wantValue string + wantRest []string + wantFound bool + }{ + {[]string{"ipam", "list"}, "", []string{"ipam", "list"}, false}, + {[]string{"--session", "a@b@c", "ipam", "list", "-o", "wide"}, "a@b@c", []string{"ipam", "list", "-o", "wide"}, true}, + {[]string{"--session=a@b", "ipam"}, "a@b", []string{"ipam"}, true}, + // A --session after the plugin name belongs to the plugin. + {[]string{"ipam", "--session", "x"}, "", []string{"ipam", "--session", "x"}, false}, + {[]string{"--project", "p", "ipam"}, "", []string{"--project", "p", "ipam"}, false}, + } + for _, tt := range tests { + v, rest, found := splitLeadingSessionFlag(tt.in) + if v != tt.wantValue || found != tt.wantFound || !slices.Equal(rest, tt.wantRest) { + t.Errorf("splitLeadingSessionFlag(%q) = %q, %q, %v; want %q, %q, %v", + tt.in, v, rest, found, tt.wantValue, tt.wantRest, tt.wantFound) + } + } +} From 82dbf25c345b7a401f694cef24d9de526afaaab5 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 25 Sep 2026 14:40:41 -0500 Subject: [PATCH 2/4] docs: Note DATUM_SESSION plugin export and stale scope Reviewers flagged that datumctl sets DATUM_SESSION for plugins and also reads it back, which looked like a hidden dependency. It is intentional: it lets a plugin's own datumctl calls act as the same session. Document that, and that a stale value (left over from a previous logout, or inherited this way) only breaks commands that actually need a session. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 2 ++ internal/cmd/root.go | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2ed8ed2..dacfbfb 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,8 @@ datumctl get dnszones --session alice@example.com@api.staging.env.datum.net DATUM_SESSION=alice@example.com datumctl get projects ``` +datumctl also sets `DATUM_SESSION` for plugins it runs, so a plugin's own `datumctl` calls act as the same session. A stale `DATUM_SESSION` — left over from a previous logout, or inherited this way from a plugin — only affects commands that need a session; `datumctl version`, `datumctl plugin list`, and similar commands are unaffected. + For machine-to-machine auth, see `datumctl login --credentials` for the machine-account flow. ## Agent Skills diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 59438d7..7c6a01d 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -107,7 +107,12 @@ Get started: Run one command as another signed-in account, leaving the active one as is: datumctl get dnszones --session user@example.com@api.staging.env.datum.net - DATUM_SESSION=user@example.com datumctl get dnszones`, + DATUM_SESSION=user@example.com datumctl get dnszones + +datumctl also sets DATUM_SESSION for plugins it runs, so a plugin's own +datumctl calls act as the same session. A stale DATUM_SESSION only affects +commands that need a session — version, plugin list, and similar commands +are unaffected.`, // ArbitraryArgs allows unknown subcommand names to reach RunE so the // plugin dispatch logic can handle them before Cobra rejects them. Args: cobra.ArbitraryArgs, From 6b0a67ea94eafc63bfe6e4dbaa27163d2bb6ed5e Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 25 Sep 2026 14:40:56 -0500 Subject: [PATCH 3/4] fix: Resolve DATUM_SESSION lazily, not up front MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PersistentPreRunE validated --session and DATUM_SESSION the same way, against the config file, before every command ran. A stale DATUM_SESSION — left behind by a previous logout, or inherited from a plugin's environment — made session-independent commands like `version --client`, `plugin list`, and `completion bash` fail, even though they never touch the active session. --session is a mistake in this one invocation, so it still fails immediately. DATUM_SESSION is recorded without validating it, and resolved only the first time something consults the active session: ConfigV1Beta1.ActiveSessionEntryE / CurrentContextEntryE, the error-returning counterparts of the existing choke point, used by the factory's REST config path, GetUserKeyForCurrentSession, whoami, and plugin env injection. A command that never needs a session now succeeds regardless of DATUM_SESSION; one that does gets the same list-of-choices UserError a bad --session gives. Bare `datumctl` reads the session to render its landing page, so a stale DATUM_SESSION there prints a note and falls back to the real active session rather than failing a command with nothing to retry. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/authutil/context.go | 8 +- internal/client/factory.go | 14 ++- internal/client/session_override_test.go | 23 +++++ internal/cmd/landing.go | 11 ++- internal/cmd/session_override.go | 10 ++- internal/cmd/session_override_test.go | 67 ++++++++++++++ internal/cmd/whoami/whoami.go | 5 +- internal/datumconfig/session_override.go | 90 ++++++++++++++++++- internal/plugindispatch/forward.go | 10 ++- .../plugindispatch/session_override_test.go | 28 +++++- 10 files changed, 248 insertions(+), 18 deletions(-) diff --git a/internal/authutil/context.go b/internal/authutil/context.go index 7b58633..a3c01f9 100644 --- a/internal/authutil/context.go +++ b/internal/authutil/context.go @@ -22,11 +22,15 @@ func GetUserKeyForCurrentSession() (string, *datumconfig.Session, error) { return "", nil, err } - if session := cfg.ActiveSessionEntry(); session != nil && session.UserKey != "" { + session, err := cfg.ActiveSessionEntryE() + if err != nil { + return "", nil, err + } + if session != nil && session.UserKey != "" { return session.UserKey, session, nil } - session, err := bootstrapSessionFromKeyring(cfg) + session, err = bootstrapSessionFromKeyring(cfg) if err != nil { return "", nil, err } diff --git a/internal/client/factory.go b/internal/client/factory.go index c5aed04..aacbba5 100644 --- a/internal/client/factory.go +++ b/internal/client/factory.go @@ -268,12 +268,22 @@ func (c *CustomConfigFlags) loadDatumContext() (*datumconfig.DiscoveredContext, if err := authutil.EnsureUserKeysMigrated(cfg); err != nil { return nil, nil, err } - ctxEntry := cfg.CurrentContextEntry() + // CurrentContextEntryE resolves a pending DATUM_SESSION override before + // consulting it, so a stale value fails here with a clear error instead of + // silently building a REST config for the real active session. + ctxEntry, err := cfg.CurrentContextEntryE() + if err != nil { + return nil, nil, err + } if ctxEntry == nil { // Under a session override with no context, still hand back the // overriding session so its endpoint and TLS settings apply. if datumconfig.HasSessionOverride() { - return nil, cfg.ActiveSessionEntry(), nil + session, err := cfg.ActiveSessionEntryE() + if err != nil { + return nil, nil, err + } + return nil, session, nil } return nil, nil, nil } diff --git a/internal/client/session_override_test.go b/internal/client/session_override_test.go index 9f23f85..7416513 100644 --- a/internal/client/session_override_test.go +++ b/internal/client/session_override_test.go @@ -3,6 +3,7 @@ package client import ( "context" "encoding/json" + "strings" "testing" "time" @@ -10,6 +11,7 @@ import ( "go.datum.net/datumctl/internal/authutil" "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" "go.datum.net/datumctl/internal/keyring" "go.datum.net/datumctl/internal/miloapi" ) @@ -115,6 +117,27 @@ func TestToRESTConfig_SessionOverride(t *testing.T) { } } +// A DATUM_SESSION that names no session must fail ToRESTConfig with the same +// clear, list-of-choices error a bad --session gives — not silently build a +// REST config for the real active session. +func TestToRESTConfig_StaleEnvSessionOverride(t *testing.T) { + f := setupFactoryOverrideEnv(t) + datumconfig.SetPendingSessionOverride("nobody@example.com") + + _, err := f.ConfigFlags.ToRESTConfig() + if err == nil { + t.Fatal("expected an error") + } + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + for _, want := range []string{"No session matches DATUM_SESSION nobody@example.com.", ovProd, ovStaging, ovSolo} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %q:\n%s", want, err) + } + } +} + func TestResolveSessionEndpoint_SessionOverride(t *testing.T) { setupFactoryOverrideEnv(t) datumconfig.SetSessionOverride(ovStaging, datumconfig.SessionOverrideFromEnv) diff --git a/internal/cmd/landing.go b/internal/cmd/landing.go index 787ede1..13bed29 100644 --- a/internal/cmd/landing.go +++ b/internal/cmd/landing.go @@ -29,7 +29,16 @@ func runLanding(cmd *cobra.Command, _ []string) { return } - session := cfg.ActiveSessionEntry() + session, err := cfg.ActiveSessionEntryE() + if err != nil { + // A stale DATUM_SESSION names no session. The landing page is a + // read-only "what's my state" view, so render it against the real + // active session rather than hard-failing a command with no + // subcommand to retry — but say so, since it's why the "Session" + // line below won't match DATUM_SESSION. + fmt.Fprintf(out, "Note: DATUM_SESSION matches no signed-in session; showing the active session instead.\n\n") + session = cfg.ActiveSessionEntry() + } if session == nil { printLoggedOutLanding(out) return diff --git a/internal/cmd/session_override.go b/internal/cmd/session_override.go index 22ed825..9debf37 100644 --- a/internal/cmd/session_override.go +++ b/internal/cmd/session_override.go @@ -18,6 +18,13 @@ const sessionFlag = "session" // and current context goes through datumconfig, so installing it here reaches // every command without touching the config file. // +// An explicit --session value is validated immediately: a wrong value fails +// this command right away, same as before. DATUM_SESSION is recorded without +// validating it — datumconfig resolves it lazily, the first time something +// consults the active session, so a stale export left behind by a previous +// logout (or inherited from a plugin) does not break commands that never read +// the session, such as version, plugin list, or completion. +// // Commands annotated with datumconfig.SessionOverrideAnnotation change the // active session themselves: they reject --session and ignore DATUM_SESSION. // A command that defines its own local --session flag (auth get-token) handles @@ -48,8 +55,7 @@ func applySessionOverride(cmd *cobra.Command) error { } if v := os.Getenv(datumconfig.SessionEnvVar); v != "" { - _, err := datumconfig.ApplySessionOverride(v, datumconfig.SessionOverrideFromEnv) - return err + datumconfig.SetPendingSessionOverride(v) } return nil } diff --git a/internal/cmd/session_override_test.go b/internal/cmd/session_override_test.go index 34323bb..b119a96 100644 --- a/internal/cmd/session_override_test.go +++ b/internal/cmd/session_override_test.go @@ -309,6 +309,73 @@ func TestSessionOverrideRejectedByActiveSessionCommands(t *testing.T) { } } +// A stale DATUM_SESSION — left over from a previous logout, or inherited from +// a plugin — must not break commands that never consult the active session. +// It is resolved lazily, only when something actually needs a session. +func TestSessionOverrideStaleEnvDoesNotBreakSessionlessCommands(t *testing.T) { + tests := []struct { + name string + args []string + }{ + {"version --client", []string{"version", "--client"}}, + {"plugin list", []string{"plugin", "list"}}, + {"completion bash", []string{"completion", "bash"}}, + {"--help", []string{"--help"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, "nobody@example.com") + + out, err := runRoot(t, tt.args...) + if err != nil { + t.Fatalf("%v should succeed with a stale DATUM_SESSION, got: %v\n%s", tt.args, err, out) + } + if strings.Contains(out, "No session matches") { + t.Errorf("%v should not resolve DATUM_SESSION at all:\n%s", tt.args, out) + } + }) + } +} + +// Bare `datumctl` (the landing page) does read the active session to render +// itself, but a stale DATUM_SESSION should not turn a no-subcommand invocation +// into a hard error: it renders the page for the real active session and says +// why DATUM_SESSION was ignored. +func TestSessionOverrideStaleEnvOnLandingPage(t *testing.T) { + setupOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, "nobody@example.com") + + out, err := runRoot(t) + if err != nil { + t.Fatalf("bare datumctl should succeed with a stale DATUM_SESSION, got: %v\n%s", err, out) + } + if !strings.Contains(out, "DATUM_SESSION matches no signed-in session") { + t.Errorf("output missing the stale-override note:\n%s", out) + } + if !strings.Contains(out, "swells@datum.net") { + t.Errorf("output should still show the real active session:\n%s", out) + } +} + +// A wrong --session value is a mistake in this invocation, not a stale +// ambient export, so it still fails immediately — even for a command that +// never otherwise consults the session. +func TestSessionOverrideBadFlagFailsSessionlessCommand(t *testing.T) { + setupOverrideEnv(t) + + out, err := runRoot(t, "version", "--client", "--session", "nobody@example.com") + if err == nil { + t.Fatalf("expected an error, got output:\n%s", out) + } + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + if !strings.Contains(out, "No session matches --session nobody@example.com.") { + t.Errorf("output missing the clear error:\n%s", out) + } +} + // Commands that change the active session ignore DATUM_SESSION, even when it // is unresolvable, and act on the stored active session as before. func TestSessionOverrideEnvIgnoredByActiveSessionCommands(t *testing.T) { diff --git a/internal/cmd/whoami/whoami.go b/internal/cmd/whoami/whoami.go index 1809604..7d45cc7 100644 --- a/internal/cmd/whoami/whoami.go +++ b/internal/cmd/whoami/whoami.go @@ -46,7 +46,10 @@ func runWhoami(cmd *cobra.Command, _ []string) error { return err } - session := cfg.ActiveSessionEntry() + session, err := cfg.ActiveSessionEntryE() + if err != nil { + return err + } if session == nil { return authutil.ErrNoActiveUser } diff --git a/internal/datumconfig/session_override.go b/internal/datumconfig/session_override.go index 764efbf..63bb24c 100644 --- a/internal/datumconfig/session_override.go +++ b/internal/datumconfig/session_override.go @@ -39,6 +39,14 @@ var sessionOverride struct { name string source SessionOverrideSource context string // context picked in this process (console switcher) + + // pending holds a DATUM_SESSION value recorded without validating it. A + // wrong --session value still fails immediately at the command line, but + // DATUM_SESSION is resolved lazily, on the first call that consults the + // active session, so a stale export left behind by a previous login only + // breaks the commands that actually need a session. See + // ResolvePendingOverride. + pending string } // SetSessionOverride makes every lookup of the active session in this process @@ -51,13 +59,29 @@ func SetSessionOverride(name string, source SessionOverrideSource) { sessionOverride.name = name sessionOverride.source = source sessionOverride.context = "" + sessionOverride.pending = "" } -// ClearSessionOverride removes the process-wide session override. +// ClearSessionOverride removes the process-wide session override, including +// any unresolved pending DATUM_SESSION value. func ClearSessionOverride() { SetSessionOverride("", "") } +// SetPendingSessionOverride records a DATUM_SESSION value without validating +// it against the config. Nothing fails until something calls +// ResolvePendingOverride (via ActiveSessionEntryE / CurrentContextEntryE), so +// commands that never consult the active session succeed even when the value +// names no session. +func SetPendingSessionOverride(value string) { + sessionOverride.mu.Lock() + defer sessionOverride.mu.Unlock() + sessionOverride.name = "" + sessionOverride.source = SessionOverrideFromEnv + sessionOverride.context = "" + sessionOverride.pending = value +} + // SessionOverride returns the overriding session name and where it came from, // or ("", "") when the process runs as the stored active session. func SessionOverride() (string, SessionOverrideSource) { @@ -66,10 +90,16 @@ func SessionOverride() (string, SessionOverrideSource) { return sessionOverride.name, sessionOverride.source } -// HasSessionOverride reports whether this process runs as an overriding session. +// HasSessionOverride reports whether this process runs as an overriding +// session: a validated --session, an already-resolved DATUM_SESSION, or a +// DATUM_SESSION still pending resolution. Callers that only need to know +// whether to skip persisting the active session (console re-login, the +// context switcher) should treat a pending override the same as a resolved +// one, since resolving it later must not change their earlier decision. func HasSessionOverride() bool { - name, _ := SessionOverride() - return name != "" + sessionOverride.mu.RLock() + defer sessionOverride.mu.RUnlock() + return sessionOverride.name != "" || sessionOverride.pending != "" } // SetOverrideContext selects a context for the rest of this process while a @@ -175,6 +205,58 @@ func (c *ConfigV1Beta1) ResolveSessionSelector(value string, source SessionOverr ) } +// resolvePendingOverride validates a DATUM_SESSION recorded via +// SetPendingSessionOverride against cfg, the first time something needs the +// active session. On success it caches the resolved session name so later +// calls in this process skip re-resolving. On failure it returns the same +// clear UserError ResolveSessionSelector would give a bad --session, so a +// stale DATUM_SESSION never reads back as "not logged in" — it fails loudly +// for whatever command needed a session. +// +// A no-op when there is nothing pending: no override, or one already +// resolved (from --session or an earlier call here). +func resolvePendingOverride(cfg *ConfigV1Beta1) error { + sessionOverride.mu.RLock() + name := sessionOverride.name + pending := sessionOverride.pending + source := sessionOverride.source + sessionOverride.mu.RUnlock() + + if name != "" || pending == "" { + return nil + } + + s, err := cfg.ResolveSessionSelector(pending, source) + if err != nil { + return err + } + SetSessionOverride(s.Name, source) + return nil +} + +// ActiveSessionEntryE is the error-returning counterpart of ActiveSessionEntry. +// It resolves a pending DATUM_SESSION override first, so a stale value +// surfaces as a UserError instead of silently falling back to the stored +// active session. Callers that need a session (the REST config factory, +// GetUserKeyForCurrentSession, whoami, plugin env injection) should call this +// instead of ActiveSessionEntry. +func (c *ConfigV1Beta1) ActiveSessionEntryE() (*Session, error) { + if err := resolvePendingOverride(c); err != nil { + return nil, err + } + return c.ActiveSessionEntry(), nil +} + +// CurrentContextEntryE is the error-returning counterpart of +// CurrentContextEntry, resolving a pending DATUM_SESSION override first. See +// ActiveSessionEntryE. +func (c *ConfigV1Beta1) CurrentContextEntryE() (*DiscoveredContext, error) { + if err := resolvePendingOverride(c); err != nil { + return nil, err + } + return c.CurrentContextEntry(), nil +} + // ApplySessionOverride resolves value against the config at the default path // and, when it names a session, installs it as the process-wide override. An // empty value clears any override. It returns the selected session. diff --git a/internal/plugindispatch/forward.go b/internal/plugindispatch/forward.go index 2afc25b..2a53da1 100644 --- a/internal/plugindispatch/forward.go +++ b/internal/plugindispatch/forward.go @@ -117,14 +117,20 @@ func splitLeadingSessionFlag(args []string) (value string, rest []string, found // applyPluginSessionOverride installs the session a plugin runs as: the // --session flag when given, else DATUM_SESSION. BuildEnv then reports that // session's name and API host to the plugin. +// +// --session is validated immediately, same as the root command. DATUM_SESSION +// is only recorded here; it is resolved lazily by GetUserKeyForCurrentSession +// inside BuildEnv, so a plugin that never needs the session (or a stale value +// left over from a previous logout) does not stop the plugin from running — +// BuildEnv exports an empty DATUM_SESSION/DATUM_API_HOST/DATUM_ORG rather than +// the unrelated real active session. func applyPluginSessionOverride(flagValue string, hasFlag bool) error { if hasFlag { _, err := datumconfig.ApplySessionOverride(flagValue, datumconfig.SessionOverrideFromFlag) return err } if v := os.Getenv(datumconfig.SessionEnvVar); v != "" { - _, err := datumconfig.ApplySessionOverride(v, datumconfig.SessionOverrideFromEnv) - return err + datumconfig.SetPendingSessionOverride(v) } return nil } diff --git a/internal/plugindispatch/session_override_test.go b/internal/plugindispatch/session_override_test.go index 4b60897..04df42a 100644 --- a/internal/plugindispatch/session_override_test.go +++ b/internal/plugindispatch/session_override_test.go @@ -112,23 +112,22 @@ func TestBuildEnv_SessionOverride(t *testing.T) { } } +// A wrong --session value still fails immediately: it is a mistake in this +// invocation, not a stale ambient export, so there is nothing to gain by +// deferring it. func TestApplyPluginSessionOverride_Errors(t *testing.T) { tests := []struct { name string flag string - env string hasFlag bool want []string }{ {name: "shared email", flag: "swells@datum.net", hasFlag: true, want: []string{"ambiguous", "--session " + ovProd, "--session " + ovStaging}}, - {name: "unknown env value", env: "nobody@example.com", - want: []string{"No session matches DATUM_SESSION nobody@example.com.", ovSolo}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { setupPluginOverrideEnv(t) - t.Setenv(datumconfig.SessionEnvVar, tt.env) err := applyPluginSessionOverride(tt.flag, tt.hasFlag) if err == nil { t.Fatal("expected an error") @@ -149,6 +148,27 @@ func TestApplyPluginSessionOverride_Errors(t *testing.T) { } } +// A stale DATUM_SESSION (matching no session) must not stop a plugin from +// running, and BuildEnv must not paper over it by exporting the real active +// session under DATUM_SESSION: that would run the plugin's own datumctl calls +// as an account the user never asked for. +func TestApplyPluginSessionOverride_StaleEnvDoesNotError(t *testing.T) { + f := setupPluginOverrideEnv(t) + t.Setenv(datumconfig.SessionEnvVar, "nobody@example.com") + + if err := applyPluginSessionOverride("", false); err != nil { + t.Fatalf("apply override: %v", err) + } + + env, err := BuildEnv(f) + if err != nil { + t.Fatalf("BuildEnv: %v", err) + } + if got := envValue(env, "DATUM_SESSION"); got != "" { + t.Errorf("DATUM_SESSION = %q, want empty (must not leak the real active session %q)", got, ovProd) + } +} + func TestSplitLeadingSessionFlag(t *testing.T) { tests := []struct { in []string From c2e194bc07d4f9d149041dd3e8c68ae259b58595 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Tue, 29 Sep 2026 17:26:04 -0500 Subject: [PATCH 4/4] fix: Accept --session on plugin commands "datumctl --session X" failed with the plugin's own "unknown flag: --session", because only a --session placed before the plugin name was consumed by datumctl; anywhere else it was forwarded as one of the plugin's arguments. That pointed users at the wrong component and pushed them toward switching their active session instead, which is exactly what the flag exists to avoid. datumctl now takes --session out of a plugin command line wherever it appears and turns it into the DATUM_SESSION (and API host) the plugin already understands. Arguments after a bare "--" still belong to the plugin. The flag continues to beat DATUM_SESSION, and --session with no value now fails with datumctl's own message rather than quietly running as the active session. Fixes #304 Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- internal/plugindispatch/forward.go | 80 +++++++++++------ .../plugindispatch/session_override_test.go | 85 +++++++++++++++++-- 3 files changed, 136 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index dacfbfb..9d589b5 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ datumctl get dnszones --session alice@example.com@api.staging.env.datum.net DATUM_SESSION=alice@example.com datumctl get projects ``` -datumctl also sets `DATUM_SESSION` for plugins it runs, so a plugin's own `datumctl` calls act as the same session. A stale `DATUM_SESSION` — left over from a previous logout, or inherited this way from a plugin — only affects commands that need a session; `datumctl version`, `datumctl plugin list`, and similar commands are unaffected. +This works on plugin commands too (`datumctl dns zones list --session alice@example.com@api.datum.net`): datumctl takes the flag itself and sets `DATUM_SESSION` for the plugin it runs, so the plugin's own `datumctl` calls act as the same session. A stale `DATUM_SESSION` — left over from a previous logout, or inherited this way from a plugin — only affects commands that need a session; `datumctl version`, `datumctl plugin list`, and similar commands are unaffected. For machine-to-machine auth, see `datumctl login --credentials` for the machine-account flow. diff --git a/internal/plugindispatch/forward.go b/internal/plugindispatch/forward.go index 2a53da1..cc32aa4 100644 --- a/internal/plugindispatch/forward.go +++ b/internal/plugindispatch/forward.go @@ -15,6 +15,7 @@ import ( "go.datum.net/datumctl/internal/client" "go.datum.net/datumctl/internal/datumconfig" + customerrors "go.datum.net/datumctl/internal/errors" "go.datum.net/datumctl/internal/pluginstore" ) @@ -35,12 +36,12 @@ import ( // can correctly distinguish plugin names from registered subcommands. // On success this function does not return (process is replaced). // -// A leading global --session flag ("datumctl --session X ...") or the -// DATUM_SESSION environment variable runs the plugin as that session. When the -// session cannot be resolved, ForwardPlugin returns the UserError so the caller -// can report it; the plugin is not run. +// The global --session flag, wherever the user put it on the command line, or +// the DATUM_SESSION environment variable runs the plugin as that session. When +// the session cannot be resolved, ForwardPlugin returns the UserError so the +// caller can report it; the plugin is not run. func ForwardPlugin(pluginsDir string, root *cobra.Command, factory *client.DatumCloudFactory) error { - sessionValue, args, hasSessionFlag := splitLeadingSessionFlag(os.Args[1:]) + sessionValue, args, hasSessionFlag := splitSessionFlag(os.Args[1:]) if len(args) == 0 || strings.HasPrefix(args[0], "-") { return nil } @@ -96,22 +97,35 @@ func ForwardPlugin(pluginsDir string, root *cobra.Command, factory *client.Datum return Exec(binaryPath, args[1:], factory) } -// splitLeadingSessionFlag removes global --session flags that appear before -// the plugin name ("--session X" or "--session=X") and returns the last value -// given, the remaining arguments, and whether the flag was present. Arguments -// after the plugin name belong to the plugin and are left alone. -func splitLeadingSessionFlag(args []string) (value string, rest []string, found bool) { - for len(args) > 0 { +// splitSessionFlag removes datumctl's global --session flag ("--session X" or +// "--session=X") from a plugin command line and returns the last value given, +// the remaining arguments, and whether the flag was present. +// +// The flag is taken from anywhere in the command line, not just before the +// plugin name: --session belongs to datumctl, and plugins do not define it, so +// leaving one in place makes the plugin refuse the whole command with its own +// "unknown flag: --session". +// +// Everything after a bare "--" is the plugin's to interpret and is left alone. +func splitSessionFlag(args []string) (value string, rest []string, found bool) { + rest = make([]string, 0, len(args)) + for i := 0; i < len(args); i++ { switch { - case args[0] == "--session" && len(args) > 1: - value, found, args = args[1], true, args[2:] - case strings.HasPrefix(args[0], "--session="): - value, found, args = strings.TrimPrefix(args[0], "--session="), true, args[1:] + case args[i] == "--": + return value, append(rest, args[i:]...), found + case args[i] == "--session": + found = true + if i+1 < len(args) { + value = args[i+1] + i++ + } + case strings.HasPrefix(args[i], "--session="): + value, found = strings.TrimPrefix(args[i], "--session="), true default: - return value, args, found + rest = append(rest, args[i]) } } - return value, args, found + return value, rest, found } // applyPluginSessionOverride installs the session a plugin runs as: the @@ -126,6 +140,15 @@ func splitLeadingSessionFlag(args []string) (value string, rest []string, found // the unrelated real active session. func applyPluginSessionOverride(flagValue string, hasFlag bool) error { if hasFlag { + // --session with nothing after it would otherwise clear the override and + // quietly run as the active session, which is the mistake the flag exists + // to prevent. + if strings.TrimSpace(flagValue) == "" { + return customerrors.NewUserErrorWithHint( + "--session needs a session name.", + "Run 'datumctl auth list' to see the available session names.", + ) + } _, err := datumconfig.ApplySessionOverride(flagValue, datumconfig.SessionOverrideFromFlag) return err } @@ -220,7 +243,13 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err return nil } - name := os.Args[2] + // Take --session out of the completion request the same way the run path + // does, so it never reaches the plugin as an argument it does not know. + sessionValue, completionArgs, hasSessionFlag := splitSessionFlag(os.Args[2:]) + if len(completionArgs) == 0 { + return nil + } + name := completionArgs[0] // Find the plugin binary. binaryPath, managed, err := FindPlugin(name, pluginsDir) @@ -253,7 +282,7 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err // Strip the plugin name from the forwarded args so the plugin sees // ["__complete", ] rather than ["__complete", "compute", ]. // The plugin's own cobra tree has no knowledge of its name as a subcommand. - pluginArgs := append([]string{"__complete"}, os.Args[3:]...) + pluginArgs := append([]string{"__complete"}, completionArgs[1:]...) cmd := exec.Command(binaryPath, pluginArgs...) cmd.Stdin = os.Stdin @@ -264,9 +293,10 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err // completion. Non-fatal: if env construction fails we proceed without injection // and the plugin will return empty candidates instead of erroring. if factory != nil { - // Honor DATUM_SESSION so completion lists what the plugin will act on. - // An unresolvable value is ignored here; running the plugin reports it. - _ = applyPluginSessionOverride("", false) + // Honor --session/DATUM_SESSION so completion lists what the plugin will + // act on. An unresolvable value is ignored here; running the plugin + // reports it. + _ = applyPluginSessionOverride(sessionValue, hasSessionFlag) if env, buildErr := BuildEnv(factory); buildErr == nil { cmd.Env = overlayEnv(os.Environ(), env) } @@ -294,9 +324,9 @@ func ForwardCompletion(pluginsDir string, factory *client.DatumCloudFactory) err // Cobra intercepts --help before RunE fires, so this must be called before // cobra.Execute(), mirroring the ForwardCompletion pattern. func ForwardHelp(pluginsDir string) error { - // Strip "datumctl" and any leading --session, so help for - // "datumctl --session X --help" reaches the plugin too. - _, args, _ := splitLeadingSessionFlag(os.Args[1:]) + // Strip "datumctl" and any --session, so help for + // "datumctl --session X --help" reaches the plugin too. + _, args, _ := splitSessionFlag(os.Args[1:]) if len(args) == 0 { return nil } diff --git a/internal/plugindispatch/session_override_test.go b/internal/plugindispatch/session_override_test.go index 04df42a..795903f 100644 --- a/internal/plugindispatch/session_override_test.go +++ b/internal/plugindispatch/session_override_test.go @@ -2,6 +2,7 @@ package plugindispatch import ( "context" + "os" "slices" "strings" "testing" @@ -10,6 +11,7 @@ import ( "go.datum.net/datumctl/internal/datumconfig" customerrors "go.datum.net/datumctl/internal/errors" "go.datum.net/datumctl/internal/keyring" + "go.datum.net/datumctl/internal/pluginstore" ) const ( @@ -169,7 +171,7 @@ func TestApplyPluginSessionOverride_StaleEnvDoesNotError(t *testing.T) { } } -func TestSplitLeadingSessionFlag(t *testing.T) { +func TestSplitSessionFlag(t *testing.T) { tests := []struct { in []string wantValue string @@ -179,15 +181,88 @@ func TestSplitLeadingSessionFlag(t *testing.T) { {[]string{"ipam", "list"}, "", []string{"ipam", "list"}, false}, {[]string{"--session", "a@b@c", "ipam", "list", "-o", "wide"}, "a@b@c", []string{"ipam", "list", "-o", "wide"}, true}, {[]string{"--session=a@b", "ipam"}, "a@b", []string{"ipam"}, true}, - // A --session after the plugin name belongs to the plugin. - {[]string{"ipam", "--session", "x"}, "", []string{"ipam", "--session", "x"}, false}, + // Issue #304: --session after the plugin name is still datumctl's, and + // must not reach the plugin as an argument it does not recognize. + {[]string{"ipam", "--session", "x"}, "x", []string{"ipam"}, true}, + {[]string{"assistant", "card", "--session", "a@b@c"}, "a@b@c", []string{"assistant", "card"}, true}, + {[]string{"assistant", "card", "--session=a@b@c"}, "a@b@c", []string{"assistant", "card"}, true}, + {[]string{"ipam", "list", "--session", "a@b", "-o", "wide"}, "a@b", []string{"ipam", "list", "-o", "wide"}, true}, + // Missing value: reported as present with no value so the caller can say + // so, rather than silently running as the active session. + {[]string{"ipam", "list", "--session"}, "", []string{"ipam", "list"}, true}, + // Everything after a bare "--" is the plugin's to interpret. + {[]string{"ipam", "--", "--session", "x"}, "", []string{"ipam", "--", "--session", "x"}, false}, {[]string{"--project", "p", "ipam"}, "", []string{"--project", "p", "ipam"}, false}, } for _, tt := range tests { - v, rest, found := splitLeadingSessionFlag(tt.in) + v, rest, found := splitSessionFlag(tt.in) if v != tt.wantValue || found != tt.wantFound || !slices.Equal(rest, tt.wantRest) { - t.Errorf("splitLeadingSessionFlag(%q) = %q, %q, %v; want %q, %q, %v", + t.Errorf("splitSessionFlag(%q) = %q, %q, %v; want %q, %q, %v", tt.in, v, rest, found, tt.wantValue, tt.wantRest, tt.wantFound) } } } + +// Issue #304: "datumctl --session X" must run the plugin as +// that session. The flag is consumed by datumctl and turned into DATUM_SESSION +// (plus the matching API host) instead of being handed to the plugin, which +// would reject the whole command with "unknown flag: --session". +func TestForwardPlugin_sessionFlagAfterPluginName(t *testing.T) { + // Not parallel — mutates os.Args, execPlatform, and environment. + factory := setupPluginOverrideEnv(t) + managedDir := t.TempDir() + // Isolate PATH so only the managed binary resolves the plugin name. + t.Setenv("PATH", t.TempDir()) + + binaryPath := writeFakeBinary(t, managedDir, "assistant") + writeManifest(t, managedDir, &pluginstore.Manifest{ + Plugins: map[string]*pluginstore.InstalledPlugin{ + "assistant": {SHA256: sha256HexFile(t, binaryPath)}, + }, + }) + + var gotArgs, gotEnv []string + execCalled := false + origExec := execPlatform + execPlatform = func(_ string, args []string, env []string) error { + execCalled, gotArgs, gotEnv = true, args, env + return nil + } + t.Cleanup(func() { execPlatform = origExec }) + + origArgs := os.Args + os.Args = []string{"datumctl", "assistant", "card", "--session", ovStaging} + t.Cleanup(func() { os.Args = origArgs }) + + if err := ForwardPlugin(managedDir, buildMinimalCobraTree(), factory); err != nil { + t.Fatalf("ForwardPlugin: %v", err) + } + if !execCalled { + t.Fatal("plugin was not exec'd") + } + if want := []string{"card"}; !slices.Equal(gotArgs, want) { + t.Errorf("forwarded args = %v, want %v (--session must not reach the plugin)", gotArgs, want) + } + if got := envValue(gotEnv, "DATUM_SESSION"); got != ovStaging { + t.Errorf("DATUM_SESSION = %q, want %q", got, ovStaging) + } + if got := envValue(gotEnv, "DATUM_API_HOST"); got != "api.staging.env.datum.net" { + t.Errorf("DATUM_API_HOST = %q, want the override session's host", got) + } +} + +// A --session with no value must fail with datumctl's own message rather than +// falling through and running the plugin as the active session. +func TestApplyPluginSessionOverride_missingValue(t *testing.T) { + setupPluginOverrideEnv(t) + err := applyPluginSessionOverride("", true) + if err == nil { + t.Fatal("expected an error for --session with no value") + } + if _, ok := customerrors.IsUserError(err); !ok { + t.Errorf("error is not a UserError: %v", err) + } + if datumconfig.HasSessionOverride() { + t.Error("a failed override must not leave one installed") + } +}