fix(policy): single-quote the policy name in the exported client.rb

cli/policyfile/export.go

@@ -11,6 +11,8 @@ import (
 	"sort"
 
 	cinc "github.com/cinc-project/cinc-api"
+
+	"github.com/cinc-project/cinc-cli/cli/rubylit"
 )
 
 // ExportResult describes a completed export.
@@ -90,9 +92,9 @@ func Export(ctx context.Context, fetcher *Fetcher, lock *cinc.PolicyRevision, lo
 func clientRB(policyName string) string {
 	return fmt.Sprintf(`# Generated by `+"`cinc policy export`"+`.
 use_policyfile true
-policy_name %q
-policy_group "local"
-`, policyName)
+policy_name %s
+policy_group 'local'
+`, rubylit.Quote(policyName))
 }
 
 func sortedLockNames(m map[string]cinc.CookbookLock) []string {

cli/policyfile/export_test.go

@@ -111,8 +111,8 @@ func TestExportAssemblesChefCompatibleTree(t *testing.T) {
 	}
 	// Generated client config selects the policy.
 	clientRB, err := os.ReadFile(filepath.Join(dest, "client.rb"))
-	if err != nil || !contains(string(clientRB), `policy_name "web"`) {
-		t.Errorf("client.rb = %q (err %v), want policy_name \"web\"", clientRB, err)
+	if err != nil || !contains(string(clientRB), `policy_name 'web'`) {
+		t.Errorf("client.rb = %q (err %v), want policy_name 'web'", clientRB, err)
 	}
 	// Archive written.
 	if result.Archive == "" {
@@ -172,3 +172,14 @@ func TestExportUsesIdentifierWhenNoDottedDecimal(t *testing.T) {
 		t.Errorf("expected cookbooks/cb-deadbeef: %v", err)
 	}
 }
+
+// A lock is untrusted input, and client.rb is Ruby that cinc-client
+// evaluates, so the policy name must land in a literal Ruby can't
+// interpolate: in a double-quoted string, #{...} would run as code.
+func TestClientRBDoesNotInterpolatePolicyName(t *testing.T) {
+	got := clientRB(`web#{system("id")}'x`)
+	want := `policy_name 'web#{system("id")}\'x'`
+	if !contains(got, want) {
+		t.Errorf("clientRB = %q, want it to contain %q", got, want)
+	}
+}

cli/remote/bootstrap.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	"github.com/cinc-project/cinc-cli/cli/rubylit"
 )
 
 const DefaultBootstrapURL = "https://omnitruck.cinc.sh/install.sh"
@@ -33,7 +35,7 @@ func BootstrapCommand(opts BootstrapOptions) (string, error) {
 	if opts.ClientKeyPEM == "" {
 		return "", fmt.Errorf("client key is required")
 	}
-	// rubyQuote keeps these out of Ruby's reach, but the client.rb it builds
+	// rubylit.Quote keeps these out of Ruby's reach, but the client.rb it builds
 	// is delivered inside a shell heredoc one layer down. A newline can close
 	// that heredoc early and turn the remainder into commands run through
 	// sudo on the target, so reject it here rather than try to escape it.
@@ -131,18 +133,7 @@ func firstBootJSON(opts BootstrapOptions) ([]byte, error) {
 
 func clientRB(opts BootstrapOptions) string {
 	return fmt.Sprintf("chef_server_url %s\nnode_name %s\nclient_key %s\n",
-		rubyQuote(opts.ServerURL), rubyQuote(opts.NodeName), rubyQuote("/etc/cinc/client.pem"))
-}
-
-// rubyQuote renders s as a single-quoted Ruby string literal. Ruby single
-// quotes do not interpolate #{...} (unlike double quotes and unlike Go's %q),
-// so this prevents a node name or server URL from injecting Ruby code into the
-// client.rb that cinc-client evaluates on the target. Only backslash and the
-// single quote itself are special inside a single-quoted Ruby literal.
-func rubyQuote(s string) string {
-	s = strings.ReplaceAll(s, `\`, `\\`)
-	s = strings.ReplaceAll(s, `'`, `\'`)
-	return "'" + s + "'"
+		rubylit.Quote(opts.ServerURL), rubylit.Quote(opts.NodeName), rubylit.Quote("/etc/cinc/client.pem"))
 }
 
 // heredocDelimiter terminates every file the bootstrap script writes. A line

cli/remote/bootstrap_test.go

@@ -141,7 +141,7 @@ func TestClientRBEscapesQuotesAndBackslashes(t *testing.T) {
 	}
 }
 
-// rubyQuote stops a node name interpolating Ruby, but the client.rb it
+// rubylit.Quote stops a node name interpolating Ruby, but the client.rb it
 // produces is then delivered inside a shell heredoc. A newline in the node
 // name (or the profile's server URL) can close that heredoc early and turn the
 // rest of the value into commands, which run through sudo on the target. The

cli/rubylit/rubylit.go

@@ -0,0 +1,17 @@
+// Package rubylit renders Go values as Ruby source literals for the Ruby
+// configuration files cinc generates (client.rb and friends), which the
+// target's cinc-client evaluates as code.
+package rubylit
+
+import "strings"
+
+// Quote renders s as a single-quoted Ruby string literal. Ruby single quotes
+// do not interpolate #{...} (unlike double quotes and unlike Go's %q), so a
+// value such as a node or policy name can't inject Ruby into the file that
+// embeds it. Only backslash and the single quote itself are special inside a
+// single-quoted Ruby literal.
+func Quote(s string) string {
+	s = strings.ReplaceAll(s, `\`, `\\`)
+	s = strings.ReplaceAll(s, `'`, `\'`)
+	return "'" + s + "'"
+}

cli/rubylit/rubylit_test.go

@@ -0,0 +1,18 @@
+package rubylit
+
+import "testing"
+
+func TestQuote(t *testing.T) {
+	for _, tc := range []struct{ in, want string }{
+		{"web", `'web'`},
+		{`#{system("id")}`, `'#{system("id")}'`},
+		{`it's`, `'it\'s'`},
+		{`C:\keys`, `'C:\\keys'`},
+		{`\'`, `'\\\''`},
+		{"", `''`},
+	} {
+		if got := Quote(tc.in); got != tc.want {
+			t.Errorf("Quote(%q) = %s, want %s", tc.in, got, tc.want)
+		}
+	}
+}