Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/config/init_interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ func runCreateAppFlow(ctx context.Context, f *cmdutil.Factory, brandOverride cor
}

// Step 1: Request app registration (begin)
// Use the shared proxy-plugin-aware transport so registration traffic is not
// a bypass of proxy plugin mode.
// Registration is platform traffic, so it must use the provider-aware
// transport as well as the shared proxy configuration.
httpClient := transport.NewHTTPClient(0)
authResp, err := larkauth.RequestAppRegistration(ctx, httpClient, larkBrand, f.IOStreams.ErrOut)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/doctor/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ func networkChecks(ctx context.Context, opts *DoctorOptions, ep core.Endpoints)
}
}

// Use the shared proxy-plugin-aware transport so connectivity checks reflect
// the real egress path (and are blocked when proxy plugin fails closed).
// Connectivity checks are platform traffic and must exercise the same
// provider-aware route as real platform requests.
httpClient := transport.NewHTTPClient(0)
mcpURL := ep.MCP + "/mcp"

Expand Down
61 changes: 61 additions & 0 deletions extension/transport/sidecar/interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@ import (
"net/http"
"testing"

exttransport "github.com/larksuite/cli/extension/transport"
"github.com/larksuite/cli/internal/envvars"
internaltransport "github.com/larksuite/cli/internal/transport"
"github.com/larksuite/cli/sidecar"
)

type roundTripFunc func(*http.Request) (*http.Response, error)

func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
return f(req)
}

// failingBody is a ReadCloser that errors on Read and tracks Close calls.
type failingBody struct {
err error
Expand Down Expand Up @@ -263,3 +272,55 @@ func TestInterceptor_EmptyBody(t *testing.T) {
t.Errorf("body SHA256 = %q, want empty-string SHA256 %q", sha, expectedEmpty)
}
}

func TestLegacySidecarProviderStillHandlesForcedExternalRequests(t *testing.T) {
t.Setenv(envvars.CliAuthProxy, "http://127.0.0.1:16384")
t.Setenv(envvars.CliProxyKey, "test-key")
previousProvider := exttransport.GetProvider()
exttransport.Register(&Provider{})
t.Cleanup(func() { exttransport.Register(previousProvider) })

seen := make(chan *http.Request, 2)
base := roundTripFunc(func(req *http.Request) (*http.Response, error) {
seen <- req.Clone(req.Context())
return &http.Response{StatusCode: http.StatusNoContent, Body: http.NoBody, Request: req}, nil
})
client := internaltransport.ClientForRequestClass(
&http.Client{Transport: internaltransport.NewHTTPPolicyRouter(base, base)},
exttransport.RequestClassExternal,
)

withSentinel, err := http.NewRequest(http.MethodGet, "https://external.example/protected", nil)
if err != nil {
t.Fatal(err)
}
withSentinel.Header.Set("Authorization", "Bearer "+sidecar.SentinelUAT)
resp, err := client.Do(withSentinel)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()

withoutSentinel, err := http.NewRequest(http.MethodGet, "https://external.example/public", nil)
if err != nil {
t.Fatal(err)
}
resp, err = client.Do(withoutSentinel)
if err != nil {
t.Fatal(err)
}
resp.Body.Close()

proxied := <-seen
if proxied.URL.Scheme != "http" || proxied.URL.Host != "127.0.0.1:16384" {
t.Fatalf("sentinel request URL = %s, want sidecar route", proxied.URL)
}
if got := proxied.Header.Get(sidecar.HeaderProxyTarget); got != "https://external.example" {
t.Fatalf("sentinel request proxy target = %q", got)
}

passthrough := <-seen
if got := passthrough.URL.String(); got != "https://external.example/public" {
t.Fatalf("non-sentinel request URL = %q, want unchanged", got)
}
}
21 changes: 21 additions & 0 deletions extension/transport/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,27 @@ type Provider interface {
ResolveInterceptor(ctx context.Context) Interceptor
}

// RequestClass describes the trust boundary of an outbound HTTP request.
// Platform requests target endpoints owned by the CLI's endpoint resolver;
// external requests target user-provided, pre-signed, CDN, registry, or other
// non-platform URLs. Redirect targets are classified again from each hop's
// logical URL; rewriting a host in an interceptor does not add that host to
// the platform endpoint catalog.
type RequestClass string

const (
RequestClassPlatform RequestClass = "platform"
RequestClassExternal RequestClass = "external"
)

// ScopedProvider optionally limits a Provider to selected request classes.
// Providers that do not implement this interface retain the original
// behavior and apply to every request class.
type ScopedProvider interface {
Provider
SupportsRequestClass(RequestClass) bool
}

// Interceptor defines network-layer customization via a pre/post hook pair.
// The built-in transport chain always executes between PreRoundTrip and the
// returned post function, and cannot be skipped or overridden by the extension.
Expand Down
12 changes: 12 additions & 0 deletions internal/auth/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
"github.com/larksuite/cli/internal/transport"
)

