From 555722ac8ef5636b366fa23e793d121f12737fab Mon Sep 17 00:00:00 2001 From: liangshuo-1 Date: Wed, 8 Apr 2026 19:14:45 +0800 Subject: [PATCH] fix: resolve concurrency races in RuntimeContext (#330) * fix: resolve concurrency races in RuntimeContext - getAPIClient: replace check-then-act with sync.OnceValues, matching the factory_default.go convention; use NewAPIClientWithConfig to avoid post-construction config override; fall back to direct construction for test contexts that bypass newRuntimeContext. - outputErr: guard first-error capture with sync.Once to prevent data races if Out() is ever called from concurrent goroutines. Change-Id: I99c94c3dcb7663fa61571c9720163e41a5fc0e36 * fix: use tenant token for auth scopes Change-Id: I83bb677e9a33e906e207679b2ba8d0364bc20fe3 --- cmd/auth/auth.go | 14 ++++---- cmd/auth/auth_test.go | 72 ++++++++++++++++++++++++++++++++++++++ shortcuts/common/runner.go | 46 ++++++++++++------------ 3 files changed, 101 insertions(+), 31 deletions(-) diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index a7672cc53..71fb58db7 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -16,6 +16,7 @@ import ( larkauth "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" ) // NewCmdAuth creates the auth command with subcommands. @@ -100,7 +101,7 @@ type appInfoResponse struct { // getAppInfo queries app info from the Lark API. func getAppInfo(ctx context.Context, f *cmdutil.Factory, appId string) (*appInfo, error) { - sdk, err := f.LarkClient() + ac, err := f.NewAPIClient() if err != nil { return nil, err } @@ -108,12 +109,11 @@ func getAppInfo(ctx context.Context, f *cmdutil.Factory, appId string) (*appInfo queryParams := make(larkcore.QueryParams) queryParams.Set("lang", "zh_cn") - apiResp, err := sdk.Do(ctx, &larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: larkauth.ApplicationInfoPath(appId), - QueryParams: queryParams, - SupportedAccessTokenTypes: []larkcore.AccessTokenType{larkcore.AccessTokenTypeTenant}, - }) + apiResp, err := ac.DoSDKRequest(ctx, &larkcore.ApiReq{ + HttpMethod: http.MethodGet, + ApiPath: larkauth.ApplicationInfoPath(appId), + QueryParams: queryParams, + }, core.AsBot) if err != nil { return nil, err } diff --git a/cmd/auth/auth_test.go b/cmd/auth/auth_test.go index 15bd828f9..3de6267ea 100644 --- a/cmd/auth/auth_test.go +++ b/cmd/auth/auth_test.go @@ -4,12 +4,16 @@ package auth import ( + "context" + "net/http" "sort" "strings" "testing" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/registry" ) @@ -231,3 +235,71 @@ func TestAuthScopesCmd_FlagParsing(t *testing.T) { t.Errorf("expected format json, got %s", gotOpts.Format) } } + +func TestAuthScopesRun_UsesTenantAccessTokenFromCredentialProvider(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "", Brand: core.BrandFeishu, + }) + tokenResolver := &authScopesTokenResolver{} + f.Credential = credential.NewCredentialProvider(nil, nil, tokenResolver, nil) + + appInfoStub := &httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/application/v6/applications/test-app", + Body: map[string]interface{}{ + "code": 0, + "msg": "ok", + "data": map[string]interface{}{ + "app": map[string]interface{}{ + "creator_id": "ou_creator", + "scopes": []map[string]interface{}{ + { + "scope": "im:message", + "token_types": []string{"tenant"}, + }, + { + "scope": "im:message:send_as_user", + "token_types": []string{"user"}, + }, + }, + }, + }, + }, + } + reg.Register(appInfoStub) + + err := authScopesRun(&ScopesOptions{ + Factory: f, + Ctx: context.Background(), + Format: "json", + }) + if err != nil { + t.Fatalf("authScopesRun() error = %v", err) + } + + if len(tokenResolver.requests) != 1 { + t.Fatalf("resolved token requests = %v, want exactly one request", tokenResolver.requests) + } + if got := tokenResolver.requests[0].Type; got != credential.TokenTypeTAT { + t.Fatalf("resolved token type = %q, want %q", got, credential.TokenTypeTAT) + } + if got := appInfoStub.CapturedHeaders.Get("Authorization"); got != "Bearer tenant-token" { + t.Fatalf("Authorization header = %q, want %q", got, "Bearer tenant-token") + } +} + +type authScopesTokenResolver struct { + requests []credential.TokenSpec +} + +func (r *authScopesTokenResolver) ResolveToken(ctx context.Context, req credential.TokenSpec) (*credential.TokenResult, error) { + r.requests = append(r.requests, req) + switch req.Type { + case credential.TokenTypeTAT: + return &credential.TokenResult{Token: "tenant-token"}, nil + case credential.TokenTypeUAT: + return &credential.TokenResult{Token: "user-token"}, nil + default: + return &credential.TokenResult{Token: "unexpected-token"}, nil + } +} diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 8cc8746c9..753b2345b 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -13,6 +13,7 @@ import ( "os" "slices" "strings" + "sync" "github.com/google/uuid" lark "github.com/larksuite/oapi-sdk-go/v3" @@ -30,17 +31,18 @@ import ( // RuntimeContext provides helpers for shortcut execution. type RuntimeContext struct { - ctx context.Context // from cmd.Context(), propagated through the call chain - Config *core.CliConfig - Cmd *cobra.Command - Format string - JqExpr string // --jq expression; empty = no filter - outputErr error // deferred error from Out()/OutFormat() jq filtering - botOnly bool // set by framework for bot-only shortcuts - resolvedAs core.Identity // effective identity resolved by framework - Factory *cmdutil.Factory // injected by framework - apiClient *client.APIClient // lazily initialized, cached - larkSDK *lark.Client // eagerly initialized in mountDeclarative + ctx context.Context // from cmd.Context(), propagated through the call chain + Config *core.CliConfig + Cmd *cobra.Command + Format string + JqExpr string // --jq expression; empty = no filter + outputErrOnce sync.Once // guards first-error capture in Out()/OutFormat() + outputErr error // deferred error from jq filtering; written at most once + botOnly bool // set by framework for bot-only shortcuts + resolvedAs core.Identity // effective identity resolved by framework + Factory *cmdutil.Factory // injected by framework + apiClientFunc func() (*client.APIClient, error) // sync.OnceValues; initialized in newRuntimeContext + larkSDK *lark.Client // eagerly initialized in mountDeclarative } // ── Identity ── @@ -73,18 +75,13 @@ func (ctx *RuntimeContext) UserOpenId() string { return ctx.Config.UserOpenId } func (ctx *RuntimeContext) Ctx() context.Context { return ctx.ctx } // getAPIClient returns the cached APIClient, creating it on first use. +// Thread-safe via sync.OnceValues (initialized in newRuntimeContext). +// Falls back to direct construction for test contexts that bypass newRuntimeContext. func (ctx *RuntimeContext) getAPIClient() (*client.APIClient, error) { - if ctx.apiClient != nil { - return ctx.apiClient, nil + if ctx.apiClientFunc != nil { + return ctx.apiClientFunc() } - ac, err := ctx.Factory.NewAPIClient() - if err != nil { - return nil, err - } - // Override config with the one resolved for this context (may differ from Factory's) - ac.Config = ctx.Config - ctx.apiClient = ac - return ac, nil + return ctx.Factory.NewAPIClientWithConfig(ctx.Config) } // AccessToken returns a valid access token for the current identity. @@ -393,9 +390,7 @@ func (ctx *RuntimeContext) Out(data interface{}, meta *output.Meta) { if ctx.JqExpr != "" { if err := output.JqFilter(ctx.IO().Out, env, ctx.JqExpr); err != nil { fmt.Fprintf(ctx.IO().ErrOut, "error: %v\n", err) - if ctx.outputErr == nil { - ctx.outputErr = err - } + ctx.outputErrOnce.Do(func() { ctx.outputErr = err }) } return } @@ -603,6 +598,9 @@ func newRuntimeContext(cmd *cobra.Command, f *cmdutil.Factory, s *Shortcut, conf ctx := cmd.Context() ctx = cmdutil.ContextWithShortcut(ctx, s.Service+":"+s.Command, uuid.New().String()) rctx := &RuntimeContext{ctx: ctx, Config: config, Cmd: cmd, botOnly: botOnly, resolvedAs: as, Factory: f} + rctx.apiClientFunc = sync.OnceValues(func() (*client.APIClient, error) { + return f.NewAPIClientWithConfig(config) + }) sdk, err := f.LarkClient() if err != nil {