From c86c348fa97dffbcf4463a23999df95d724a2fb7 Mon Sep 17 00:00:00 2001 From: luozhixiong Date: Fri, 24 Jul 2026 17:55:33 +0800 Subject: [PATCH] fix(auth): reserve incomplete credential arbitration for env --- extension/credential/types.go | 12 ++--- internal/cmdutil/factory_test.go | 15 +++--- internal/credential/credential_provider.go | 19 ++++++++ .../credential_provider_selection_test.go | 47 +++++++++++++++++++ 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/extension/credential/types.go b/extension/credential/types.go index 85f0da7c2..4c90e2227 100644 --- a/extension/credential/types.go +++ b/extension/credential/types.go @@ -103,12 +103,12 @@ type TokenSpec struct { 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 a Code propagate unchanged. + // BlockReasonCredentialIncomplete marks incomplete inputs from the builtin + // process-env credential provider. It is reserved for that provider because + // direct-credential arbitration and diagnostics currently name the fixed + // LARKSUITE_CLI_* env surface. Third-party providers must return an + // unclassified BlockError until the SPI carries provider-owned input + // descriptors. Blocks without a Code propagate unchanged. BlockReasonCredentialIncomplete BlockReason = "credential_incomplete" // BlockReasonInvalidPolicy marks a user-supplied policy input (e.g. diff --git a/internal/cmdutil/factory_test.go b/internal/cmdutil/factory_test.go index ef63a3d64..a5f00d7b5 100644 --- a/internal/cmdutil/factory_test.go +++ b/internal/cmdutil/factory_test.go @@ -13,6 +13,7 @@ import ( "github.com/larksuite/cli/errs" extcred "github.com/larksuite/cli/extension/credential" + envprovider "github.com/larksuite/cli/extension/credential/env" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/envvars" @@ -458,6 +459,10 @@ func TestRequireBuiltinCredentialProvider_AllowsBuiltinProvider(t *testing.T) { func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv(envvars.CliAppID, "cli_a") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv(envvars.CliUserAccessToken, "") + t.Setenv(envvars.CliTenantAccessToken, "") if err := core.SaveMultiAppConfig(&core.MultiAppConfig{ CurrentApp: "tenant_a", Apps: []core.AppConfig{{ @@ -470,16 +475,8 @@ func TestRequireBuiltinCredentialProvider_AllowsMatchingAppIDOnlyProfile(t *test 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}, + []extcred.Provider{&envprovider.Provider{}}, &stubDefaultAccountResolver{acct: &credential.Account{AppID: "cli_a", AppSecret: "test-secret"}}, nil, nil, diff --git a/internal/credential/credential_provider.go b/internal/credential/credential_provider.go index 0b81b457b..edb1fe83e 100644 --- a/internal/credential/credential_provider.go +++ b/internal/credential/credential_provider.go @@ -306,6 +306,15 @@ func (p *CredentialProvider) gatherIdentityInputs(ctx context.Context) (identity if errors.As(err, &blockErr) { switch blockErr.Code { case extcred.BlockReasonCredentialIncomplete: + // app_credential_incomplete, profile matching, and + // DirectCredentialEnv diagnostics are defined in terms of + // the builtin LARKSUITE_CLI_* env surface. Until the SPI + // carries provider-owned input descriptors, accepting this + // classification from another provider would produce + // contradictory arbitration and repair hints. + if _, builtin := prov.(*envprovider.Provider); !builtin { + return in, newCredentialIncompleteProviderContractError(prov) + } in.directBlock = blockErr case extcred.BlockReasonInvalidPolicy: // A user-supplied policy value failed validation; that is @@ -600,6 +609,11 @@ func newInvalidPolicyError(blockErr *extcred.BlockError) error { WithHint("set %s to a supported value or unset it.", blockErr.Param) } +func newCredentialIncompleteProviderContractError(prov extcred.Provider) error { + return errs.NewInternalError(errs.SubtypeUnknown, + "credential provider %q returned credential_incomplete, which is reserved for the builtin env provider", prov.Name()) +} + // newProfileSecretInvalidError is deliberately generic (SECURITY): the // underlying cause may carry secret material, so neither it nor its message // may reach the envelope. app_id is plaintext and safe to echo. @@ -850,6 +864,11 @@ func (p *CredentialProvider) ActiveExtensionProviderName(ctx context.Context) (s if blockErr.Code == extcred.BlockReasonInvalidPolicy { return "", newInvalidPolicyError(blockErr) } + if blockErr.Code == extcred.BlockReasonCredentialIncomplete { + if _, builtin := prov.(*envprovider.Provider); !builtin { + return "", newCredentialIncompleteProviderContractError(prov) + } + } name := blockErr.Provider if name == "" { name = prov.Name() diff --git a/internal/credential/credential_provider_selection_test.go b/internal/credential/credential_provider_selection_test.go index ffe1b0201..48641cc7b 100644 --- a/internal/credential/credential_provider_selection_test.go +++ b/internal/credential/credential_provider_selection_test.go @@ -1031,6 +1031,53 @@ func TestSelection_ForgedDirectProviderRejected(t *testing.T) { } } +// forgedIncompleteProvider exercises the public SPI boundary: although it +// supplies provider-owned metadata, credential_incomplete is reserved for the +// builtin env provider while arbitration diagnostics name LARKSUITE_CLI_*. +type forgedIncompleteProvider struct{} + +func (forgedIncompleteProvider) Name() string { return "vault" } +func (forgedIncompleteProvider) Priority() int { return 0 } +func (forgedIncompleteProvider) ResolveAccount(context.Context) (*extcred.Account, error) { + return nil, &extcred.BlockError{ + Provider: "vault", + Reason: "vault app credential is incomplete", + Code: extcred.BlockReasonCredentialIncomplete, + AppID: "vault_app", + PresentKeys: []string{"VAULT_APP_ID"}, + } +} +func (forgedIncompleteProvider) ResolveToken(context.Context, extcred.TokenSpec) (*extcred.Token, error) { + return nil, nil +} + +func TestSelection_NonEnvCredentialIncompleteRejected(t *testing.T) { + t.Setenv(envvars.CliAppID, "") + t.Setenv(envvars.CliAppSecret, "") + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + + providers := []extcred.Provider{forgedIncompleteProvider{}} + probeCP := credential.NewCredentialProvider(providers, nil, nil, nil) + name, probeErr := probeCP.ActiveExtensionProviderName(context.Background()) + if name != "" { + t.Fatalf("provider name = %q, want none for an invalid SPI classification", name) + } + + arbCP := credential.NewCredentialProvider(providers, nil, nil, nil) + _, arbErr := arbCP.ResolveAccount(context.Background()) + for label, err := range map[string]error{"probe": probeErr, "arbitration": arbErr} { + if err == nil || !strings.Contains(err.Error(), "reserved for the builtin env provider") { + t.Fatalf("%s err = %v, want credential_incomplete reservation failure for a non-env provider", label, err) + } + if got := subtypeOf(t, err); got != errs.SubtypeUnknown { + t.Fatalf("%s subtype = %q, want unknown internal contract violation", label, got) + } + } + if probeErr.Error() != arbErr.Error() { + t.Fatalf("probe and arbitration diverge:\n probe: %v\n arbitration: %v", probeErr, arbErr) + } +} + // policyBlockProvider blocks with an invalid_policy classification, standing // in for the env provider having seen a bad LARKSUITE_CLI_DEFAULT_AS. type policyBlockProvider struct{}