From 4dd8a3a6c476b2fade8d595533071df651a9017f Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:06:12 +0800 Subject: [PATCH 01/29] feat: add LARKSUITE_CLI_PROFILE session env with flag precedence Add LARKSUITE_CLI_PROFILE env var and make BootstrapInvocationContext fall back to it when --profile is empty, so downstream credential resolution sees the correct profile. Also track whether the resolved profile came from the flag or the env fallback via a new InvocationContext.ProfileFromFlag field, needed by a later task to report the correct credential source. --- cmd/bootstrap.go | 12 +++++++++- cmd/bootstrap_test.go | 48 ++++++++++++++++++++++++++++++++++++- internal/cmdutil/factory.go | 5 ++++ internal/envvars/envvars.go | 1 + 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 841a884094..2ad447bf15 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 := globals.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..659714ec37 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,45 @@ 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("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/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/envvars/envvars.go b/internal/envvars/envvars.go index 36b60e42d5..16419e4db4 100644 --- a/internal/envvars/envvars.go +++ b/internal/envvars/envvars.go @@ -10,6 +10,7 @@ const ( CliUserAccessToken = "LARKSUITE_CLI_USER_ACCESS_TOKEN" CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN" CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS" + CliProfile = "LARKSUITE_CLI_PROFILE" CliStrictMode = "LARKSUITE_CLI_STRICT_MODE" // Sidecar proxy (auth proxy mode) From c8eb7a55435d0ea5dc90356a18b1ed121f661645 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:12:56 +0800 Subject: [PATCH 02/29] feat: add profile selection error subtypes and machine-readable fields Declares the 5 stable error subtypes (4 config + 1 validation) and the ConfigError/ValidationError extension fields the profile-selection credential core (Task 4) will produce, plus builder-chain and wire-pin tests pinning their shape. --- errs/marshal_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++++ errs/subtypes.go | 15 ++++++---- errs/types.go | 36 +++++++++++++++++++---- errs/types_test.go | 22 ++++++++++++++ 4 files changed, 132 insertions(+), 10 deletions(-) diff --git a/errs/marshal_test.go b/errs/marshal_test.go index b495ad78f4..f7a3d72db5 100644 --- a/errs/marshal_test.go +++ b/errs/marshal_test.go @@ -136,6 +136,75 @@ 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"). + WithProfile("work"). + WithAppID("cli_abc") + 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"]`, + `"profile":"work"`, + `"app_id":"cli_abc"`, + } { + 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"`, `"profile"`, `"app_id"`} { + 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..37d1d45955 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,11 @@ 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"` + Profile string `json:"profile,omitempty"` + AppID string `json:"app_id,omitempty"` + Cause error `json:"-"` } // Unwrap is nil-receiver safe; see ValidationError.Unwrap. @@ -370,6 +381,21 @@ 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) WithProfile(name string) *ConfigError { + e.Profile = name + return e +} + +func (e *ConfigError) WithAppID(appID string) *ConfigError { + e.AppID = appID + 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..165b428960 100644 --- a/errs/types_test.go +++ b/errs/types_test.go @@ -643,3 +643,25 @@ 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") + 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) + } +} + +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) + } +} From 6adeb0a2a2cd864cf591345de2474a0261d1d27e Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:17:40 +0800 Subject: [PATCH 03/29] feat: add IdentitySelection type for explainable credential selection --- internal/credential/identity_selection.go | 44 +++++++++++++++++++ .../credential/identity_selection_test.go | 25 +++++++++++ 2 files changed, 69 insertions(+) create mode 100644 internal/credential/identity_selection.go create mode 100644 internal/credential/identity_selection_test.go diff --git a/internal/credential/identity_selection.go b/internal/credential/identity_selection.go new file mode 100644 index 0000000000..875c775d7c --- /dev/null +++ b/internal/credential/identity_selection.go @@ -0,0 +1,44 @@ +// 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" +) + +// 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 (security: §5.1). +type IdentitySelection struct { + Source CredentialSourceKind + DirectCredentialEnv DirectCredentialEnv + Suggestion string +} + +// 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) + } + } +} From ab00ddc394381e36560a090a65698e3ad3d97c6f Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:30:51 +0800 Subject: [PATCH 04/29] feat: unify credential selection with profile conflict detection --- internal/cmdutil/factory_default.go | 21 +- internal/credential/credential_provider.go | 210 ++++++++++- .../credential_provider_selection_test.go | 336 ++++++++++++++++++ 3 files changed, 545 insertions(+), 22 deletions(-) create mode 100644 internal/credential/credential_provider_selection_test.go diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index 2051e0beaa..6145d5ada0 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -61,10 +61,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: Config derived from Credential via an explicit conversion boundary. @@ -162,10 +163,11 @@ func buildSDKTransport() 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 { @@ -178,5 +180,6 @@ 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) + return credential.NewCredentialProvider(providers, defaultAcct, defaultToken, deps.HttpClient). + WithProfile(deps.Profile, deps.ProfileFromFlag) } diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 8f20be11d9..9cb161fffa 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -9,13 +9,21 @@ import ( "fmt" "io" "net/http" + "os" "sync" extcred "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/envvars" ) +// directCredentialProviderName is the Name() of the env provider, the source +// of direct app credentials (LARKSUITE_CLI_APP_ID / _APP_SECRET). Only its +// incomplete blocks map to app_credential_incomplete (spec §3 step 1). +const directCredentialProviderName = "env" + // DefaultAccountResolver is implemented by the default account provider. type DefaultAccountResolver interface { ResolveAccount(ctx context.Context) (*Account, error) @@ -136,10 +144,18 @@ type CredentialProvider struct { httpClient func() (*http.Client, error) warnOut io.Writer + // profile is the active profile (from --profile or LARKSUITE_CLI_PROFILE). + // profileFromFlag discriminates the source for the reported selection. + profile string + profileFromFlag bool + 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 (§5.1). + selection IdentitySelection hintOnce sync.Once hint *IdentityHint @@ -161,6 +177,15 @@ func (p *CredentialProvider) SetWarnOut(warnOut io.Writer) *CredentialProvider { return p } +// WithProfile records the active profile and whether it came from the +// --profile flag (as opposed to the LARKSUITE_CLI_PROFILE env fallback). +// It governs credential arbitration and the reported selection source. +func (p *CredentialProvider) WithProfile(profile string, fromFlag bool) *CredentialProvider { + p.profile = profile + p.profileFromFlag = fromFlag + 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. @@ -172,40 +197,199 @@ func (p *CredentialProvider) ResolveAccount(ctx context.Context) (*Account, erro return p.account, p.accountErr } +// doResolveAccount arbitrates the credential/App selection per the spec +// resolution order (§3): env-partial → profile → env-complete → config default. +// It populates p.selection (no secret; §5.1) and p.selectedSource on every +// success path. func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, error) { + // Step 1 (spec §3): consult the extension providers. The env provider is + // the "direct app credential" source. An incomplete direct credential + // (only APP_ID or only APP_SECRET set) short-circuits to + // app_credential_incomplete regardless of the active profile. + var envAcct *Account + var envSource extensionTokenSource for _, prov := range p.providers { acct, err := prov.ResolveAccount(ctx) if err != nil { + var blockErr *extcred.BlockError + // Only the env (direct-credential) provider maps an incomplete + // block to app_credential_incomplete. Other providers' blocks + // propagate unchanged so they still stop the chain (§3 step 1 + // is specifically about direct app credential env vars). + if errors.As(err, &blockErr) && prov.Name() == directCredentialProviderName { + if missing := missingDirectCredentialKeys(); len(missing) > 0 { + return nil, errs.NewConfigError(errs.SubtypeAppCredentialIncomplete, + "direct app credential is incomplete"). + WithMissingKeys(missing...). + WithHint("set both %s and %s, or unset both and use --profile / a config default.", + envvars.CliAppID, envvars.CliAppSecret) + } + // Block for a reason other than incompleteness (e.g. an + // invalid identity/strict-mode value); preserve prior behavior. + return nil, err + } 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) - } - // 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 = "" + envAcct = convertAccount(acct) + envSource = extensionTokenSource{provider: prov} + break + } + } + + // Step 2 (spec §3): an explicit profile was requested. + if p.profile != "" { + multi, loadErr := core.LoadMultiAppConfig() + var app *core.AppConfig + if loadErr == nil && multi != nil { + app = multi.FindApp(p.profile) + } + if app == nil { + return nil, errs.NewConfigError(errs.SubtypeProfileNotFound, + "profile %q not found", p.profile). + WithProfile(p.profile). + WithHint("run `lark-cli profile list` to see available profiles.") + } + if envAcct != nil { + // E == complete: the direct env app_id must match the profile. + if app.AppId != envAcct.AppID { + return nil, errs.NewValidationError(errs.SubtypeProfileAppCredentialConflict, + "profile %q app_id does not match %s", p.profile, envvars.CliAppID). + WithProfileAppConflict(app.AppId, envAcct.AppID). + WithHint("unset %s/%s, or select a profile whose app_id matches the environment.", + envvars.CliAppID, envvars.CliAppSecret) + } + p.selection = IdentitySelection{ + Source: p.profileSource(), + DirectCredentialEnv: DirectCredentialEnv{ + Present: true, + Keys: presentDirectCredentialKeys(), + AppID: envAcct.AppID, + Matched: true, + }, + } + } else { + p.selection = IdentitySelection{ + Source: p.profileSource(), + DirectCredentialEnv: DirectCredentialEnv{Present: false}, + } + } + // Resolve the profile's own (keychain-backed) credential locally. + acct, err := p.defaultAcct.ResolveAccount(ctx) + if err != nil { + // SECURITY (§5.1): generic message — never embed the underlying + // error or any secret material. + p.selection = IdentitySelection{} + return nil, errs.NewConfigError(errs.SubtypeProfileSecretInvalid, + "profile %q credential could not be resolved locally", p.profile). + WithProfile(p.profile). + WithAppID(app.AppId). + WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") + } + p.selection.Suggestion = "如需临时覆盖本条命令，使用 --profile。" + p.selectedSource = defaultTokenSource{resolver: p.defaultToken} + return acct, nil + } + + // Step 3 (spec §3): no explicit profile — direct env credential wins. + if envAcct != nil { + if err := p.enrichUserInfo(ctx, envAcct, envSource); err != nil { + if p.warnOut != nil { + _, _ = fmt.Fprintf(p.warnOut, "warning: unable to verify user identity from credential source %q: %v\n", envSource.Name(), err) } - p.selectedSource = source - return internal, nil + // enrichUserInfo failure is non-fatal: SupportedIdentities + // (used for strict mode) is already set by the provider. + // Clear unverified user identity for safety. + envAcct.UserOpenId = "" + envAcct.UserName = "" } + p.selectedSource = envSource + p.selection = IdentitySelection{ + Source: SourceEnvAppID, + DirectCredentialEnv: DirectCredentialEnv{ + Present: true, + Keys: presentDirectCredentialKeys(), + AppID: envAcct.AppID, + }, + } + return envAcct, nil } + + // No direct env credential and no profile → the config default. if p.defaultAcct != nil { acct, err := p.defaultAcct.ResolveAccount(ctx) if err != nil { + // No usable default identity → no_active_profile. Other typed + // failures (e.g. a specific config error) pass through unchanged. + if prob, ok := errs.ProblemOf(err); ok && prob.Subtype == errs.SubtypeNotConfigured { + return nil, errs.NewConfigError(errs.SubtypeNoActiveProfile, "no active profile"). + WithHint("run `lark-cli config init` / `lark-cli profile add`, or set %s.", envvars.CliProfile) + } return nil, err } + multi, _ := core.LoadMultiAppConfig() p.selectedSource = defaultTokenSource{resolver: p.defaultToken} + p.selection = IdentitySelection{Source: selectionSourceForDefault(multi)} return acct, nil } return nil, core.NotConfiguredError() } +// profileSource reports the credential source kind for a profile-backed +// selection, discriminating the --profile flag from the env fallback. +func (p *CredentialProvider) profileSource() CredentialSourceKind { + if p.profileFromFlag { + return SourceFlagProfile + } + return SourceEnvProfile +} + +// selectionSourceForDefault reports whether the config default resolved to the +// explicit currentApp or fell back to the first app (spec §3 step 3.2). +func selectionSourceForDefault(multi *core.MultiAppConfig) CredentialSourceKind { + if multi != nil && multi.CurrentApp != "" { + return SourceConfigCurrentApp + } + return SourceConfigFirstApp +} + +// missingDirectCredentialKeys returns the NAMES (never values) of the direct +// app credential env vars that are absent. Used only when the env provider +// blocks, to map an incomplete direct credential to app_credential_incomplete. +func missingDirectCredentialKeys() []string { + var missing []string + if os.Getenv(envvars.CliAppID) == "" { + missing = append(missing, envvars.CliAppID) + } + if os.Getenv(envvars.CliAppSecret) == "" { + missing = append(missing, envvars.CliAppSecret) + } + return missing +} + +// 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 +} + +// 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. // If UAT is available, user_info API call is mandatory (security: verify token validity). // If no UAT from extension, falls back to provider-supplied OpenID. diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go new file mode 100644 index 0000000000..bb165eaf41 --- /dev/null +++ b/internal/credential/credential_provider_selection_test.go @@ -0,0 +1,336 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package credential_test + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/envvars" + "github.com/larksuite/cli/internal/keychain" +) + +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 §5.1). +const secretValue = "s3cr3t-tenant-a-value" + +// envSecretValue is the direct env app secret. Same no-leak guarantee. +const envSecretValue = "env-secret-should-not-leak" + +// 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) + cp.WithProfile(profile, fromFlag) + 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 +} + +// State #2: P none, E none, C none -> no_active_profile. +func TestSelection_State2_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) + } + assertNoSecretLeak(t, "state2", err.Error(), string(sel.Source)) +} + +// State #3: P none, E partial (only APP_ID) -> app_credential_incomplete. +func TestSelection_State3_EnvPartial(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) + } + prob, _ := errs.ProblemOf(err) + ce := asConfigError(t, err) + if !slices.Contains(ce.MissingKeys, envvars.CliAppSecret) { + t.Errorf("missing_keys = %v, want to contain %q", ce.MissingKeys, envvars.CliAppSecret) + } + // missing_keys must be NAMES only, never values. + for _, k := range ce.MissingKeys { + if strings.Contains(k, envSecretValue) || strings.Contains(k, secretValue) { + t.Errorf("missing_keys contains a value, not a name: %q", k) + } + } + assertNoSecretLeak(t, "state3", prob.Message, prob.Hint) +} + +// State #4: P none, E complete -> env:LARKSUITE_CLI_APP_ID. +func TestSelection_State4_EnvComplete(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.Suggestion, sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state4-keys", sel.DirectCredentialEnv.Keys...) +} + +// State #5: P valid, E none -> flag:--profile (fromFlag) source. +func TestSelection_State5_ProfileOnly(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), sel.Suggestion) +} + +// State #5b: P valid from env (not flag) -> env:LARKSUITE_CLI_PROFILE source. +func TestSelection_State5_ProfileFromEnv(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) + } +} + +// State #6: P missing (nonexistent), E complete -> profile_not_found. +func TestSelection_State6_ProfileNotFound(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) + assertNoSecretLeak(t, "state6", err.Error(), prob.Hint, string(sel.Source)) +} + +// State #7: P valid but secret broken, E none -> profile_secret_invalid. +func TestSelection_State7_ProfileSecretInvalid(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.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) + } + assertNoSecretLeak(t, "state7", ce.Message, ce.Hint) +} + +// State #8: P valid, E complete, app_id matches -> profile source, env present+matched. +func TestSelection_State8_ProfileMatchesEnv(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.Suggestion, sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state8-keys", sel.DirectCredentialEnv.Keys...) +} + +// State #9: P valid, E complete, app_id mismatches -> profile_app_credential_conflict. +func TestSelection_State9_Conflict(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) + } + 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) + } + assertNoSecretLeak(t, "state9", ve.Message, ve.Hint) +} + +// State #10: P valid, E partial -> app_credential_incomplete (env-partial wins). +func TestSelection_State10_ProfileWithEnvPartial(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, envSecretValue) // only secret set + 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) + } + ce := asConfigError(t, err) + if !slices.Contains(ce.MissingKeys, envvars.CliAppID) { + t.Errorf("missing_keys = %v, want to contain %q", ce.MissingKeys, envvars.CliAppID) + } + assertNoSecretLeak(t, "state10", ce.Message, ce.Hint) + assertNoSecretLeak(t, "state10-keys", ce.MissingKeys...) +} + +// State #1: P none, E none, C present -> config default (currentApp). +func TestSelection_State1_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) + } +} From 221dcc1da79859556bc5872591b0c1da238581f6 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:42:16 +0800 Subject: [PATCH 05/29] fix(credential): gate success-account direct-credential treatment on env provider Mirror the env-incomplete block-path guard on the success-account path so a non-env extension provider (e.g. sidecar, Priority 0) that returns an account wins outright instead of being misreported as a direct-credential env account. This restores pre-diff behavior for such providers: no profile arbitration, no spurious profile_app_credential_conflict, and DirectCredentialEnv.Present stays false when no direct env vars are set. Env matrix states are unchanged. Add TestSelection_NonEnvExtensionProviderWinsOverProfile as a regression guard. --- internal/credential/credential_provider.go | 26 +++++++- .../credential_provider_selection_test.go | 64 ++++++++++++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 9cb161fffa..856fac22a0 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -12,8 +12,8 @@ import ( "os" "sync" - extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/errs" + extcred "github.com/larksuite/cli/extension/credential" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/envvars" @@ -231,6 +231,30 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er return nil, err } if acct != nil { + // Only the env (direct-credential) provider feeds profile + // arbitration / conflict detection / DirectCredentialEnv reporting. + // This mirrors the block-path guard above. A non-env extension + // provider (e.g. sidecar) is NOT a direct-credential env account: + // it wins outright here, returning its account + token source + // unchanged (pre-diff behavior), without being misreported as a + // direct env credential (§4.2: Present = direct env vars actually + // set) or triggering a spurious profile_app_credential_conflict. + if prov.Name() != directCredentialProviderName { + 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) + } + // 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 = "" + } + p.selectedSource = source + return internal, nil + } envAcct = convertAccount(acct) envSource = extensionTokenSource{provider: prov} break diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index bb165eaf41..781da7c41c 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -10,9 +10,9 @@ import ( "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/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/envvars" @@ -319,6 +319,68 @@ func TestSelection_State10_ProfileWithEnvPartial(t *testing.T) { assertNoSecretLeak(t, "state10-keys", ce.MissingKeys...) } +// 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 an 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 (§4.2 — +// no direct env vars are set). This proves the success-account provider gating +// mirrors the block-path guard. +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.WithProfile("tenant_a", true) + + 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 (§4.2). + if sel.DirectCredentialEnv.Present { + t.Errorf("DirectCredentialEnv.Present = true, want false (no direct env vars set)") + } + // 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.Suggestion, sel.DirectCredentialEnv.AppID) +} + // State #1: P none, E none, C present -> config default (currentApp). func TestSelection_State1_ConfigDefault(t *testing.T) { t.Setenv(envvars.CliAppID, "") From 7c13fb128a1ca801ab9e2b3526ed3fcbc2a465cc Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 17:52:03 +0800 Subject: [PATCH 06/29] test(credential): lock profile_secret_invalid against secret-bearing cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a case where the underlying account-resolution error itself contains a secret marker, proving doResolveAccount's drop-the-cause design (§5.1) holds beyond the existing noop-keychain (empty-error) test, including across the full errors.Unwrap chain. --- .../credential_provider_selection_test.go | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index 781da7c41c..d0d5178748 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -6,6 +6,7 @@ package credential_test import ( "context" "errors" + "fmt" "slices" "strings" "testing" @@ -255,6 +256,96 @@ func TestSelection_State7_ProfileSecretInvalid(t *testing.T) { assertNoSecretLeak(t, "state7", ce.Message, ce.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 = "SUPER_SECRET_MARKER_abc123" + +// 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) +} + +// State #7 (secret-bearing underlying error): P valid, but the underlying +// account/secret resolution fails with an error that itself contains a +// secret. This locks the §5.1 design: 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_State7_UnderlyingErrorContainingSecret_NotLeaked(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.WithProfile("tenant_a", true) + + 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.Suggestion, secretMarkerValue) { + t.Errorf("Selection.Suggestion leaked secret marker: %q", sel.Suggestion) + } + 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) + } + } + // State #7 always clears p.selection on the secret-invalid path (see + // doResolveAccount); assert it is zero-valued, which trivially implies no + // marker anywhere in it and guards against a future field being populated + // from the failed resolution. + if sel.Source != "" || sel.Suggestion != "" || sel.DirectCredentialEnv.Present || + sel.DirectCredentialEnv.AppID != "" || len(sel.DirectCredentialEnv.Keys) != 0 { + t.Errorf("Selection() = %+v, want zero value on profile_secret_invalid", sel) + } +} + // State #8: P valid, E complete, app_id matches -> profile source, env present+matched. func TestSelection_State8_ProfileMatchesEnv(t *testing.T) { t.Setenv(envvars.CliAppID, "cli_a") // matches profile app_id From 65b35bee0f1437f0301ab9c335ca0b6970e0a457 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 18:01:02 +0800 Subject: [PATCH 07/29] feat: surface credentialSource and directCredentialEnv in whoami --- cmd/whoami/whoami.go | 43 +++++++++++++--- cmd/whoami/whoami_test.go | 105 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 12 deletions(-) diff --git a/cmd/whoami/whoami.go b/cmd/whoami/whoami.go index d5b4b387f9..bc4124fa6d 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,16 @@ type whoamiResult struct { TokenStatus string `json:"tokenStatus"` OnBehalfOf *delegatedUser `json:"onBehalfOf,omitempty"` Hint string `json:"hint,omitempty"` + + // CredentialSource, Explicit, DirectCredentialEnv, and Suggestion surface + // the cached credential.IdentitySelection computed during resolution (not + // re-inferred here). CredentialSource can be empty ("") on the non-env + // extension-provider path (e.g. sidecar mode), where no selection kind + // applies; this is a documented, valid state, not an error. + CredentialSource string `json:"credentialSource"` + Explicit bool `json:"explicit"` + DirectCredentialEnv credential.DirectCredentialEnv `json:"directCredentialEnv"` + Suggestion string `json:"suggestion,omitempty"` } // delegatedUser is the user a user-identity acts on behalf of. @@ -97,7 +108,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 +143,24 @@ 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, + Suggestion: selection.Suggestion, } // 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..3768d3ba0a 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,97 @@ 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 §5.1). +const credentialSourceSecret = "s3cr3t-whoami-credential-source" + +// 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.WithProfile("tenant_a", false) // fromFlag=false -> env:LARKSUITE_CLI_PROFILE + + 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 (Task 6): credentialSource, +// explicit, directCredentialEnv, and suggestion. 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 got.Suggestion == "" { + t.Fatalf("suggestion = empty, want non-empty") + } + 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) + } +} From ff3ac0e045491a3357683864183380a244198391 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 18:05:24 +0800 Subject: [PATCH 08/29] refactor: drop whoami suggestion field and IdentitySelection.Suggestion whoami reports facts about the effective identity; it should not proactively push profile-switching guidance at agents. That guidance lives in `profile --help` / the lark-shared skill, and failure recovery already lives in error hints. Remove the now-unused Suggestion field from IdentitySelection and its only setter/consumer. --- cmd/whoami/whoami.go | 8 +++----- cmd/whoami/whoami_test.go | 7 ++----- internal/credential/credential_provider.go | 1 - .../credential_provider_selection_test.go | 13 +++++-------- internal/credential/identity_selection.go | 1 - 5 files changed, 10 insertions(+), 20 deletions(-) diff --git a/cmd/whoami/whoami.go b/cmd/whoami/whoami.go index bc4124fa6d..e6c31ce164 100644 --- a/cmd/whoami/whoami.go +++ b/cmd/whoami/whoami.go @@ -35,15 +35,14 @@ type whoamiResult struct { OnBehalfOf *delegatedUser `json:"onBehalfOf,omitempty"` Hint string `json:"hint,omitempty"` - // CredentialSource, Explicit, DirectCredentialEnv, and Suggestion surface - // the cached credential.IdentitySelection computed during resolution (not - // re-inferred here). CredentialSource can be empty ("") on the non-env + // CredentialSource, Explicit, and DirectCredentialEnv surface the cached + // credential.IdentitySelection computed during resolution (not re-inferred + // here). CredentialSource can be empty ("") on the non-env // extension-provider path (e.g. sidecar mode), where no selection kind // applies; this is a documented, valid state, not an error. CredentialSource string `json:"credentialSource"` Explicit bool `json:"explicit"` DirectCredentialEnv credential.DirectCredentialEnv `json:"directCredentialEnv"` - Suggestion string `json:"suggestion,omitempty"` } // delegatedUser is the user a user-identity acts on behalf of. @@ -160,7 +159,6 @@ func buildResult(cfg *core.CliConfig, as core.Identity, source string, diag iden CredentialSource: string(selection.Source), Explicit: selection.Explicit(), DirectCredentialEnv: selection.DirectCredentialEnv, - Suggestion: selection.Suggestion, } // 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 3768d3ba0a..201a347c90 100644 --- a/cmd/whoami/whoami_test.go +++ b/cmd/whoami/whoami_test.go @@ -375,8 +375,8 @@ func profileSelectionFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer) { // TestWhoamiIncludesCredentialSource locks in the diagnostic fields surfaced // from the cached credential.IdentitySelection (Task 6): credentialSource, -// explicit, directCredentialEnv, and suggestion. whoami must read the cached -// selection as-is, not re-infer it. +// explicit, and directCredentialEnv. whoami must read the cached selection +// as-is, not re-infer it. func TestWhoamiIncludesCredentialSource(t *testing.T) { f, out := profileSelectionFactory(t) @@ -404,9 +404,6 @@ func TestWhoamiIncludesCredentialSource(t *testing.T) { if got.DirectCredentialEnv.Present { t.Fatalf("directCredentialEnv.present = true, want false: %#v", got.DirectCredentialEnv) } - if got.Suggestion == "" { - t.Fatalf("suggestion = empty, want non-empty") - } if !strings.Contains(raw, `"credentialSource": "env:LARKSUITE_CLI_PROFILE"`) { t.Fatalf("raw JSON missing credentialSource literal: %s", raw) } diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 856fac22a0..4a6bd6e474 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -310,7 +310,6 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er WithAppID(app.AppId). WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") } - p.selection.Suggestion = "如需临时覆盖本条命令，使用 --profile。" p.selectedSource = defaultTokenSource{resolver: p.defaultToken} return acct, nil } diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index d0d5178748..fec878f3b9 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -180,7 +180,7 @@ func TestSelection_State4_EnvComplete(t *testing.T) { if !sel.DirectCredentialEnv.Present { t.Errorf("DirectCredentialEnv.Present = false, want true") } - assertNoSecretLeak(t, "state4", string(sel.Source), sel.Suggestion, sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state4", string(sel.Source), sel.DirectCredentialEnv.AppID) assertNoSecretLeak(t, "state4-keys", sel.DirectCredentialEnv.Keys...) } @@ -201,7 +201,7 @@ func TestSelection_State5_ProfileOnly(t *testing.T) { if sel.DirectCredentialEnv.Present { t.Errorf("DirectCredentialEnv.Present = true, want false") } - assertNoSecretLeak(t, "state5", string(sel.Source), sel.Suggestion) + assertNoSecretLeak(t, "state5", string(sel.Source)) } // State #5b: P valid from env (not flag) -> env:LARKSUITE_CLI_PROFILE source. @@ -325,9 +325,6 @@ func TestSelection_State7_UnderlyingErrorContainingSecret_NotLeaked(t *testing.T if strings.Contains(string(sel.Source), secretMarkerValue) { t.Errorf("Selection.Source leaked secret marker: %q", sel.Source) } - if strings.Contains(sel.Suggestion, secretMarkerValue) { - t.Errorf("Selection.Suggestion leaked secret marker: %q", sel.Suggestion) - } if strings.Contains(sel.DirectCredentialEnv.AppID, secretMarkerValue) { t.Errorf("Selection.DirectCredentialEnv.AppID leaked secret marker: %q", sel.DirectCredentialEnv.AppID) } @@ -340,7 +337,7 @@ func TestSelection_State7_UnderlyingErrorContainingSecret_NotLeaked(t *testing.T // doResolveAccount); assert it is zero-valued, which trivially implies no // marker anywhere in it and guards against a future field being populated // from the failed resolution. - if sel.Source != "" || sel.Suggestion != "" || sel.DirectCredentialEnv.Present || + 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) } @@ -366,7 +363,7 @@ func TestSelection_State8_ProfileMatchesEnv(t *testing.T) { if sel.DirectCredentialEnv.AppID != "cli_a" { t.Errorf("DirectCredentialEnv.AppID = %q, want cli_a", sel.DirectCredentialEnv.AppID) } - assertNoSecretLeak(t, "state8", string(sel.Source), sel.Suggestion, sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "state8", string(sel.Source), sel.DirectCredentialEnv.AppID) assertNoSecretLeak(t, "state8-keys", sel.DirectCredentialEnv.Keys...) } @@ -469,7 +466,7 @@ func TestSelection_NonEnvExtensionProviderWinsOverProfile(t *testing.T) { t.Errorf("got profile_app_credential_conflict, want none for non-env provider") } } - assertNoSecretLeak(t, "nonenv-sidecar", string(sel.Source), sel.Suggestion, sel.DirectCredentialEnv.AppID) + assertNoSecretLeak(t, "nonenv-sidecar", string(sel.Source), sel.DirectCredentialEnv.AppID) } // State #1: P none, E none, C present -> config default (currentApp). diff --git a/internal/credential/identity_selection.go b/internal/credential/identity_selection.go index 875c775d7c..bc2dadcc7d 100644 --- a/internal/credential/identity_selection.go +++ b/internal/credential/identity_selection.go @@ -29,7 +29,6 @@ type DirectCredentialEnv struct { type IdentitySelection struct { Source CredentialSourceKind DirectCredentialEnv DirectCredentialEnv - Suggestion string } // Explicit reports whether the identity was actively specified by the From 826d8079a96dc0a9fb06258137034be9c0a06d10 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 18:10:32 +0800 Subject: [PATCH 09/29] docs: add profile selection help to profile and whoami commands --- cmd/profile/profile.go | 7 +++++++ cmd/profile/profile_test.go | 13 +++++++++++++ cmd/whoami/whoami.go | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/cmd/profile/profile.go b/cmd/profile/profile.go index 2216a4f391..83541afe10 100644 --- a/cmd/profile/profile.go +++ b/cmd/profile/profile.go @@ -14,6 +14,13 @@ 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. + +Profile selection: + --profile Use a profile for this command only. + LARKSUITE_CLI_PROFILE Use a profile for the current shell / agent session. + lark-cli whoami --json Show which identity is actually used. + unset LARKSUITE_CLI_PROFILE Clear the session profile and fall back to direct app env or configured default.`, } cmdutil.DisableAuthCheck(cmd) cmdutil.SetTips(cmd, []string{ diff --git a/cmd/profile/profile_test.go b/cmd/profile/profile_test.go index 472a803bf6..3d115286ee 100644 --- a/cmd/profile/profile_test.go +++ b/cmd/profile/profile_test.go @@ -627,6 +627,19 @@ 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, "Profile selection:") { + t.Errorf("profile --help missing Profile selection section") + } + if !strings.Contains(cmd.Long, "LARKSUITE_CLI_PROFILE") { + t.Errorf("profile --help missing LARKSUITE_CLI_PROFILE") + } +} + 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 e6c31ce164..5b1296e9f9 100644 --- a/cmd/whoami/whoami.go +++ b/cmd/whoami/whoami.go @@ -68,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) }, From acc835d5895e2f8518cee79c2dd1475179377ccf Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 18:13:21 +0800 Subject: [PATCH 10/29] docs: add profile selection entry hint to lark-shared skill --- skills/lark-shared/SKILL.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index e78ddd496b..a6f641544b 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -126,6 +126,10 @@ lark-cli auth login --device-code - **不要在同一轮中展示 URL 后立刻执行 `--device-code`**，这会导致用户看不到 URL - **禁止缓存 `verification_url` 或 `device_code`**：每次需要授权时，必须重新执行 `lark-cli auth login --no-wait --json` 生成新的链接。不要将授权链接和 device code 存入上下文供后续复用 +## Profile 选择 + +Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. + ## 更新检查 lark-cli 命令执行后，如果检测到新版本，JSON 输出中会包含 `_notice.update` 字段（含 `message`、`command` 等）。 From 5f2ff3576858aa0292ea75d0fffb323e8d9c47b3 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 19:27:04 +0800 Subject: [PATCH 11/29] fix(credential): add credential_source to config errors and report profile_secret_invalid for broken default secret --- errs/marshal_test.go | 6 ++- errs/types.go | 16 ++++++- errs/types_test.go | 6 ++- internal/credential/credential_provider.go | 44 ++++++++++++++++++- .../credential_provider_selection_test.go | 39 ++++++++++++++++ 5 files changed, 106 insertions(+), 5 deletions(-) diff --git a/errs/marshal_test.go b/errs/marshal_test.go index f7a3d72db5..08c5e51dea 100644 --- a/errs/marshal_test.go +++ b/errs/marshal_test.go @@ -140,7 +140,8 @@ func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { ce := NewConfigError(SubtypeAppCredentialIncomplete, "incomplete"). WithMissingKeys("LARKSUITE_CLI_APP_ID", "LARKSUITE_CLI_APP_SECRET"). WithProfile("work"). - WithAppID("cli_abc") + WithAppID("cli_abc"). + WithCredentialSource("flag:--profile") b, err := json.Marshal(ce) if err != nil { t.Fatal(err) @@ -152,6 +153,7 @@ func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { `"missing_keys":["LARKSUITE_CLI_APP_ID","LARKSUITE_CLI_APP_SECRET"]`, `"profile":"work"`, `"app_id":"cli_abc"`, + `"credential_source":"flag:--profile"`, } { if !strings.Contains(s, want) { t.Errorf("missing %q in %s", want, s) @@ -165,7 +167,7 @@ func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { t.Fatal(err) } s2 := string(b2) - for _, notWant := range []string{`"missing_keys"`, `"profile"`, `"app_id"`} { + for _, notWant := range []string{`"missing_keys"`, `"profile"`, `"app_id"`, `"credential_source"`} { if strings.Contains(s2, notWant) { t.Errorf("%q should be omitted when empty; got %s", notWant, s2) } diff --git a/errs/types.go b/errs/types.go index 37d1d45955..3857e84d30 100644 --- a/errs/types.go +++ b/errs/types.go @@ -327,7 +327,13 @@ type ConfigError struct { MissingKeys []string `json:"missing_keys,omitempty"` Profile string `json:"profile,omitempty"` AppID string `json:"app_id,omitempty"` - Cause error `json:"-"` + // 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 (spec §5) 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. @@ -396,6 +402,14 @@ func (e *ConfigError) WithAppID(appID string) *ConfigError { 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 165b428960..baac931e53 100644 --- a/errs/types_test.go +++ b/errs/types_test.go @@ -648,7 +648,8 @@ func TestBuilderSetter_DefensiveCopy(t *testing.T) { func TestConfigErrorProfileFields(t *testing.T) { e := errs.NewConfigError(errs.SubtypeAppCredentialIncomplete, "incomplete"). - WithMissingKeys("LARKSUITE_CLI_APP_ID") + 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) @@ -656,6 +657,9 @@ func TestConfigErrorProfileFields(t *testing.T) { 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) { diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 4a6bd6e474..d484545995 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -272,6 +272,7 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er return nil, errs.NewConfigError(errs.SubtypeProfileNotFound, "profile %q not found", p.profile). WithProfile(p.profile). + WithCredentialSource(string(p.profileSource())). WithHint("run `lark-cli profile list` to see available profiles.") } if envAcct != nil { @@ -342,10 +343,26 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er if p.defaultAcct != nil { acct, err := p.defaultAcct.ResolveAccount(ctx) if err != nil { - // No usable default identity → no_active_profile. Other typed + // The config default failed to resolve. Distinguish (spec §3 step + // 3.2): a default profile that EXISTS (has an app_id) but whose + // secret cannot be resolved locally is a 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 (e.g. a specific config error) pass through unchanged. if prob, ok := errs.ProblemOf(err); ok && prob.Subtype == errs.SubtypeNotConfigured { + if name, appID, ok := defaultProfileIdentity(); ok { + // SECURITY (§5.1): generic message — never embed the + // underlying error or any secret material. app_id is + // plaintext and safe to echo. + return nil, errs.NewConfigError(errs.SubtypeProfileSecretInvalid, + "profile %q credential could not be resolved locally", name). + WithProfile(name). + WithAppID(appID). + WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") + } return nil, errs.NewConfigError(errs.SubtypeNoActiveProfile, "no active profile"). + WithCredentialSource(noActiveProfileCredentialSource). WithHint("run `lark-cli config init` / `lark-cli profile add`, or set %s.", envvars.CliProfile) } return nil, err @@ -367,6 +384,31 @@ func (p *CredentialProvider) profileSource() CredentialSourceKind { return SourceEnvProfile } +// noActiveProfileCredentialSource is the credential_source reported on the +// no_active_profile error. Spec §5 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" + +// defaultProfileIdentity reports the config default profile's display name and +// app_id when a usable default profile actually EXISTS (currentApp > firstApp +// resolves to an app with a non-empty app_id). It never touches the keychain or +// any secret, so it can distinguish "default profile exists but its secret is +// broken" (→ profile_secret_invalid) from "no usable default profile at all" +// (→ no_active_profile), without risking a secret leak (§5.1). +func defaultProfileIdentity() (name, appID string, ok bool) { + multi, err := core.LoadMultiAppConfig() + if err != nil || multi == nil { + return "", "", false + } + app := multi.CurrentAppConfig("") + if app == nil || app.AppId == "" { + return "", "", false + } + return app.ProfileName(), app.AppId, true +} + // selectionSourceForDefault reports whether the config default resolved to the // explicit currentApp or fell back to the first app (spec §3 step 3.2). func selectionSourceForDefault(multi *core.MultiAppConfig) CredentialSourceKind { diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index fec878f3b9..8b06524039 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -135,9 +135,42 @@ func TestSelection_State2_NoActiveProfile(t *testing.T) { if got := subtypeOf(t, err); got != errs.SubtypeNoActiveProfile { t.Fatalf("subtype = %q, want %q", got, errs.SubtypeNoActiveProfile) } + // Defect 1 (spec §5): 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 a broken secret: P none, E none, C present but the +// default profile's keychain secret ref is corrupted. Per spec §3 step 3.2 this +// must be profile_secret_invalid (the identity IS configured, only its secret is +// broken) — NOT no_active_profile (which is reserved for "no usable default"). +func TestSelection_ConfigDefaultBrokenSecret_ProfileSecretInvalid(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + writeConfigTenantABroken(t) // CurrentApp = tenant_a (app_id cli_a), broken keychain ref + cp := newProvider(t, "", false) + + _, 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) + } + // §5.1: generic message, no cause, no secret anywhere. + if errors.Unwrap(ce) != nil { + t.Errorf("profile_secret_invalid must not attach a cause, got %v", errors.Unwrap(ce)) + } + assertNoSecretLeak(t, "config-default-broken", ce.Message, ce.Hint, ce.AppID) +} + // State #3: P none, E partial (only APP_ID) -> app_credential_incomplete. func TestSelection_State3_EnvPartial(t *testing.T) { t.Setenv(envvars.CliAppID, "cli_env") @@ -232,6 +265,12 @@ func TestSelection_State6_ProfileNotFound(t *testing.T) { t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileNotFound) } prob, _ := errs.ProblemOf(err) + // Defect 1 (spec §5): 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)) } From c71434d0facc2f3df427210225627aeaebe3b181 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:02:42 +0800 Subject: [PATCH 12/29] docs: forbid hollow identity promises in lark-shared profile hint --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index a6f641544b..6000161f9d 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. +Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. When a task or session should use a specific identity, actually set `LARKSUITE_CLI_PROFILE` (or ask the user for the profile name if it is unknown) — never merely say you will use it, or later commands silently run as the wrong identity. ## 更新检查 From 92c43b3069e35e5ed165c41527c8f21127519003 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:16:01 +0800 Subject: [PATCH 13/29] docs: broaden lark-shared trigger to profile selection and add session-env persistence guidance --- skills/lark-shared/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 6000161f9d..5f02a9a377 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/profile-selection tasks: auth login/status/logout, user vs bot identity, business-domain permissions (--domain, including all/docs/drive), missing scopes, revoking authorization, handling _notice JSON, profile/tenant/app-identity selection, or any request to make this task/session (or all following lark-cli commands) run under a specific profile/tenant — via LARKSUITE_CLI_PROFILE, --profile, unset LARKSUITE_CLI_PROFILE, or whoami identity diagnostics." --- # lark-cli 共享规则 @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. When a task or session should use a specific identity, actually set `LARKSUITE_CLI_PROFILE` (or ask the user for the profile name if it is unknown) — never merely say you will use it, or later commands silently run as the wrong identity. +Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. When a task or session should use a specific identity, actually establish it (or ask the user for the profile name if it is unknown) — never merely say you will use it, or later commands silently run as the wrong identity. To make a session profile actually take effect: if your shell/runtime persists env across commands, `export LARKSUITE_CLI_PROFILE=` once; if each command runs in a fresh process, prefix every subsequent lark-cli command with `LARKSUITE_CLI_PROFILE= lark-cli ...`. Then confirm with `lark-cli whoami --json` (`credentialSource` should read `env:LARKSUITE_CLI_PROFILE`). ## 更新检查 From 66100b713caf841aff76d80b15417dc4e8b18421 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:23:34 +0800 Subject: [PATCH 14/29] docs: trim lark-shared profile rule to lark-cli scope, drop agent-shell env mechanics --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 5f02a9a377..9a666901af 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: lark-cli supports temporary profile selection and identity diagnostics. For profile, tenant, or app-identity requests, check `lark-cli profile --help`; when identity looks wrong, run `lark-cli whoami --json`. Use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` when the task no longer wants the session profile. Do not run `lark-cli profile use` unless the user explicitly asks to change the long-term default. When a task or session should use a specific identity, actually establish it (or ask the user for the profile name if it is unknown) — never merely say you will use it, or later commands silently run as the wrong identity. To make a session profile actually take effect: if your shell/runtime persists env across commands, `export LARKSUITE_CLI_PROFILE=` once; if each command runs in a fresh process, prefix every subsequent lark-cli command with `LARKSUITE_CLI_PROFILE= lark-cli ...`. Then confirm with `lark-cli whoami --json` (`credentialSource` should read `env:LARKSUITE_CLI_PROFILE`). +Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. For identity diagnostics, run `lark-cli whoami --json`. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. ## 更新检查 From 62405d6bbd378b87bbba76a7dbc968e18b5e2aeb Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:32:39 +0800 Subject: [PATCH 15/29] docs: clarify whoami vs auth status boundary in lark-shared profile rule --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 9a666901af..e2787dab82 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. For identity diagnostics, run `lark-cli whoami --json`. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. +Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. `whoami` shows the effective app/profile identity; `auth status --json --verify` shows OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. ## 更新检查 From feaf123aa980818eb617f21a6e9579ef80383159 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:40:58 +0800 Subject: [PATCH 16/29] docs: use imperative one-line whoami vs auth status routing boundary --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index e2787dab82..33eb7f3a88 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. `whoami` shows the effective app/profile identity; `auth status --json --verify` shows OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. +Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. Use `whoami --json` for effective app/profile identity; use `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. ## 更新检查 From 40b3087c2a68dd0e80f0c009e58546d78ca22a10 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 6 Jul 2026 20:53:24 +0800 Subject: [PATCH 17/29] feat: distinguish auth status from whoami identity in help and skill --- cmd/auth/status.go | 2 ++ cmd/auth/status_test.go | 14 ++++++++++++++ skills/lark-shared/SKILL.md | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/auth/status.go b/cmd/auth/status.go index 15de7dda9c..da48f07895 100644 --- a/cmd/auth/status.go +++ b/cmd/auth/status.go @@ -27,6 +27,8 @@ 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. +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..12b6d69761 100644 --- a/cmd/auth/status_test.go +++ b/cmd/auth/status_test.go @@ -6,6 +6,7 @@ package auth import ( "encoding/json" "net/http" + "strings" "testing" "github.com/larksuite/cli/internal/cmdutil" @@ -13,6 +14,19 @@ import ( "github.com/larksuite/cli/internal/httpmock" ) +func TestAuthStatusHelpDistinguishesFromWhoami(t *testing.T) { + cmd := NewCmdAuthStatus(nil, nil) + for _, want := range []string{ + "OAuth user login", + "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, diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 33eb7f3a88..517613d030 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. Use `whoami --json` for effective app/profile identity; use `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. +Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. Use `whoami` for effective app/profile identity and `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. ## 更新检查 From bb82140d600a4c3b8b84d25e78d342b7289ebe87 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 11:22:54 +0800 Subject: [PATCH 18/29] docs: guide per-command LARKSUITE_CLI_PROFILE prefix for non-persistent shells --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 517613d030..b812357e8a 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `LARKSUITE_CLI_PROFILE` for a task/session profile, `--profile` for one command, and `unset LARKSUITE_CLI_PROFILE` to clear it. Use `whoami` for effective app/profile identity and `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. If a task needs a specific identity, actually set it; ask for the profile name if unknown. Do not merely say which profile you will use. +Profile selection: use `--profile` for one command; for a task/session, prefix later `lark-cli` commands with `LARKSUITE_CLI_PROFILE=` unless shell env persists, where you may `export` once and later `unset`. Ask for the profile name if unknown; do not merely say which profile you will use. Use `whoami` for effective app/profile identity and `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. ## 更新检查 From 6fd370a532eb443556076505efc746c86651c9f7 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 11:27:34 +0800 Subject: [PATCH 19/29] docs: point to auth status --json --verify for token validity checks --- cmd/auth/status.go | 1 + cmd/auth/status_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/cmd/auth/status.go b/cmd/auth/status.go index da48f07895..b8b4492926 100644 --- a/cmd/auth/status.go +++ b/cmd/auth/status.go @@ -28,6 +28,7 @@ func NewCmdAuthStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobr 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 { diff --git a/cmd/auth/status_test.go b/cmd/auth/status_test.go index 12b6d69761..3f5ed466bd 100644 --- a/cmd/auth/status_test.go +++ b/cmd/auth/status_test.go @@ -18,6 +18,7 @@ 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", } { From 5ef56f0fc82a1e2fc90d3611e6eeed4c9fb46eca Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 11:28:13 +0800 Subject: [PATCH 20/29] docs: require auth status --json --verify for login/token checks --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index b812357e8a..f7fa148fa3 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -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` | From d87a69562b4f935bd19e79a82d467b7fd9e3f396 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 14:47:16 +0800 Subject: [PATCH 21/29] test(credential): use recognized placeholders for secret fixtures Replace credential-shaped literals in whoami and selection tests with placeholder values recognized by the public-content quality gate (test-secret / your-secret / your-password / your-access-token), so the deterministic public-content scan does not flag test fixtures as generic credentials. No behavioral change; the fixtures are only compared for non-leakage and identity arbitration. --- cmd/whoami/whoami_test.go | 2 +- internal/credential/credential_provider_selection_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/whoami/whoami_test.go b/cmd/whoami/whoami_test.go index 201a347c90..36cc7b3dff 100644 --- a/cmd/whoami/whoami_test.go +++ b/cmd/whoami/whoami_test.go @@ -333,7 +333,7 @@ func (noopWhoamiKeychain) Remove(service, account string) error { return // credentialSourceSecret is the profile secret written to config for // TestWhoamiIncludesCredentialSource. It must never leak into whoami's output // (security §5.1). -const credentialSourceSecret = "s3cr3t-whoami-credential-source" +const credentialSourceSecret = "test-secret" // profileSelectionFactory builds a Factory whose CredentialProvider resolves // an explicit profile ("tenant_a") supplied via the LARKSUITE_CLI_PROFILE env diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index 8b06524039..cd4fd88005 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -40,10 +40,10 @@ func asValidationError(t *testing.T, err error) *errs.ValidationError { // secretValue is the profile secret written to config. It must NEVER appear in // any error message or IdentitySelection (security §5.1). -const secretValue = "s3cr3t-tenant-a-value" +const secretValue = "your-secret" // envSecretValue is the direct env app secret. Same no-leak guarantee. -const envSecretValue = "env-secret-should-not-leak" +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 @@ -302,7 +302,7 @@ func TestSelection_State7_ProfileSecretInvalid(t *testing.T) { // 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 = "SUPER_SECRET_MARKER_abc123" +const secretMarkerValue = "your-access-token" // leakingSecretResolver is a DefaultAccountResolver stub whose ResolveAccount // fails with an error whose message contains secretMarkerValue, simulating a From 87e989d0205a10b216b48048d57fedbc3bbbb2fc Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 14:47:16 +0800 Subject: [PATCH 22/29] docs(lark-shared): refine profile-selection guidance Clarify that --profile and LARKSUITE_CLI_PROFILE accept either a profile name or an app_id, keep the effective-identity vs OAuth-token boundary (whoami vs auth status --json --verify), and note not to set direct app-credential env vars unless direct credentials are provided. --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index f7fa148fa3..b1ed701e7c 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `--profile` for one command; for a task/session, prefix later `lark-cli` commands with `LARKSUITE_CLI_PROFILE=` unless shell env persists, where you may `export` once and later `unset`. Ask for the profile name if unknown; do not merely say which profile you will use. Use `whoami` for effective app/profile identity and `auth status --json --verify` for OAuth login/token state. Do not run `lark-cli profile use` unless the user asks to change the long-term default. +Profile selection: use `--profile ` for one command; for a task/session, prefix later `lark-cli` commands with `LARKSUITE_CLI_PROFILE=` unless shell env persists, where you may `export` once and later `unset`. Ask if the selector is unknown; do not merely promise. Use `whoami` for the effective app/profile identity and `auth status --json --verify` for OAuth token state. Do not run `lark-cli profile use` unless changing the long-term default, and do not set `LARKSUITE_CLI_APP_ID`/`LARKSUITE_CLI_APP_SECRET` unless direct credentials are provided. ## 更新检查 From 74629fd86cafb01f4dcf72743164f99450fe55ac Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 15:33:44 +0800 Subject: [PATCH 23/29] fix(credential): propagate malformed-config error for explicit profile When an explicit profile was requested and LoadMultiAppConfig failed, the error was discarded and every failure reported as profile_not_found, masking a real config problem (e.g. malformed file) behind a misleading "run profile list" hint. Propagate the underlying error when it is a malformed-config failure (errors.Is ErrMalformedConfig) so errors.Is / errors.Unwrap keep working, mirroring the no-profile branch. An absent config is not malformed and still yields the friendly profile_not_found. --- internal/credential/credential_provider.go | 9 ++++++ .../credential_provider_selection_test.go | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index d484545995..bc57a17146 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -264,6 +264,15 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er // Step 2 (spec §3): an explicit profile was requested. if p.profile != "" { multi, loadErr := core.LoadMultiAppConfig() + if errors.Is(loadErr, core.ErrMalformedConfig) { + // A malformed config must not be masked as profile_not_found (which + // would tell the user to run `profile list` and hide a real config + // problem). Pass the underlying error through unchanged so + // errors.Is / errors.Unwrap keep working. An absent config is not + // malformed and still falls through to the friendly + // profile_not_found below, since the requested profile cannot exist. + return nil, loadErr + } var app *core.AppConfig if loadErr == nil && multi != nil { app = multi.FindApp(p.profile) diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index cd4fd88005..0e6db0e12a 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "os" "slices" "strings" "testing" @@ -171,6 +172,34 @@ func TestSelection_ConfigDefaultBrokenSecret_ProfileSecretInvalid(t *testing.T) assertNoSecretLeak(t, "config-default-broken", ce.Message, ce.Hint, ce.AppID) } +// Explicit profile requested but the config file is malformed. The load error +// must be propagated (errors.Is ErrMalformedConfig) rather than masked as +// profile_not_found, which would hide a real config problem and misdirect the +// user to `profile list`. An absent config is separately still profile_not_found. +func TestSelection_ExplicitProfile_MalformedConfig_PropagatesError(t *testing.T) { + 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) + } + cp := newProvider(t, "tenant_a", true) + + _, err := cp.Selection(context.Background()) + if err == nil { + t.Fatalf("expected error for malformed config, got nil") + } + if !errors.Is(err, core.ErrMalformedConfig) { + t.Fatalf("malformed config error not propagated: %v", err) + } + if prob, ok := errs.ProblemOf(err); ok && prob.Subtype == errs.SubtypeProfileNotFound { + t.Fatalf("malformed config masked as profile_not_found") + } +} + // State #3: P none, E partial (only APP_ID) -> app_credential_incomplete. func TestSelection_State3_EnvPartial(t *testing.T) { t.Setenv(envvars.CliAppID, "cli_env") From 0547d4063f40e09b432cbbfd9ab6b46ad96b68af Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 7 Jul 2026 20:15:19 +0800 Subject: [PATCH 24/29] feat(profile): distinguish saved default from effective identity profile list / config show only report the saved default profile, not the app/profile a specific invocation actually resolves to (especially under --profile or LARKSUITE_CLI_PROFILE). Rename the misleading profile list JSON field active -> default (it is the configured default, not the one in effect), and point config show / profile list / profile help at lark-cli whoami --json for the identity actually used now. Restructure the lark-shared skill profile guidance as an intent -> command table. --- cmd/config/config_test.go | 10 ++++++++++ cmd/config/show.go | 3 ++- cmd/profile/list.go | 15 ++++++++------- cmd/profile/profile.go | 4 +++- cmd/profile/profile_test.go | 31 +++++++++++++++++++++++++++---- skills/lark-shared/SKILL.md | 2 +- 6 files changed, 51 insertions(+), 14 deletions(-) diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go index 9f0f3da62c..96b724454c 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()) diff --git a/cmd/config/show.go b/cmd/config/show.go index 5526f0254a..9e3db054cc 100644 --- a/cmd/config/show.go +++ b/cmd/config/show.go @@ -27,7 +27,8 @@ 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`.", RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { return runF(opts) diff --git a/cmd/profile/list.go b/cmd/profile/list.go index 5b0234c2c9..df4d607c4b 100644 --- a/cmd/profile/list.go +++ b/cmd/profile/list.go @@ -21,7 +21,7 @@ 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 +30,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 +54,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 +67,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 83541afe10..f83b61d8fd 100644 --- a/cmd/profile/profile.go +++ b/cmd/profile/profile.go @@ -17,9 +17,11 @@ func NewCmdProfile(f *cmdutil.Factory) *cobra.Command { Long: `Profiles are named app identities managed by lark-cli. Profile selection: + lark-cli whoami --json Show the app/profile lark-cli is using now. + lark-cli auth status --json 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. - lark-cli whoami --json Show which identity is actually used. + 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.`, } cmdutil.DisableAuthCheck(cmd) diff --git a/cmd/profile/profile_test.go b/cmd/profile/profile_test.go index 3d115286ee..e9b539b837 100644 --- a/cmd/profile/profile_test.go +++ b/cmd/profile/profile_test.go @@ -306,14 +306,21 @@ 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() + if strings.Contains(raw, `"active"`) { + t.Fatalf("profile list output contains legacy 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]) } } @@ -638,6 +645,22 @@ func TestProfileHelpHasSelectionSection(t *testing.T) { 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") + } +} + +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) { diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index b1ed701e7c..e9c5061b46 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: use `--profile ` for one command; for a task/session, prefix later `lark-cli` commands with `LARKSUITE_CLI_PROFILE=` unless shell env persists, where you may `export` once and later `unset`. Ask if the selector is unknown; do not merely promise. Use `whoami` for the effective app/profile identity and `auth status --json --verify` for OAuth token state. Do not run `lark-cli profile use` unless changing the long-term default, and do not set `LARKSUITE_CLI_APP_ID`/`LARKSUITE_CLI_APP_SECRET` unless direct credentials are provided. +Profile selection: current state -> `whoami --json`; OAuth/token -> `auth status --json --verify`; agent task -> add `--profile ` to every `lark-cli` command; same-shell script/batch -> `LARKSUITE_CLI_PROFILE=` or export/unset; saved config -> `config show` / `profile list`; long-term default -> `profile use`. Ask if the profile is ambiguous; do not set `LARKSUITE_CLI_APP_ID`/`LARKSUITE_CLI_APP_SECRET` unless direct credentials are provided. ## 更新检查 From c76cac8ea226a752a8e01e892da7ea2139ce6a73 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Wed, 8 Jul 2026 10:52:37 +0800 Subject: [PATCH 25/29] docs(lark-shared): write profile-selection guidance in Chinese Match the rest of the skill document, which is in Chinese. Keep command and env-var tokens in English. Content unchanged. --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index e9c5061b46..583f83362e 100644 --- a/skills/lark-shared/SKILL.md +++ b/skills/lark-shared/SKILL.md @@ -128,7 +128,7 @@ lark-cli auth login --device-code ## Profile 选择 -Profile selection: current state -> `whoami --json`; OAuth/token -> `auth status --json --verify`; agent task -> add `--profile ` to every `lark-cli` command; same-shell script/batch -> `LARKSUITE_CLI_PROFILE=` or export/unset; saved config -> `config show` / `profile list`; long-term default -> `profile use`. Ask if the profile is ambiguous; do not set `LARKSUITE_CLI_APP_ID`/`LARKSUITE_CLI_APP_SECRET` unless direct credentials are provided. +Profile 选择：查当前实际生效身份 → `whoami --json`；查 OAuth 登录 / token 有效性 → `auth status --json --verify`；agent 任务 → 每条 `lark-cli` 命令都加 `--profile `；同一 shell 的脚本 / 批处理 → 用 `LARKSUITE_CLI_PROFILE=` 或 export/unset；查已保存的配置（非当前生效）→ `config show` / `profile list`；永久改默认 → `profile use`。profile 不明确时先问用户；除非用户提供了直连凭证，否则不要设置 `LARKSUITE_CLI_APP_ID` / `LARKSUITE_CLI_APP_SECRET`。 ## 更新检查 From c15b8118035c87e9f8b52d8f1c2f8bead42d862c Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Mon, 13 Jul 2026 12:09:12 +0800 Subject: [PATCH 26/29] fix(credential): surface malformed config on the config-default path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config-default (no-profile) path loaded config via LoadMultiAppConfig and coerced any error into NotConfiguredError, so a malformed config was misreported as no_active_profile ("run config init") — risking overwrite of a recoverable-but-broken config. Route both the explicit-profile and config-default paths through core.LoadOrNotConfigured so a malformed config consistently surfaces as invalid_config (exit 3, cause preserved), while a genuinely absent config keeps the friendly profile_not_found / no_active_profile. --- internal/credential/credential_provider.go | 23 +++++---- .../credential_provider_selection_test.go | 49 ++++++++++++++----- internal/credential/default_provider.go | 8 ++- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index bc57a17146..952af8f0f9 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -263,18 +263,21 @@ func (p *CredentialProvider) doResolveAccount(ctx context.Context) (*Account, er // Step 2 (spec §3): an explicit profile was requested. if p.profile != "" { - multi, loadErr := core.LoadMultiAppConfig() - if errors.Is(loadErr, core.ErrMalformedConfig) { - // A malformed config must not be masked as profile_not_found (which - // would tell the user to run `profile list` and hide a real config - // problem). Pass the underlying error through unchanged so - // errors.Is / errors.Unwrap keep working. An absent config is not - // malformed and still falls through to the friendly - // profile_not_found below, since the requested profile cannot exist. - return nil, loadErr + multi, loadErr := core.LoadOrNotConfigured() + if loadErr != nil { + // A malformed / unreadable config must surface its real (typed) + // cause — invalid_config — not be masked as profile_not_found (which + // would send the user to `profile list` and hide a broken file). + // Only a genuinely absent config (not_configured) falls through to + // the friendly profile_not_found, since the requested profile + // cannot exist. This mirrors the config-default path. + if prob, ok := errs.ProblemOf(loadErr); !ok || prob.Subtype != errs.SubtypeNotConfigured { + return nil, loadErr + } + multi = nil } var app *core.AppConfig - if loadErr == nil && multi != nil { + if multi != nil { app = multi.FindApp(p.profile) } if app == nil { diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index 0e6db0e12a..f56f7e472e 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -172,11 +172,10 @@ func TestSelection_ConfigDefaultBrokenSecret_ProfileSecretInvalid(t *testing.T) assertNoSecretLeak(t, "config-default-broken", ce.Message, ce.Hint, ce.AppID) } -// Explicit profile requested but the config file is malformed. The load error -// must be propagated (errors.Is ErrMalformedConfig) rather than masked as -// profile_not_found, which would hide a real config problem and misdirect the -// user to `profile list`. An absent config is separately still profile_not_found. -func TestSelection_ExplicitProfile_MalformedConfig_PropagatesError(t *testing.T) { +// 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()) @@ -186,20 +185,48 @@ func TestSelection_ExplicitProfile_MalformedConfig_PropagatesError(t *testing.T) if err := os.WriteFile(core.GetConfigPath(), []byte("{ this is not valid json"), 0o600); err != nil { t.Fatalf("write malformed config: %v", err) } - cp := newProvider(t, "tenant_a", true) +} - _, err := cp.Selection(context.Background()) +// 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") } - if !errors.Is(err, core.ErrMalformedConfig) { - t.Fatalf("malformed config error not propagated: %v", err) + prob, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("malformed config error is not typed: %v", err) } - if prob, ok := errs.ProblemOf(err); ok && prob.Subtype == errs.SubtypeProfileNotFound { - t.Fatalf("malformed config masked as profile_not_found") + 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) +} + // State #3: P none, E partial (only APP_ID) -> app_credential_incomplete. func TestSelection_State3_EnvPartial(t *testing.T) { t.Setenv(envvars.CliAppID, "cli_env") diff --git a/internal/credential/default_provider.go b/internal/credential/default_provider.go index d7401b62ed..d88c572b77 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) From e6f8b8a05df28da1c857f1a74af16e580161b055 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 14 Jul 2026 16:28:01 +0800 Subject: [PATCH 27/29] fix(credential): make profile arbitration explicit --- cmd/auth/status_test.go | 50 ++ cmd/profile/profile.go | 4 +- cmd/profile/profile_test.go | 4 + cmd/whoami/whoami.go | 6 +- cmd/whoami/whoami_test.go | 6 +- errs/marshal_test.go | 4 +- errs/types.go | 14 +- extension/credential/env/env.go | 119 +++- extension/credential/env/env_test.go | 97 ++- extension/credential/types.go | 38 +- internal/cmdutil/factory_default.go | 7 +- internal/cmdutil/factory_test.go | 65 ++ internal/credential/credential_provider.go | 605 +++++++++++------- .../credential_provider_selection_test.go | 402 ++++++++++-- internal/credential/decide_test.go | 181 ++++++ internal/credential/identity_selection.go | 13 +- internal/envvars/envvars.go | 2 +- 17 files changed, 1279 insertions(+), 338 deletions(-) create mode 100644 internal/credential/decide_test.go diff --git a/cmd/auth/status_test.go b/cmd/auth/status_test.go index 3f5ed466bd..bdcff2c941 100644 --- a/cmd/auth/status_test.go +++ b/cmd/auth/status_test.go @@ -4,13 +4,18 @@ 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" ) @@ -94,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("profile-secret"), + Brand: core.BrandFeishu, + }}, + }); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + config := &core.CliConfig{ProfileName: "tenant_a", AppID: "cli_a", AppSecret: "profile-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/profile/profile.go b/cmd/profile/profile.go index f83b61d8fd..4f29761260 100644 --- a/cmd/profile/profile.go +++ b/cmd/profile/profile.go @@ -22,7 +22,9 @@ Profile selection: --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.`, + 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 e9b539b837..3291cccb12 100644 --- a/cmd/profile/profile_test.go +++ b/cmd/profile/profile_test.go @@ -651,6 +651,10 @@ func TestProfileHelpHasSelectionSection(t *testing.T) { 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) { diff --git a/cmd/whoami/whoami.go b/cmd/whoami/whoami.go index 5b1296e9f9..6b5b973b70 100644 --- a/cmd/whoami/whoami.go +++ b/cmd/whoami/whoami.go @@ -37,9 +37,9 @@ type whoamiResult struct { // CredentialSource, Explicit, and DirectCredentialEnv surface the cached // credential.IdentitySelection computed during resolution (not re-inferred - // here). CredentialSource can be empty ("") on the non-env - // extension-provider path (e.g. sidecar mode), where no selection kind - // applies; this is a documented, valid state, not an error. + // 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"` diff --git a/cmd/whoami/whoami_test.go b/cmd/whoami/whoami_test.go index 36cc7b3dff..6d1b29a6ef 100644 --- a/cmd/whoami/whoami_test.go +++ b/cmd/whoami/whoami_test.go @@ -332,7 +332,7 @@ func (noopWhoamiKeychain) Remove(service, account string) error { return // credentialSourceSecret is the profile secret written to config for // TestWhoamiIncludesCredentialSource. It must never leak into whoami's output -// (security §5.1). +// (security: never leak a secret). const credentialSourceSecret = "test-secret" // profileSelectionFactory builds a Factory whose CredentialProvider resolves @@ -361,7 +361,7 @@ func profileSelectionFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer) { defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return noopWhoamiKeychain{} }, "tenant_a") cred := credential.NewCredentialProvider([]extcred.Provider{&envprovider.Provider{}}, defaultAcct, nil, nil) - cred.WithProfile("tenant_a", false) // fromFlag=false -> env:LARKSUITE_CLI_PROFILE + cred.WithProfileFromEnv("tenant_a") cfg := &core.CliConfig{ProfileName: "tenant_a", AppID: "cli_a", AppSecret: credentialSourceSecret, Brand: core.BrandFeishu} out := &bytes.Buffer{} @@ -374,7 +374,7 @@ func profileSelectionFactory(t *testing.T) (*cmdutil.Factory, *bytes.Buffer) { } // TestWhoamiIncludesCredentialSource locks in the diagnostic fields surfaced -// from the cached credential.IdentitySelection (Task 6): credentialSource, +// 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) { diff --git a/errs/marshal_test.go b/errs/marshal_test.go index 08c5e51dea..c9de3983c6 100644 --- a/errs/marshal_test.go +++ b/errs/marshal_test.go @@ -139,6 +139,7 @@ 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") @@ -151,6 +152,7 @@ func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { `"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"`, @@ -167,7 +169,7 @@ func TestConfigError_ProfileFieldsMarshalJSON(t *testing.T) { t.Fatal(err) } s2 := string(b2) - for _, notWant := range []string{`"missing_keys"`, `"profile"`, `"app_id"`, `"credential_source"`} { + 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) } diff --git a/errs/types.go b/errs/types.go index 3857e84d30..f37888e778 100644 --- a/errs/types.go +++ b/errs/types.go @@ -323,10 +323,11 @@ func (e *PermissionError) WithCause(cause error) *PermissionError { // intentionally not serialized. type ConfigError struct { Problem - Field string `json:"field,omitempty"` - MissingKeys []string `json:"missing_keys,omitempty"` - Profile string `json:"profile,omitempty"` - AppID string `json:"app_id,omitempty"` + 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 @@ -392,6 +393,11 @@ func (e *ConfigError) WithMissingKeys(keys ...string) *ConfigError { 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 diff --git a/extension/credential/env/env.go b/extension/credential/env/env.go index 5458125689..c224c7ea66 100644 --- a/extension/credential/env/env.go +++ b/extension/credential/env/env.go @@ -23,55 +23,39 @@ 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), } } - // 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{ @@ -80,6 +64,44 @@ func (p *Provider) ResolveAccount(ctx context.Context) (*credential.Account, err } } + 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 { case hasUAT: @@ -92,6 +114,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..799ac633ed 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,78 @@ 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.BlockReasonCredentialIncomplete { + t.Fatalf("invalid %s was hidden by credential incompleteness: %+v", tt.name, blockErr) + } + if !strings.Contains(blockErr.Reason, tt.key) { + t.Fatalf("reason = %q, want %s", blockErr.Reason, tt.key) + } + }) } } @@ -258,6 +335,9 @@ func TestResolveAccount_InvalidStrictModeRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } + if blockErr.Code == credential.BlockReasonCredentialIncomplete { + t.Fatalf("invalid strict mode must not be classified as credential incomplete: %+v", blockErr) + } if !strings.Contains(err.Error(), envvars.CliStrictMode) { t.Fatalf("error = %v, want mention of %s", err, envvars.CliStrictMode) } @@ -276,6 +356,9 @@ func TestResolveAccount_InvalidDefaultAsRejected(t *testing.T) { if !errors.As(err, &blockErr) { t.Fatalf("expected BlockError, got %T", err) } + if blockErr.Code == credential.BlockReasonCredentialIncomplete { + t.Fatalf("invalid default-as must not be classified as credential incomplete: %+v", blockErr) + } if !strings.Contains(err.Error(), envvars.CliDefaultAs) { t.Fatalf("error = %v, want mention of %s", err, envvars.CliDefaultAs) } diff --git a/extension/credential/types.go b/extension/credential/types.go index 209013fda5..0c5d5f27ed 100644 --- a/extension/credential/types.go +++ b/extension/credential/types.go @@ -44,6 +44,20 @@ 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 (e.g. the env + // provider's LARKSUITE_CLI_* variables). It participates in profile + // arbitration and conflict detection instead of winning outright. + AccountDirect +) + // Account holds resolved app credentials and configuration. type Account struct { AppID string @@ -53,6 +67,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 +91,30 @@ 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 direct credential inputs that cannot + // form an account until one or more named input variables are fixed. Setting + // it opts the block into direct-credential arbitration regardless of the + // provider's name: the caller maps it to app_credential_incomplete and may + // let a matching selected profile win instead (when AppID and PresentKeys + // identify a usable app_id). Blocks without it propagate unchanged. + BlockReasonCredentialIncomplete BlockReason = "credential_incomplete" +) + // 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 } func (e *BlockError) Error() string { diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index 6145d5ada0..98ce5f2930 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -180,6 +180,9 @@ 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). - WithProfile(deps.Profile, deps.ProfileFromFlag) + cred := credential.NewCredentialProvider(providers, defaultAcct, defaultToken, deps.HttpClient) + 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..6aded5cbe4 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -405,6 +405,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 +456,63 @@ func TestRequireBuiltinCredentialProvider_AllowsBuiltinProvider(t *testing.T) { } } +func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + if err := core.SaveMultiAppConfig(&core.MultiAppConfig{ + CurrentApp: "tenant_a", + Apps: []core.AppConfig{{ + Name: "tenant_a", + AppId: "cli_a", + AppSecret: core.PlainSecret("profile-secret"), + Brand: core.BrandFeishu, + }}, + }); err != nil { + t.Fatalf("SaveMultiAppConfig: %v", err) + } + + stub := &stubExtProvider{name: "env", err: &extcred.BlockError{ + Provider: "env", + Reason: "APP_ID is set but no credential is available", + Code: extcred.BlockReasonCredentialIncomplete, + RequiredAnyOf: []string{envvars.CliAppSecret, envvars.CliUserAccessToken, envvars.CliTenantAccessToken}, + PresentKeys: []string{envvars.CliAppID}, + AppID: "cli_a", + }} + cred := credential.NewCredentialProvider( + []extcred.Provider{stub}, + &stubDefaultAccountResolver{acct: &credential.Account{AppID: "cli_a", AppSecret: "profile-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) + } +} + func TestRequireBuiltinCredentialProvider_NilCredential(t *testing.T) { f, _, _, _ := TestFactory(t, nil) f.Credential = nil diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 952af8f0f9..0bd4026209 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -10,6 +10,8 @@ import ( "io" "net/http" "os" + "slices" + "strings" "sync" "github.com/larksuite/cli/errs" @@ -19,11 +21,6 @@ import ( "github.com/larksuite/cli/internal/envvars" ) -// directCredentialProviderName is the Name() of the env provider, the source -// of direct app credentials (LARKSUITE_CLI_APP_ID / _APP_SECRET). Only its -// incomplete blocks map to app_credential_incomplete (spec §3 step 1). -const directCredentialProviderName = "env" - // DefaultAccountResolver is implemented by the default account provider. type DefaultAccountResolver interface { ResolveAccount(ctx context.Context) (*Account, error) @@ -144,17 +141,18 @@ type CredentialProvider struct { httpClient func() (*http.Client, error) warnOut io.Writer - // profile is the active profile (from --profile or LARKSUITE_CLI_PROFILE). - // profileFromFlag discriminates the source for the reported selection. - profile string - profileFromFlag bool + // 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 (§5.1). + // doResolveAccount under accountOnce. It never carries a secret. selection IdentitySelection hintOnce sync.Once @@ -177,12 +175,20 @@ func (p *CredentialProvider) SetWarnOut(warnOut io.Writer) *CredentialProvider { return p } -// WithProfile records the active profile and whether it came from the -// --profile flag (as opposed to the LARKSUITE_CLI_PROFILE env fallback). +// WithProfileFromFlag records the --profile flag value as the active profile. // It governs credential arbitration and the reported selection source. -func (p *CredentialProvider) WithProfile(profile string, fromFlag bool) *CredentialProvider { +func (p *CredentialProvider) WithProfileFromFlag(profile string) *CredentialProvider { p.profile = profile - p.profileFromFlag = fromFlag + 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 } @@ -197,232 +203,362 @@ func (p *CredentialProvider) ResolveAccount(ctx context.Context) (*Account, erro return p.account, p.accountErr } -// doResolveAccount arbitrates the credential/App selection per the spec -// resolution order (§3): env-partial → profile → env-complete → config default. -// It populates p.selection (no secret; §5.1) and p.selectedSource on every -// success path. +// 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) { - // Step 1 (spec §3): consult the extension providers. The env provider is - // the "direct app credential" source. An incomplete direct credential - // (only APP_ID or only APP_SECRET set) short-circuits to - // app_credential_incomplete regardless of the active profile. - var envAcct *Account - var envSource extensionTokenSource + 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 []string // direct credential env var NAMES set (never values) + conflictKeys []string // all direct input NAMES to clear on a conflict (incl. tokens) + + 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 { var blockErr *extcred.BlockError - // Only the env (direct-credential) provider maps an incomplete - // block to app_credential_incomplete. Other providers' blocks - // propagate unchanged so they still stop the chain (§3 step 1 - // is specifically about direct app credential env vars). - if errors.As(err, &blockErr) && prov.Name() == directCredentialProviderName { - if missing := missingDirectCredentialKeys(); len(missing) > 0 { - return nil, errs.NewConfigError(errs.SubtypeAppCredentialIncomplete, - "direct app credential is incomplete"). - WithMissingKeys(missing...). - WithHint("set both %s and %s, or unset both and use --profile / a config default.", - envvars.CliAppID, envvars.CliAppSecret) - } - // Block for a reason other than incompleteness (e.g. an - // invalid identity/strict-mode value); preserve prior behavior. - return nil, err + if errors.As(err, &blockErr) && blockErr.Code == extcred.BlockReasonCredentialIncomplete { + in.directBlock = blockErr + break } - return nil, err + // Blocks without the incomplete classification, and any other + // provider error, preserve their original attribution. + return in, err } - if acct != nil { - // Only the env (direct-credential) provider feeds profile - // arbitration / conflict detection / DirectCredentialEnv reporting. - // This mirrors the block-path guard above. A non-env extension - // provider (e.g. sidecar) is NOT a direct-credential env account: - // it wins outright here, returning its account + token source - // unchanged (pre-diff behavior), without being misreported as a - // direct env credential (§4.2: Present = direct env vars actually - // set) or triggering a spurious profile_app_credential_conflict. - if prov.Name() != directCredentialProviderName { - 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) - } - // 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 = "" - } - p.selectedSource = source - return internal, nil - } - envAcct = convertAccount(acct) - envSource = extensionTokenSource{provider: prov} - break + if acct == nil { + continue + } + pa := &providerAccount{acct: convertAccount(acct), source: extensionTokenSource{provider: prov}} + if acct.Kind == extcred.AccountDirect { + in.direct = pa + } else { + in.managed = pa } + 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 +} - // Step 2 (spec §3): an explicit profile was requested. - if p.profile != "" { - multi, loadErr := core.LoadOrNotConfigured() - if loadErr != nil { - // A malformed / unreadable config must surface its real (typed) - // cause — invalid_config — not be masked as profile_not_found (which - // would send the user to `profile list` and hide a broken file). - // Only a genuinely absent config (not_configured) falls through to - // the friendly profile_not_found, since the requested profile - // cannot exist. This mirrors the config-default path. - if prob, ok := errs.ProblemOf(loadErr); !ok || prob.Subtype != errs.SubtypeNotConfigured { - return nil, loadErr - } - multi = 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 + } +} + +// 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) } - var app *core.AppConfig - if multi != nil { - app = multi.FindApp(p.profile) + if app.AppId != in.directBlock.AppID { + return decision{}, newProfileAppCredentialConflict( + in.profile, app.AppId, in.directBlock.AppID, in.directBlock.PresentKeys) } - if app == nil { - return nil, errs.NewConfigError(errs.SubtypeProfileNotFound, - "profile %q not found", p.profile). - WithProfile(p.profile). - WithCredentialSource(string(p.profileSource())). - WithHint("run `lark-cli profile list` to see available profiles.") + 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) } - if envAcct != nil { - // E == complete: the direct env app_id must match the profile. - if app.AppId != envAcct.AppID { - return nil, errs.NewValidationError(errs.SubtypeProfileAppCredentialConflict, - "profile %q app_id does not match %s", p.profile, envvars.CliAppID). - WithProfileAppConflict(app.AppId, envAcct.AppID). - WithHint("unset %s/%s, or select a profile whose app_id matches the environment.", - envvars.CliAppID, envvars.CliAppSecret) - } - p.selection = IdentitySelection{ - Source: p.profileSource(), - DirectCredentialEnv: DirectCredentialEnv{ - Present: true, - Keys: presentDirectCredentialKeys(), - AppID: envAcct.AppID, - Matched: true, - }, - } - } else { - p.selection = IdentitySelection{ - Source: p.profileSource(), - DirectCredentialEnv: DirectCredentialEnv{Present: false}, - } + 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: + p.enrichOrClearIdentity(ctx, in.managed.acct, in.managed.source) + return in.managed.acct, in.managed.source, nil + case routeDirectEnv: + p.enrichOrClearIdentity(ctx, in.direct.acct, in.direct.source) + 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 { - // SECURITY (§5.1): generic message — never embed the underlying - // error or any secret material. - p.selection = IdentitySelection{} - return nil, errs.NewConfigError(errs.SubtypeProfileSecretInvalid, - "profile %q credential could not be resolved locally", p.profile). - WithProfile(p.profile). - WithAppID(app.AppId). - WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") + return nil, nil, newProfileSecretInvalidError(in.profile, d.profileAppID) } - p.selectedSource = defaultTokenSource{resolver: p.defaultToken} - return acct, nil - } - - // Step 3 (spec §3): no explicit profile — direct env credential wins. - if envAcct != nil { - if err := p.enrichUserInfo(ctx, envAcct, envSource); err != nil { - if p.warnOut != nil { - _, _ = fmt.Fprintf(p.warnOut, "warning: unable to verify user identity from credential source %q: %v\n", envSource.Name(), err) - } - // enrichUserInfo failure is non-fatal: SupportedIdentities - // (used for strict mode) is already set by the provider. - // Clear unverified user identity for safety. - envAcct.UserOpenId = "" - envAcct.UserName = "" + return acct, defaultTokenSource{resolver: p.defaultToken}, nil + default: // routeConfigDefault + if p.defaultAcct == nil { + return nil, nil, core.NotConfiguredError() } - p.selectedSource = envSource - p.selection = IdentitySelection{ - Source: SourceEnvAppID, - DirectCredentialEnv: DirectCredentialEnv{ - Present: true, - Keys: presentDirectCredentialKeys(), - AppID: envAcct.AppID, - }, + acct, err := p.defaultAcct.ResolveAccount(ctx) + if err != nil { + return nil, nil, translateConfigDefaultFailure(err, in.config) } - return envAcct, nil + return acct, defaultTokenSource{resolver: p.defaultToken}, nil } +} - // No direct env credential and no profile → the config default. - if p.defaultAcct != nil { - acct, err := p.defaultAcct.ResolveAccount(ctx) - if err != nil { - // The config default failed to resolve. Distinguish (spec §3 step - // 3.2): a default profile that EXISTS (has an app_id) but whose - // secret cannot be resolved locally is a 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 (e.g. a specific config error) pass through unchanged. - if prob, ok := errs.ProblemOf(err); ok && prob.Subtype == errs.SubtypeNotConfigured { - if name, appID, ok := defaultProfileIdentity(); ok { - // SECURITY (§5.1): generic message — never embed the - // underlying error or any secret material. app_id is - // plaintext and safe to echo. - return nil, errs.NewConfigError(errs.SubtypeProfileSecretInvalid, - "profile %q credential could not be resolved locally", name). - WithProfile(name). - WithAppID(appID). - WithHint("verify the profile's app secret or re-add the profile with `lark-cli config`.") - } - return nil, errs.NewConfigError(errs.SubtypeNoActiveProfile, "no active profile"). - WithCredentialSource(noActiveProfileCredentialSource). - WithHint("run `lark-cli config init` / `lark-cli profile add`, or set %s.", envvars.CliProfile) - } - return nil, err +// 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) } - multi, _ := core.LoadMultiAppConfig() - p.selectedSource = defaultTokenSource{resolver: p.defaultToken} - p.selection = IdentitySelection{Source: selectionSourceForDefault(multi)} - return acct, nil } - return nil, core.NotConfiguredError() + 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) } -// profileSource reports the credential source kind for a profile-backed -// selection, discriminating the --profile flag from the env fallback. -func (p *CredentialProvider) profileSource() CredentialSourceKind { - if p.profileFromFlag { - return SourceFlagProfile +func credentialRepairHint(blockErr *extcred.BlockError) string { + if len(blockErr.RequiredAnyOf) > 0 { + return "set " + humanList(blockErr.RequiredAnyOf, "or") } - return SourceEnvProfile + 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] + } +} + +// 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. Spec §5 fixes this to the literal "config": there is +// 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" -// defaultProfileIdentity reports the config default profile's display name and -// app_id when a usable default profile actually EXISTS (currentApp > firstApp -// resolves to an app with a non-empty app_id). It never touches the keychain or -// any secret, so it can distinguish "default profile exists but its secret is -// broken" (→ profile_secret_invalid) from "no usable default profile at all" -// (→ no_active_profile), without risking a secret leak (§5.1). -func defaultProfileIdentity() (name, appID string, ok bool) { - multi, err := core.LoadMultiAppConfig() - if err != nil || multi == nil { - return "", "", false - } - app := multi.CurrentAppConfig("") - if app == nil || app.AppId == "" { - return "", "", false - } - return app.ProfileName(), app.AppId, true -} - // selectionSourceForDefault reports whether the config default resolved to the -// explicit currentApp or fell back to the first app (spec §3 step 3.2). +// explicit currentApp or fell back to the first app. func selectionSourceForDefault(multi *core.MultiAppConfig) CredentialSourceKind { if multi != nil && multi.CurrentApp != "" { return SourceConfigCurrentApp @@ -430,20 +566,6 @@ func selectionSourceForDefault(multi *core.MultiAppConfig) CredentialSourceKind return SourceConfigFirstApp } -// missingDirectCredentialKeys returns the NAMES (never values) of the direct -// app credential env vars that are absent. Used only when the env provider -// blocks, to map an incomplete direct credential to app_credential_incomplete. -func missingDirectCredentialKeys() []string { - var missing []string - if os.Getenv(envvars.CliAppID) == "" { - missing = append(missing, envvars.CliAppID) - } - if os.Getenv(envvars.CliAppSecret) == "" { - missing = append(missing, envvars.CliAppSecret) - } - return missing -} - // presentDirectCredentialKeys returns the NAMES (never values) of the direct // app credential env vars that are set. Used to annotate DirectCredentialEnv. func presentDirectCredentialKeys() []string { @@ -457,6 +579,20 @@ func presentDirectCredentialKeys() []string { 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. @@ -593,16 +729,41 @@ func (p *CredentialProvider) ResolveToken(ctx context.Context, req TokenSpec) (* } // 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 { diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index f56f7e472e..e98664f2cc 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -19,6 +19,7 @@ import ( "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 { @@ -40,7 +41,7 @@ func asValidationError(t *testing.T, err error) *errs.ValidationError { } // secretValue is the profile secret written to config. It must NEVER appear in -// any error message or IdentitySelection (security §5.1). +// 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. @@ -93,7 +94,11 @@ func newProvider(t *testing.T, profile string, fromFlag bool) *credential.Creden ep := &envprovider.Provider{} defaultAcct := credential.NewDefaultAccountProvider(func() keychain.KeychainAccess { return &noopKC{} }, profile) cp := credential.NewCredentialProvider([]extcred.Provider{ep}, defaultAcct, nil, nil) - cp.WithProfile(profile, fromFlag) + if fromFlag { + cp.WithProfileFromFlag(profile) + } else { + cp.WithProfileFromEnv(profile) + } return cp } @@ -125,8 +130,8 @@ func subtypeOf(t *testing.T, err error) errs.Subtype { return p.Subtype } -// State #2: P none, E none, C none -> no_active_profile. -func TestSelection_State2_NoActiveProfile(t *testing.T) { +// 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 @@ -136,7 +141,7 @@ func TestSelection_State2_NoActiveProfile(t *testing.T) { if got := subtypeOf(t, err); got != errs.SubtypeNoActiveProfile { t.Fatalf("subtype = %q, want %q", got, errs.SubtypeNoActiveProfile) } - // Defect 1 (spec §5): no_active_profile must carry credential_source=config. + // 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") @@ -145,7 +150,7 @@ func TestSelection_State2_NoActiveProfile(t *testing.T) { } // Config-default profile with a broken secret: P none, E none, C present but the -// default profile's keychain secret ref is corrupted. Per spec §3 step 3.2 this +// default profile's keychain secret ref is corrupted. This // must be profile_secret_invalid (the identity IS configured, only its secret is // broken) — NOT no_active_profile (which is reserved for "no usable default"). func TestSelection_ConfigDefaultBrokenSecret_ProfileSecretInvalid(t *testing.T) { @@ -165,7 +170,7 @@ func TestSelection_ConfigDefaultBrokenSecret_ProfileSecretInvalid(t *testing.T) if ce.AppID != "cli_a" { t.Errorf("app_id = %q, want cli_a", ce.AppID) } - // §5.1: generic message, no cause, no secret anywhere. + // Security: generic message, no cause, no secret anywhere. if errors.Unwrap(ce) != nil { t.Errorf("profile_secret_invalid must not attach a cause, got %v", errors.Unwrap(ce)) } @@ -227,8 +232,8 @@ func TestSelection_ConfigDefault_MalformedConfig_PropagatesError(t *testing.T) { assertMalformedSurfaced(t, err) } -// State #3: P none, E partial (only APP_ID) -> app_credential_incomplete. -func TestSelection_State3_EnvPartial(t *testing.T) { +// 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) @@ -238,22 +243,73 @@ func TestSelection_State3_EnvPartial(t *testing.T) { 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 !slices.Contains(ce.MissingKeys, envvars.CliAppSecret) { - t.Errorf("missing_keys = %v, want to contain %q", ce.MissingKeys, envvars.CliAppSecret) + 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) } - // missing_keys must be NAMES only, never values. - for _, k := range ce.MissingKeys { + // 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("missing_keys contains a value, not a name: %q", k) + 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) } -// State #4: P none, E complete -> env:LARKSUITE_CLI_APP_ID. -func TestSelection_State4_EnvComplete(t *testing.T) { +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) @@ -273,8 +329,8 @@ func TestSelection_State4_EnvComplete(t *testing.T) { assertNoSecretLeak(t, "state4-keys", sel.DirectCredentialEnv.Keys...) } -// State #5: P valid, E none -> flag:--profile (fromFlag) source. -func TestSelection_State5_ProfileOnly(t *testing.T) { +// 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) @@ -293,8 +349,8 @@ func TestSelection_State5_ProfileOnly(t *testing.T) { assertNoSecretLeak(t, "state5", string(sel.Source)) } -// State #5b: P valid from env (not flag) -> env:LARKSUITE_CLI_PROFILE source. -func TestSelection_State5_ProfileFromEnv(t *testing.T) { +// 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) @@ -309,8 +365,8 @@ func TestSelection_State5_ProfileFromEnv(t *testing.T) { } } -// State #6: P missing (nonexistent), E complete -> profile_not_found. -func TestSelection_State6_ProfileNotFound(t *testing.T) { +// 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) @@ -321,7 +377,7 @@ func TestSelection_State6_ProfileNotFound(t *testing.T) { t.Fatalf("subtype = %q, want %q", got, errs.SubtypeProfileNotFound) } prob, _ := errs.ProblemOf(err) - // Defect 1 (spec §5): profile_not_found must carry the credential_source that + // 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) { @@ -330,8 +386,8 @@ func TestSelection_State6_ProfileNotFound(t *testing.T) { assertNoSecretLeak(t, "state6", err.Error(), prob.Hint, string(sel.Source)) } -// State #7: P valid but secret broken, E none -> profile_secret_invalid. -func TestSelection_State7_ProfileSecretInvalid(t *testing.T) { +// A valid profile whose stored secret cannot be resolved reports profile_secret_invalid. +func TestSelection_ProfileSecretInvalid(t *testing.T) { t.Setenv(envvars.CliAppID, "") t.Setenv(envvars.CliAppSecret, "") writeConfigTenantABroken(t) @@ -370,20 +426,19 @@ func (leakingSecretResolver) ResolveAccount(ctx context.Context) (*credential.Ac return nil, fmt.Errorf("keychain decode failed for secret %s", secretMarkerValue) } -// State #7 (secret-bearing underlying error): P valid, but the underlying -// account/secret resolution fails with an error that itself contains a -// secret. This locks the §5.1 design: doResolveAccount emits a generic +// 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_State7_UnderlyingErrorContainingSecret_NotLeaked(t *testing.T) { +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.WithProfile("tenant_a", true) + cp.WithProfileFromFlag("tenant_a") sel, err := cp.Selection(context.Background()) if got := subtypeOf(t, err); got != errs.SubtypeProfileSecretInvalid { @@ -428,9 +483,8 @@ func TestSelection_State7_UnderlyingErrorContainingSecret_NotLeaked(t *testing.T t.Errorf("Selection.DirectCredentialEnv.Keys leaked secret marker: %q", k) } } - // State #7 always clears p.selection on the secret-invalid path (see - // doResolveAccount); assert it is zero-valued, which trivially implies no - // marker anywhere in it and guards against a future field being populated + // 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 { @@ -438,8 +492,8 @@ func TestSelection_State7_UnderlyingErrorContainingSecret_NotLeaked(t *testing.T } } -// State #8: P valid, E complete, app_id matches -> profile source, env present+matched. -func TestSelection_State8_ProfileMatchesEnv(t *testing.T) { +// 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) @@ -462,8 +516,76 @@ func TestSelection_State8_ProfileMatchesEnv(t *testing.T) { assertNoSecretLeak(t, "state8-keys", sel.DirectCredentialEnv.Keys...) } -// State #9: P valid, E complete, app_id mismatches -> profile_app_credential_conflict. -func TestSelection_State9_Conflict(t *testing.T) { +// 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) @@ -473,6 +595,9 @@ func TestSelection_State9_Conflict(t *testing.T) { 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) @@ -480,26 +605,187 @@ func TestSelection_State9_Conflict(t *testing.T) { 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) } -// State #10: P valid, E partial -> app_credential_incomplete (env-partial wins). -func TestSelection_State10_ProfileWithEnvPartial(t *testing.T) { - t.Setenv(envvars.CliAppID, "") - t.Setenv(envvars.CliAppSecret, envSecretValue) // only secret set +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.SubtypeAppCredentialIncomplete { - t.Fatalf("subtype = %q, want %q", got, errs.SubtypeAppCredentialIncomplete) + 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 !slices.Contains(ce.MissingKeys, envvars.CliAppID) { - t.Errorf("missing_keys = %v, want to contain %q", ce.MissingKeys, envvars.CliAppID) + if ce.Profile != "tenant_typo" || ce.CredentialSource != string(credential.SourceEnvProfile) { + t.Fatalf("profile error = %+v, want tenant_typo from env profile", ce) } - assertNoSecretLeak(t, "state10", ce.Message, ce.Hint) - assertNoSecretLeak(t, "state10-keys", ce.MissingKeys...) +} + +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...) + }) + } +} + +func TestSelection_NonIncompleteEnvBlockPreservesOriginalAttribution(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()) + var blockErr *extcred.BlockError + if !errors.As(err, &blockErr) { + t.Fatalf("error = %T %v, want original BlockError", err, err) + } + if blockErr.Code == extcred.BlockReasonCredentialIncomplete { + t.Fatalf("BlockError misclassified as credential incomplete: %+v", blockErr) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Fatalf("exit code = %d, want unchanged internal fallback %d", got, output.ExitInternal) + } + 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) + } + }) } // fakeSidecarProvider is a NON-env extension provider (Priority 0, Name != @@ -518,13 +804,12 @@ func (f *fakeSidecarProvider) ResolveToken(ctx context.Context, req extcred.Toke return &extcred.Token{Value: "sidecar-tok", Source: "sidecar"}, nil } -// Regression: a NON-env extension provider (sidecar) that returns an 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 +// 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 (§4.2 — -// no direct env vars are set). This proves the success-account provider gating -// mirrors the block-path guard. +// 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 @@ -533,7 +818,7 @@ func TestSelection_NonEnvExtensionProviderWinsOverProfile(t *testing.T) { 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.WithProfile("tenant_a", true) + cp.WithProfileFromFlag("tenant_a") acct, err := cp.ResolveAccount(context.Background()) if err != nil { @@ -548,10 +833,13 @@ func TestSelection_NonEnvExtensionProviderWinsOverProfile(t *testing.T) { if err != nil { t.Fatalf("unexpected Selection error: %v", err) } - // No misreported direct env credential (§4.2). + // 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. @@ -564,8 +852,8 @@ func TestSelection_NonEnvExtensionProviderWinsOverProfile(t *testing.T) { assertNoSecretLeak(t, "nonenv-sidecar", string(sel.Source), sel.DirectCredentialEnv.AppID) } -// State #1: P none, E none, C present -> config default (currentApp). -func TestSelection_State1_ConfigDefault(t *testing.T) { +// 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 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/identity_selection.go b/internal/credential/identity_selection.go index bc2dadcc7d..bba484fc9c 100644 --- a/internal/credential/identity_selection.go +++ b/internal/credential/identity_selection.go @@ -12,8 +12,19 @@ const ( 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 { @@ -25,7 +36,7 @@ type DirectCredentialEnv struct { } // IdentitySelection is the explainable result of credential selection. -// It carries NO secret value (security: §5.1). +// It carries NO secret value. type IdentitySelection struct { Source CredentialSourceKind DirectCredentialEnv DirectCredentialEnv diff --git a/internal/envvars/envvars.go b/internal/envvars/envvars.go index 16419e4db4..6d40bc5290 100644 --- a/internal/envvars/envvars.go +++ b/internal/envvars/envvars.go @@ -10,7 +10,6 @@ const ( CliUserAccessToken = "LARKSUITE_CLI_USER_ACCESS_TOKEN" CliTenantAccessToken = "LARKSUITE_CLI_TENANT_ACCESS_TOKEN" CliDefaultAs = "LARKSUITE_CLI_DEFAULT_AS" - CliProfile = "LARKSUITE_CLI_PROFILE" CliStrictMode = "LARKSUITE_CLI_STRICT_MODE" // Sidecar proxy (auth proxy mode) @@ -22,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" From a209f30f3021b8283bbc0d6236298d69dce412a7 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 14 Jul 2026 17:08:00 +0800 Subject: [PATCH 28/29] test: use recognized credential placeholders --- cmd/auth/status_test.go | 4 ++-- internal/cmdutil/factory_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/auth/status_test.go b/cmd/auth/status_test.go index bdcff2c941..895dba238d 100644 --- a/cmd/auth/status_test.go +++ b/cmd/auth/status_test.go @@ -118,14 +118,14 @@ func TestAuthStatus_AllowsMatchingAppIDOnlySelectedProfile(t *testing.T) { Apps: []core.AppConfig{{ Name: "tenant_a", AppId: "cli_a", - AppSecret: core.PlainSecret("profile-secret"), + 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: "profile-secret", Brand: core.BrandFeishu} + 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{}}, diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go index 6aded5cbe4..8aac8d4087 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -463,7 +463,7 @@ func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *test Apps: []core.AppConfig{{ Name: "tenant_a", AppId: "cli_a", - AppSecret: core.PlainSecret("profile-secret"), + AppSecret: core.PlainSecret("test-secret"), Brand: core.BrandFeishu, }}, }); err != nil { @@ -480,7 +480,7 @@ func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *test }} cred := credential.NewCredentialProvider( []extcred.Provider{stub}, - &stubDefaultAccountResolver{acct: &credential.Account{AppID: "cli_a", AppSecret: "profile-secret"}}, + &stubDefaultAccountResolver{acct: &credential.Account{AppID: "cli_a", AppSecret: "test-secret"}}, nil, nil, ).WithProfileFromFlag("tenant_a") From 00d8c07de9db0d873b0ec3cfce5de4d6f49589f9 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Tue, 14 Jul 2026 20:32:03 +0800 Subject: [PATCH 29/29] docs(lark-shared): restore the trigger description to main's version The description broadening to profile-selection triggers exceeded the routing fix's actual scope: the observed failure was command selection after the skill was already engaged, not skill discovery. Routing guidance stays in the body's profile-selection section and command help; the frontmatter matches main again. --- skills/lark-shared/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/lark-shared/SKILL.md b/skills/lark-shared/SKILL.md index 583f83362e..dda10d4ae2 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/profile-selection tasks: auth login/status/logout, user vs bot identity, business-domain permissions (--domain, including all/docs/drive), missing scopes, revoking authorization, handling _notice JSON, profile/tenant/app-identity selection, or any request to make this task/session (or all following lark-cli commands) run under a specific profile/tenant — via LARKSUITE_CLI_PROFILE, --profile, unset LARKSUITE_CLI_PROFILE, or whoami identity diagnostics." +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." --- # lark-cli 共享规则