Skip to content
Merged
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
16 changes: 14 additions & 2 deletions internal/cmdutil/factory_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,19 @@ func safeRedirectPolicy(req *http.Request, via []*http.Request) error {
return nil
}

// warnIfProxied is a test seam for the proxy-warning gate. Production wires it
// to transport.WarnIfProxied; tests swap in a spy to count invocations. It is
// needed because the real function is guarded by an internal sync.Once, so
// calling it directly would only fire on the first test (see
// factory_proxy_warn_test.go). The terminal check is the IOStreams
// .StderrIsTerminal field, which tests set directly.
var warnIfProxied = transport.WarnIfProxied

func cachedHttpClientFunc(f *Factory) func() (*http.Client, error) {
return sync.OnceValues(func() (*http.Client, error) {
transport.WarnIfProxied(f.IOStreams.ErrOut)
if f.IOStreams.StderrIsTerminal {
warnIfProxied(f.IOStreams.ErrOut)
}

var rt http.RoundTripper = transport.Shared()
rt = &RetryTransport{Base: rt}
Expand All @@ -129,7 +139,9 @@ func cachedLarkClientFunc(f *Factory) func() (*lark.Client, error) {
lark.WithLogLevel(larkcore.LogLevelError),
lark.WithHeaders(BaseSecurityHeaders()),
}
transport.WarnIfProxied(f.IOStreams.ErrOut)
if f.IOStreams.StderrIsTerminal {
warnIfProxied(f.IOStreams.ErrOut)
}
opts = append(opts, lark.WithHttpClient(&http.Client{
Transport: buildSDKTransport(),
CheckRedirect: safeRedirectPolicy,
Expand Down
85 changes: 85 additions & 0 deletions internal/cmdutil/factory_proxy_warn_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

package cmdutil

import (
"io"
"testing"

_ "github.com/larksuite/cli/extension/credential/env" // registers the env-backed account provider
"github.com/larksuite/cli/internal/envvars"
)

// installProxyWarnSpy replaces warnIfProxied with a counter for one test and
// restores it on cleanup. Returns a pointer to the call count so the caller can
// assert how many times the warning fired. The terminal state is controlled via
// the IOStreams.StderrIsTerminal field, not a seam.
func installProxyWarnSpy(t *testing.T) *int {
t.Helper()
prevWarn := warnIfProxied
t.Cleanup(func() { warnIfProxied = prevWarn })
calls := 0
warnIfProxied = func(io.Writer) { calls++ }
return &calls
}

var proxyWarnGateCases = []struct {
name string
terminal bool
want int
}{
{"terminal stderr warns once", true, 1},
{"non-terminal stderr stays silent", false, 0},
}

// TestCachedHttpClientFunc_ProxyWarnGate verifies the http-client init path
// invokes WarnIfProxied only when stderr is an interactive terminal.
func TestCachedHttpClientFunc_ProxyWarnGate(t *testing.T) {
for _, tc := range proxyWarnGateCases {
t.Run(tc.name, func(t *testing.T) {
calls := installProxyWarnSpy(t)

fn := cachedHttpClientFunc(&Factory{IOStreams: &IOStreams{
ErrOut: io.Discard, StderrIsTerminal: tc.terminal,
}})
if _, err := fn(); err != nil {
t.Fatalf("http client init: %v", err)
}

if *calls != tc.want {
t.Errorf("WarnIfProxied calls = %d, want %d", *calls, tc.want)
}
})
}
}

// TestCachedLarkClientFunc_ProxyWarnGate verifies the lark-client init path
// invokes WarnIfProxied only when stderr is an interactive terminal. The gate
// runs after ResolveAccount, so an env-backed credential is wired up to let
// account resolution succeed without network or config files.
func TestCachedLarkClientFunc_ProxyWarnGate(t *testing.T) {
for _, tc := range proxyWarnGateCases {
t.Run(tc.name, func(t *testing.T) {
t.Setenv(envvars.CliAppID, "env-app")
t.Setenv(envvars.CliAppSecret, "env-secret")
t.Setenv(envvars.CliDefaultAs, "")
t.Setenv(envvars.CliUserAccessToken, "")
t.Setenv(envvars.CliTenantAccessToken, "")
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())

calls := installProxyWarnSpy(t)

// normalizeStreams copies the struct (out := *s), so the
// StderrIsTerminal field survives into f.IOStreams.
f := NewDefault(&IOStreams{ErrOut: io.Discard, StderrIsTerminal: tc.terminal}, InvocationContext{})
if _, err := cachedLarkClientFunc(f)(); err != nil {
t.Fatalf("lark client init: %v", err)
}

if *calls != tc.want {
t.Errorf("WarnIfProxied calls = %d, want %d", *calls, tc.want)
}
})
}
}
17 changes: 14 additions & 3 deletions internal/cmdutil/iostreams.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,28 @@ type IOStreams struct {
Out io.Writer
ErrOut io.Writer
IsTerminal bool
// StderrIsTerminal reports whether ErrOut is an interactive terminal.
// Advisory warnings written to stderr (e.g. the proxy notice) gate on this
// so they stay out of non-interactive output (pipes, CI, agent runs).
// Computed once in NewIOStreams, mirroring IsTerminal; tests assign it
// directly like cmd/config/bind_test.go does for IsTerminal.
StderrIsTerminal bool
}

// NewIOStreams builds an IOStreams from arbitrary readers/writers.
// IsTerminal is derived from in's underlying *os.File, if any; non-file
// readers (bytes.Buffer, strings.Reader, …) yield IsTerminal=false.
// IsTerminal / StderrIsTerminal are derived from in's / errOut's underlying
// *os.File, if any; non-file streams (bytes.Buffer, strings.Reader, …) yield
// false.
func NewIOStreams(in io.Reader, out, errOut io.Writer) *IOStreams {
isTerminal := false
if f, ok := in.(*os.File); ok {
isTerminal = term.IsTerminal(int(f.Fd()))
}
return &IOStreams{In: in, Out: out, ErrOut: errOut, IsTerminal: isTerminal}
stderrIsTerminal := false
if f, ok := errOut.(*os.File); ok {
stderrIsTerminal = term.IsTerminal(int(f.Fd()))
}
return &IOStreams{In: in, Out: out, ErrOut: errOut, IsTerminal: isTerminal, StderrIsTerminal: stderrIsTerminal}
}

// SystemIO creates an IOStreams wired to the process's standard file descriptors.
Expand Down
14 changes: 2 additions & 12 deletions internal/transport/warn.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"os"
"strings"
"sync"

"github.com/larksuite/cli/internal/envvars"
)

// Proxy environment constants control shared transport proxy behavior.
Expand Down Expand Up @@ -79,16 +77,8 @@ func WarnIfProxied(w io.Writer) {
// Shared), so its warning and disable instructions take precedence.
// Emitting the env-proxy warning here would be misleading: it tells the
// user to set LARK_CLI_NO_PROXY=1, which does NOT disable the plugin proxy.
if addr, caPath, enabled := proxyPluginStatus(); enabled {
fmt.Fprintf(w, "[lark-cli] [WARN] proxy plugin enabled: all requests (including credentials) are forced through %s. To disable, set %s=false or remove %s.\n",
redactProxyURL(addr), envvars.CliProxyEnable, Path())
if strings.TrimSpace(caPath) != "" {
// A custom CA means upstream TLS can be intercepted/inspected by
// the proxy (MITM). Surface it so the operator is aware traffic
// (including Bearer tokens) is decryptable on this host.
fmt.Fprintf(w, "[lark-cli] [WARN] proxy plugin trusts a custom CA (%s); TLS to upstreams can be intercepted/inspected by this proxy.\n",
caPath)
}
if _, _, enabled := proxyPluginStatus(); enabled {
fmt.Fprintln(w, "[lark-cli] [WARN] proxy plugin enabled: all requests are forced through proxy.")
return
}
if os.Getenv(EnvNoProxy) != "" {
Expand Down
58 changes: 13 additions & 45 deletions internal/transport/warn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"strings"
"sync"
"testing"

"github.com/larksuite/cli/internal/envvars"
)

// TestDetectProxyEnv verifies proxy environment detection priority and empty-state behavior.
Expand Down Expand Up @@ -120,9 +118,9 @@ func TestWarnIfProxied_OnlyOnce(t *testing.T) {
}

// TestWarnIfProxied_ProxyPluginEnabled verifies that when proxy plugin mode is
// enabled, the warning describes the plugin proxy and the correct disable method
// (LARKSUITE_CLI_PROXY_ENABLE=false) instead of the misleading LARK_CLI_NO_PROXY
// instruction — even when env proxy and LARK_CLI_NO_PROXY are also set.
// enabled, the warning is a single concise line that does not leak the proxy
// address or give the misleading LARK_CLI_NO_PROXY disable instruction — even
// when env proxy and LARK_CLI_NO_PROXY are also set.
func TestWarnIfProxied_ProxyPluginEnabled(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
unsetProxyPluginEnv(t)
Expand All @@ -140,51 +138,24 @@ func TestWarnIfProxied_ProxyPluginEnabled(t *testing.T) {
WarnIfProxied(&buf)
out := buf.String()

if !strings.Contains(out, "127.0.0.1:3128") {
t.Errorf("warning should mention the plugin proxy address, got: %s", out)
if !strings.Contains(out, "proxy plugin enabled") {
t.Errorf("warning should announce proxy plugin enabled, got: %s", out)
}
// Single line only — no address, CA, or disable hints.
if strings.Count(out, "\n") != 1 {
t.Errorf("warning must be a single line, got: %s", out)
}
if !strings.Contains(out, envvars.CliProxyEnable) {
t.Errorf("warning should mention %s as the disable method, got: %s", envvars.CliProxyEnable, out)
if strings.Contains(out, "127.0.0.1:3128") || strings.Contains(out, "corp-proxy") {
t.Errorf("warning must not leak the proxy address, got: %s", out)
}
if strings.Contains(out, "Set "+EnvNoProxy+"=1") {
t.Errorf("warning must NOT give the misleading %s disable instruction when plugin is enabled, got: %s", EnvNoProxy, out)
}
// No custom CA configured -> no interception warning.
if strings.Contains(out, "custom CA") {
t.Errorf("warning should not mention a custom CA when none is configured, got: %s", out)
}
}

// TestWarnIfProxied_ProxyPluginCustomCAWarns verifies that when a custom CA is
// trusted, the warning surfaces the TLS-interception capability.
func TestWarnIfProxied_ProxyPluginCustomCAWarns(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
unsetProxyPluginEnv(t)
proxyWarningOnce = sync.Once{}

old := proxyPluginStatus
proxyPluginStatus = func() (string, string, bool) {
return "http://127.0.0.1:3128", "/etc/lark/extra_ca.pem", true
}
t.Cleanup(func() { proxyPluginStatus = old })

var buf bytes.Buffer
WarnIfProxied(&buf)
out := buf.String()

if !strings.Contains(out, "custom CA") {
t.Errorf("warning should mention the custom CA, got: %s", out)
}
if !strings.Contains(out, "/etc/lark/extra_ca.pem") {
t.Errorf("warning should include the CA path, got: %s", out)
}
if !strings.Contains(out, "intercept") {
t.Errorf("warning should mention TLS interception, got: %s", out)
}
}

// TestWarnIfProxied_ProxyPluginEnabledRedactsCredentials verifies the plugin
// warning never leaks credentials embedded in the configured proxy address.
// warning never leaks credentials embedded in the configured proxy address —
// the simplified message omits the address entirely.
func TestWarnIfProxied_ProxyPluginEnabledRedactsCredentials(t *testing.T) {
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
unsetProxyPluginEnv(t)
Expand All @@ -204,9 +175,6 @@ func TestWarnIfProxied_ProxyPluginEnabledRedactsCredentials(t *testing.T) {
if strings.Contains(out, "user:") {
t.Errorf("plugin warning leaked username, got: %s", out)
}
if !strings.Contains(out, "***@127.0.0.1:3128") {
t.Errorf("plugin warning should contain redacted proxy URL, got: %s", out)
}
}

// TestRedactProxyURL verifies redaction of proxy credentials across supported formats.
Expand Down
Loading