fix(api): escape IDs in request paths, never follow redirects, hide secrets unless --show-secrets

README.md

@@ -232,6 +232,8 @@ UUID) so Plivo can attribute usage per account. These headers never
 carry flag values, argument values, or free-text bodies — those only
 travel in the actual Plivo API request you asked the CLI to make.
 
+To see exactly what the CLI sends, point it at a capture server on this machine with `PLIVO_API_URL=http://127.0.0.1:18555/v1/cli/api` (any port; host localhost, 127.0.0.0/8 or ::1, http or https only, with no user, query or fragment; anything else is refused).
+
 Turn the identity headers off entirely:
 
 ```bash

cmd/action_json_test.go

@@ -16,7 +16,7 @@ func TestActions_printJSONOnStdout(t *testing.T) {
 		"voice calls play UUID --urls https://example.com/a.mp3", "voice calls stop-play UUID",
 		"voice calls speak UUID --text hi", "voice calls stop-speak UUID", "voice calls dtmf UUID --digits 12",
 		"voice calls stop-record UUID", "voice calls streams stop UUID --yes", "voice calls streams stop UUID UUID --yes",
-		"voice conferences hangup room --yes", "voice conferences stop-record room",
+		"voice conferences hangup room --yes", "voice conferences stop-record room --yes",
 		"voice conferences member mute room 11", "voice conferences member unmute room 11",
 		"voice conferences member deaf room 11", "voice conferences member undeaf room 11",
 		"voice conferences member kick room 11 --yes", "voice conferences member stop-play room 11",

cmd/api.go

@@ -72,6 +72,10 @@ the CLI). Use --dry-run to preview without sending. GET and HEAD pass through.
 Here --query adds URL query parameters (key=value). It is not the JMESPath
 --query filter other commands take; -o jsonl, yaml and csv still apply.
 
+Auth tokens and passwords in a response read [REDACTED], as on other
+commands; pass --show-secrets to see them. Agent flows (/AgentFlow paths)
+come back as sent, as agents get prints them: the server masks their secrets.
+
 Examples:
 
   plivo api GET /v1/Account/MA…/            # absolute: the account itself
@@ -238,11 +242,16 @@ func resolveAPIMethodAndPath(args []string, methodFlag string) (string, string,
 // validateAPIPath rejects any path that looks like a full URL. `plivo api`
 // is intentionally limited to api.plivo.com (whatever the resolved BaseURL
 // points at) — letting users pass arbitrary hosts here would be a footgun
-// and a security boundary the rest of the CLI doesn't cross.
+// and a security boundary the rest of the CLI doesn't cross. A control
+// character is refused too: a newline would add a line to the --dry-run,
+// --explain and debug output, such as a forged [dry-run] one.
 func validateAPIPath(path string) error {
 	if strings.Contains(path, "://") {
 		return clierr.BadInput("path must not include a scheme — plivo api only talks to the configured Plivo API base URL")
 	}
+	if strings.ContainsFunc(path, func(r rune) bool { return r < 0x20 || r == 0x7f }) {
+		return clierr.BadInput(fmt.Sprintf("path %q has a control character", path))
+	}
 	return nil
 }
 
@@ -424,16 +433,25 @@ func looksLikeJSON(b []byte) bool {
 }
 
 // renderAPIResponse converts the upstream response into our standard envelope
-// shape — success path emits the upstream body verbatim (in a `data` wrapper
-// for JSON mode), error path emits the unified error envelope with the
-// upstream payload nested under `context.upstream` so agents can branch on it.
+// shape — success path emits the upstream body, credential values hidden
+// unless --show-secrets (in a `data` wrapper for JSON mode), error path emits
+// the unified error envelope with the upstream payload nested under
+// `context.upstream` so agents can branch on it.
 func renderAPIResponse(resp *http.Response, body []byte) error {
 	contentType := resp.Header.Get("Content-Type")
 	isJSON := strings.Contains(strings.ToLower(contentType), "application/json")
 	format := effectiveFormat()
 
-	if resp.StatusCode >= 400 {
-		return apiErrorFromUpstream(resp.StatusCode, resp.Header.Get("X-Request-ID"), body, isJSON)
+	if resp.StatusCode >= 300 { // a redirect too, as none is followed
+		err := apiErrorFromUpstream(resp.StatusCode, resp.Header.Get("X-Request-ID"), body, isJSON)
+		if e, ok := err.(*clierr.Error); ok && resp.StatusCode < 400 {
+			e.Hint = clierr.RedirectHint(resp.Header.Get("Location"), resp.Request.URL.Path)
+		}
+		return err
+	}
+	// An agent flow comes back as sent, as agents get does: see api.RawBodyWithSecrets.
+	if !strings.Contains(resp.Request.URL.Path, "/AgentFlow") {
+		body = api.Visible(body)
 	}
 
 	// Success — pretty-print JSON if upstream sent it; otherwise raw passthrough.

cmd/api_test.go

@@ -73,6 +73,8 @@ func TestValidateAPIPath(t *testing.T) {
 		{"https://api.plivo.com/v1/Account/", true},
 		{"http://evil.example.com/", true},
 		{"ftp://host/path", true},
+		{"/Message/\n[dry-run] GET /Account/", true},
+		{"/Message/\x7f", true},
 	}
 	for _, tc := range cases {
 		t.Run(tc.path, func(t *testing.T) {
@@ -476,6 +478,19 @@ func TestAPICmd_5xxEmitsRetryableEnvelope(t *testing.T) {
 	}
 }
 
+// The client follows no redirect, so a 3xx is the answer, and an error.
+func TestAPICmd_redirectIsAnError(t *testing.T) {
+	setFakeCreds(t)
+	srv, hits := startAPIServer(t, http.StatusTemporaryRedirect, "", "")
+	apiClientForTest = api.New("MAFAKEFORTEST", "fake-token", 0)
+	apiClientForTest.BaseURL = srv.URL
+	t.Cleanup(func() { apiClientForTest = nil })
+	err, _, _ := execCmd(t, "api", "GET", "/Account")
+	if e, ok := err.(*clierr.Error); !ok || e.StatusCode != http.StatusTemporaryRedirect || len(hits()) != 1 || !strings.Contains(e.Hint, "end in a slash") {
+		t.Errorf("got %v after %d requests, want the 307 as an error after 1", err, len(hits()))
+	}
+}
+
 func TestAPICmd_QueryParamsForwarded(t *testing.T) {
 	setFakeCreds(t)
 	srv, hits := startAPIServer(t, 200, "application/json", `{}`)
@@ -767,3 +782,29 @@ func TestAPICmd_sendsClientTypeHeaders(t *testing.T) {
 		t.Error("Client-Version missing on `plivo api`")
 	}
 }
+
+// -o json shows auth tokens and passwords only with --show-secrets, on typed
+// commands, a merged --all list and plivo api alike; subaccounts create still
+// shows the new token, as that is what it is run for, and agent flows come out
+// as sent, as agents update writes them back.
+func TestJSONOutput_hidesSecretsUnlessShowSecrets(t *testing.T) {
+	setFakeCreds(t)
+	srv, _ := startAPIServer(t, 200, "application/json", `{"auth_id":"SA1","auth_token":"tok1","objects":[{"new_auth_token":"tok2"}]}`)
+	pointAPIAtTestServer(t, srv)
+	clientForTest = apiClientForTest
+	t.Cleanup(func() { clientForTest = nil })
+	for _, tc := range []struct {
+		args string
+		show bool
+	}{
+		{"account subaccounts list", false}, {"account subaccounts list --all", false}, {"api GET /Subaccount/", false},
+		{"account subaccounts list --show-secrets", true}, {"account subaccounts create --name x", true},
+		{"agents get d1e2f3a4-0000-4000-8000-000000000001", true}, {"api GET /AgentFlow/d1e2f3a4-0000-4000-8000-000000000001/", true},
+		{"agents list", true}, {"agents runs list d1e2f3a4-0000-4000-8000-000000000001", true}, {"agents nodes list", true},
+	} {
+		err, stdout, _ := execCmd(t, append(strings.Fields(tc.args), "-o", "json")...)
+		if err != nil || strings.Contains(stdout, "tok2") != tc.show || strings.Contains(stdout, "[REDACTED]") == tc.show {
+			t.Errorf("%s: err %v, stdout %s", tc.args, err, stdout)
+		}
+	}
+}

cmd/authToken.go

@@ -8,6 +8,7 @@ package cmd
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"os"
 	"strconv"
 	"strings"
@@ -359,13 +360,16 @@ func runAuthTokenRevoke(cmd *cobra.Command, args []string) error {
 	if id == "" {
 		return clierr.BadInput("token id is required")
 	}
+	if id == "." || id == ".." {
+		return clierr.BadInput(fmt.Sprintf("%q is not a token id", id))
+	}
 
 	c, err := getContactoClient()
 	if err != nil {
 		return err
 	}
 
-	path := authTokenPath + "/" + id
+	path := authTokenPath + "/" + url.PathEscape(id)
 
 	if dryRunFlag {
 		fmt.Fprintf(os.Stderr, "[dry-run] DELETE %s\n", path)

cmd/buddy.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"io"
 	"math/rand"
+	"net"
+	"net/url"
 	"os"
 	"os/signal"
 	"strings"
@@ -113,18 +115,45 @@ func init() {
 //
 //	PLIVO_BUDDY_URL env  >  current `--env <X>` (login only)  >
 //	[buddy].url config  >  built-in prod default
-func applyBuddyURL(c *api.Client) {
-	if u := os.Getenv("PLIVO_BUDDY_URL"); u != "" {
-		c.BuddyBaseURL = u
-		return
+//
+// The requests carry the account's credentials, and login gets a token back,
+// so an override must be https, or http to this machine for a local server.
+func applyBuddyURL(c *api.Client) error {
+	u, source := os.Getenv("PLIVO_BUDDY_URL"), "PLIVO_BUDDY_URL"
+	if u == "" {
+		cfg, err := config.Load()
+		if err != nil {
+			return nil
+		}
+		u, source = cfg.Buddy.EffectiveURL(), "[buddy].url in ~/.plivo/config.toml"
 	}
-	cfg, err := config.Load()
-	if err != nil {
-		return
+	if u == "" {
+		return nil
 	}
-	if u := cfg.Buddy.EffectiveURL(); u != "" {
-		c.BuddyBaseURL = u
+	if !safeBuddyURL(u) { // not echoed: it may hold a password
+		e := clierr.BadInput(source + " must be https, or http to this machine")
+		e.Hint = "Use an https URL, or http://127.0.0.1, http://[::1] or http://localhost for a local server; unset it to use the default."
+		return e
 	}
+	c.BuddyBaseURL = u
+	return nil
+}
+
+// safeBuddyURL reports whether u is https, or http to this machine.
+func safeBuddyURL(u string) bool {
+	p, err := url.Parse(u)
+	if err != nil || p.Host == "" {
+		return false
+	}
+	return p.Scheme == "https" || (p.Scheme == "http" && onThisMachine(p))
+}
+
+// onThisMachine reports whether a URL's host is this machine: localhost,
+// 127.0.0.0/8 or ::1, in any letter case. url.Parse lowers the scheme.
+func onThisMachine(p *url.URL) bool {
+	host := p.Hostname()
+	ip := net.ParseIP(host)
+	return strings.EqualFold(host, "localhost") || (ip != nil && ip.IsLoopback())
 }
 
 func runAsk(cmd *cobra.Command, args []string) error {
@@ -137,7 +166,9 @@ func runAsk(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	applyBuddyURL(client)
+	if err := applyBuddyURL(client); err != nil {
+		return err
+	}
 	url := client.BuddyURL("/v1/aiassist/buddy-ext/chat")
 
 	if askInteractive {
@@ -1065,7 +1096,9 @@ func runSupport(cmd *cobra.Command, args []string) error {
 			Hint:    "Run `plivo login` — a manually-entered profile has no per-user identity to filter by.",
 		}
 	}
-	applyBuddyURL(client)
+	if err := applyBuddyURL(client); err != nil {
+		return err
+	}
 
 	url := client.BuddyURL("/v1/aiassist/buddy-ext/escalations")
 	// Real response shape (from `data_adapters/buddy_escalation.py`):

cmd/buddy_test.go

@@ -405,3 +405,25 @@ func TestAsk_timeoutEndsTheStream(t *testing.T) {
 		t.Errorf("-i stderr = %q, want the turn's timeout", stderr)
 	}
 }
+
+// Requests to the assistant carry the account's credentials, so an override
+// must be https, or http to this machine.
+func TestApplyBuddyURL_refusesPlainHTTPOffThisMachine(t *testing.T) {
+	home := setEmptyHome(t)
+	for u, ok := range map[string]bool{
+		"https://example.com": true, "http://127.0.0.1:8080": true, "http://[::1]:80": true, "http://localhost:3000": true, "HTTP://LocalHost:1": true,
+		"http://example.com": false, "http://127.0.0.1.example.com": false, "ftp://127.0.0.1": false, "https://": false, "localhost:3000": false,
+		"http://localhost.example.com": false, "http://u:SECRET@example.com": false,
+	} {
+		t.Setenv("PLIVO_BUDDY_URL", u)
+		c := &api.Client{BuddyBaseURL: api.DefaultBuddyBase}
+		if err := applyBuddyURL(c); (err == nil) != ok || (c.BuddyBaseURL == u) != ok || (err != nil && strings.Contains(err.Error(), "SECRET")) {
+			t.Errorf("%q: err %v, base %q", u, err, c.BuddyBaseURL)
+		}
+	}
+	t.Setenv("PLIVO_BUDDY_URL", "")
+	writeProfile(t, home, "[buddy]\nurl = \"http://example.com\"\n")
+	if err := applyBuddyURL(&api.Client{}); err == nil || !strings.Contains(err.Error(), "[buddy].url") {
+		t.Errorf("[buddy].url over plain http: got %v", err)
+	}
+}

cmd/command_map.go

@@ -67,7 +67,8 @@ type commandDetail struct {
 
 const (
 	outputFieldsNote = "Paths under data, so --query 'data.<path>'. The fields this CLI maps, a subset of the raw API response " +
-		"that -o json passes through whole: the response can hold more, and a listed field can be missing."
+		"that -o json prints, auth tokens and passwords as [REDACTED] without --show-secrets: the response can hold more, " +
+		"and a listed field can be missing."
 	resultFieldsNote = "Paths under data, so --query 'data.<path>'. The result this CLI builds, so -o json has these fields " +
 		"and no others; a field with no value is null or left out."
 	noOutputFieldsNote = "Not described for this command. Run it with -o json to see its output."

cmd/conference.go

@@ -134,7 +134,7 @@ var confRecordCmd = &cobra.Command{
 
 var confStopRecordCmd = &cobra.Command{
 	Use:   "stop-record <conference_name>",
-	Short: "Stop the active recording on a conference",
+	Short: "Stop the active recording on a conference (requires --yes)",
 	Args:  cobra.ExactArgs(1),
 	RunE:  runConferenceStopRecord,
 }
@@ -325,6 +325,9 @@ func runConferenceRecord(cmd *cobra.Command, args []string) error {
 }
 
 func runConferenceStopRecord(cmd *cobra.Command, args []string) error {
+	if err := confirmDestructive("stop recording conference " + args[0]); err != nil {
+		return err
+	}
 	return doConferenceDelete(args[0], "Record")
 }
 

cmd/diagnose.go

@@ -153,6 +153,9 @@ func requireResourceExists(cmd *cobra.Command, segment, uuid, label string) (jso
 		return nil, err
 	}
 	record, status := readRecord(client, segment, uuid)
+	if err := client.Refused(); err != nil {
+		return nil, err // a blank, . or .. id, which the read did not send
+	}
 	if status == http.StatusNotFound {
 		// A trunk call lives in a different store and a different debugger reads
 		// it, so name the right command instead of reporting a bare not-found.

cmd/diagnose_precheck_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -205,3 +206,15 @@ func TestDiagnose_unknownID_neverFilesATicket(t *testing.T) {
 		})
 	}
 }
+
+// The pre-check read swallows its errors, but not the refusal of a blank, . or
+// .. id: that would hand the assistant a turn about nothing.
+func TestDiagnose_refusesABlankIDBeforeAnyRequest(t *testing.T) {
+	for _, args := range [][]string{{"voice", "calls", "diagnose", " "}, {"messaging", "sms", "diagnose", ".."}} {
+		setFakeCreds(t)
+		_, paths := diagnoseServer(t, http.StatusOK)
+		if err, _, _ := execCmd(t, args...); !strings.Contains(fmt.Sprint(err), "BAD_INPUT") || len(paths()) != 0 {
+			t.Errorf("%q: got %v after %q, want BAD_INPUT before any request", args, err, paths())
+		}
+	}
+}

cmd/internal_registration_test.go

@@ -5,6 +5,8 @@ package cmd
 import (
 	"strings"
 	"testing"
+
+	"github.com/plivo/plivo-cli/internal/config"
 )
 
 // These assertions only hold in the internal build (`-tags internal`), where
@@ -40,3 +42,17 @@ func TestInternal_hodorServerFlagPresent(t *testing.T) {
 		t.Error("internal build: --hodor-server flag should be registered")
 	}
 }
+
+// The revoke id is escaped into the request path, and . or .. is refused.
+func TestInternal_authTokenRevokeEscapesTheID(t *testing.T) {
+	setEmptyHome(t)
+	if err := config.SaveContacto(&config.ContactoProfile{AuthToken: "t", Region: "us", Environment: "dev"}); err != nil {
+		t.Fatal(err)
+	}
+	if err, _, stderr := execCmd(t, "auth", "token", "revoke", "a/b?c", "--dry-run"); err != nil || !strings.Contains(stderr, "DELETE /v1/agent/token/a%2Fb%3Fc\n") {
+		t.Errorf("a/b?c: err %v, stderr %q", err, stderr)
+	}
+	if err, _, _ := execCmd(t, "auth", "token", "revoke", "..", "--dry-run"); err == nil || !strings.Contains(err.Error(), "BAD_INPUT") {
+		t.Errorf("..: got %v, want BAD_INPUT", err)
+	}
+}

cmd/login_browser.go

@@ -83,7 +83,9 @@ func runLoginBrowser(nameExplicit bool) error {
 	// env, config, default. The CLI's --buddy-url override applies here too
 	// since both surfaces share the same edge.
 	client := api.New("", "", 30*time.Second) // creds-less; auth happens in /token
-	applyBuddyURL(client)
+	if err := applyBuddyURL(client); err != nil {
+		return err
+	}
 
 	// Send only the loopback port (see buildAuthorizeURL — dodges the WAF SSRF block).
 	authURL := buildAuthorizeURL(client.BuddyBaseURL, port, state, challenge, deviceHint())

cmd/mpc.go

@@ -5,6 +5,7 @@ import (
 	"net/url"
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/plivo/plivo-cli/internal/api"
 	"github.com/plivo/plivo-cli/internal/clierr"
@@ -333,7 +334,10 @@ func mpcResourceURL(c *api.Client, id string, parts ...string) string {
 	// Plivo accepts both. We use the `uuid_<id>/` form when the id has dashes
 	// (UUID-shaped), otherwise treat it as a friendly_name.
 	seg := "name_" + id
-	if looksLikeUUID(id) {
+	switch {
+	case strings.TrimSpace(id) == "":
+		seg = id // AccountURL refuses it, where name_ would be sent
+	case looksLikeUUID(id):
 		seg = "uuid_" + id
 	}
 	return c.AccountURL(append([]string{"MultiPartyCall", seg}, parts...)...)