From 0f1a6ba122171235ae353c15ec43ac1000441e9e Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Sat, 19 Sep 2026 09:16:42 +0200
Subject: [PATCH 1/5] One shell-quoting implementation, not four
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

internal/richtext.ShellQuote is now the only one. The copies in
internal/auth, internal/config and internal/commands are gone and their
callers moved to it.

auth's copy was byte-identical. commands' copy differed only in spelling — a
compiled regexp where richtext walks runes — and agrees with it on every
byte and every rune up to U+2FFFF, so nothing those callers emit changed.

config's copy did differ: it wrapped every value in single quotes whether or
not it needed them, so a path that can mean nothing to a shell now appears
bare in the untrusted-config warnings. The same shell word, spelled shorter.
A test asserted the old spelling as "shell safety", and that assertion was
weaker than it read: it passed for a wrapper that never spliced an embedded
quote at all. It now asks for the property instead — a path carrying an
apostrophe is spliced, not merely wrapped — and fails against such a
wrapper, which the one it replaces did not.

One call site is left on the old name: connect_run.go, which another change
owns. shellQuote there delegates to richtext.ShellQuote and goes when that
line moves.
---
 internal/auth/agent.go                       |  9 ++-
 internal/auth/auth.go                        | 26 +-----
 internal/commands/config.go                  |  3 +-
 internal/commands/config_test.go             | 85 +++++++++++++-------
 internal/commands/connect.go                 | 12 +--
 internal/commands/connect_doctor.go          |  4 +-
 internal/commands/connect_doctor_mcp_unix.go |  5 +-
 internal/commands/connect_operator.go        | 10 +--
 internal/commands/feed.go                    |  3 +-
 internal/commands/files.go                   | 33 ++------
 internal/commands/files_test.go              | 36 ++++++---
 internal/commands/templates.go               |  3 +-
 internal/config/config.go                    | 39 +++------
 internal/config/trust_test.go                | 54 +++++++++++++
 internal/richtext/shellquote.go              |  8 +-
 internal/richtext/shellquote_unix_test.go    |  7 +-
 16 files changed, 193 insertions(+), 144 deletions(-)

diff --git a/internal/auth/agent.go b/internal/auth/agent.go
index a01b0c301..60f14e171 100644
--- a/internal/auth/agent.go
+++ b/internal/auth/agent.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/basecamp/basecamp-cli/internal/config"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // Agent self-tokens: the RFC 6749 §4.4 client_credentials grant.
@@ -465,7 +466,7 @@ func (m *Manager) accountToBind() string {
 	source := config.Source(m.cfg.Sources["account_id"])
 	supplied := source == config.SourceFlag || source == config.SourceEnv
 	if supplied && m.cfg.AccountID != "" {
-		return shellQuote(m.cfg.AccountID)
+		return richtext.ShellQuote(m.cfg.AccountID)
 	}
 	return "<account-id>"
 }
@@ -511,11 +512,11 @@ func (m *Manager) agentRemedy(err error, clientID, scope string) error {
 func (m *Manager) agentLoginCommand(clientID, scope string) string {
 	id := "<client-id>"
 	if clientID != "" {
-		id = shellQuote(clientID)
+		id = richtext.ShellQuote(clientID)
 	}
 	profile := "<profile>"
 	if m.cfg.ActiveProfile != "" {
-		profile = shellQuote(m.cfg.ActiveProfile)
+		profile = richtext.ShellQuote(m.cfg.ActiveProfile)
 	}
 	command := "... | basecamp auth login --with-client-credentials --client-id " + id + " -P " + profile
 	// A profile with no entry yet is one the login creates, and creating
@@ -538,7 +539,7 @@ func (m *Manager) agentLoginCommand(clientID, scope string) string {
 	// back with full access, or be refused for asking for more than its
 	// client is allowed. full is the default and adding it says nothing.
 	if scope != "" && scope != scopeFull {
-		command += " --scope " + shellQuote(scope)
+		command += " --scope " + richtext.ShellQuote(scope)
 	}
 	return command
 }
diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index 29c5cc58c..c5a7b56aa 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -192,35 +192,11 @@ func (m *Manager) loginRemedy() (command, lead string) {
 		return m.agentLoginCommand(kind.clientID, kind.scope), "Pipe the agent's client secret in:"
 	}
 	if m.cfg.ActiveProfile != "" {
-		return "basecamp auth login -P " + shellQuote(m.cfg.ActiveProfile), "Run:"
+		return "basecamp auth login -P " + richtext.ShellQuote(m.cfg.ActiveProfile), "Run:"
 	}
 	return "basecamp auth login", "Run:"
 }
 
-// shellQuote renders s safe to embed in an emitted shell command: a clearly
-// inert name passes through bare, anything else is single-quoted — the one
-// POSIX form in which nothing substitutes — with embedded single quotes
-// spelled:
-//
-//	'\''
-//
-// indented so gofmt leaves it as written. Profile names come from
-// configuration files, which do not
-// apply the create-time name check.
-func shellQuote(s string) string {
-	if s != "" && strings.IndexFunc(s, shellActive) < 0 {
-		return s
-	}
-	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
-}
-
-// shellActive reports whether r can mean anything to a POSIX shell outside
-// quotes; letters, digits and a few inert punctuation marks cannot.
-func shellActive(r rune) bool {
-	inert := r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || strings.ContainsRune("_./:@%+=-", r)
-	return !inert
-}
-
 // remember records what the credential in play is, for the remedy an
 // error will carry. Every Manager path that gets its hands on a
 // credential — serving a token, renewing one, storing one, answering
diff --git a/internal/commands/config.go b/internal/commands/config.go
index ea5c39324..0f7ede307 100644
--- a/internal/commands/config.go
+++ b/internal/commands/config.go
@@ -16,6 +16,7 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/config"
 	"github.com/basecamp/basecamp-cli/internal/hostutil"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 	"github.com/basecamp/basecamp-cli/internal/tui/resolve"
 )
 
@@ -361,7 +362,7 @@ Valid keys: account_id, project_id (or project), todolist_id, base_url, cache_di
 				absPath, _ := filepath.Abs(configPath)
 				ts := config.LoadTrustStore(config.GlobalConfigDir())
 				if ts == nil || !ts.IsTrusted(configPath) {
-					fmt.Fprintf(os.Stderr, "warning: %q in local config requires trust to take effect; run:\n  basecamp config trust %s\n", key, config.ShellQuote(absPath))
+					fmt.Fprintf(os.Stderr, "warning: %q in local config requires trust to take effect; run:\n  basecamp config trust %s\n", key, richtext.ShellQuote(absPath))
 				}
 			}
 
