fix(scripts): escape flag values per output format in generate-federation-iac

internal/api/handler_federation.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/fs"
+	"reflect"
 	"regexp"
 	"strings"
 	"text/template"
@@ -400,6 +401,31 @@ func shellEscape(s string) string {
 	return r.Replace(s)
 }
 
+// hclEscape escapes a string for use inside a double-quoted HCL string. HCL
+// also starts an interpolation at "${" and a template directive at "%{", so
+// both are doubled into their literal "$${" and "%%{" forms (#952). Line
+// breaks are not handled: the one user-supplied value, the session email,
+// cannot hold one (mail.ParseAddress rejects CR and LF), and the rest come
+// from server configuration or are derived by the server.
+func hclEscape(s string) string {
+	return strings.NewReplacer(`\`, `\\`, `"`, `\"`, "${", "$${", "%{", "%%{").Replace(s)
+}
+
+// hclEscapeData returns a copy of data with every string field HCL-escaped,
+// for rendering the bundle's tfvars. Most fields are server-derived, but
+// ContactEmail is the session user's email, which only has to pass
+// mail.ParseAddress and so can carry quotes and "${" in a quoted local part
+// (#952). It walks the struct by reflection so a new field cannot be missed.
+func hclEscapeData(data federationIaCData) federationIaCData {
+	v := reflect.ValueOf(&data).Elem()
+	for i := 0; i < v.NumField(); i++ {
+		if f := v.Field(i); f.Kind() == reflect.String {
+			f.SetString(hclEscape(f.String()))
+		}
+	}
+	return data
+}
+
 // validFederationTargets is the allowlist of valid target cloud providers.
 var validFederationTargets = map[string]bool{"aws": true, "azure": true, "gcp": true}
 
@@ -589,7 +615,7 @@ func addBundleTerraform(zw *zip.Writer, data federationIaCData, target, source,
 		return fmt.Errorf("bundle: terraform dir: %w", err)
 	}
 	tfvarsTmpl, tfvarsName := bundleTfvarsSpec(target, source, slug)
-	tfvarsContent, err := renderTemplate(tfvarsTmpl, data)
+	tfvarsContent, err := renderTemplate(tfvarsTmpl, hclEscapeData(data))
 	if err != nil {
 		return fmt.Errorf("bundle: %w", err)
 	}

internal/api/handler_federation_test.go

@@ -1758,3 +1758,64 @@ func TestAWSTargetTemplate_ThumbprintWiring(t *testing.T) {
 	assert.NotContains(t, src, `Default: "`+allZeroThumbprint+`"`,
 		"the all-zeros placeholder must not be reintroduced as a default")
 }
+
+// TestGetFederationIaC_BundleTfvarsEscapesSessionEmail is the server half of
+// #952. The bundle's tfvars embed the session user's email, which only has to
+// pass mail.ParseAddress, so a quoted local part can carry '"', '\' and HCL's
+// "${" / "%{" sequences. The tfvars must carry it as one HCL string literal,
+// and the shell-escaped CFN deploy script in the same bundle must not be
+// HCL-escaped on top of that.
+func TestGetFederationIaC_BundleTfvarsEscapesSessionEmail(t *testing.T) {
+	t.Setenv("CUDLY_SOURCE_CLOUD", "gcp")
+	const email = `"o\"q${x}%{y}"@example.com`
+	mockAuth := new(MockAuthService)
+	mockAuth.On("ValidateSession", mock.Anything, "admin-token").Return(&Session{
+		UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd",
+		Email:  email,
+	}, nil)
+	mockAuth.grantAdmin()
+	h := NewHandler(HandlerConfig{ConfigStore: new(MockConfigStore), AuthService: mockAuth})
+	mockSourceIdentity(h, defaultTestSourceIdentity())
+
+	for _, tc := range []struct{ target, source string }{{"aws", "gcp"}, {"gcp", "aws"}, {"azure", "gcp"}} {
+		t.Run(tc.target+"-"+tc.source, func(t *testing.T) {
+			res, err := h.getFederationIaC(context.Background(), federationReqWithDomain(map[string]string{
+				"target": tc.target, "source": tc.source, "format": "bundle",
+			}))
+			require.NoError(t, err)
+			zipBytes, err := base64.StdEncoding.DecodeString(res.Content)
+			require.NoError(t, err)
+			zr, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes)))
+			require.NoError(t, err)
+
+			files := map[string]string{}
+			for _, f := range zr.File {
+				rc, err := f.Open()
+				require.NoError(t, err)
+				var buf bytes.Buffer
+				_, err = buf.ReadFrom(rc)
+				require.NoError(t, rc.Close())
+				require.NoError(t, err)
+				files[f.Name] = buf.String()
+			}
+
+			var tfvars string
+			for name, content := range files {
+				if strings.HasSuffix(name, ".tfvars") {
+					tfvars = content
+				}
+			}
+			require.NotEmpty(t, tfvars, "bundle must contain a .tfvars file")
+			// Decodes back to the email exactly: \" -> ", \\ -> \, $${ -> ${, %%{ -> %{.
+			assert.Contains(t, tfvars, `contact_email = "\"o\\\"q$${x}%%{y}\"@example.com"`+"\n")
+			assert.NotContains(t, tfvars, email, "raw email must not reach the tfvars")
+
+			if tc.target == "aws" {
+				deploy := files["cloudformation/deploy-cfn.sh"]
+				require.NotEmpty(t, deploy, "aws bundle must contain the CFN deploy script")
+				assert.Contains(t, deploy, shellEscape(email), "CFN deploy script keeps its shell escaping")
+				assert.NotContains(t, deploy, "$${", "CFN deploy script must not be HCL-escaped")
+			}
+		})
+	}
+}

scripts/generate-federation-iac.go

@@ -95,14 +95,17 @@ package main
 import (
 	"archive/zip"
 	"bytes"
+	"encoding/json"
 	"errors"
 	"flag"
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"strings"
 	"text/template"
+	"unicode"
 )
 
 // iacData mirrors internal/api/handler_federation.go:federationIaCData.
@@ -175,7 +178,91 @@ func gcpOIDCIssuerURI(source, tenantID string) string {
 	return ""
 }
 
+// hclEscape escapes s for use inside a double-quoted HCL string. Besides the
+// backslash and the quote, HCL treats "${" and "%{" as the start of an
+// interpolation or a template directive, so those are doubled into the
+// literal "$${" and "%%{" forms (#952).
+func hclEscape(s string) string {
+	return strings.NewReplacer(`\`, `\\`, `"`, `\"`, "${", "$${", "%{", "%%{").Replace(s)
+}
+
+// shellEscape escapes s for use inside a double-quoted Bash word. It is the
+// same rule as shellEscape in internal/api/handler_federation.go.
+func shellEscape(s string) string {
+	return strings.NewReplacer(`\`, `\\`, `"`, `\"`, "`", "\\`", `$`, `\$`).Replace(s)
+}
+
+// jsonEscape escapes s for use inside a JSON string literal. HTML escaping is
+// off so that values such as the <AZURE_TENANT_ID> placeholder render as is.
+func jsonEscape(s string) string {
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false)
+	if err := enc.Encode(s); err != nil {
+		panic("encoding a string as JSON cannot fail: " + err.Error())
+	}
+	// Encode writes "<escaped>" plus a newline; keep only what is between the quotes.
+	quoted := strings.TrimSuffix(buf.String(), "\n")
+	return quoted[1 : len(quoted)-1]
+}
+
+// escaperFor picks the escaping rule for a template from its file name, so
+// that every value lands in its output file as data rather than syntax (#952).
+// The rule follows the output grammar, not the field: the same AccountName is
+// HCL in a .tfvars file and Bash in a .sh file. An unknown suffix is an error
+// rather than "no escaping", so a new template type cannot render raw values.
+//
+// Escaping covers quoted strings. Values also appear in # comments, where only
+// a line break could change the structure; checkNoControlChars rejects those
+// before anything is rendered, which is why the escapers need not handle them.
+func escaperFor(tmplPath string) (func(string) string, error) {
+	switch {
+	case strings.HasSuffix(tmplPath, ".tfvars.tmpl"):
+		return hclEscape, nil
+	case strings.HasSuffix(tmplPath, ".sh.tmpl"):
+		return shellEscape, nil
+	case strings.HasSuffix(tmplPath, ".json.tmpl"):
+		return jsonEscape, nil
+	default:
+		return nil, fmt.Errorf("no escaping rule for template %s", tmplPath)
+	}
+}
+
+// escapeData returns a copy of data with escape applied to every string
+// field. It walks the struct by reflection so that a field added to iacData
+// later is escaped without anyone having to remember to list it here.
+func escapeData(data iacData, escape func(string) string) iacData {
+	v := reflect.ValueOf(&data).Elem()
+	for i := 0; i < v.NumField(); i++ {
+		if f := v.Field(i); f.Kind() == reflect.String {
+			f.SetString(escape(f.String()))
+		}
+	}
+	return data
+}
+
+// checkNoControlChars rejects any flag whose value contains a control
+// character such as a newline. No flag this script takes can legitimately hold
+// one, and a line break is the one character the per-format escaping above
+// does not neutralize: values are also rendered into # comment lines, where a
+// newline would start a new line of HCL or Bash (#952).
+func checkNoControlChars(fs *flag.FlagSet) error {
+	var err error
+	fs.Visit(func(f *flag.Flag) {
+		if err == nil && strings.IndexFunc(f.Value.String(), unicode.IsControl) >= 0 {
+			err = fmt.Errorf("--%s %s contains a control character such as a newline or tab, which is not allowed",
+				f.Name, displayClaim(f.Value.String()))
+		}
+	})
+	return err
+}
+
 func renderTmpl(tmplPath string, data iacData) (string, error) {
+	escape, err := escaperFor(tmplPath)
+	if err != nil {
+		return "", err
+	}
+	data = escapeData(data, escape)
 	b, err := os.ReadFile(tmplPath)
 	if err != nil {
 		return "", fmt.Errorf("read %s: %w\n\nRun this script from the repository root directory.", tmplPath, err)
@@ -657,6 +744,12 @@ func main() {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
 	}
+	// After populateData, so the field-specific validators above still give
+	// their more precise diagnosis for the flags they cover.
+	if err := checkNoControlChars(flag.CommandLine); err != nil {
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+		os.Exit(1)
+	}
 
 	if *format == "bundle" {
 		runBundle(data, *target, *source, slug, *templDir, *modulesDir, *outFile)