diff --git a/internal/README.md b/internal/README.md index 92c01b5..0d460fb 100644 --- a/internal/README.md +++ b/internal/README.md @@ -34,6 +34,7 @@ These apply to every client-mode command. Each command selects the agent it talk | `--agent-card ` | `-a` | Agent Card reference: a host/origin (the well-known path is appended), a full card URL, or a local file path. The card is resolved and a transport negotiated. | | `--endpoint ` | `-e` | Agent interface URL for a direct connection, skipping card resolution. Must be paired with exactly one `--transport`. Mutually exclusive with `--agent-card`. | | `--transport ` | | Transport preference: `rest`, `jsonrpc`, `grpc`. Repeatable and ordered (highest preference first). With `--agent-card` it overrides the card's preference order; with `--endpoint` exactly one is required. | +| `--insecure` | | Use plaintext for the built-in gRPC client. By default it uses TLS and verifies the server certificate against the system trust store. | | `--output ` | `-o` | Output format: `text` (default), `json` (indented), or `jsonl` (one compact JSON object per line). | | `--svc-param ` | | Service parameter (repeatable). The chosen transport defines how it's passed. Split on the first `=`. | | `--auth ` | | Shorthand for `--svc-param "Authorization="`. | diff --git a/internal/cli/client.go b/internal/cli/client.go index 6a88570..98207d0 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -22,6 +22,7 @@ import ( "strings" "google.golang.org/grpc" + "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "github.com/a2aproject/a2a-cli/internal/clierr" @@ -35,7 +36,7 @@ import ( a2agrpc "github.com/a2aproject/a2a-go/v2/a2agrpc/v1" ) -const insecureCredentialWarning = "warning: sending a credential with TLS verification disabled (--insecure); the token may be exposed." +const insecureCredentialWarning = "warning: --insecure disables TLS for gRPC; a credential sent over gRPC may be exposed." // warnInsecureCredential warns once when a credential is sent with --insecure. func warnInsecureCredential(cfg *globalConfig) { @@ -68,12 +69,7 @@ func newClientFromEndpoint(ctx context.Context, cfg *globalConfig, ref string, e if err != nil { return nil, err } - endpointURL := ref - if protocol == a2a.TransportProtocolGRPC { - endpointURL = stripHTTPScheme(ref) - } - - cfg.logf("connecting directly to %s via %s (skipping card resolution)", endpointURL, protocol) + cfg.logf("connecting directly to %s via %s (skipping card resolution)", ref, protocol) factoryOpts := append(clientFactoryOpts(cfg), extraOpts...) if !transportplugin.IsBuiltin(protocol) { @@ -84,12 +80,11 @@ func newClientFromEndpoint(ctx context.Context, cfg *globalConfig, ref string, e factoryOpts = append(factoryOpts, pluginOpt) } - endpoint := a2a.NewAgentInterface(endpointURL, protocol) + endpoint := a2a.NewAgentInterface(ref, protocol) if cfg.a2aVersion != "" { endpoint.ProtocolVersion = a2a.ProtocolVersion(cfg.a2aVersion) } - client, err := a2aclient.NewFromEndpoints(ctx, []*a2a.AgentInterface{endpoint}, factoryOpts...) - return client, hintInsecure(err) + return a2aclient.NewFromEndpoints(ctx, []*a2a.AgentInterface{endpoint}, factoryOpts...) } // newClientFromCard resolves the Agent Card and builds a client for it, honoring @@ -117,8 +112,7 @@ func newClientFromCard(ctx context.Context, cfg *globalConfig, ref string, extra )) } cfg.logf("creating client for %s", card.Name) - client, err := a2aclient.NewFromCard(ctx, card, factoryOpts...) - return client, hintInsecure(err) + return a2aclient.NewFromCard(ctx, card, factoryOpts...) } // resolveCard fetches the agent card with every service parameter sent as a request @@ -149,19 +143,12 @@ func cardResolver(cfg *globalConfig) *agentcard.Resolver { return resolver } -// hintInsecure wraps gRPC "no transport security set" errors with a user-friendly suggestion to pass --insecure. -func hintInsecure(err error) error { - if err != nil && strings.Contains(err.Error(), "no transport security set") { - return fmt.Errorf("%w\n\nhint: pass --insecure to allow plaintext gRPC connections", err) - } - return err -} - func clientFactoryOpts(cfg *globalConfig) []a2aclient.FactoryOption { - var grpcOpts []grpc.DialOption + creds := credentials.NewTLS(nil) if cfg.insecureGRPC { - grpcOpts = append(grpcOpts, grpc.WithTransportCredentials(insecure.NewCredentials())) + creds = insecure.NewCredentials() } + grpcOpt := grpc.WithTransportCredentials(creds) // The a2a-go default HTTP client caps every request at 3 minutes. Use one without a // total timeout so the --timeout context deadline is what bounds a request. httpClient := &http.Client{Transport: cfg.httpTransport} @@ -174,7 +161,7 @@ func clientFactoryOpts(cfg *globalConfig) []a2aclient.FactoryOption { opts, a2aclient.WithRESTTransport(httpClient), a2aclient.WithJSONRPCTransport(httpClient), - a2agrpc.WithGRPCTransport(grpcOpts...), + a2aclient.WithTransport(a2a.TransportProtocolGRPC, grpcTransportFactory(a2agrpc.NewGRPCTransport, grpcOpt)), ) } if cfg.a2aVersion == "" || cfg.a2aVersion == "0.3" { @@ -182,12 +169,25 @@ func clientFactoryOpts(cfg *globalConfig) []a2aclient.FactoryOption { opts, a2av0.WithRESTTransport(a2av0.RESTTransportConfig{Client: httpClient}), a2av0.WithJSONRPCTransport(a2av0.JSONRPCTransportConfig{Client: httpClient}), - a2agrpcv0.WithGRPCTransport(grpcOpts...), + a2aclient.WithCompatTransport(a2av0.Version, a2a.TransportProtocolGRPC, grpcTransportFactory(a2agrpcv0.NewGRPCTransport, grpcOpt)), ) } return opts } +// grpcTransportFactory dials the host and port of a gRPC interface URL, which +// cards may advertise with an http(s) scheme. The a2agrpc options pass the URL +// to grpc.NewClient as is, which cannot resolve such a target. +func grpcTransportFactory(newTransport func(*grpc.ClientConn) a2aclient.Transport, opts ...grpc.DialOption) a2aclient.TransportFactory { + return a2aclient.TransportFactoryFn(func(_ context.Context, _ *a2a.AgentCard, iface *a2a.AgentInterface) (a2aclient.Transport, error) { + conn, err := grpc.NewClient(stripHTTPScheme(iface.URL), opts...) + if err != nil { + return nil, err + } + return newTransport(conn), nil + }) +} + func stripHTTPScheme(raw string) string { u, err := url.Parse(raw) if err != nil || (u.Scheme != "http" && u.Scheme != "https") { diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go new file mode 100644 index 0000000..e896757 --- /dev/null +++ b/internal/cli/client_test.go @@ -0,0 +1,126 @@ +// Copyright 2026 The A2A Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cli + +import ( + "crypto/x509" + "fmt" + "net" + "net/http/httptest" + "os" + "os/exec" + "strings" + "testing" + + "google.golang.org/grpc" + + "github.com/a2aproject/a2a-cli/internal/clierr" + "github.com/a2aproject/a2a-cli/internal/localsrv" + "github.com/a2aproject/a2a-cli/internal/testutil" + "github.com/a2aproject/a2a-go/v2/a2a" + a2agrpcv0 "github.com/a2aproject/a2a-go/v2/a2agrpc/v0" + a2agrpc "github.com/a2aproject/a2a-go/v2/a2agrpc/v1" + "github.com/a2aproject/a2a-go/v2/a2asrv" +) + +func TestGRPCTransportSecurity(t *testing.T) { + t.Parallel() + // SetFallbackRoots changes the whole process's trust store. + const child = "A2ACLI_TEST_GRPC_ROOTS" + if os.Getenv(child) != "1" { + cmd := exec.CommandContext(t.Context(), os.Args[0], "-test.run=^TestGRPCTransportSecurity$") + cmd.Env = append(os.Environ(), child+"=1", "GODEBUG=x509usefallbackroots=1") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("gRPC transport tests: %v\n%s", err, out) + } + return + } + + handler := a2asrv.NewHandler(localsrv.NewEchoExecutor()) + srv := grpc.NewServer() + t.Cleanup(srv.Stop) + a2agrpc.NewHandler(handler).RegisterWith(srv) + a2agrpcv0.NewHandler(handler).RegisterWith(srv) + + secured := httptest.NewUnstartedServer(srv) + secured.EnableHTTP2 = true + secured.StartTLS() + t.Cleanup(secured.Close) + + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen() error = %v", err) + } + go func() { + if err := srv.Serve(lis); err != nil { + t.Errorf("srv.Serve() error = %v", err) + } + }() + plainAddr := lis.Addr().String() + + _, err = runCMD(t, "send", "--endpoint", secured.URL, "--transport", "grpc", "--timeout", "3s", "hi") + if err == nil || !strings.Contains(err.Error(), "certificate") { + t.Fatalf("send with untrusted certificate error = %v, want certificate verification failure", err) + } + + roots := x509.NewCertPool() + roots.AddCert(secured.Certificate()) + x509.SetFallbackRoots(roots) + wrongHost := fmt.Sprintf("passthrough:///localhost:%d", secured.Listener.Addr().(*net.TCPAddr).Port) + cardPath := func(address string) string { + return testutil.MustWriteTmpCardFile(t, &a2a.AgentCard{ + Name: "gRPC Echo", Version: "1.0.0", + SupportedInterfaces: []*a2a.AgentInterface{a2a.NewAgentInterface(address, a2a.TransportProtocolGRPC)}, + }) + } + + tests := []struct { + name string + args []string + wantErr string + wantHint string + }{ + {name: "TLS endpoint", args: []string{"--endpoint", secured.URL}}, + {name: "TLS Agent Card", args: []string{"--agent-card", cardPath(secured.URL)}}, + {name: "TLS 0.3 endpoint", args: []string{"--endpoint", secured.URL, "--a2a-version", "0.3"}}, + {name: "TLS Agent Card with bare address", args: []string{"--agent-card", cardPath(secured.Listener.Addr().String())}}, + {name: "plaintext Agent Card", args: []string{"--agent-card", cardPath(plainAddr), "--insecure"}}, + {name: "explicit plaintext", args: []string{"--endpoint", "http://" + plainAddr, "--insecure"}}, + {name: "mismatched hostname", args: []string{"--endpoint", wrongHost}, wantErr: "certificate"}, + {name: "plaintext server without --insecure", args: []string{"--endpoint", "http://" + plainAddr}, wantErr: "TLS handshake", wantHint: "--insecure"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + args := append([]string{"send", "--transport", "grpc", "--timeout", "3s", "-o", "json", "hello"}, tc.args...) + out, err := runCMD(t, args...) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("a2a %s error = %v, want %q", strings.Join(args, " "), err, tc.wantErr) + } + if hint := clierr.Classify(err).Hint; !strings.Contains(hint, tc.wantHint) { + t.Fatalf("a2a %s hint = %q, want %q", strings.Join(args, " "), hint, tc.wantHint) + } + return + } + if err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) + } + if got := testutil.AllArtifactText(mustDecodeTask(t, out)); got != "hello" { + t.Fatalf("a2a %s artifact text = %q, want %q", strings.Join(args, " "), got, "hello") + } + }) + } +} diff --git a/internal/clierr/clierr.go b/internal/clierr/clierr.go index 4e640be..c02f7d5 100644 --- a/internal/clierr/clierr.go +++ b/internal/clierr/clierr.go @@ -23,6 +23,7 @@ import ( "io" "net" "net/http" + "strings" "syscall" "github.com/a2aproject/a2a-go/v2/a2a" @@ -78,6 +79,10 @@ func (e *Error) Unwrap() error { return e.err } // agents, whose bundled skill may lag the installed binary's commands and flags. const UsageHint = "if you are an agent, your a2a skill may be out of date — run `a2a skill` for the current commands and update your local copy" +// plaintextServerHint is remediation for a TLS handshake that reached a server +// without TLS, such as a local agent serving plaintext gRPC. +const plaintextServerHint = "the agent may not use TLS; pass --insecure to allow plaintext gRPC connections" + // Usage builds a usage error for invalid arguments, flags, or flag combinations. func Usage(msg string) *Error { return &Error{Code: CodeUsage, Message: msg, Hint: UsageHint, Exit: 2} @@ -177,6 +182,18 @@ func Classify(err error) *Error { } } + // gRPC reports a failed TLS handshake as a status message, which a2a-go turns + // into an a2a.Error, so a server answering in plaintext is recognized by text. + if strings.Contains(msg, "first record does not look like a TLS handshake") { + return &Error{ + Code: CodeIO, + Message: msg, + Exit: 3, + Hint: plaintextServerHint, + err: err, + } + } + var a2aErr *a2a.Error if errors.As(err, &a2aErr) { return &Error{ diff --git a/internal/clierr/clierr_test.go b/internal/clierr/clierr_test.go index f4e6579..641b414 100644 --- a/internal/clierr/clierr_test.go +++ b/internal/clierr/clierr_test.go @@ -140,6 +140,35 @@ func TestClassify(t *testing.T) { } } +func TestClassifyPlaintextServer(t *testing.T) { + t.Parallel() + + const handshake = `connection error: desc = "transport: authentication handshake failed: tls: first record does not look like a TLS handshake"` + tests := []struct { + name string + err error + }{ + {name: "gRPC status converted to an a2a error", err: fmt.Errorf("failed to send message: %w", a2a.NewError(a2a.ErrInternalError, handshake))}, + {name: "transport error", err: fmt.Errorf("failed to send message: %w", errors.New(handshake))}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := Classify(tt.err) + if got.Code != CodeIO { + t.Fatalf("Classify(%v).Code = %q, want %q", tt.err, got.Code, CodeIO) + } + if got.Exit != 3 { + t.Fatalf("Classify(%v).Exit = %d, want 3", tt.err, got.Exit) + } + if got.Hint != plaintextServerHint { + t.Fatalf("Classify(%v).Hint = %q, want %q", tt.err, got.Hint, plaintextServerHint) + } + }) + } +} + func TestClassifyNil(t *testing.T) { t.Parallel() if got := Classify(nil); got != nil {