diff --git a/internal/commands/config_test.go b/internal/commands/config_test.go
index 932bb4365..61bd06243 100644
--- a/internal/commands/config_test.go
+++ b/internal/commands/config_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/config"
 	"github.com/basecamp/basecamp-cli/internal/names"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 func TestAtomicWriteFile_OverwriteExisting(t *testing.T) {
@@ -310,37 +311,61 @@ func TestIsAuthorityKey(t *testing.T) {
 
 // --- config set authority-key warning test ---
 
+// The warning names a `basecamp config trust <path>` a person pastes, so the
+// path in it is shell-quoted.
+//
+// This used to assert single quotes around the path unconditionally, because
+// the copy of the quoting this package called wrapped every value whether or
+// not it needed it. The shared richtext.ShellQuote leaves a path that can
+// mean nothing to a shell alone, so an ordinary path now appears bare — the
+// same word, spelled shorter. What "for shell safety" was actually claiming
+// is the second case: a path that does carry shell syntax is encoded, and an
+// apostrophe in it is spliced rather than merely wrapped, which is the one
+// spelling a wrapper gets wrong.
 func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
-	app, _ := setupConfigTestApp(t)
-
-	// Work in a temp dir so config set writes .basecamp/config.json there
-	tmpDir, _ := filepath.EvalSymlinks(t.TempDir())
-	origDir, _ := os.Getwd()
-	require.NoError(t, os.Chdir(tmpDir))
-	defer os.Chdir(origDir)
-
-	require.NoError(t, os.MkdirAll(".basecamp", 0755))
-
-	// Capture stderr for the warning
-	origStderr := os.Stderr
-	r, w, _ := os.Pipe()
-	os.Stderr = w
-
-	err := executeConfigCommand(app, "set", "base_url", "https://custom.example.com")
-	require.NoError(t, err)
-
-	w.Close()
-	var buf [4096]byte
-	n, _ := r.Read(buf[:])
-	os.Stderr = origStderr
-
-	stderr := string(buf[:n])
-	absPath := filepath.Join(tmpDir, ".basecamp", "config.json")
-
-	assert.Contains(t, stderr, `"base_url"`)
-	assert.Contains(t, stderr, "requires trust")
-	assert.Contains(t, stderr, absPath, "warning must include the exact config path")
-	assert.Contains(t, stderr, "'"+absPath+"'", "path must be single-quoted for shell safety")
+	for name, dir := range map[string]string{
+		"an ordinary path needs no quoting": "plain",
+		"an apostrophe in it is spliced":    "o'brien",
+	} {
+		t.Run(name, func(t *testing.T) {
+			app, _ := setupConfigTestApp(t)
+
+			// Work in a temp dir so config set writes .basecamp/config.json there
+			tmpDir, _ := filepath.EvalSymlinks(t.TempDir())
+			tmpDir = filepath.Join(tmpDir, dir)
+			require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".basecamp"), 0o755))
+			origDir, _ := os.Getwd()
+			require.NoError(t, os.Chdir(tmpDir))
+			defer os.Chdir(origDir)
+
+			// Capture stderr for the warning
+			origStderr := os.Stderr
+			r, w, _ := os.Pipe()
+			os.Stderr = w
+
+			err := executeConfigCommand(app, "set", "base_url", "https://custom.example.com")
+			require.NoError(t, err)
+
+			w.Close()
+			var buf [4096]byte
+			n, _ := r.Read(buf[:])
+			os.Stderr = origStderr
+
+			stderr := string(buf[:n])
+			absPath := filepath.Join(tmpDir, ".basecamp", "config.json")
+
+			assert.Contains(t, stderr, `"base_url"`)
+			assert.Contains(t, stderr, "requires trust")
+			assert.Contains(t, stderr, "basecamp config trust "+richtext.ShellQuote(absPath),
+				"the warning must name the command with the path encoded for a shell")
+			if dir == "plain" {
+				assert.Contains(t, stderr, absPath, "an unquoted path stays readable")
+			} else {
+				assert.NotContains(t, stderr, absPath, "a path with an apostrophe cannot appear raw")
+				assert.Contains(t, stderr, `'\''`, "the apostrophe must be spliced out and back in")
+			}
+		})
+	}
 }
 
 // TestConfigSet_GatedNonAuthorityKeyWarns verifies the trust warning also fires
