mirror of
https://github.com/larksuite/cli.git
synced 2026-07-03 14:02:43 +08:00
feat: simplify proxy plugin warning and gate on tty (#1448)
This commit is contained in:
@@ -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}
|
||||
@@ -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,
|
||||
|
||||
85
internal/cmdutil/factory_proxy_warn_test.go
Normal file
85
internal/cmdutil/factory_proxy_warn_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
@@ -10,8 +10,6 @@ import (
|
||||
"os"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/larksuite/cli/internal/envvars"
|
||||
)
|
||||
|
||||
// Proxy environment constants control shared transport proxy behavior.
|
||||
@@ -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) != "" {
|
||||
|
||||
@@ -8,8 +8,6 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/larksuite/cli/internal/envvars"
|
||||
)
|
||||
|
||||
// TestDetectProxyEnv verifies proxy environment detection priority and empty-state behavior.
|
||||
@@ -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)
|
||||
@@ -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)
|
||||
}
|
||||
if !strings.Contains(out, envvars.CliProxyEnable) {
|
||||
t.Errorf("warning should mention %s as the disable method, got: %s", envvars.CliProxyEnable, 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, "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)
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user