From 5979ab6aadc259bf62860111fecc6d58dd403140 Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Tue, 27 Oct 2026 07:17:11 +0000 Subject: [PATCH 1/8] feat: add --unix-socket and credential helpers Agent servers reach their harnesses over a Unix socket and hand out per-agent tokens. --unix-socket routes HTTP requests through a socket, for every host or one. --credential-helper runs Bazel credential helpers to get the headers for an agent, and `a2a auth get` shows what a helper returns. --- go.mod | 1 + go.sum | 2 + internal/README.md | 26 +++ internal/cli/auth.go | 38 ++++ internal/cli/auth_get.go | 101 +++++++++ internal/cli/card_get.go | 18 +- internal/cli/client.go | 36 +++- internal/cli/credentials.go | 75 +++++++ internal/cli/credentials_test.go | 272 +++++++++++++++++++++++++ internal/cli/root.go | 12 ++ internal/credhelper/credhelper.go | 170 ++++++++++++++++ internal/credhelper/credhelper_test.go | 155 ++++++++++++++ internal/flagparse/svcparams.go | 47 ++++- internal/flagparse/svcparams_test.go | 49 +++++ 14 files changed, 986 insertions(+), 16 deletions(-) create mode 100644 internal/cli/auth.go create mode 100644 internal/cli/auth_get.go create mode 100644 internal/cli/credentials.go create mode 100644 internal/cli/credentials_test.go create mode 100644 internal/credhelper/credhelper.go create mode 100644 internal/credhelper/credhelper_test.go diff --git a/go.mod b/go.mod index fb68b07..6952bcf 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/a2aproject/a2a-go v0.3.15 github.com/a2aproject/a2a-go/v2 v2.6.0 github.com/google/go-cmp v0.7.0 + github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/joho/godotenv v1.5.1 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index a0d3013..0028932 100644 --- a/go.sum +++ b/go.sum @@ -7,6 +7,8 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= diff --git a/internal/README.md b/internal/README.md index e8d4cf0..e8a984f 100644 --- a/internal/README.md +++ b/internal/README.md @@ -37,6 +37,8 @@ These apply to every client-mode command. Each command selects the agent it talk | `--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="`. | +| `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]command [args...]`. Repeatable. See [Credentials](#credentials). | +| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | | `--verbose` | `-v` | Verbose output to stderr. | @@ -261,6 +263,30 @@ Streams events to stdout until the task reaches a terminal state. Output format --- +## Credentials + +A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. + +Configure helpers with `--credential-helper`, `A2ACLI_CREDENTIAL_HELPER`, or `credential-helper` in a config file: + +```yaml +# ~/.config/a2a-cli/config.yaml +credential-helper: + - a2a-serve credential # every host + - "*.corp.example.com=/usr/local/bin/corp-helper" +``` + +Each value is `[scope=]command [args...]`. A scope is a host (`example.com`), or `*.example.com` for the domain and its subdomains. For each agent, the helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent with the card request and every A2A call, unless `--auth` or `--svc-param` set the same header. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. + +### `auth get` - Inspect credentials + +```bash +a2a auth get https://agent.example.com # header names, values redacted +a2a auth get https://agent.example.com --reveal # header values +``` + +--- + ## Server Mode `a2a server` starts an A2A-compliant server backed by one of three modes. diff --git a/internal/cli/auth.go b/internal/cli/auth.go new file mode 100644 index 0000000..36786e5 --- /dev/null +++ b/internal/cli/auth.go @@ -0,0 +1,38 @@ +// 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 ( + "github.com/spf13/cobra" +) + +func newAuthCmd(cfg *globalConfig) *cobra.Command { + cmd := &cobra.Command{ + Use: "auth", + Short: "Work with the credentials sent to agents", + Long: `Work with the credentials sent to agents. + +A credential helper (--credential-helper, A2ACLI_CREDENTIAL_HELPER, or +credential-helper in the config file) is a program that returns the HTTP +headers to send to an agent, following the Bazel credential helper protocol: +https://github.com/EngFlow/credential-helper-spec. Its value is +"[scope=]command [args...]", where scope is a host such as example.com, or +*.example.com for a domain and its subdomains. The most specific helper for +the agent's host runs as "command args... get", and its headers are sent +unless --auth or --svc-param set the same header.`, + } + cmd.AddCommand(newAuthGetCmd(cfg)) + return cmd +} diff --git a/internal/cli/auth_get.go b/internal/cli/auth_get.go new file mode 100644 index 0000000..d140d7f --- /dev/null +++ b/internal/cli/auth_get.go @@ -0,0 +1,101 @@ +// 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 ( + "context" + "fmt" + "slices" + "strings" + + "github.com/spf13/cobra" + + "github.com/a2aproject/a2a-cli/internal/clierr" + "github.com/a2aproject/a2a-cli/internal/flagparse" +) + +func newAuthGetCmd(cfg *globalConfig) *cobra.Command { + var reveal bool + cmd := &cobra.Command{ + Use: "get ", + Short: "Print the headers the credential helper returns for an agent", + Example: ` a2a auth get https://agent.example.com + a2a auth get https://agent.example.com --reveal`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + var ref flagparse.URLOrPath + if err := ref.Set(args[0]); err != nil { + return err + } + target := ref.URL() + h, ok, err := cfg.credentialHelper(target) + if err != nil { + return clierr.Usage(err.Error()) + } + if !ok { + return fmt.Errorf("no credential helper for %s", target) + } + ctx, cancel := context.WithTimeout(cmd.Context(), cfg.timeout) + defer cancel() + resp, err := h.Get(ctx, target) + if err != nil { + return err + } + headers := resp.Headers + if !reveal { + headers = redactHeaders(headers) + } + if cfg.IsJSON() { + return cfg.PrintJSON(map[string]any{"url": target, "helper": h.Spec, "headers": headers, "expires": resp.Expires}) + } + names := make([]string, 0, len(headers)) + for name := range headers { + names = append(names, name) + } + slices.Sort(names) + rows := make([][]string, 0, len(names)) + for _, name := range names { + rows = append(rows, []string{name, strings.Join(headers[name], ", ")}) + } + if _, err := fmt.Fprintf(cfg.Out, "helper: %s\n", h.Spec); err != nil { + return err + } + if resp.Expires != "" { + if _, err := fmt.Fprintf(cfg.Out, "expires: %s\n", resp.Expires); err != nil { + return err + } + } + if len(rows) == 0 { + _, err := fmt.Fprintln(cfg.Out, "no headers") + return err + } + return cfg.PrintTable([]string{"HEADER", "VALUE"}, rows) + }, + } + cmd.Flags().BoolVar(&reveal, "reveal", false, "Print header values instead of redacting them") + return cmd +} + +func redactHeaders(headers map[string][]string) map[string][]string { + out := make(map[string][]string, len(headers)) + for name, values := range headers { + redacted := make([]string, len(values)) + for i := range values { + redacted[i] = "" + } + out[name] = redacted + } + return out +} diff --git a/internal/cli/card_get.go b/internal/cli/card_get.go index 307d766..e847fd9 100644 --- a/internal/cli/card_get.go +++ b/internal/cli/card_get.go @@ -30,19 +30,23 @@ func newCardGetCmd(cfg *globalConfig) *cobra.Command { cmd := &cobra.Command{ Use: "get [url]", Short: "Fetch and display an agent card", - Args: cobra.MaximumNArgs(1), + // The positional URL is taken while validating args, which runs + // before the root's PersistentPreRunE asks the credential helper. + Args: func(cmd *cobra.Command, args []string) error { + if err := cobra.MaximumNArgs(1)(cmd, args); err != nil { + return err + } + if len(args) == 1 { + return cfg.agentCard.Set(args[0]) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() ctx, cancel := context.WithTimeout(ctx, cfg.timeout) defer cancel() - if len(args) == 1 { - if err := cfg.agentCard.Set(args[0]); err != nil { - return err - } - } - var card *a2a.AgentCard var err error if extended { diff --git a/internal/cli/client.go b/internal/cli/client.go index 0e14a2d..b081915 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -17,6 +17,7 @@ package cli import ( "context" "fmt" + "net" "net/http" "net/url" "strings" @@ -46,7 +47,7 @@ const insecureCredentialWarning = "warning: sending a credential with TLS verifi // warnInsecureCredential warns once when a credential is sent with --insecure. func warnInsecureCredential(cfg *globalConfig) { - if cfg.insecureGRPC && cfg.svcParams.HasCredential() { + if cfg.insecureGRPC && cfg.svcParams.SendsCredential() { _, _ = fmt.Fprintln(cfg.stderr(), insecureCredentialWarning) } } @@ -136,7 +137,12 @@ func resolveCard(ctx context.Context, cfg *globalConfig, ref string) (*a2a.Agent opts = append(opts, agentcard.WithRequestHeader(k, strings.Join(vals, ", "))) } cfg.logf("resolving agent card from %s", ref) - card, err := compatCardResolver.Resolve(ctx, ref, opts...) + resolver := compatCardResolver + if cfg.unixSocket != "" { + resolver = agentcard.NewResolver(&http.Client{Transport: cfg.httpTransport, Timeout: 30 * time.Second}) + resolver.CardParser = a2av0.NewAgentCardParser() + } + card, err := resolver.Resolve(ctx, ref, opts...) if err != nil { return nil, clierr.CardResolution(err) } @@ -201,3 +207,29 @@ func destroyClient(cfg *globalConfig, client *a2aclient.Client) { cfg.logf("failed to destroy client: %v", err) } } + +// unixSocketTransport sends HTTP requests through a Unix socket. spec is +// , for every host, or =, for requests to host only. +func unixSocketTransport(spec string) http.RoundTripper { + host, path, scoped := strings.Cut(spec, "=") + if !scoped { + host, path = "", spec + } + viaSocket := func(h string) bool { return host == "" || h == host } + t := http.DefaultTransport.(*http.Transport).Clone() + proxy := t.Proxy + t.Proxy = func(r *http.Request) (*url.URL, error) { + if viaSocket(r.URL.Hostname()) || proxy == nil { + return nil, nil + } + return proxy(r) + } + var d net.Dialer + t.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + if h, _, err := net.SplitHostPort(addr); err == nil && viaSocket(h) { + return d.DialContext(ctx, "unix", path) + } + return d.DialContext(ctx, network, addr) + } + return t +} diff --git a/internal/cli/credentials.go b/internal/cli/credentials.go new file mode 100644 index 0000000..4009a3a --- /dev/null +++ b/internal/cli/credentials.go @@ -0,0 +1,75 @@ +// 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 ( + "context" + "fmt" + "net/url" + "slices" + + "github.com/a2aproject/a2a-cli/internal/credhelper" +) + +// targetURL returns the HTTP(S) URL of the agent the command talks to, or +// "" when there is none, such as for a local card file. +func (g *globalConfig) targetURL() string { + target := g.url + if target == "" && g.agentCard.IsSet() { + target = g.agentCard.URL() + } + u, err := url.Parse(target) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return "" + } + return target +} + +// credentialHelper returns the --credential-helper for target. +func (g *globalConfig) credentialHelper(target string) (credhelper.Helper, bool, error) { + helpers, err := credhelper.ParseAll(g.credentialHelpers) + if err != nil { + return credhelper.Helper{}, false, err + } + h, ok := credhelper.Select(helpers, target) + return h, ok, nil +} + +// applyCredentialHelper sends the headers the credential helper returns for +// the target agent, unless --auth or --svc-param set the same header. +func applyCredentialHelper(ctx context.Context, cfg *globalConfig) error { + target := cfg.targetURL() + if target == "" || len(cfg.credentialHelpers) == 0 { + return nil + } + h, ok, err := cfg.credentialHelper(target) + if err != nil || !ok { + return err + } + cfg.logf("asking credential helper %q for %s", h.Spec, target) + resp, err := h.Get(ctx, target) + if err != nil { + return fmt.Errorf("getting credentials for %s: %w", target, err) + } + names := make([]string, 0, len(resp.Headers)) + for name := range resp.Headers { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + cfg.svcParams.SetDefault(name, resp.Headers[name]...) + } + return nil +} diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go new file mode 100644 index 0000000..2a66048 --- /dev/null +++ b/internal/cli/credentials_test.go @@ -0,0 +1,272 @@ +// 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 ( + "context" + "encoding/json" + "iter" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + + "github.com/a2aproject/a2a-go/v2/a2a" + "github.com/a2aproject/a2a-go/v2/a2asrv" + "github.com/google/go-cmp/cmp" + + "github.com/a2aproject/a2a-cli/internal/localsrv" +) + +// authRecorder records the Authorization header of every request. +type authRecorder struct { + mu sync.Mutex + seen []string +} + +func (r *authRecorder) wrap(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + r.mu.Lock() + r.seen = append(r.seen, req.Header.Get("Authorization")) + r.mu.Unlock() + h.ServeHTTP(w, req) + }) +} + +func (r *authRecorder) all() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.seen...) +} + +func startAuthRecordingServer(t *testing.T) (string, *authRecorder) { + t.Helper() + capabilities := a2a.AgentCapabilities{Streaming: true} + handler := a2asrv.NewHandler(localsrv.NewEchoExecutor(), a2asrv.WithCapabilityChecks(&capabilities)) + mux := http.NewServeMux() + mux.Handle("/", a2asrv.NewRESTHandler(handler)) + rec := &authRecorder{} + server := httptest.NewServer(rec.wrap(mux)) + t.Cleanup(server.Close) + mux.Handle(a2asrv.WellKnownAgentCardPath, a2asrv.NewStaticAgentCardHandler(newAgentCard(server.URL, capabilities))) + return server.URL, rec +} + +// writeCredentialHelper writes a helper that answers "get" with the +// Authorization header "Bearer ". +func writeCredentialHelper(t *testing.T) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("helper scripts need a POSIX shell") + } + path := filepath.Join(t.TempDir(), "helper") + script := `#!/bin/sh +[ "$1" = get ] || exit 2 +uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/') +printf '{"headers":{"Authorization":["Bearer %s"]}}' "$uri" +` + if err := os.WriteFile(path, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + return path +} + +func TestCredentialHelper(t *testing.T) { + t.Parallel() + helper := writeCredentialHelper(t) + tests := []struct { + name string + args func(url string) []string + wantAuth func(url string) string + wantErr string + }{ + { + name: "send", + args: func(url string) []string { return []string{"send", "-a", url, "--credential-helper", helper, "hello"} }, + wantAuth: func(url string) string { return "Bearer " + url }, + }, + { + name: "card get positional", + args: func(url string) []string { return []string{"card", "get", url, "--credential-helper", helper} }, + wantAuth: func(url string) string { return "Bearer " + url }, + }, + { + name: "scoped to the agent host", + args: func(url string) []string { + return []string{"send", "-a", url, "--credential-helper", "127.0.0.1=" + helper, "hello"} + }, + wantAuth: func(url string) string { return "Bearer " + url }, + }, + { + name: "scoped to another host", + args: func(url string) []string { + return []string{"send", "-a", url, "--credential-helper", "other.example.com=" + helper, "hello"} + }, + wantAuth: func(string) string { return "" }, + }, + { + name: "explicit auth wins", + args: func(url string) []string { + return []string{"send", "-a", url, "--credential-helper", helper, "--auth", "Bearer flag", "hello"} + }, + wantAuth: func(string) string { return "Bearer flag" }, + }, + { + name: "failing helper", + args: func(url string) []string { return []string{"send", "-a", url, "--credential-helper", "false", "hello"} }, + wantErr: "credential helper", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + url, rec := startAuthRecordingServer(t) + args := tc.args(url) + _, 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) + } + return + } + if err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) + } + seen := rec.all() + if len(seen) == 0 { + t.Fatalf("a2a %s sent no requests", strings.Join(args, " ")) + } + for _, got := range seen { + if want := tc.wantAuth(url); got != want { + t.Fatalf("a2a %s Authorization = %q, want %q", strings.Join(args, " "), got, want) + } + } + }) + } +} + +func TestAuthGet(t *testing.T) { + t.Parallel() + helper := writeCredentialHelper(t) + tests := []struct { + name string + args []string + want map[string]any + wantErr string + }{ + { + name: "redacted", + args: []string{"auth", "get", "https://agent.example.com", "--credential-helper", helper, "-o", "json"}, + want: map[string]any{"url": "https://agent.example.com", "helper": helper, "headers": map[string]any{"Authorization": []any{""}}, "expires": ""}, + }, + { + name: "revealed", + args: []string{"auth", "get", "https://agent.example.com", "--credential-helper", helper, "--reveal", "-o", "json"}, + want: map[string]any{"url": "https://agent.example.com", "helper": helper, "headers": map[string]any{"Authorization": []any{"Bearer https://agent.example.com"}}, "expires": ""}, + }, + { + name: "no helper for the host", + args: []string{"auth", "get", "https://agent.example.com", "--credential-helper", "other.example.com=" + helper}, + wantErr: "no credential helper", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + out, err := runCMD(t, tc.args...) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("a2a %s error = %v, want %q", strings.Join(tc.args, " "), err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(tc.args, " "), err) + } + var got map[string]any + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("json.Unmarshal(%q) error = %v", out, err) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("a2a %s wrong result (-want +got) diff = %s", strings.Join(tc.args, " "), diff) + } + }) + } +} + +func TestSendOverUnixSocket(t *testing.T) { + t.Parallel() + dir, err := os.MkdirTemp("", "a2a") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.RemoveAll(dir); err != nil { + t.Errorf("removing %s: %v", dir, err) + } + }) + sock := filepath.Join(dir, "a2a.sock") + ln, err := net.Listen("unix", sock) + if err != nil { + t.Fatal(err) + } + const base = "http://agent.invalid" + mux := http.NewServeMux() + mux.Handle("/.well-known/agent-card.json", a2asrv.NewStaticAgentCardHandler(&a2a.AgentCard{ + Name: "socket", + SupportedInterfaces: []*a2a.AgentInterface{a2a.NewAgentInterface(base, a2a.TransportProtocolHTTPJSON)}, + Capabilities: a2a.AgentCapabilities{Streaming: true}, + })) + mux.Handle("/", a2asrv.NewRESTHandler(a2asrv.NewHandler( + a2asrv.AgentExecutorFunc(func(ctx context.Context, ec *a2asrv.ExecutorContext) iter.Seq2[a2a.Event, error] { + return func(yield func(a2a.Event, error) bool) { + yield(a2a.NewMessage(a2a.MessageRoleAgent, a2a.NewTextPart("over the socket")), nil) + } + }), + ))) + server := &httptest.Server{Listener: ln, Config: &http.Server{Handler: mux}} + server.Start() + t.Cleanup(server.Close) + + tests := []struct { + name string + socket string + args []string + wantErr bool + }{ + {name: "agent card", socket: sock, args: []string{"-a", base}}, + {name: "endpoint", socket: sock, args: []string{"-e", base, "--transport", "rest"}}, + {name: "scoped to the host", socket: "agent.invalid=" + sock, args: []string{"-a", base}}, + {name: "scoped to another host", socket: "other.invalid=" + sock, args: []string{"-a", base, "--timeout", "2s"}, wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + args := append(append([]string{"send", "--unix-socket", tc.socket}, tc.args...), "hello") + out, err := runCMD(t, args...) + if (err != nil) != tc.wantErr { + t.Fatalf("a2a %s error = %v, want error %v", strings.Join(args, " "), err, tc.wantErr) + } + if !tc.wantErr && !strings.Contains(out, "over the socket") { + t.Fatalf("a2a %s = %q, want the agent's answer", strings.Join(args, " "), out) + } + }) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index ee29bf6..6cdbd41 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -63,8 +63,11 @@ type globalConfig struct { timeout time.Duration verbose bool insecureGRPC bool + unixSocket string configPath string + credentialHelpers []string + pluginsEnabled bool bindings []clicfg.FlagBinding @@ -156,6 +159,12 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { } cfg.bindings = bindings cfg.store = store + if cfg.unixSocket != "" && cfg.httpTransport == nil { + cfg.httpTransport = unixSocketTransport(cfg.unixSocket) + } + if err := applyCredentialHelper(cmd.Context(), cfg); err != nil { + return err + } switch output.Mode(cfg.output) { case output.ModeText, output.ModeJson, output.ModeJSONL: @@ -179,6 +188,8 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.DurationVar(&cfg.timeout, "timeout", 30*time.Second, "Request timeout") pf.BoolVarP(&cfg.verbose, "verbose", "v", false, "Verbose output to stderr") pf.BoolVar(&cfg.insecureGRPC, "insecure", false, "Use insecure (plaintext) gRPC transport credentials") + pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send HTTP requests through a Unix socket: for every host, or = for one host") + pf.StringArrayVar(&cfg.credentialHelpers, "credential-helper", nil, "Credential helper returning the headers for an agent: [scope=]command [args...] (repeatable; see a2a auth --help)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") cmd.AddCommand( @@ -186,6 +197,7 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { newSendCmd(cfg, deps.poller), newTaskCmd(cfg), newConfigCmd(cfg), + newAuthCmd(cfg), newServeCmd(cfg), newTransportCmd(cfg), newPluginCmd(cfg), diff --git a/internal/credhelper/credhelper.go b/internal/credhelper/credhelper.go new file mode 100644 index 0000000..bc7f83e --- /dev/null +++ b/internal/credhelper/credhelper.go @@ -0,0 +1,170 @@ +// 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 credhelper runs credential helpers: programs that return the HTTP +// headers to send to a URI. It implements the Bazel credential helper protocol +// (https://github.com/EngFlow/credential-helper-spec), so helpers written for +// Bazel work unchanged. +package credhelper + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/url" + "os/exec" + "strings" + "time" + + "github.com/google/shlex" +) + +// Timeout bounds a single helper invocation. +const Timeout = 10 * time.Second + +// Helper is a configured credential helper, from "[scope=]command [args...]". +type Helper struct { + // Spec is the configured value, for messages. + Spec string + // Scope is the host the helper applies to: "example.com", "*.example.com" + // for the domain and its subdomains, or empty for every host. + Scope string + // Command runs the helper; "get" is appended to it. + Command []string +} + +// Response is what a helper prints for a "get" request. +type Response struct { + Headers map[string][]string `json:"headers"` + // Expires is when the headers stop being valid, in RFC 3339. + Expires string `json:"expires,omitempty"` +} + +type request struct { + URI string `json:"uri"` +} + +// Parse parses "[scope=]command [args...]". The scope is the text before the +// first '=' when it looks like a host pattern; the command is split like a +// shell would. +func Parse(spec string) (Helper, error) { + h := Helper{Spec: spec} + rest := strings.TrimSpace(spec) + if scope, cmd, ok := strings.Cut(rest, "="); ok && isScope(scope) { + h.Scope, rest = strings.ToLower(scope), cmd + } + args, err := shlex.Split(rest) + if err != nil { + return Helper{}, fmt.Errorf("credential helper %q: %w", spec, err) + } + if len(args) == 0 { + return Helper{}, fmt.Errorf("credential helper %q: no command", spec) + } + h.Command = args + return h, nil +} + +func isScope(s string) bool { + if s == "" || strings.ContainsAny(s, " \t/\\\"'") { + return false + } + host := strings.TrimPrefix(s, "*.") + return host != "" && !strings.Contains(host, "*") +} + +// ParseAll parses every spec. +func ParseAll(specs []string) ([]Helper, error) { + helpers := make([]Helper, 0, len(specs)) + for _, s := range specs { + h, err := Parse(s) + if err != nil { + return nil, err + } + helpers = append(helpers, h) + } + return helpers, nil +} + +// Select returns the helper for rawURL: the one with the most specific +// matching scope, an exact host before a wildcard and a longer wildcard +// before a shorter one, with unscoped helpers last. Among equally specific +// helpers the last one wins, so a later setting overrides an earlier one. +func Select(helpers []Helper, rawURL string) (Helper, bool) { + u, err := url.Parse(rawURL) + if err != nil || u.Hostname() == "" { + return Helper{}, false + } + host := strings.ToLower(u.Hostname()) + best, bestRank := Helper{}, -1 + for _, h := range helpers { + rank := matchRank(h.Scope, host) + if rank >= bestRank && rank >= 0 { + best, bestRank = h, rank + } + } + return best, bestRank >= 0 +} + +// matchRank is -1 when scope doesn't match host, and higher for more +// specific matches. +func matchRank(scope, host string) int { + switch { + case scope == "": + return 0 + case scope == host: + return 1 << 20 + case strings.HasPrefix(scope, "*."): + domain := scope[2:] + if host == domain || strings.HasSuffix(host, "."+domain) { + return len(domain) + } + } + return -1 +} + +// Get runs the helper for uri. +func (h Helper) Get(ctx context.Context, uri string) (*Response, error) { + ctx, cancel := context.WithTimeout(ctx, Timeout) + defer cancel() + in, err := json.Marshal(request{URI: uri}) + if err != nil { + return nil, err + } + cmd := exec.CommandContext(ctx, h.Command[0], append(h.Command[1:], "get")...) + cmd.Stdin = bytes.NewReader(in) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + if err := cmd.Run(); err != nil { + if msg := strings.TrimSpace(stderr.String()); msg != "" { + return nil, fmt.Errorf("credential helper %q: %w: %s", h.Spec, err, msg) + } + return nil, fmt.Errorf("credential helper %q: %w", h.Spec, err) + } + var resp Response + if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { + return nil, fmt.Errorf("credential helper %q: invalid response: %w", h.Spec, err) + } + if resp.Expires != "" { + if _, err := time.Parse(time.RFC3339, resp.Expires); err != nil { + return nil, fmt.Errorf("credential helper %q: invalid expires: %w", h.Spec, err) + } + } + for name := range resp.Headers { + if name == "" { + return nil, fmt.Errorf("credential helper %q: empty header name", h.Spec) + } + } + return &resp, nil +} diff --git a/internal/credhelper/credhelper_test.go b/internal/credhelper/credhelper_test.go new file mode 100644 index 0000000..5fcc2c3 --- /dev/null +++ b/internal/credhelper/credhelper_test.go @@ -0,0 +1,155 @@ +// 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 credhelper + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestParse(t *testing.T) { + t.Parallel() + tests := []struct { + spec string + want Helper + wantErr bool + }{ + {spec: "a2a-serve credential", want: Helper{Command: []string{"a2a-serve", "credential"}}}, + {spec: "/usr/bin/helper", want: Helper{Command: []string{"/usr/bin/helper"}}}, + {spec: "example.com=/usr/bin/helper", want: Helper{Scope: "example.com", Command: []string{"/usr/bin/helper"}}}, + {spec: "*.Example.com=helper --flag", want: Helper{Scope: "*.example.com", Command: []string{"helper", "--flag"}}}, + {spec: `helper --opt=a\ b`, want: Helper{Command: []string{"helper", "--opt=a b"}}}, + {spec: "/opt/x=y/helper", want: Helper{Command: []string{"/opt/x=y/helper"}}}, + {spec: "example.com=", wantErr: true}, + {spec: "", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.spec, func(t *testing.T) { + t.Parallel() + got, err := Parse(tc.spec) + if (err != nil) != tc.wantErr { + t.Fatalf("Parse(%q) error = %v, want error %v", tc.spec, err, tc.wantErr) + } + if tc.wantErr { + return + } + tc.want.Spec = tc.spec + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("Parse(%q) wrong result (-want +got) diff = %s", tc.spec, diff) + } + }) + } +} + +func TestSelect(t *testing.T) { + t.Parallel() + helpers, err := ParseAll([]string{ + "default", + "*.example.com=wildcard", + "*.dev.example.com=dev-wildcard", + "api.example.com=exact", + "api.example.com=exact-override", + }) + if err != nil { + t.Fatalf("ParseAll() error = %v", err) + } + tests := []struct { + url string + want string + }{ + {url: "https://api.example.com/agent", want: "exact-override"}, + {url: "https://API.example.com:8443", want: "exact-override"}, + {url: "https://foo.dev.example.com", want: "dev-wildcard"}, + {url: "https://foo.example.com", want: "wildcard"}, + {url: "https://example.com", want: "wildcard"}, + {url: "https://other.org", want: "default"}, + } + for _, tc := range tests { + t.Run(tc.url, func(t *testing.T) { + t.Parallel() + got, ok := Select(helpers, tc.url) + if !ok || got.Command[0] != tc.want { + t.Fatalf("Select(%q) = %v, %v, want %s", tc.url, got.Command, ok, tc.want) + } + }) + } + + scoped, err := ParseAll([]string{"example.com=helper"}) + if err != nil { + t.Fatalf("ParseAll() error = %v", err) + } + if got, ok := Select(scoped, "https://other.org"); ok { + t.Fatalf("Select() = %v, want no helper for a host outside every scope", got.Command) + } +} + +func writeHelper(t *testing.T, script string) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("helper scripts need a POSIX shell") + } + path := filepath.Join(t.TempDir(), "helper") + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700); err != nil { + t.Fatal(err) + } + return path +} + +func TestGet(t *testing.T) { + t.Parallel() + tests := []struct { + name string + script string + want *Response + wantErr string + }{ + { + name: "headers for the uri", + script: `[ "$1" = get ] || exit 2; uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/'); printf '{"headers":{"Authorization":["Bearer %s"]},"expires":"2030-01-02T03:04:05Z"}' "$uri"`, + want: &Response{Headers: map[string][]string{"Authorization": {"Bearer https://agent.example.com"}}, Expires: "2030-01-02T03:04:05Z"}, + }, + {name: "no headers", script: `cat >/dev/null; echo '{}'`, want: &Response{}}, + {name: "failure", script: `echo "no token" >&2; exit 1`, wantErr: "no token"}, + {name: "invalid json", script: `echo nope`, wantErr: "invalid response"}, + {name: "invalid expires", script: `echo '{"expires":"tomorrow"}'`, wantErr: "invalid expires"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h, err := Parse(writeHelper(t, tc.script)) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + got, err := h.Get(t.Context(), "https://agent.example.com") + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("Get() error = %v, want %q", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("Get() error = %v", err) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("Get() wrong result (-want +got) diff = %s", diff) + } + }) + } +} diff --git a/internal/flagparse/svcparams.go b/internal/flagparse/svcparams.go index f066ba3..1e9ab22 100644 --- a/internal/flagparse/svcparams.go +++ b/internal/flagparse/svcparams.go @@ -28,8 +28,9 @@ import ( // registers several flags backed by one accumulator; merging happens as flags // are parsed, so Params can be read repeatedly without accumulating duplicates. type ServiceParams struct { - entries []serviceParam - auth string + entries []serviceParam + auth string + defaults []serviceParam } type serviceParam struct { @@ -48,7 +49,7 @@ func (s *ServiceParams) Attach(f *pflag.FlagSet) { // to the result and repeated calls never accumulate duplicates. Returns nil when // nothing was provided. func (s *ServiceParams) Params() a2aclient.ServiceParams { - if len(s.entries) == 0 && s.auth == "" { + if len(s.entries) == 0 && s.auth == "" && len(s.defaults) == 0 { return nil } params := a2aclient.ServiceParams{} @@ -58,22 +59,54 @@ func (s *ServiceParams) Params() a2aclient.ServiceParams { if s.auth != "" { params.Append("Authorization", s.auth) } + for _, d := range s.defaults { + if !s.isSet(d.key) { + params.Append(d.key, d.value) + } + } return params } -// HasCredential reports whether --auth or an Authorization --svc-param is set. -func (s *ServiceParams) HasCredential() bool { - if s.auth != "" { +// SetDefault adds values for key that are sent unless --auth or --svc-param +// set the same key, such as the headers from a credential helper. +func (s *ServiceParams) SetDefault(key string, values ...string) { + for _, v := range values { + s.defaults = append(s.defaults, serviceParam{key: key, value: v}) + } +} + +// isSet reports whether --svc-param, or --auth for Authorization, sets key. +func (s *ServiceParams) isSet(key string) bool { + if s.auth != "" && strings.EqualFold(key, "Authorization") { return true } for _, e := range s.entries { - if strings.EqualFold(e.key, "Authorization") { + if strings.EqualFold(e.key, key) { + return true + } + } + return false +} + +// SendsCredential reports whether Params includes an Authorization entry, +// set explicitly or by default. +func (s *ServiceParams) SendsCredential() bool { + if s.HasCredential() { + return true + } + for _, d := range s.defaults { + if strings.EqualFold(d.key, "Authorization") { return true } } return false } +// HasCredential reports whether --auth or an Authorization --svc-param is set. +func (s *ServiceParams) HasCredential() bool { + return s.isSet("Authorization") +} + type svcParamValue struct{ s *ServiceParams } func (v *svcParamValue) Set(kv string) error { diff --git a/internal/flagparse/svcparams_test.go b/internal/flagparse/svcparams_test.go index 9ca933d..bad3d61 100644 --- a/internal/flagparse/svcparams_test.go +++ b/internal/flagparse/svcparams_test.go @@ -146,3 +146,52 @@ func TestServiceParamsEmpty(t *testing.T) { t.Fatalf("sp.Params() = %v, want nil", sp.Params()) } } + +func TestServiceParamsDefaults(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + args []string + want a2aclient.ServiceParams + wantCredential bool + }{ + { + name: "defaults apply without flags", + want: a2aclient.ServiceParams{"authorization": {"Bearer helper"}, "x-tenant": {"a", "b"}}, + wantCredential: true, + }, + { + name: "auth overrides the default Authorization", + args: []string{"--auth", "Bearer flag"}, + want: a2aclient.ServiceParams{"authorization": {"Bearer flag"}, "x-tenant": {"a", "b"}}, + wantCredential: true, + }, + { + name: "svc-param overrides a default of the same key in any case", + args: []string{"--svc-param", "X-TENANT=c"}, + want: a2aclient.ServiceParams{"authorization": {"Bearer helper"}, "x-tenant": {"c"}}, + wantCredential: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var sp ServiceParams + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + sp.Attach(fs) + if err := fs.Parse(tt.args); err != nil { + t.Fatalf("fs.Parse(%v) error = %v, want nil", tt.args, err) + } + sp.SetDefault("Authorization", "Bearer helper") + sp.SetDefault("X-Tenant", "a", "b") + if diff := cmp.Diff(tt.want, sp.Params()); diff != "" { + t.Fatalf("sp.Params() wrong result (-want +got) diff = %s", diff) + } + if got := sp.SendsCredential(); got != tt.wantCredential { + t.Fatalf("sp.SendsCredential() = %v, want %v", got, tt.wantCredential) + } + }) + } +} From 86c21a7e1882bf6719cf85f70d71e295aaa72f5a Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 08:58:13 +0000 Subject: [PATCH 2/8] fix: run credential helpers per request host Helpers ran in the root pre-run for every command, so unrelated commands failed when a helper did, and their headers went to whatever interface host the card advertised, outside the helper's scope. Helpers now run lazily: the card request uses the helper for the card URL and an a2a client interceptor picks one per interface URL, caching headers per URL until they expire. --timeout bounds each run. Also keeps credential-helper env values unsplit on commas, rejects ports in helper and --unix-socket scopes, lets --unix-socket paths contain '=', and warns in the README that a local .env can set a helper. --- internal/README.md | 10 +- internal/cli/auth.go | 6 +- internal/cli/auth_get.go | 10 +- internal/cli/card_get.go | 18 ++- internal/cli/cli_test.go | 5 + internal/cli/client.go | 43 +++----- internal/cli/credentials.go | 101 ++++++++++------- internal/cli/credentials_test.go | 87 +++++++++++++-- internal/cli/root.go | 8 +- internal/clicfg/binder.go | 9 ++ internal/clicfg/binder_test.go | 27 ++++- internal/credhelper/credhelper.go | 10 +- internal/credhelper/credhelper_test.go | 145 +++++++++++++++++++++---- internal/credhelper/source.go | 110 +++++++++++++++++++ internal/flagparse/svcparams.go | 47 ++------ internal/flagparse/svcparams_test.go | 49 --------- internal/unixsocket/unixsocket.go | 77 +++++++++++++ internal/unixsocket/unixsocket_test.go | 75 +++++++++++++ 18 files changed, 616 insertions(+), 221 deletions(-) create mode 100644 internal/credhelper/source.go create mode 100644 internal/unixsocket/unixsocket.go create mode 100644 internal/unixsocket/unixsocket_test.go diff --git a/internal/README.md b/internal/README.md index e8a984f..6913ac2 100644 --- a/internal/README.md +++ b/internal/README.md @@ -38,7 +38,7 @@ These apply to every client-mode command. Each command selects the agent it talk | `--svc-param ` | | Service parameter (repeatable). The chosen transport defines how it's passed. Split on the first `=`. | | `--auth ` | | Shorthand for `--svc-param "Authorization="`. | | `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]command [args...]`. Repeatable. See [Credentials](#credentials). | -| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. | +| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | | `--verbose` | `-v` | Verbose output to stderr. | @@ -265,9 +265,9 @@ Streams events to stdout until the task reaches a terminal state. Output format ## Credentials -A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. +A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. -Configure helpers with `--credential-helper`, `A2ACLI_CREDENTIAL_HELPER`, or `credential-helper` in a config file: +Configure helpers with `--credential-helper`, `A2ACLI_CREDENTIAL_HELPER` (one helper; commas are not separators), or `credential-helper` in a config file: ```yaml # ~/.config/a2a-cli/config.yaml @@ -276,7 +276,9 @@ credential-helper: - "*.corp.example.com=/usr/local/bin/corp-helper" ``` -Each value is `[scope=]command [args...]`. A scope is a host (`example.com`), or `*.example.com` for the domain and its subdomains. For each agent, the helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent with the card request and every A2A call, unless `--auth` or `--svc-param` set the same header. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. +Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card URL, and A2A calls use the helper for the interface URL the client connects to, so headers never reach a host outside the helper's scope. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. + +> **Warning:** a2a executes the configured helper command. The local `.env` is found by walking up from the current directory, so a `.env` in a cloned repository can set `A2ACLI_CREDENTIAL_HELPER`. Check the `.env` of untrusted directories, or run `a2a config list` to see where `credential-helper` comes from. ### `auth get` - Inspect credentials diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 36786e5..c87a925 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -29,9 +29,9 @@ credential-helper in the config file) is a program that returns the HTTP headers to send to an agent, following the Bazel credential helper protocol: https://github.com/EngFlow/credential-helper-spec. Its value is "[scope=]command [args...]", where scope is a host such as example.com, or -*.example.com for a domain and its subdomains. The most specific helper for -the agent's host runs as "command args... get", and its headers are sent -unless --auth or --svc-param set the same header.`, +*.example.com for a domain and its subdomains. For each request, the most +specific helper for the request's host runs as "command args... get", and +its headers are sent unless --auth or --svc-param set the same header.`, } cmd.AddCommand(newAuthGetCmd(cfg)) return cmd diff --git a/internal/cli/auth_get.go b/internal/cli/auth_get.go index d140d7f..5a2f6ef 100644 --- a/internal/cli/auth_get.go +++ b/internal/cli/auth_get.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" "github.com/a2aproject/a2a-cli/internal/clierr" + "github.com/a2aproject/a2a-cli/internal/credhelper" "github.com/a2aproject/a2a-cli/internal/flagparse" ) @@ -40,10 +41,13 @@ func newAuthGetCmd(cfg *globalConfig) *cobra.Command { return err } target := ref.URL() - h, ok, err := cfg.credentialHelper(target) - if err != nil { - return clierr.Usage(err.Error()) + if !isHTTPURL(target) { + return clierr.Usage(fmt.Sprintf("auth get needs an http(s) agent URL, got %q", args[0])) + } + if cfg.credentials == nil { + return fmt.Errorf("no credential helper configured") } + h, ok := credhelper.Select(cfg.credentials.Helpers, target) if !ok { return fmt.Errorf("no credential helper for %s", target) } diff --git a/internal/cli/card_get.go b/internal/cli/card_get.go index e847fd9..307d766 100644 --- a/internal/cli/card_get.go +++ b/internal/cli/card_get.go @@ -30,23 +30,19 @@ func newCardGetCmd(cfg *globalConfig) *cobra.Command { cmd := &cobra.Command{ Use: "get [url]", Short: "Fetch and display an agent card", - // The positional URL is taken while validating args, which runs - // before the root's PersistentPreRunE asks the credential helper. - Args: func(cmd *cobra.Command, args []string) error { - if err := cobra.MaximumNArgs(1)(cmd, args); err != nil { - return err - } - if len(args) == 1 { - return cfg.agentCard.Set(args[0]) - } - return nil - }, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() ctx, cancel := context.WithTimeout(ctx, cfg.timeout) defer cancel() + if len(args) == 1 { + if err := cfg.agentCard.Set(args[0]); err != nil { + return err + } + } + var card *a2a.AgentCard var err error if extended { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index d21f56a..024e1fb 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1059,6 +1059,11 @@ func TestInsecureCredentialWarning(t *testing.T) { args: []string{"send", "-a", url, "--insecure", "hi"}, wantWarn: false, }, + { + name: "insecure with a credential helper warns", + args: []string{"send", "-a", url, "--insecure", "--credential-helper", writeCredentialHelper(t), "hi"}, + wantWarn: true, + }, } for _, tt := range tests { diff --git a/internal/cli/client.go b/internal/cli/client.go index b081915..414c207 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -17,7 +17,6 @@ package cli import ( "context" "fmt" - "net" "net/http" "net/url" "strings" @@ -47,7 +46,7 @@ const insecureCredentialWarning = "warning: sending a credential with TLS verifi // warnInsecureCredential warns once when a credential is sent with --insecure. func warnInsecureCredential(cfg *globalConfig) { - if cfg.insecureGRPC && cfg.svcParams.SendsCredential() { + if cfg.insecureGRPC && cfg.svcParams.HasCredential() { _, _ = fmt.Fprintln(cfg.stderr(), insecureCredentialWarning) } } @@ -136,10 +135,17 @@ func resolveCard(ctx context.Context, cfg *globalConfig, ref string) (*a2a.Agent for k, vals := range cfg.svcParams.Params() { opts = append(opts, agentcard.WithRequestHeader(k, strings.Join(vals, ", "))) } + headers, err := cfg.cardHeaders(ctx, ref) + if err != nil { + return nil, err + } + for k, vals := range headers { + opts = append(opts, agentcard.WithRequestHeader(k, strings.Join(vals, ", "))) + } cfg.logf("resolving agent card from %s", ref) resolver := compatCardResolver - if cfg.unixSocket != "" { - resolver = agentcard.NewResolver(&http.Client{Transport: cfg.httpTransport, Timeout: 30 * time.Second}) + if cfg.httpTransport != nil { + resolver = agentcard.NewResolver(&http.Client{Transport: cfg.httpTransport}) resolver.CardParser = a2av0.NewAgentCardParser() } card, err := resolver.Resolve(ctx, ref, opts...) @@ -166,6 +172,9 @@ func clientFactoryOpts(cfg *globalConfig) []a2aclient.FactoryOption { // total timeout so the --timeout context deadline is what bounds a request. httpClient := &http.Client{Transport: cfg.httpTransport} opts := []a2aclient.FactoryOption{a2aclient.WithDefaultsDisabled()} + if cfg.credentials != nil { + opts = append(opts, cfg.credentialInterceptor()) + } if cfg.a2aVersion == "" || cfg.a2aVersion == "1.0" { opts = append( opts, @@ -207,29 +216,3 @@ func destroyClient(cfg *globalConfig, client *a2aclient.Client) { cfg.logf("failed to destroy client: %v", err) } } - -// unixSocketTransport sends HTTP requests through a Unix socket. spec is -// , for every host, or =, for requests to host only. -func unixSocketTransport(spec string) http.RoundTripper { - host, path, scoped := strings.Cut(spec, "=") - if !scoped { - host, path = "", spec - } - viaSocket := func(h string) bool { return host == "" || h == host } - t := http.DefaultTransport.(*http.Transport).Clone() - proxy := t.Proxy - t.Proxy = func(r *http.Request) (*url.URL, error) { - if viaSocket(r.URL.Hostname()) || proxy == nil { - return nil, nil - } - return proxy(r) - } - var d net.Dialer - t.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { - if h, _, err := net.SplitHostPort(addr); err == nil && viaSocket(h) { - return d.DialContext(ctx, "unix", path) - } - return d.DialContext(ctx, network, addr) - } - return t -} diff --git a/internal/cli/credentials.go b/internal/cli/credentials.go index 4009a3a..6c02afe 100644 --- a/internal/cli/credentials.go +++ b/internal/cli/credentials.go @@ -18,58 +18,83 @@ import ( "context" "fmt" "net/url" - "slices" + "strings" + "sync" + "github.com/a2aproject/a2a-go/v2/a2aclient" + + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/credhelper" + "github.com/a2aproject/a2a-cli/internal/unixsocket" ) -// targetURL returns the HTTP(S) URL of the agent the command talks to, or -// "" when there is none, such as for a local card file. -func (g *globalConfig) targetURL() string { - target := g.url - if target == "" && g.agentCard.IsSet() { - target = g.agentCard.URL() +// initHTTPTransport routes HTTP requests through --unix-socket, if set. +func (g *globalConfig) initHTTPTransport() error { + if g.unixSocket == "" { + return nil } - u, err := url.Parse(target) - if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { - return "" + spec, err := unixsocket.Parse(g.unixSocket) + if err != nil { + return clierr.Usage(err.Error()) } - return target + g.httpTransport = spec.Transport() + return nil } -// credentialHelper returns the --credential-helper for target. -func (g *globalConfig) credentialHelper(target string) (credhelper.Helper, bool, error) { +// initCredentials parses --credential-helper. Helpers run only when a +// request is made to a host in their scope. +func (g *globalConfig) initCredentials() error { + if len(g.credentialHelpers) == 0 { + return nil + } helpers, err := credhelper.ParseAll(g.credentialHelpers) if err != nil { - return credhelper.Helper{}, false, err + return clierr.Usage(err.Error()) } - h, ok := credhelper.Select(helpers, target) - return h, ok, nil + g.credentials = &credhelper.Source{Helpers: helpers, Logf: g.logf} + return nil } -// applyCredentialHelper sends the headers the credential helper returns for -// the target agent, unless --auth or --svc-param set the same header. -func applyCredentialHelper(ctx context.Context, cfg *globalConfig) error { - target := cfg.targetURL() - if target == "" || len(cfg.credentialHelpers) == 0 { - return nil +// cardHeaders returns the helper headers for the card request to ref that +// --auth and --svc-param don't already set. +func (g *globalConfig) cardHeaders(ctx context.Context, ref string) (map[string][]string, error) { + if g.credentials == nil || !isHTTPURL(ref) { + return nil, nil } - h, ok, err := cfg.credentialHelper(target) - if err != nil || !ok { - return err + _, resp, err := g.credentials.Headers(ctx, ref) + if err != nil || resp == nil { + return nil, err } - cfg.logf("asking credential helper %q for %s", h.Spec, target) - resp, err := h.Get(ctx, target) - if err != nil { - return fmt.Errorf("getting credentials for %s: %w", target, err) - } - names := make([]string, 0, len(resp.Headers)) - for name := range resp.Headers { - names = append(names, name) + explicit := g.svcParams.Params() + headers := map[string][]string{} + for name, values := range resp.Headers { + if len(explicit.Get(name)) == 0 { + headers[name] = values + } } - slices.Sort(names) - for _, name := range names { - cfg.svcParams.SetDefault(name, resp.Headers[name]...) - } - return nil + return headers, nil +} + +// credentialInterceptor adds the helper headers to every A2A call, chosen +// by the host of the interface the client is connected to. +func (g *globalConfig) credentialInterceptor() a2aclient.FactoryOption { + var warn sync.Once + return a2aclient.WithCallInterceptors(&credhelper.Interceptor{ + Source: g.credentials, + OnApply: func(headers map[string][]string) { + if !g.insecureGRPC { + return + } + for name := range headers { + if strings.EqualFold(name, "Authorization") { + warn.Do(func() { _, _ = fmt.Fprintln(g.stderr(), insecureCredentialWarning) }) + } + } + }, + }) +} + +func isHTTPURL(raw string) bool { + u, err := url.Parse(raw) + return err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" } diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go index 2a66048..5e6f014 100644 --- a/internal/cli/credentials_test.go +++ b/internal/cli/credentials_test.go @@ -32,31 +32,44 @@ import ( "github.com/a2aproject/a2a-go/v2/a2asrv" "github.com/google/go-cmp/cmp" + "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/localsrv" ) -// authRecorder records the Authorization header of every request. +type recordedRequest struct { + Path string + Auth string +} + +// authRecorder records the path and Authorization header of every request. type authRecorder struct { mu sync.Mutex - seen []string + seen []recordedRequest } func (r *authRecorder) wrap(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { r.mu.Lock() - r.seen = append(r.seen, req.Header.Get("Authorization")) + r.seen = append(r.seen, recordedRequest{Path: req.URL.Path, Auth: req.Header.Get("Authorization")}) r.mu.Unlock() h.ServeHTTP(w, req) }) } -func (r *authRecorder) all() []string { +func (r *authRecorder) all() []recordedRequest { r.mu.Lock() defer r.mu.Unlock() - return append([]string(nil), r.seen...) + return append([]recordedRequest(nil), r.seen...) } func startAuthRecordingServer(t *testing.T) (string, *authRecorder) { + t.Helper() + return startAuthRecordingServerWithInterface(t, func(serverURL string) string { return serverURL }) +} + +// startAuthRecordingServerWithInterface serves a card advertising the +// interface URL that iface derives from the server URL. +func startAuthRecordingServerWithInterface(t *testing.T, iface func(serverURL string) string) (string, *authRecorder) { t.Helper() capabilities := a2a.AgentCapabilities{Streaming: true} handler := a2asrv.NewHandler(localsrv.NewEchoExecutor(), a2asrv.WithCapabilityChecks(&capabilities)) @@ -65,7 +78,7 @@ func startAuthRecordingServer(t *testing.T) (string, *authRecorder) { rec := &authRecorder{} server := httptest.NewServer(rec.wrap(mux)) t.Cleanup(server.Close) - mux.Handle(a2asrv.WellKnownAgentCardPath, a2asrv.NewStaticAgentCardHandler(newAgentCard(server.URL, capabilities))) + mux.Handle(a2asrv.WellKnownAgentCardPath, a2asrv.NewStaticAgentCardHandler(newAgentCard(iface(server.URL), capabilities))) return server.URL, rec } @@ -154,14 +167,61 @@ func TestCredentialHelper(t *testing.T) { t.Fatalf("a2a %s sent no requests", strings.Join(args, " ")) } for _, got := range seen { - if want := tc.wantAuth(url); got != want { - t.Fatalf("a2a %s Authorization = %q, want %q", strings.Join(args, " "), got, want) + if want := tc.wantAuth(url); got.Auth != want { + t.Fatalf("a2a %s Authorization for %s = %q, want %q", strings.Join(args, " "), got.Path, got.Auth, want) } } }) } } +func TestCredentialHelperScopedToRequestHost(t *testing.T) { + t.Parallel() + helper := writeCredentialHelper(t) + url, rec := startAuthRecordingServerWithInterface(t, func(serverURL string) string { + return strings.Replace(serverURL, "127.0.0.1", "localhost", 1) + }) + args := []string{"send", "-a", url, "--credential-helper", "127.0.0.1=" + helper, "hello"} + if _, err := runCMD(t, args...); err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) + } + want := []recordedRequest{ + {Path: a2asrv.WellKnownAgentCardPath, Auth: "Bearer " + url}, + {Path: "/message:send", Auth: ""}, + } + if diff := cmp.Diff(want, rec.all()); diff != "" { + t.Fatalf("a2a %s wrong result (-want +got) diff = %s", strings.Join(args, " "), diff) + } +} + +func TestCredentialHelperOnlyRunsForRequests(t *testing.T) { + t.Parallel() + tests := []struct { + name string + args []string + }{ + {name: "config list", args: []string{"config", "list", "-a", "https://agent.example.com", "--credential-helper", "false"}}, + {name: "transport list", args: []string{"transport", "list", "-a", "https://agent.example.com", "--credential-helper", "false"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if _, err := runCMD(t, tc.args...); err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(tc.args, " "), err) + } + }) + } +} + +func TestCredentialHelperInvalidSpec(t *testing.T) { + t.Parallel() + args := []string{"send", "-a", "https://agent.example.com", "--credential-helper", `sh -c "unterminated`, "hello"} + _, err := runCMD(t, args...) + if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage { + t.Fatalf("a2a %s error = %v, want a usage error", strings.Join(args, " "), err) + } +} + func TestAuthGet(t *testing.T) { t.Parallel() helper := writeCredentialHelper(t) @@ -181,6 +241,16 @@ func TestAuthGet(t *testing.T) { args: []string{"auth", "get", "https://agent.example.com", "--credential-helper", helper, "--reveal", "-o", "json"}, want: map[string]any{"url": "https://agent.example.com", "helper": helper, "headers": map[string]any{"Authorization": []any{"Bearer https://agent.example.com"}}, "expires": ""}, }, + { + name: "ignores the helper of the configured agent", + args: []string{"auth", "get", "https://other.example.com", "-a", "https://agent.example.com", "--credential-helper", "agent.example.com=false", "--credential-helper", "other.example.com=" + helper, "-o", "json"}, + want: map[string]any{"url": "https://other.example.com", "helper": "other.example.com=" + helper, "headers": map[string]any{"Authorization": []any{""}}, "expires": ""}, + }, + { + name: "local file", + args: []string{"auth", "get", "./card.json", "--credential-helper", helper}, + wantErr: "http(s) agent URL", + }, { name: "no helper for the host", args: []string{"auth", "get", "https://agent.example.com", "--credential-helper", "other.example.com=" + helper}, @@ -254,6 +324,7 @@ func TestSendOverUnixSocket(t *testing.T) { {name: "agent card", socket: sock, args: []string{"-a", base}}, {name: "endpoint", socket: sock, args: []string{"-e", base, "--transport", "rest"}}, {name: "scoped to the host", socket: "agent.invalid=" + sock, args: []string{"-a", base}}, + {name: "scoped to the host in another case", socket: "AGENT.invalid=" + sock, args: []string{"-a", base}}, {name: "scoped to another host", socket: "other.invalid=" + sock, args: []string{"-a", base, "--timeout", "2s"}, wantErr: true}, } for _, tc := range tests { diff --git a/internal/cli/root.go b/internal/cli/root.go index 6cdbd41..3a1d062 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -29,6 +29,7 @@ import ( "github.com/a2aproject/a2a-cli/internal/clicfg" "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/commandplugin" + "github.com/a2aproject/a2a-cli/internal/credhelper" "github.com/a2aproject/a2a-cli/internal/flagparse" "github.com/a2aproject/a2a-cli/internal/output" "github.com/a2aproject/a2a-cli/internal/polling" @@ -67,6 +68,7 @@ type globalConfig struct { configPath string credentialHelpers []string + credentials *credhelper.Source pluginsEnabled bool @@ -159,10 +161,10 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { } cfg.bindings = bindings cfg.store = store - if cfg.unixSocket != "" && cfg.httpTransport == nil { - cfg.httpTransport = unixSocketTransport(cfg.unixSocket) + if err := cfg.initHTTPTransport(); err != nil { + return err } - if err := applyCredentialHelper(cmd.Context(), cfg); err != nil { + if err := cfg.initCredentials(); err != nil { return err } diff --git a/internal/clicfg/binder.go b/internal/clicfg/binder.go index 608e4da..6bb28cd 100644 --- a/internal/clicfg/binder.go +++ b/internal/clicfg/binder.go @@ -157,6 +157,9 @@ func setFlagValue(f *pflag.Flag, raw any) error { switch v := raw.(type) { case string: if sv, ok := f.Value.(pflag.SliceValue); ok { + if !splitsOnComma(f.Name) { + return sv.Replace([]string{v}) + } parts := strings.Split(v, ",") for i := range parts { parts[i] = strings.TrimSpace(parts[i]) @@ -171,6 +174,12 @@ func setFlagValue(f *pflag.Flag, raw any) error { } } +// splitsOnComma reports whether a single string value of a repeatable flag +// holds a comma-separated list. Credential helper commands may contain commas. +func splitsOnComma(name string) bool { + return name != "credential-helper" +} + func flagToEnvVar(name string) string { return envPrefix + strings.ToUpper(strings.ReplaceAll(name, "-", "_")) } diff --git a/internal/clicfg/binder_test.go b/internal/clicfg/binder_test.go index 7b64b92..dce7978 100644 --- a/internal/clicfg/binder_test.go +++ b/internal/clicfg/binder_test.go @@ -78,6 +78,7 @@ func TestApplyConfig(t *testing.T) { vals["agent-card"] = fs.String("agent-card", "", "") vals["tenant"] = fs.String("tenant", "", "") fs.StringArray("transport", nil, "") + fs.StringArray("credential-helper", nil, "") fs.Bool("insecure", false, "") fs.Bool("stream", false, "") fs.String("config", "", "") @@ -147,6 +148,23 @@ func TestApplyConfig(t *testing.T) { } }) + t.Run("keeps a credential helper with commas as one entry", func(t *testing.T) { + t.Parallel() + fs, _ := newFlags() + s := store(t, map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper --scopes=a,b"}) + + if _, err := Bind(fs, s); err != nil { + t.Fatalf("applyConfig() error = %v", err) + } + got, err := fs.GetStringArray("credential-helper") + if err != nil { + t.Fatalf("fs.GetStringArray() error = %v", err) + } + if diff := cmp.Diff([]string{"helper --scopes=a,b"}, got); diff != "" { + t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) + } + }) + t.Run("excluded flags are never read from config", func(t *testing.T) { t.Parallel() fs, _ := newFlags() @@ -580,10 +598,11 @@ tenant: yaml-tenant got[r.Name] = r.Source } want := map[string]string{ - "agent-card": "env", - "tenant": "flag", - "transport": "default", - "insecure": "default", + "agent-card": "env", + "tenant": "flag", + "transport": "default", + "credential-helper": "default", + "insecure": "default", } if diff := cmp.Diff(want, got); diff != "" { t.Fatalf("resolution sources wrong result (-want +got) diff = %s", diff) diff --git a/internal/credhelper/credhelper.go b/internal/credhelper/credhelper.go index bc7f83e..5a3d132 100644 --- a/internal/credhelper/credhelper.go +++ b/internal/credhelper/credhelper.go @@ -31,9 +31,6 @@ import ( "github.com/google/shlex" ) -// Timeout bounds a single helper invocation. -const Timeout = 10 * time.Second - // Helper is a configured credential helper, from "[scope=]command [args...]". type Helper struct { // Spec is the configured value, for messages. @@ -63,6 +60,9 @@ func Parse(spec string) (Helper, error) { h := Helper{Spec: spec} rest := strings.TrimSpace(spec) if scope, cmd, ok := strings.Cut(rest, "="); ok && isScope(scope) { + if strings.Contains(scope, ":") { + return Helper{}, fmt.Errorf("credential helper %q: scope %q must be a host without a port", spec, scope) + } h.Scope, rest = strings.ToLower(scope), cmd } args, err := shlex.Split(rest) @@ -134,10 +134,8 @@ func matchRank(scope, host string) int { return -1 } -// Get runs the helper for uri. +// Get runs the helper for uri. ctx bounds the helper's run. func (h Helper) Get(ctx context.Context, uri string) (*Response, error) { - ctx, cancel := context.WithTimeout(ctx, Timeout) - defer cancel() in, err := json.Marshal(request{URI: uri}) if err != nil { return nil, err diff --git a/internal/credhelper/credhelper_test.go b/internal/credhelper/credhelper_test.go index 5fcc2c3..5f1d731 100644 --- a/internal/credhelper/credhelper_test.go +++ b/internal/credhelper/credhelper_test.go @@ -21,6 +21,7 @@ import ( "strings" "testing" + "github.com/a2aproject/a2a-go/v2/a2aclient" "github.com/google/go-cmp/cmp" ) @@ -38,6 +39,7 @@ func TestParse(t *testing.T) { {spec: `helper --opt=a\ b`, want: Helper{Command: []string{"helper", "--opt=a b"}}}, {spec: "/opt/x=y/helper", want: Helper{Command: []string{"/opt/x=y/helper"}}}, {spec: "example.com=", wantErr: true}, + {spec: "localhost:8080=helper", wantErr: true}, {spec: "", wantErr: true}, } for _, tc := range tests { @@ -60,44 +62,43 @@ func TestParse(t *testing.T) { func TestSelect(t *testing.T) { t.Parallel() - helpers, err := ParseAll([]string{ + all := []string{ "default", "*.example.com=wildcard", "*.dev.example.com=dev-wildcard", "api.example.com=exact", "api.example.com=exact-override", - }) - if err != nil { - t.Fatalf("ParseAll() error = %v", err) } tests := []struct { - url string - want string + specs []string + url string + want string + wantOK bool }{ - {url: "https://api.example.com/agent", want: "exact-override"}, - {url: "https://API.example.com:8443", want: "exact-override"}, - {url: "https://foo.dev.example.com", want: "dev-wildcard"}, - {url: "https://foo.example.com", want: "wildcard"}, - {url: "https://example.com", want: "wildcard"}, - {url: "https://other.org", want: "default"}, + {specs: all, url: "https://api.example.com/agent", want: "exact-override", wantOK: true}, + {specs: all, url: "https://API.example.com:8443", want: "exact-override", wantOK: true}, + {specs: all, url: "https://foo.dev.example.com", want: "dev-wildcard", wantOK: true}, + {specs: all, url: "https://foo.example.com", want: "wildcard", wantOK: true}, + {specs: all, url: "https://example.com", want: "wildcard", wantOK: true}, + {specs: all, url: "https://other.org", want: "default", wantOK: true}, + {specs: []string{"example.com=helper"}, url: "https://other.org", wantOK: false}, } for _, tc := range tests { t.Run(tc.url, func(t *testing.T) { t.Parallel() + helpers, err := ParseAll(tc.specs) + if err != nil { + t.Fatalf("ParseAll(%q) error = %v", tc.specs, err) + } got, ok := Select(helpers, tc.url) - if !ok || got.Command[0] != tc.want { - t.Fatalf("Select(%q) = %v, %v, want %s", tc.url, got.Command, ok, tc.want) + if ok != tc.wantOK { + t.Fatalf("Select(%q) ok = %v, want %v", tc.url, ok, tc.wantOK) + } + if ok && got.Command[0] != tc.want { + t.Fatalf("Select(%q) = %v, want %s", tc.url, got.Command, tc.want) } }) } - - scoped, err := ParseAll([]string{"example.com=helper"}) - if err != nil { - t.Fatalf("ParseAll() error = %v", err) - } - if got, ok := Select(scoped, "https://other.org"); ok { - t.Fatalf("Select() = %v, want no helper for a host outside every scope", got.Command) - } } func writeHelper(t *testing.T, script string) string { @@ -153,3 +154,103 @@ func TestGet(t *testing.T) { }) } } + +func TestSourceHeaders(t *testing.T) { + t.Parallel() + tests := []struct { + name string + expires string + uris []string + wantRuns int + }{ + {name: "reused for the same uri", uris: []string{"https://a.example.com", "https://a.example.com"}, wantRuns: 1}, + {name: "reused until expiry", expires: "2999-01-01T00:00:00Z", uris: []string{"https://a.example.com", "https://a.example.com"}, wantRuns: 1}, + {name: "refreshed after expiry", expires: "2000-01-01T00:00:00Z", uris: []string{"https://a.example.com", "https://a.example.com"}, wantRuns: 2}, + {name: "run per uri", uris: []string{"https://a.example.com", "https://a.example.com/agent"}, wantRuns: 2}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + runs := filepath.Join(t.TempDir(), "runs") + h, err := Parse(writeHelper(t, `echo run >>`+runs+`; printf '{"headers":{"X":["v"]},"expires":"%s"}' "`+tc.expires+`"`)) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + src := &Source{Helpers: []Helper{h}} + for _, uri := range tc.uris { + _, resp, err := src.Headers(t.Context(), uri) + if err != nil { + t.Fatalf("Source.Headers(%q) error = %v", uri, err) + } + if diff := cmp.Diff(map[string][]string{"X": {"v"}}, resp.Headers); diff != "" { + t.Fatalf("Source.Headers(%q) wrong result (-want +got) diff = %s", uri, diff) + } + } + out, err := os.ReadFile(runs) + if err != nil { + t.Fatalf("os.ReadFile() error = %v", err) + } + if got := strings.Count(string(out), "run"); got != tc.wantRuns { + t.Fatalf("helper runs = %d, want %d", got, tc.wantRuns) + } + }) + } +} + +func TestInterceptor(t *testing.T) { + t.Parallel() + helper := writeHelper(t, `uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/'); printf '{"headers":{"Authorization":["Bearer %s"]}}' "$uri"`) + tests := []struct { + name string + spec string + baseURL string + params a2aclient.ServiceParams + want a2aclient.ServiceParams + }{ + { + name: "adds the headers for the interface url", + spec: helper, + baseURL: "https://agent.example.com/a2a", + params: a2aclient.ServiceParams{}, + want: a2aclient.ServiceParams{"authorization": {"Bearer https://agent.example.com/a2a"}}, + }, + { + name: "keeps an explicit service param", + spec: helper, + baseURL: "https://agent.example.com", + params: a2aclient.ServiceParams{"authorization": {"Bearer flag"}}, + want: a2aclient.ServiceParams{"authorization": {"Bearer flag"}}, + }, + { + name: "skips a host outside the scope", + spec: "agent.example.com=" + helper, + baseURL: "https://other.example.com", + params: a2aclient.ServiceParams{}, + want: a2aclient.ServiceParams{}, + }, + { + name: "matches a grpc address without a scheme", + spec: "agent.example.com=" + helper, + baseURL: "agent.example.com:443", + params: a2aclient.ServiceParams{}, + want: a2aclient.ServiceParams{"authorization": {"Bearer https://agent.example.com:443"}}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h, err := Parse(tc.spec) + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + i := &Interceptor{Source: &Source{Helpers: []Helper{h}}} + req := &a2aclient.Request{BaseURL: tc.baseURL, ServiceParams: tc.params} + if _, _, err := i.Before(t.Context(), req); err != nil { + t.Fatalf("Interceptor.Before() error = %v", err) + } + if diff := cmp.Diff(tc.want, req.ServiceParams); diff != "" { + t.Fatalf("Interceptor.Before() wrong result (-want +got) diff = %s", diff) + } + }) + } +} diff --git a/internal/credhelper/source.go b/internal/credhelper/source.go new file mode 100644 index 0000000..48097bf --- /dev/null +++ b/internal/credhelper/source.go @@ -0,0 +1,110 @@ +// 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 credhelper + +import ( + "context" + "fmt" + "strings" + "sync" + "time" + + "github.com/a2aproject/a2a-go/v2/a2aclient" +) + +// Source runs the helper selected for each URI and reuses its headers for +// that URI until they expire. +type Source struct { + Helpers []Helper + // Logf, if set, is called before a helper runs. + Logf func(format string, args ...any) + + mu sync.Mutex + cache map[string]*Response +} + +// Headers returns the selected helper and its response for uri, or a nil +// response when no helper's scope matches. +func (s *Source) Headers(ctx context.Context, uri string) (Helper, *Response, error) { + h, ok := Select(s.Helpers, uri) + if !ok { + return Helper{}, nil, nil + } + s.mu.Lock() + defer s.mu.Unlock() + if resp, ok := s.cache[uri]; ok && !expired(resp) { + return h, resp, nil + } + if s.Logf != nil { + s.Logf("asking credential helper %q for %s", h.Spec, uri) + } + resp, err := h.Get(ctx, uri) + if err != nil { + return h, nil, fmt.Errorf("getting credentials for %s: %w", uri, err) + } + if s.cache == nil { + s.cache = map[string]*Response{} + } + s.cache[uri] = resp + return h, resp, nil +} + +func expired(resp *Response) bool { + if resp.Expires == "" { + return false + } + t, err := time.Parse(time.RFC3339, resp.Expires) + return err != nil || !time.Now().Before(t) +} + +// Interceptor adds the headers from the helper for each call's interface URL +// as service parameters, unless the call already carries the same parameter. +type Interceptor struct { + a2aclient.PassthroughInterceptor + Source *Source + // OnApply, if set, is called with the headers added to a call. + OnApply func(headers map[string][]string) +} + +var _ a2aclient.CallInterceptor = (*Interceptor)(nil) + +// Before implements [a2aclient.CallInterceptor]. +func (i *Interceptor) Before(ctx context.Context, req *a2aclient.Request) (context.Context, any, error) { + _, resp, err := i.Source.Headers(ctx, interfaceURI(req.BaseURL)) + if err != nil || resp == nil { + return ctx, nil, err + } + applied := map[string][]string{} + for name, values := range resp.Headers { + if len(req.ServiceParams.Get(name)) > 0 { + continue + } + req.ServiceParams.Append(name, values...) + applied[name] = values + } + if i.OnApply != nil && len(applied) > 0 { + i.OnApply(applied) + } + return ctx, nil, nil +} + +// interfaceURI gives gRPC "host:port" addresses a scheme, so that helpers +// receive a URI and the scope is matched against the host. +func interfaceURI(baseURL string) string { + if strings.Contains(baseURL, "://") { + return baseURL + } + return "https://" + baseURL +} diff --git a/internal/flagparse/svcparams.go b/internal/flagparse/svcparams.go index 1e9ab22..f066ba3 100644 --- a/internal/flagparse/svcparams.go +++ b/internal/flagparse/svcparams.go @@ -28,9 +28,8 @@ import ( // registers several flags backed by one accumulator; merging happens as flags // are parsed, so Params can be read repeatedly without accumulating duplicates. type ServiceParams struct { - entries []serviceParam - auth string - defaults []serviceParam + entries []serviceParam + auth string } type serviceParam struct { @@ -49,7 +48,7 @@ func (s *ServiceParams) Attach(f *pflag.FlagSet) { // to the result and repeated calls never accumulate duplicates. Returns nil when // nothing was provided. func (s *ServiceParams) Params() a2aclient.ServiceParams { - if len(s.entries) == 0 && s.auth == "" && len(s.defaults) == 0 { + if len(s.entries) == 0 && s.auth == "" { return nil } params := a2aclient.ServiceParams{} @@ -59,54 +58,22 @@ func (s *ServiceParams) Params() a2aclient.ServiceParams { if s.auth != "" { params.Append("Authorization", s.auth) } - for _, d := range s.defaults { - if !s.isSet(d.key) { - params.Append(d.key, d.value) - } - } return params } -// SetDefault adds values for key that are sent unless --auth or --svc-param -// set the same key, such as the headers from a credential helper. -func (s *ServiceParams) SetDefault(key string, values ...string) { - for _, v := range values { - s.defaults = append(s.defaults, serviceParam{key: key, value: v}) - } -} - -// isSet reports whether --svc-param, or --auth for Authorization, sets key. -func (s *ServiceParams) isSet(key string) bool { - if s.auth != "" && strings.EqualFold(key, "Authorization") { +// HasCredential reports whether --auth or an Authorization --svc-param is set. +func (s *ServiceParams) HasCredential() bool { + if s.auth != "" { return true } for _, e := range s.entries { - if strings.EqualFold(e.key, key) { - return true - } - } - return false -} - -// SendsCredential reports whether Params includes an Authorization entry, -// set explicitly or by default. -func (s *ServiceParams) SendsCredential() bool { - if s.HasCredential() { - return true - } - for _, d := range s.defaults { - if strings.EqualFold(d.key, "Authorization") { + if strings.EqualFold(e.key, "Authorization") { return true } } return false } -// HasCredential reports whether --auth or an Authorization --svc-param is set. -func (s *ServiceParams) HasCredential() bool { - return s.isSet("Authorization") -} - type svcParamValue struct{ s *ServiceParams } func (v *svcParamValue) Set(kv string) error { diff --git a/internal/flagparse/svcparams_test.go b/internal/flagparse/svcparams_test.go index bad3d61..9ca933d 100644 --- a/internal/flagparse/svcparams_test.go +++ b/internal/flagparse/svcparams_test.go @@ -146,52 +146,3 @@ func TestServiceParamsEmpty(t *testing.T) { t.Fatalf("sp.Params() = %v, want nil", sp.Params()) } } - -func TestServiceParamsDefaults(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - args []string - want a2aclient.ServiceParams - wantCredential bool - }{ - { - name: "defaults apply without flags", - want: a2aclient.ServiceParams{"authorization": {"Bearer helper"}, "x-tenant": {"a", "b"}}, - wantCredential: true, - }, - { - name: "auth overrides the default Authorization", - args: []string{"--auth", "Bearer flag"}, - want: a2aclient.ServiceParams{"authorization": {"Bearer flag"}, "x-tenant": {"a", "b"}}, - wantCredential: true, - }, - { - name: "svc-param overrides a default of the same key in any case", - args: []string{"--svc-param", "X-TENANT=c"}, - want: a2aclient.ServiceParams{"authorization": {"Bearer helper"}, "x-tenant": {"c"}}, - wantCredential: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - var sp ServiceParams - fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - sp.Attach(fs) - if err := fs.Parse(tt.args); err != nil { - t.Fatalf("fs.Parse(%v) error = %v, want nil", tt.args, err) - } - sp.SetDefault("Authorization", "Bearer helper") - sp.SetDefault("X-Tenant", "a", "b") - if diff := cmp.Diff(tt.want, sp.Params()); diff != "" { - t.Fatalf("sp.Params() wrong result (-want +got) diff = %s", diff) - } - if got := sp.SendsCredential(); got != tt.wantCredential { - t.Fatalf("sp.SendsCredential() = %v, want %v", got, tt.wantCredential) - } - }) - } -} diff --git a/internal/unixsocket/unixsocket.go b/internal/unixsocket/unixsocket.go new file mode 100644 index 0000000..e8fa0e7 --- /dev/null +++ b/internal/unixsocket/unixsocket.go @@ -0,0 +1,77 @@ +// 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 unixsocket routes HTTP requests through a Unix socket. +package unixsocket + +import ( + "context" + "fmt" + "net" + "net/http" + "net/url" + "strings" +) + +// Spec is a parsed "" or "=" value. +type Spec struct { + // Host limits the socket to requests for one host; empty for every host. + Host string + Path string +} + +// Parse parses "" or "=". The text before the first '=' +// is a host only when it contains no '/', so paths containing '=' work. +func Parse(spec string) (Spec, error) { + if spec == "" { + return Spec{}, fmt.Errorf("unix socket: empty value") + } + host, path, ok := strings.Cut(spec, "=") + if !ok || host == "" || strings.Contains(host, "/") { + return Spec{Path: spec}, nil + } + if strings.Contains(host, ":") { + return Spec{}, fmt.Errorf("unix socket %q: host %q must not include a port", spec, host) + } + if path == "" { + return Spec{}, fmt.Errorf("unix socket %q: no path", spec) + } + return Spec{Host: strings.ToLower(host), Path: path}, nil +} + +// Matches reports whether requests to host go through the socket. +func (s Spec) Matches(host string) bool { + return s.Host == "" || strings.EqualFold(s.Host, host) +} + +// Transport returns an HTTP transport that dials the socket for matching +// hosts and bypasses the proxy for them. +func (s Spec) Transport() *http.Transport { + t := http.DefaultTransport.(*http.Transport).Clone() + proxy := t.Proxy + t.Proxy = func(r *http.Request) (*url.URL, error) { + if s.Matches(r.URL.Hostname()) || proxy == nil { + return nil, nil + } + return proxy(r) + } + var d net.Dialer + t.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + if h, _, err := net.SplitHostPort(addr); err == nil && s.Matches(h) { + return d.DialContext(ctx, "unix", s.Path) + } + return d.DialContext(ctx, network, addr) + } + return t +} diff --git a/internal/unixsocket/unixsocket_test.go b/internal/unixsocket/unixsocket_test.go new file mode 100644 index 0000000..15685b0 --- /dev/null +++ b/internal/unixsocket/unixsocket_test.go @@ -0,0 +1,75 @@ +// 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 unixsocket + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestParse(t *testing.T) { + t.Parallel() + tests := []struct { + spec string + want Spec + wantErr bool + }{ + {spec: "/tmp/a.sock", want: Spec{Path: "/tmp/a.sock"}}, + {spec: "agent.local=/tmp/a.sock", want: Spec{Host: "agent.local", Path: "/tmp/a.sock"}}, + {spec: "Agent.Local=/tmp/a.sock", want: Spec{Host: "agent.local", Path: "/tmp/a.sock"}}, + {spec: "/tmp/a=b.sock", want: Spec{Path: "/tmp/a=b.sock"}}, + {spec: "a.sock", want: Spec{Path: "a.sock"}}, + {spec: "localhost:8080=/tmp/a.sock", wantErr: true}, + {spec: "agent.local=", wantErr: true}, + {spec: "", wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.spec, func(t *testing.T) { + t.Parallel() + got, err := Parse(tc.spec) + if (err != nil) != tc.wantErr { + t.Fatalf("Parse(%q) error = %v, want error %v", tc.spec, err, tc.wantErr) + } + if tc.wantErr { + return + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("Parse(%q) wrong result (-want +got) diff = %s", tc.spec, diff) + } + }) + } +} + +func TestMatches(t *testing.T) { + t.Parallel() + tests := []struct { + spec Spec + host string + want bool + }{ + {spec: Spec{Path: "/s"}, host: "anything.example.com", want: true}, + {spec: Spec{Host: "agent.local", Path: "/s"}, host: "AGENT.local", want: true}, + {spec: Spec{Host: "agent.local", Path: "/s"}, host: "other.local", want: false}, + } + for _, tc := range tests { + t.Run(tc.host, func(t *testing.T) { + t.Parallel() + if got := tc.spec.Matches(tc.host); got != tc.want { + t.Fatalf("%+v.Matches(%q) = %v, want %v", tc.spec, tc.host, got, tc.want) + } + }) + } +} From 462ba2ac713a7ce178f37343293ddbba9ad7e95f Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 09:10:47 +0000 Subject: [PATCH 3/8] fix: keep credential helpers out of discovered .env files A .env in a cloned repository could set A2ACLI_CREDENTIAL_HELPER and run a command, or set A2ACLI_UNIX_SOCKET and redirect requests. Both flags now fail with a usage error when they come from a .env found by walking up from the working directory; --config, the environment, and the user config still work. Flag-specific config behavior moves from hard-coded names in clicfg to pflag annotations. Test helper scripts run through sh, since executing a file written while parallel tests fork can fail with ETXTBSY. The docs now say helpers receive the card reference or interface URL rather than the full request URL. --- internal/README.md | 10 +++--- internal/cli/auth.go | 7 +++- internal/cli/credentials_test.go | 46 ++++++++++++++++++++++---- internal/cli/root.go | 11 ++++++ internal/clicfg/binder.go | 27 ++++++++++++--- internal/clicfg/binder_test.go | 43 ++++++++++++++++++++++++ internal/clicfg/loader.go | 2 +- internal/clicfg/store.go | 10 ++++-- internal/credhelper/credhelper_test.go | 6 ++-- internal/credhelper/source.go | 2 ++ 10 files changed, 141 insertions(+), 23 deletions(-) diff --git a/internal/README.md b/internal/README.md index 6913ac2..9d7628a 100644 --- a/internal/README.md +++ b/internal/README.md @@ -37,8 +37,8 @@ These apply to every client-mode command. Each command selects the agent it talk | `--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="`. | -| `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]command [args...]`. Repeatable. See [Credentials](#credentials). | -| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. | +| `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]command [args...]`. Repeatable. Not read from the local `.env`. See [Credentials](#credentials). | +| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. Not read from the local `.env`. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | | `--verbose` | `-v` | Verbose output to stderr. | @@ -265,7 +265,7 @@ Streams events to stdout until the task reaches a terminal state. Output format ## Credentials -A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. +A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. Configure helpers with `--credential-helper`, `A2ACLI_CREDENTIAL_HELPER` (one helper; commas are not separators), or `credential-helper` in a config file: @@ -276,9 +276,9 @@ credential-helper: - "*.corp.example.com=/usr/local/bin/corp-helper" ``` -Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card URL, and A2A calls use the helper for the interface URL the client connects to, so headers never reach a host outside the helper's scope. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. +Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card reference as given (`--agent-card` or the positional URL), and A2A calls use the helper for the interface URL the client connects to, so headers never reach a host outside the helper's scope. That URL is also the `uri` the helper receives. Unlike Bazel, which sends the full URL of each request, a2a does not append the `/.well-known/agent-card.json` path or the A2A method path. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. -> **Warning:** a2a executes the configured helper command. The local `.env` is found by walking up from the current directory, so a `.env` in a cloned repository can set `A2ACLI_CREDENTIAL_HELPER`. Check the `.env` of untrusted directories, or run `a2a config list` to see where `credential-helper` comes from. +a2a executes the configured helper command, so `credential-helper` and `--unix-socket` are not read from the local `.env` found by walking up from the current directory: a `.env` in a cloned repository could otherwise run a command or redirect requests. a2a fails with a usage error when that `.env` sets either one. Set them with the flag, the environment, `~/.config/a2a-cli/config.yaml`, or a file passed with `--config`. ### `auth get` - Inspect credentials diff --git a/internal/cli/auth.go b/internal/cli/auth.go index c87a925..5b67498 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -31,7 +31,12 @@ https://github.com/EngFlow/credential-helper-spec. Its value is "[scope=]command [args...]", where scope is a host such as example.com, or *.example.com for a domain and its subdomains. For each request, the most specific helper for the request's host runs as "command args... get", and -its headers are sent unless --auth or --svc-param set the same header.`, +its headers are sent unless --auth or --svc-param set the same header. + +The helper receives the card reference for the card request and the +interface URL for A2A calls, not the full URL of each request. +credential-helper is not read from a .env found by walking up from the +working directory; pass that file with --config to use it.`, } cmd.AddCommand(newAuthGetCmd(cfg)) return cmd diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go index 5e6f014..693f7d6 100644 --- a/internal/cli/credentials_test.go +++ b/internal/cli/credentials_test.go @@ -32,6 +32,7 @@ import ( "github.com/a2aproject/a2a-go/v2/a2asrv" "github.com/google/go-cmp/cmp" + "github.com/a2aproject/a2a-cli/internal/clicfg" "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/localsrv" ) @@ -82,23 +83,24 @@ func startAuthRecordingServerWithInterface(t *testing.T, iface func(serverURL st return server.URL, rec } -// writeCredentialHelper writes a helper that answers "get" with the -// Authorization header "Bearer ". +// writeCredentialHelper returns a helper spec that answers "get" with the +// Authorization header "Bearer ". The script runs with sh: executing a +// file written while parallel tests fork can fail with ETXTBSY +// (golang/go#22315). func writeCredentialHelper(t *testing.T) string { t.Helper() if runtime.GOOS == "windows" { t.Skip("helper scripts need a POSIX shell") } path := filepath.Join(t.TempDir(), "helper") - script := `#!/bin/sh -[ "$1" = get ] || exit 2 + script := `[ "$1" = get ] || exit 2 uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/') printf '{"headers":{"Authorization":["Bearer %s"]}}' "$uri" ` - if err := os.WriteFile(path, []byte(script), 0o700); err != nil { + if err := os.WriteFile(path, []byte(script), 0o600); err != nil { t.Fatal(err) } - return path + return "sh " + path } func TestCredentialHelper(t *testing.T) { @@ -222,6 +224,38 @@ func TestCredentialHelperInvalidSpec(t *testing.T) { } } +func TestCredentialFlagsRejectedFromDiscoveredEnv(t *testing.T) { + t.Parallel() + tests := []struct { + name string + env string + }{ + {name: "credential helper", env: "A2ACLI_CREDENTIAL_HELPER=helper"}, + {name: "unix socket", env: "A2ACLI_UNIX_SOCKET=/tmp/a2a.sock"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, ".env"), []byte(tc.env+"\n"), 0o600); err != nil { + t.Fatal(err) + } + loader := cfgLoaderFunc(func(opts clicfg.LoadOpts) (*clicfg.Store, error) { + opts.WorkingDir = dir + opts.GlobalPath = filepath.Join(dir, "none.env") + opts.UserPath = filepath.Join(dir, "none.yaml") + opts.LookupEnv = func(string) (string, bool) { return "", false } + return clicfg.Load(opts) + }) + args := []string{"card", "get", "https://agent.example.com"} + _, err := runCMDWithConfig(t, deps{cfgLoader: loader}, args...) + if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage { + t.Fatalf("a2a %s error = %v, want a usage error", strings.Join(args, " "), err) + } + }) + } +} + func TestAuthGet(t *testing.T) { t.Parallel() helper := writeCredentialHelper(t) diff --git a/internal/cli/root.go b/internal/cli/root.go index 3a1d062..367c983 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -193,6 +193,17 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send HTTP requests through a Unix socket: for every host, or = for one host") pf.StringArrayVar(&cfg.credentialHelpers, "credential-helper", nil, "Credential helper returning the headers for an agent: [scope=]command [args...] (repeatable; see a2a auth --help)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") + annotations := map[string][]string{ + "unix-socket": {clicfg.AnnotationNotFromDiscoveredFile}, + "credential-helper": {clicfg.AnnotationNotFromDiscoveredFile, clicfg.AnnotationWholeValue}, + } + for name, keys := range annotations { + for _, key := range keys { + if err := pf.SetAnnotation(name, key, []string{"true"}); err != nil { + return nil, err + } + } + } cmd.AddCommand( newCardCmd(cfg), diff --git a/internal/clicfg/binder.go b/internal/clicfg/binder.go index 6bb28cd..8279163 100644 --- a/internal/clicfg/binder.go +++ b/internal/clicfg/binder.go @@ -26,6 +26,17 @@ import ( const envPrefix = "A2ACLI_" +// Flag annotations that change how [Bind] reads a flag from configuration. +const ( + // AnnotationWholeValue keeps a string value of a repeatable flag as one + // entry instead of splitting it on commas. + AnnotationWholeValue = "a2a-cli/whole-value" + // AnnotationNotFromDiscoveredFile rejects a value from a .env found by + // walking up from the working directory, for flags that a cloned + // repository must not be able to set. + AnnotationNotFromDiscoveredFile = "a2a-cli/not-from-discovered-file" +) + // FlagBinding records how a single flag was resolved. type FlagBinding struct { Name string @@ -75,6 +86,13 @@ func Bind(flags *pflag.FlagSet, store *Store) ([]FlagBinding, error) { return } + if source.Discovered && hasAnnotation(f, AnnotationNotFromDiscoveredFile) { + errs = append(errs, clierr.Usage(fmt.Sprintf( + "--%s is not read from %s, a .env found from the working directory; pass --config %s to use it, or set it in the environment or ~/.config/a2a-cli/config.yaml", + f.Name, source.Path, source.Path))) + return + } + if err := setFlagValue(f, value); err != nil { var msg string if source.Path != "" { @@ -157,7 +175,7 @@ func setFlagValue(f *pflag.Flag, raw any) error { switch v := raw.(type) { case string: if sv, ok := f.Value.(pflag.SliceValue); ok { - if !splitsOnComma(f.Name) { + if hasAnnotation(f, AnnotationWholeValue) { return sv.Replace([]string{v}) } parts := strings.Split(v, ",") @@ -174,10 +192,9 @@ func setFlagValue(f *pflag.Flag, raw any) error { } } -// splitsOnComma reports whether a single string value of a repeatable flag -// holds a comma-separated list. Credential helper commands may contain commas. -func splitsOnComma(name string) bool { - return name != "credential-helper" +func hasAnnotation(f *pflag.Flag, key string) bool { + _, ok := f.Annotations[key] + return ok } func flagToEnvVar(name string) string { diff --git a/internal/clicfg/binder_test.go b/internal/clicfg/binder_test.go index dce7978..39e9235 100644 --- a/internal/clicfg/binder_test.go +++ b/internal/clicfg/binder_test.go @@ -79,6 +79,10 @@ func TestApplyConfig(t *testing.T) { vals["tenant"] = fs.String("tenant", "", "") fs.StringArray("transport", nil, "") fs.StringArray("credential-helper", nil, "") + fs.Lookup("credential-helper").Annotations = map[string][]string{ + AnnotationWholeValue: {"true"}, + AnnotationNotFromDiscoveredFile: {"true"}, + } fs.Bool("insecure", false, "") fs.Bool("stream", false, "") fs.String("config", "", "") @@ -165,6 +169,45 @@ func TestApplyConfig(t *testing.T) { } }) + t.Run("rejects a protected flag from a discovered .env", func(t *testing.T) { + t.Parallel() + fs, _ := newFlags() + dir := t.TempDir() + writeDotenv(t, filepath.Join(dir, ".env"), map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper"}) + s, err := Load(LoadOpts{WorkingDir: dir, GlobalPath: filepath.Join(dir, "none.env"), LookupEnv: makeEnv(nil)}) + if err != nil { + t.Fatalf("config.Load() error = %v", err) + } + + _, err = Bind(fs, s) + if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage || !strings.Contains(err.Error(), "--config") { + t.Fatalf("Bind() error = %v, want a usage error suggesting --config", err) + } + }) + + t.Run("reads a protected flag from an explicit --config", func(t *testing.T) { + t.Parallel() + fs, _ := newFlags() + dir := t.TempDir() + cfgPath := filepath.Join(dir, ".env") + writeDotenv(t, cfgPath, map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper"}) + s, err := Load(LoadOpts{ConfigPath: cfgPath, WorkingDir: dir, GlobalPath: filepath.Join(dir, "none.env"), LookupEnv: makeEnv(nil)}) + if err != nil { + t.Fatalf("config.Load() error = %v", err) + } + + if _, err := Bind(fs, s); err != nil { + t.Fatalf("Bind() error = %v", err) + } + got, err := fs.GetStringArray("credential-helper") + if err != nil { + t.Fatalf("fs.GetStringArray() error = %v", err) + } + if diff := cmp.Diff([]string{"helper"}, got); diff != "" { + t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) + } + }) + t.Run("excluded flags are never read from config", func(t *testing.T) { t.Parallel() fs, _ := newFlags() diff --git a/internal/clicfg/loader.go b/internal/clicfg/loader.go index 5d9756f..a6d1840 100644 --- a/internal/clicfg/loader.go +++ b/internal/clicfg/loader.go @@ -104,7 +104,7 @@ func loadLocal(opts LoadOpts) (*loadedFile, error) { if err != nil { return nil, err } - return &loadedFile{path: path, values: toStringAnyMap(values)}, nil + return &loadedFile{path: path, values: toStringAnyMap(values), discovered: true}, nil } func loadGlobal(opts LoadOpts) (*loadedFile, error) { diff --git a/internal/clicfg/store.go b/internal/clicfg/store.go index ac67ce4..a05379c 100644 --- a/internal/clicfg/store.go +++ b/internal/clicfg/store.go @@ -40,6 +40,9 @@ type Source struct { Kind SourceKind // Path is set for file-backed sources. Path string + // Discovered is set when the file is a .env found by walking up from the + // working directory rather than one named by --config. + Discovered bool } // String implements Stringer. @@ -92,7 +95,7 @@ func (s *Store) LookupFlag(name, envVar string) (any, Source, bool) { } if s.local != nil { if v, ok := s.local.lookup(name, envVar); ok { - return v, Source{Kind: SourceLocalFile, Path: s.local.path}, true + return v, Source{Kind: SourceLocalFile, Path: s.local.path, Discovered: s.local.discovered}, true } } if s.user != nil { @@ -109,8 +112,9 @@ func (s *Store) LookupFlag(name, envVar string) (any, Source, bool) { } type loadedFile struct { - path string - values map[string]any + path string + values map[string]any + discovered bool } func (f *loadedFile) lookup(name, envVar string) (any, bool) { diff --git a/internal/credhelper/credhelper_test.go b/internal/credhelper/credhelper_test.go index 5f1d731..f4226f5 100644 --- a/internal/credhelper/credhelper_test.go +++ b/internal/credhelper/credhelper_test.go @@ -101,16 +101,18 @@ func TestSelect(t *testing.T) { } } +// writeHelper returns a helper spec running script with sh. Executing a file +// written while parallel tests fork can fail with ETXTBSY (golang/go#22315). func writeHelper(t *testing.T, script string) string { t.Helper() if runtime.GOOS == "windows" { t.Skip("helper scripts need a POSIX shell") } path := filepath.Join(t.TempDir(), "helper") - if err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o700); err != nil { + if err := os.WriteFile(path, []byte(script), 0o600); err != nil { t.Fatal(err) } - return path + return "sh " + path } func TestGet(t *testing.T) { diff --git a/internal/credhelper/source.go b/internal/credhelper/source.go index 48097bf..f340b1d 100644 --- a/internal/credhelper/source.go +++ b/internal/credhelper/source.go @@ -71,6 +71,8 @@ func expired(resp *Response) bool { // Interceptor adds the headers from the helper for each call's interface URL // as service parameters, unless the call already carries the same parameter. +// The helper receives the interface URL rather than the URL of the method, +// which the interceptor does not see. type Interceptor struct { a2aclient.PassthroughInterceptor Source *Source From 7b5e85d074d8f40c9585b24a0074d03d178a1bdc Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 11:04:28 +0000 Subject: [PATCH 4/8] fix: ignore credential helpers and unix sockets from discovered .env files Failing every command when a cloned repository's .env sets them was more disruptive than needed; ignoring the value with a warning is equally safe. --- internal/README.md | 4 +-- internal/cli/cli_test.go | 7 +++- internal/cli/credentials_test.go | 25 +++++++++----- internal/cli/root.go | 8 +++++ internal/clicfg/binder.go | 16 ++++----- internal/clicfg/binder_test.go | 56 ++++++++++++++++++++++++++------ internal/clicfg/store.go | 12 ++++++- 7 files changed, 98 insertions(+), 30 deletions(-) diff --git a/internal/README.md b/internal/README.md index 9d7628a..178c22a 100644 --- a/internal/README.md +++ b/internal/README.md @@ -276,9 +276,9 @@ credential-helper: - "*.corp.example.com=/usr/local/bin/corp-helper" ``` -Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card reference as given (`--agent-card` or the positional URL), and A2A calls use the helper for the interface URL the client connects to, so headers never reach a host outside the helper's scope. That URL is also the `uri` the helper receives. Unlike Bazel, which sends the full URL of each request, a2a does not append the `/.well-known/agent-card.json` path or the A2A method path. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. +Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card reference as given (`--agent-card` or the positional URL), and A2A calls use the helper for the interface URL the client connects to. That URL is also the `uri` the helper receives. Unlike Bazel, which sends the full URL of each request, a2a does not append the `/.well-known/agent-card.json` path or the A2A method path. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. -a2a executes the configured helper command, so `credential-helper` and `--unix-socket` are not read from the local `.env` found by walking up from the current directory: a `.env` in a cloned repository could otherwise run a command or redirect requests. a2a fails with a usage error when that `.env` sets either one. Set them with the flag, the environment, `~/.config/a2a-cli/config.yaml`, or a file passed with `--config`. +a2a executes the configured helper command, so `credential-helper` and `--unix-socket` are not read from the local `.env` found by walking up from the current directory: a `.env` in a cloned repository could otherwise run a command or redirect requests. a2a ignores them there with a warning. Set them with the flag, the environment, `~/.config/a2a-cli/config.yaml`, or a file passed with `--config`. ### `auth get` - Inspect credentials diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 024e1fb..8556ceb 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1220,6 +1220,11 @@ func runCMDWithConfig(t *testing.T, deps deps, args ...string) (string, error) { } func runCMDCapturingStderr(t *testing.T, args ...string) (stdout, stderr string, err error) { + t.Helper() + return runCMDCapturingStderrWithConfig(t, deps{poller: polling.Stream, cfgLoader: clicfg.LoadEmpty}, args...) +} + +func runCMDCapturingStderrWithConfig(t *testing.T, deps deps, args ...string) (stdout, stderr string, err error) { t.Helper() var out, errBuf bytes.Buffer cfg := &globalConfig{ @@ -1227,7 +1232,7 @@ func runCMDCapturingStderr(t *testing.T, args ...string) (stdout, stderr string, svcParams: &flagparse.ServiceParams{}, errOut: &errBuf, } - root := mustNewRoot(t, cfg, deps{poller: polling.Stream, cfgLoader: clicfg.LoadEmpty}) + root := mustNewRoot(t, cfg, deps) root.SetArgs(args) err = root.Execute() return out.String(), errBuf.String(), err diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go index 693f7d6..3555ac3 100644 --- a/internal/cli/credentials_test.go +++ b/internal/cli/credentials_test.go @@ -224,20 +224,23 @@ func TestCredentialHelperInvalidSpec(t *testing.T) { } } -func TestCredentialFlagsRejectedFromDiscoveredEnv(t *testing.T) { +func TestCredentialFlagsIgnoredFromDiscoveredEnv(t *testing.T) { t.Parallel() tests := []struct { name string env string + flag string }{ - {name: "credential helper", env: "A2ACLI_CREDENTIAL_HELPER=helper"}, - {name: "unix socket", env: "A2ACLI_UNIX_SOCKET=/tmp/a2a.sock"}, + {name: "credential helper", env: "A2ACLI_CREDENTIAL_HELPER=false", flag: "credential-helper"}, + {name: "unix socket", env: "A2ACLI_UNIX_SOCKET=/nonexistent/a2a.sock", flag: "unix-socket"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() + url, rec := startAuthRecordingServer(t) dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, ".env"), []byte(tc.env+"\n"), 0o600); err != nil { + envPath := filepath.Join(dir, ".env") + if err := os.WriteFile(envPath, []byte(tc.env+"\n"), 0o600); err != nil { t.Fatal(err) } loader := cfgLoaderFunc(func(opts clicfg.LoadOpts) (*clicfg.Store, error) { @@ -247,10 +250,16 @@ func TestCredentialFlagsRejectedFromDiscoveredEnv(t *testing.T) { opts.LookupEnv = func(string) (string, bool) { return "", false } return clicfg.Load(opts) }) - args := []string{"card", "get", "https://agent.example.com"} - _, err := runCMDWithConfig(t, deps{cfgLoader: loader}, args...) - if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage { - t.Fatalf("a2a %s error = %v, want a usage error", strings.Join(args, " "), err) + args := []string{"card", "get", url} + _, stderr, err := runCMDCapturingStderrWithConfig(t, deps{cfgLoader: loader}, args...) + if err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) + } + if want := "warning: ignoring " + tc.flag + " from " + envPath; !strings.Contains(stderr, want) { + t.Fatalf("a2a %s stderr = %q, want %q", strings.Join(args, " "), stderr, want) + } + if len(rec.all()) == 0 { + t.Fatalf("a2a %s sent no requests", strings.Join(args, " ")) } }) } diff --git a/internal/cli/root.go b/internal/cli/root.go index 367c983..c0fb3b3 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -161,6 +161,14 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { } cfg.bindings = bindings cfg.store = store + for _, b := range bindings { + if b.Ignored == "" { + continue + } + if _, err := fmt.Fprintf(cfg.stderr(), "warning: ignoring %s from %s; pass --config %s to use it\n", b.Name, b.Ignored, b.Ignored); err != nil { + return err + } + } if err := cfg.initHTTPTransport(); err != nil { return err } diff --git a/internal/clicfg/binder.go b/internal/clicfg/binder.go index 8279163..1831c4c 100644 --- a/internal/clicfg/binder.go +++ b/internal/clicfg/binder.go @@ -31,7 +31,7 @@ const ( // AnnotationWholeValue keeps a string value of a repeatable flag as one // entry instead of splitting it on commas. AnnotationWholeValue = "a2a-cli/whole-value" - // AnnotationNotFromDiscoveredFile rejects a value from a .env found by + // AnnotationNotFromDiscoveredFile ignores a value from a .env found by // walking up from the working directory, for flags that a cloned // repository must not be able to set. AnnotationNotFromDiscoveredFile = "a2a-cli/not-from-discovered-file" @@ -45,6 +45,9 @@ type FlagBinding struct { Source string Path string Sensitive bool + // Ignored is the path of a discovered .env whose value for the flag was + // ignored because of [AnnotationNotFromDiscoveredFile]. + Ignored string } // Bind fills every eligible flag that was not set on the command line from @@ -79,6 +82,10 @@ func Bind(flags *pflag.FlagSet, store *Store) ([]FlagBinding, error) { } value, source, ok := store.LookupFlag(binding.Name, binding.EnvVar) + if ok && source.Discovered && hasAnnotation(f, AnnotationNotFromDiscoveredFile) { + binding.Ignored = source.Path + value, source, ok = store.LookupFlagSkipDiscovered(binding.Name, binding.EnvVar) + } if !ok { binding.Source = "default" binding.Value = flagValueString(f) @@ -86,13 +93,6 @@ func Bind(flags *pflag.FlagSet, store *Store) ([]FlagBinding, error) { return } - if source.Discovered && hasAnnotation(f, AnnotationNotFromDiscoveredFile) { - errs = append(errs, clierr.Usage(fmt.Sprintf( - "--%s is not read from %s, a .env found from the working directory; pass --config %s to use it, or set it in the environment or ~/.config/a2a-cli/config.yaml", - f.Name, source.Path, source.Path))) - return - } - if err := setFlagValue(f, value); err != nil { var msg string if source.Path != "" { diff --git a/internal/clicfg/binder_test.go b/internal/clicfg/binder_test.go index 39e9235..3de1566 100644 --- a/internal/clicfg/binder_test.go +++ b/internal/clicfg/binder_test.go @@ -169,19 +169,55 @@ func TestApplyConfig(t *testing.T) { } }) - t.Run("rejects a protected flag from a discovered .env", func(t *testing.T) { + t.Run("ignores a protected flag from a discovered .env", func(t *testing.T) { t.Parallel() - fs, _ := newFlags() - dir := t.TempDir() - writeDotenv(t, filepath.Join(dir, ".env"), map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper"}) - s, err := Load(LoadOpts{WorkingDir: dir, GlobalPath: filepath.Join(dir, "none.env"), LookupEnv: makeEnv(nil)}) - if err != nil { - t.Fatalf("config.Load() error = %v", err) + tests := []struct { + name string + user string + want []string + }{ + {name: "unset", want: []string{}}, + {name: "falls back to the user config", user: "credential-helper:\n - user-helper\n", want: []string{"user-helper"}}, } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + fs, _ := newFlags() + dir := t.TempDir() + envPath := filepath.Join(dir, ".env") + writeDotenv(t, envPath, map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper"}) + userPath := filepath.Join(dir, "config.yaml") + if tc.user != "" { + if err := os.WriteFile(userPath, []byte(tc.user), 0o600); err != nil { + t.Fatalf("os.WriteFile(%q) error = %v", userPath, err) + } + } + s, err := Load(LoadOpts{WorkingDir: dir, GlobalPath: filepath.Join(dir, "none.env"), UserPath: userPath, LookupEnv: makeEnv(nil)}) + if err != nil { + t.Fatalf("config.Load() error = %v", err) + } - _, err = Bind(fs, s) - if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage || !strings.Contains(err.Error(), "--config") { - t.Fatalf("Bind() error = %v, want a usage error suggesting --config", err) + bindings, err := Bind(fs, s) + if err != nil { + t.Fatalf("Bind() error = %v", err) + } + got, err := fs.GetStringArray("credential-helper") + if err != nil { + t.Fatalf("fs.GetStringArray() error = %v", err) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) + } + var ignored string + for _, b := range bindings { + if b.Name == "credential-helper" { + ignored = b.Ignored + } + } + if ignored != envPath { + t.Fatalf("Bind() credential-helper Ignored = %q, want %q", ignored, envPath) + } + }) } }) diff --git a/internal/clicfg/store.go b/internal/clicfg/store.go index a05379c..cb4700c 100644 --- a/internal/clicfg/store.go +++ b/internal/clicfg/store.go @@ -88,12 +88,22 @@ func (s *Store) lookupValue(key string) (any, Source, bool) { // LookupFlag returns the configuration value for a flag, checking environment variables // by envVar and config files by flag name or envVar. func (s *Store) LookupFlag(name, envVar string) (any, Source, bool) { + return s.lookupFlag(name, envVar, false) +} + +// LookupFlagSkipDiscovered is [Store.LookupFlag] without a .env found by +// walking up from the working directory. +func (s *Store) LookupFlagSkipDiscovered(name, envVar string) (any, Source, bool) { + return s.lookupFlag(name, envVar, true) +} + +func (s *Store) lookupFlag(name, envVar string, skipDiscovered bool) (any, Source, bool) { if envVar != "" { if v, ok := s.lookupEnv(envVar); ok { return v, Source{Kind: SourceEnv}, true } } - if s.local != nil { + if s.local != nil && !(skipDiscovered && s.local.discovered) { if v, ok := s.local.lookup(name, envVar); ok { return v, Source{Kind: SourceLocalFile, Path: s.local.path, Discovered: s.local.discovered}, true } From 2e5799fef0da2d524bde631370c77441dc0c871d Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 12:13:42 +0000 Subject: [PATCH 5/8] fix: keep the auth group help general More auth commands are planned (#98), so credential helper details belong on auth get. --- internal/cli/auth.go | 17 +---------------- internal/cli/auth_get.go | 7 +++++++ internal/cli/root.go | 2 +- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 5b67498..19e151d 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -21,22 +21,7 @@ import ( func newAuthCmd(cfg *globalConfig) *cobra.Command { cmd := &cobra.Command{ Use: "auth", - Short: "Work with the credentials sent to agents", - Long: `Work with the credentials sent to agents. - -A credential helper (--credential-helper, A2ACLI_CREDENTIAL_HELPER, or -credential-helper in the config file) is a program that returns the HTTP -headers to send to an agent, following the Bazel credential helper protocol: -https://github.com/EngFlow/credential-helper-spec. Its value is -"[scope=]command [args...]", where scope is a host such as example.com, or -*.example.com for a domain and its subdomains. For each request, the most -specific helper for the request's host runs as "command args... get", and -its headers are sent unless --auth or --svc-param set the same header. - -The helper receives the card reference for the card request and the -interface URL for A2A calls, not the full URL of each request. -credential-helper is not read from a .env found by walking up from the -working directory; pass that file with --config to use it.`, + Short: "Manage the credentials sent to agents", } cmd.AddCommand(newAuthGetCmd(cfg)) return cmd diff --git a/internal/cli/auth_get.go b/internal/cli/auth_get.go index 5a2f6ef..de0b63b 100644 --- a/internal/cli/auth_get.go +++ b/internal/cli/auth_get.go @@ -32,6 +32,13 @@ func newAuthGetCmd(cfg *globalConfig) *cobra.Command { cmd := &cobra.Command{ Use: "get ", Short: "Print the headers the credential helper returns for an agent", + Long: `Print the headers the credential helper returns for an agent. + +A credential helper (--credential-helper) follows the Bazel credential helper +protocol: https://github.com/EngFlow/credential-helper-spec. Its value is +"[scope=]command [args...]", where scope is a host such as example.com, or +*.example.com for a domain and its subdomains. The most specific helper for +the agent's host runs.`, Example: ` a2a auth get https://agent.example.com a2a auth get https://agent.example.com --reveal`, Args: cobra.ExactArgs(1), diff --git a/internal/cli/root.go b/internal/cli/root.go index 8c04d68..ef66cd7 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -210,7 +210,7 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.BoolVarP(&cfg.verbose, "verbose", "v", false, "Verbose output to stderr") pf.BoolVar(&cfg.insecureGRPC, "insecure", false, "Use insecure (plaintext) gRPC transport credentials") pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send HTTP requests through a Unix socket: for every host, or = for one host") - pf.StringArrayVar(&cfg.credentialHelpers, "credential-helper", nil, "Credential helper returning the headers for an agent: [scope=]command [args...] (repeatable; see a2a auth --help)") + pf.StringArrayVar(&cfg.credentialHelpers, "credential-helper", nil, "Credential helper returning the headers for an agent: [scope=]command [args...] (repeatable; see a2a auth get --help)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") annotations := map[string][]string{ "unix-socket": {clicfg.AnnotationNotFromDiscoveredFile}, From fd8e80597c5f16940b33d46dccf999a8928091d8 Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 13:22:15 +0000 Subject: [PATCH 6/8] fix: configure credential helpers as YAML entries instead of shell strings Shell-split command strings were cryptic and pulled in shlex. Config files now list {scope, command} entries, and the flag and env var take a Bazel-style [scope=]path. --- go.mod | 1 - go.sum | 2 - internal/README.md | 13 +-- internal/cli/auth_get.go | 10 +- internal/cli/cli_test.go | 2 +- internal/cli/credentials.go | 14 +-- internal/cli/credentials_test.go | 84 +++++++++++++---- internal/cli/root.go | 20 ++-- internal/clicfg/binder.go | 29 +++--- internal/clicfg/binder_test.go | 29 ++---- internal/credhelper/credhelper.go | 74 +++++++++------ internal/credhelper/credhelper_test.go | 45 ++++----- internal/credhelper/flag.go | 121 +++++++++++++++++++++++++ internal/credhelper/flag_test.go | 70 ++++++++++++++ 14 files changed, 370 insertions(+), 144 deletions(-) create mode 100644 internal/credhelper/flag.go create mode 100644 internal/credhelper/flag_test.go diff --git a/go.mod b/go.mod index 6952bcf..fb68b07 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/a2aproject/a2a-go v0.3.15 github.com/a2aproject/a2a-go/v2 v2.6.0 github.com/google/go-cmp v0.7.0 - github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/joho/godotenv v1.5.1 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index 0028932..a0d3013 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,6 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= -github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= diff --git a/internal/README.md b/internal/README.md index 1894dcc..a425b56 100644 --- a/internal/README.md +++ b/internal/README.md @@ -37,7 +37,7 @@ These apply to every client-mode command. Each command selects the agent it talk | `--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="`. | -| `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]command [args...]`. Repeatable. Not read from the local `.env`. See [Credentials](#credentials). | +| `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]path`. Repeatable. Not read from the local `.env`. See [Credentials](#credentials). | | `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. Not read from the local `.env`. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | @@ -297,18 +297,19 @@ Streams events to stdout until the task reaches a terminal state. Output format ## Credentials -A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` [args...] get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. +A credential helper is a program that returns the HTTP headers to send to an agent. a2a implements the [Bazel credential helper protocol](https://github.com/EngFlow/credential-helper-spec), so helpers written for Bazel work unchanged: a2a runs ` get`, writes `{"uri": ""}` to its stdin, and reads `{"headers": {"Authorization": ["Bearer …"]}, "expires": ""}` from its stdout. -Configure helpers with `--credential-helper`, `A2ACLI_CREDENTIAL_HELPER` (one helper; commas are not separators), or `credential-helper` in a config file: +List helpers under `credential-helper` in a config file: ```yaml # ~/.config/a2a-cli/config.yaml credential-helper: - - a2a-serve credential # every host - - "*.corp.example.com=/usr/local/bin/corp-helper" + - command: [a2a-serve, credential] # every host + - scope: "*.corp.example.com" + command: [/usr/local/bin/corp-helper] ``` -Each value is `[scope=]command [args...]`. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card reference as given (`--agent-card` or the positional URL), and A2A calls use the helper for the interface URL the client connects to. That URL is also the `uri` the helper receives. Unlike Bazel, which sends the full URL of each request, a2a does not append the `/.well-known/agent-card.json` path or the A2A method path. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. +`--credential-helper` and `A2ACLI_CREDENTIAL_HELPER` take `[scope=]path`, like Bazel's `--credential_helper`: a path with no arguments. A scope is a host without a port (`example.com`), or `*.example.com` for the domain and its subdomains. Helpers are chosen per request: the card request uses the helper for the card reference as given (`--agent-card` or the positional URL), and A2A calls use the helper for the interface URL the client connects to. That URL is also the `uri` the helper receives. Unlike Bazel, which sends the full URL of each request, a2a does not append the `/.well-known/agent-card.json` path or the A2A method path. The helper with the most specific matching scope runs; among equal scopes the last one wins. Its headers are sent unless `--auth` or `--svc-param` set the same header. a2a runs a helper once per URL and reuses its headers until they expire. `--timeout` bounds each run. A helper that exits non-zero fails the command; a helper with no credentials for the URL prints `{}`. a2a executes the configured helper command, so `credential-helper` and `--unix-socket` are not read from the local `.env` found by walking up from the current directory: a `.env` in a cloned repository could otherwise run a command or redirect requests. a2a ignores them there with a warning. Set them with the flag, the environment, `~/.config/a2a-cli/config.yaml`, or a file passed with `--config`. diff --git a/internal/cli/auth_get.go b/internal/cli/auth_get.go index de0b63b..5843609 100644 --- a/internal/cli/auth_get.go +++ b/internal/cli/auth_get.go @@ -34,11 +34,11 @@ func newAuthGetCmd(cfg *globalConfig) *cobra.Command { Short: "Print the headers the credential helper returns for an agent", Long: `Print the headers the credential helper returns for an agent. -A credential helper (--credential-helper) follows the Bazel credential helper -protocol: https://github.com/EngFlow/credential-helper-spec. Its value is -"[scope=]command [args...]", where scope is a host such as example.com, or -*.example.com for a domain and its subdomains. The most specific helper for -the agent's host runs.`, +A credential helper follows the Bazel credential helper protocol: +https://github.com/EngFlow/credential-helper-spec. --credential-helper takes +"[scope=]path", where scope is a host such as example.com, or *.example.com +for a domain and its subdomains. Config files list {scope, command} entries +under credential-helper. The most specific helper for the agent's host runs.`, Example: ` a2a auth get https://agent.example.com a2a auth get https://agent.example.com --reveal`, Args: cobra.ExactArgs(1), diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index ff2dc37..5871cb1 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1143,7 +1143,7 @@ func TestInsecureCredentialWarning(t *testing.T) { }, { name: "insecure with a credential helper warns", - args: []string{"send", "-a", url, "--insecure", "--credential-helper", writeCredentialHelper(t), "hi"}, + args: []string{"send", "-a", url, "--insecure", "--credential-helper", credentialHelper(t), "hi"}, wantWarn: true, }, } diff --git a/internal/cli/credentials.go b/internal/cli/credentials.go index 6c02afe..29e6a7e 100644 --- a/internal/cli/credentials.go +++ b/internal/cli/credentials.go @@ -41,18 +41,12 @@ func (g *globalConfig) initHTTPTransport() error { return nil } -// initCredentials parses --credential-helper. Helpers run only when a +// initCredentials sets up --credential-helper. Helpers run only when a // request is made to a host in their scope. -func (g *globalConfig) initCredentials() error { - if len(g.credentialHelpers) == 0 { - return nil +func (g *globalConfig) initCredentials() { + if helpers := g.credentialHelpers.Helpers(); len(helpers) > 0 { + g.credentials = &credhelper.Source{Helpers: helpers, Logf: g.logf} } - helpers, err := credhelper.ParseAll(g.credentialHelpers) - if err != nil { - return clierr.Usage(err.Error()) - } - g.credentials = &credhelper.Source{Helpers: helpers, Logf: g.logf} - return nil } // cardHeaders returns the helper headers for the card request to ref that diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go index 3555ac3..0dfa268 100644 --- a/internal/cli/credentials_test.go +++ b/internal/cli/credentials_test.go @@ -17,6 +17,7 @@ package cli import ( "context" "encoding/json" + "fmt" "iter" "net" "net/http" @@ -83,29 +84,51 @@ func startAuthRecordingServerWithInterface(t *testing.T, iface func(serverURL st return server.URL, rec } -// writeCredentialHelper returns a helper spec that answers "get" with the -// Authorization header "Bearer ". The script runs with sh: executing a -// file written while parallel tests fork can fail with ETXTBSY +// credentialHelperPath is a helper that answers "get" with the Authorization +// header "Bearer ". TestMain writes it before tests run in parallel: +// executing a file written while other tests fork can fail with ETXTBSY // (golang/go#22315). -func writeCredentialHelper(t *testing.T) string { - t.Helper() - if runtime.GOOS == "windows" { - t.Skip("helper scripts need a POSIX shell") +var credentialHelperPath string + +func TestMain(m *testing.M) { + os.Exit(runTests(m)) +} + +func runTests(m *testing.M) int { + dir, err := os.MkdirTemp("", "a2a-cli-test") + if err != nil { + fmt.Fprintln(os.Stderr, err) + return 1 } - path := filepath.Join(t.TempDir(), "helper") - script := `[ "$1" = get ] || exit 2 + defer func() { + if err := os.RemoveAll(dir); err != nil { + fmt.Fprintln(os.Stderr, err) + } + }() + credentialHelperPath = filepath.Join(dir, "helper") + script := `#!/bin/sh +[ "$1" = get ] || exit 2 uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/') printf '{"headers":{"Authorization":["Bearer %s"]}}' "$uri" ` - if err := os.WriteFile(path, []byte(script), 0o600); err != nil { - t.Fatal(err) + if err := os.WriteFile(credentialHelperPath, []byte(script), 0o700); err != nil { + fmt.Fprintln(os.Stderr, err) + return 1 + } + return m.Run() +} + +func credentialHelper(t *testing.T) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("helper scripts need a POSIX shell") } - return "sh " + path + return credentialHelperPath } func TestCredentialHelper(t *testing.T) { t.Parallel() - helper := writeCredentialHelper(t) + helper := credentialHelper(t) tests := []struct { name string args func(url string) []string @@ -177,9 +200,38 @@ func TestCredentialHelper(t *testing.T) { } } +func TestCredentialHelperFromConfigFile(t *testing.T) { + t.Parallel() + url, rec := startAuthRecordingServer(t) + dir := t.TempDir() + userPath := filepath.Join(dir, "config.yaml") + yaml := fmt.Sprintf("credential-helper:\n - scope: 127.0.0.1\n command: [sh, %q]\n", credentialHelper(t)) + if err := os.WriteFile(userPath, []byte(yaml), 0o600); err != nil { + t.Fatal(err) + } + loader := cfgLoaderFunc(func(opts clicfg.LoadOpts) (*clicfg.Store, error) { + opts.WorkingDir = dir + opts.GlobalPath = filepath.Join(dir, "none.env") + opts.UserPath = userPath + opts.LookupEnv = func(string) (string, bool) { return "", false } + return clicfg.Load(opts) + }) + args := []string{"send", "-a", url, "hello"} + if _, err := runCMDWithConfig(t, deps{cfgLoader: loader}, args...); err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) + } + want := []recordedRequest{ + {Path: a2asrv.WellKnownAgentCardPath, Auth: "Bearer " + url}, + {Path: "/message:send", Auth: "Bearer " + url}, + } + if diff := cmp.Diff(want, rec.all()); diff != "" { + t.Fatalf("a2a %s wrong result (-want +got) diff = %s", strings.Join(args, " "), diff) + } +} + func TestCredentialHelperScopedToRequestHost(t *testing.T) { t.Parallel() - helper := writeCredentialHelper(t) + helper := credentialHelper(t) url, rec := startAuthRecordingServerWithInterface(t, func(serverURL string) string { return strings.Replace(serverURL, "127.0.0.1", "localhost", 1) }) @@ -217,7 +269,7 @@ func TestCredentialHelperOnlyRunsForRequests(t *testing.T) { func TestCredentialHelperInvalidSpec(t *testing.T) { t.Parallel() - args := []string{"send", "-a", "https://agent.example.com", "--credential-helper", `sh -c "unterminated`, "hello"} + args := []string{"send", "-a", "https://agent.example.com", "--credential-helper", "example.com=", "hello"} _, err := runCMD(t, args...) if ce := clierr.Classify(err); ce.Code != clierr.CodeUsage { t.Fatalf("a2a %s error = %v, want a usage error", strings.Join(args, " "), err) @@ -267,7 +319,7 @@ func TestCredentialFlagsIgnoredFromDiscoveredEnv(t *testing.T) { func TestAuthGet(t *testing.T) { t.Parallel() - helper := writeCredentialHelper(t) + helper := credentialHelper(t) tests := []struct { name string args []string diff --git a/internal/cli/root.go b/internal/cli/root.go index ef66cd7..f3c466d 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -78,7 +78,7 @@ type globalConfig struct { unixSocket string configPath string - credentialHelpers []string + credentialHelpers credhelper.Flag credentials *credhelper.Source pluginsEnabled bool @@ -183,9 +183,7 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { if err := cfg.initHTTPTransport(); err != nil { return err } - if err := cfg.initCredentials(); err != nil { - return err - } + cfg.initCredentials() switch output.Mode(cfg.output) { case output.ModeText, output.ModeJson, output.ModeJSONL: @@ -210,17 +208,11 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.BoolVarP(&cfg.verbose, "verbose", "v", false, "Verbose output to stderr") pf.BoolVar(&cfg.insecureGRPC, "insecure", false, "Use insecure (plaintext) gRPC transport credentials") pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send HTTP requests through a Unix socket: for every host, or = for one host") - pf.StringArrayVar(&cfg.credentialHelpers, "credential-helper", nil, "Credential helper returning the headers for an agent: [scope=]command [args...] (repeatable; see a2a auth get --help)") + pf.Var(&cfg.credentialHelpers, "credential-helper", "Credential helper returning the headers for an agent: [scope=]path (repeatable; see a2a auth get --help)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") - annotations := map[string][]string{ - "unix-socket": {clicfg.AnnotationNotFromDiscoveredFile}, - "credential-helper": {clicfg.AnnotationNotFromDiscoveredFile, clicfg.AnnotationWholeValue}, - } - for name, keys := range annotations { - for _, key := range keys { - if err := pf.SetAnnotation(name, key, []string{"true"}); err != nil { - return nil, err - } + for _, name := range []string{"unix-socket", "credential-helper"} { + if err := pf.SetAnnotation(name, clicfg.AnnotationNotFromDiscoveredFile, []string{"true"}); err != nil { + return nil, err } } diff --git a/internal/clicfg/binder.go b/internal/clicfg/binder.go index 1831c4c..f6efffc 100644 --- a/internal/clicfg/binder.go +++ b/internal/clicfg/binder.go @@ -26,16 +26,10 @@ import ( const envPrefix = "A2ACLI_" -// Flag annotations that change how [Bind] reads a flag from configuration. -const ( - // AnnotationWholeValue keeps a string value of a repeatable flag as one - // entry instead of splitting it on commas. - AnnotationWholeValue = "a2a-cli/whole-value" - // AnnotationNotFromDiscoveredFile ignores a value from a .env found by - // walking up from the working directory, for flags that a cloned - // repository must not be able to set. - AnnotationNotFromDiscoveredFile = "a2a-cli/not-from-discovered-file" -) +// AnnotationNotFromDiscoveredFile is a flag annotation that makes [Bind] +// ignore a value from a .env found by walking up from the working directory, +// for flags that a cloned repository must not be able to set. +const AnnotationNotFromDiscoveredFile = "a2a-cli/not-from-discovered-file" // FlagBinding records how a single flag was resolved. type FlagBinding struct { @@ -113,6 +107,9 @@ func Bind(flags *pflag.FlagSet, store *Store) ([]FlagBinding, error) { value = redactServiceParams(value) } binding.Value = formatValue(value) + if _, ok := f.Value.(ConfigValue); ok { + binding.Value = flagValueString(f) + } bindings = append(bindings, binding) }) @@ -159,7 +156,16 @@ func flagValueString(f *pflag.Flag) string { return f.Value.String() } +// ConfigValue is a flag value that reads configuration values itself, such +// as lists of objects that have no string form. +type ConfigValue interface { + SetConfig(raw any) error +} + func setFlagValue(f *pflag.Flag, raw any) error { + if cv, ok := f.Value.(ConfigValue); ok { + return cv.SetConfig(raw) + } if slice, ok := toStringSlice(raw); ok { if sv, ok := f.Value.(pflag.SliceValue); ok { return sv.Replace(slice) @@ -175,9 +181,6 @@ func setFlagValue(f *pflag.Flag, raw any) error { switch v := raw.(type) { case string: if sv, ok := f.Value.(pflag.SliceValue); ok { - if hasAnnotation(f, AnnotationWholeValue) { - return sv.Replace([]string{v}) - } parts := strings.Split(v, ",") for i := range parts { parts[i] = strings.TrimSpace(parts[i]) diff --git a/internal/clicfg/binder_test.go b/internal/clicfg/binder_test.go index 3de1566..c97a201 100644 --- a/internal/clicfg/binder_test.go +++ b/internal/clicfg/binder_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/a2aproject/a2a-cli/internal/clierr" + "github.com/a2aproject/a2a-cli/internal/credhelper" "github.com/google/go-cmp/cmp" "github.com/spf13/pflag" ) @@ -78,11 +79,8 @@ func TestApplyConfig(t *testing.T) { vals["agent-card"] = fs.String("agent-card", "", "") vals["tenant"] = fs.String("tenant", "", "") fs.StringArray("transport", nil, "") - fs.StringArray("credential-helper", nil, "") - fs.Lookup("credential-helper").Annotations = map[string][]string{ - AnnotationWholeValue: {"true"}, - AnnotationNotFromDiscoveredFile: {"true"}, - } + fs.Var(&credhelper.Flag{}, "credential-helper", "") + fs.Lookup("credential-helper").Annotations = map[string][]string{AnnotationNotFromDiscoveredFile: {"true"}} fs.Bool("insecure", false, "") fs.Bool("stream", false, "") fs.String("config", "", "") @@ -155,16 +153,13 @@ func TestApplyConfig(t *testing.T) { t.Run("keeps a credential helper with commas as one entry", func(t *testing.T) { t.Parallel() fs, _ := newFlags() - s := store(t, map[string]string{"A2ACLI_CREDENTIAL_HELPER": "helper --scopes=a,b"}) + s := store(t, map[string]string{"A2ACLI_CREDENTIAL_HELPER": "/opt/helper,v2"}) if _, err := Bind(fs, s); err != nil { t.Fatalf("applyConfig() error = %v", err) } - got, err := fs.GetStringArray("credential-helper") - if err != nil { - t.Fatalf("fs.GetStringArray() error = %v", err) - } - if diff := cmp.Diff([]string{"helper --scopes=a,b"}, got); diff != "" { + got := fs.Lookup("credential-helper").Value.(pflag.SliceValue).GetSlice() + if diff := cmp.Diff([]string{"/opt/helper,v2"}, got); diff != "" { t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) } }) @@ -177,7 +172,7 @@ func TestApplyConfig(t *testing.T) { want []string }{ {name: "unset", want: []string{}}, - {name: "falls back to the user config", user: "credential-helper:\n - user-helper\n", want: []string{"user-helper"}}, + {name: "falls back to the user config", user: "credential-helper:\n - command: [user-helper, --flag]\n", want: []string{"user-helper --flag"}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -201,10 +196,7 @@ func TestApplyConfig(t *testing.T) { if err != nil { t.Fatalf("Bind() error = %v", err) } - got, err := fs.GetStringArray("credential-helper") - if err != nil { - t.Fatalf("fs.GetStringArray() error = %v", err) - } + got := fs.Lookup("credential-helper").Value.(pflag.SliceValue).GetSlice() if diff := cmp.Diff(tc.want, got); diff != "" { t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) } @@ -235,10 +227,7 @@ func TestApplyConfig(t *testing.T) { if _, err := Bind(fs, s); err != nil { t.Fatalf("Bind() error = %v", err) } - got, err := fs.GetStringArray("credential-helper") - if err != nil { - t.Fatalf("fs.GetStringArray() error = %v", err) - } + got := fs.Lookup("credential-helper").Value.(pflag.SliceValue).GetSlice() if diff := cmp.Diff([]string{"helper"}, got); diff != "" { t.Fatalf("credential-helper wrong result (-want +got) diff = %s", diff) } diff --git a/internal/credhelper/credhelper.go b/internal/credhelper/credhelper.go index 5a3d132..b62fe52 100644 --- a/internal/credhelper/credhelper.go +++ b/internal/credhelper/credhelper.go @@ -27,11 +27,9 @@ import ( "os/exec" "strings" "time" - - "github.com/google/shlex" ) -// Helper is a configured credential helper, from "[scope=]command [args...]". +// Helper is a configured credential helper. type Helper struct { // Spec is the configured value, for messages. Spec string @@ -53,27 +51,56 @@ type request struct { URI string `json:"uri"` } -// Parse parses "[scope=]command [args...]". The scope is the text before the -// first '=' when it looks like a host pattern; the command is split like a -// shell would. +// Parse parses "[scope=]path", the form Bazel's --credential_helper takes. +// The scope is the text before the first '=' when it looks like a host +// pattern. func Parse(spec string) (Helper, error) { h := Helper{Spec: spec} - rest := strings.TrimSpace(spec) - if scope, cmd, ok := strings.Cut(rest, "="); ok && isScope(scope) { - if strings.Contains(scope, ":") { - return Helper{}, fmt.Errorf("credential helper %q: scope %q must be a host without a port", spec, scope) + path := strings.TrimSpace(spec) + if scope, rest, ok := strings.Cut(path, "="); ok && isScope(scope) { + if err := checkScope(scope); err != nil { + return Helper{}, fmt.Errorf("credential helper %q: %w", spec, err) } - h.Scope, rest = strings.ToLower(scope), cmd + h.Scope, path = strings.ToLower(scope), strings.TrimSpace(rest) } - args, err := shlex.Split(rest) - if err != nil { - return Helper{}, fmt.Errorf("credential helper %q: %w", spec, err) + if path == "" { + return Helper{}, fmt.Errorf("credential helper %q: no path", spec) + } + h.Command = []string{path} + return h, nil +} + +// Entry is a credential helper from a config file. +type Entry struct { + Scope string `json:"scope"` + Command []string `json:"command"` +} + +// Helper validates e. +func (e Entry) Helper() (Helper, error) { + spec := strings.Join(e.Command, " ") + if e.Scope != "" { + spec = e.Scope + "=" + spec } - if len(args) == 0 { + if len(e.Command) == 0 || e.Command[0] == "" { return Helper{}, fmt.Errorf("credential helper %q: no command", spec) } - h.Command = args - return h, nil + if e.Scope != "" { + if !isScope(e.Scope) { + return Helper{}, fmt.Errorf("credential helper %q: scope %q must be a host or *.domain", spec, e.Scope) + } + if err := checkScope(e.Scope); err != nil { + return Helper{}, fmt.Errorf("credential helper %q: %w", spec, err) + } + } + return Helper{Spec: spec, Scope: strings.ToLower(e.Scope), Command: e.Command}, nil +} + +func checkScope(scope string) error { + if strings.Contains(scope, ":") { + return fmt.Errorf("scope %q must be a host without a port", scope) + } + return nil } func isScope(s string) bool { @@ -84,19 +111,6 @@ func isScope(s string) bool { return host != "" && !strings.Contains(host, "*") } -// ParseAll parses every spec. -func ParseAll(specs []string) ([]Helper, error) { - helpers := make([]Helper, 0, len(specs)) - for _, s := range specs { - h, err := Parse(s) - if err != nil { - return nil, err - } - helpers = append(helpers, h) - } - return helpers, nil -} - // Select returns the helper for rawURL: the one with the most specific // matching scope, an exact host before a wildcard and a longer wildcard // before a shorter one, with unscoped helpers last. Among equally specific diff --git a/internal/credhelper/credhelper_test.go b/internal/credhelper/credhelper_test.go index f4226f5..418a976 100644 --- a/internal/credhelper/credhelper_test.go +++ b/internal/credhelper/credhelper_test.go @@ -32,11 +32,10 @@ func TestParse(t *testing.T) { want Helper wantErr bool }{ - {spec: "a2a-serve credential", want: Helper{Command: []string{"a2a-serve", "credential"}}}, {spec: "/usr/bin/helper", want: Helper{Command: []string{"/usr/bin/helper"}}}, {spec: "example.com=/usr/bin/helper", want: Helper{Scope: "example.com", Command: []string{"/usr/bin/helper"}}}, - {spec: "*.Example.com=helper --flag", want: Helper{Scope: "*.example.com", Command: []string{"helper", "--flag"}}}, - {spec: `helper --opt=a\ b`, want: Helper{Command: []string{"helper", "--opt=a b"}}}, + {spec: "*.Example.com=helper", want: Helper{Scope: "*.example.com", Command: []string{"helper"}}}, + {spec: "/opt/my helper", want: Helper{Command: []string{"/opt/my helper"}}}, {spec: "/opt/x=y/helper", want: Helper{Command: []string{"/opt/x=y/helper"}}}, {spec: "example.com=", wantErr: true}, {spec: "localhost:8080=helper", wantErr: true}, @@ -86,9 +85,13 @@ func TestSelect(t *testing.T) { for _, tc := range tests { t.Run(tc.url, func(t *testing.T) { t.Parallel() - helpers, err := ParseAll(tc.specs) - if err != nil { - t.Fatalf("ParseAll(%q) error = %v", tc.specs, err) + var helpers []Helper + for _, spec := range tc.specs { + h, err := Parse(spec) + if err != nil { + t.Fatalf("Parse(%q) error = %v", spec, err) + } + helpers = append(helpers, h) } got, ok := Select(helpers, tc.url) if ok != tc.wantOK { @@ -101,9 +104,9 @@ func TestSelect(t *testing.T) { } } -// writeHelper returns a helper spec running script with sh. Executing a file +// writeHelper returns a helper running script with sh. Executing a file // written while parallel tests fork can fail with ETXTBSY (golang/go#22315). -func writeHelper(t *testing.T, script string) string { +func writeHelper(t *testing.T, script string) Helper { t.Helper() if runtime.GOOS == "windows" { t.Skip("helper scripts need a POSIX shell") @@ -112,7 +115,7 @@ func writeHelper(t *testing.T, script string) string { if err := os.WriteFile(path, []byte(script), 0o600); err != nil { t.Fatal(err) } - return "sh " + path + return Helper{Spec: path, Command: []string{"sh", path}} } func TestGet(t *testing.T) { @@ -136,10 +139,7 @@ func TestGet(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - h, err := Parse(writeHelper(t, tc.script)) - if err != nil { - t.Fatalf("Parse() error = %v", err) - } + h := writeHelper(t, tc.script) got, err := h.Get(t.Context(), "https://agent.example.com") if tc.wantErr != "" { if err == nil || !strings.Contains(err.Error(), tc.wantErr) { @@ -174,10 +174,7 @@ func TestSourceHeaders(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() runs := filepath.Join(t.TempDir(), "runs") - h, err := Parse(writeHelper(t, `echo run >>`+runs+`; printf '{"headers":{"X":["v"]},"expires":"%s"}' "`+tc.expires+`"`)) - if err != nil { - t.Fatalf("Parse() error = %v", err) - } + h := writeHelper(t, `echo run >>`+runs+`; printf '{"headers":{"X":["v"]},"expires":"%s"}' "`+tc.expires+`"`) src := &Source{Helpers: []Helper{h}} for _, uri := range tc.uris { _, resp, err := src.Headers(t.Context(), uri) @@ -204,35 +201,33 @@ func TestInterceptor(t *testing.T) { helper := writeHelper(t, `uri=$(sed 's/.*"uri":"\([^"]*\)".*/\1/'); printf '{"headers":{"Authorization":["Bearer %s"]}}' "$uri"`) tests := []struct { name string - spec string + scope string baseURL string params a2aclient.ServiceParams want a2aclient.ServiceParams }{ { name: "adds the headers for the interface url", - spec: helper, baseURL: "https://agent.example.com/a2a", params: a2aclient.ServiceParams{}, want: a2aclient.ServiceParams{"authorization": {"Bearer https://agent.example.com/a2a"}}, }, { name: "keeps an explicit service param", - spec: helper, baseURL: "https://agent.example.com", params: a2aclient.ServiceParams{"authorization": {"Bearer flag"}}, want: a2aclient.ServiceParams{"authorization": {"Bearer flag"}}, }, { name: "skips a host outside the scope", - spec: "agent.example.com=" + helper, + scope: "agent.example.com", baseURL: "https://other.example.com", params: a2aclient.ServiceParams{}, want: a2aclient.ServiceParams{}, }, { name: "matches a grpc address without a scheme", - spec: "agent.example.com=" + helper, + scope: "agent.example.com", baseURL: "agent.example.com:443", params: a2aclient.ServiceParams{}, want: a2aclient.ServiceParams{"authorization": {"Bearer https://agent.example.com:443"}}, @@ -241,10 +236,8 @@ func TestInterceptor(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - h, err := Parse(tc.spec) - if err != nil { - t.Fatalf("Parse() error = %v", err) - } + h := helper + h.Scope = tc.scope i := &Interceptor{Source: &Source{Helpers: []Helper{h}}} req := &a2aclient.Request{BaseURL: tc.baseURL, ServiceParams: tc.params} if _, _, err := i.Before(t.Context(), req); err != nil { diff --git a/internal/credhelper/flag.go b/internal/credhelper/flag.go new file mode 100644 index 0000000..a8cfa36 --- /dev/null +++ b/internal/credhelper/flag.go @@ -0,0 +1,121 @@ +// 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 credhelper + +import ( + "bytes" + "encoding/json" + "fmt" + "strings" +) + +// Flag is a repeatable flag of credential helpers. On the command line and +// in environment variables each value is "[scope=]path"; config files list +// [Entry] objects. +type Flag struct { + helpers []Helper +} + +// Helpers returns the configured helpers in order. +func (f *Flag) Helpers() []Helper { + return f.helpers +} + +// String implements [pflag.Value]. +func (f *Flag) String() string { + return strings.Join(f.GetSlice(), ",") +} + +// Set implements [pflag.Value]. +func (f *Flag) Set(spec string) error { + return f.Append(spec) +} + +// Type implements [pflag.Value]. +func (f *Flag) Type() string { + return "helper" +} + +// Append implements [pflag.SliceValue]. +func (f *Flag) Append(spec string) error { + h, err := Parse(spec) + if err != nil { + return err + } + f.helpers = append(f.helpers, h) + return nil +} + +// Replace implements [pflag.SliceValue]. +func (f *Flag) Replace(specs []string) error { + helpers := make([]Helper, 0, len(specs)) + for _, spec := range specs { + h, err := Parse(spec) + if err != nil { + return err + } + helpers = append(helpers, h) + } + f.helpers = helpers + return nil +} + +// GetSlice implements [pflag.SliceValue]. +func (f *Flag) GetSlice() []string { + specs := make([]string, len(f.helpers)) + for i, h := range f.helpers { + specs[i] = h.Spec + } + return specs +} + +// SetConfig sets the helpers from a configuration value: one "[scope=]path" +// string, or a list of [Entry] objects. +func (f *Flag) SetConfig(raw any) error { + if s, ok := raw.(string); ok { + return f.Replace([]string{s}) + } + items, ok := raw.([]any) + if !ok { + return fmt.Errorf("want a list of credential helpers, got %T", raw) + } + helpers := make([]Helper, 0, len(items)) + for _, item := range items { + h, err := parseConfigItem(item) + if err != nil { + return err + } + helpers = append(helpers, h) + } + f.helpers = helpers + return nil +} + +func parseConfigItem(item any) (Helper, error) { + if _, ok := item.(map[string]any); !ok { + return Helper{}, fmt.Errorf("credential helper %v: want {scope, command}", item) + } + data, err := json.Marshal(item) + if err != nil { + return Helper{}, fmt.Errorf("credential helper %v: %w", item, err) + } + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + var e Entry + if err := dec.Decode(&e); err != nil { + return Helper{}, fmt.Errorf("credential helper %s: want {scope, command}: %w", data, err) + } + return e.Helper() +} diff --git a/internal/credhelper/flag_test.go b/internal/credhelper/flag_test.go new file mode 100644 index 0000000..2cb02e2 --- /dev/null +++ b/internal/credhelper/flag_test.go @@ -0,0 +1,70 @@ +// 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 credhelper + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestFlagSetConfig(t *testing.T) { + t.Parallel() + tests := []struct { + name string + raw any + want []Helper + wantErr bool + }{ + { + name: "path string", + raw: "example.com=/usr/bin/helper", + want: []Helper{{Spec: "example.com=/usr/bin/helper", Scope: "example.com", Command: []string{"/usr/bin/helper"}}}, + }, + { + name: "entries", + raw: []any{ + map[string]any{"command": []any{"helper", "--flag"}}, + map[string]any{"scope": "*.Example.com", "command": []any{"corp-helper"}}, + }, + want: []Helper{ + {Spec: "helper --flag", Command: []string{"helper", "--flag"}}, + {Spec: "*.Example.com=corp-helper", Scope: "*.example.com", Command: []string{"corp-helper"}}, + }, + }, + {name: "string entry", raw: []any{"/usr/bin/helper"}, wantErr: true}, + {name: "unknown field", raw: []any{map[string]any{"cmd": []any{"helper"}}}, wantErr: true}, + {name: "no command", raw: []any{map[string]any{"scope": "example.com"}}, wantErr: true}, + {name: "scope with a port", raw: []any{map[string]any{"scope": "example.com:443", "command": []any{"helper"}}}, wantErr: true}, + {name: "scope with a path", raw: []any{map[string]any{"scope": "example.com/a2a", "command": []any{"helper"}}}, wantErr: true}, + {name: "map", raw: map[string]any{"command": []any{"helper"}}, wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var f Flag + err := f.SetConfig(tc.raw) + if (err != nil) != tc.wantErr { + t.Fatalf("Flag.SetConfig(%v) error = %v, want error %v", tc.raw, err, tc.wantErr) + } + if tc.wantErr { + return + } + if diff := cmp.Diff(tc.want, f.Helpers()); diff != "" { + t.Fatalf("Flag.SetConfig(%v) wrong result (-want +got) diff = %s", tc.raw, diff) + } + }) + } +} From 081a781e0450c1ae4306d7b0c4f1f9dc62d3a00c Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Thu, 29 Oct 2026 18:33:20 +0000 Subject: [PATCH 7/8] fix: document ConfigValue.SetConfig for revive --- internal/clicfg/binder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/clicfg/binder.go b/internal/clicfg/binder.go index f6efffc..2ccac31 100644 --- a/internal/clicfg/binder.go +++ b/internal/clicfg/binder.go @@ -159,6 +159,8 @@ func flagValueString(f *pflag.Flag) string { // ConfigValue is a flag value that reads configuration values itself, such // as lists of objects that have no string form. type ConfigValue interface { + // SetConfig sets the value from a raw config value: a string from an + // environment variable or .env, or a decoded YAML or JSON value. SetConfig(raw any) error } From 119325ec6edbbe60efc09e7560bf4bf84b7aab42 Mon Sep 17 00:00:00 2001 From: Yaroslav Shevchuk Date: Fri, 30 Oct 2026 07:15:30 +0000 Subject: [PATCH 8/8] refactor: route every request through --unix-socket instead of scoping it per host --- internal/README.md | 2 +- internal/cli/credentials.go | 13 +--- internal/cli/credentials_test.go | 21 +++--- internal/cli/root.go | 6 +- internal/unixsocket/unixsocket.go | 55 ++-------------- internal/unixsocket/unixsocket_test.go | 91 +++++++++++++------------- 6 files changed, 65 insertions(+), 123 deletions(-) diff --git a/internal/README.md b/internal/README.md index a425b56..92c01b5 100644 --- a/internal/README.md +++ b/internal/README.md @@ -38,7 +38,7 @@ These apply to every client-mode command. Each command selects the agent it talk | `--svc-param ` | | Service parameter (repeatable). The chosen transport defines how it's passed. Split on the first `=`. | | `--auth ` | | Shorthand for `--svc-param "Authorization="`. | | `--credential-helper ` | | Credential helper returning the headers for an agent: `[scope=]path`. Repeatable. Not read from the local `.env`. See [Credentials](#credentials). | -| `--unix-socket ` | | Send HTTP requests through a Unix socket: `` for every host, or `=` for one host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. Not read from the local `.env`. | +| `--unix-socket ` | | Send every HTTP request through the Unix socket at ``, whatever the URL's host. Applies to the card request and the REST and JSON-RPC transports, not gRPC. Not read from the local `.env`. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | | `--verbose` | `-v` | Verbose output to stderr. | diff --git a/internal/cli/credentials.go b/internal/cli/credentials.go index 29e6a7e..6b3e956 100644 --- a/internal/cli/credentials.go +++ b/internal/cli/credentials.go @@ -23,22 +23,15 @@ import ( "github.com/a2aproject/a2a-go/v2/a2aclient" - "github.com/a2aproject/a2a-cli/internal/clierr" "github.com/a2aproject/a2a-cli/internal/credhelper" "github.com/a2aproject/a2a-cli/internal/unixsocket" ) // initHTTPTransport routes HTTP requests through --unix-socket, if set. -func (g *globalConfig) initHTTPTransport() error { - if g.unixSocket == "" { - return nil +func (g *globalConfig) initHTTPTransport() { + if g.unixSocket != "" { + g.httpTransport = unixsocket.Transport(g.unixSocket) } - spec, err := unixsocket.Parse(g.unixSocket) - if err != nil { - return clierr.Usage(err.Error()) - } - g.httpTransport = spec.Transport() - return nil } // initCredentials sets up --credential-helper. Helpers run only when a diff --git a/internal/cli/credentials_test.go b/internal/cli/credentials_test.go index 0dfa268..d672ba0 100644 --- a/internal/cli/credentials_test.go +++ b/internal/cli/credentials_test.go @@ -411,26 +411,21 @@ func TestSendOverUnixSocket(t *testing.T) { t.Cleanup(server.Close) tests := []struct { - name string - socket string - args []string - wantErr bool + name string + args []string }{ - {name: "agent card", socket: sock, args: []string{"-a", base}}, - {name: "endpoint", socket: sock, args: []string{"-e", base, "--transport", "rest"}}, - {name: "scoped to the host", socket: "agent.invalid=" + sock, args: []string{"-a", base}}, - {name: "scoped to the host in another case", socket: "AGENT.invalid=" + sock, args: []string{"-a", base}}, - {name: "scoped to another host", socket: "other.invalid=" + sock, args: []string{"-a", base, "--timeout", "2s"}, wantErr: true}, + {name: "agent card", args: []string{"-a", base}}, + {name: "endpoint", args: []string{"-e", base, "--transport", "rest"}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - args := append(append([]string{"send", "--unix-socket", tc.socket}, tc.args...), "hello") + args := append(append([]string{"send", "--unix-socket", sock}, tc.args...), "hello") out, err := runCMD(t, args...) - if (err != nil) != tc.wantErr { - t.Fatalf("a2a %s error = %v, want error %v", strings.Join(args, " "), err, tc.wantErr) + if err != nil { + t.Fatalf("a2a %s error = %v", strings.Join(args, " "), err) } - if !tc.wantErr && !strings.Contains(out, "over the socket") { + if !strings.Contains(out, "over the socket") { t.Fatalf("a2a %s = %q, want the agent's answer", strings.Join(args, " "), out) } }) diff --git a/internal/cli/root.go b/internal/cli/root.go index f3c466d..beab8c1 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -180,9 +180,7 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { return err } } - if err := cfg.initHTTPTransport(); err != nil { - return err - } + cfg.initHTTPTransport() cfg.initCredentials() switch output.Mode(cfg.output) { @@ -207,7 +205,7 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.DurationVar(&cfg.timeout, "timeout", 30*time.Second, "Request timeout") pf.BoolVarP(&cfg.verbose, "verbose", "v", false, "Verbose output to stderr") pf.BoolVar(&cfg.insecureGRPC, "insecure", false, "Use insecure (plaintext) gRPC transport credentials") - pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send HTTP requests through a Unix socket: for every host, or = for one host") + pf.StringVar(&cfg.unixSocket, "unix-socket", "", "Send every HTTP request through the Unix socket at this path") pf.Var(&cfg.credentialHelpers, "credential-helper", "Credential helper returning the headers for an agent: [scope=]path (repeatable; see a2a auth get --help)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") for _, name := range []string{"unix-socket", "credential-helper"} { diff --git a/internal/unixsocket/unixsocket.go b/internal/unixsocket/unixsocket.go index e8fa0e7..50cde5c 100644 --- a/internal/unixsocket/unixsocket.go +++ b/internal/unixsocket/unixsocket.go @@ -17,61 +17,18 @@ package unixsocket import ( "context" - "fmt" "net" "net/http" - "net/url" - "strings" ) -// Spec is a parsed "" or "=" value. -type Spec struct { - // Host limits the socket to requests for one host; empty for every host. - Host string - Path string -} - -// Parse parses "" or "=". The text before the first '=' -// is a host only when it contains no '/', so paths containing '=' work. -func Parse(spec string) (Spec, error) { - if spec == "" { - return Spec{}, fmt.Errorf("unix socket: empty value") - } - host, path, ok := strings.Cut(spec, "=") - if !ok || host == "" || strings.Contains(host, "/") { - return Spec{Path: spec}, nil - } - if strings.Contains(host, ":") { - return Spec{}, fmt.Errorf("unix socket %q: host %q must not include a port", spec, host) - } - if path == "" { - return Spec{}, fmt.Errorf("unix socket %q: no path", spec) - } - return Spec{Host: strings.ToLower(host), Path: path}, nil -} - -// Matches reports whether requests to host go through the socket. -func (s Spec) Matches(host string) bool { - return s.Host == "" || strings.EqualFold(s.Host, host) -} - -// Transport returns an HTTP transport that dials the socket for matching -// hosts and bypasses the proxy for them. -func (s Spec) Transport() *http.Transport { +// Transport returns an HTTP transport that sends every request through the +// socket at path, bypassing any proxy. +func Transport(path string) *http.Transport { t := http.DefaultTransport.(*http.Transport).Clone() - proxy := t.Proxy - t.Proxy = func(r *http.Request) (*url.URL, error) { - if s.Matches(r.URL.Hostname()) || proxy == nil { - return nil, nil - } - return proxy(r) - } + t.Proxy = nil var d net.Dialer - t.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { - if h, _, err := net.SplitHostPort(addr); err == nil && s.Matches(h) { - return d.DialContext(ctx, "unix", s.Path) - } - return d.DialContext(ctx, network, addr) + t.DialContext = func(ctx context.Context, _, _ string) (net.Conn, error) { + return d.DialContext(ctx, "unix", path) } return t } diff --git a/internal/unixsocket/unixsocket_test.go b/internal/unixsocket/unixsocket_test.go index 15685b0..9e98bdc 100644 --- a/internal/unixsocket/unixsocket_test.go +++ b/internal/unixsocket/unixsocket_test.go @@ -15,60 +15,59 @@ package unixsocket import ( + "io" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" "testing" - - "github.com/google/go-cmp/cmp" ) -func TestParse(t *testing.T) { +func TestTransport(t *testing.T) { t.Parallel() - tests := []struct { - spec string - want Spec - wantErr bool - }{ - {spec: "/tmp/a.sock", want: Spec{Path: "/tmp/a.sock"}}, - {spec: "agent.local=/tmp/a.sock", want: Spec{Host: "agent.local", Path: "/tmp/a.sock"}}, - {spec: "Agent.Local=/tmp/a.sock", want: Spec{Host: "agent.local", Path: "/tmp/a.sock"}}, - {spec: "/tmp/a=b.sock", want: Spec{Path: "/tmp/a=b.sock"}}, - {spec: "a.sock", want: Spec{Path: "a.sock"}}, - {spec: "localhost:8080=/tmp/a.sock", wantErr: true}, - {spec: "agent.local=", wantErr: true}, - {spec: "", wantErr: true}, + // t.TempDir paths can exceed the Unix socket path length limit. + dir, err := os.MkdirTemp("", "a2a") + if err != nil { + t.Fatalf("os.MkdirTemp() error = %v", err) } - for _, tc := range tests { - t.Run(tc.spec, func(t *testing.T) { - t.Parallel() - got, err := Parse(tc.spec) - if (err != nil) != tc.wantErr { - t.Fatalf("Parse(%q) error = %v, want error %v", tc.spec, err, tc.wantErr) - } - if tc.wantErr { - return - } - if diff := cmp.Diff(tc.want, got); diff != "" { - t.Fatalf("Parse(%q) wrong result (-want +got) diff = %s", tc.spec, diff) - } - }) + t.Cleanup(func() { + if err := os.RemoveAll(dir); err != nil { + t.Errorf("os.RemoveAll(%q) error = %v", dir, err) + } + }) + sock := filepath.Join(dir, "a2a.sock") + ln, err := net.Listen("unix", sock) + if err != nil { + t.Fatalf("net.Listen(%q) error = %v", sock, err) } -} + server := &httptest.Server{Listener: ln, Config: &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, err := io.WriteString(w, r.Host); err != nil { + t.Errorf("io.WriteString() error = %v", err) + } + })}} + server.Start() + t.Cleanup(server.Close) -func TestMatches(t *testing.T) { - t.Parallel() - tests := []struct { - spec Spec - host string - want bool - }{ - {spec: Spec{Path: "/s"}, host: "anything.example.com", want: true}, - {spec: Spec{Host: "agent.local", Path: "/s"}, host: "AGENT.local", want: true}, - {spec: Spec{Host: "agent.local", Path: "/s"}, host: "other.local", want: false}, - } - for _, tc := range tests { - t.Run(tc.host, func(t *testing.T) { + client := &http.Client{Transport: Transport(sock)} + for _, host := range []string{"agent.invalid", "other.invalid:8080"} { + t.Run(host, func(t *testing.T) { t.Parallel() - if got := tc.spec.Matches(tc.host); got != tc.want { - t.Fatalf("%+v.Matches(%q) = %v, want %v", tc.spec, tc.host, got, tc.want) + resp, err := client.Get("http://" + host + "/") + if err != nil { + t.Fatalf("client.Get(%q) error = %v", host, err) + } + defer func() { + if err := resp.Body.Close(); err != nil { + t.Errorf("resp.Body.Close() error = %v", err) + } + }() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("io.ReadAll() error = %v", err) + } + if got := string(body); got != host { + t.Fatalf("client.Get(%q) Host = %q, want %q", host, got, host) } }) }