From 971e3201f14ea74fa4b0bdc71ad4846bdc55cbee Mon Sep 17 00:00:00 2001 From: shanglei Date: Tue, 21 Apr 2026 22:54:52 +0800 Subject: [PATCH 1/2] feat(cmdutil): add X-Cli-Build header for CLI build classification Adds X-Cli-Build (official / extended / unknown) so the gateway can distinguish official CLI from ISV-repackaged builds. --- internal/cmdutil/factory_default.go | 1 + internal/cmdutil/secheader.go | 85 +++++++++++- internal/cmdutil/secheader_sidecar_test.go | 34 +++++ internal/cmdutil/secheader_test.go | 146 +++++++++++++++++++++ internal/cmdutil/transport.go | 18 +++ internal/cmdutil/transport_test.go | 108 ++++++++++++++- 6 files changed, 384 insertions(+), 8 deletions(-) create mode 100644 internal/cmdutil/secheader_sidecar_test.go create mode 100644 internal/cmdutil/secheader_test.go diff --git a/internal/cmdutil/factory_default.go b/internal/cmdutil/factory_default.go index c1dc10817..ffd97c449 100644 --- a/internal/cmdutil/factory_default.go +++ b/internal/cmdutil/factory_default.go @@ -132,6 +132,7 @@ func buildSDKTransport() http.RoundTripper { var sdkTransport http.RoundTripper = util.SharedTransport() sdkTransport = &RetryTransport{Base: sdkTransport} sdkTransport = &UserAgentTransport{Base: sdkTransport} + sdkTransport = &BuildHeaderTransport{Base: sdkTransport} sdkTransport = &auth.SecurityPolicyTransport{Base: sdkTransport} return wrapWithExtension(sdkTransport) } diff --git a/internal/cmdutil/secheader.go b/internal/cmdutil/secheader.go index 411232c8c..aa76628c0 100644 --- a/internal/cmdutil/secheader.go +++ b/internal/cmdutil/secheader.go @@ -6,7 +6,14 @@ package cmdutil import ( "context" "net/http" - + "reflect" + "runtime/debug" + "strings" + "sync" + + "github.com/larksuite/cli/extension/credential" + "github.com/larksuite/cli/extension/fileio" + exttransport "github.com/larksuite/cli/extension/transport" "github.com/larksuite/cli/internal/build" larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) @@ -14,12 +21,21 @@ import ( const ( HeaderSource = "X-Cli-Source" HeaderVersion = "X-Cli-Version" + HeaderBuild = "X-Cli-Build" HeaderShortcut = "X-Cli-Shortcut" HeaderExecutionId = "X-Cli-Execution-Id" SourceValue = "lark-cli" HeaderUserAgent = "User-Agent" + + // BuildKindOfficial / BuildKindExtended / BuildKindUnknown are the values + // reported in the X-Cli-Build header; see DetectBuildKind for semantics. + BuildKindOfficial = "official" + BuildKindExtended = "extended" + BuildKindUnknown = "unknown" + + officialModulePath = "github.com/larksuite/cli" ) // UserAgentValue returns the User-Agent value: "lark-cli/{version}". @@ -32,10 +48,77 @@ func BaseSecurityHeaders() http.Header { h := make(http.Header) h.Set(HeaderSource, SourceValue) h.Set(HeaderVersion, build.Version) + h.Set(HeaderBuild, DetectBuildKind()) h.Set(HeaderUserAgent, UserAgentValue()) return h } +var ( + buildKindOnce sync.Once + buildKindVal string +) + +// DetectBuildKind reports whether this binary is the official CLI, an +// extended/repackaged build, or unknown. The result is cached via sync.Once +// so it is computed only on the first call. +// +// IMPORTANT: must NOT be called from any package init(). Go's init ordering +// follows the import graph; ISV providers registered via blank import may not +// have run yet, which would misclassify an extended build as official. Call +// only when handling an actual request (e.g. from BaseSecurityHeaders). +func DetectBuildKind() string { + buildKindOnce.Do(func() { + buildKindVal = computeBuildKind() + }) + return buildKindVal +} + +// computeBuildKind performs the actual detection without any caching. +// Exposed for tests. +func computeBuildKind() string { + info, ok := debug.ReadBuildInfo() + if !ok { + return BuildKindUnknown + } + // Priority 1: main module path. ISV wrappers live under their own module, + // which is the strongest signal that this is a repackaged build. + if info.Main.Path != "" && info.Main.Path != officialModulePath { + return BuildKindExtended + } + // Priority 2: non-builtin extension providers. + for _, p := range credential.Providers() { + if !isBuiltinProvider(p) { + return BuildKindExtended + } + } + if tp := exttransport.GetProvider(); tp != nil && !isBuiltinProvider(tp) { + return BuildKindExtended + } + if fp := fileio.GetProvider(); fp != nil && !isBuiltinProvider(fp) { + return BuildKindExtended + } + return BuildKindOfficial +} + +// isBuiltinProvider reports whether p is declared under the official module +// path. Third-party providers live under their own module and fail this check. +// Using reflect.PkgPath makes this robust against Name() spoofing since +// package paths are fixed at compile time. +func isBuiltinProvider(p any) bool { + if p == nil { + return false + } + t := reflect.TypeOf(p) + if t == nil { + return false + } + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + pkg := t.PkgPath() + return pkg == officialModulePath || strings.HasPrefix(pkg, officialModulePath+"/") +} + // ── Context utilities ── type ctxKey string diff --git a/internal/cmdutil/secheader_sidecar_test.go b/internal/cmdutil/secheader_sidecar_test.go new file mode 100644 index 000000000..0f4092eb6 --- /dev/null +++ b/internal/cmdutil/secheader_sidecar_test.go @@ -0,0 +1,34 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +//go:build authsidecar + +package cmdutil + +import ( + "testing" + + sidecarcred "github.com/larksuite/cli/extension/credential/sidecar" + sidecartrans "github.com/larksuite/cli/extension/transport/sidecar" +) + +// TestIsBuiltinProvider_SidecarProviders locks the classification for the +// sidecar-mode providers enumerated in design doc §3.3.2 as "官方自带". These +// types only compile when the `authsidecar` build tag is active, so the test +// is guarded by the same tag. +func TestIsBuiltinProvider_SidecarProviders(t *testing.T) { + cases := []struct { + name string + provider any + }{ + {"sidecar credential provider", &sidecarcred.Provider{}}, + {"sidecar transport provider", &sidecartrans.Provider{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if !isBuiltinProvider(tc.provider) { + t.Fatalf("%T must be classified as builtin (PkgPath under %s)", tc.provider, officialModulePath) + } + }) + } +} diff --git a/internal/cmdutil/secheader_test.go b/internal/cmdutil/secheader_test.go new file mode 100644 index 000000000..2af8856b8 --- /dev/null +++ b/internal/cmdutil/secheader_test.go @@ -0,0 +1,146 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdutil + +import ( + "context" + "net/http" + "testing" + + "github.com/larksuite/cli/extension/credential" + envcred "github.com/larksuite/cli/extension/credential/env" + "github.com/larksuite/cli/internal/vfs/localfileio" +) + +// --------------------------------------------------------------------------- +// isBuiltinProvider +// --------------------------------------------------------------------------- + +// cmdutilLocalProvider has PkgPath under the official module +// ("github.com/larksuite/cli/internal/cmdutil") and should be classified +// as builtin. +type cmdutilLocalProvider struct{} + +func (cmdutilLocalProvider) Name() string { return "local" } +func (cmdutilLocalProvider) ResolveAccount(context.Context) (*credential.Account, error) { + return nil, nil +} +func (cmdutilLocalProvider) ResolveToken(context.Context, credential.TokenSpec) (*credential.Token, error) { + return nil, nil +} + +func TestIsBuiltinProvider_Nil(t *testing.T) { + if isBuiltinProvider(nil) { + t.Fatal("isBuiltinProvider(nil) = true, want false") + } +} + +func TestIsBuiltinProvider_TypeUnderOfficialModule(t *testing.T) { + if !isBuiltinProvider(&cmdutilLocalProvider{}) { + t.Fatal("type under github.com/larksuite/cli/... should be builtin") + } +} + +func TestIsBuiltinProvider_StdlibTypeIsNotBuiltin(t *testing.T) { + // A standard library type has PkgPath "net/http" — outside official module. + // This covers the non-builtin branch, which we cannot trigger from inside + // this test file using a locally-defined type. + if isBuiltinProvider(&http.Server{}) { + t.Fatal("stdlib type classified as builtin, PkgPath check is broken") + } +} + +func TestIsBuiltinProvider_PkgPathNotSpoofableByName(t *testing.T) { + // Name() returns a string, but classification uses reflect.Type.PkgPath + // which is compile-time fixed. Confirm by using a locally-defined type + // that returns a name mimicking an external provider. + p := &cmdutilLocalProvider{} + if p.Name() != "local" { + t.Fatalf("sanity check: Name() = %q", p.Name()) + } + if !isBuiltinProvider(p) { + t.Fatal("isBuiltinProvider should decide by PkgPath, not Name()") + } +} + +// TestIsBuiltinProvider_RealBuiltinProviders locks down the classification +// for the concrete providers enumerated in design doc §3.3.2 as "官方自带": +// env credential provider and local fileio provider. If any of these is +// moved out of the official module tree in the future, this test must flip +// red so the new package path is explicitly considered. +// +// The sidecar providers (extension/credential/sidecar and +// extension/transport/sidecar) are guarded by the `authsidecar` build tag +// and covered in secheader_sidecar_test.go under that tag. +func TestIsBuiltinProvider_RealBuiltinProviders(t *testing.T) { + cases := []struct { + name string + provider any + }{ + {"env credential provider", &envcred.Provider{}}, + {"local fileio provider", &localfileio.Provider{}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if !isBuiltinProvider(tc.provider) { + t.Fatalf("%T must be classified as builtin (PkgPath under %s)", tc.provider, officialModulePath) + } + }) + } +} + +// --------------------------------------------------------------------------- +// computeBuildKind +// --------------------------------------------------------------------------- + +func TestComputeBuildKind_ReturnsKnownValue(t *testing.T) { + // Under `go test`, Main.Path is typically the module being tested + // ("github.com/larksuite/cli"); the concrete return may still be + // official, extended, or unknown depending on Main.Path and the + // registered providers. Just assert it's one of the defined values. + got := computeBuildKind() + switch got { + case BuildKindOfficial, BuildKindExtended, BuildKindUnknown: + default: + t.Fatalf("computeBuildKind() = %q, want one of official/extended/unknown", got) + } +} + +// --------------------------------------------------------------------------- +// DetectBuildKind — sync.Once caching +// --------------------------------------------------------------------------- + +func TestDetectBuildKind_StableAcrossCalls(t *testing.T) { + a := DetectBuildKind() + b := DetectBuildKind() + if a != b { + t.Fatalf("DetectBuildKind() returned different values on repeat: %q vs %q", a, b) + } +} + +// --------------------------------------------------------------------------- +// BaseSecurityHeaders +// --------------------------------------------------------------------------- + +func TestBaseSecurityHeaders_IncludesBuildHeader(t *testing.T) { + h := BaseSecurityHeaders() + v := h.Get(HeaderBuild) + if v == "" { + t.Fatal("BaseSecurityHeaders missing X-Cli-Build header") + } + switch v { + case BuildKindOfficial, BuildKindExtended, BuildKindUnknown: + default: + t.Fatalf("X-Cli-Build = %q, want one of official/extended/unknown", v) + } +} + +func TestBaseSecurityHeaders_AllRequiredHeaders(t *testing.T) { + h := BaseSecurityHeaders() + for _, key := range []string{HeaderSource, HeaderVersion, HeaderBuild, HeaderUserAgent} { + if h.Get(key) == "" { + t.Errorf("BaseSecurityHeaders missing %s", key) + } + } +} diff --git a/internal/cmdutil/transport.go b/internal/cmdutil/transport.go index 627755ea6..ccb661194 100644 --- a/internal/cmdutil/transport.go +++ b/internal/cmdutil/transport.go @@ -72,6 +72,24 @@ func (t *UserAgentTransport) RoundTrip(req *http.Request) (*http.Response, error return util.FallbackTransport().RoundTrip(req) } +// BuildHeaderTransport is an http.RoundTripper that force-writes the +// X-Cli-Build header before every request. Used in the SDK transport chain, +// where SecurityHeaderTransport is not installed, to prevent extensions from +// tampering with the build classification. The direct HTTP chain is already +// covered by SecurityHeaderTransport iterating BaseSecurityHeaders. +type BuildHeaderTransport struct { + Base http.RoundTripper +} + +func (t *BuildHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.Header.Set(HeaderBuild, DetectBuildKind()) + if t.Base != nil { + return t.Base.RoundTrip(req) + } + return util.FallbackTransport().RoundTrip(req) +} + // SecurityHeaderTransport is an http.RoundTripper that injects CLI security // headers into every request. Shortcut headers are read from the request context. type SecurityHeaderTransport struct { diff --git a/internal/cmdutil/transport_test.go b/internal/cmdutil/transport_test.go index ced9e1e7e..b9a097d89 100644 --- a/internal/cmdutil/transport_test.go +++ b/internal/cmdutil/transport_test.go @@ -97,13 +97,18 @@ func TestRetryTransport_DefaultNoRetry(t *testing.T) { func TestBuildSDKTransport_IncludesRetryTransport(t *testing.T) { transport := buildSDKTransport() + // Chain: SecurityPolicy → BuildHeader → UserAgent → Retry → Base sec, ok := transport.(*internalauth.SecurityPolicyTransport) if !ok { t.Fatalf("outer transport type = %T, want *auth.SecurityPolicyTransport", transport) } - ua, ok := sec.Base.(*UserAgentTransport) + bh, ok := sec.Base.(*BuildHeaderTransport) if !ok { - t.Fatalf("middle transport type = %T, want *UserAgentTransport", sec.Base) + t.Fatalf("layer after SecurityPolicy = %T, want *BuildHeaderTransport", sec.Base) + } + ua, ok := bh.Base.(*UserAgentTransport) + if !ok { + t.Fatalf("layer after BuildHeader = %T, want *UserAgentTransport", bh.Base) } if _, ok := ua.Base.(*RetryTransport); !ok { t.Fatalf("inner transport type = %T, want *RetryTransport", ua.Base) @@ -116,7 +121,7 @@ func TestBuildSDKTransport_WithExtension(t *testing.T) { transport := buildSDKTransport() - // Chain: extensionMiddleware → SecurityPolicy → UserAgent → Retry → Base + // Chain: extensionMiddleware → SecurityPolicy → BuildHeader → UserAgent → Retry → Base mid, ok := transport.(*extensionMiddleware) if !ok { t.Fatalf("outer transport type = %T, want *extensionMiddleware", transport) @@ -125,9 +130,13 @@ func TestBuildSDKTransport_WithExtension(t *testing.T) { if !ok { t.Fatalf("transport type = %T, want *auth.SecurityPolicyTransport", mid.Base) } - ua, ok := sec.Base.(*UserAgentTransport) + bh, ok := sec.Base.(*BuildHeaderTransport) + if !ok { + t.Fatalf("layer after SecurityPolicy = %T, want *BuildHeaderTransport", sec.Base) + } + ua, ok := bh.Base.(*UserAgentTransport) if !ok { - t.Fatalf("transport type = %T, want *UserAgentTransport", sec.Base) + t.Fatalf("layer after BuildHeader = %T, want *UserAgentTransport", bh.Base) } if _, ok := ua.Base.(*RetryTransport); !ok { t.Fatalf("innermost transport type = %T, want *RetryTransport", ua.Base) @@ -139,13 +148,18 @@ func TestBuildSDKTransport_WithoutExtension(t *testing.T) { transport := buildSDKTransport() + // Chain: SecurityPolicy → BuildHeader → UserAgent → Retry → Base sec, ok := transport.(*internalauth.SecurityPolicyTransport) if !ok { t.Fatalf("outer transport type = %T, want *auth.SecurityPolicyTransport", transport) } - ua, ok := sec.Base.(*UserAgentTransport) + bh, ok := sec.Base.(*BuildHeaderTransport) if !ok { - t.Fatalf("middle transport type = %T, want *UserAgentTransport", sec.Base) + t.Fatalf("layer after SecurityPolicy = %T, want *BuildHeaderTransport", sec.Base) + } + ua, ok := bh.Base.(*UserAgentTransport) + if !ok { + t.Fatalf("layer after BuildHeader = %T, want *UserAgentTransport", bh.Base) } if _, ok := ua.Base.(*RetryTransport); !ok { t.Fatalf("inner transport type = %T, want *RetryTransport", ua.Base) @@ -236,6 +250,86 @@ func TestExtensionInterceptor_ExecutionOrder(t *testing.T) { } } +// buildTamperingInterceptor tries to delete and spoof X-Cli-Build via +// PreRoundTrip. The SDK chain's BuildHeaderTransport must restore the real +// value before the request leaves the process. +type buildTamperingInterceptor struct{} + +func (buildTamperingInterceptor) PreRoundTrip(req *http.Request) func(*http.Response, error) { + req.Header.Del(HeaderBuild) + req.Header.Set(HeaderBuild, "ext-tampered-build") + return nil +} + +// TestBuildHeaderTransport_SDKChain_OverridesTamperedHeader verifies that the +// X-Cli-Build header is force-written by BuildHeaderTransport in the SDK +// transport chain, even when an extension tries to delete or spoof it. This +// closes the gap where the SDK chain had no equivalent of +// SecurityHeaderTransport (see design doc §3.3.3). +func TestBuildHeaderTransport_SDKChain_OverridesTamperedHeader(t *testing.T) { + var receivedBuild string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedBuild = r.Header.Get(HeaderBuild) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + exttransport.Register(&stubTransportProvider{interceptor: buildTamperingInterceptor{}}) + t.Cleanup(func() { exttransport.Register(nil) }) + + // Replicate the SDK chain layering used by buildSDKTransport. + var base http.RoundTripper = http.DefaultTransport + base = &RetryTransport{Base: base} + base = &UserAgentTransport{Base: base} + base = &BuildHeaderTransport{Base: base} + transport := wrapWithExtension(base) + client := &http.Client{Transport: transport} + + req, _ := http.NewRequest("GET", srv.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() + + if receivedBuild == "ext-tampered-build" { + t.Fatalf("%s = %q, extension tampering leaked to network", HeaderBuild, receivedBuild) + } + want := DetectBuildKind() + if receivedBuild != want { + t.Fatalf("%s = %q, want %q", HeaderBuild, receivedBuild, want) + } +} + +// TestBuildHeaderTransport_OverridesEvenWithoutTamper verifies that even if +// no extension is registered, BuildHeaderTransport writes X-Cli-Build. +func TestBuildHeaderTransport_OverridesEvenWithoutTamper(t *testing.T) { + var receivedBuild string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedBuild = r.Header.Get(HeaderBuild) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + transport := &BuildHeaderTransport{Base: http.DefaultTransport} + client := &http.Client{Transport: transport} + + req, _ := http.NewRequest("GET", srv.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() + + if receivedBuild == "" { + t.Fatalf("%s header missing, BuildHeaderTransport did not inject", HeaderBuild) + } + want := DetectBuildKind() + if receivedBuild != want { + t.Fatalf("%s = %q, want %q", HeaderBuild, receivedBuild, want) + } +} + // interceptorFunc adapts a function to exttransport.Interceptor. type interceptorFunc func(*http.Request) func(*http.Response, error) From 486d9910547ad7767adf49d833349a1d9305c6b7 Mon Sep 17 00:00:00 2001 From: shanglei Date: Wed, 22 Apr 2026 15:02:14 +0800 Subject: [PATCH 2/2] test(cmdutil): lift coverage on build-kind classification Extract classifyBuild as a pure helper so every branch (unknown / extended main-path / extended credential / extended transport / extended fileio / official) is reachable without mutating process-wide provider registries. Also cover: isBuiltinProvider non-pointer values, BuildHeaderTransport nil-Base fallback path, and fix the Name-spoof test so the test double returns a value that actually mimics an ISV provider. Coverage on PR-changed functions: - classifyBuild: 100% (new) - computeBuildKind: 61.5% -> 93.3% - BuildHeaderTransport.RoundTrip: 80% -> 100% --- internal/cmdutil/secheader.go | 49 ++++++++--- internal/cmdutil/secheader_test.go | 126 +++++++++++++++++++++++++++-- internal/cmdutil/transport_test.go | 29 +++++++ 3 files changed, 190 insertions(+), 14 deletions(-) diff --git a/internal/cmdutil/secheader.go b/internal/cmdutil/secheader.go index aa76628c0..f93713f4a 100644 --- a/internal/cmdutil/secheader.go +++ b/internal/cmdutil/secheader.go @@ -74,27 +74,58 @@ func DetectBuildKind() string { } // computeBuildKind performs the actual detection without any caching. -// Exposed for tests. +// Exposed for tests. Gathers runtime/global inputs and delegates the pure +// branching logic to classifyBuild so that logic can be unit-tested without +// mutating process-wide provider registries. func computeBuildKind() string { info, ok := debug.ReadBuildInfo() - if !ok { + mainPath := "" + if ok { + mainPath = info.Main.Path + } + + credProviders := credential.Providers() + creds := make([]any, len(credProviders)) + for i, p := range credProviders { + creds[i] = p + } + + var tp any + if p := exttransport.GetProvider(); p != nil { + tp = p + } + var fp any + if p := fileio.GetProvider(); p != nil { + fp = p + } + return classifyBuild(mainPath, ok, creds, tp, fp) +} + +// classifyBuild is the pure classification logic used by computeBuildKind. +// Callers supply concrete values so every branch is reachable from tests +// without touching debug.ReadBuildInfo or the extension registries. +// +// Priority order mirrors the design doc: +// 1. no build info → unknown +// 2. main module path not the official one → extended (ISV wrapper) +// 3. any non-builtin provider (credential / transport / fileio) → extended +// 4. otherwise → official +func classifyBuild(mainPath string, haveBuildInfo bool, credProviders []any, transportProvider, fileioProvider any) string { + if !haveBuildInfo { return BuildKindUnknown } - // Priority 1: main module path. ISV wrappers live under their own module, - // which is the strongest signal that this is a repackaged build. - if info.Main.Path != "" && info.Main.Path != officialModulePath { + if mainPath != "" && mainPath != officialModulePath { return BuildKindExtended } - // Priority 2: non-builtin extension providers. - for _, p := range credential.Providers() { + for _, p := range credProviders { if !isBuiltinProvider(p) { return BuildKindExtended } } - if tp := exttransport.GetProvider(); tp != nil && !isBuiltinProvider(tp) { + if transportProvider != nil && !isBuiltinProvider(transportProvider) { return BuildKindExtended } - if fp := fileio.GetProvider(); fp != nil && !isBuiltinProvider(fp) { + if fileioProvider != nil && !isBuiltinProvider(fileioProvider) { return BuildKindExtended } return BuildKindOfficial diff --git a/internal/cmdutil/secheader_test.go b/internal/cmdutil/secheader_test.go index 2af8856b8..54c61f1d4 100644 --- a/internal/cmdutil/secheader_test.go +++ b/internal/cmdutil/secheader_test.go @@ -22,7 +22,9 @@ import ( // as builtin. type cmdutilLocalProvider struct{} -func (cmdutilLocalProvider) Name() string { return "local" } +// Name intentionally returns a value that mimics an external provider; the +// PkgPath-based classifier must ignore it. See TestIsBuiltinProvider_PkgPathNotSpoofableByName. +func (cmdutilLocalProvider) Name() string { return "external-spoofed-provider" } func (cmdutilLocalProvider) ResolveAccount(context.Context) (*credential.Account, error) { return nil, nil } @@ -53,17 +55,30 @@ func TestIsBuiltinProvider_StdlibTypeIsNotBuiltin(t *testing.T) { func TestIsBuiltinProvider_PkgPathNotSpoofableByName(t *testing.T) { // Name() returns a string, but classification uses reflect.Type.PkgPath - // which is compile-time fixed. Confirm by using a locally-defined type - // that returns a name mimicking an external provider. + // which is compile-time fixed. The local type returns a name that looks + // like an ISV provider; it must still classify as builtin. p := &cmdutilLocalProvider{} - if p.Name() != "local" { - t.Fatalf("sanity check: Name() = %q", p.Name()) + if p.Name() != "external-spoofed-provider" { + t.Fatalf("sanity check: Name() = %q, spoof value lost", p.Name()) } if !isBuiltinProvider(p) { t.Fatal("isBuiltinProvider should decide by PkgPath, not Name()") } } +// TestIsBuiltinProvider_NonPointerValues covers the non-pointer reflect branch. +// The existing tests only exercise pointer receivers (&T{}); when a provider +// is passed by value the reflect.Kind is not Ptr and t.Elem() is skipped. +func TestIsBuiltinProvider_NonPointerValues(t *testing.T) { + if !isBuiltinProvider(cmdutilLocalProvider{}) { + t.Fatal("non-pointer local type should be builtin (PkgPath still under official module)") + } + // http.Server as a non-pointer — PkgPath "net/http", not under official. + if isBuiltinProvider(http.Server{}) { + t.Fatal("non-pointer stdlib type should not be builtin") + } +} + // TestIsBuiltinProvider_RealBuiltinProviders locks down the classification // for the concrete providers enumerated in design doc §3.3.2 as "官方自带": // env credential provider and local fileio provider. If any of these is @@ -107,6 +122,107 @@ func TestComputeBuildKind_ReturnsKnownValue(t *testing.T) { } } +// --------------------------------------------------------------------------- +// classifyBuild — pure branching logic +// --------------------------------------------------------------------------- +// +// These tests cover every branch of classifyBuild with explicit inputs, +// which is impossible from computeBuildKind alone because debug.ReadBuildInfo +// and the process-wide provider registries can't be reshaped in a test. + +func TestClassifyBuild_NoBuildInfo_ReturnsUnknown(t *testing.T) { + if got := classifyBuild("", false, nil, nil, nil); got != BuildKindUnknown { + t.Fatalf("classifyBuild(haveBuildInfo=false) = %q, want %q", got, BuildKindUnknown) + } +} + +func TestClassifyBuild_ExtendedMainPath_ReturnsExtended(t *testing.T) { + cases := []string{ + "github.com/acme/lark-cli-wrapper", + "example.com/isv/lark", + "gitlab.mycorp.internal/tools/lark-cli-fork", + } + for _, mp := range cases { + t.Run(mp, func(t *testing.T) { + if got := classifyBuild(mp, true, nil, nil, nil); got != BuildKindExtended { + t.Fatalf("mainPath=%q classifyBuild = %q, want %q", mp, got, BuildKindExtended) + } + }) + } +} + +func TestClassifyBuild_OfficialMainPath_NoProviders_ReturnsOfficial(t *testing.T) { + if got := classifyBuild(officialModulePath, true, nil, nil, nil); got != BuildKindOfficial { + t.Fatalf("classifyBuild(official, no providers) = %q, want %q", got, BuildKindOfficial) + } +} + +func TestClassifyBuild_EmptyMainPath_DoesNotTriggerExtended(t *testing.T) { + // An empty Main.Path (rare, e.g. `go run` pre-1.18) must not be treated + // as extended by itself — the classifier falls through to provider checks. + if got := classifyBuild("", true, nil, nil, nil); got != BuildKindOfficial { + t.Fatalf("classifyBuild(empty mainPath, no providers) = %q, want %q", got, BuildKindOfficial) + } +} + +func TestClassifyBuild_NonBuiltinCredentialProvider_ReturnsExtended(t *testing.T) { + // Any non-builtin credential provider flips the verdict to extended. + got := classifyBuild(officialModulePath, true, []any{&http.Server{}}, nil, nil) + if got != BuildKindExtended { + t.Fatalf("classifyBuild with external credential = %q, want %q", got, BuildKindExtended) + } +} + +func TestClassifyBuild_MixedCredentialProviders_ExtendedWins(t *testing.T) { + // Even if most providers are builtin, a single external one decides. + providers := []any{&cmdutilLocalProvider{}, &http.Server{}} + if got := classifyBuild(officialModulePath, true, providers, nil, nil); got != BuildKindExtended { + t.Fatalf("classifyBuild mixed providers = %q, want %q", got, BuildKindExtended) + } +} + +func TestClassifyBuild_NonBuiltinTransportProvider_ReturnsExtended(t *testing.T) { + got := classifyBuild(officialModulePath, true, nil, &http.Server{}, nil) + if got != BuildKindExtended { + t.Fatalf("classifyBuild with external transport = %q, want %q", got, BuildKindExtended) + } +} + +func TestClassifyBuild_NonBuiltinFileioProvider_ReturnsExtended(t *testing.T) { + got := classifyBuild(officialModulePath, true, nil, nil, &http.Server{}) + if got != BuildKindExtended { + t.Fatalf("classifyBuild with external fileio = %q, want %q", got, BuildKindExtended) + } +} + +func TestClassifyBuild_AllBuiltinProviders_ReturnsOfficial(t *testing.T) { + // All three slots filled with builtin providers must still classify as official. + got := classifyBuild( + officialModulePath, true, + []any{&cmdutilLocalProvider{}}, + &cmdutilLocalProvider{}, + &cmdutilLocalProvider{}, + ) + if got != BuildKindOfficial { + t.Fatalf("classifyBuild all-builtin = %q, want %q", got, BuildKindOfficial) + } +} + +// TestClassifyBuild_MainPathPriorityOverProviders documents that the main +// module path takes precedence: even with only builtin providers, a non- +// official main path still yields extended. +func TestClassifyBuild_MainPathPriorityOverProviders(t *testing.T) { + got := classifyBuild( + "github.com/acme/lark-wrapper", true, + []any{&cmdutilLocalProvider{}}, + &cmdutilLocalProvider{}, + &cmdutilLocalProvider{}, + ) + if got != BuildKindExtended { + t.Fatalf("main-path override failed: got %q, want %q", got, BuildKindExtended) + } +} + // --------------------------------------------------------------------------- // DetectBuildKind — sync.Once caching // --------------------------------------------------------------------------- diff --git a/internal/cmdutil/transport_test.go b/internal/cmdutil/transport_test.go index b9a097d89..f76b0c326 100644 --- a/internal/cmdutil/transport_test.go +++ b/internal/cmdutil/transport_test.go @@ -330,6 +330,35 @@ func TestBuildHeaderTransport_OverridesEvenWithoutTamper(t *testing.T) { } } +// TestBuildHeaderTransport_NilBase_UsesFallback verifies that when Base is nil, +// the transport still sets X-Cli-Build and routes the request through +// util.FallbackTransport rather than panicking. This covers the fallback +// branch in RoundTrip that is otherwise unreachable with a non-nil Base. +func TestBuildHeaderTransport_NilBase_UsesFallback(t *testing.T) { + var receivedBuild string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedBuild = r.Header.Get(HeaderBuild) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + transport := &BuildHeaderTransport{Base: nil} + client := &http.Client{Transport: transport} + + req, _ := http.NewRequest("GET", srv.URL, nil) + resp, err := client.Do(req) + if err != nil { + t.Fatalf("request via nil-Base transport failed: %v", err) + } + resp.Body.Close() + + want := DetectBuildKind() + if receivedBuild != want { + t.Fatalf("%s = %q, want %q (header must be set even on nil-Base path)", + HeaderBuild, receivedBuild, want) + } +} + // interceptorFunc adapts a function to exttransport.Interceptor. type interceptorFunc func(*http.Request) func(*http.Response, error)