diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index f633a61433..289b9e70a1 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -386,7 +386,7 @@ func TestAuthScopesRun_UsesTenantAccessTokenFromCredentialProvider(t *testing.T) AppID: "test-app", AppSecret: "", Brand: core.BrandFeishu, }) tokenResolver := &authScopesTokenResolver{} - f.Credential = credential.NewCredentialProvider(nil, nil, tokenResolver, nil) + f.Credential = newAuthTestCredentialProvider("test-app", tokenResolver) appInfoStub := &httpmock.Stub{ Method: http.MethodGet, @@ -442,7 +442,7 @@ func TestAuthScopesRun_LarkPermissionError_TypedAsPermissionError(t *testing.T) AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, }) tokenResolver := &authScopesTokenResolver{} - f.Credential = credential.NewCredentialProvider(nil, nil, tokenResolver, nil) + f.Credential = newAuthTestCredentialProvider("test-app", tokenResolver) reg.Register(&httpmock.Stub{ Method: http.MethodGet, @@ -485,6 +485,18 @@ type authScopesTokenResolver struct { requests []credential.TokenSpec } +type authTestAccountResolver struct { + appID string +} + +func (r authTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID, Brand: core.BrandFeishu}, nil +} + +func newAuthTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, authTestAccountResolver{appID: appID}, tokenResolver, nil) +} + func (r *authScopesTokenResolver) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.TokenResult, error) { r.requests = append(r.requests, req) switch req.Type { diff --git a/cmd/auth/status.go b/cmd/auth/status.go index 15de7dda9c..b8b4492926 100644 --- a/cmd/auth/status.go +++ b/cmd/auth/status.go @@ -27,6 +27,9 @@ func NewCmdAuthStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobr cmd := &cobra.Command{ Use: "status", Short: "View current auth status", + Long: `Show OAuth user login, token validity, and granted scopes. +For token-validity checks, run lark-cli auth status --json --verify. +This is not profile/app selection diagnostics; use lark-cli whoami for the effective app/profile identity used by an invocation.`, RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { return runF(opts) diff --git a/cmd/auth/status_test.go b/cmd/auth/status_test.go index 7bf0608c77..895dba238d 100644 --- a/cmd/auth/status_test.go +++ b/cmd/auth/status_test.go @@ -4,15 +4,35 @@ package auth import ( + "context" "encoding/json" "net/http" + "strings" "testing" + extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/httpmock" ) +func TestAuthStatusHelpDistinguishesFromWhoami(t *testing.T) { + cmd := NewCmdAuthStatus(nil, nil) + for _, want := range []string{ + "OAuth user login", + "auth status --json --verify", + "not profile/app selection diagnostics", + "lark-cli whoami", + } { + if !strings.Contains(cmd.Long, want) { + t.Errorf("auth status --help Long missing %q; got:\n%s", want, cmd.Long) + } + } +} + func TestAuthStatusRun_SplitsBotAndUserIdentity(t *testing.T) { f, stdout, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app", AppSecret: "secret", Brand: core.BrandFeishu, @@ -79,6 +99,51 @@ func TestAuthStatusRun_VerifyReportsBotIdentity(t *testing.T) { } } +type fixedStatusAccountResolver struct { + account *credential.Account +} + +func (r *fixedStatusAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return r.account, nil +} + +func TestAuthStatus_AllowsMatchingAppIDOnlySelectedProfile(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + if err := core.SaveMultiAppConfig(&core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.PlainSecret("test-secret"), + Brand: core.BrandFeishu, + }}, + }); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + config := &core.CliConfig{ProfileName: "tenant_a", AppID: "cli_a", AppSecret: "test-secret", Brand: core.BrandFeishu} + f, stdout, _, _ := cmdutil.TestFactory(t, config) + f.Credential = credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, + &fixedStatusAccountResolver{account: credential.AccountFromCliConfig(config)}, + nil, + nil, + ).WithProfileFromFlag("tenant_a") + + cmd := NewCmdAuth(f) + cmd.SetArgs([]string{"status", "--json"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("auth status should use the selected built-in profile: %v", err) + } + if strings.Contains(stdout.String(), "credentials are provided externally") { + t.Fatalf("matching APP_ID-only env was misclassified as external:\n%s", stdout.String()) + } +} + type statusOutput struct { Identity string `json:"identity"` Verified *bool `json:"verified"` diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 841a884094..c5f6c667d8 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -6,8 +6,10 @@ package cmd import ( "errors" "io" + "os" "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/envvars" "github.com/spf13/pflag" ) @@ -26,5 +28,13 @@ func BootstrapInvocationContext(args []string) (cmdutil.InvocationContext, error if err := fs.Parse(args); err != nil && !errors.Is(err, pflag.ErrHelp) { return cmdutil.InvocationContext{}, err } - return cmdutil.InvocationContext{Profile: globals.Profile}, nil + + profileFromFlag := fs.Changed("profile") + if !profileFromFlag { + globals.Profile = os.Getenv(envvars.CliProfile) + } + return cmdutil.InvocationContext{ + Profile: globals.Profile, + ProfileFromFlag: profileFromFlag, + }, nil } diff --git a/cmd/bootstrap_test.go b/cmd/bootstrap_test.go index aa5fd3de79..2ea162d995 100644 --- a/cmd/bootstrap_test.go +++ b/cmd/bootstrap_test.go @@ -3,7 +3,11 @@ package cmd -import "testing" +import ( + "testing" + + "github.com/larksuite/cli/internal/envvars" +) func TestBootstrapInvocationContext_ProfileFlag(t *testing.T) { inv, err := BootstrapInvocationContext([]string{"--profile", "target", "auth", "status"}) @@ -70,3 +74,58 @@ func TestBootstrapInvocationContext_HelpWithProfile(t *testing.T) { t.Fatalf("profile = %q, want %q", inv.Profile, "target") } } + +func TestBootstrapProfileEnvFallback(t *testing.T) { + t.Run("flag wins over env", func(t *testing.T) { + t.Setenv(envvars.CliProfile, "tenant_env") + inv, err := BootstrapInvocationContext([]string{"--profile", "tenant_flag", "whoami"}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if inv.Profile != "tenant_flag" { + t.Errorf("got %q, want tenant_flag", inv.Profile) + } + if !inv.ProfileFromFlag { + t.Errorf("ProfileFromFlag = false, want true") + } + }) + t.Run("explicit empty flag clears env selection", func(t *testing.T) { + t.Setenv(envvars.CliProfile, "tenant_env") + inv, err := BootstrapInvocationContext([]string{"--profile=", "whoami"}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if inv.Profile != "" { + t.Errorf("got %q, want empty", inv.Profile) + } + if !inv.ProfileFromFlag { + t.Errorf("ProfileFromFlag = false, want true") + } + }) + t.Run("env used when flag absent", func(t *testing.T) { + t.Setenv(envvars.CliProfile, "tenant_env") + inv, err := BootstrapInvocationContext([]string{"whoami"}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if inv.Profile != "tenant_env" { + t.Errorf("got %q, want tenant_env", inv.Profile) + } + if inv.ProfileFromFlag { + t.Errorf("ProfileFromFlag = true, want false") + } + }) + t.Run("empty when neither set", func(t *testing.T) { + t.Setenv(envvars.CliProfile, "") + inv, err := BootstrapInvocationContext([]string{"whoami"}) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if inv.Profile != "" { + t.Errorf("got %q, want empty", inv.Profile) + } + if inv.ProfileFromFlag { + t.Errorf("ProfileFromFlag = true, want false") + } + }) +} diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index 9f0f3da62c..074100b00d 100644 --- a/cmd/config/config_test.go +++ b/cmd/config/config_test.go @@ -84,6 +84,16 @@ func TestConfigShowCmd_FlagParsing(t *testing.T) { } } +func TestConfigShowHelpClarifiesSavedConfig(t *testing.T) { + cmd := NewCmdConfigShow(nil, nil) + if !strings.Contains(cmd.Short, "saved config") { + t.Errorf("config show short = %q, want saved config", cmd.Short) + } + if !strings.Contains(cmd.Long, "lark-cli whoami --json") { + t.Errorf("config show help missing whoami route") + } +} + func TestConfigShowRun_NotConfiguredReturnsStructuredError(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) @@ -106,6 +116,77 @@ func TestConfigShowRun_NotConfiguredReturnsStructuredError(t *testing.T) { } } +// config show promises "saved config, not current usage" (help + skill +// routing): the session profile (--profile / LARKSUITE_CLI_PROFILE) must not +// change what it shows. +func TestConfigShowRun_IgnoresSessionProfile(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + multi := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{ + {Name: "tenant_a", AppId: "cli_a", AppSecret: core.PlainSecret("your-secret-a"), Brand: core.BrandFeishu}, + {Name: "tenant_b", AppId: "cli_b", AppSecret: core.PlainSecret("your-secret-b"), Brand: core.BrandFeishu}, + }, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + f.Invocation.Profile = "tenant_b" // session selection must not leak in + + if err := configShowRun(&ConfigShowOptions{Factory: f}); err != nil { + t.Fatalf("configShowRun: %v", err) + } + out := stdout.String() + if !strings.Contains(out, `"cli_a"`) || !strings.Contains(out, `"tenant_a"`) { + t.Fatalf("output = %s, want the saved default tenant_a/cli_a", out) + } + if strings.Contains(out, `"cli_b"`) { + t.Fatalf("output = %s, session profile tenant_b must not change saved-config view", out) + } +} + +// engagedEnvStub simulates a fully engaged external credential provider. +type engagedEnvStub struct{} + +func (engagedEnvStub) Name() string { return "env" } +func (engagedEnvStub) Priority() int { return 10 } +func (engagedEnvStub) ResolveAccount(context.Context) (*extcred.Account, error) { + return &extcred.Account{AppID: "cli_env", AppSecret: "your-password"}, nil // managed takeover +} +func (engagedEnvStub) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +// config show inspects the SAVED config only, so the parent command's +// external-credential gate must not apply: even with a fully engaged direct +// env credential, `config show` still answers from the saved config. +func TestConfigShow_BypassesExternalCredentialGate(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + multi := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", AppId: "cli_a", AppSecret: core.PlainSecret("your-secret-a"), Brand: core.BrandFeishu, + }}, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + f, stdout, _, _ := cmdutil.TestFactory(t, nil) + f.Credential = credential.NewCredentialProvider([]extcred.Provider{engagedEnvStub{}}, nil, nil, nil) + + cmd := NewCmdConfig(f) + cmd.SetArgs([]string{"show"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("config show must bypass the external-credential gate: %v", err) + } + if out := stdout.String(); !strings.Contains(out, `"cli_a"`) { + t.Fatalf("output = %s, want the saved config shown", out) + } +} + func TestConfigShowRun_NoActiveProfileReturnsStructuredError(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) multi := &core.MultiAppConfig{ @@ -481,7 +562,8 @@ func TestConfigBlockedByExternalProvider(t *testing.T) { }{ {"init", []string{"init", "--app-id", "x", "--app-secret-stdin"}}, {"remove", []string{"remove"}}, - {"show", []string{"show"}}, + // "show" is deliberately absent: it inspects the SAVED config only + // and bypasses this gate (TestConfigShow_BypassesExternalCredentialGate). {"default-as", []string{"default-as", "user"}}, {"strict-mode", []string{"strict-mode", "off"}}, } diff --git a/cmd/config/show.go b/cmd/config/show.go index 5526f0254a..4bcf8d335c 100644 --- a/cmd/config/show.go +++ b/cmd/config/show.go @@ -27,7 +27,16 @@ func NewCmdConfigShow(f *cmdutil.Factory, runF func(*ConfigShowOptions) error) * cmd := &cobra.Command{ Use: "show", - Short: "Show current configuration", + Short: "Show saved config", + Long: "Shows saved config. To see the app/profile lark-cli is using now, run `lark-cli whoami --json`.", + // Override parent's RequireBuiltinCredentialProvider check: this + // command reads the SAVED config only (its own help promises "saved + // config, not current usage"), so the currently effective credential + // source — external or otherwise — must not gate it. + PersistentPreRunE: func(c *cobra.Command, _ []string) error { + c.SilenceUsage = true + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { return runF(opts) @@ -53,7 +62,10 @@ func configShowRun(opts *ConfigShowOptions) error { if config == nil || len(config.Apps) == 0 { return core.NotConfiguredError() } - app := config.CurrentAppConfig(f.Invocation.Profile) + // Saved config only: the session profile (--profile / LARKSUITE_CLI_PROFILE) + // must not change what this command shows — the help and skill routing + // promise "saved config, not current usage" (use whoami for that). + app := config.CurrentAppConfig("") if app == nil { return errs.NewConfigError(errs.SubtypeNotConfigured, "no active profile").WithHint("run: lark-cli profile list") } diff --git a/cmd/event/consume_test.go b/cmd/event/consume_test.go index ff0906fad0..08b4326ff0 100644 --- a/cmd/event/consume_test.go +++ b/cmd/event/consume_test.go @@ -110,8 +110,20 @@ func (failingTokenResolver) ResolveToken(_ context.Context, _ credential.TokenSp return nil, errors.New("backend unavailable") } +type eventTestAccountResolver struct { + appID string +} + +func (r eventTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + +func newEventTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, eventTestAccountResolver{appID: appID}, tokenResolver, nil) +} + func factoryWithResolver(r credential.DefaultTokenResolver) *cmdutil.Factory { - return &cmdutil.Factory{Credential: credential.NewCredentialProvider(nil, nil, r, nil)} + return &cmdutil.Factory{Credential: newEventTestCredentialProvider("cli_x", r)} } func TestResolveTenantToken_EmptyTokenResult(t *testing.T) { diff --git a/cmd/event/runtime_test.go b/cmd/event/runtime_test.go index 67f2bea1c2..06729c8c3f 100644 --- a/cmd/event/runtime_test.go +++ b/cmd/event/runtime_test.go @@ -44,7 +44,7 @@ func newTestConsumeRuntime(rt http.RoundTripper) *consumeRuntime { client: &client.APIClient{ SDK: sdk, ErrOut: io.Discard, - Credential: credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil), + Credential: newEventTestCredentialProvider("test-app", &staticTokenResolver{}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, }, accessIdentity: core.AsBot, diff --git a/cmd/profile/list.go b/cmd/profile/list.go index 5b0234c2c9..f19c463402 100644 --- a/cmd/profile/list.go +++ b/cmd/profile/list.go @@ -17,11 +17,14 @@ import ( ) // profileListItem is the JSON output for a single profile entry. +// `default` (formerly `active`, renamed in this feature as a declared +// breaking change) marks the saved default profile — never the identity +// effective for the current invocation; that is whoami's job. type profileListItem struct { Name string `json:"name"` AppID string `json:"appId"` Brand core.LarkBrand `json:"brand"` - Active bool `json:"active"` + Default bool `json:"default"` User string `json:"user,omitempty"` TokenStatus string `json:"tokenStatus,omitempty"` } @@ -30,7 +33,8 @@ type profileListItem struct { func NewCmdProfileList(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "list", - Short: "List all profiles", + Short: "List saved profiles", + Long: "Lists saved profiles. To see the app/profile lark-cli is using now, run `lark-cli whoami --json`.", RunE: func(cmd *cobra.Command, args []string) error { return profileListRun(f) }, @@ -53,7 +57,7 @@ func profileListRun(f *cmdutil.Factory) error { return nil } - // Intentionally uses "" to show the persistent active profile, not the ephemeral --profile override. + // Intentionally uses "" to show the saved default profile, not the ephemeral --profile override. currentApp := multi.CurrentAppConfig("") currentName := "" if currentApp != nil { @@ -66,10 +70,10 @@ func profileListRun(f *cmdutil.Factory) error { name := app.ProfileName() item := profileListItem{ - Name: name, - AppID: app.AppId, - Brand: app.Brand, - Active: name == currentName, + Name: name, + AppID: app.AppId, + Brand: app.Brand, + Default: name == currentName, } if len(app.Users) > 0 { diff --git a/cmd/profile/profile.go b/cmd/profile/profile.go index 2216a4f391..9373c99169 100644 --- a/cmd/profile/profile.go +++ b/cmd/profile/profile.go @@ -14,6 +14,17 @@ func NewCmdProfile(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "profile", Short: "Manage configuration profiles", + Long: `Profiles are named app identities managed by lark-cli. + +Identity diagnostics and profile selection: + lark-cli whoami --json Show the app/profile lark-cli is using now. + lark-cli auth status --json --verify Verify OAuth login and token state. + --profile Use a profile for this command only. + LARKSUITE_CLI_PROFILE Use a profile for the current shell / agent session. + config show / profile list Inspect saved config, not current usage. + unset LARKSUITE_CLI_PROFILE Clear the session profile and fall back to direct app env or configured default. + +A selected profile takes precedence over matching direct env credentials and tokens.`, } cmdutil.DisableAuthCheck(cmd) cmdutil.SetTips(cmd, []string{ diff --git a/cmd/profile/profile_test.go b/cmd/profile/profile_test.go index 472a803bf6..f776f6050f 100644 --- a/cmd/profile/profile_test.go +++ b/cmd/profile/profile_test.go @@ -306,14 +306,24 @@ func TestProfileListRun_OutputsProfiles(t *testing.T) { if err := json.Unmarshal(stdout.Bytes(), &got); err != nil { t.Fatalf("Unmarshal() error = %v; output=%s", err, stdout.String()) } + raw := stdout.String() + // `active` is renamed to `default` as a declared breaking change: keeping + // a permanently mirrored alias would keep misleading agents into reading + // it as the currently effective identity (whoami's job). + if strings.Contains(raw, `"active"`) { + t.Fatalf("profile list output contains renamed active field: %s", raw) + } + if !strings.Contains(raw, `"default"`) { + t.Fatalf("profile list output missing default field: %s", raw) + } if len(got) != 2 { t.Fatalf("len(got) = %d, want 2", len(got)) } - if got[0].Name != "default" || !got[0].Active { - t.Fatalf("got[0] = %#v, want active default profile", got[0]) + if got[0].Name != "default" || !got[0].Default { + t.Fatalf("got[0] = %#v, want configured default profile", got[0]) } - if got[1].Name != "target" || got[1].Active { - t.Fatalf("got[1] = %#v, want inactive target profile", got[1]) + if got[1].Name != "target" || got[1].Default { + t.Fatalf("got[1] = %#v, want non-default target profile", got[1]) } } @@ -627,6 +637,39 @@ func TestProfileRemoveRun_ValidationErrors(t *testing.T) { }) } +// TestProfileHelpHasSelectionSection asserts `profile --help` documents the +// per-invocation flag and session-scoped env var for selecting a profile, so +// users and AI agents can find LARKSUITE_CLI_PROFILE without reading source. +func TestProfileHelpHasSelectionSection(t *testing.T) { + cmd := NewCmdProfile(nil) + if !strings.Contains(cmd.Long, "Identity diagnostics and profile selection:") { + t.Errorf("profile --help missing identity diagnostics and profile selection section") + } + if !strings.Contains(cmd.Long, "LARKSUITE_CLI_PROFILE") { + t.Errorf("profile --help missing LARKSUITE_CLI_PROFILE") + } + if !strings.Contains(cmd.Long, "lark-cli whoami --json") { + t.Errorf("profile --help missing whoami identity route") + } + if !strings.Contains(cmd.Long, "config show / profile list") { + t.Errorf("profile --help missing saved-config boundary") + } + const precedence = "A selected profile takes precedence over matching direct env credentials and tokens." + if !strings.Contains(cmd.Long, precedence) { + t.Errorf("profile --help missing precedence statement %q", precedence) + } +} + +func TestProfileListHelpClarifiesSavedProfiles(t *testing.T) { + cmd := NewCmdProfileList(nil) + if !strings.Contains(cmd.Short, "saved profiles") { + t.Errorf("profile list short = %q, want saved profiles", cmd.Short) + } + if !strings.Contains(cmd.Long, "lark-cli whoami --json") { + t.Errorf("profile list help missing whoami route") + } +} + func TestProfileListRun_InvalidConfigReturnsValidationError(t *testing.T) { dir := setupProfileConfigDir(t) if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte("{invalid json"), 0600); err != nil { diff --git a/cmd/whoami/whoami.go b/cmd/whoami/whoami.go index d5b4b387f9..6b5b973b70 100644 --- a/cmd/whoami/whoami.go +++ b/cmd/whoami/whoami.go @@ -10,6 +10,7 @@ import ( "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/identitydiag" "github.com/larksuite/cli/internal/output" ) @@ -33,6 +34,15 @@ type whoamiResult struct { TokenStatus string `json:"tokenStatus"` OnBehalfOf *delegatedUser `json:"onBehalfOf,omitempty"` Hint string `json:"hint,omitempty"` + + // CredentialSource, Explicit, and DirectCredentialEnv surface the cached + // credential.IdentitySelection computed during resolution (not re-inferred + // here). On the non-env extension-provider path CredentialSource is + // "extension:" (e.g. "extension:sidecar"); an empty value only + // means the selection was never resolved. + CredentialSource string `json:"credentialSource"` + Explicit bool `json:"explicit"` + DirectCredentialEnv credential.DirectCredentialEnv `json:"directCredentialEnv"` } // delegatedUser is the user a user-identity acts on behalf of. @@ -58,6 +68,10 @@ func NewCmdWhoami(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "whoami", Short: "Show the current effective identity, app, profile, and token status (JSON)", + Long: `Show the effective app identity used by this invocation. This is not OAuth login status; +use ` + "`lark-cli auth status --json`" + ` for OAuth user/token state. +The JSON output includes credentialSource, appId, brand, and whether direct app credential +env is present and matches the selected profile.`, RunE: func(cmd *cobra.Command, args []string) error { return whoamiRun(cmd, opts) }, @@ -97,7 +111,17 @@ func whoamiRun(cmd *cobra.Command, opts *Options) error { f.ResolveStrictMode(ctx).ForcedIdentity(), ) diag := identitydiag.Diagnose(ctx, f, cfg, false) - res := buildResult(cfg, as, source, diag) + // Read the cached selection computed during resolution; never re-infer it + // here. A resolution failure (e.g. under a non-env extension provider that + // doesn't populate a selection) degrades to the zero value rather than + // regressing whoami's own error/diagnostic path above. + var selection credential.IdentitySelection + if f.Credential != nil { + if sel, err := f.Credential.Selection(ctx); err == nil { + selection = sel + } + } + res := buildResult(cfg, as, source, diag, selection) output.PrintJson(f.IOStreams.Out, res) return nil } @@ -122,18 +146,23 @@ func resolveSource(changedAs bool, flagAs core.Identity, autoDetected bool, stri // buildResult maps the resolved identity and local diagnostics into the output. // ResolveAs only ever returns user or bot, so the default branch handles user. -func buildResult(cfg *core.CliConfig, as core.Identity, source string, diag identitydiag.Result) *whoamiResult { +// selection is the cached credential.IdentitySelection from resolution; it is +// read as-is, never recomputed. +func buildResult(cfg *core.CliConfig, as core.Identity, source string, diag identitydiag.Result, selection credential.IdentitySelection) *whoamiResult { defaultAs := cfg.DefaultAs if defaultAs == "" { defaultAs = core.AsAuto } res := &whoamiResult{ - Profile: cfg.ProfileName, - AppID: cfg.AppID, - Brand: cfg.Brand, - DefaultAs: string(defaultAs), - Identity: string(as), - IdentitySource: source, + Profile: cfg.ProfileName, + AppID: cfg.AppID, + Brand: cfg.Brand, + DefaultAs: string(defaultAs), + Identity: string(as), + IdentitySource: source, + CredentialSource: string(selection.Source), + Explicit: selection.Explicit(), + DirectCredentialEnv: selection.DirectCredentialEnv, } // Use the diagnosed hint as-is: it is tailored to the credential source, so // it never says "auth login" when that is blocked under an external provider. diff --git a/cmd/whoami/whoami_test.go b/cmd/whoami/whoami_test.go index 0888623974..6d1b29a6ef 100644 --- a/cmd/whoami/whoami_test.go +++ b/cmd/whoami/whoami_test.go @@ -15,10 +15,13 @@ import ( "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" "github.com/larksuite/cli/internal/identitydiag" + "github.com/larksuite/cli/internal/keychain" ) func TestResolveSource(t *testing.T) { @@ -52,7 +55,7 @@ func TestBuildResult_UserValid(t *testing.T) { diag := identitydiag.Result{ User: identitydiag.Identity{Available: true, Status: "ready", TokenStatus: "valid", OpenID: "ou_x", UserName: "Alice"}, } - r := buildResult(cfg, core.AsUser, "auto_detect", diag) + r := buildResult(cfg, core.AsUser, "auto_detect", diag, credential.IdentitySelection{}) if r.Identity != "user" || r.IdentitySource != "auto_detect" { t.Fatalf("identity/source = %q/%q", r.Identity, r.IdentitySource) @@ -77,7 +80,7 @@ func TestBuildResult_UserMissingToken(t *testing.T) { diag := identitydiag.Result{ User: identitydiag.Identity{Available: false, Status: "missing", Hint: "run: lark-cli auth login --help"}, // never logged in } - r := buildResult(cfg, core.AsUser, "auto_detect", diag) + r := buildResult(cfg, core.AsUser, "auto_detect", diag, credential.IdentitySelection{}) if r.Available { t.Fatalf("available = true, want false") @@ -100,7 +103,7 @@ func TestBuildResult_BotReady(t *testing.T) { diag := identitydiag.Result{ Bot: identitydiag.Identity{Available: true, Status: "ready"}, } - r := buildResult(cfg, core.AsBot, "default_as", diag) + r := buildResult(cfg, core.AsBot, "default_as", diag, credential.IdentitySelection{}) if r.Identity != "bot" || r.IdentitySource != "default_as" { t.Fatalf("identity/source = %q/%q", r.Identity, r.IdentitySource) @@ -121,7 +124,7 @@ func TestBuildResult_BotNotConfigured(t *testing.T) { diag := identitydiag.Result{ Bot: identitydiag.Identity{Available: false, Status: "not_configured", Hint: "run: lark-cli config --help"}, } - r := buildResult(cfg, core.AsBot, "auto_detect", diag) + r := buildResult(cfg, core.AsBot, "auto_detect", diag, credential.IdentitySelection{}) if r.Available { t.Fatalf("available = true, want false") @@ -318,3 +321,94 @@ func TestWhoami_ExternalProvider_UserHintNotKeychain(t *testing.T) { t.Fatalf("hint should explain external management: %q", got.Hint) } } + +// noopWhoamiKeychain is a no-op KeychainAccess; the profile below uses a +// plaintext secret, so no keychain lookup is actually required. +type noopWhoamiKeychain struct{} + +func (noopWhoamiKeychain) Get(service, account string) (string, error) { return "", nil } +func (noopWhoamiKeychain) Set(service, account, value string) error { return nil } +func (noopWhoamiKeychain) Remove(service, account string) error { return nil } + +// credentialSourceSecret is the profile secret written to config for +// TestWhoamiIncludesCredentialSource. It must never leak into whoami's output +// (security: never leak a secret). +const credentialSourceSecret = "test-secret" + +// profileSelectionFactory builds a Factory whose CredentialProvider resolves +// an explicit profile ("tenant_a") supplied via the LARKSUITE_CLI_PROFILE env +// fallback (not --profile), so Selection().Source resolves to +// env:LARKSUITE_CLI_PROFILE and Explicit() is true, with no direct +// app-credential env vars present. +func profileSelectionFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer) { + t.Helper() + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + multi := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.PlainSecret(credentialSourceSecret), + Brand: core.BrandFeishu, + }}, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return noopWhoamiKeychain{} }, "tenant_a") + cred := credential.NewCredentialProvider([]extcred.Provider{&envprovider.Provider{}}, defaultAcct, nil, nil) + cred.WithProfileFromEnv("tenant_a") + + cfg := &core.CliConfig{ProfileName: "tenant_a", AppID: "cli_a", AppSecret: credentialSourceSecret, Brand: core.BrandFeishu} + out := &bytes.Buffer{} + f := &cmdutil.Factory{ + Config: func() (*core.CliConfig, error) { return cfg, nil }, + Credential: cred, + IOStreams: &cmdutil.IOStreams{Out: out, ErrOut: &bytes.Buffer{}}, + } + return f, out +} + +// TestWhoamiIncludesCredentialSource locks in the diagnostic fields surfaced +// from the cached credential.IdentitySelection: credentialSource, +// explicit, and directCredentialEnv. whoami must read the cached selection +// as-is, not re-infer it. +func TestWhoamiIncludesCredentialSource(t *testing.T) { + f, out := profileSelectionFactory(t) + + cmd := NewCmdWhoami(f) + cmd.SetArgs([]string{}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute() error = %v", err) + } + + raw := out.String() + if strings.Contains(raw, credentialSourceSecret) { + t.Fatalf("whoami output leaked the profile secret: %s", raw) + } + + var got whoamiResult + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatalf("json.Unmarshal() error = %v\n%s", err, raw) + } + if got.CredentialSource != string(credential.SourceEnvProfile) { + t.Fatalf("credentialSource = %q, want %q", got.CredentialSource, credential.SourceEnvProfile) + } + if !got.Explicit { + t.Fatalf("explicit = false, want true") + } + if got.DirectCredentialEnv.Present { + t.Fatalf("directCredentialEnv.present = true, want false: %#v", got.DirectCredentialEnv) + } + if !strings.Contains(raw, `"credentialSource": "env:LARKSUITE_CLI_PROFILE"`) { + t.Fatalf("raw JSON missing credentialSource literal: %s", raw) + } + if got.DirectCredentialEnv.Present || len(got.DirectCredentialEnv.Keys) != 0 || + got.DirectCredentialEnv.AppID != "" || got.DirectCredentialEnv.Matched || got.DirectCredentialEnv.ConflictsWithProfile { + t.Fatalf("directCredentialEnv = %#v, want only present:false set", got.DirectCredentialEnv) + } +} diff --git a/errs/ERROR_CONTRACT.md b/errs/ERROR_CONTRACT.md index 5262ce58dd..383a640536 100644 --- a/errs/ERROR_CONTRACT.md +++ b/errs/ERROR_CONTRACT.md @@ -67,6 +67,17 @@ Typed errors render to **stderr** as one JSON object per process exit: | `error.params` | per-Subtype-stable | per-parameter validation detail array (`ValidationError`); see **Validation parameters** | | per-Subtype extension fields | per-Subtype-stable | e.g. `missing_scopes`, `console_url`, `challenge_url` | +Credential/identity-selection extension fields (per-Subtype-stable): + +| Field | Carrier | Subtypes | Notes | +|-------|---------|----------|-------| +| `missing_keys` | `ConfigError` | `app_credential_incomplete` | env var NAMES that must all be set; never values | +| `required_any_of` | `ConfigError` | `app_credential_incomplete` | env var NAMES where any one completes the credential; mutually exclusive with `missing_keys` | +| `profile` | `ConfigError` | `profile_not_found`, `profile_secret_invalid` | requested profile name | +| `app_id` | `ConfigError` | `profile_secret_invalid` | plaintext app id; never a secret | +| `credential_source` | `ConfigError` | `profile_not_found`, `no_active_profile` | how the identity was (not) chosen: `flag:--profile` \| `env:LARKSUITE_CLI_PROFILE` \| `config` | +| `profile_app_id`, `env_app_id` | `ValidationError` | `profile_app_credential_conflict` | the two conflicting plaintext app ids | + `SecurityPolicyError` renders through the same typed envelope as every other category. `error.type` is `"policy"`, `error.subtype` is one of `challenge_required` / `access_denied`, and process exit is `6` via diff --git a/errs/marshal_test.go b/errs/marshal_test.go index b495ad78f4..c9de3983c6 100644 --- a/errs/marshal_test.go +++ b/errs/marshal_test.go @@ -136,6 +136,79 @@ func TestConfigError_MarshalJSON(t *testing.T) { } } +func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { + ce := NewConfigError(SubtypeAppCredentialIncomplete, "incomplete"). + WithMissingKeys("LARKSUITE_CLI_APP_ID", "LARKSUITE_CLI_APP_SECRET"). + WithRequiredAnyOf("LARKSUITE_CLI_APP_SECRET", "LARKSUITE_CLI_USER_ACCESS_TOKEN"). + WithProfile("work"). + WithAppID("cli_abc"). + WithCredentialSource("flag:--profile") + b, err := json.Marshal(ce) + if err != nil { + t.Fatal(err) + } + s := string(b) + for _, want := range []string{ + `"type":"config"`, + `"subtype":"app_credential_incomplete"`, + `"missing_keys":["LARKSUITE_CLI_APP_ID","LARKSUITE_CLI_APP_SECRET"]`, + `"required_any_of":["LARKSUITE_CLI_APP_SECRET","LARKSUITE_CLI_USER_ACCESS_TOKEN"]`, + `"profile":"work"`, + `"app_id":"cli_abc"`, + `"credential_source":"flag:--profile"`, + } { + if !strings.Contains(s, want) { + t.Errorf("missing %q in %s", want, s) + } + } + + // omitempty: unset fields must not appear on the wire. + empty := NewConfigError(SubtypeProfileNotFound, "x") + b2, err := json.Marshal(empty) + if err != nil { + t.Fatal(err) + } + s2 := string(b2) + for _, notWant := range []string{`"missing_keys"`, `"required_any_of"`, `"profile"`, `"app_id"`, `"credential_source"`} { + if strings.Contains(s2, notWant) { + t.Errorf("%q should be omitted when empty; got %s", notWant, s2) + } + } +} + +func TestValidationError_ProfileConflictMarshalJSON(t *testing.T) { + ve := NewValidationError(SubtypeProfileAppCredentialConflict, "conflict"). + WithProfileAppConflict("cli_profile", "cli_env") + b, err := json.Marshal(ve) + if err != nil { + t.Fatal(err) + } + s := string(b) + for _, want := range []string{ + `"type":"validation"`, + `"subtype":"profile_app_credential_conflict"`, + `"profile_app_id":"cli_profile"`, + `"env_app_id":"cli_env"`, + } { + if !strings.Contains(s, want) { + t.Errorf("missing %q in %s", want, s) + } + } + + // omitempty: unset conflict fields must not appear on the wire. + empty := NewValidationError(SubtypeInvalidArgument, "x") + b2, err := json.Marshal(empty) + if err != nil { + t.Fatal(err) + } + s2 := string(b2) + for _, notWant := range []string{`"profile_app_id"`, `"env_app_id"`} { + if strings.Contains(s2, notWant) { + t.Errorf("%q should be omitted when empty; got %s", notWant, s2) + } + } +} + func TestNetworkError_MarshalJSON(t *testing.T) { ne := &NetworkError{ Problem: Problem{Category: CategoryNetwork, Subtype: SubtypeNetworkTimeout, Message: "dial timeout"}, diff --git a/errs/subtypes.go b/errs/subtypes.go index df0cf8ab85..b84c5266b9 100644 --- a/errs/subtypes.go +++ b/errs/subtypes.go @@ -12,8 +12,9 @@ const ( // CategoryValidation subtypes const ( - SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment) - SubtypeFailedPrecondition Subtype = "failed_precondition" // request is valid but the system/resource state is not in the state required to execute; caller must change state (not retry) — e.g. ambiguous remote mapping (gRPC FAILED_PRECONDITION alignment) + SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment) + SubtypeFailedPrecondition Subtype = "failed_precondition" // request is valid but the system/resource state is not in the state required to execute; caller must change state (not retry) — e.g. ambiguous remote mapping (gRPC FAILED_PRECONDITION alignment) + SubtypeProfileAppCredentialConflict Subtype = "profile_app_credential_conflict" // profile and direct app env both set but app_id differs ) // CategoryAuthentication subtypes @@ -41,9 +42,13 @@ const ( // CategoryConfig subtypes const ( - SubtypeInvalidClient Subtype = "invalid_client" // app_id / app_secret incorrect (RFC 6749 §5.2 alignment) - SubtypeNotConfigured Subtype = "not_configured" // local config file absent (user has not run `config init`) - SubtypeInvalidConfig Subtype = "invalid_config" // local config file present but malformed + SubtypeInvalidClient Subtype = "invalid_client" // app_id / app_secret incorrect (RFC 6749 §5.2 alignment) + SubtypeNotConfigured Subtype = "not_configured" // local config file absent (user has not run `config init`) + SubtypeInvalidConfig Subtype = "invalid_config" // local config file present but malformed + SubtypeProfileNotFound Subtype = "profile_not_found" // --profile / LARKSUITE_CLI_PROFILE points to a nonexistent profile + SubtypeNoActiveProfile Subtype = "no_active_profile" // no active identity input and no usable default profile + SubtypeAppCredentialIncomplete Subtype = "app_credential_incomplete" // direct app env missing app_id or app_secret + SubtypeProfileSecretInvalid Subtype = "profile_secret_invalid" // profile exists but its secret cannot be resolved locally ) // CategoryNetwork subtypes diff --git a/errs/types.go b/errs/types.go index 94b67d65f7..027c9d5432 100644 --- a/errs/types.go +++ b/errs/types.go @@ -61,9 +61,11 @@ type TypedError interface { // it is intentionally not serialized. type ValidationError struct { Problem - Param string `json:"param,omitempty"` - Params []InvalidParam `json:"params,omitempty"` - Cause error `json:"-"` + Param string `json:"param,omitempty"` + Params []InvalidParam `json:"params,omitempty"` + ProfileAppID string `json:"profile_app_id,omitempty"` + EnvAppID string `json:"env_app_id,omitempty"` + Cause error `json:"-"` } // InvalidParam is one structured validation diagnostic: the parameter that @@ -150,6 +152,12 @@ func (e *ValidationError) WithCause(cause error) *ValidationError { return e } +func (e *ValidationError) WithProfileAppConflict(profileAppID, envAppID string) *ValidationError { + e.ProfileAppID = profileAppID + e.EnvAppID = envAppID + return e +} + // =========================== AuthenticationError ============================= // AuthenticationError is the typed error for CategoryAuthentication. @@ -315,8 +323,18 @@ func (e *PermissionError) WithCause(cause error) *PermissionError { // intentionally not serialized. type ConfigError struct { Problem - Field string `json:"field,omitempty"` - Cause error `json:"-"` + Field string `json:"field,omitempty"` + MissingKeys []string `json:"missing_keys,omitempty"` + RequiredAnyOf []string `json:"required_any_of,omitempty"` + Profile string `json:"profile,omitempty"` + AppID string `json:"app_id,omitempty"` + // CredentialSource is the machine-readable App/credential selection source + // that produced this config error (e.g. "flag:--profile", + // "env:LARKSUITE_CLI_PROFILE", "config"). It is required on + // profile_not_found and no_active_profile so an agent can branch + // on how the identity was (or was not) chosen. It is never a secret. + CredentialSource string `json:"credential_source,omitempty"` + Cause error `json:"-"` } // Unwrap is nil-receiver safe; see ValidationError.Unwrap. @@ -370,6 +388,34 @@ func (e *ConfigError) WithField(field string) *ConfigError { return e } +func (e *ConfigError) WithMissingKeys(keys ...string) *ConfigError { + e.MissingKeys = slices.Clone(keys) + return e +} + +func (e *ConfigError) WithRequiredAnyOf(keys ...string) *ConfigError { + e.RequiredAnyOf = slices.Clone(keys) + return e +} + +func (e *ConfigError) WithProfile(name string) *ConfigError { + e.Profile = name + return e +} + +func (e *ConfigError) WithAppID(appID string) *ConfigError { + e.AppID = appID + return e +} + +// WithCredentialSource records the machine-readable credential-selection source +// on the wire (snake_case credential_source). The value is an enum string +// (e.g. "flag:--profile", "config"), never a secret. +func (e *ConfigError) WithCredentialSource(source string) *ConfigError { + e.CredentialSource = source + return e +} + func (e *ConfigError) WithCause(cause error) *ConfigError { e.Cause = cause return e diff --git a/errs/types_test.go b/errs/types_test.go index 8279c2e46b..baac931e53 100644 --- a/errs/types_test.go +++ b/errs/types_test.go @@ -643,3 +643,29 @@ func TestBuilderSetter_DefensiveCopy(t *testing.T) { } }) } + +// ======================= Profile selection error subtypes ======================= + +func TestConfigErrorProfileFields(t *testing.T) { + e := errs.NewConfigError(errs.SubtypeAppCredentialIncomplete, "incomplete"). + WithMissingKeys("LARKSUITE_CLI_APP_ID"). + WithCredentialSource("env:LARKSUITE_CLI_PROFILE") + p, ok := errs.ProblemOf(e) + if !ok || p.Subtype != errs.SubtypeAppCredentialIncomplete { + t.Fatalf("subtype mismatch: %+v", p) + } + if len(e.MissingKeys) != 1 || e.MissingKeys[0] != "LARKSUITE_CLI_APP_ID" { + t.Errorf("missing_keys not set: %v", e.MissingKeys) + } + if e.CredentialSource != "env:LARKSUITE_CLI_PROFILE" { + t.Errorf("credential_source not set: %q", e.CredentialSource) + } +} + +func TestValidationErrorProfileConflict(t *testing.T) { + e := errs.NewValidationError(errs.SubtypeProfileAppCredentialConflict, "conflict"). + WithProfileAppConflict("cli_profile", "cli_env") + if e.ProfileAppID != "cli_profile" || e.EnvAppID != "cli_env" { + t.Errorf("conflict fields not set: %q %q", e.ProfileAppID, e.EnvAppID) + } +} diff --git a/extension/credential/env/env.go b/extension/credential/env/env.go index 5458125689..e45686433a 100644 --- a/extension/credential/env/env.go +++ b/extension/credential/env/env.go @@ -23,62 +23,88 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err appSecret := os.Getenv(envvars.CliAppSecret) hasUAT := os.Getenv(envvars.CliUserAccessToken) != "" hasTAT := os.Getenv(envvars.CliTenantAccessToken) != "" - if appID == "" && appSecret == "" { - switch { - case hasUAT: - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliUserAccessToken + " is set but " + envvars.CliAppID + " is missing"} - case hasTAT: - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliTenantAccessToken + " is set but " + envvars.CliAppID + " is missing"} - default: - return nil, nil - } - } - if appID == "" { - return nil, &credential.BlockError{Provider: "env", Reason: envvars.CliAppSecret + " is set but " + envvars.CliAppID + " is missing"} - } - if appSecret == "" && !hasUAT && !hasTAT { - return nil, &credential.BlockError{ - Provider: "env", - Reason: envvars.CliAppID + " is set but no app secret or access token is available", - } + presentKeys := presentCredentialEnvKeys(appID, appSecret, hasUAT, hasTAT) + if len(presentKeys) == 0 { + return nil, nil } - brand := credential.Brand(core.ParseBrand(os.Getenv(envvars.CliBrand))) - acct := &credential.Account{AppID: appID, AppSecret: appSecret, Brand: brand} - switch id := credential.Identity(os.Getenv(envvars.CliDefaultAs)); id { - case "", credential.IdentityAuto: - acct.DefaultAs = id - case credential.IdentityUser, credential.IdentityBot: - acct.DefaultAs = id + // Identity policy variables are validated whenever a direct credential + // input is present. Their errors must not be hidden by a later credential + // completeness check or profile arbitration. + defaultAs := credential.Identity(os.Getenv(envvars.CliDefaultAs)) + switch defaultAs { + case "", credential.IdentityAuto, credential.IdentityUser, credential.IdentityBot: default: return nil, &credential.BlockError{ Provider: "env", - Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, id), + Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, defaultAs), + Code: credential.BlockReasonInvalidPolicy, + Param: envvars.CliDefaultAs, } } - // Explicit strict mode policy takes priority - switch strictMode := os.Getenv(envvars.CliStrictMode); strictMode { + strictMode := os.Getenv(envvars.CliStrictMode) + var supported credential.IdentitySupport + switch strictMode { case "bot": - acct.SupportedIdentities = credential.SupportsBot + supported = credential.SupportsBot case "user": - acct.SupportedIdentities = credential.SupportsUser + supported = credential.SupportsUser case "off": - acct.SupportedIdentities = credential.SupportsAll + supported = credential.SupportsAll case "": - // Infer from available tokens if hasUAT { - acct.SupportedIdentities |= credential.SupportsUser + supported |= credential.SupportsUser } if hasTAT { - acct.SupportedIdentities |= credential.SupportsBot + supported |= credential.SupportsBot } default: return nil, &credential.BlockError{ Provider: "env", Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envvars.CliStrictMode, strictMode), + Code: credential.BlockReasonInvalidPolicy, + Param: envvars.CliStrictMode, + } + } + + if appID == "" && appSecret == "" { + switch { + case hasUAT: + return nil, incompleteCredentialError( + appID, + envvars.CliUserAccessToken+" is set but "+envvars.CliAppID+" is missing", + []string{envvars.CliAppID}, nil, presentKeys) + case hasTAT: + return nil, incompleteCredentialError( + appID, + envvars.CliTenantAccessToken+" is set but "+envvars.CliAppID+" is missing", + []string{envvars.CliAppID}, nil, presentKeys) } } + if appID == "" { + return nil, incompleteCredentialError( + appID, + envvars.CliAppSecret+" is set but "+envvars.CliAppID+" is missing", + []string{envvars.CliAppID}, nil, presentKeys) + } + if appSecret == "" && !hasUAT && !hasTAT { + return nil, incompleteCredentialError( + appID, + envvars.CliAppID+" is set but no app secret or access token is available", + nil, + []string{envvars.CliAppSecret, envvars.CliUserAccessToken, envvars.CliTenantAccessToken}, + presentKeys) + } + brand := credential.Brand(core.ParseBrand(os.Getenv(envvars.CliBrand))) + acct := &credential.Account{ + AppID: appID, + AppSecret: appSecret, + Brand: brand, + DefaultAs: defaultAs, + SupportedIdentities: supported, + Kind: credential.AccountDirect, + } if acct.DefaultAs == "" { switch { @@ -92,6 +118,35 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err return acct, nil } +func incompleteCredentialError(appID, reason string, missingKeys, requiredAnyOf, presentKeys []string) *credential.BlockError { + return &credential.BlockError{ + Provider: "env", + Reason: reason, + Code: credential.BlockReasonCredentialIncomplete, + MissingKeys: missingKeys, + RequiredAnyOf: requiredAnyOf, + PresentKeys: presentKeys, + AppID: appID, + } +} + +func presentCredentialEnvKeys(appID, appSecret string, hasUAT, hasTAT bool) []string { + var keys []string + if appID != "" { + keys = append(keys, envvars.CliAppID) + } + if appSecret != "" { + keys = append(keys, envvars.CliAppSecret) + } + if hasUAT { + keys = append(keys, envvars.CliUserAccessToken) + } + if hasTAT { + keys = append(keys, envvars.CliTenantAccessToken) + } + return keys +} + func (p *Provider) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.Token, error) { var envKey string switch req.Type { diff --git a/extension/credential/env/env_test.go b/extension/credential/env/env_test.go index 096bd9fe29..ddaae85e26 100644 --- a/extension/credential/env/env_test.go +++ b/extension/credential/env/env_test.go @@ -6,6 +6,7 @@ package env import ( "context" "errors" + "slices" "strings" "testing" @@ -47,6 +48,22 @@ func TestResolveAccount_OnlyIDSet(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %v", err) } + if blockErr.Code != credential.BlockReasonCredentialIncomplete { + t.Fatalf("Code = %q, want %q", blockErr.Code, credential.BlockReasonCredentialIncomplete) + } + want := []string{envvars.CliAppSecret, envvars.CliUserAccessToken, envvars.CliTenantAccessToken} + if !slices.Equal(blockErr.RequiredAnyOf, want) { + t.Fatalf("RequiredAnyOf = %v, want %v", blockErr.RequiredAnyOf, want) + } + if len(blockErr.MissingKeys) != 0 { + t.Fatalf("MissingKeys = %v, want empty", blockErr.MissingKeys) + } + if !slices.Equal(blockErr.PresentKeys, []string{envvars.CliAppID}) { + t.Fatalf("PresentKeys = %v, want [%s]", blockErr.PresentKeys, envvars.CliAppID) + } + if blockErr.AppID != "cli_test" { + t.Fatalf("AppID = %q, want cli_test", blockErr.AppID) + } } func TestResolveAccount_AppIDAndUserTokenWithoutSecret(t *testing.T) { @@ -75,18 +92,81 @@ func TestResolveAccount_OnlySecretSet(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %v", err) } + if blockErr.Code != credential.BlockReasonCredentialIncomplete || + !slices.Equal(blockErr.MissingKeys, []string{envvars.CliAppID}) || + !slices.Equal(blockErr.PresentKeys, []string{envvars.CliAppSecret}) { + t.Fatalf("BlockError = %+v, want incomplete with missing APP_ID and present APP_SECRET", blockErr) + } + if len(blockErr.RequiredAnyOf) != 0 { + t.Fatalf("RequiredAnyOf = %v, want empty for APP_SECRET-only", blockErr.RequiredAnyOf) + } } func TestResolveAccount_OnlyTokenSetWithoutAppID(t *testing.T) { - t.Setenv(envvars.CliUserAccessToken, "uat_test") + for _, tt := range []struct { + name string + key string + }{ + {name: "UAT", key: envvars.CliUserAccessToken}, + {name: "TAT", key: envvars.CliTenantAccessToken}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.key, "token_test") - _, err := (&Provider{}).ResolveAccount(context.Background()) - var blockErr *credential.BlockError - if !errors.As(err, &blockErr) { - t.Fatalf("expected BlockError, got %v", err) + _, err := (&Provider{}).ResolveAccount(context.Background()) + var blockErr *credential.BlockError + if !errors.As(err, &blockErr) { + t.Fatalf("expected BlockError, got %v", err) + } + if !strings.Contains(err.Error(), envvars.CliAppID) { + t.Fatalf("error = %v, want mention of %s", err, envvars.CliAppID) + } + if blockErr.Code != credential.BlockReasonCredentialIncomplete || + !slices.Equal(blockErr.MissingKeys, []string{envvars.CliAppID}) || + !slices.Equal(blockErr.PresentKeys, []string{tt.key}) { + t.Fatalf("BlockError = %+v, want incomplete for %s", blockErr, tt.key) + } + if len(blockErr.RequiredAnyOf) != 0 { + t.Fatalf("RequiredAnyOf = %v, want empty for %s-only", blockErr.RequiredAnyOf, tt.name) + } + }) } - if !strings.Contains(err.Error(), envvars.CliAppID) { - t.Fatalf("error = %v, want mention of %s", err, envvars.CliAppID) +} + +func TestResolveAccount_InvalidPolicyRejectedBeforeIncomplete(t *testing.T) { + for _, tt := range []struct { + name string + key string + }{ + {name: "DEFAULT_AS", key: envvars.CliDefaultAs}, + {name: "STRICT_MODE", key: envvars.CliStrictMode}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_test") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.key, "banana") + + _, err := (&Provider{}).ResolveAccount(context.Background()) + var blockErr *credential.BlockError + if !errors.As(err, &blockErr) { + t.Fatalf("error = %T %v, want BlockError", err, err) + } + if blockErr.Code != credential.BlockReasonInvalidPolicy { + t.Fatalf("Code = %q, want %q", blockErr.Code, credential.BlockReasonInvalidPolicy) + } + if blockErr.Param != tt.key { + t.Fatalf("Param = %q, want %q", blockErr.Param, tt.key) + } + if !strings.Contains(blockErr.Reason, tt.key) { + t.Fatalf("reason = %q, want %s", blockErr.Reason, tt.key) + } + }) } } @@ -258,6 +338,9 @@ func TestResolveAccount_InvalidStrictModeRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } + if blockErr.Code != credential.BlockReasonInvalidPolicy || blockErr.Param != envvars.CliStrictMode { + t.Fatalf("BlockError = %+v, want invalid_policy with Param %s", blockErr, envvars.CliStrictMode) + } if !strings.Contains(err.Error(), envvars.CliStrictMode) { t.Fatalf("error = %v, want mention of %s", err, envvars.CliStrictMode) } @@ -276,6 +359,9 @@ func TestResolveAccount_InvalidDefaultAsRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } + if blockErr.Code != credential.BlockReasonInvalidPolicy || blockErr.Param != envvars.CliDefaultAs { + t.Fatalf("BlockError = %+v, want invalid_policy with Param %s", blockErr, envvars.CliDefaultAs) + } if !strings.Contains(err.Error(), envvars.CliDefaultAs) { t.Fatalf("error = %v, want mention of %s", err, envvars.CliDefaultAs) } diff --git a/extension/credential/sidecar/provider.go b/extension/credential/sidecar/provider.go index d01f56616b..1ca31ccc1c 100644 --- a/extension/credential/sidecar/provider.go +++ b/extension/credential/sidecar/provider.go @@ -77,6 +77,8 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err return nil, &credential.BlockError{ Provider: "sidecar", Reason: fmt.Sprintf("invalid %s %q (want user, bot, or auto)", envvars.CliDefaultAs, id), + Code: credential.BlockReasonInvalidPolicy, + Param: envvars.CliDefaultAs, } } @@ -92,6 +94,8 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err return nil, &credential.BlockError{ Provider: "sidecar", Reason: fmt.Sprintf("invalid %s %q (want bot, user, or off)", envvars.CliStrictMode, strictMode), + Code: credential.BlockReasonInvalidPolicy, + Param: envvars.CliStrictMode, } } diff --git a/extension/credential/sidecar/provider_test.go b/extension/credential/sidecar/provider_test.go index e265b5652f..8873c12050 100644 --- a/extension/credential/sidecar/provider_test.go +++ b/extension/credential/sidecar/provider_test.go @@ -7,7 +7,9 @@ package sidecar import ( "context" + "errors" "os" + "strings" "testing" "github.com/larksuite/cli/extension/credential" @@ -146,6 +148,57 @@ func TestResolveAccount_StrictMode(t *testing.T) { } } +func TestResolveAccount_InvalidPolicyClassified(t *testing.T) { + setEnv(t, envvars.CliAuthProxy, "http://127.0.0.1:16384") + setEnv(t, envvars.CliProxyKey, "test-key") + setEnv(t, envvars.CliAppID, "cli_test") + + tests := []struct { + name string + key string + value string + supportedText string + }{ + { + name: "default as", + key: envvars.CliDefaultAs, + value: "banana", + supportedText: "want user, bot, or auto", + }, + { + name: "strict mode", + key: envvars.CliStrictMode, + value: "banana", + supportedText: "want bot, user, or off", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + unsetEnv(t, envvars.CliDefaultAs) + unsetEnv(t, envvars.CliStrictMode) + setEnv(t, tt.key, tt.value) + + _, err := (&Provider{}).ResolveAccount(context.Background()) + var blockErr *credential.BlockError + if !errors.As(err, &blockErr) { + t.Fatalf("error = %T %v, want BlockError", err, err) + } + if blockErr.Code != credential.BlockReasonInvalidPolicy { + t.Fatalf("Code = %q, want %q", blockErr.Code, credential.BlockReasonInvalidPolicy) + } + if blockErr.Param != tt.key { + t.Fatalf("Param = %q, want %q", blockErr.Param, tt.key) + } + if !strings.Contains(blockErr.Reason, tt.key) || + !strings.Contains(blockErr.Reason, tt.value) || + !strings.Contains(blockErr.Reason, tt.supportedText) { + t.Fatalf("Reason = %q, want variable, invalid value, and supported values", blockErr.Reason) + } + }) + } +} + func TestResolveToken_NotActive(t *testing.T) { unsetEnv(t, envvars.CliAuthProxy) diff --git a/extension/credential/types.go b/extension/credential/types.go index 209013fda5..4c90e2227f 100644 --- a/extension/credential/types.go +++ b/extension/credential/types.go @@ -44,6 +44,27 @@ func (s IdentitySupport) UserOnly() bool { return s == SupportsUser } // BotOnly returns true if only bot identity is supported. func (s IdentitySupport) BotOnly() bool { return s == SupportsBot } +// AccountKind declares how an account participates in credential arbitration. +type AccountKind int + +const ( + // AccountManaged means the provider owns the whole identity; winning it + // ends arbitration outright. The zero value, so existing providers are + // unchanged. + AccountManaged AccountKind = iota + // AccountDirect marks an actively supplied raw credential (the env + // provider's LARKSUITE_CLI_* variables). It participates in profile + // arbitration and conflict detection instead of winning outright. + // + // RESERVED: only the builtin env provider may declare AccountDirect + // today — the arbitration's direct-credential diagnostics are defined in + // terms of the process environment, and the caller rejects AccountDirect + // from any other provider. Third-party providers must return + // AccountManaged until the SPI carries provider-reported input + // descriptors. + AccountDirect +) + // Account holds resolved app credentials and configuration. type Account struct { AppID string @@ -53,6 +74,7 @@ type Account struct { ProfileName string OpenID string // optional; if UAT is available, API result takes precedence SupportedIdentities IdentitySupport // zero = provider did not declare; treat as no restriction + Kind AccountKind // AccountManaged (default) or AccountDirect } // Token holds a resolved access token and optional metadata. @@ -76,11 +98,38 @@ type TokenSpec struct { AppID string } +// BlockReason classifies provider-originated block conditions that callers may +// safely map to a more specific public error contract. +type BlockReason string + +const ( + // BlockReasonCredentialIncomplete marks incomplete inputs from the builtin + // process-env credential provider. It is reserved for that provider because + // direct-credential arbitration and diagnostics currently name the fixed + // LARKSUITE_CLI_* env surface. Third-party providers must return an + // unclassified BlockError until the SPI carries provider-owned input + // descriptors. Blocks without a Code propagate unchanged. + BlockReasonCredentialIncomplete BlockReason = "credential_incomplete" + + // BlockReasonInvalidPolicy marks a user-supplied policy input (e.g. + // LARKSUITE_CLI_DEFAULT_AS / LARKSUITE_CLI_STRICT_MODE) that failed + // validation. The caller maps it to a typed validation error carrying + // Param and a repair hint, so user input mistakes never surface as + // internal errors. + BlockReasonInvalidPolicy BlockReason = "invalid_policy" +) + // BlockError is returned by a Provider to actively reject a request // and prevent subsequent providers in the chain from being consulted. type BlockError struct { - Provider string - Reason string + Provider string + Reason string + Code BlockReason + MissingKeys []string // environment variable names only; never values + RequiredAnyOf []string // environment variable names only; never values + PresentKeys []string // environment variable names only; never values + AppID string // plaintext app identifier used only for source comparison; never a secret + Param string // name of the invalid input variable on invalid_policy blocks; never a value } func (e *BlockError) Error() string { diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 97018078d0..8ccc4b796a 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -48,6 +48,18 @@ func (s *staticTokenResolver) ResolveToken(_ context.Context, _ credential.Token return &credential.TokenResult{Token: "test-token"}, nil } +type clientTestAccountResolver struct { + appID string +} + +func (r clientTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID, Brand: core.BrandFeishu}, nil +} + +func newClientTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, clientTestAccountResolver{appID: appID}, tokenResolver, nil) +} + // newTestAPIClient creates an APIClient with a mock HTTP transport. func newTestAPIClient(t *testing.T, rt http.RoundTripper) (*APIClient, *bytes.Buffer) { t.Helper() @@ -58,7 +70,7 @@ func newTestAPIClient(t *testing.T, rt http.RoundTripper) (*APIClient, *bytes.Bu lark.WithLogLevel(larkcore.LogLevelError), lark.WithHttpClient(httpClient), ) - testCred := credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil) + testCred := newClientTestCredentialProvider("test-app", &staticTokenResolver{}) cfg := &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu} return &APIClient{ SDK: sdk, @@ -463,7 +475,7 @@ func TestDoStream_IgnoresBaseHTTPClientTimeout(t *testing.T) { ac := &APIClient{ HTTP: &http.Client{Timeout: 5 * time.Millisecond}, - Credential: credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil), + Credential: newClientTestCredentialProvider("test-app", &staticTokenResolver{}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, } @@ -498,7 +510,7 @@ func TestDoStream_TransportFailureSplitsSubtype(t *testing.T) { }) ac := &APIClient{ HTTP: &http.Client{Transport: rt}, - Credential: credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil), + Credential: newClientTestCredentialProvider("test-app", &staticTokenResolver{}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, } @@ -532,7 +544,7 @@ func (f *failingTokenResolver) ResolveToken(_ context.Context, spec credential.T func TestResolveAccessToken_NoToken_ReturnsTypedAuthenticationError(t *testing.T) { ac := &APIClient{ HTTP: &http.Client{}, - Credential: credential.NewCredentialProvider(nil, nil, &failingTokenResolver{}, nil), + Credential: newClientTestCredentialProvider("test-app", &failingTokenResolver{}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, } @@ -572,7 +584,7 @@ func (f *needAuthTokenResolver) ResolveToken(_ context.Context, _ credential.Tok func TestResolveAccessToken_NeedAuthorization_SurfacesAsTypedAuthentication(t *testing.T) { ac := &APIClient{ HTTP: &http.Client{}, - Credential: credential.NewCredentialProvider(nil, nil, &needAuthTokenResolver{userOpenID: "ou_test_user"}, nil), + Credential: newClientTestCredentialProvider("test-app", &needAuthTokenResolver{userOpenID: "ou_test_user"}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, } @@ -612,7 +624,7 @@ func TestResolveAccessToken_NeedAuthorization_SurfacesAsTypedAuthentication(t *t func TestDoSDKRequest_AuthFailureSurfacesTypedAuthenticationError(t *testing.T) { ac := &APIClient{ HTTP: &http.Client{}, - Credential: credential.NewCredentialProvider(nil, nil, &failingTokenResolver{}, nil), + Credential: newClientTestCredentialProvider("test-app", &failingTokenResolver{}), Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, } diff --git a/internal/cmdutil/factory.go b/internal/cmdutil/factory.go index 1b167b7863..c90fa1d4a7 100644 --- a/internal/cmdutil/factory.go +++ b/internal/cmdutil/factory.go @@ -27,6 +27,11 @@ import ( // In tests, replace any field to stub out external dependencies. type InvocationContext struct { Profile string + // ProfileFromFlag is true when Profile was set via the --profile flag, + // and false when it came from the LARKSUITE_CLI_PROFILE env fallback + // (or neither was set). Downstream credential resolution uses this to + // report the correct profile source. + ProfileFromFlag bool } type Factory struct { diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index 9527346a7b..5a5a76516a 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -63,10 +63,11 @@ func NewDefault(streams *IOStreams, inv InvocationContext) *Factory { // Phase 2: Credential (sole data source) // Keychain is read via closure so callers can replace f.Keychain after construction. f.Credential = buildCredentialProvider(credentialDeps{ - Keychain: func() keychain.KeychainAccess { return f.Keychain }, - Profile: inv.Profile, - HttpClient: f.HttpClient, - ErrOut: f.IOStreams.ErrOut, + Keychain: func() keychain.KeychainAccess { return f.Keychain }, + Profile: inv.Profile, + ProfileFromFlag: inv.ProfileFromFlag, + HttpClient: f.HttpClient, + ErrOut: f.IOStreams.ErrOut, }) // Phase 3: Runtime config contains resolved account data only. @@ -174,10 +175,11 @@ func wrapSDKTransport(next http.RoundTripper) http.RoundTripper { } type credentialDeps struct { - Keychain func() keychain.KeychainAccess - Profile string - HttpClient func() (*http.Client, error) - ErrOut io.Writer + Keychain func() keychain.KeychainAccess + Profile string + ProfileFromFlag bool + HttpClient func() (*http.Client, error) + ErrOut io.Writer } func buildCredentialProvider(deps credentialDeps) *credential.CredentialProvider { @@ -190,5 +192,13 @@ func buildCredentialProvider(deps credentialDeps) *credential.CredentialProvider // depend on. enrichUserInfo failures are already non-fatal (the // provider clears unverified identity fields), so silencing the // warning is safe. - return credential.NewCredentialProvider(providers, defaultAcct, defaultToken, deps.HttpClient) + cred := credential.NewCredentialProvider(providers, defaultAcct, defaultToken, deps.HttpClient) + if deps.Profile == "" { + // No profile selected — don't record a phantom env source. + return cred + } + if deps.ProfileFromFlag { + return cred.WithProfileFromFlag(deps.Profile) + } + return cred.WithProfileFromEnv(deps.Profile) } diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go index 97eed80975..a5f00d7b5f 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -13,6 +13,7 @@ import ( "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/envvars" @@ -405,6 +406,14 @@ type stubExtProvider struct { err error } +type stubDefaultAccountResolver struct { + acct *credential.Account +} + +func (s *stubDefaultAccountResolver) ResolveAccount(_ context.Context) (*credential.Account, error) { + return s.acct, nil +} + func (s *stubExtProvider) Name() string { return s.name } func (s *stubExtProvider) ResolveAccount(_ context.Context) (*extcred.Account, error) { return s.acct, s.err @@ -448,6 +457,86 @@ func TestRequireBuiltinCredentialProvider_AllowsBuiltinProvider(t *testing.T) { } } +func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + if err := core.SaveMultiAppConfig(&core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.PlainSecret("test-secret"), + Brand: core.BrandFeishu, + }}, + }); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + cred := credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, + &stubDefaultAccountResolver{acct: &credential.Account{AppID: "cli_a", AppSecret: "test-secret"}}, + nil, + nil, + ).WithProfileFromFlag("tenant_a") + f, _, _, _ := TestFactory(t, nil) + f.Credential = cred + + if err := f.RequireBuiltinCredentialProvider(context.Background(), "auth"); err != nil { + t.Fatalf("matching APP_ID-only profile should use builtin credentials: %v", err) + } +} + +// A stale LARKSUITE_CLI_PROFILE (profile that cannot resolve) must not lock +// the user out of the builtin setup/repair commands this gate guards: the +// probe falls back to provider engagement and lets the command run. +func TestRequireBuiltinCredentialProvider_StaleProfileDoesNotLockOut(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) // no config -> "ghost" cannot resolve + + stub := &stubExtProvider{name: "env"} // not engaged: returns nil, nil + cred := credential.NewCredentialProvider( + []extcred.Provider{stub}, + &stubDefaultAccountResolver{}, + nil, + nil, + ).WithProfileFromEnv("ghost") + f, _, _, _ := TestFactory(t, nil) + f.Credential = cred + + if err := f.RequireBuiltinCredentialProvider(context.Background(), "config"); err != nil { + t.Fatalf("stale profile must not lock out builtin auth/config commands: %v", err) + } +} + +// An invalid policy variable (e.g. LARKSUITE_CLI_DEFAULT_AS=banana) is a user +// input error, not an external credential takeover: the gate surfaces the +// same typed validation error as formal arbitration instead of a misleading +// "provided externally" refusal. +func TestRequireBuiltinCredentialProvider_InvalidPolicySurfacesTypedError(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + stub := &stubExtProvider{name: "env", err: &extcred.BlockError{ + Provider: "env", + Reason: "invalid LARKSUITE_CLI_DEFAULT_AS \"banana\" (want user, bot, or auto)", + Code: extcred.BlockReasonInvalidPolicy, + Param: envvars.CliDefaultAs, + }} + cred := credential.NewCredentialProvider([]extcred.Provider{stub}, &stubDefaultAccountResolver{}, nil, nil) + f, _, _, _ := TestFactory(t, nil) + f.Credential = cred + + err := f.RequireBuiltinCredentialProvider(context.Background(), "auth") + prob, ok := errs.ProblemOf(err) + if !ok || prob.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("err = %v, want typed invalid_argument (same as formal arbitration)", err) + } + if strings.Contains(err.Error(), "provided externally") { + t.Fatalf("err = %v, must not read as external takeover", err) + } +} + func TestRequireBuiltinCredentialProvider_NilCredential(t *testing.T) { f, _, _, _ := TestFactory(t, nil) f.Credential = nil diff --git a/internal/core/config.go b/internal/core/config.go index 9a96065938..2d4817473f 100644 --- a/internal/core/config.go +++ b/internal/core/config.go @@ -255,7 +255,11 @@ func ResolveConfigFromMulti(raw *MultiAppConfig, kc keychain.KeychainAccess, pro } if err := ValidateSecretKeyMatch(app.AppId, app.AppSecret); err != nil { - return nil, errs.NewConfigError(errs.SubtypeNotConfigured, "appId and appSecret keychain key are out of sync"). + // invalid_config, not not_configured: the config exists but is + // internally inconsistent. not_configured would let callers degrade + // this into a generic "secret invalid" answer and destroy the precise + // repair hint (which names the expected keychain key — never a value). + return nil, errs.NewConfigError(errs.SubtypeInvalidConfig, "appId and appSecret keychain key are out of sync"). WithHint("%s", err.Error()). WithCause(err) } diff --git a/internal/core/notconfigured.go b/internal/core/notconfigured.go index 068ea86cfa..5ac3ee3bd6 100644 --- a/internal/core/notconfigured.go +++ b/internal/core/notconfigured.go @@ -36,16 +36,13 @@ func LoadOrNotConfigured() (*MultiAppConfig, error) { if errors.Is(err, os.ErrNotExist) { return nil, NotConfiguredError() } - // Surface the real cause (parse error, permission denied, etc.) - // so the user can fix the broken file. A malformed file is - // invalid_config; anything else (permission denied, etc.) is - // not_configured. Both stay on the typed structured-envelope path - // at the root command's error sink. - subtype := errs.SubtypeNotConfigured - if isMalformedConfigError(err) { - subtype = errs.SubtypeInvalidConfig - } - return nil, errs.NewConfigError(subtype, "failed to load config: %v", err).WithCause(err) + // Surface the real cause so the user can fix the broken file. Every + // non-ENOENT load failure — malformed JSON, permission denied, I/O + // error — means a config EXISTS but cannot be used: invalid_config. + // Only a genuinely absent config is not_configured; anything else + // classified as not_configured would let callers degrade it into + // profile_not_found / no_active_profile and hide the real cause. + return nil, errs.NewConfigError(errs.SubtypeInvalidConfig, "failed to load config: %v", err).WithCause(err) } if multi == nil || len(multi.Apps) == 0 { return nil, NotConfiguredError() diff --git a/internal/credential/authsidecar_contract_test.go b/internal/credential/authsidecar_contract_test.go new file mode 100644 index 0000000000..d096b44bc0 --- /dev/null +++ b/internal/credential/authsidecar_contract_test.go @@ -0,0 +1,154 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +package credential_test + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + extcred "github.com/larksuite/cli/extension/credential" + sidecarprovider "github.com/larksuite/cli/extension/credential/sidecar" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/sidecar" +) + +func newRealSidecarCredentialProvider(t *testing.T) *credential.CredentialProvider { + t.Helper() + t.Setenv(envvars.CliAuthProxy, "http://127.0.0.1:16384") + t.Setenv(envvars.CliProxyKey, "test-key") + t.Setenv(envvars.CliAppID, "cli_sidecar") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(envvars.CliDefaultAs, "") + t.Setenv(envvars.CliStrictMode, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + return credential.NewCredentialProvider( + []extcred.Provider{&sidecarprovider.Provider{}}, + nil, + nil, + nil, + ) +} + +func TestAuthSidecarInvalidPolicyUsesValidationContract(t *testing.T) { + for _, tt := range []struct { + name string + key string + }{ + {name: "default as", key: envvars.CliDefaultAs}, + {name: "strict mode", key: envvars.CliStrictMode}, + } { + t.Run(tt.name, func(t *testing.T) { + cp := newRealSidecarCredentialProvider(t) + t.Setenv(tt.key, "banana") + + _, err := cp.ResolveAccount(context.Background()) + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error = %T %v, want typed validation error", err, err) + } + if problem.Category != errs.CategoryValidation || problem.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("problem = %s/%s, want %s/%s", problem.Category, problem.Subtype, errs.CategoryValidation, errs.SubtypeInvalidArgument) + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want ValidationError", err, err) + } + if validationErr.Param != tt.key { + t.Fatalf("param = %q, want %q", validationErr.Param, tt.key) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) + } + if !strings.Contains(problem.Hint, tt.key) { + t.Fatalf("hint = %q, want variable name %s", problem.Hint, tt.key) + } + var blockErr *extcred.BlockError + if !errors.As(err, &blockErr) || + blockErr.Code != extcred.BlockReasonInvalidPolicy || + blockErr.Param != tt.key { + t.Fatalf("cause = %T %v, want classified BlockError for %s", err, err, tt.key) + } + }) + } +} + +func TestAuthSidecarGateProbeUsesValidationContract(t *testing.T) { + cp := newRealSidecarCredentialProvider(t) + t.Setenv(envvars.CliStrictMode, "banana") + + name, err := cp.ActiveExtensionProviderName(context.Background()) + if name != "" { + t.Fatalf("provider name = %q, want empty on invalid policy", name) + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error = %T %v, want typed validation error", err, err) + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want ValidationError", err, err) + } + if problem.Category != errs.CategoryValidation || + problem.Subtype != errs.SubtypeInvalidArgument || + validationErr.Param != envvars.CliStrictMode { + t.Fatalf("problem = %+v param = %q, want validation/invalid_argument param %s", problem, validationErr.Param, envvars.CliStrictMode) + } +} + +func TestAuthSidecarTokenHonorsSelectedAppID(t *testing.T) { + t.Run("matching app returns sentinel", func(t *testing.T) { + cp := newRealSidecarCredentialProvider(t) + + result, err := cp.ResolveToken(context.Background(), credential.TokenSpec{ + Type: credential.TokenTypeUAT, + AppID: "cli_sidecar", + }) + if err != nil { + t.Fatalf("ResolveToken: %v", err) + } + if result == nil || result.Token != sidecar.SentinelUAT { + t.Fatalf("result = %+v, want sidecar UAT sentinel", result) + } + }) + + for _, tt := range []struct { + name string + appID string + }{ + {name: "empty app id", appID: ""}, + {name: "conflicting app id", appID: "cli_other"}, + } { + t.Run(tt.name, func(t *testing.T) { + cp := newRealSidecarCredentialProvider(t) + + result, err := cp.ResolveToken(context.Background(), credential.TokenSpec{ + Type: credential.TokenTypeUAT, + AppID: tt.appID, + }) + if result != nil { + t.Fatalf("result = %+v, want no sidecar sentinel", result) + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error = %T %v, want typed internal error", err, err) + } + if problem.Category != errs.CategoryInternal || problem.Subtype != errs.SubtypeUnknown { + t.Fatalf("problem = %s/%s, want %s/%s", problem.Category, problem.Subtype, errs.CategoryInternal, errs.SubtypeUnknown) + } + if strings.Contains(err.Error(), sidecar.SentinelUAT) { + t.Fatalf("error leaked sidecar sentinel: %v", err) + } + }) + } +} diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 8f20be11d9..edb1fe83e0 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -9,11 +9,17 @@ import ( "fmt" "io" "net/http" + "os" + "slices" + "strings" "sync" + "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/envvars" ) // DefaultAccountResolver is implemented by the default account provider. @@ -136,10 +142,21 @@ type CredentialProvider struct { httpClient func() (*http.Client, error) warnOut io.Writer + // profile is the active profile (from --profile or LARKSUITE_CLI_PROFILE); + // profileSrc records which of the two supplied it, for the reported + // selection and error attribution. + profile string + profileSrc CredentialSourceKind + accountOnce sync.Once account *Account accountErr error selectedSource credentialSource + // selection is the explainable credential-selection result, populated by + // doResolveAccount under accountOnce. It never carries a secret. + selection IdentitySelection + + enrichOnce sync.Once hintOnce sync.Once hint *IdentityHint @@ -161,49 +178,521 @@ func (p *CredentialProvider) SetWarnOut(warnOut io.Writer) *CredentialProvider { return p } +// WithProfileFromFlag records the --profile flag value as the active profile. +// It governs credential arbitration and the reported selection source. +func (p *CredentialProvider) WithProfileFromFlag(profile string) *CredentialProvider { + p.profile = profile + p.profileSrc = SourceFlagProfile + return p +} + +// WithProfileFromEnv records the LARKSUITE_CLI_PROFILE env fallback as the +// active profile. It governs credential arbitration and the reported +// selection source. +func (p *CredentialProvider) WithProfileFromEnv(profile string) *CredentialProvider { + p.profile = profile + p.profileSrc = SourceEnvProfile + return p +} + // ResolveAccount resolves app credentials. Result is cached after first call. // NOTE: Uses sync.Once — only the context from the first call is used for resolution. // Subsequent calls return the cached result regardless of their context. // This is acceptable for CLI (single invocation per process) but not for long-running servers. func (p *CredentialProvider) ResolveAccount(ctx context.Context) (*Account, error) { + acct, err := p.resolveAccountSelection(ctx) + if err != nil || acct == nil { + return acct, err + } + if _, ok := p.selectedSource.(extensionTokenSource); ok { + p.enrichOnce.Do(func() { + p.enrichOrClearIdentity(ctx, acct, p.selectedSource) + }) + } + return acct, nil +} + +// resolveAccountSelection performs and caches only credential selection. It +// deliberately does not resolve tokens or user_info, so callers can validate +// the selected app before any token work begins. +func (p *CredentialProvider) resolveAccountSelection(ctx context.Context) (*Account, error) { p.accountOnce.Do(func() { p.account, p.accountErr = p.doResolveAccount(ctx) }) return p.account, p.accountErr } +// doResolveAccount arbitrates the credential/App selection in three phases: +// gather all arbitration inputs in a single I/O pass, decide the route with a +// pure function, then execute the remaining I/O for the chosen route. +// +// Resolution order (encoded in decideIdentity): a managed extension provider +// (e.g. sidecar) wins outright; then an explicit profile (--profile / +// LARKSUITE_CLI_PROFILE) arbitrates against the direct env credential +// (matching app_id → profile supplies credential and tokens; mismatch → hard +// conflict; incomplete env without a usable app_id → repair error); then a +// complete direct env credential; then the config default (currentApp → +// firstApp). +// +// It populates p.selection (never carries a secret) and p.selectedSource on +// every success path. func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, error) { + in, err := p.gatherIdentityInputs(ctx) + if err != nil { + return nil, err + } + d, err := decideIdentity(in) + if err != nil { + return nil, err + } + acct, source, err := p.execute(ctx, d, in) + if err != nil { + return nil, err + } + p.selectedSource = source + // Assigned only after full success: error paths can never leave a + // partial selection behind. + p.selection = d.selection + return acct, nil +} + +// providerAccount pairs an extension-provider account with its token source. +type providerAccount struct { + acct *Account + source extensionTokenSource +} + +// identityInputs is one invocation's complete arbitration input, gathered in +// a single pass by gatherIdentityInputs. It is read-only after gathering; +// decideIdentity consumes it without further I/O. +type identityInputs struct { + profile string + profileSrc CredentialSourceKind + + managed *providerAccount // managed extension account; wins arbitration outright + direct *providerAccount // complete direct env credential + // directBlock is a provider's explicit incomplete-direct-credential + // classification (BlockError.Code == credential_incomplete). It + // participates in profile arbitration instead of failing outright. + directBlock *extcred.BlockError + + // directKeys / conflictKeys describe the BUILTIN process-env direct + // credential surface (LARKSUITE_CLI_* variable NAMES, never values). + // They annotate DirectCredentialEnv and conflict hints; a third-party + // AccountDirect provider reports its own inputs via BlockError metadata + // (PresentKeys/AppID), not through these. + directKeys []string + conflictKeys []string + + config *core.MultiAppConfig + configErr error +} + +// gatherIdentityInputs performs the arbitration's read phase: it consults the +// extension providers and snapshots the config. Providers classify their own +// failures at the source (BlockError.Code); this layer must not infer them by +// re-reading environment variables or parsing Reason. +func (p *CredentialProvider) gatherIdentityInputs(ctx context.Context) (identityInputs, error) { + in := identityInputs{ + profile: p.profile, + profileSrc: p.profileSrc, + directKeys: presentDirectCredentialKeys(), + conflictKeys: presentDirectCredentialInputKeys(), + } for _, prov := range p.providers { acct, err := prov.ResolveAccount(ctx) if err != nil { - return nil, err - } - if acct != nil { - internal := convertAccount(acct) - source := extensionTokenSource{provider: prov} - if err := p.enrichUserInfo(ctx, internal, source); err != nil { - if p.warnOut != nil { - _, _ = fmt.Fprintf(p.warnOut, "warning: unable to verify user identity from credential source %q: %v\n", source.Name(), err) + var blockErr *extcred.BlockError + if errors.As(err, &blockErr) { + switch blockErr.Code { + case extcred.BlockReasonCredentialIncomplete: + // app_credential_incomplete, profile matching, and + // DirectCredentialEnv diagnostics are defined in terms of + // the builtin LARKSUITE_CLI_* env surface. Until the SPI + // carries provider-owned input descriptors, accepting this + // classification from another provider would produce + // contradictory arbitration and repair hints. + if _, builtin := prov.(*envprovider.Provider); !builtin { + return in, newCredentialIncompleteProviderContractError(prov) + } + in.directBlock = blockErr + case extcred.BlockReasonInvalidPolicy: + // A user-supplied policy value failed validation; that is + // a validation error, never an internal one. + return in, newInvalidPolicyError(blockErr) + default: + // Blocks without a recognized Code preserve their + // original attribution. + return in, err } - // enrichUserInfo failure is non-fatal: SupportedIdentities - // (used for strict mode) is already set by the provider. - // Clear unverified user identity for safety. - internal.UserOpenId = "" - internal.UserName = "" + break } - p.selectedSource = source - return internal, nil + // Any other provider error preserves its original attribution. + return in, err } + if acct == nil { + continue + } + pa := &providerAccount{acct: convertAccount(acct), source: extensionTokenSource{provider: prov}} + switch acct.Kind { + case extcred.AccountDirect: + // The arbitration's direct-credential surface — DirectCredentialEnv, + // the env:LARKSUITE_CLI_APP_ID selection source, conflict-hint + // keys — is defined in terms of the builtin process-env variables. + // Until the SPI carries provider-reported input descriptors, only + // the builtin env provider may declare AccountDirect; accepting it + // from anyone else would produce self-contradictory diagnostics + // (e.g. credentialSource "env:LARKSUITE_CLI_APP_ID" with + // directCredentialEnv.present=false). The check is by concrete + // type: the registry reserves neither names nor uniqueness, so a + // Name() comparison would be forgeable. + if _, builtin := prov.(*envprovider.Provider); !builtin { + return in, errs.NewInternalError(errs.SubtypeUnknown, + "credential provider %q declared AccountDirect, which is reserved for the builtin env provider", prov.Name()) + } + in.direct = pa + case extcred.AccountManaged: + in.managed = pa + default: + return in, errs.NewInternalError(errs.SubtypeUnknown, + "credential provider %q returned unknown AccountKind %d", prov.Name(), acct.Kind) + } + break // the first engaged provider ends the scan (registry priority order) + } + // The config snapshot backs profile lookup, the config-default route, and + // config-default failure attribution. A winning managed or direct-env + // identity without a profile never needs it — and managed identities must + // keep working when the config is absent or malformed. + if in.managed == nil && (in.profile != "" || in.direct == nil) { + in.config, in.configErr = core.LoadOrNotConfigured() + } + return in, nil +} + +// credentialRoute names which source serves the selected account and tokens. +type credentialRoute int + +const ( + routeManaged credentialRoute = iota + routeProfile + routeDirectEnv + routeConfigDefault +) + +// decision is decideIdentity's complete verdict. Nothing in it touched I/O. +type decision struct { + route credentialRoute + selection IdentitySelection + // profileAppID is set on routeProfile; app_id is plaintext and safe to + // echo in the secret-invalid error. + profileAppID string +} + +// decideIdentity holds every selection rule in one place: precedence +// (managed > profile > direct env > config default), profile/direct-env +// conflict detection, and error attribution. It is pure — same inputs, same +// verdict — so the full selection matrix is table-testable without env vars +// or config fixtures. +func decideIdentity(in identityInputs) (decision, error) { + // DirectCredentialEnv reports the direct env vars truthfully on every + // route: Present always means "direct credential env vars are set". + directEnv := DirectCredentialEnv{Present: len(in.directKeys) > 0, Keys: in.directKeys} + if in.direct != nil { + directEnv.AppID = in.direct.acct.AppID + } + switch { + case in.managed != nil: + return decision{route: routeManaged, selection: IdentitySelection{ + Source: SourceExtension(in.managed.source.Name()), + DirectCredentialEnv: directEnv, + }}, nil + case in.profile != "": + return decideProfile(in, directEnv) + case in.directBlock != nil: + return decision{}, newAppCredentialIncompleteError(in.directBlock, false) + case in.direct != nil: + return decision{route: routeDirectEnv, selection: IdentitySelection{ + Source: SourceEnvAppID, + DirectCredentialEnv: directEnv, + }}, nil + default: + return decision{route: routeConfigDefault, selection: IdentitySelection{ + Source: selectionSourceForDefault(in.config), + DirectCredentialEnv: directEnv, + }}, nil } - if p.defaultAcct != nil { +} + +// decideProfile arbitrates an explicit profile against the direct env +// credential state. +func decideProfile(in identityInputs, directEnv DirectCredentialEnv) (decision, error) { + app, err := findProfile(in) + if err != nil { + return decision{}, err + } + if in.directBlock != nil { + // APP_ID-only is sufficient to compare sources: a matching selected + // profile supplies the credential and tokens; a mismatch is the same + // hard conflict as a complete direct env. Anything less than a usable + // app_id keeps the provider's repair error, extended with the + // unset-to-use-the-profile path. + if in.directBlock.AppID == "" || !slices.Contains(in.directBlock.PresentKeys, envvars.CliAppID) { + return decision{}, newAppCredentialIncompleteError(in.directBlock, true) + } + if app.AppId != in.directBlock.AppID { + return decision{}, newProfileAppCredentialConflict( + in.profile, app.AppId, in.directBlock.AppID, in.directBlock.PresentKeys) + } + directEnv.AppID = in.directBlock.AppID + directEnv.Matched = true + } + if in.direct != nil { + // E == complete: the direct env app_id must match the profile. + if app.AppId != in.direct.acct.AppID { + return decision{}, newProfileAppCredentialConflict( + in.profile, app.AppId, in.direct.acct.AppID, in.conflictKeys) + } + directEnv.Matched = true + } + return decision{ + route: routeProfile, + selection: IdentitySelection{Source: in.profileSrc, DirectCredentialEnv: directEnv}, + profileAppID: app.AppId, + }, nil +} + +// findProfile resolves the requested profile against the config snapshot. +// A malformed config must surface its real typed cause (invalid_config): +// reporting it as profile_not_found would send the user to `profile list` +// and hide the broken file. Only a genuinely absent config degrades to +// profile_not_found, because the profile then cannot exist anywhere. Both +// deliberately outrank an incomplete direct env: fixing the profile side is +// what makes the selected profile usable. +func findProfile(in identityInputs) (*core.AppConfig, error) { + if in.configErr != nil { + if prob, ok := errs.ProblemOf(in.configErr); !ok || prob.Subtype != errs.SubtypeNotConfigured { + return nil, in.configErr + } + } + if in.config != nil { + if app := in.config.FindApp(in.profile); app != nil { + return app, nil + } + } + return nil, errs.NewConfigError(errs.SubtypeProfileNotFound, + "profile %q not found", in.profile). + WithProfile(in.profile). + WithCredentialSource(string(in.profileSrc)). + WithHint("run `lark-cli profile list` to see available profiles.") +} + +// execute performs the remaining I/O for the decided route and returns the +// account together with its token source. +func (p *CredentialProvider) execute(ctx context.Context, d decision, in identityInputs) (*Account, credentialSource, error) { + switch d.route { + case routeManaged: + return in.managed.acct, in.managed.source, nil + case routeDirectEnv: + return in.direct.acct, in.direct.source, nil + case routeProfile: + // Resolve the profile's own (keychain-backed) credential locally. acct, err := p.defaultAcct.ResolveAccount(ctx) if err != nil { - return nil, err + // A typed failure other than not_configured carries its own + // precise, secret-free diagnosis (typed errors never embed secret + // material per the error contract) — pass it through instead of + // flattening it into the generic secret error. Untyped failures + // and a config that vanished mid-resolution stay masked: their + // content is not guaranteed secret-free. + if prob, ok := errs.ProblemOf(err); ok && prob.Subtype != errs.SubtypeNotConfigured { + return nil, nil, err + } + return nil, nil, newProfileSecretInvalidError(in.profile, d.profileAppID) } - p.selectedSource = defaultTokenSource{resolver: p.defaultToken} - return acct, nil + // The resolver re-reads the config; a concurrent profile edit between + // gather and here could hand back a different app. Refuse the mismatch + // instead of silently using credentials the arbitration never checked. + if acct.AppID != d.profileAppID { + return nil, nil, errs.NewInternalError(errs.SubtypeUnknown, + "config changed during resolution: profile %q resolved to a different app", in.profile). + WithHint("retry the command.") + } + return acct, defaultTokenSource{resolver: p.defaultToken}, nil + default: // routeConfigDefault + if p.defaultAcct == nil { + return nil, nil, core.NotConfiguredError() + } + acct, err := p.defaultAcct.ResolveAccount(ctx) + if err != nil { + return nil, nil, translateConfigDefaultFailure(err, in.config) + } + return acct, defaultTokenSource{resolver: p.defaultToken}, nil + } +} + +// translateConfigDefaultFailure attributes a config-default failure from the +// snapshot: a default profile that EXISTS (has an app_id) but whose secret +// cannot be resolved locally is profile_secret_invalid — "identity is +// configured, its secret is broken" is more actionable than "no active +// profile". Only when there is genuinely no usable default profile do we +// report no_active_profile. Other typed failures pass through unchanged. +func translateConfigDefaultFailure(err error, multi *core.MultiAppConfig) error { + if prob, ok := errs.ProblemOf(err); !ok || prob.Subtype != errs.SubtypeNotConfigured { + return err + } + if multi != nil { + if app := multi.CurrentAppConfig(""); app != nil && app.AppId != "" { + return newProfileSecretInvalidError(app.ProfileName(), app.AppId) + } + } + return errs.NewConfigError(errs.SubtypeNoActiveProfile, "no active profile"). + WithCredentialSource(noActiveProfileCredentialSource). + WithHint("run `lark-cli config init` / `lark-cli profile add`, or set %s.", envvars.CliProfile) +} + +func newProfileAppCredentialConflict(profile, profileAppID, envAppID string, presentKeys []string) error { + err := errs.NewValidationError(errs.SubtypeProfileAppCredentialConflict, + "profile %q app_id does not match %s", profile, envvars.CliAppID). + WithProfileAppConflict(profileAppID, envAppID) + if len(presentKeys) > 0 { + return err.WithHint("unset %s, or select a profile whose app_id matches the environment.", + humanList(presentKeys, "and")) + } + return err.WithHint("unset the direct credential environment variables, or select a profile whose app_id matches the environment.") +} + +func newAppCredentialIncompleteError(blockErr *extcred.BlockError, selectedProfileAvailable bool) *errs.ConfigError { + err := errs.NewConfigError(errs.SubtypeAppCredentialIncomplete, "%s", blockErr.Reason). + WithCause(blockErr) + if len(blockErr.MissingKeys) > 0 { + err.WithMissingKeys(blockErr.MissingKeys...) + } + if len(blockErr.RequiredAnyOf) > 0 { + err.WithRequiredAnyOf(blockErr.RequiredAnyOf...) + } + + hint := credentialRepairHint(blockErr) + if selectedProfileAvailable && len(blockErr.PresentKeys) > 0 { + hint += fmt.Sprintf(", or unset %s to use the selected profile", humanList(blockErr.PresentKeys, "and")) + } + return err.WithHint("%s.", hint) +} + +func credentialRepairHint(blockErr *extcred.BlockError) string { + if len(blockErr.RequiredAnyOf) > 0 { + return "set " + humanList(blockErr.RequiredAnyOf, "or") + } + return "set " + humanList(blockErr.MissingKeys, "and") +} + +func humanList(items []string, conjunction string) string { + switch len(items) { + case 0: + return "the missing direct credential variables" + case 1: + return items[0] + case 2: + return items[0] + " " + conjunction + " " + items[1] + default: + return strings.Join(items[:len(items)-1], ", ") + ", " + conjunction + " " + items[len(items)-1] + } +} + +// newInvalidPolicyError translates a provider's invalid-policy block into the +// typed validation contract: the failed variable name travels in param, the +// repair path in the hint, and the original block stays on the cause chain. +// Reason carries only the variable name and its non-secret value. +func newInvalidPolicyError(blockErr *extcred.BlockError) error { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", blockErr.Reason). + WithParam(blockErr.Param). + WithCause(blockErr). + WithHint("set %s to a supported value or unset it.", blockErr.Param) +} + +func newCredentialIncompleteProviderContractError(prov extcred.Provider) error { + return errs.NewInternalError(errs.SubtypeUnknown, + "credential provider %q returned credential_incomplete, which is reserved for the builtin env provider", prov.Name()) +} + +// newProfileSecretInvalidError is deliberately generic (SECURITY): the +// underlying cause may carry secret material, so neither it nor its message +// may reach the envelope. app_id is plaintext and safe to echo. +func newProfileSecretInvalidError(profile, appID string) error { + return errs.NewConfigError(errs.SubtypeProfileSecretInvalid, + "profile %q credential could not be resolved locally", profile). + WithProfile(profile). + WithAppID(appID). + WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") +} + +// enrichOrClearIdentity verifies a provider-supplied user identity via +// enrichUserInfo. Verification failure is non-fatal — SupportedIdentities +// (used for strict mode) is already set by the provider — but an unverified +// identity must not survive it: a stale OpenID would attribute calls to a +// user the token can no longer act for. +func (p *CredentialProvider) enrichOrClearIdentity(ctx context.Context, acct *Account, source credentialSource) { + err := p.enrichUserInfo(ctx, acct, source) + if err == nil { + return + } + if p.warnOut != nil { + _, _ = fmt.Fprintf(p.warnOut, "warning: unable to verify user identity from credential source %q: %v\n", source.Name(), err) + } + acct.UserOpenId = "" + acct.UserName = "" +} + +// noActiveProfileCredentialSource is the credential_source reported on the +// no_active_profile error. The error contract fixes this to the literal "config": there is +// no resolved default profile at all, so the more specific config:currentApp / +// config:firstApp source values (used on successful config-default selections) +// would be misleading. It is an enum string, never a secret. +const noActiveProfileCredentialSource = "config" + +// selectionSourceForDefault reports whether the config default resolved to the +// explicit currentApp or fell back to the first app. +func selectionSourceForDefault(multi *core.MultiAppConfig) CredentialSourceKind { + if multi != nil && multi.CurrentApp != "" { + return SourceConfigCurrentApp } - return nil, core.NotConfiguredError() + return SourceConfigFirstApp +} + +// presentDirectCredentialKeys returns the NAMES (never values) of the direct +// app credential env vars that are set. Used to annotate DirectCredentialEnv. +func presentDirectCredentialKeys() []string { + var keys []string + if os.Getenv(envvars.CliAppID) != "" { + keys = append(keys, envvars.CliAppID) + } + if os.Getenv(envvars.CliAppSecret) != "" { + keys = append(keys, envvars.CliAppSecret) + } + return keys +} + +// presentDirectCredentialInputKeys returns all direct env input names that +// must be cleared together to remove a profile/app_id conflict. Values are +// never returned. +func presentDirectCredentialInputKeys() []string { + keys := presentDirectCredentialKeys() + if os.Getenv(envvars.CliUserAccessToken) != "" { + keys = append(keys, envvars.CliUserAccessToken) + } + if os.Getenv(envvars.CliTenantAccessToken) != "" { + keys = append(keys, envvars.CliTenantAccessToken) + } + return keys +} + +// Selection resolves the account (once) and returns the cached, secret-free +// explanation of how the credential/App was selected. It mirrors +// selectedCredentialSource: resolve-then-return. +func (p *CredentialProvider) Selection(ctx context.Context) (IdentitySelection, error) { + if _, err := p.ResolveAccount(ctx); err != nil { + return IdentitySelection{}, err + } + return p.selection, nil } // enrichUserInfo resolves user identity when extension provides a UAT. @@ -239,17 +728,13 @@ func (p *CredentialProvider) enrichUserInfo(ctx context.Context, acct *Account, } func (p *CredentialProvider) selectedCredentialSource(ctx context.Context) (credentialSource, error) { - if p.selectedSource != nil { - return p.selectedSource, nil - } - if p.defaultAcct == nil { - return nil, nil - } - if _, err := p.ResolveAccount(ctx); err != nil { + if _, err := p.resolveAccountSelection(ctx); err != nil { return nil, err } if p.selectedSource == nil { - return nil, fmt.Errorf("credential provider resolved an account without selecting a token source") + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "credential provider resolved an account without selecting a token source"). + WithHint("retry the command.") } return p.selectedSource, nil } @@ -302,51 +787,88 @@ func (p *CredentialProvider) doResolveIdentityHint(ctx context.Context) (*Identi // ResolveToken resolves an access token. func (p *CredentialProvider) ResolveToken(ctx context.Context, req TokenSpec) (*TokenResult, error) { - source, err := p.selectedCredentialSource(ctx) + acct, err := p.resolveAccountSelection(ctx) if err != nil { return nil, err } - if source != nil { - return resolveTokenFromSource(ctx, source, req) + if acct == nil { + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "credential provider resolved no account before %s token resolution", req.Type). + WithHint("retry the command.") } - - for _, prov := range p.providers { - source := extensionTokenSource{provider: prov} - result, found, err := source.TryResolveToken(ctx, req) - if err != nil { - return nil, err - } - if found { - return result, nil - } + source := p.selectedSource + if source == nil { + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "credential provider resolved app %q without selecting a token source", acct.AppID). + WithHint("retry the command.") } - source = defaultTokenSource{resolver: p.defaultToken} - result, found, err := source.TryResolveToken(ctx, req) - if err != nil { - return nil, err + if req.AppID == "" { + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "TokenSpec.AppID is required for %s token resolution", req.Type). + WithHint("retry the command.") } - if found { - return result, nil + if req.AppID != acct.AppID { + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "token requested for app %q but the selected account belongs to app %q", req.AppID, acct.AppID). + WithHint("retry the command.") } - return nil, &TokenUnavailableError{Type: req.Type} + return resolveTokenFromSource(ctx, source, req) } // ActiveExtensionProviderName reports whether an extension provider is managing -// credentials. It probes p.providers (extension providers only, not defaultAcct) -// and returns the name of the first engaged provider. +// the credentials that actually win selection. With an explicit profile that +// resolves successfully it reuses ResolveAccount's cached arbitration result; +// otherwise it probes extension providers directly and returns the first +// engaged provider. // // "Engaged" means: ResolveAccount returns a non-nil account, OR returns a // *extcred.BlockError (provider configured but misconfigured — still counts as -// external). Any other error is propagated to the caller. +// external). Any other probe error is propagated to the caller. +// +// A failed profile resolution (profile not found, broken secret, malformed +// config, incomplete direct env, ...) deliberately does NOT propagate: this +// probe guards the builtin setup/repair commands (auth, config), and an +// unresolvable credential must never lock the user out of the commands that +// fix it. It falls back to the engagement probe, which answers the only +// question this function owns: is an extension provider holding credentials? // // Returns ("", nil) when no extension provider is active (built-in keychain path). -// Safe to call multiple times — probes providers directly without the sync.Once cache. +// Safe to call multiple times: explicit-profile resolution uses sync.Once, while +// the probe path only consults providers. func (p *CredentialProvider) ActiveExtensionProviderName(ctx context.Context) (string, error) { + // With an explicit profile, report the source that actually won the same + // arbitration used by commands. A matching APP_ID-only env block is not an + // external takeover once the selected profile supplies credentials/tokens. + if p.profile != "" { + if _, err := p.ResolveAccount(ctx); err == nil { + if p.selectedSource == nil { + return "", nil + } + if _, builtin := p.selectedSource.(defaultTokenSource); builtin { + return "", nil + } + return p.selectedSource.Name(), nil + } + // Resolution failed — fall through to the engagement probe. + } for _, prov := range p.providers { acct, err := prov.ResolveAccount(ctx) if err != nil { var blockErr *extcred.BlockError if errors.As(err, &blockErr) { + // Align with formal arbitration: a misconfigured policy + // variable is the same typed validation error everywhere — + // not an external takeover of the provider that reported it, + // and not license to keep scanning and blame a later + // provider instead. + if blockErr.Code == extcred.BlockReasonInvalidPolicy { + return "", newInvalidPolicyError(blockErr) + } + if blockErr.Code == extcred.BlockReasonCredentialIncomplete { + if _, builtin := prov.(*envprovider.Provider); !builtin { + return "", newCredentialIncompleteProviderContractError(prov) + } + } name := blockErr.Provider if name == "" { name = prov.Name() diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go new file mode 100644 index 0000000000..48641cc7bd --- /dev/null +++ b/internal/credential/credential_provider_selection_test.go @@ -0,0 +1,1124 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package credential_test + +import ( + "context" + "errors" + "fmt" + "os" + "runtime" + "slices" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/internal/keychain" + "github.com/larksuite/cli/internal/output" +) + +func asConfigError(t *testing.T, err error) *errs.ConfigError { + t.Helper() + var ce *errs.ConfigError + if !errors.As(err, &ce) { + t.Fatalf("expected *errs.ConfigError, got %T: %v", err, err) + } + return ce +} + +func asValidationError(t *testing.T, err error) *errs.ValidationError { + t.Helper() + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + return ve +} + +// secretValue is the profile secret written to config. It must NEVER appear in +// any error message or IdentitySelection (security: never leak a secret). +const secretValue = "your-secret" + +// envSecretValue is the direct env app secret. Same no-leak guarantee. +const envSecretValue = "your-password" + +// writeConfigTenantA writes a config with a single profile "tenant_a" (app_id +// "cli_a"). The secret is a plaintext secret stored in config, which resolves +// locally without a keychain lookup. +func writeConfigTenantA(t *testing.T) { + t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + multi := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.PlainSecret(secretValue), + Brand: core.BrandFeishu, + }}, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } +} + +// writeConfigTenantABroken writes tenant_a with a keychain-backed secret ref +// that cannot be resolved (noop keychain returns empty), so profile secret +// resolution fails locally. +func writeConfigTenantABroken(t *testing.T) { + t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + // A keychain SecretRef whose key does NOT match app_id cli_a. Local secret + // resolution fails (ValidateSecretKeyMatch), exercising profile_secret_invalid. + multi := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.SecretInput{Ref: &core.SecretRef{Source: "keychain", ID: "appsecret:wrong_key"}}, + Brand: core.BrandFeishu, + }}, + } + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } +} + +func newProvider(t *testing.T, profile string, fromFlag bool) *credential.CredentialProvider { + t.Helper() + ep := &envprovider.Provider{} + defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return &noopKC{} }, profile) + cp := credential.NewCredentialProvider([]extcred.Provider{ep}, defaultAcct, nil, nil) + if fromFlag { + cp.WithProfileFromFlag(profile) + } else { + cp.WithProfileFromEnv(profile) + } + return cp +} + +// assertNoSecretLeak fails if any secret value appears in the given strings. +func assertNoSecretLeak(t *testing.T, where string, vals ...string) { + t.Helper() + for _, v := range vals { + if v == "" { + continue + } + if strings.Contains(v, secretValue) { + t.Errorf("%s leaked profile secret: %q", where, v) + } + if strings.Contains(v, envSecretValue) { + t.Errorf("%s leaked env secret: %q", where, v) + } + } +} + +func subtypeOf(t *testing.T, err error) errs.Subtype { + t.Helper() + if err == nil { + t.Fatalf("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error is not a typed problem: %v", err) + } + return p.Subtype +} + +// With no profile, direct credential, or config default, selection reports no_active_profile. +func TestSelection_NoActiveProfile(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) // empty dir -> no config + cp := newProvider(t, "", false) + + sel, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeNoActiveProfile { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeNoActiveProfile) + } + // no_active_profile must carry credential_source=config. + ce := asConfigError(t, err) + if ce.CredentialSource != "config" { + t.Errorf("credential_source = %q, want %q", ce.CredentialSource, "config") + } + assertNoSecretLeak(t, "state2", err.Error(), string(sel.Source)) +} + +// Config-default profile with an out-of-sync keychain ref: the precise typed +// cause (invalid_config) passes through on the default route too — parity +// with main, which surfaced this exact diagnosis before the profile feature. +func TestSelection_ConfigDefaultKeychainOutOfSyncSurfacesRealCause(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantABroken(t) // CurrentApp = tenant_a (app_id cli_a), out-of-sync keychain ref + cp := newProvider(t, "", false) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeInvalidConfig { + t.Fatalf("subtype = %q, want invalid_config (precise cause, not no_active_profile)", got) + } + prob, _ := errs.ProblemOf(err) + if !strings.Contains(prob.Message, "out of sync") { + t.Errorf("message = %q, want the out-of-sync diagnosis", prob.Message) + } + if !strings.Contains(prob.Hint, "appsecret:cli_a") { + t.Errorf("hint = %q, want the expected keychain key named", prob.Hint) + } + assertNoSecretLeak(t, "config-default-out-of-sync", prob.Message, prob.Hint) +} + +// writeMalformedConfig points the config dir at a temp file containing invalid +// JSON, with no direct-credential env set. +func writeMalformedConfig(t *testing.T) { + t.Helper() + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := os.MkdirAll(core.GetConfigDir(), 0o700); err != nil { + t.Fatalf("mkdir config dir: %v", err) + } + if err := os.WriteFile(core.GetConfigPath(), []byte("{ this is not valid json"), 0o600); err != nil { + t.Fatalf("write malformed config: %v", err) + } +} + +// assertMalformedSurfaced requires the malformed config to surface as the typed +// invalid_config subtype — never masked (as profile_not_found on the explicit +// path, or no_active_profile on the config-default path), which would hide a +// broken file and misdirect the user (e.g. to `config init`, risking overwrite +// of a recoverable config). +func assertMalformedSurfaced(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatalf("expected error for malformed config, got nil") + } + prob, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("malformed config error is not typed: %v", err) + } + if prob.Subtype != errs.SubtypeInvalidConfig { + t.Fatalf("malformed config subtype = %q, want invalid_config (not masked)", prob.Subtype) + } +} + +// Explicit profile + malformed config → invalid_config, not profile_not_found. +func TestSelection_ExplicitProfile_MalformedConfig_PropagatesError(t *testing.T) { + writeMalformedConfig(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + assertMalformedSurfaced(t, err) +} + +// Config-default path (no profile) + malformed config → invalid_config, not +// no_active_profile. Both paths route through core.LoadOrNotConfigured so a +// temporarily broken config is never misreported as "no active profile, run +// config init". +func TestSelection_ConfigDefault_MalformedConfig_PropagatesError(t *testing.T) { + writeMalformedConfig(t) + cp := newProvider(t, "", false) + + _, err := cp.Selection(context.Background()) + assertMalformedSurfaced(t, err) +} + +// APP_ID without a secret or access token reports app_credential_incomplete. +func TestSelection_AppIDOnlyWithoutProfile(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_env") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantA(t) + cp := newProvider(t, "", false) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeAppCredentialIncomplete { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeAppCredentialIncomplete) + } + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Fatalf("exit code = %d, want %d", got, output.ExitAuth) + } + prob, _ := errs.ProblemOf(err) + ce := asConfigError(t, err) + if len(ce.MissingKeys) != 0 { + t.Errorf("missing_keys = %v, want empty because no single key is required", ce.MissingKeys) + } + wantAnyOf := []string{envvars.CliAppSecret, envvars.CliUserAccessToken, envvars.CliTenantAccessToken} + if !slices.Equal(ce.RequiredAnyOf, wantAnyOf) { + t.Errorf("required_any_of = %v, want %v", ce.RequiredAnyOf, wantAnyOf) + } + // required_any_of must be NAMES only, never values. + for _, k := range ce.RequiredAnyOf { + if strings.Contains(k, envSecretValue) || strings.Contains(k, secretValue) { + t.Errorf("required_any_of contains a value, not a name: %q", k) + } + } + wantHint := "set LARKSUITE_CLI_APP_SECRET, LARKSUITE_CLI_USER_ACCESS_TOKEN, or LARKSUITE_CLI_TENANT_ACCESS_TOKEN." + if ce.Hint != wantHint { + t.Errorf("hint = %q, want %q", ce.Hint, wantHint) + } + var blockErr *extcred.BlockError + if !errors.As(err, &blockErr) || blockErr.Code != extcred.BlockReasonCredentialIncomplete { + t.Fatalf("cause chain does not preserve credential BlockError: %T %v", err, err) + } + assertNoSecretLeak(t, "state3", prob.Message, prob.Hint) +} + +func TestSelection_IncompleteDirectEnvWithoutProfile_UsesDirectRepairOnly(t *testing.T) { + for _, tt := range []struct { + name string + key string + }{ + {name: "APP_SECRET-only", key: envvars.CliAppSecret}, + {name: "UAT-only", key: envvars.CliUserAccessToken}, + {name: "TAT-only", key: envvars.CliTenantAccessToken}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.key, "direct-value") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + cp := newProvider(t, "", false) + _, err := cp.Selection(context.Background()) + ce := asConfigError(t, err) + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Fatalf("exit code = %d, want %d", got, output.ExitAuth) + } + if ce.Message != tt.key+" is set but "+envvars.CliAppID+" is missing" { + t.Errorf("message = %q, want provider's exact reason", ce.Message) + } + if ce.Hint != "set "+envvars.CliAppID+"." { + t.Errorf("hint = %q, want direct repair only", ce.Hint) + } + if !slices.Equal(ce.MissingKeys, []string{envvars.CliAppID}) { + t.Errorf("missing_keys = %v, want [%s]", ce.MissingKeys, envvars.CliAppID) + } + }) + } +} + +// A complete direct credential without a profile is selected from APP_ID env. +func TestSelection_CompleteDirectEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_env") + t.Setenv(envvars.CliAppSecret, envSecretValue) + writeConfigTenantA(t) + cp := newProvider(t, "", false) + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sel.Source != credential.SourceEnvAppID { + t.Fatalf("source = %q, want %q", sel.Source, credential.SourceEnvAppID) + } + if !sel.DirectCredentialEnv.Present { + t.Errorf("DirectCredentialEnv.Present = false, want true") + } + assertNoSecretLeak(t, "state4", string(sel.Source), sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state4-keys", sel.DirectCredentialEnv.Keys...) +} + +// A valid profile selected by flag reports flag:--profile as its source. +func TestSelection_ProfileOnlyFromFlag(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sel.Source != credential.SourceFlagProfile { + t.Fatalf("source = %q, want %q", sel.Source, credential.SourceFlagProfile) + } + if sel.DirectCredentialEnv.Present { + t.Errorf("DirectCredentialEnv.Present = true, want false") + } + assertNoSecretLeak(t, "state5", string(sel.Source)) +} + +// A valid profile selected by env reports LARKSUITE_CLI_PROFILE as its source. +func TestSelection_ProfileOnlyFromEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", false) + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sel.Source != credential.SourceEnvProfile { + t.Fatalf("source = %q, want %q", sel.Source, credential.SourceEnvProfile) + } +} + +// An explicitly selected missing profile reports profile_not_found even when direct env is complete. +func TestSelection_MissingProfileWinsOverDirectEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_env") + t.Setenv(envvars.CliAppSecret, envSecretValue) + writeConfigTenantA(t) + cp := newProvider(t, "does_not_exist", true) + + sel, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileNotFound { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileNotFound) + } + prob, _ := errs.ProblemOf(err) + // profile_not_found must carry the credential_source that + // named the profile — here the --profile flag. + ce := asConfigError(t, err) + if ce.CredentialSource != string(credential.SourceFlagProfile) { + t.Errorf("credential_source = %q, want %q", ce.CredentialSource, credential.SourceFlagProfile) + } + assertNoSecretLeak(t, "state6", err.Error(), prob.Hint, string(sel.Source)) +} + +// REAL-path regression for review F1: a valid profile whose saved keychain +// ref is inconsistent with its app_id must surface the precise typed cause — +// invalid_config, "out of sync", hint naming the expected keychain key — via +// the actual DefaultAccountProvider, not be flattened into the generic +// profile_secret_invalid (which is reserved for untyped failures). +func TestSelection_ProfileKeychainOutOfSyncSurfacesRealCause(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantABroken(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeInvalidConfig { + t.Fatalf("subtype = %q, want invalid_config (precise cause, not the generic secret error)", got) + } + prob, _ := errs.ProblemOf(err) + if !strings.Contains(prob.Message, "out of sync") { + t.Errorf("message = %q, want the out-of-sync diagnosis", prob.Message) + } + if !strings.Contains(prob.Hint, "appsecret:cli_a") { + t.Errorf("hint = %q, want the expected keychain key named", prob.Hint) + } + assertNoSecretLeak(t, "keychain-out-of-sync", prob.Message, prob.Hint) +} + +// secretMarkerValue is a distinctive string used to prove that the +// profile_secret_invalid path drops the underlying error entirely, even when +// that underlying error's own message CONTAINS a secret. Unlike +// writeConfigTenantABroken (whose noop-keychain failure is a harmless empty +// error), this uses a custom DefaultAccountResolver whose error text embeds +// the marker, closing the gap where a leak could hide in a cause chain that +// happens to be empty in the noop-keychain case. +const secretMarkerValue = "your-access-token" + +// leakingSecretResolver is a DefaultAccountResolver stub whose ResolveAccount +// fails with an error whose message contains secretMarkerValue, simulating a +// real keychain/secret-resolution failure that echoes back sensitive material +// (e.g. a keychain library including the attempted secret in its error text). +type leakingSecretResolver struct{} + +func (leakingSecretResolver) ResolveAccount(ctx context.Context) (*credential.Account, error) { + return nil, fmt.Errorf("keychain decode failed for secret %s", secretMarkerValue) +} + +// When account/secret resolution fails with an error that itself contains a +// secret, doResolveAccount emits a generic +// profile_secret_invalid ConfigError WITHOUT attaching the underlying cause, +// so a secret embedded in that underlying error can never surface through +// err.Error(), Message, Hint, the unwrapped cause chain, or Selection(). +func TestSelection_ProfileSecretErrorDoesNotLeak(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantA(t) // profile "tenant_a" exists with app_id "cli_a" + + ep := &envprovider.Provider{} + cp := credential.NewCredentialProvider([]extcred.Provider{ep}, leakingSecretResolver{}, nil, nil) + cp.WithProfileFromFlag("tenant_a") + + sel, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileSecretInvalid { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileSecretInvalid) + } + ce := asConfigError(t, err) + if ce.Profile != "tenant_a" { + t.Errorf("profile = %q, want tenant_a", ce.Profile) + } + if ce.AppID != "cli_a" { + t.Errorf("app_id = %q, want cli_a", ce.AppID) + } + + // Walk the full unwrap chain. This is the assertion that would catch a + // regression where the profile_secret_invalid branch starts attaching the + // underlying error via WithCause: if it did, this loop would find the + // marker in a wrapped link even though err.Error()/Message/Hint (which + // only reflect the top-level ConfigError, not the chain) might look clean. + for cur := error(ce); cur != nil; cur = errors.Unwrap(cur) { + if strings.Contains(cur.Error(), secretMarkerValue) { + t.Errorf("cause chain leaked secret marker: %v", cur) + } + } + + if strings.Contains(err.Error(), secretMarkerValue) { + t.Errorf("err.Error() leaked secret marker: %q", err.Error()) + } + if strings.Contains(ce.Message, secretMarkerValue) { + t.Errorf("Message leaked secret marker: %q", ce.Message) + } + if strings.Contains(ce.Hint, secretMarkerValue) { + t.Errorf("Hint leaked secret marker: %q", ce.Hint) + } + if strings.Contains(string(sel.Source), secretMarkerValue) { + t.Errorf("Selection.Source leaked secret marker: %q", sel.Source) + } + if strings.Contains(sel.DirectCredentialEnv.AppID, secretMarkerValue) { + t.Errorf("Selection.DirectCredentialEnv.AppID leaked secret marker: %q", sel.DirectCredentialEnv.AppID) + } + for _, k := range sel.DirectCredentialEnv.Keys { + if strings.Contains(k, secretMarkerValue) { + t.Errorf("Selection.DirectCredentialEnv.Keys leaked secret marker: %q", k) + } + } + // The secret-invalid path leaves p.selection zero-valued; this trivially + // implies no marker anywhere in it and guards against a future field being populated + // from the failed resolution. + if sel.Source != "" || sel.DirectCredentialEnv.Present || + sel.DirectCredentialEnv.AppID != "" || len(sel.DirectCredentialEnv.Keys) != 0 { + t.Errorf("Selection() = %+v, want zero value on profile_secret_invalid", sel) + } +} + +// A profile matching a complete direct env credential wins and records the env as matched. +func TestSelection_ProfileMatchesCompleteDirectEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") // matches profile app_id + t.Setenv(envvars.CliAppSecret, envSecretValue) + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sel.Source != credential.SourceFlagProfile { + t.Fatalf("source = %q, want %q", sel.Source, credential.SourceFlagProfile) + } + if !sel.DirectCredentialEnv.Present || !sel.DirectCredentialEnv.Matched { + t.Fatalf("DirectCredentialEnv = %+v, want Present && Matched", sel.DirectCredentialEnv) + } + if sel.DirectCredentialEnv.AppID != "cli_a" { + t.Errorf("DirectCredentialEnv.AppID = %q, want cli_a", sel.DirectCredentialEnv.AppID) + } + assertNoSecretLeak(t, "state8", string(sel.Source), sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state8-keys", sel.DirectCredentialEnv.Keys...) +} + +// APP_ID-only is sufficient for profile arbitration: a matching selected +// profile supplies all credentials and tokens, so the incomplete direct env +// must not block the profile. +func TestSelection_ProfileMatchesAppIDOnly_ProfileWins(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + acct, err := cp.ResolveAccount(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if acct.AppID != "cli_a" || acct.AppSecret != secretValue { + t.Fatalf("account = %+v, want selected profile credentials", acct) + } + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("Selection: %v", err) + } + if !sel.DirectCredentialEnv.Present || !sel.DirectCredentialEnv.Matched { + t.Fatalf("DirectCredentialEnv = %+v, want Present && Matched", sel.DirectCredentialEnv) + } +} + +func TestSelection_ProfileMatchingEnvTokens_UsesProfileTokenSource(t *testing.T) { + for _, tt := range []struct { + name string + tokenType credential.TokenType + envKey string + }{ + {name: "UAT", tokenType: credential.TokenTypeUAT, envKey: envvars.CliUserAccessToken}, + {name: "TAT", tokenType: credential.TokenTypeTAT, envKey: envvars.CliTenantAccessToken}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.envKey, "env-token") + writeConfigTenantA(t) + + ep := &envprovider.Provider{} + defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return &noopKC{} }, "tenant_a") + defaultToken := &mockDefaultTokenProvider{token: "profile-token"} + cp := credential.NewCredentialProvider([]extcred.Provider{ep}, defaultAcct, defaultToken, nil) + cp.WithProfileFromFlag("tenant_a") + + result, err := cp.ResolveToken(context.Background(), credential.TokenSpec{Type: tt.tokenType, AppID: "cli_a"}) + if err != nil { + t.Fatalf("ResolveToken: %v", err) + } + if result.Token != "profile-token" { + t.Fatalf("token = %q, want profile-token (env token must not override selected profile)", result.Token) + } + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("Selection: %v", err) + } + if sel.Source != credential.SourceFlagProfile || !sel.DirectCredentialEnv.Present || !sel.DirectCredentialEnv.Matched { + t.Fatalf("Selection = %+v, want profile source with matched direct env", sel) + } + }) + } +} + +// A profile that conflicts with a complete direct env credential reports a hard conflict. +func TestSelection_ProfileConflictsWithCompleteDirectEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_x") // mismatches profile app_id cli_a + t.Setenv(envvars.CliAppSecret, envSecretValue) + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileAppCredentialConflict { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileAppCredentialConflict) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) + } + ve := asValidationError(t, err) + if ve.ProfileAppID != "cli_a" { + t.Errorf("profile_app_id = %q, want cli_a", ve.ProfileAppID) + } + if ve.EnvAppID != "cli_x" { + t.Errorf("env_app_id = %q, want cli_x", ve.EnvAppID) + } + if !strings.Contains(ve.Hint, "unset "+envvars.CliAppID+" and "+envvars.CliAppSecret) { + t.Errorf("hint = %q, want exact conflicting env variable names", ve.Hint) + } + assertNoSecretLeak(t, "state9", ve.Message, ve.Hint) +} + +func TestSelection_ProfileConflictsWithAppIDOnly(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_x") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileAppCredentialConflict { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileAppCredentialConflict) + } + ve := asValidationError(t, err) + if ve.ProfileAppID != "cli_a" || ve.EnvAppID != "cli_x" { + t.Fatalf("conflict = profile:%q env:%q, want cli_a/cli_x", ve.ProfileAppID, ve.EnvAppID) + } + if !strings.Contains(ve.Hint, "unset "+envvars.CliAppID) { + t.Errorf("hint = %q, want exact APP_ID unset instruction", ve.Hint) + } +} + +func TestSelection_ExplicitMissingProfileWinsOverIncompleteEnv(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_typo", false) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileNotFound { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileNotFound) + } + ce := asConfigError(t, err) + if ce.Profile != "tenant_typo" || ce.CredentialSource != string(credential.SourceEnvProfile) { + t.Fatalf("profile error = %+v, want tenant_typo from env profile", ce) + } +} + +func TestSelection_ExplicitProfileMalformedConfigWinsOverIncompleteEnv(t *testing.T) { + writeMalformedConfig(t) + t.Setenv(envvars.CliAppSecret, "direct-value") + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + assertMalformedSurfaced(t, err) +} + +// A direct secret or token without APP_ID remains incomplete even when a valid profile is selected. +func TestSelection_ProfileWithDirectEnvMissingAppID(t *testing.T) { + for _, tt := range []struct { + name string + key string + }{ + {name: "APP_SECRET-only", key: envvars.CliAppSecret}, + {name: "UAT-only", key: envvars.CliUserAccessToken}, + {name: "TAT-only", key: envvars.CliTenantAccessToken}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.key, "direct-value") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeAppCredentialIncomplete { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeAppCredentialIncomplete) + } + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Fatalf("exit code = %d, want %d", got, output.ExitAuth) + } + ce := asConfigError(t, err) + if !slices.Equal(ce.MissingKeys, []string{envvars.CliAppID}) { + t.Errorf("missing_keys = %v, want [%s]", ce.MissingKeys, envvars.CliAppID) + } + wantHint := "set " + envvars.CliAppID + ", or unset " + tt.key + " to use the selected profile." + if ce.Hint != wantHint { + t.Errorf("hint = %q, want %q", ce.Hint, wantHint) + } + var blockErr *extcred.BlockError + if !errors.As(err, &blockErr) || blockErr.Code != extcred.BlockReasonCredentialIncomplete { + t.Fatalf("cause chain does not preserve credential BlockError: %T %v", err, err) + } + assertNoSecretLeak(t, "state10", ce.Message, ce.Hint) + assertNoSecretLeak(t, "state10-keys", ce.MissingKeys...) + }) + } +} + +// An invalid policy variable is a user input mistake: it must surface as a +// typed validation error (exit 2) carrying the variable name in param and a +// repair hint — never as an internal error, and never rewritten into +// app_credential_incomplete. The original BlockError stays on the cause chain. +func TestSelection_InvalidPolicyEnvBlockBecomesTypedValidation(t *testing.T) { + for _, tt := range []struct { + name string + key string + }{ + {name: "invalid DEFAULT_AS", key: envvars.CliDefaultAs}, + {name: "invalid STRICT_MODE", key: envvars.CliStrictMode}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv(tt.key, "banana") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want validation %d", got, output.ExitValidation) + } + ve := asValidationError(t, err) + if ve.Param != tt.key { + t.Errorf("param = %q, want %q", ve.Param, tt.key) + } + if !strings.Contains(ve.Message, tt.key) || !strings.Contains(ve.Message, "banana") { + t.Errorf("message = %q, want the variable name and its invalid value", ve.Message) + } + if !strings.Contains(ve.Hint, tt.key) { + t.Errorf("hint = %q, want repair guidance naming %s", ve.Hint, tt.key) + } + var blockErr *extcred.BlockError + if !errors.As(err, &blockErr) || blockErr.Code != extcred.BlockReasonInvalidPolicy { + t.Fatalf("cause chain does not preserve the invalid-policy BlockError: %T %v", err, err) + } + if strings.Contains(err.Error(), string(errs.SubtypeAppCredentialIncomplete)) { + t.Fatalf("error was rewritten as app_credential_incomplete: %v", err) + } + }) + } +} + +func TestActiveExtensionProviderName_MatchingAppIDOnlyProfileUsesBuiltin(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil { + t.Fatalf("ActiveExtensionProviderName: %v", err) + } + if name != "" { + t.Fatalf("provider name = %q, want builtin profile (empty)", name) + } +} + +// A failed profile resolution must neither propagate its error nor report an +// external takeover: this probe guards the builtin setup/repair commands +// (auth, config), which must stay usable to fix exactly these states. It +// falls back to the engagement probe instead. +func TestActiveExtensionProviderName_ProfileResolutionFailureFallsBackToProbe(t *testing.T) { + t.Run("no provider engaged, stale profile -> builtin allowed", func(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) // no config -> profile_not_found + cp := newProvider(t, "ghost", false) + + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil || name != "" { + t.Fatalf("ActiveExtensionProviderName = %q, %v; want \"\", nil", name, err) + } + }) + t.Run("engaged env provider still reported on conflict", func(t *testing.T) { + t.Setenv(envvars.CliAppID, "cli_x") // conflicts with tenant_a -> resolution fails + t.Setenv(envvars.CliAppSecret, envSecretValue) + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + cp := newProvider(t, "tenant_a", true) + + name, err := cp.ActiveExtensionProviderName(context.Background()) + if err != nil || name != "env" { + t.Fatalf("ActiveExtensionProviderName = %q, %v; want \"env\", nil", name, err) + } + }) +} + +// stubFailureResolver fails ResolveAccount with a fixed error. +type stubFailureResolver struct{ err error } + +func (s *stubFailureResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return nil, s.err +} + +// stubAccountResolver returns a fixed account. +type stubAccountResolver struct{ acct *credential.Account } + +func (s *stubAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return s.acct, nil +} + +// Contract test: ANY typed resolver failure (other than not_configured) on +// the profile route passes through with its own subtype and hint. The +// real-path proof for the keychain out-of-sync case is +// TestSelection_ProfileKeychainOutOfSyncSurfacesRealCause above. +func TestSelection_ProfileTypedResolverFailurePassesThrough(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + + typed := errs.NewValidationError(errs.SubtypeFailedPrecondition, + "credential backend rejected the stored profile"). + WithHint("repair the stored profile input.") + cp := credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, &stubFailureResolver{err: typed}, nil, nil) + cp.WithProfileFromFlag("tenant_a") + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeFailedPrecondition { + t.Fatalf("subtype = %q, want the typed failure passed through", got) + } + if strings.Contains(err.Error(), "could not be resolved locally") { + t.Fatalf("typed failure was flattened into the generic secret error: %v", err) + } +} + +// An untyped resolver failure must stay masked behind the generic secret +// error: its content is not guaranteed secret-free. +func TestSelection_ProfileUntypedResolverFailureStaysGeneric(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + + cp := credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, + &stubFailureResolver{err: fmt.Errorf("read keychain: %s", secretValue)}, nil, nil) + cp.WithProfileFromFlag("tenant_a") + + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeProfileSecretInvalid { + t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileSecretInvalid) + } + assertNoSecretLeak(t, "untyped-resolver-failure", err.Error()) +} + +// The resolver re-reads the config after arbitration validated the snapshot; +// if a concurrent edit hands back a different app, the mismatch must be +// refused rather than silently using credentials arbitration never checked. +func TestSelection_ProfileResolverAppIDMismatchRefused(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) // snapshot sees tenant_a -> cli_a + + cp := credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, + &stubAccountResolver{acct: &credential.Account{AppID: "cli_other", AppSecret: secretValue}}, nil, nil) + cp.WithProfileFromFlag("tenant_a") + + _, err := cp.Selection(context.Background()) + if err == nil { + t.Fatal("expected error for app_id mismatch between snapshot and resolver") + } + if !strings.Contains(err.Error(), "config changed during resolution") { + t.Fatalf("error = %v, want config-changed refusal", err) + } +} + +// A config that exists but cannot be read (permission denied) must surface +// invalid_config with the real cause — not profile_not_found (the profile may +// well exist in the unreadable file) and not no_active_profile. +func TestSelection_UnreadableConfigSurfacesRealCause(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("relies on POSIX file permissions") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses file permissions") + } + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") + writeConfigTenantA(t) + path := core.GetConfigPath() + if err := os.Chmod(path, 0o000); err != nil { + t.Fatalf("chmod: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(path, 0o600) }) + + t.Run("explicit profile", func(t *testing.T) { + cp := newProvider(t, "tenant_a", true) + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeInvalidConfig { + t.Fatalf("subtype = %q, want invalid_config (not profile_not_found)", got) + } + }) + t.Run("config default", func(t *testing.T) { + cp := newProvider(t, "", false) + _, err := cp.Selection(context.Background()) + if got := subtypeOf(t, err); got != errs.SubtypeInvalidConfig { + t.Fatalf("subtype = %q, want invalid_config (not no_active_profile)", got) + } + }) +} + +// fakeSidecarProvider is a NON-env extension provider (Priority 0, Name != +// directCredentialProviderName) that always returns a non-nil account. It +// stands in for the sidecar extension provider without needing a build tag. +type fakeSidecarProvider struct { + appID string +} + +func (f *fakeSidecarProvider) Name() string { return "sidecar" } +func (f *fakeSidecarProvider) Priority() int { return 0 } +func (f *fakeSidecarProvider) ResolveAccount(ctx context.Context) (*extcred.Account, error) { + return &extcred.Account{AppID: f.appID, Brand: extcred.Brand("feishu")}, nil +} +func (f *fakeSidecarProvider) ResolveToken(ctx context.Context, req extcred.TokenSpec) (*extcred.Token, error) { + return &extcred.Token{Value: "sidecar-tok", Source: "sidecar"}, nil +} + +// Regression: a NON-env extension provider (sidecar) that returns a managed +// account must win outright even when a profile is set. It must NOT be treated +// as a direct-credential env account: no profile arbitration, no +// profile_app_credential_conflict (even though its app_id differs from the +// profile's cli_a), and DirectCredentialEnv.Present must stay false because no +// direct env vars are set. The selection source names the provider explicitly. +func TestSelection_NonEnvExtensionProviderWinsOverProfile(t *testing.T) { + t.Setenv(envvars.CliAppID, "") // no direct env credential + t.Setenv(envvars.CliAppSecret, "") // no direct env credential + writeConfigTenantA(t) // profile tenant_a exists, app_id cli_a + + sidecar := &fakeSidecarProvider{appID: "sidecar_app"} // differs from cli_a + defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return &noopKC{} }, "tenant_a") + cp := credential.NewCredentialProvider([]extcred.Provider{sidecar}, defaultAcct, nil, nil) + cp.WithProfileFromFlag("tenant_a") + + acct, err := cp.ResolveAccount(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // The sidecar account is used as-is, NOT overridden by profile arbitration. + if acct == nil || acct.AppID != "sidecar_app" { + t.Fatalf("account = %+v, want AppID sidecar_app (sidecar wins outright)", acct) + } + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected Selection error: %v", err) + } + // No misreported direct env credential. + if sel.DirectCredentialEnv.Present { + t.Errorf("DirectCredentialEnv.Present = true, want false (no direct env vars set)") + } + if sel.Source != credential.SourceExtension("sidecar") { + t.Errorf("source = %q, want %q", sel.Source, credential.SourceExtension("sidecar")) + } + // The mismatched app_id (sidecar_app vs profile cli_a) must NOT trigger a + // profile_app_credential_conflict: both ResolveAccount and Selection above + // returned nil errors, so no conflict (or any other) error was produced. + // Guard against a future regression that surfaces a conflict via Selection. + if _, selErr := cp.Selection(context.Background()); selErr != nil { + if subtypeOf(t, selErr) == errs.SubtypeProfileAppCredentialConflict { + t.Errorf("got profile_app_credential_conflict, want none for non-env provider") + } + } + assertNoSecretLeak(t, "nonenv-sidecar", string(sel.Source), sel.DirectCredentialEnv.AppID) +} + +// Without an explicit profile or direct credential, selection uses the configured current app. +func TestSelection_ConfigDefault(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantA(t) // CurrentApp = tenant_a + cp := newProvider(t, "", false) + + sel, err := cp.Selection(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if sel.Source != credential.SourceConfigCurrentApp { + t.Fatalf("source = %q, want %q", sel.Source, credential.SourceConfigCurrentApp) + } +} + +// forgedDirectProvider impersonates the builtin env provider by name and +// declares AccountDirect. The reservation must hold by concrete type, not by +// the forgeable Name() string. +type forgedDirectProvider struct{} + +func (forgedDirectProvider) Name() string { return "env" } +func (forgedDirectProvider) Priority() int { return 0 } +func (forgedDirectProvider) ResolveAccount(context.Context) (*extcred.Account, error) { + return &extcred.Account{AppID: "forged_app", Kind: extcred.AccountDirect}, nil +} +func (forgedDirectProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func TestSelection_ForgedDirectProviderRejected(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + cp := credential.NewCredentialProvider([]extcred.Provider{forgedDirectProvider{}}, nil, nil, nil) + _, err := cp.ResolveAccount(context.Background()) + if err == nil || !strings.Contains(err.Error(), "reserved for the builtin env provider") { + t.Fatalf("err = %v, want AccountDirect reservation failure for a name-forging provider", err) + } +} + +// forgedIncompleteProvider exercises the public SPI boundary: although it +// supplies provider-owned metadata, credential_incomplete is reserved for the +// builtin env provider while arbitration diagnostics name LARKSUITE_CLI_*. +type forgedIncompleteProvider struct{} + +func (forgedIncompleteProvider) Name() string { return "vault" } +func (forgedIncompleteProvider) Priority() int { return 0 } +func (forgedIncompleteProvider) ResolveAccount(context.Context) (*extcred.Account, error) { + return nil, &extcred.BlockError{ + Provider: "vault", + Reason: "vault app credential is incomplete", + Code: extcred.BlockReasonCredentialIncomplete, + AppID: "vault_app", + PresentKeys: []string{"VAULT_APP_ID"}, + } +} +func (forgedIncompleteProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func TestSelection_NonEnvCredentialIncompleteRejected(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + providers := []extcred.Provider{forgedIncompleteProvider{}} + probeCP := credential.NewCredentialProvider(providers, nil, nil, nil) + name, probeErr := probeCP.ActiveExtensionProviderName(context.Background()) + if name != "" { + t.Fatalf("provider name = %q, want none for an invalid SPI classification", name) + } + + arbCP := credential.NewCredentialProvider(providers, nil, nil, nil) + _, arbErr := arbCP.ResolveAccount(context.Background()) + for label, err := range map[string]error{"probe": probeErr, "arbitration": arbErr} { + if err == nil || !strings.Contains(err.Error(), "reserved for the builtin env provider") { + t.Fatalf("%s err = %v, want credential_incomplete reservation failure for a non-env provider", label, err) + } + if got := subtypeOf(t, err); got != errs.SubtypeUnknown { + t.Fatalf("%s subtype = %q, want unknown internal contract violation", label, got) + } + } + if probeErr.Error() != arbErr.Error() { + t.Fatalf("probe and arbitration diverge:\n probe: %v\n arbitration: %v", probeErr, arbErr) + } +} + +// policyBlockProvider blocks with an invalid_policy classification, standing +// in for the env provider having seen a bad LARKSUITE_CLI_DEFAULT_AS. +type policyBlockProvider struct{} + +func (policyBlockProvider) Name() string { return "env" } +func (policyBlockProvider) Priority() int { return 0 } +func (policyBlockProvider) ResolveAccount(context.Context) (*extcred.Account, error) { + return nil, &extcred.BlockError{ + Provider: "env", + Reason: "invalid LARKSUITE_CLI_DEFAULT_AS \"banana\" (want user, bot, or auto)", + Code: extcred.BlockReasonInvalidPolicy, + Param: envvars.CliDefaultAs, + } +} +func (policyBlockProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +// The gate probe must stay aligned with formal arbitration when multiple +// providers are registered: provider A's invalid_policy block surfaces as the +// same typed validation error in both, instead of the probe scanning on and +// blaming provider B as an external takeover. +func TestActiveExtensionProviderName_InvalidPolicyAlignsWithArbitration(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + providers := []extcred.Provider{policyBlockProvider{}, &fakeSidecarProvider{appID: "sidecar_app"}} + + probeCP := credential.NewCredentialProvider(providers, nil, nil, nil) + name, probeErr := probeCP.ActiveExtensionProviderName(context.Background()) + if name != "" { + t.Fatalf("provider name = %q, want none (no external takeover)", name) + } + if got := subtypeOf(t, probeErr); got != errs.SubtypeInvalidArgument { + t.Fatalf("probe subtype = %q, want invalid_argument", got) + } + + arbCP := credential.NewCredentialProvider(providers, nil, nil, nil) + _, arbErr := arbCP.ResolveAccount(context.Background()) + if got := subtypeOf(t, arbErr); got != errs.SubtypeInvalidArgument { + t.Fatalf("arbitration subtype = %q, want invalid_argument", got) + } + if probeErr.Error() != arbErr.Error() { + t.Fatalf("probe and arbitration diverge:\n probe: %v\n arbitration: %v", probeErr, arbErr) + } +} diff --git a/internal/credential/credential_provider_test.go b/internal/credential/credential_provider_test.go index 6dd13a985d..c54bddb662 100644 --- a/internal/credential/credential_provider_test.go +++ b/internal/credential/credential_provider_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/core" @@ -23,6 +24,7 @@ type mockExtProvider struct { err error accountErr error tokenErr error + tokenCalls int } func (m *mockExtProvider) Name() string { return m.name } @@ -33,6 +35,7 @@ func (m *mockExtProvider) ResolveAccount(ctx context.Context) (*extcred.Account, return m.account, m.err } func (m *mockExtProvider) ResolveToken(ctx context.Context, req extcred.TokenSpec) (*extcred.Token, error) { + m.tokenCalls++ if m.tokenErr != nil { return nil, m.tokenErr } @@ -49,11 +52,13 @@ func (m *mockDefaultAcct) ResolveAccount(ctx context.Context) (*Account, error) } type mockDefaultToken struct { - result *TokenResult - err error + result *TokenResult + err error + tokenCalls int } func (m *mockDefaultToken) ResolveToken(ctx context.Context, req TokenSpec) (*TokenResult, error) { + m.tokenCalls++ return m.result, m.err } @@ -116,35 +121,45 @@ func TestCredentialProvider_AccountCached(t *testing.T) { } func TestCredentialProvider_TokenFromExtension(t *testing.T) { - cp := NewCredentialProvider( - []extcred.Provider{&mockExtProvider{ - name: "env", - account: &extcred.Account{AppID: "ext_app", Brand: "feishu"}, - token: &extcred.Token{Value: "ext_tok", Source: "env"}, - }}, - &mockDefaultAcct{}, &mockDefaultToken{result: &TokenResult{Token: "default_tok"}}, nil, - ) - result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) - if err != nil { - t.Fatal(err) - } - if result.Token != "ext_tok" { - t.Errorf("expected ext_tok, got %s", result.Token) + for _, sourceName := range []string{"env", "authsidecar"} { + t.Run(sourceName, func(t *testing.T) { + cp := NewCredentialProvider( + []extcred.Provider{&mockExtProvider{ + name: sourceName, + account: &extcred.Account{AppID: "ext_app", Brand: "feishu"}, + token: &extcred.Token{Value: "ext_tok", Source: sourceName}, + }}, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, + &mockDefaultToken{result: &TokenResult{Token: "default_tok"}}, nil, + ) + result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "ext_app"}) + if err != nil { + t.Fatal(err) + } + if result.Token != "ext_tok" { + t.Errorf("expected ext_tok, got %s", result.Token) + } + }) } } func TestCredentialProvider_TokenFallsToDefault(t *testing.T) { + defaultToken := &mockDefaultToken{result: &TokenResult{Token: "default_tok"}} cp := NewCredentialProvider( []extcred.Provider{&mockExtProvider{name: "skip"}}, - &mockDefaultAcct{}, &mockDefaultToken{result: &TokenResult{Token: "default_tok"}}, nil, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, + defaultToken, nil, ) - result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "default_app"}) if err != nil { t.Fatal(err) } if result.Token != "default_tok" { t.Errorf("expected default_tok, got %s", result.Token) } + if defaultToken.tokenCalls != 1 { + t.Fatalf("default ResolveToken() calls = %d, want 1", defaultToken.tokenCalls) + } } func TestCredentialProvider_TokenDoesNotMixSourcesAfterDefaultAccountSelection(t *testing.T) { @@ -159,7 +174,7 @@ func TestCredentialProvider_TokenDoesNotMixSourcesAfterDefaultAccountSelection(t t.Fatalf("ResolveAccount() error = %v", err) } - result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "default_app"}) if err != nil { t.Fatalf("ResolveToken() error = %v", err) } @@ -181,7 +196,7 @@ func TestCredentialProvider_SelectedSourceWithoutTokenReturnsUnavailableError(t t.Fatalf("ResolveAccount() error = %v", err) } - _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "ext_app"}) if err == nil { t.Fatal("ResolveToken() error = nil, want unavailable error") } @@ -202,7 +217,7 @@ func TestCredentialProvider_ResolveTokenPropagatesNonBlockExtensionError(t *test nil, ) - _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "ext_app"}) if err == nil || err.Error() != "provider exploded" { t.Fatalf("ResolveToken() error = %v, want provider exploded", err) } @@ -312,12 +327,12 @@ func TestCredentialProvider_ResolveIdentityHint_CachesResult(t *testing.T) { func TestCredentialProvider_ResolveTokenTreatsEmptyDefaultTokenAsMalformed(t *testing.T) { cp := NewCredentialProvider( nil, - nil, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, &mockDefaultToken{result: &TokenResult{Token: ""}}, nil, ) - _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "default_app"}) if err == nil || !strings.Contains(err.Error(), "empty token") { t.Fatalf("ResolveToken() error = %v, want malformed empty token error", err) } @@ -410,17 +425,189 @@ func TestCredentialProvider_ResolveAccountWarnsWhenExtensionIdentityVerification } func TestCredentialProvider_ResolveTokenDoesNotBypassFailedDefaultAccountResolution(t *testing.T) { + defaultToken := &mockDefaultToken{result: &TokenResult{Token: "default_tok"}} cp := NewCredentialProvider( nil, &mockDefaultAcct{err: errors.New("config unavailable")}, - &mockDefaultToken{result: &TokenResult{Token: "default_tok"}}, + defaultToken, nil, ) - _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT}) + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "default_app"}) if err == nil || err.Error() != "config unavailable" { t.Fatalf("ResolveToken() error = %v, want config unavailable", err) } + if defaultToken.tokenCalls != 0 { + t.Fatalf("default ResolveToken() calls = %d, want 0", defaultToken.tokenCalls) + } +} + +func TestCredentialProvider_ResolveTokenRejectsUnboundAppBeforeExtensionIO(t *testing.T) { + tests := []struct { + name string + appID string + }{ + {name: "empty app id"}, + {name: "different app id", appID: "other_app"}, + } + + for _, tt := range tests { + for _, sourceName := range []string{"env", "authsidecar"} { + t.Run(tt.name+"/"+sourceName, func(t *testing.T) { + provider := &mockExtProvider{ + name: sourceName, + account: &extcred.Account{AppID: "ext_app", Brand: "feishu"}, + token: &extcred.Token{Value: "ext_tok", Source: sourceName}, + } + httpClientCalls := 0 + cp := NewCredentialProvider( + []extcred.Provider{provider}, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, + &mockDefaultToken{result: &TokenResult{Token: "default_tok"}}, + func() (*http.Client, error) { + httpClientCalls++ + return nil, errors.New("unexpected user_info call") + }, + ) + + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: tt.appID}) + if err == nil { + t.Fatal("ResolveToken() error = nil, want app binding error") + } + assertInternalUnknownWithRetryHint(t, err) + if provider.tokenCalls != 0 { + t.Fatalf("extension ResolveToken() calls = %d, want 0", provider.tokenCalls) + } + if httpClientCalls != 0 { + t.Fatalf("httpClient() calls = %d, want 0", httpClientCalls) + } + }) + } + } +} + +func TestCredentialProvider_ResolveTokenRejectsUnboundAppBeforeDefaultIO(t *testing.T) { + tests := []struct { + name string + appID string + }{ + {name: "empty app id"}, + {name: "different app id", appID: "other_app"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defaultToken := &mockDefaultToken{result: &TokenResult{Token: "default_tok"}} + cp := NewCredentialProvider( + nil, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, + defaultToken, + nil, + ) + + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: tt.appID}) + if err == nil { + t.Fatal("ResolveToken() error = nil, want app binding error") + } + assertInternalUnknownWithRetryHint(t, err) + if defaultToken.tokenCalls != 0 { + t.Fatalf("default ResolveToken() calls = %d, want 0", defaultToken.tokenCalls) + } + }) + } +} + +func TestCredentialProvider_ResolveTokenRejectsNilAccountBeforeTokenIO(t *testing.T) { + defaultToken := &mockDefaultToken{result: &TokenResult{Token: "default_tok"}} + cp := NewCredentialProvider( + nil, + &mockDefaultAcct{}, + defaultToken, + nil, + ) + + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "requested_app"}) + if err == nil { + t.Fatal("ResolveToken() error = nil, want nil account error") + } + assertInternalUnknownWithRetryHint(t, err) + if defaultToken.tokenCalls != 0 { + t.Fatalf("default ResolveToken() calls = %d, want 0", defaultToken.tokenCalls) + } +} + +func TestCredentialProvider_ResolveTokenRejectsMissingSelectedSourceWithoutFallback(t *testing.T) { + extension := &mockExtProvider{ + name: "env", + token: &extcred.Token{Value: "ext_tok", Source: "env"}, + } + defaultToken := &mockDefaultToken{result: &TokenResult{Token: "default_tok"}} + cp := NewCredentialProvider( + []extcred.Provider{extension}, + &mockDefaultAcct{account: &Account{AppID: "default_app"}}, + defaultToken, + nil, + ) + cp.account = &Account{AppID: "selected_app"} + cp.accountOnce.Do(func() {}) + + _, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "selected_app"}) + if err == nil { + t.Fatal("ResolveToken() error = nil, want missing selected source error") + } + assertInternalUnknownWithRetryHint(t, err) + if extension.tokenCalls != 0 { + t.Fatalf("extension ResolveToken() calls = %d, want 0", extension.tokenCalls) + } + if defaultToken.tokenCalls != 0 { + t.Fatalf("default ResolveToken() calls = %d, want 0", defaultToken.tokenCalls) + } +} + +func TestCredentialProvider_ResolveTokenMatchingExtensionDoesNotEnrichIdentity(t *testing.T) { + provider := &mockExtProvider{ + name: "env", + account: &extcred.Account{AppID: "ext_app", Brand: "feishu"}, + token: &extcred.Token{Value: "ext_tok", Source: "env"}, + } + httpClientCalls := 0 + cp := NewCredentialProvider( + []extcred.Provider{provider}, + nil, + nil, + func() (*http.Client, error) { + httpClientCalls++ + return nil, errors.New("unexpected user_info call") + }, + ) + + result, err := cp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "ext_app"}) + if err != nil { + t.Fatalf("ResolveToken() error = %v", err) + } + if result.Token != "ext_tok" { + t.Fatalf("ResolveToken() token = %q, want %q", result.Token, "ext_tok") + } + if provider.tokenCalls != 1 { + t.Fatalf("extension ResolveToken() calls = %d, want 1", provider.tokenCalls) + } + if httpClientCalls != 0 { + t.Fatalf("httpClient() calls = %d, want 0", httpClientCalls) + } +} + +func assertInternalUnknownWithRetryHint(t *testing.T, err error) { + t.Helper() + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error type = %T, want typed internal error", err) + } + if problem.Category != errs.CategoryInternal || problem.Subtype != errs.SubtypeUnknown { + t.Fatalf("error problem = %+v, want internal/unknown", problem) + } + if problem.Hint != "retry the command." { + t.Fatalf("error hint = %q, want retry hint", problem.Hint) + } } func TestActiveExtensionProviderName_ExtActive(t *testing.T) { diff --git a/internal/credential/decide_test.go b/internal/credential/decide_test.go new file mode 100644 index 0000000000..8e8aa87630 --- /dev/null +++ b/internal/credential/decide_test.go @@ -0,0 +1,181 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package credential + +import ( + "context" + "testing" + + "github.com/larksuite/cli/errs" + extcred "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/envvars" +) + +// stubDecideProvider satisfies extcred.Provider for building providerAccount +// literals; decideIdentity only ever calls Name() on it. +type stubDecideProvider struct{ name string } + +func (s stubDecideProvider) Name() string { return s.name } +func (s stubDecideProvider) Priority() int { return 0 } +func (s stubDecideProvider) ResolveAccount(context.Context) (*extcred.Account, error) { + return nil, nil +} +func (s stubDecideProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func pa(providerName, appID string) *providerAccount { + return &providerAccount{ + acct: &Account{AppID: appID}, + source: extensionTokenSource{provider: stubDecideProvider{name: providerName}}, + } +} + +func appIDOnlyBlock(appID string) *extcred.BlockError { + return &extcred.BlockError{ + Provider: "env", + Reason: envvars.CliAppID + " is set but no app secret or access token is available", + Code: extcred.BlockReasonCredentialIncomplete, + RequiredAnyOf: []string{envvars.CliAppSecret, envvars.CliUserAccessToken, envvars.CliTenantAccessToken}, + PresentKeys: []string{envvars.CliAppID}, + AppID: appID, + } +} + +func uatOnlyBlock() *extcred.BlockError { + return &extcred.BlockError{ + Provider: "env", + Reason: envvars.CliUserAccessToken + " is set but " + envvars.CliAppID + " is missing", + Code: extcred.BlockReasonCredentialIncomplete, + MissingKeys: []string{envvars.CliAppID}, + PresentKeys: []string{envvars.CliUserAccessToken}, + } +} + +// TestDecideIdentity exercises the selection matrix as data: decideIdentity is +// pure, so every rule (precedence, conflict detection, error attribution) is +// table-testable without env vars or config fixtures. +func TestDecideIdentity(t *testing.T) { + tenantA := &core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{Name: "tenant_a", AppId: "cli_a"}}, + } + noCurrent := &core.MultiAppConfig{ + Apps: []core.AppConfig{{Name: "tenant_a", AppId: "cli_a"}}, + } + invalidConfigErr := errs.NewConfigError(errs.SubtypeInvalidConfig, "invalid config format") + notConfiguredErr := core.NotConfiguredError() + + cases := []struct { + name string + in identityInputs + route credentialRoute + source CredentialSourceKind + matched bool + subtype errs.Subtype // "" = success expected + }{ + { + name: "managed provider wins over explicit profile", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, managed: pa("sidecar", "sidecar_app"), config: tenantA}, + route: routeManaged, + source: SourceExtension("sidecar"), + }, + { + name: "profile conflicts with complete direct env app_id", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, direct: pa("env", "cli_x"), directKeys: []string{envvars.CliAppID, envvars.CliAppSecret}, config: tenantA}, + subtype: errs.SubtypeProfileAppCredentialConflict, + }, + { + name: "matched complete direct env yields profile route", + in: identityInputs{profile: "tenant_a", profileSrc: SourceEnvProfile, direct: pa("env", "cli_a"), directKeys: []string{envvars.CliAppID, envvars.CliAppSecret}, config: tenantA}, + route: routeProfile, + source: SourceEnvProfile, + matched: true, + }, + { + name: "APP_ID-only block matching the profile yields profile route", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, directBlock: appIDOnlyBlock("cli_a"), directKeys: []string{envvars.CliAppID}, config: tenantA}, + route: routeProfile, + source: SourceFlagProfile, + matched: true, + }, + { + name: "APP_ID-only block mismatching the profile is a hard conflict", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, directBlock: appIDOnlyBlock("cli_x"), directKeys: []string{envvars.CliAppID}, config: tenantA}, + subtype: errs.SubtypeProfileAppCredentialConflict, + }, + { + name: "UAT-only block with a valid profile keeps the repair error", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, directBlock: uatOnlyBlock(), config: tenantA}, + subtype: errs.SubtypeAppCredentialIncomplete, + }, + { + name: "block without profile is app_credential_incomplete", + in: identityInputs{directBlock: appIDOnlyBlock("cli_a"), directKeys: []string{envvars.CliAppID}}, + subtype: errs.SubtypeAppCredentialIncomplete, + }, + { + name: "complete direct env without profile wins", + in: identityInputs{direct: pa("env", "cli_env"), directKeys: []string{envvars.CliAppID, envvars.CliAppSecret}}, + route: routeDirectEnv, + source: SourceEnvAppID, + }, + { + name: "malformed config is not masked as profile_not_found", + in: identityInputs{profile: "tenant_a", profileSrc: SourceFlagProfile, configErr: invalidConfigErr}, + subtype: errs.SubtypeInvalidConfig, + }, + { + name: "absent config degrades to profile_not_found", + in: identityInputs{profile: "ghost", profileSrc: SourceEnvProfile, configErr: notConfiguredErr}, + subtype: errs.SubtypeProfileNotFound, + }, + { + name: "profile missing from a valid config is profile_not_found even with incomplete env", + in: identityInputs{profile: "ghost", profileSrc: SourceEnvProfile, directBlock: appIDOnlyBlock("cli_a"), directKeys: []string{envvars.CliAppID}, config: tenantA}, + subtype: errs.SubtypeProfileNotFound, + }, + { + name: "config default reports currentApp", + in: identityInputs{config: tenantA}, + route: routeConfigDefault, + source: SourceConfigCurrentApp, + }, + { + name: "config default without currentApp reports firstApp", + in: identityInputs{config: noCurrent}, + route: routeConfigDefault, + source: SourceConfigFirstApp, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + d, err := decideIdentity(tc.in) + if tc.subtype != "" { + if err == nil { + t.Fatalf("decideIdentity = %+v, want error subtype %q", d, tc.subtype) + } + prob, ok := errs.ProblemOf(err) + if !ok || prob.Subtype != tc.subtype { + t.Fatalf("error = %v, want subtype %q", err, tc.subtype) + } + return + } + if err != nil { + t.Fatalf("decideIdentity: %v", err) + } + if d.route != tc.route { + t.Errorf("route = %d, want %d", d.route, tc.route) + } + if d.selection.Source != tc.source { + t.Errorf("source = %q, want %q", d.selection.Source, tc.source) + } + if d.selection.DirectCredentialEnv.Matched != tc.matched { + t.Errorf("matched = %v, want %v", d.selection.DirectCredentialEnv.Matched, tc.matched) + } + }) + } +} diff --git a/internal/credential/default_provider.go b/internal/credential/default_provider.go index d7401b62ed..d927917fe1 100644 --- a/internal/credential/default_provider.go +++ b/internal/credential/default_provider.go @@ -74,9 +74,13 @@ func NewDefaultAccountProvider(kc func() keychain.KeychainAccess, profile string func (p *DefaultAccountProvider) ResolveAccount(ctx context.Context) (*Account, error) { // Load config once — used for both credentials and strict mode. - multi, err := core.LoadMultiAppConfig() + // LoadOrNotConfigured distinguishes an absent config (→ not_configured) + // from a malformed/unreadable one (→ invalid_config with cause), so a + // broken config is never masked as "run config init" — matching the + // explicit-profile path in doResolveAccount. + multi, err := core.LoadOrNotConfigured() if err != nil { - return nil, core.NotConfiguredError() + return nil, err } cfg, err := core.ResolveConfigFromMulti(multi, p.keychain(), p.profile) @@ -116,6 +120,7 @@ type DefaultTokenProvider struct { tatOnce sync.Once tatResult *TokenResult + tatAppID string tatErr error } @@ -126,21 +131,42 @@ func NewDefaultTokenProvider(defaultAcct *DefaultAccountProvider, httpClient fun func (p *DefaultTokenProvider) ResolveToken(ctx context.Context, req TokenSpec) (*TokenResult, error) { switch req.Type { case TokenTypeUAT: - return p.resolveUAT(ctx) + return p.resolveUAT(ctx, req) case TokenTypeTAT: - return p.resolveTAT(ctx) + return p.resolveTAT(ctx, req) default: return nil, fmt.Errorf("unsupported token type: %s", req.Type) } } +// checkTokenAppID refuses to hand out a token for a different app than the +// caller resolved. The token provider re-reads the config, so a concurrent +// profile edit between account resolution and token resolution could otherwise +// cross tokens between apps. TokenSpec.AppID is REQUIRED here: an empty value +// would silently disable the guarantee, so it is rejected rather than skipped. +func checkTokenAppID(req TokenSpec, resolvedAppID string) error { + if req.AppID == "" { + return errs.NewInternalError(errs.SubtypeUnknown, + "TokenSpec.AppID is required for %s token resolution", req.Type) + } + if req.AppID == resolvedAppID { + return nil + } + return errs.NewInternalError(errs.SubtypeUnknown, + "config changed during resolution: token requested for app %q but the saved profile now resolves to a different app", req.AppID). + WithHint("retry the command.") +} + // resolveUAT resolves a user access token. Not cached (unlike TAT) because UAT // may be refreshed between calls and GetValidAccessToken handles its own caching. -func (p *DefaultTokenProvider) resolveUAT(ctx context.Context) (*TokenResult, error) { +func (p *DefaultTokenProvider) resolveUAT(ctx context.Context, req TokenSpec) (*TokenResult, error) { acct, err := p.defaultAcct.ResolveAccount(ctx) if err != nil { return nil, err } + if err := checkTokenAppID(req, acct.AppID); err != nil { + return nil, err + } httpClient, err := p.httpClient() if err != nil { return nil, err @@ -157,20 +183,36 @@ func (p *DefaultTokenProvider) resolveUAT(ctx context.Context) (*TokenResult, er return &TokenResult{Token: token, Scopes: scopes}, nil } -// resolveTAT resolves a tenant access token. The result is cached after the first -// call via sync.Once — only the context from the first call is used. -func (p *DefaultTokenProvider) resolveTAT(ctx context.Context) (*TokenResult, error) { - p.tatOnce.Do(func() { - p.tatResult, p.tatErr = p.doResolveTAT(ctx) - }) - return p.tatResult, p.tatErr -} - -func (p *DefaultTokenProvider) doResolveTAT(ctx context.Context) (*TokenResult, error) { +// resolveTAT resolves a tenant access token. The result is cached after the +// first mint via sync.Once — only the context from that call is used. +// +// The account is resolved and checked against the request BEFORE any token +// work: a mismatched request must not trigger a token mint (network call, +// quota, audit trail) for the wrong app. The cached result is additionally +// re-checked on every hit, so a token minted for one app is never served to +// a request that resolved another. +func (p *DefaultTokenProvider) resolveTAT(ctx context.Context, req TokenSpec) (*TokenResult, error) { acct, err := p.defaultAcct.ResolveAccount(ctx) if err != nil { return nil, err } + if err := checkTokenAppID(req, acct.AppID); err != nil { + return nil, err + } + p.tatOnce.Do(func() { + p.tatResult, p.tatErr = p.doResolveTAT(ctx, acct) + p.tatAppID = acct.AppID + }) + if p.tatErr != nil { + return nil, p.tatErr + } + if err := checkTokenAppID(req, p.tatAppID); err != nil { + return nil, err + } + return p.tatResult, nil +} + +func (p *DefaultTokenProvider) doResolveTAT(ctx context.Context, acct *Account) (*TokenResult, error) { httpClient, err := p.httpClient() if err != nil { return nil, err diff --git a/internal/credential/default_provider_test.go b/internal/credential/default_provider_test.go index 057f1dba80..06b8332461 100644 --- a/internal/credential/default_provider_test.go +++ b/internal/credential/default_provider_test.go @@ -4,10 +4,15 @@ package credential import ( + "context" "errors" + "io" + "net/http" + "strings" "testing" "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/core" ) func TestDefaultTokenProvider_Dispatches(t *testing.T) { @@ -92,3 +97,136 @@ func TestClassifyTATResponseCode_CodeZeroOtherError_StillTyped(t *testing.T) { t.Fatalf("code-0 invalid_scope must not be a ConfigError, got %T", err) } } + +func TestCheckTokenAppID(t *testing.T) { + if err := checkTokenAppID(TokenSpec{Type: TokenTypeUAT}, "cli_a"); err == nil { + t.Fatal("empty requested app must be rejected: it would silently disable the guarantee") + } + if err := checkTokenAppID(TokenSpec{AppID: "cli_a"}, "cli_a"); err != nil { + t.Fatalf("matching app must pass: %v", err) + } + err := checkTokenAppID(TokenSpec{AppID: "cli_a"}, "cli_b") + if err == nil { + t.Fatal("mismatched app must be refused") + } + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("error type = %T, want *errs.InternalError", err) + } +} + +// REAL-path regression for review F2: the token provider re-reads the config, +// so a profile edit between account resolution and token resolution must not +// hand a token minted for the new app to a caller that resolved the old one. +// Uses the real DefaultAccountProvider + DefaultTokenProvider; the HTTP stub +// makes the network step unreachable, so reaching it proves the app check ran +// and passed first. +func TestDefaultTokenProvider_RefusesTokenAfterConfigSwap(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + writeCfg := func(appID string) { + t.Helper() + multi := &core.MultiAppConfig{CurrentApp: "tenant_a", Apps: []core.AppConfig{{ + Name: "tenant_a", AppId: appID, AppSecret: core.PlainSecret("your-secret"), Brand: core.BrandFeishu, + }}} + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + } + writeCfg("cli_a") + + httpSentinel := errors.New("http client sentinel: unreachable in test") + tp := NewDefaultTokenProvider( + NewDefaultAccountProvider(nil, "tenant_a"), + func() (*http.Client, error) { return nil, httpSentinel }, + nil, + ) + + // Matching app: the consistency check passes and resolution proceeds to + // the (stubbed) HTTP step. + _, err := tp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "cli_a"}) + if !errors.Is(err, httpSentinel) { + t.Fatalf("err = %v, want the HTTP sentinel (check must pass for a matching app)", err) + } + + // The profile now resolves to a different app: the token request that was + // arbitrated for cli_a must be refused before any token work happens. + writeCfg("cli_b") + _, err = tp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeUAT, AppID: "cli_a"}) + if err == nil || !strings.Contains(err.Error(), "config changed during resolution") { + t.Fatalf("err = %v, want config-changed refusal", err) + } +} + +// F1 regression: a TAT request for a mismatched app must be refused BEFORE +// any token work starts — no HTTP client construction, no mint, no cache — +// otherwise the CLI mints (and caches) a token for the wrong app and only +// then refuses to return it, leaving auth audit/quota side effects behind. +func TestDefaultTokenProvider_TATChecksAppBeforeAnyTokenWork(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + multi := &core.MultiAppConfig{CurrentApp: "tenant_a", Apps: []core.AppConfig{{ + Name: "tenant_a", AppId: "cli_b", AppSecret: core.PlainSecret("your-secret"), Brand: core.BrandFeishu, + }}} + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + httpCalled := false + tp := NewDefaultTokenProvider( + NewDefaultAccountProvider(nil, "tenant_a"), + func() (*http.Client, error) { httpCalled = true; return nil, errors.New("http sentinel") }, + nil, + ) + + // The profile resolves to cli_b, but the caller arbitrated cli_a. + _, err := tp.ResolveToken(context.Background(), TokenSpec{Type: TokenTypeTAT, AppID: "cli_a"}) + if err == nil || !strings.Contains(err.Error(), "config changed during resolution") { + t.Fatalf("err = %v, want config-changed refusal", err) + } + if httpCalled { + t.Fatal("token work started for a mismatched app: the check must run before any HTTP client is built") + } +} + +// countingTATTripper serves a canned successful TAT response and counts calls. +type countingTATTripper struct{ calls int } + +func (c *countingTATTripper) RoundTrip(*http.Request) (*http.Response, error) { + c.calls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(`{"code":0,"access_token":"your-access-token"}`)), + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, nil +} + +// TAT happy path: the first request mints the token over HTTP, the second is +// served from the sync.Once cache without another HTTP call. +func TestDefaultTokenProvider_TATSuccessAndCacheHit(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + multi := &core.MultiAppConfig{CurrentApp: "tenant_a", Apps: []core.AppConfig{{ + Name: "tenant_a", AppId: "cli_a", AppSecret: core.PlainSecret("your-secret"), Brand: core.BrandFeishu, + }}} + if err := core.SaveMultiAppConfig(multi); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + tripper := &countingTATTripper{} + tp := NewDefaultTokenProvider( + NewDefaultAccountProvider(nil, "tenant_a"), + func() (*http.Client, error) { return &http.Client{Transport: tripper}, nil }, + nil, + ) + + req := TokenSpec{Type: TokenTypeTAT, AppID: "cli_a"} + first, err := tp.ResolveToken(context.Background(), req) + if err != nil || first.Token != "your-access-token" { + t.Fatalf("first resolve = %+v, %v; want minted token", first, err) + } + second, err := tp.ResolveToken(context.Background(), req) + if err != nil || second.Token != "your-access-token" { + t.Fatalf("second resolve = %+v, %v; want cached token", second, err) + } + if tripper.calls != 1 { + t.Fatalf("HTTP calls = %d, want exactly 1 (second resolve must hit the cache)", tripper.calls) + } +} diff --git a/internal/credential/identity_selection.go b/internal/credential/identity_selection.go new file mode 100644 index 0000000000..bba484fc9c --- /dev/null +++ b/internal/credential/identity_selection.go @@ -0,0 +1,54 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package credential + +// CredentialSourceKind is the wire-stable App/credential selection source. +type CredentialSourceKind string + +const ( + SourceFlagProfile CredentialSourceKind = "flag:--profile" + SourceEnvProfile CredentialSourceKind = "env:LARKSUITE_CLI_PROFILE" + SourceEnvAppID CredentialSourceKind = "env:LARKSUITE_CLI_APP_ID" + SourceConfigCurrentApp CredentialSourceKind = "config:currentApp" + SourceConfigFirstApp CredentialSourceKind = "config:firstApp" + + // SourceExtensionPrefix prefixes the name of a managed extension provider + // that won selection outright (e.g. "extension:sidecar"). With it, an + // empty Source is left with exactly one meaning: not resolved. + SourceExtensionPrefix CredentialSourceKind = "extension:" +) + +// SourceExtension reports the selection source for a managed extension +// provider by name. +func SourceExtension(name string) CredentialSourceKind { + return SourceExtensionPrefix + CredentialSourceKind(name) +} + +// DirectCredentialEnv describes the state of direct app credential env vars. +// It never carries a secret value — only names and the non-sensitive app_id. +type DirectCredentialEnv struct { + Present bool `json:"present"` + Keys []string `json:"keys,omitempty"` + AppID string `json:"appId,omitempty"` + Matched bool `json:"matched,omitempty"` + ConflictsWithProfile bool `json:"conflictsWithProfile,omitempty"` +} + +// IdentitySelection is the explainable result of credential selection. +// It carries NO secret value. +type IdentitySelection struct { + Source CredentialSourceKind + DirectCredentialEnv DirectCredentialEnv +} + +// Explicit reports whether the identity was actively specified by the +// user/agent (flag or env), which governs no-fallback behavior. +func (s IdentitySelection) Explicit() bool { + switch s.Source { + case SourceFlagProfile, SourceEnvProfile, SourceEnvAppID: + return true + default: + return false + } +} diff --git a/internal/credential/identity_selection_test.go b/internal/credential/identity_selection_test.go new file mode 100644 index 0000000000..326e8753f5 --- /dev/null +++ b/internal/credential/identity_selection_test.go @@ -0,0 +1,25 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package credential + +import "testing" + +func TestIdentitySelectionExplicit(t *testing.T) { + cases := []struct { + src CredentialSourceKind + explicit bool + }{ + {SourceFlagProfile, true}, + {SourceEnvProfile, true}, + {SourceEnvAppID, true}, + {SourceConfigCurrentApp, false}, + {SourceConfigFirstApp, false}, + } + for _, c := range cases { + sel := IdentitySelection{Source: c.src} + if sel.Explicit() != c.explicit { + t.Errorf("source %q: Explicit()=%v want %v", c.src, sel.Explicit(), c.explicit) + } + } +} diff --git a/internal/credential/integration_test.go b/internal/credential/integration_test.go index de173d1948..531e1e0483 100644 --- a/internal/credential/integration_test.go +++ b/internal/credential/integration_test.go @@ -52,6 +52,24 @@ func TestFullChain_EnvWins(t *testing.T) { } } +func TestFullChain_EnvRejectsDifferentApp(t *testing.T) { + t.Setenv(envvars.CliAppID, "env_app") + t.Setenv(envvars.CliAppSecret, "env_secret") + t.Setenv(envvars.CliUserAccessToken, "env_uat") + + cp := credential.NewCredentialProvider( + []extcred.Provider{&envprovider.Provider{}}, + nil, nil, nil, + ) + + _, err := cp.ResolveToken(context.Background(), credential.TokenSpec{ + Type: credential.TokenTypeUAT, AppID: "other_app", + }) + if err == nil { + t.Fatal("ResolveToken() error = nil, want app binding error") + } +} + func TestFullChain_Fallthrough(t *testing.T) { // env provider returns nil (no env vars set), falls through to default token ep := &envprovider.Provider{} @@ -59,7 +77,8 @@ func TestFullChain_Fallthrough(t *testing.T) { cp := credential.NewCredentialProvider( []extcred.Provider{ep}, - nil, mock, nil, + &mockDefaultAccountProvider{account: &credential.Account{AppID: "app1"}}, + mock, nil, ) result, err := cp.ResolveToken(context.Background(), credential.TokenSpec{ Type: credential.TokenTypeUAT, AppID: "app1", @@ -72,6 +91,14 @@ func TestFullChain_Fallthrough(t *testing.T) { } } +type mockDefaultAccountProvider struct { + account *credential.Account +} + +func (m *mockDefaultAccountProvider) ResolveAccount(context.Context) (*credential.Account, error) { + return m.account, nil +} + type mockDefaultTokenProvider struct { token string scopes string diff --git a/internal/envvars/envvars.go b/internal/envvars/envvars.go index 36b60e42d5..6d40bc5290 100644 --- a/internal/envvars/envvars.go +++ b/internal/envvars/envvars.go @@ -21,6 +21,7 @@ const ( CliAgentName = "LARKSUITE_CLI_AGENT_NAME" CliAgentTrace = "LARKSUITE_CLI_AGENT_TRACE" + CliProfile = "LARKSUITE_CLI_PROFILE" CliProxyEnable = "LARKSUITE_CLI_PROXY_ENABLE" CliProxyAddress = "LARKSUITE_CLI_PROXY_ADDRESS" diff --git a/shortcuts/common/runner_scope_test.go b/shortcuts/common/runner_scope_test.go index c3313eaf43..fa1f217a1e 100644 --- a/shortcuts/common/runner_scope_test.go +++ b/shortcuts/common/runner_scope_test.go @@ -25,6 +25,18 @@ func (r *scopeCheckTokenResolver) ResolveToken(ctx context.Context, req credenti return r.result, r.err } +type scopeCheckAccountResolver struct { + appID string +} + +func (r scopeCheckAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + +func newScopeCheckCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, scopeCheckAccountResolver{appID: appID}, tokenResolver, nil) +} + // TestEnhancePermissionError_TypedPermissionErrorRouted pins typed routing: // an *errs.PermissionError gets enhanced regardless of its Message text, // decoupling this helper from canonical-message rewrites that would @@ -106,7 +118,7 @@ func TestEnhancePermissionError_PermissionErrorGetsScopeHint(t *testing.T) { func TestCheckShortcutScopes_PropagatesContextCancellation(t *testing.T) { f := &cmdutil.Factory{ - Credential: credential.NewCredentialProvider(nil, nil, &scopeCheckTokenResolver{err: context.Canceled}, nil), + Credential: newScopeCheckCredentialProvider("app-1", &scopeCheckTokenResolver{err: context.Canceled}), } err := checkShortcutScopes(f, context.Background(), core.AsUser, &core.CliConfig{AppID: "app-1"}, []string{"im:message:read"}) @@ -124,9 +136,9 @@ func TestCheckShortcutScopes_PropagatesContextCancellation(t *testing.T) { // command for human consumers. func TestCheckShortcutScopes_ReturnsTypedPermissionError(t *testing.T) { f := &cmdutil.Factory{ - Credential: credential.NewCredentialProvider(nil, nil, &scopeCheckTokenResolver{ + Credential: newScopeCheckCredentialProvider("app-1", &scopeCheckTokenResolver{ result: &credential.TokenResult{Token: "t", Scopes: "im:message:read calendar:calendar:read"}, - }, nil), + }), } required := []string{"im:message:read", "drive:drive:read", "docx:document:read"} @@ -168,7 +180,7 @@ func TestCheckShortcutScopes_ReturnsTypedPermissionError(t *testing.T) { func TestCheckShortcutScopes_IgnoresNonContextTokenErrors(t *testing.T) { f := &cmdutil.Factory{ - Credential: credential.NewCredentialProvider(nil, nil, &scopeCheckTokenResolver{err: errors.New("token cache unavailable")}, nil), + Credential: newScopeCheckCredentialProvider("app-1", &scopeCheckTokenResolver{err: errors.New("token cache unavailable")}), } err := checkShortcutScopes(f, context.Background(), core.AsUser, &core.CliConfig{AppID: "app-1"}, []string{"im:message:read"}) diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index d00498460b..9a504397fb 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -33,6 +33,18 @@ func (r *driveStatusScopedTokenResolver) ResolveToken(ctx context.Context, req c return &credential.TokenResult{Token: "test-token", Scopes: r.scopes}, nil } +type driveTestAccountResolver struct { + appID string +} + +func (r driveTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + +func newDriveTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, driveTestAccountResolver{appID: appID}, tokenResolver, nil) +} + // TestDriveStatusCategorizesByHash exercises the four-bucket classification // against a real walk of the temp dir and a mocked Drive listing. func TestDriveStatusCategorizesByHash(t *testing.T) { @@ -308,7 +320,7 @@ func TestDriveStatusQuickMarksUntrustedTimestampAsModified(t *testing.T) { // requiring drive:file:download even after quick mode made download optional. func TestDriveStatusExactRejectsMissingDownloadScope(t *testing.T) { f, _, _, _ := cmdutil.TestFactory(t, driveTestConfig()) - f.Credential = credential.NewCredentialProvider(nil, nil, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}, nil) + f.Credential = newDriveTestCredentialProvider(driveTestConfig().AppID, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}) tmpDir := t.TempDir() withDriveWorkingDir(t, tmpDir) @@ -357,7 +369,7 @@ func TestDriveStatusExactRejectsMissingDownloadScope(t *testing.T) { // blocked on the exact-mode download scope precheck. func TestDriveStatusQuickAcceptsMissingDownloadScope(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) - f.Credential = credential.NewCredentialProvider(nil, nil, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}, nil) + f.Credential = newDriveTestCredentialProvider(driveTestConfig().AppID, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}) tmpDir := t.TempDir() withDriveWorkingDir(t, tmpDir) diff --git a/shortcuts/drive/drive_sync_test.go b/shortcuts/drive/drive_sync_test.go index 70104a2f37..0faf591b83 100644 --- a/shortcuts/drive/drive_sync_test.go +++ b/shortcuts/drive/drive_sync_test.go @@ -22,7 +22,6 @@ import ( "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" @@ -1593,7 +1592,7 @@ func TestDriveSyncAskConflictEOFDuringPlanningPreventsAnyWrites(t *testing.T) { func TestDriveSyncDryRunQuickAcceptsMetadataOnlyScope(t *testing.T) { f, stdout, _, _ := cmdutil.TestFactory(t, driveTestConfig()) - f.Credential = credential.NewCredentialProvider(nil, nil, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}, nil) + f.Credential = newDriveTestCredentialProvider(driveTestConfig().AppID, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly"}) tmpDir := t.TempDir() withDriveWorkingDir(t, tmpDir) @@ -1622,7 +1621,7 @@ func TestDriveSyncPreflightsActionScopesBeforeListing(t *testing.T) { AppID: "drive-sync-download-scope-only", AppSecret: "test-secret", Brand: core.BrandFeishu, } f, stdout, _, _ := cmdutil.TestFactory(t, syncTestConfig) - f.Credential = credential.NewCredentialProvider(nil, nil, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly drive:file:download"}, nil) + f.Credential = newDriveTestCredentialProvider(syncTestConfig.AppID, &driveStatusScopedTokenResolver{scopes: "drive:drive.metadata:readonly drive:file:download"}) tmpDir := t.TempDir() withDriveWorkingDir(t, tmpDir) diff --git a/shortcuts/drive/drive_task_result_test.go b/shortcuts/drive/drive_task_result_test.go index 99a4ef7c2c..5f36920d90 100644 --- a/shortcuts/drive/drive_task_result_test.go +++ b/shortcuts/drive/drive_task_result_test.go @@ -329,7 +329,7 @@ func newDriveTaskResultRuntimeWithScopes(t *testing.T, as core.Identity, scopes cfg := driveTestConfig() factory, _, _, _ := cmdutil.TestFactory(t, cfg) - factory.Credential = credential.NewCredentialProvider(nil, nil, &mockDriveTaskResultTokenResolver{scopes: scopes}, nil) + factory.Credential = newDriveTestCredentialProvider(cfg.AppID, &mockDriveTaskResultTokenResolver{scopes: scopes}) runtime := common.TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "drive +task_result"}, cfg, as) runtime.Factory = factory @@ -919,7 +919,7 @@ func TestValidateDriveTaskResultScopesPropagatesContextCancellation(t *testing.T cfg := driveTestConfig() factory, _, _, _ := cmdutil.TestFactory(t, cfg) - factory.Credential = credential.NewCredentialProvider(nil, nil, cancelingTokenResolver{}, nil) + factory.Credential = newDriveTestCredentialProvider(cfg.AppID, cancelingTokenResolver{}) runtime := common.TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "drive +task_result"}, cfg, core.AsUser) runtime.Factory = factory diff --git a/shortcuts/im/convert_lib/runtime_test.go b/shortcuts/im/convert_lib/runtime_test.go index 5ab8ac154c..fac7a6e0af 100644 --- a/shortcuts/im/convert_lib/runtime_test.go +++ b/shortcuts/im/convert_lib/runtime_test.go @@ -27,6 +27,14 @@ func (s *staticConvertlibTokenResolver) ResolveToken(_ context.Context, _ creden return &credential.TokenResult{Token: "test-token"}, nil } +type convertlibTestAccountResolver struct { + appID string +} + +func (r convertlibTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + type convertlibRoundTripFunc func(*http.Request) (*http.Response, error) func (f convertlibRoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { @@ -68,7 +76,12 @@ func newBotConvertlibRuntime(t *testing.T, rt http.RoundTripper) *common.Runtime AppSecret: "test-secret", Brand: core.BrandFeishu, } - testCred := credential.NewCredentialProvider(nil, nil, &staticConvertlibTokenResolver{}, nil) + testCred := credential.NewCredentialProvider( + nil, + convertlibTestAccountResolver{appID: cfg.AppID}, + &staticConvertlibTokenResolver{}, + nil, + ) runtime := &common.RuntimeContext{ Config: cfg, Factory: &cmdutil.Factory{ diff --git a/shortcuts/im/helpers_network_test.go b/shortcuts/im/helpers_network_test.go index 9d75fcf62a..6f992e1c42 100644 --- a/shortcuts/im/helpers_network_test.go +++ b/shortcuts/im/helpers_network_test.go @@ -37,6 +37,18 @@ func (s *staticShortcutTokenResolver) ResolveToken(_ context.Context, _ credenti return &credential.TokenResult{Token: "tenant-token"}, nil } +type imTestAccountResolver struct { + appID string +} + +func (r imTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + +func newIMTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, imTestAccountResolver{appID: appID}, tokenResolver, nil) +} + type shortcutRoundTripFunc func(*http.Request) (*http.Response, error) func (f shortcutRoundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { @@ -90,7 +102,7 @@ func newBotShortcutRuntime(t *testing.T, rt http.RoundTripper) *common.RuntimeCo AppSecret: "test-secret", Brand: core.BrandFeishu, } - testCred := credential.NewCredentialProvider(nil, nil, &staticShortcutTokenResolver{}, nil) + testCred := newIMTestCredentialProvider(cfg.AppID, &staticShortcutTokenResolver{}) runtime := &common.RuntimeContext{ Config: cfg, Factory: &cmdutil.Factory{ diff --git a/shortcuts/im/im_flag_test.go b/shortcuts/im/im_flag_test.go index d370c4cde6..f1af6dd1ff 100644 --- a/shortcuts/im/im_flag_test.go +++ b/shortcuts/im/im_flag_test.go @@ -288,12 +288,12 @@ func (r errorTokenResolver) ResolveToken(_ context.Context, _ credential.TokenSp func setRuntimeScopes(t *testing.T, rt *common.RuntimeContext, scopes string) { t.Helper() - rt.Factory.Credential = credential.NewCredentialProvider(nil, nil, scopedTokenResolver{scopes: scopes}, nil) + rt.Factory.Credential = newIMTestCredentialProvider(rt.Config.AppID, scopedTokenResolver{scopes: scopes}) } func setRuntimeTokenError(t *testing.T, rt *common.RuntimeContext, err error) { t.Helper() - rt.Factory.Credential = credential.NewCredentialProvider(nil, nil, errorTokenResolver{err: err}, nil) + rt.Factory.Credential = newIMTestCredentialProvider(rt.Config.AppID, errorTokenResolver{err: err}) } func TestFlagMessageID(t *testing.T) { diff --git a/shortcuts/wiki/wiki_delete_test.go b/shortcuts/wiki/wiki_delete_test.go index b2772884ec..4bdcfec33b 100644 --- a/shortcuts/wiki/wiki_delete_test.go +++ b/shortcuts/wiki/wiki_delete_test.go @@ -16,7 +16,6 @@ import ( "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" - "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/shortcuts/common" @@ -79,7 +78,7 @@ func newWikiDeleteSpaceRuntimeWithScopes(t *testing.T, as core.Identity, scopes cfg := wikiTestConfig() factory, _, stderr, _ := cmdutil.TestFactory(t, cfg) - factory.Credential = credential.NewCredentialProvider(nil, nil, &mockWikiMoveTokenResolver{scopes: scopes}, nil) + factory.Credential = newWikiTestCredentialProvider(cfg.AppID, &mockWikiMoveTokenResolver{scopes: scopes}) runtime := common.TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "wiki +delete-space"}, cfg, as) runtime.Factory = factory diff --git a/shortcuts/wiki/wiki_move_test.go b/shortcuts/wiki/wiki_move_test.go index 6d9a7e64f9..bef854cfe2 100644 --- a/shortcuts/wiki/wiki_move_test.go +++ b/shortcuts/wiki/wiki_move_test.go @@ -27,6 +27,18 @@ type fakeWikiMoveNodeCall struct { Spec wikiMoveSpec } +type wikiTestAccountResolver struct { + appID string +} + +func (r wikiTestAccountResolver) ResolveAccount(context.Context) (*credential.Account, error) { + return &credential.Account{AppID: r.appID}, nil +} + +func newWikiTestCredentialProvider(appID string, tokenResolver credential.DefaultTokenResolver) *credential.CredentialProvider { + return credential.NewCredentialProvider(nil, wikiTestAccountResolver{appID: appID}, tokenResolver, nil) +} + type fakeWikiDocsToWikiMoveCall struct { TargetSpaceID string Spec wikiMoveSpec @@ -143,7 +155,7 @@ func newWikiMoveRuntimeWithScopes(t *testing.T, as core.Identity, scopes string) cfg := wikiTestConfig() factory, _, stderr, _ := cmdutil.TestFactory(t, cfg) - factory.Credential = credential.NewCredentialProvider(nil, nil, &mockWikiMoveTokenResolver{scopes: scopes}, nil) + factory.Credential = newWikiTestCredentialProvider(cfg.AppID, &mockWikiMoveTokenResolver{scopes: scopes}) runtime := common.TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "wiki +move"}, cfg, as) runtime.Factory = factory diff --git a/sidecar/server-demo/handler_test.go b/sidecar/server-demo/handler_test.go index edd2446fef..834d217a95 100644 --- a/sidecar/server-demo/handler_test.go +++ b/sidecar/server-demo/handler_test.go @@ -26,12 +26,13 @@ import ( // fakeExtProvider is a stub extcred.Provider for tests that returns a fixed token. type fakeExtProvider struct { + appID string token string } func (f *fakeExtProvider) Name() string { return "fake" } func (f *fakeExtProvider) ResolveAccount(ctx context.Context) (*extcred.Account, error) { - return nil, nil + return &extcred.Account{AppID: f.appID}, nil } func (f *fakeExtProvider) ResolveToken(ctx context.Context, req extcred.TokenSpec) (*extcred.Token, error) { return &extcred.Token{Value: f.token, Source: "fake"}, nil @@ -381,7 +382,7 @@ func TestProxyHandler_AcceptsAllowedAuthHeaders(t *testing.T) { // Use a handler with a real (fake) credential provider so we can // distinguish auth-header reject (403) from later failures. cred := credential.NewCredentialProvider( - []extcred.Provider{&fakeExtProvider{token: "real-token"}}, + []extcred.Provider{&fakeExtProvider{appID: "cli_test", token: "real-token"}}, nil, nil, nil, ) h := &proxyHandler{ @@ -501,7 +502,7 @@ func TestProxyHandler_StripsClientSuppliedAuthHeaders(t *testing.T) { upstreamHost := strings.TrimPrefix(upstream.URL, "https://") cred := credential.NewCredentialProvider( - []extcred.Provider{&fakeExtProvider{token: realToken}}, + []extcred.Provider{&fakeExtProvider{appID: "cli_test", token: realToken}}, nil, nil, nil, ) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 5b1c94e247..5d8ccf531d 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -1,7 +1,7 @@ --- name: lark-shared version: 1.0.0 -description: "Use for lark-cli setup/auth tasks: auth login/status/logout, user vs bot identity, business-domain permissions (--domain, including all/docs/drive), missing scopes, revoking authorization, or handling _notice JSON." +description: "Use for lark-cli setup/auth tasks: auth login/status/logout, user vs bot identity, business-domain permissions (--domain, including all/docs/drive), missing scopes, revoking authorization, handling _notice JSON, or pinning/clearing a profile/tenant identity for a task or session." --- # lark-cli 共享规则 @@ -32,7 +32,7 @@ lark-cli config init --new | 获取全部权限 | `lark-cli auth login --domain all --no-wait --json` | | 按业务域授权 | `lark-cli auth login --domain docs --domain drive --no-wait --json`;`--domain` 可重复,也可用逗号分隔 | | 指定单个 scope 授权 | `lark-cli auth login --scope "" --no-wait --json` | -| 检查当前登录态、是谁登录、token 是否有效 | `lark-cli auth status --json --verify`;回答时引用 `identity`、`verified`、`identities.user.status`、`identities.user.userName`、`identities.user.openId`(用户 open id)、`identities.user.tokenStatus`、`identities.user.scope` | +| 检查当前登录态、是谁登录、token 是否有效 | 必须运行 `lark-cli auth status --json --verify`;回答时引用 `identity`、`verified`、`identities.user.status`、`identities.user.userName`、`identities.user.openId`(用户 open id)、`identities.user.tokenStatus`、`identities.user.scope` | | 快速查看当前身份状态 | `lark-cli whoami`;实际生效的那一个身份 | | 退出当前机器的用户登录态 | `lark-cli auth logout --json`;`loggedOut:true` 表示注销成功 | | bot 缺少权限 | 不要执行 `auth login`;引导用户在开发者后台开通 bot scope,优先复用错误里的 `console_url` | @@ -126,6 +126,18 @@ lark-cli auth login --device-code - **不要在同一轮中展示 URL 后立刻执行 `--device-code`**,这会导致用户看不到 URL - **禁止缓存 `verification_url` 或 `device_code`**:每次需要授权时,必须重新执行 `lark-cli auth login --no-wait --json` 生成新的链接。不要将授权链接和 device code 存入上下文供后续复用 +## Profile 选择 + +- 查当前实际生效身份:`whoami --json` +- 查 OAuth 登录或 token 有效性:`auth status --json --verify` +- 为 agent 任务指定身份:每条 `lark-cli` 命令都加 `--profile ` +- 为同一 shell 的脚本或批处理指定身份:使用 `LARKSUITE_CLI_PROFILE=`,或 export/unset +- 清除会话身份并恢复默认:使用 `unset LARKSUITE_CLI_PROFILE`;即使变量未设置,也向用户说明该操作。不要为此使用 `profile use` 或 `profile remove` +- 查已保存的配置:`config show` 或 `profile list`;它们不表示当前实际生效身份 +- 永久修改默认 profile:`profile use` + +profile 不明确时先问用户。除非用户提供了直连凭证,否则不要设置 `LARKSUITE_CLI_APP_ID` 或 `LARKSUITE_CLI_APP_SECRET`。 + ## 更新检查 lark-cli 命令执行后,如果检测到新版本,JSON 输出中会包含 `_notice.update` 字段(含 `message`、`command` 等)。