var _ transport.RoundTripperDecorator = (*SecurityPolicyTransport)(nil)

// SecurityPolicyTransport is an http.RoundTripper that intercepts all responses
// and checks for security policy errors.
type SecurityPolicyTransport struct {
Expand All @@ -31,6 +33,16 @@
return transport.Fallback()
}

func (t *SecurityPolicyTransport) BaseRoundTripper() http.RoundTripper {
return t.base()

Check warning on line 37 in internal/auth/transport.go

View check run for this annotation

Codecov / codecov/patch

internal/auth/transport.go#L36-L37

Added lines #L36 - L37 were not covered by tests
}

func (t *SecurityPolicyTransport) WithBaseRoundTripper(base http.RoundTripper) http.RoundTripper {
cloned := *t
cloned.Base = base
return &cloned

Check warning on line 43 in internal/auth/transport.go

View check run for this annotation

Codecov / codecov/patch

internal/auth/transport.go#L40-L43

Added lines #L40 - L43 were not covered by tests
}

// RoundTrip implements http.RoundTripper.
func (t *SecurityPolicyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
resp, err := t.base().RoundTrip(req)
Expand Down
3 changes: 3 additions & 0 deletions internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ func (c *APIClient) DoStream(ctx context.Context, req *larkcore.ApiReq, as core.
resp, err := httpClient.Do(httpReq)
if err != nil {
cancel()
if _, ok := errs.ProblemOf(err); ok {
return nil, err
}
return nil, errs.NewNetworkError(classifyNetworkSubtype(err), "stream request failed: %s", err).WithCause(err)
}
resp.Body = &cancelOnCloseBody{ReadCloser: resp.Body, cancel: cancel}
Expand Down
23 changes: 23 additions & 0 deletions internal/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,29 @@ func TestDoStream_TransportFailureSplitsSubtype(t *testing.T) {
}
}

func TestDoStream_PreservesTypedTransportError(t *testing.T) {
policyErr := errs.NewSecurityPolicyError(errs.SubtypeAccessDenied, "blocked redirect")
ac := &APIClient{
HTTP: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
return nil, policyErr
})},
Credential: credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil),
Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu},
}

_, err := ac.DoStream(context.Background(), &larkcore.ApiReq{
HttpMethod: http.MethodGet,
ApiPath: "/open-apis/drive/v1/files/file_token/download",
}, core.AsBot)
problem, ok := errs.ProblemOf(err)
if !ok || problem.Category != errs.CategoryPolicy || problem.Subtype != errs.SubtypeAccessDenied {
t.Fatalf("DoStream() problem = %#v, %v; want policy/access_denied", problem, ok)
}
if !errors.Is(err, policyErr) {
t.Fatal("DoStream() did not preserve the typed transport error")
}
}

// failingTokenResolver always returns TokenUnavailableError, exercising the
// auth/credential failure path through resolveAccessToken.
type failingTokenResolver struct{}
Expand Down
16 changes: 15 additions & 1 deletion internal/cmdutil/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
"github.com/larksuite/cli/errs"
extcred "github.com/larksuite/cli/extension/credential"
"github.com/larksuite/cli/extension/fileio"
exttransport "github.com/larksuite/cli/extension/transport"
"github.com/larksuite/cli/internal/client"
"github.com/larksuite/cli/internal/core"
"github.com/larksuite/cli/internal/credential"
"github.com/larksuite/cli/internal/keychain"
"github.com/larksuite/cli/internal/transport"
)

// Factory holds shared dependencies injected into every command.
Expand All @@ -31,7 +33,7 @@

type Factory struct {
Config func() (*core.CliConfig, error) // lazily loads app config from Credential
HttpClient func() (*http.Client, error) // HTTP client for non-Lark API calls (with retry and security headers)
HttpClient func() (*http.Client, error) // policy-routed HTTP client for direct requests
LarkClient func() (*lark.Client, error) // Lark SDK client for all Open API calls
IOStreams *IOStreams // stdin/stdout/stderr streams

Expand All @@ -48,6 +50,18 @@
SkillContent fs.FS // embedded skill tree (rooted at the skill list); nil when the build embeds no skills
}

// ExternalHTTPClient returns a clone of the existing Factory client whose
// requests are explicitly classified as external. The underlying client,
// redirect policy, timeout, proxy configuration, and legacy transport provider
// behavior are preserved.
func (f *Factory) ExternalHTTPClient() (*http.Client, error) {
client, err := f.HttpClient()
if err != nil {
return nil, err

Check warning on line 60 in internal/cmdutil/factory.go

View check run for this annotation

Codecov / codecov/patch

internal/cmdutil/factory.go#L60

Added line #L60 was not covered by tests
}
return transport.ClientForRequestClass(client, exttransport.RequestClassExternal), nil
}

// ResolveFileIO resolves a FileIO instance using the current execution context.
// The provider controls whether the returned instance is fresh or cached.
func (f *Factory) ResolveFileIO(ctx context.Context) fileio.FileIO {
Expand Down
Loading
Loading