diff --git a/internal/commands/connect.go b/internal/commands/connect.go
index 8ccf841a3..be759dcf1 100644
--- a/internal/commands/connect.go
+++ b/internal/commands/connect.go
@@ -130,7 +130,7 @@ func runConnectShow(app *appctx.App) error {
 	switch {
 	case errors.Is(err, os.ErrNotExist):
 		return output.ErrNotFoundHint("connect.json for profile", name,
-			"The profile has not been set up. Set it up: basecamp connect setup -P "+shellQuote(name)+" --operator-profile '<your profile>' --serve <project-id>")
+			"The profile has not been set up. Set it up: basecamp connect setup -P "+richtext.ShellQuote(name)+" --operator-profile '<your profile>' --serve <project-id>")
 	case err != nil && runtime.GOOS == "windows":
 		return output.ErrUsageHint("connect.json cannot be used: "+setup.ErrorText(err),
 			"The connector's setup is not supported on Windows: this CLI cannot verify who can change connect.json there.")
@@ -618,7 +618,7 @@ func runConnectSetup(cmd *cobra.Command, app *appctx.App, f *connectSetupFlags)
 // connect.json nobody else may change is usage.
 func classifyWriteError(name string, err error) error {
 	var apiErr *output.Error
-	profile := shellQuote(name)
+	profile := richtext.ShellQuote(name)
 	switch {
 	case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded):
 		// A person who stopped the command, or a deadline: the run ended,
@@ -853,7 +853,7 @@ func parsePositiveID(flag, raw string) (int64, error) {
 // against an Agent's credential or connect.json, and a person's login with
 // nothing pinning it to the bot.
 func refuseCredentialConflicts(name, path, held string, expect int64, exists bool, existing setup.File) error {
-	profile := shellQuote(name)
+	profile := richtext.ShellQuote(name)
 	switch {
 	// What connect.json already says comes first: a remediation that the
 	// existing file would refuse anyway is no remediation.
@@ -891,7 +891,7 @@ func connectAccount(app *appctx.App, name string) (string, error) {
 	p := app.Config.Profiles[name]
 	if p == nil {
 		return "", output.ErrUsageHint(fmt.Sprintf("Profile %q does not exist", name),
-			"Connect the agent first: basecamp auth agent connect -P "+shellQuote(name))
+			"Connect the agent first: basecamp auth agent connect -P "+richtext.ShellQuote(name))
 	}
 	bound, err := canonicalAccount(p.AccountID)
 	if err != nil {
@@ -937,7 +937,7 @@ func unboundProfileError(cfg *config.Config, name string) error {
 	}
 	global := filepath.Join(config.GlobalConfigDir(), "config.json")
 	return output.ErrUsageHint(message,
-		"For an Agent, connecting it binds its own account: basecamp auth agent connect -P "+shellQuote(name)+
+		"For an Agent, connecting it binds its own account: basecamp auth agent connect -P "+richtext.ShellQuote(name)+
 			". A bot user's browser login binds none, so for a bot add account_id to the profile's entry in "+richtext.SanitizeSingleLine(global)+".")
 }
 
@@ -1036,7 +1036,7 @@ func operatorProfileManager(ctx context.Context, app *appctx.App, profile string
 	case err != nil:
 		return nil, err
 	case kind == "":
-		return nil, output.ErrUsageHint(fmt.Sprintf("Operator profile %q holds no credential", profile), "Log in: basecamp auth login -P "+shellQuote(profile))
+		return nil, output.ErrUsageHint(fmt.Sprintf("Operator profile %q holds no credential", profile), "Log in: basecamp auth login -P "+richtext.ShellQuote(profile))
 	case kind == setup.KindAgent:
 		return nil, output.ErrUsage(fmt.Sprintf("Operator profile %q holds an Agent's credential; an operator is a person", profile))
 	}
diff --git a/internal/commands/connect_doctor.go b/internal/commands/connect_doctor.go
index 5fdcda3b5..d5dded65f 100644
--- a/internal/commands/connect_doctor.go
+++ b/internal/commands/connect_doctor.go
@@ -170,7 +170,7 @@ func ledgerChecks(ctx context.Context, p connectProfile) []setup.Check {
 	if s.Hold != nil {
 		checks = append(checks, setup.Check{Name: "Hold", Status: setup.StatusWarn,
 			Message: fmt.Sprintf("Held since %s by %s: nothing dispatches or posts", s.Hold.HeldAt.UTC().Format(time.RFC3339), richtext.SanitizeSingleLine(s.Hold.HeldBy)),
-			Hint:    "Review held records in basecamp connect status, then basecamp connect release -P " + shellQuote(p.name)})
+			Hint:    "Review held records in basecamp connect status, then basecamp connect release -P " + richtext.ShellQuote(p.name)})
 	}
 	if len(s.Indeterminate) > 0 {
 		checks = append(checks, setup.Check{Name: "Lifecycle messages", Status: setup.StatusWarn,
@@ -373,7 +373,7 @@ func driverChecks(p connectProfile) []setup.Check {
 	if p.file.Driver != setup.DriverSpawn && p.file.Driver != setup.DriverACP {
 		checks = append(checks, setup.Check{Name: "Driver", Status: setup.StatusFail,
 			Message: fmt.Sprintf("Driver %q is not %q or %q, and the connector refuses to start on it", p.file.Driver, setup.DriverSpawn, setup.DriverACP),
-			Hint:    "basecamp connect setup -P " + shellQuote(p.name) + " --driver spawn"})
+			Hint:    "basecamp connect setup -P " + richtext.ShellQuote(p.name) + " --driver spawn"})
 	}
 	return checks
 }
diff --git a/internal/commands/connect_doctor_mcp_unix.go b/internal/commands/connect_doctor_mcp_unix.go
index 6659fbc93..b01ecfcaa 100644
--- a/internal/commands/connect_doctor_mcp_unix.go
+++ b/internal/commands/connect_doctor_mcp_unix.go
@@ -14,6 +14,7 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/connector"
 	"github.com/basecamp/basecamp-cli/internal/connector/driver"
 	"github.com/basecamp/basecamp-cli/internal/connector/setup"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 	"github.com/basecamp/basecamp-cli/internal/version"
 )
 
@@ -83,7 +84,7 @@ func mcpHandshakeCheck(ctx context.Context, profile string) setup.Check {
 	if err != nil {
 		session = nil
 		c.Status, c.Message = setup.StatusFail, "The agent's MCP server did not complete the handshake: "+setup.ErrorText(err)
-		c.Hint = "Run basecamp mcp -P " + shellQuote(profile) + " and read its stderr."
+		c.Hint = "Run basecamp mcp -P " + richtext.ShellQuote(profile) + " and read its stderr."
 		return c
 	}
 	tools := 0
@@ -98,6 +99,6 @@ func mcpHandshakeCheck(ctx context.Context, profile string) setup.Check {
 		c.Status, c.Message = setup.StatusFail, "The agent's MCP server lists no tools"
 		return c
 	}
-	c.Status, c.Message = setup.StatusPass, fmt.Sprintf("The agent's MCP server (basecamp mcp -P %s) answered with %d tools; the basecamp_connect domain is served only to a dispatched worker", shellQuote(profile), tools)
+	c.Status, c.Message = setup.StatusPass, fmt.Sprintf("The agent's MCP server (basecamp mcp -P %s) answered with %d tools; the basecamp_connect domain is served only to a dispatched worker", richtext.ShellQuote(profile), tools)
 	return c
 }
diff --git a/internal/commands/connect_operator.go b/internal/commands/connect_operator.go
index f906f3065..3709e2754 100644
--- a/internal/commands/connect_operator.go
+++ b/internal/commands/connect_operator.go
@@ -71,7 +71,7 @@ func loadConnectProfile(cmd *cobra.Command) (connectProfile, error) {
 	file, err := setup.Load(path)
 	switch {
 	case errors.Is(err, os.ErrNotExist):
-		return connectProfile{}, output.ErrUsageHint(fmt.Sprintf("Profile %q is not set up as a connector", name), "Run: basecamp connect setup -P "+shellQuote(name))
+		return connectProfile{}, output.ErrUsageHint(fmt.Sprintf("Profile %q is not set up as a connector", name), "Run: basecamp connect setup -P "+richtext.ShellQuote(name))
 	case err != nil:
 		return connectProfile{}, output.ErrUsage("connect.json cannot be used: " + err.Error())
 	}
@@ -119,7 +119,7 @@ func openConnectLedger(ctx context.Context, p connectProfile, requireStopped boo
 	}
 	path := filepath.Join(dir, connector.LedgerFile)
 	if _, err := os.Lstat(path); errors.Is(err, os.ErrNotExist) {
-		return nil, done, output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Run the connector first: basecamp connect -P "+shellQuote(p.name))
+		return nil, done, output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Run the connector first: basecamp connect -P "+richtext.ShellQuote(p.name))
 	}
 	// Opening for a decision migrates the ledger, and a connector running on
 	// an older binary's schema must not have its triggers replaced under it.
@@ -232,9 +232,9 @@ func runConnectStatus(cmd *cobra.Command, shadow bool) error {
 	ledger, err := connector.OpenLedgerReadOnly(cmd.Context(), filepath.Join(dir, connector.LedgerFile))
 	if errors.Is(err, os.ErrNotExist) {
 		if shadow {
-			return output.ErrUsageHint(fmt.Sprintf("Profile %q has no shadow ledger", p.name), "Run the shadow connector first: basecamp connect -P "+shellQuote(p.name)+" --shadow")
+			return output.ErrUsageHint(fmt.Sprintf("Profile %q has no shadow ledger", p.name), "Run the shadow connector first: basecamp connect -P "+richtext.ShellQuote(p.name)+" --shadow")
 		}
-		return output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Run the connector first: basecamp connect -P "+shellQuote(p.name))
+		return output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Run the connector first: basecamp connect -P "+richtext.ShellQuote(p.name))
 	}
 	if err != nil {
 		return err
@@ -764,7 +764,7 @@ The file is JSON:
 				return err
 			}
 			if _, err := os.Lstat(filepath.Join(dir, connector.LedgerFile)); errors.Is(err, os.ErrNotExist) {
-				return output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Promote the shadow first: basecamp connect shadow promote -P "+shellQuote(p.name))
+				return output.ErrUsageHint(fmt.Sprintf("Profile %q's connector has no ledger yet", p.name), "Promote the shadow first: basecamp connect shadow promote -P "+richtext.ShellQuote(p.name))
 			}
 			// The connector must be stopped: the open takes the instance lock
 			// and holds it for the import.
diff --git a/internal/commands/feed.go b/internal/commands/feed.go
index d03240926..a7c024615 100644
--- a/internal/commands/feed.go
+++ b/internal/commands/feed.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/basecamp/basecamp-cli/internal/appctx"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // The account event feed: the catch-up poll lane (`events poll`), the agent
@@ -549,7 +550,7 @@ func (r feedRequest) resumeNotice(position string, capped bool, maxPages int) st
 // into a command somebody pastes, and nothing about it promises to be a bare
 // word.
 func (r feedRequest) positionCommand(position string) string {
-	return fmt.Sprintf("%s --position %s%s", r.lane.pollCmd, shellQuote(position), r.filters)
+	return fmt.Sprintf("%s --position %s%s", r.lane.pollCmd, richtext.ShellQuote(position), r.filters)
 }
 
 // newEventsPollCmd builds `basecamp events poll`.
diff --git a/internal/commands/files.go b/internal/commands/files.go
index decd18e54..ed272e4af 100644
--- a/internal/commands/files.go
+++ b/internal/commands/files.go
@@ -7,7 +7,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"slices"
 	"strconv"
 	"strings"
@@ -1675,7 +1674,7 @@ You can pass either an upload ID or a Basecamp URL:
 			// its project scope, and an explicit --project carries over —
 			// unlike versions, both follow-up commands resolve a project
 			// before fetching, so a bare ID could prompt or fail headless.
-			ref := shellQuote(args[0])
+			ref := richtext.ShellQuote(args[0])
 			scope := breadcrumbScope(scopeProject(app, *project))
 
 			respOpts := []output.ResponseOption{
@@ -1871,7 +1870,7 @@ You can pass either an upload ID or a Basecamp URL:
 			// Breadcrumbs reuse the caller's own reference and carry an
 			// explicit --project: download resolves a project before fetching,
 			// so a bare ID without the scope could prompt or fail headless.
-			ref := shellQuote(args[0])
+			ref := richtext.ShellQuote(args[0])
 			scope := breadcrumbScope(scopeProject(app, *project))
 
 			return app.OK(upload,
@@ -1900,28 +1899,10 @@ You can pass either an upload ID or a Basecamp URL:
 	return cmd
 }
 
-// shellSafeRe matches strings that need no quoting in an emitted shell
-// command: IDs, plain Basecamp URLs, and simple names. Everything else gets
-// single-quoted.
-var shellSafeRe = regexp.MustCompile(`^[A-Za-z0-9_./:@%+=-]+$`)
-
-// shellQuote renders s safe to embed in an emitted shell command. Clearly
-// inert strings pass through bare; anything else is single-quoted — the one
-// POSIX form in which nothing substitutes — with embedded single quotes
-// spliced out and back in as:
-//
-//	'\''
-//
-// indented so gofmt leaves it as written. This is an
-// encoding applied to every embedded value, not a
-// metacharacter list: breadcrumbs interpolate user- and API-controlled text,
-// and escaping cases one at a time is how quoting bugs recur.
-func shellQuote(s string) string {
-	if s != "" && shellSafeRe.MatchString(s) {
-		return s
-	}
-	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
-}
+// shellQuote is richtext.ShellQuote under the name this package gave its own
+// copy, which this change removed. One call site is left — connect_run.go,
+// which another change owns — and this goes when that one moves.
+func shellQuote(s string) string { return richtext.ShellQuote(s) }
 
 // scopeProject resolves the project value a breadcrumb should carry: the
 // group-level flag, else the root-level --project (app.Flags.Project) — the
@@ -1939,7 +1920,7 @@ func breadcrumbScope(project string) string {
 	if project == "" {
 		return ""
 	}
-	return " --project " + shellQuote(project)
+	return " --project " + richtext.ShellQuote(project)
 }
 
 func newFilesUpdateCmd(project *string) *cobra.Command {
diff --git a/internal/commands/files_test.go b/internal/commands/files_test.go
index c8db0e84c..5227d9b6d 100644
--- a/internal/commands/files_test.go
+++ b/internal/commands/files_test.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/basecamp/basecamp-cli/internal/appctx"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 func TestIsStorageURL(t *testing.T) {
@@ -1493,21 +1494,36 @@ func TestScopeProject(t *testing.T) {
 	assert.Equal(t, "", scopeProject(app, ""))
 }
 
-// TestShellQuote pins the breadcrumb encoding: inert strings (IDs, plain
-// Basecamp URLs) pass bare, and everything else is single-quoted so no shell
-// syntax survives — quoting is an encoding, not a metacharacter list.
-func TestShellQuote(t *testing.T) {
+// TestBreadcrumbsQuoteTheProjectTheyCarry pins the breadcrumb encoding at
+// the caller. This package used to hold its own copy of the quoting and
+// this test called it directly; the copy is richtext.ShellQuote now, so the
+// test asks the breadcrumb itself instead — inert names pass bare, and
+// everything else is single-quoted so no shell syntax survives. Quoting is
+// an encoding, not a metacharacter list.
+func TestBreadcrumbsQuoteTheProjectTheyCarry(t *testing.T) {
+	for input, want := range map[string]string{
+		"789":                " --project 789",
+		"release;id":         ` --project 'release;id'`,
+		"$(command) project": ` --project '$(command) project'`,
+		"My Project":         ` --project 'My Project'`,
+		"O'Brien's":          ` --project 'O'\''Brien'\''s'`,
+		"":                   "",
+	} {
+		assert.Equal(t, want, breadcrumbScope(input), "breadcrumbScope(%q)", input)
+	}
+}
+
+// And the upload reference a breadcrumb echoes back gets the same treatment:
+// a plain ID or URL stays readable, a URL carrying shell syntax does not
+// survive as shell syntax.
+func TestABreadcrumbReferenceIsQuotedLikeTheProject(t *testing.T) {
 	for input, want := range map[string]string{
 		"789": "789",
 		"https://3.basecamp.com/99999/buckets/456/uploads/789":                  "https://3.basecamp.com/99999/buckets/456/uploads/789",
 		"https://3.basecamp.com/99999/buckets/456/uploads/789?x=$(touch pwned)": `'https://3.basecamp.com/99999/buckets/456/uploads/789?x=$(touch pwned)'`,
-		"release;id":         `'release;id'`,
-		"$(command) project": `'$(command) project'`,
-		"My Project":         `'My Project'`,
-		"O'Brien's":          `'O'\''Brien'\''s'`,
-		"":                   `''`,
+		"": `''`,
 	} {
-		assert.Equal(t, want, shellQuote(input), "shellQuote(%q)", input)
+		assert.Equal(t, want, richtext.ShellQuote(input), "richtext.ShellQuote(%q)", input)
 	}
 }
 
diff --git a/internal/commands/templates.go b/internal/commands/templates.go
index a8482b3dc..34da8a7fc 100644
--- a/internal/commands/templates.go
+++ b/internal/commands/templates.go
@@ -14,6 +14,7 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/appctx"
 	"github.com/basecamp/basecamp-cli/internal/dateparse"
 	"github.com/basecamp/basecamp-cli/internal/output"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // NewTemplatesCmd creates the templates command for managing project and to-do list templates.
@@ -295,7 +296,7 @@ func validateTemplateCopyTodoset(cmd *cobra.Command, app *appctx.App, todosetID,
 func templateCommandContextArgs(profile string, persistentAccount bool, accountID string) string {
 	args := ""
 	if profile != "" {
-		args += " --profile " + shellQuote(profile)
+		args += " --profile " + richtext.ShellQuote(profile)
 	}
 	return args + replyAccountArg(persistentAccount, accountID)
 }
diff --git a/internal/config/config.go b/internal/config/config.go
index f62b5dc24..1c64a3ad5 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -8,6 +8,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // Config holds the resolved configuration.
@@ -246,7 +248,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 
 	if v, ok := fileCfg["base_url"].(string); ok && v != "" {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring base_url %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring base_url %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.BaseURL = v
 			cfg.Sources["base_url"] = string(source)
@@ -274,7 +276,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 		// point it at any user-writable path, so gate it like other authority
 		// keys. filepath.Clean normalizes the accepted value.
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring cache_dir %q from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring cache_dir %q from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.CacheDir = filepath.Clean(v)
 			cfg.Sources["cache_dir"] = string(source)
@@ -282,7 +284,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	}
 	if v, ok := fileCfg["cache_enabled"].(bool); ok {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring cache_enabled from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring cache_enabled from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.CacheEnabled = v
 			cfg.Sources["cache_enabled"] = string(source)
@@ -315,7 +317,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	}
 	if v, ok := fileCfg["llm_provider"].(string); ok && v != "" {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring llm_provider %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring llm_provider %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.LLMProvider = v
 			cfg.Sources["llm_provider"] = string(source)
@@ -325,7 +327,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 		// Gate like other LLM authority keys: an untrusted config could
 		// silently substitute a costlier paid model.
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring llm_model %q from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring llm_model %q from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.LLMModel = v
 			cfg.Sources["llm_model"] = string(source)
@@ -342,7 +344,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	}
 	if v, ok := fileCfg["llm_endpoint"].(string); ok && v != "" {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring llm_endpoint %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring llm_endpoint %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			// Keep the value even if malformed (non-http(s)/hostless):
 			// summarize.ValidateEndpoint rejects it at the point of
@@ -361,7 +363,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 			// Gate like other LLM authority keys: block a malicious repo from
 			// inflating paid-LLM concurrency (cost amplification).
 			if untrusted {
-				fmt.Fprintf(os.Stderr, "warning: ignoring llm_max_concurrent from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+				fmt.Fprintf(os.Stderr, "warning: ignoring llm_max_concurrent from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, richtext.ShellQuote(path))
 			} else if iv >= 1 && iv <= 10 && fv == float64(iv) {
 				cfg.LLMMaxConcurrent = iv
 				cfg.Sources["llm_max_concurrent"] = string(source)
@@ -373,7 +375,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 			iv := int(fv)
 			// Gate like other LLM authority keys (cost amplification).
 			if untrusted {
-				fmt.Fprintf(os.Stderr, "warning: ignoring llm_token_budget from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+				fmt.Fprintf(os.Stderr, "warning: ignoring llm_token_budget from %s config at %s\n  (trust-gated key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, richtext.ShellQuote(path))
 			} else if iv >= 100 && iv <= 100000 && fv == float64(iv) {
 				cfg.LLMTokenBudget = iv
 				cfg.Sources["llm_token_budget"] = string(source)
@@ -393,7 +395,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	}
 	if v, ok := fileCfg["default_profile"].(string); ok && v != "" {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring default_profile %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring default_profile %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, richtext.ShellQuote(path))
 		} else {
 			cfg.DefaultProfile = v
 			cfg.Sources["default_profile"] = string(source)
@@ -401,7 +403,7 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	}
 	if v, ok := fileCfg["profiles"].(map[string]any); ok {
 		if untrusted {
-			fmt.Fprintf(os.Stderr, "warning: ignoring profiles from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+			fmt.Fprintf(os.Stderr, "warning: ignoring profiles from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, richtext.ShellQuote(path))
 		} else {
 			for name, profileData := range v {
 				if profileMap, ok := profileData.(map[string]any); ok {
@@ -894,20 +896,3 @@ func IsHTTPURL(rawURL string) bool {
 	}
 	return (u.Scheme == "http" || u.Scheme == "https") && u.Hostname() != ""
 }
-
-// ShellQuote returns a POSIX single-quoted string safe for copy-paste into
-// a shell. A single quote cannot be escaped inside single quotes, so each
-// one in the value is spliced out and back in as:
-//
-//	'\''
-//
-// that is: quote, backslash, quote, quote. It is written as an indented
-// block on purpose — gofmt reformats doc-comment prose and rewrites that
-// sequence into a curly closing quote, so a caller copying it out of prose
-// would get a form no shell reads (Copilot on #765).
-//
-// internal/richtext.ShellQuote is the shared one new code should use; this
-// copy predates it, as do the ones in internal/commands and internal/auth.
-func ShellQuote(s string) string {
-	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
-}
diff --git a/internal/config/trust_test.go b/internal/config/trust_test.go
index ed302ba42..e0af15e19 100644
--- a/internal/config/trust_test.go
+++ b/internal/config/trust_test.go
@@ -2,12 +2,16 @@ package config
 
 import (
 	"encoding/json"
+	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 func TestTrustStore_EmptyByDefault(t *testing.T) {
@@ -234,3 +238,53 @@ func TestLoadTrustStore_EmptyDir(t *testing.T) {
 	ts := LoadTrustStore("")
 	assert.Nil(t, ts)
 }
+
+// The untrusted-config warning names a command a person pastes, so the path
+// in it is shell-quoted. This package used to quote it with a copy of its
+// own that wrapped every value in single quotes unconditionally; it now
+// calls richtext.ShellQuote, which leaves a path that needs no quoting
+// bare. The same shell word either way — these pin which spelling, so the
+// change to the message is a decision and not a drift.
+func TestTheTrustWarningQuotesThePathOnlyWhenItHasTo(t *testing.T) {
+	for name, dirName := range map[string]string{
+		"a plain path goes bare":         "plain",
+		"an apostrophe in it is spliced": "o'brien",
+	} {
+		t.Run(name, func(t *testing.T) {
+			dir := filepath.Join(t.TempDir(), dirName)
+			require.NoError(t, os.MkdirAll(dir, 0o755))
+			configPath := filepath.Join(dir, "config.json")
+			require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
+
+			want := "basecamp config trust " + richtext.ShellQuote(configPath) + "`"
+			if dirName == "plain" {
+				require.NotContains(t, want, "'", "a plain path must come through unquoted")
+			} else {
+				require.Contains(t, want, `'\''`, "an apostrophe must be spliced, not wrapped")
+			}
+
+			assert.Contains(t, captureStderr(t, func() {
+				loadFromFile(Default(), configPath, SourceLocal, nil)
+			}), want)
+		})
+	}
+}
+
+// captureStderr runs fn with os.Stderr redirected and returns what it wrote.
+func captureStderr(t *testing.T, fn func()) string {
+	t.Helper()
+	orig := os.Stderr
+	r, w, err := os.Pipe()
+	require.NoError(t, err)
+	os.Stderr = w
+	done := make(chan string, 1)
+	go func() {
+		var b strings.Builder
+		_, _ = io.Copy(&b, r)
+		done <- b.String()
+	}()
+	fn()
+	require.NoError(t, w.Close())
+	os.Stderr = orig
+	return <-done
+}
diff --git a/internal/richtext/shellquote.go b/internal/richtext/shellquote.go
index 8002b08d7..e191eedf1 100644
--- a/internal/richtext/shellquote.go
+++ b/internal/richtext/shellquote.go
@@ -21,9 +21,11 @@ import "strings"
 // which is held to whatever check applied when the value was first created.
 // Escaping cases one at a time is how quoting bugs recur.
 //
-// internal/commands and internal/auth each carry a copy of this, written
-// before there was a shared home for it. This is the one new code should
-// use; converging those two is not this change's to do.
+// internal/auth, internal/config and internal/commands each carried a copy
+// of this, written before there was a shared home for it. They are gone:
+// this is the only one, and the only one new code should use. The config
+// copy quoted unconditionally, so paths that need no quoting now appear
+// bare in its warnings — the same shell word, spelled shorter.
 func ShellQuote(s string) string {
 	if s != "" && strings.IndexFunc(s, shellActive) < 0 {
 		return s
diff --git a/internal/richtext/shellquote_unix_test.go b/internal/richtext/shellquote_unix_test.go
index 467e2affe..aad1c5656 100644
--- a/internal/richtext/shellquote_unix_test.go
+++ b/internal/richtext/shellquote_unix_test.go
@@ -22,7 +22,12 @@ import (
 func TestShellQuoteSurvivesARealShell(t *testing.T) {
 	for _, in := range []string{
 		"agent", "", "two words", "a;rm -rf /", "$(echo pwned)", "`echo pwned`",
-		"it's", "'", "'; echo pwned; '", "a\nb", `back\slash`, "*", "~root",
+		"it's", "'", "'; echo pwned; '", "a\nb", "a\tb", `back\slash`, "*", "~root",
+		// The shapes the three deleted copies carried: a profile name
+		// (auth), a config path (config), and an opaque feed position
+		// (commands). An apostrophe in a home directory is the case that
+		// tells quoting apart from wrapping, and it is not hypothetical.
+		"work profile", "/home/o'brien/.basecamp/config.json", "pos 1; rm -rf /",
 	} {
 		out, err := exec.CommandContext(t.Context(), "/bin/sh", "-c", "printf %s "+ShellQuote(in)).Output()
 		require.NoError(t, err, "input %q quoted as %s", in, ShellQuote(in))

From df536eb0948a866d6f58e026aac5f35526a54dd8 Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Sat, 19 Sep 2026 09:32:54 +0200
Subject: [PATCH 2/5] Gate the bare-path assertion on POSIX, where the property
 exists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

"An inert path comes through unquoted" is a statement about POSIX paths. A
Windows path carries backslashes, which mean something to a shell, so
richtext.ShellQuote rightly quotes it and there is no bare path to assert —
the new trust-warning test would have failed on Windows, which this
repository builds for.

The Go suite runs on ubuntu-latest only, so nothing would have caught it:
the assertion would have stayed green here while being wrong for anyone
running the tests on Windows.

Split rather than deleted. The apostrophe case is the portable half and
stays in trust_test.go — no escape reaches inside single quotes on any
platform, so the splice has to happen everywhere. The bare-path case moves
to trust_unix_test.go behind //go:build unix, alongside the real-shell tests
in internal/richtext that are gated for the same reason. The commands-side
warning test asks richtext.ShellQuote for the expected spelling rather than
writing quotes out, so it means the same thing on both.

Copilot on #769.
---
 internal/commands/config_test.go   | 23 +++++++-----
 internal/config/trust_test.go      | 56 ++++++++++++++----------------
 internal/config/trust_unix_test.go | 42 ++++++++++++++++++++++
 3 files changed, 83 insertions(+), 38 deletions(-)
 create mode 100644 internal/config/trust_unix_test.go

diff --git a/internal/commands/config_test.go b/internal/commands/config_test.go
index 61bd06243..ca4a3e1b4 100644
--- a/internal/commands/config_test.go
+++ b/internal/commands/config_test.go
@@ -316,16 +316,21 @@ func TestIsAuthorityKey(t *testing.T) {
 //
 // This used to assert single quotes around the path unconditionally, because
 // the copy of the quoting this package called wrapped every value whether or
-// not it needed it. The shared richtext.ShellQuote leaves a path that can
-// mean nothing to a shell alone, so an ordinary path now appears bare — the
-// same word, spelled shorter. What "for shell safety" was actually claiming
-// is the second case: a path that does carry shell syntax is encoded, and an
-// apostrophe in it is spliced rather than merely wrapped, which is the one
-// spelling a wrapper gets wrong.
+// not it needed it. The shared richtext.ShellQuote leaves a value that can
+// mean nothing to a shell alone, so on a POSIX filesystem an ordinary path
+// now appears bare — the same word, spelled shorter. What "for shell safety"
+// was actually claiming is the second case: a path that does carry shell
+// syntax is encoded, and an apostrophe in it is spliced rather than merely
+// wrapped, which is the one spelling a wrapper gets wrong.
+//
+// Both cases are asked of richtext.ShellQuote rather than spelled out, so
+// they mean the same thing on a platform whose paths are not built from
+// inert characters. Whether an ordinary path comes out bare is a POSIX
+// question and is pinned in internal/config/trust_unix_test.go.
 func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
 	for name, dir := range map[string]string{
-		"an ordinary path needs no quoting": "plain",
-		"an apostrophe in it is spliced":    "o'brien",
+		"an ordinary path":             "plain",
+		"a path with an apostrophe in": "o'brien",
 	} {
 		t.Run(name, func(t *testing.T) {
 			app, _ := setupConfigTestApp(t)
@@ -359,7 +364,7 @@ func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
 			assert.Contains(t, stderr, "basecamp config trust "+richtext.ShellQuote(absPath),
 				"the warning must name the command with the path encoded for a shell")
 			if dir == "plain" {
-				assert.Contains(t, stderr, absPath, "an unquoted path stays readable")
+				assert.Contains(t, stderr, absPath, "the path itself appears in the command a person pastes")
 			} else {
 				assert.NotContains(t, stderr, absPath, "a path with an apostrophe cannot appear raw")
 				assert.Contains(t, stderr, `'\''`, "the apostrophe must be spliced out and back in")
diff --git a/internal/config/trust_test.go b/internal/config/trust_test.go
index e0af15e19..25dbd9a6d 100644
--- a/internal/config/trust_test.go
+++ b/internal/config/trust_test.go
@@ -239,35 +239,33 @@ func TestLoadTrustStore_EmptyDir(t *testing.T) {
 	assert.Nil(t, ts)
 }
 
-// The untrusted-config warning names a command a person pastes, so the path
-// in it is shell-quoted. This package used to quote it with a copy of its
-// own that wrapped every value in single quotes unconditionally; it now
-// calls richtext.ShellQuote, which leaves a path that needs no quoting
-// bare. The same shell word either way — these pin which spelling, so the
-// change to the message is a decision and not a drift.
-func TestTheTrustWarningQuotesThePathOnlyWhenItHasTo(t *testing.T) {
-	for name, dirName := range map[string]string{
-		"a plain path goes bare":         "plain",
-		"an apostrophe in it is spliced": "o'brien",
-	} {
-		t.Run(name, func(t *testing.T) {
-			dir := filepath.Join(t.TempDir(), dirName)
-			require.NoError(t, os.MkdirAll(dir, 0o755))
-			configPath := filepath.Join(dir, "config.json")
-			require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
-
-			want := "basecamp config trust " + richtext.ShellQuote(configPath) + "`"
-			if dirName == "plain" {
-				require.NotContains(t, want, "'", "a plain path must come through unquoted")
-			} else {
-				require.Contains(t, want, `'\''`, "an apostrophe must be spliced, not wrapped")
-			}
-
-			assert.Contains(t, captureStderr(t, func() {
-				loadFromFile(Default(), configPath, SourceLocal, nil)
-			}), want)
-		})
-	}
+// The untrusted-config warning names a `basecamp config trust <path>` a
+// person pastes, so the path in it is shell-quoted. An apostrophe in the
+// path is the case that tells quoting apart from wrapping — it has to be
+// spliced out of the quotes and back in, since no escape reaches inside
+// single quotes — and that holds wherever this builds.
+//
+// The other half of the change lives in trust_unix_test.go: this package's
+// deleted copy wrapped every value whether or not it needed it, and the
+// shared richtext.ShellQuote leaves an inert one alone. That is a statement
+// about POSIX paths only. A Windows path carries backslashes, which mean
+// something to a shell, so there is no bare path to assert there — and the
+// Go suite runs on Linux, so an ungated assertion about it would have gone
+// on passing while being wrong for everyone who runs the tests on Windows
+// (Copilot on #769).
+func TestTheTrustWarningSplicesAnApostropheInThePath(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "o'brien")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	configPath := filepath.Join(dir, "config.json")
+	require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
+
+	want := "basecamp config trust " + richtext.ShellQuote(configPath) + "`"
+	require.Contains(t, want, `'\''`, "an apostrophe must be spliced, not wrapped")
+	require.NotContains(t, want, "/o'brien/", "and the raw apostrophe must not survive")
+
+	assert.Contains(t, captureStderr(t, func() {
+		loadFromFile(Default(), configPath, SourceLocal, nil)
+	}), want)
 }
 
 // captureStderr runs fn with os.Stderr redirected and returns what it wrote.
diff --git a/internal/config/trust_unix_test.go b/internal/config/trust_unix_test.go
new file mode 100644
index 000000000..ae3d09c2d
--- /dev/null
+++ b/internal/config/trust_unix_test.go
@@ -0,0 +1,42 @@
+//go:build unix
+
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/richtext"
+)
+
+// A POSIX path is built from characters that can mean nothing to a shell, so
+// the trust warning names it bare. This package used to quote it anyway —
+// its own copy of the quoting wrapped every value unconditionally — and the
+// move to richtext.ShellQuote is what changed the message. Pinned here so
+// the change is a decision and not a drift.
+//
+// Unix-gated because the property is about POSIX paths: a Windows path
+// carries backslashes, which are shell-active, so `richtext.ShellQuote`
+// rightly quotes it there and there is nothing bare to assert. The
+// apostrophe half of the same behavior is portable and lives next door in
+// trust_test.go.
+func TestTheTrustWarningLeavesAPathThatNeedsNoQuotingBare(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "plain")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	configPath := filepath.Join(dir, "config.json")
+	require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
+	require.NotContains(t, configPath, "'", "the temp path itself must need no quoting")
+
+	require.Equal(t, configPath, richtext.ShellQuote(configPath), "an inert path must come through unchanged")
+
+	want := "basecamp config trust " + configPath + "`"
+	require.NotContains(t, want, "'", "and so the warning names it unquoted")
+
+	assert.Contains(t, captureStderr(t, func() {
+		loadFromFile(Default(), configPath, SourceLocal, nil)
+	}), want)
+}

From 25a99fbf84ee150212575a2fbcca262617d41b3a Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Sat, 19 Sep 2026 09:48:17 +0200
Subject: [PATCH 3/5] The quoting fixtures stop being about the machine
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

t.TempDir() is rooted under TMPDIR, and a valid POSIX TMPDIR may hold
apostrophes, spaces or anything else; macOS resolves /var through a symlink,
CI sandboxes use paths nobody would guess, and Windows spells the whole thing
differently. A shell-quoting test rooted in it asserts something about the
host rather than about the quoting — and the bare-path one would have failed
outright on a host whose temporary directory carries a quote.

Both config fixtures are now a relative path under a directory the test
changes into, so the expectation is an exact string that means the same
thing on every machine: `basecamp config trust repo/.basecamp/config.json`
and `basecamp config trust 'o'\''brien/config.json'`. Verified by running
them with TMPDIR set to a directory whose name carries an apostrophe, a
space, a non-ASCII character and a command substitution; the shape they
replace fails there.

The commands-side warning is about an absolute path and cannot be made
relative, so it drops the raw-substring assertion on the ordinary fixture —
which was the same dependency one step milder — and keeps comparing against
richtext.ShellQuote. The splice checks stay on the fixture that deliberately
carries an apostrophe.

Copilot on #769.
---
 internal/commands/config_test.go   | 21 ++++++++++++------
 internal/config/trust_test.go      | 22 +++++++++++--------
 internal/config/trust_unix_test.go | 34 +++++++++++++++++-------------
 3 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/internal/commands/config_test.go b/internal/commands/config_test.go
index ca4a3e1b4..dd800efdb 100644
--- a/internal/commands/config_test.go
+++ b/internal/commands/config_test.go
@@ -323,10 +323,14 @@ func TestIsAuthorityKey(t *testing.T) {
 // syntax is encoded, and an apostrophe in it is spliced rather than merely
 // wrapped, which is the one spelling a wrapper gets wrong.
 //
-// Both cases are asked of richtext.ShellQuote rather than spelled out, so
-// they mean the same thing on a platform whose paths are not built from
-// inert characters. Whether an ordinary path comes out bare is a POSIX
-// question and is pinned in internal/config/trust_unix_test.go.
+// The expectation is asked of richtext.ShellQuote rather than spelled out,
+// because this path is absolute and so is rooted in whatever the host calls
+// its temporary directory — a spelling that may hold apostrophes or spaces
+// of its own, and that is different again on Windows. Comparing against the
+// encoder means the same thing on every machine. Whether an ordinary path
+// comes out bare cannot be asked of an absolute path for that reason; it is
+// pinned against a fixed relative fixture in
+// internal/config/trust_unix_test.go.
 func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
 	for name, dir := range map[string]string{
 		"an ordinary path":             "plain",
@@ -363,9 +367,12 @@ func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
 			assert.Contains(t, stderr, "requires trust")
 			assert.Contains(t, stderr, "basecamp config trust "+richtext.ShellQuote(absPath),
 				"the warning must name the command with the path encoded for a shell")
-			if dir == "plain" {
-				assert.Contains(t, stderr, absPath, "the path itself appears in the command a person pastes")
-			} else {
+			// Only the apostrophe fixture gets extra assertions. The
+			// ordinary one is covered by the exact ShellQuote comparison
+			// above; asking it for a raw substring would be asking about
+			// TMPDIR, which may hold an apostrophe of its own and would
+			// then correctly be spliced (Copilot on #769).
+			if dir != "plain" {
 				assert.NotContains(t, stderr, absPath, "a path with an apostrophe cannot appear raw")
 				assert.Contains(t, stderr, `'\''`, "the apostrophe must be spliced out and back in")
 			}
diff --git a/internal/config/trust_test.go b/internal/config/trust_test.go
index 25dbd9a6d..41a74076e 100644
--- a/internal/config/trust_test.go
+++ b/internal/config/trust_test.go
@@ -245,27 +245,31 @@ func TestLoadTrustStore_EmptyDir(t *testing.T) {
 // spliced out of the quotes and back in, since no escape reaches inside
 // single quotes — and that holds wherever this builds.
 //
+// The fixture is a relative path under a directory this test changes into,
+// so the expectation is an exact string rather than a substring of whatever
+// the host spells its temporary directory as. TMPDIR may itself hold
+// apostrophes or spaces, and a quoting test rooted in it would be asserting
+// something about the machine (Copilot on #769).
+//
 // The other half of the change lives in trust_unix_test.go: this package's
 // deleted copy wrapped every value whether or not it needed it, and the
 // shared richtext.ShellQuote leaves an inert one alone. That is a statement
 // about POSIX paths only. A Windows path carries backslashes, which mean
 // something to a shell, so there is no bare path to assert there — and the
 // Go suite runs on Linux, so an ungated assertion about it would have gone
-// on passing while being wrong for everyone who runs the tests on Windows
-// (Copilot on #769).
+// on passing while being wrong for everyone who runs the tests on Windows.
 func TestTheTrustWarningSplicesAnApostropheInThePath(t *testing.T) {
-	dir := filepath.Join(t.TempDir(), "o'brien")
-	require.NoError(t, os.MkdirAll(dir, 0o755))
-	configPath := filepath.Join(dir, "config.json")
+	t.Chdir(t.TempDir())
+	const configPath = "o'brien/config.json"
+	require.NoError(t, os.MkdirAll("o'brien", 0o755))
 	require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
 
-	want := "basecamp config trust " + richtext.ShellQuote(configPath) + "`"
-	require.Contains(t, want, `'\''`, "an apostrophe must be spliced, not wrapped")
-	require.NotContains(t, want, "/o'brien/", "and the raw apostrophe must not survive")
+	// quote, backslash, quote, quote — the splice, not a wrapper.
+	require.Equal(t, `'o'\''brien/config.json'`, richtext.ShellQuote(configPath))
 
 	assert.Contains(t, captureStderr(t, func() {
 		loadFromFile(Default(), configPath, SourceLocal, nil)
-	}), want)
+	}), "basecamp config trust "+`'o'\''brien/config.json'`+"`")
 }
 
 // captureStderr runs fn with os.Stderr redirected and returns what it wrote.
diff --git a/internal/config/trust_unix_test.go b/internal/config/trust_unix_test.go
index ae3d09c2d..81d1b4138 100644
--- a/internal/config/trust_unix_test.go
+++ b/internal/config/trust_unix_test.go
@@ -4,7 +4,6 @@ package config
 
 import (
 	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -13,30 +12,35 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
-// A POSIX path is built from characters that can mean nothing to a shell, so
-// the trust warning names it bare. This package used to quote it anyway —
-// its own copy of the quoting wrapped every value unconditionally — and the
-// move to richtext.ShellQuote is what changed the message. Pinned here so
-// the change is a decision and not a drift.
+// A path built from characters that can mean nothing to a shell is named
+// bare in the trust warning. This package used to quote it anyway — its own
+// copy of the quoting wrapped every value unconditionally — and the move to
+// richtext.ShellQuote is what changed the message. Pinned here so the change
+// is a decision and not a drift.
+//
+// The fixture is a relative path under a directory this test changes into,
+// not a t.TempDir() path, because TMPDIR is the machine talking: a valid
+// POSIX one may hold spaces, apostrophes or anything else, and on macOS /var
+// resolves through a symlink. A quoting test rooted in it asserts something
+// about the host rather than about the quoting, and would fail on a host
+// that spells its temporary directory differently (Copilot on #769). The
+// expectation below is therefore an exact string, the same on every machine
+// this builds for.
 //
 // Unix-gated because the property is about POSIX paths: a Windows path
-// carries backslashes, which are shell-active, so `richtext.ShellQuote`
+// carries backslashes, which are shell-active, so richtext.ShellQuote
 // rightly quotes it there and there is nothing bare to assert. The
 // apostrophe half of the same behavior is portable and lives next door in
 // trust_test.go.
 func TestTheTrustWarningLeavesAPathThatNeedsNoQuotingBare(t *testing.T) {
-	dir := filepath.Join(t.TempDir(), "plain")
-	require.NoError(t, os.MkdirAll(dir, 0o755))
-	configPath := filepath.Join(dir, "config.json")
+	t.Chdir(t.TempDir())
+	const configPath = "repo/.basecamp/config.json"
+	require.NoError(t, os.MkdirAll("repo/.basecamp", 0o755))
 	require.NoError(t, os.WriteFile(configPath, []byte(`{"base_url": "https://evil.example.com"}`), 0o644))
-	require.NotContains(t, configPath, "'", "the temp path itself must need no quoting")
 
 	require.Equal(t, configPath, richtext.ShellQuote(configPath), "an inert path must come through unchanged")
 
-	want := "basecamp config trust " + configPath + "`"
-	require.NotContains(t, want, "'", "and so the warning names it unquoted")
-
 	assert.Contains(t, captureStderr(t, func() {
 		loadFromFile(Default(), configPath, SourceLocal, nil)
-	}), want)
+	}), "basecamp config trust repo/.basecamp/config.json`")
 }

From 741c1af8783342ce929bd4f1c54b2cb829b009cd Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Sat, 19 Sep 2026 10:07:38 +0200
Subject: [PATCH 4/5] The fifth copy, in the connector's recovery harness
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A one-line hand-rolled implementation in a test file, and the naive
spelling: it wrapped unconditionally with no inert check. It builds the
#!/bin/sh shim a fake agent is launched through, so it is generating a
script a real shell runs rather than asserting against a fixed expected
string — there was no reason for it to spell its own.

No cycle to work around: internal/connector already imports internal/
richtext, and richtext has no internal dependencies at all. The harness
tests execute that shim through /bin/sh, which is what confirms the move.

richtext.ShellQuote is now the only implementation in the repository. The
one remaining use of the old `shellQuote` name, in internal/commands, is a
forwarder to it and says so at its definition.
---
 internal/connector/recovery_harness_test.go |  5 ++---
 internal/richtext/shellquote.go             | 11 ++++++-----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/internal/connector/recovery_harness_test.go b/internal/connector/recovery_harness_test.go
index 11141d1a3..381cc2ce8 100644
--- a/internal/connector/recovery_harness_test.go
+++ b/internal/connector/recovery_harness_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/connector/driver/drivertest"
 	"github.com/basecamp/basecamp-cli/internal/connector/driver/spawn"
 	"github.com/basecamp/basecamp-cli/internal/connector/setup"
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // The integrated recovery harness (plan step 22).
@@ -361,7 +362,7 @@ func newHarness(t *testing.T, d harnessDriver, sc harnessScenario) *harness {
 	require.NoError(t, exeErr)
 	h.agent = filepath.Join(dir, "agent")
 	wrapper := "#!/bin/sh\n" +
-		harnessAgentEnv + "=" + shellQuote(d.Name) + " " + harnessDirEnv + "=" + shellQuote(dir) + " exec " + shellQuote(exe) + ` "$@"` + "\n"
+		harnessAgentEnv + "=" + richtext.ShellQuote(d.Name) + " " + harnessDirEnv + "=" + richtext.ShellQuote(dir) + " exec " + richtext.ShellQuote(exe) + ` "$@"` + "\n"
 	require.NoError(t, os.WriteFile(h.agent, []byte(wrapper), 0o700)) //nolint:gosec // the fake agent's wrapper must be executable
 	for _, name := range []string{feedFile, storeFile, linesFile, pollsFile, agentLogFile, liveFile} {
 		require.NoError(t, os.WriteFile(filepath.Join(dir, name), nil, 0o600))
@@ -389,8 +390,6 @@ func harnessStateDir(t *testing.T, dir string) string {
 	return state
 }
 
-func shellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" }
-
 func (h *harness) writeScenario() {
 	data, err := json.Marshal(h.sc)
 	require.NoError(h.t, err)
diff --git a/internal/richtext/shellquote.go b/internal/richtext/shellquote.go
index e191eedf1..51f92c7f8 100644
--- a/internal/richtext/shellquote.go
+++ b/internal/richtext/shellquote.go
@@ -21,11 +21,12 @@ import "strings"
 // which is held to whatever check applied when the value was first created.
 // Escaping cases one at a time is how quoting bugs recur.
 //
-// internal/auth, internal/config and internal/commands each carried a copy
-// of this, written before there was a shared home for it. They are gone:
-// this is the only one, and the only one new code should use. The config
-// copy quoted unconditionally, so paths that need no quoting now appear
-// bare in its warnings — the same shell word, spelled shorter.
+// internal/auth, internal/config, internal/commands and the connector's
+// recovery harness each carried a copy of this, written before there was a
+// shared home for it. They are gone: this is the only implementation, and
+// the only one new code should use. Two of those copies quoted
+// unconditionally, so a value that needs no quoting now appears bare where
+// they were used — the same shell word, spelled shorter.
 func ShellQuote(s string) string {
 	if s != "" && strings.IndexFunc(s, shellActive) < 0 {
 		return s

From 6598b4f4bf56d92d81d64385565721455ea19ad4 Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Sat, 19 Sep 2026 10:22:20 +0200
Subject: [PATCH 5/5] The copy with no name on it, in the codex harness
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

internal/harness/codex_unix_test.go spliced the quotes inline, with no
function around them, to put a temporary path into a script it runs through
sh. Searching for `func shellQuote` could never have found it — and an
inline expression is the copy most likely to drift, because it has no doc
comment to be wrong and nothing to grep for later. Converging an
implementation means grepping for the pattern, not the function.

No cycle: internal/richtext has no internal dependencies at all, so nothing
can import it into one.

The quoting there is load-bearing, and now proven so rather than assumed.
With TMPDIR set to a directory named `o'brien dir café $(id)`, passing the
path through unencoded fails the descendant test — the script writes its pid
somewhere the test does not look — and encoding it passes.

The rest of the sweep: richtext.go's HTML-entity replacements are a
different job, and the composer's `'a' 'b' 'c'` handling parses a drag
payload back apart rather than encoding anything for a shell.
---
 internal/harness/codex_unix_test.go | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/internal/harness/codex_unix_test.go b/internal/harness/codex_unix_test.go
index db4fdbb32..21e6f8cd1 100644
--- a/internal/harness/codex_unix_test.go
+++ b/internal/harness/codex_unix_test.go
@@ -15,6 +15,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/basecamp/basecamp-cli/internal/richtext"
 )
 
 // codexWrapper runs script through sh as the probe's command, with a
@@ -27,9 +29,13 @@ func codexWrapper(t *testing.T, script string, deadline time.Duration) (int, err
 		t.Skip("sh not available")
 	}
 	pidFile := filepath.Join(t.TempDir(), "descendant.pid")
-	// TempDir follows TMPDIR, which may hold a space or a shell metacharacter,
-	// so the path goes into the script single-quoted.
-	script = strings.ReplaceAll(script, "PIDFILE", "'"+strings.ReplaceAll(pidFile, "'", `'\''`)+"'")
+	// TempDir follows TMPDIR, which may hold a space or a shell
+	// metacharacter, so the path is encoded before it goes into the script.
+	// richtext.ShellQuote is the one implementation of that; this used to
+	// splice the quotes inline here, which is a copy with no name on it and
+	// so the kind a search for duplicate quoting never finds (Copilot on
+	// #769).
+	script = strings.ReplaceAll(script, "PIDFILE", richtext.ShellQuote(pidFile))
 
 	ctx, cancel := context.WithTimeout(context.Background(), deadline)
 	defer cancel()