From 69d52aeffbe8958e5c1af9d8e0204402256c0dd0 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 16 Oct 2026 11:55:16 +0100 Subject: [PATCH] fix(scripts): escape flag values per output format in generate-federation-iac The generator put --account-name, --account-slug, --contact-email, --cudly-api-url, --tenant-id, --project-id and --service-account-email into the shared templates unescaped, so a quote, "${" or a newline could change the structure of the generated tfvars, JSON or Bash file. - renderTmpl now escapes every string field of iacData for the grammar of the template it renders, chosen by file suffix: HCL for *.tfvars.tmpl (\, ", and the "${" / "%{" template sequences), the server's shellEscape rule for *.sh.tmpl, and JSON for *.json.tmpl. An unknown suffix is an error. Fields are walked by reflection so a new one cannot be missed. - Any flag value containing a control character is rejected after the existing field validators run. Values are also rendered into # comment lines, where only a line break could change the structure. - The server's bundle tfvars get the same HCL escaping. Their one user-supplied value, ContactEmail (the session email), only has to pass mail.ParseAddress and can carry quotes and "${" in a quoted local part. CLI and CFN shell output keeps its existing shell escaping. Output for values without metacharacters is byte-identical. Closes #952 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_federation.go | 28 +- internal/api/handler_federation_test.go | 61 +++ scripts/generate-federation-iac.go | 93 +++++ .../generate_federation_iac_escape_test.go | 356 ++++++++++++++++++ 4 files changed, 537 insertions(+), 1 deletion(-) create mode 100644 scripts/generate_federation_iac_escape_test.go diff --git a/internal/api/handler_federation.go b/internal/api/handler_federation.go index 60d480515..73da5ea31 100644 --- a/internal/api/handler_federation.go +++ b/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) } diff --git a/internal/api/handler_federation_test.go b/internal/api/handler_federation_test.go index 04aa802c5..07240ae6d 100644 --- a/internal/api/handler_federation_test.go +++ b/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") + } + }) + } +} diff --git a/scripts/generate-federation-iac.go b/scripts/generate-federation-iac.go index 29a9870c1..7e1852c84 100644 --- a/scripts/generate-federation-iac.go +++ b/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 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 "" 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) diff --git a/scripts/generate_federation_iac_escape_test.go b/scripts/generate_federation_iac_escape_test.go new file mode 100644 index 000000000..ab69012f5 --- /dev/null +++ b/scripts/generate_federation_iac_escape_test.go @@ -0,0 +1,356 @@ +package main + +// Regression tests for #952: flag values used to reach the tfvars, JSON and +// Bash templates unescaped, so a quote, "${" or a newline in a value such as +// --account-name could change the structure of the generated file. These live +// in their own file because the two existing test files for the script are at +// the repository's 500-line limit. + +import ( + "archive/zip" + "encoding/json" + "io" + "os" + "os/exec" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Hostile values for the flags that have no field-specific validation. Each +// one carries the metacharacters of all three output grammars: quotes and +// backslashes (HCL, JSON, Bash), "${" and "%{" (HCL interpolation and +// directives) and "$(...)" plus backticks (Bash command substitution). +const ( + hostileName = `prod" ${x} %{y} \z $(printf INJECTED) ` + "`printf INJECTED`" + hostileEmail = `"o\"q${x}%{y}"@example.com` + hostileURL = `https://cudly.example.com/a"b${x}` + hostileSlug = `slug"x` + hostileSA = `sa"x${y}@example.com` + // hostileShellEmail is hostileEmail without a '}'. The shared shellEscape + // rule (the server's, mirrored by the script) does not escape '}', so inside + // a "${VAR:-default}" expansion a '}' in the value ends the expansion early + // and corrupts the value. It cannot run a command or leave the quotes, + // because '$', '`', '"' and '\' are all escaped; fixing it needs a + // context-aware rule for the server's CLI templates too, so it is out of + // scope for #952. + hostileShellEmail = `"o\"q$x $(printf INJECTED) ` + "`printf INJECTED`" + `"@example.com` +) + +// hclAttrRE matches one `name = "value"` tfvars line, optionally followed by a +// trailing # comment. The value is a complete HCL quoted string: any run of +// non-quote, non-backslash characters or backslash escapes. +var hclAttrRE = regexp.MustCompile(`^([a-z_]+)\s*=\s*"((?:[^"\\]|\\.)*)"\s*(#.*)?$`) + +// hclUnquote decodes the body of an HCL quoted string, failing on anything +// HCL would read as syntax rather than text: an unescaped "${" or "%{". +func hclUnquote(t *testing.T, s string) string { + t.Helper() + var b strings.Builder + for i := 0; i < len(s); i++ { + c := s[i] + rest := s[i:] + switch { + case c == '\\': + require.Less(t, i+1, len(s), "dangling backslash in %q", s) + i++ + switch s[i] { + case '\\', '"': + b.WriteByte(s[i]) + default: + t.Fatalf("unexpected escape \\%c in %q", s[i], s) + } + case strings.HasPrefix(rest, "$${"), strings.HasPrefix(rest, "%%{"): + b.WriteString(rest[1:3]) + i += 2 + case strings.HasPrefix(rest, "${"), strings.HasPrefix(rest, "%{"): + t.Fatalf("unescaped HCL template sequence at %q in %q", rest, s) + default: + b.WriteByte(c) + } + } + return b.String() +} + +// parseTfvars returns the attributes of a generated tfvars file, failing if +// any line is neither blank, a # comment, nor a single quoted attribute. +func parseTfvars(t *testing.T, content string) map[string]string { + t.Helper() + attrs := map[string]string{} + for _, line := range strings.Split(content, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + m := hclAttrRE.FindStringSubmatch(trimmed) + require.NotNil(t, m, "line is not a single quoted HCL attribute: %q\nfile:\n%s", line, content) + _, dup := attrs[m[1]] + require.False(t, dup, "attribute %s set twice\nfile:\n%s", m[1], content) + attrs[m[1]] = hclUnquote(t, m[2]) + } + return attrs +} + +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// tfvarsCase is one single-file tfvars invocation. base holds the +// target-specific flags; hostile and benign hold the values for the flags +// under test, and want maps a tfvars attribute to the hostile value it must +// decode back to. +type tfvarsCase struct { + name string + base []string + hostile []string + benign []string + want map[string]string +} + +func tfvarsCases() []tfvarsCase { + common := []string{"--account-name", hostileName, "--contact-email", hostileEmail} + commonBenign := []string{"--account-name", "Acme", "--contact-email", "ops@example.com"} + commonWant := map[string]string{"account_name": hostileName, "contact_email": hostileEmail} + withWant := func(extra map[string]string) map[string]string { + out := map[string]string{} + for k, v := range commonWant { + out[k] = v + } + for k, v := range extra { + out[k] = v + } + return out + } + return []tfvarsCase{ + { + name: "aws-wif", + base: []string{"--target", "aws", "--source", "azure", "--account-id", validAccountIDFor("aws"), + "--oidc-subject-claim", "11111111-2222-3333-4444-555555555555"}, + hostile: append([]string{"--tenant-id", `t"x${y}`, "--cudly-api-url", hostileURL, "--account-slug", hostileSlug}, common...), + benign: append([]string{"--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", "--cudly-api-url", "https://cudly.example.com", "--account-slug", "acme"}, commonBenign...), + want: withWant(map[string]string{ + "oidc_issuer_url": "https://login.microsoftonline.com/" + `t"x${y}` + "/v2.0", + "cudly_api_url": hostileURL, + "role_name": "CUDly-" + hostileSlug, + }), + }, + { + name: "aws-cross-account", + base: []string{"--target", "aws", "--source", "aws", "--account-id", validAccountIDFor("aws"), + "--source-account-id", "111122223333"}, + hostile: append([]string{"--cudly-api-url", hostileURL}, common...), + benign: append([]string{"--cudly-api-url", "https://cudly.example.com"}, commonBenign...), + want: withWant(map[string]string{"cudly_api_url": hostileURL}), + }, + { + name: "azure-wif", + base: []string{"--target", "azure", "--source", "aws", "--account-id", validAccountIDFor("azure"), + "--cudly-api-url", "https://cudly.example.com"}, + hostile: append([]string{"--tenant-id", `t"x${y}`}, common...), + benign: append([]string{"--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"}, commonBenign...), + want: withWant(map[string]string{"tenant_id": `t"x${y}`}), + }, + { + name: "gcp-sa-impersonation", + base: []string{"--target", "gcp", "--source", "gcp", "--account-id", validAccountIDFor("gcp")}, + hostile: append([]string{"--project-id", `p"x%{y}`, "--service-account-email", hostileSA}, common...), + benign: append([]string{"--project-id", "my-project", "--service-account-email", "cudly@my-project.iam.gserviceaccount.com"}, commonBenign...), + want: withWant(map[string]string{"project_id": `p"x%{y}`, "service_account_email": hostileSA}), + }, + { + name: "gcp-wif", + base: []string{"--target", "gcp", "--source", "azure", "--account-id", validAccountIDFor("gcp"), + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"}, + hostile: append([]string{"--project-id", `p"x%{y}`, "--service-account-email", hostileSA, "--cudly-api-url", hostileURL}, common...), + benign: append([]string{"--project-id", "my-project", "--service-account-email", "cudly@my-project.iam.gserviceaccount.com", "--cudly-api-url", "https://cudly.example.com"}, commonBenign...), + want: withWant(map[string]string{"project": `p"x%{y}`, "service_account_email": hostileSA, "cudly_api_url": hostileURL}), + }, + } +} + +// TestGenerator_TfvarsValuesCannotChangeStructure is the #952 regression test +// for the tfvars output: with hostile values in every free-form flag, the file +// must hold exactly the attributes a benign run produces, and each value must +// decode back to the flag value it came from, so the input was carried as data +// and never read as HCL. +func TestGenerator_TfvarsValuesCannotChangeStructure(t *testing.T) { + for _, tc := range tfvarsCases() { + t.Run(tc.name, func(t *testing.T) { + benign := runGenerator(t, append(append(append([]string{}, tc.base...), tc.benign...), "--output", "-")...) + require.Zero(t, benign.exitCode, "stderr: %s", benign.stderr) + wantKeys := sortedKeys(parseTfvars(t, benign.stdout)) + + res := runGenerator(t, append(append(append([]string{}, tc.base...), tc.hostile...), "--output", "-")...) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + got := parseTfvars(t, res.stdout) + require.Equal(t, wantKeys, sortedKeys(got), "hostile input changed the set of attributes") + for attr, want := range tc.want { + require.Equal(t, want, got[attr], "attribute %s", attr) + } + }) + } +} + +// TestGenerator_CFParamsValuesCannotChangeStructure covers the JSON output: +// a --tenant-id that tries to close the string and open a new parameter must +// stay inside the OIDCIssuerURL value. +func TestGenerator_CFParamsValuesCannotChangeStructure(t *testing.T) { + tenant := `t"},{"ParameterKey":"Injected","ParameterValue":"x${y}%{z}\` + res := runGenerator(t, + "--target", "aws", "--source", "azure", "--format", "cf-params", + "--account-name", hostileName, "--account-slug", hostileSlug, + "--account-id", validAccountIDFor("aws"), "--tenant-id", tenant, + "--oidc-subject-claim", "11111111-2222-3333-4444-555555555555", "--output", "-") + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + + var params []map[string]string + require.NoError(t, json.Unmarshal([]byte(res.stdout), ¶ms), "output must be valid JSON:\n%s", res.stdout) + got := map[string]string{} + for _, p := range params { + require.Len(t, p, 2, "unexpected parameter shape %v", p) + got[p["ParameterKey"]] = p["ParameterValue"] + } + require.Equal(t, map[string]string{ + "OIDCIssuerURL": "https://login.microsoftonline.com/" + tenant + "/v2.0", + "OIDCAudience": "api://AzureADTokenExchange", + "OIDCSubjectClaim": "11111111-2222-3333-4444-555555555555", + "RoleName": "CUDly-" + hostileSlug, + }, got) +} + +// probeDeployScript stands in for aws-cfn-deploy.sh.tmpl. The real template +// cannot be rendered by this script today (iacData lacks OIDCIssuerHost, +// #153), so this probe exercises the same .sh.tmpl escaping path with the +// quoting shapes the real Bash templates use, and prints what Bash made of +// each value instead of calling any cloud CLI. +const probeDeployScript = `#!/usr/bin/env bash +# Target: {{.AccountName}} ({{.AccountExternalID}}) +set -euo pipefail +: "${STACK_NAME:=CUDly-{{.AccountSlug}}-WIF}" +NAME="{{.AccountName}}" +CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-{{.ContactEmail}}}" +URL="{{.CUDlyAPIURL}}/api/register" +printf '%s\n' "$STACK_NAME" "$NAME" "$CONTACT_EMAIL" "$URL" +` + +// TestGenerator_ShellTemplatesEscapeValues covers the Bash output: the bundle's +// deploy script is rendered through the .sh.tmpl path and then run, and Bash +// must see each hostile value as the literal string that was passed. +func TestGenerator_ShellTemplatesEscapeValues(t *testing.T) { + bash, lookErr := exec.LookPath("bash") + if lookErr != nil { + t.Skip("bash not available") + } + templates := t.TempDir() + for _, name := range []string{"aws-wif.tfvars.tmpl", "aws-wif-cf-params.json.tmpl"} { + b, err := os.ReadFile(filepath.Join("..", "internal", "iacfiles", "templates", name)) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(templates, name), b, 0o600)) + } + require.NoError(t, os.WriteFile(filepath.Join(templates, "aws-cfn-deploy.sh.tmpl"), []byte(probeDeployScript), 0o600)) + + bundle := filepath.Join(t.TempDir(), "bundle.zip") + res := runGenerator(t, + "--target", "aws", "--source", "gcp", "--format", "bundle", + "--templates-dir", templates, "--output", bundle, + "--account-name", hostileName, "--account-slug", hostileSlug, + "--account-id", validAccountIDFor("aws"), + "--oidc-subject-claim", "123456789012345678901", + "--contact-email", hostileShellEmail, "--cudly-api-url", hostileURL) + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + + archive, err := zip.OpenReader(bundle) + require.NoError(t, err) + defer archive.Close() + entry, err := archive.Open("cloudformation/deploy-cfn.sh") + require.NoError(t, err) + script, err := io.ReadAll(entry) + require.NoError(t, entry.Close()) + require.NoError(t, err) + + path := filepath.Join(t.TempDir(), "deploy-cfn.sh") + require.NoError(t, os.WriteFile(path, script, 0o600)) + cmd := exec.Command(bash, path) + cmd.Env = []string{"PATH=" + os.Getenv("PATH")} + out, err := cmd.CombinedOutput() + require.NoError(t, err, "rendered script failed:\n%s\nscript:\n%s", out, script) + require.Equal(t, strings.Join([]string{ + "CUDly-" + hostileSlug + "-WIF", + hostileName, + hostileShellEmail, + hostileURL + "/api/register", + }, "\n")+"\n", string(out), "script:\n%s", script) +} + +// TestGenerator_RejectsControlCharacters covers the one shape escaping cannot +// neutralize: values are also rendered into # comment lines, where a newline +// starts a new line of HCL or Bash. The issue's own example is the first case. +// Each flag here has no field-specific validator, so this check is the only +// thing between it and the template. +func TestGenerator_RejectsControlCharacters(t *testing.T) { + gcpBase := []string{"--target", "gcp", "--source", "gcp", "--account-name", "Acme", "--account-id", validAccountIDFor("gcp")} + cases := []struct { + name string + flag string + val string + }{ + {name: "account-name newline (issue example)", flag: "account-name", val: "prod\"\nevil = \"x"}, + {name: "account-name carriage return", flag: "account-name", val: "prod\rx"}, + {name: "account-name tab", flag: "account-name", val: "prod\tx"}, + {name: "account-name escape sequence", flag: "account-name", val: "prod\x1b[2Jx"}, + {name: "account-slug newline", flag: "account-slug", val: "acme\nx"}, + {name: "contact-email newline", flag: "contact-email", val: "ops@example.com\nevil = \"x\""}, + {name: "cudly-api-url newline", flag: "cudly-api-url", val: "https://cudly.example.com\nx"}, + {name: "project-id newline", flag: "project-id", val: "my-project\nx"}, + {name: "service-account-email newline", flag: "service-account-email", val: "sa@example.com\nx"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + output := filepath.Join(t.TempDir(), "output") + args := append(append([]string{}, gcpBase...), "--output", output, "--"+tc.flag+"="+tc.val) + res := runGenerator(t, args...) + require.NotZero(t, res.exitCode, "stdout: %s; stderr: %s", res.stdout, res.stderr) + require.Contains(t, res.stderr, "--"+tc.flag+" ") + require.Contains(t, res.stderr, "contains a control character") + require.Empty(t, res.stdout) + _, err := os.Stat(output) + require.ErrorIs(t, err, os.ErrNotExist, "no artifact may be written for a rejected value") + }) + } + + t.Run("tenant-id newline", func(t *testing.T) { + res := runGenerator(t, + "--target", "azure", "--source", "aws", "--account-name", "Acme", + "--account-id", validAccountIDFor("azure"), "--cudly-api-url", "https://cudly.example.com", + "--tenant-id", "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee\nx", "--output", "-") + require.NotZero(t, res.exitCode, "stdout: %s", res.stdout) + require.Contains(t, res.stderr, "--tenant-id ") + require.Contains(t, res.stderr, "contains a control character") + require.Empty(t, res.stdout) + }) +} + +// TestGenerator_PlainValuesRenderVerbatim is the positive control: a value with +// no metacharacters must come out byte for byte, so escaping never alters +// ordinary input (including '+' and '-', which have no meaning in any of the +// three grammars). +func TestGenerator_PlainValuesRenderVerbatim(t *testing.T) { + res := runGenerator(t, + "--target", "gcp", "--source", "gcp", "--account-name", "Acme Prod-1", + "--account-id", validAccountIDFor("gcp"), "--contact-email", "user+tag@example.com", + "--cudly-api-url", "https://cudly.example.com:8443/base", "--output", "-") + require.Zero(t, res.exitCode, "stderr: %s", res.stderr) + require.Contains(t, res.stdout, `account_name = "Acme Prod-1"`) + require.Contains(t, res.stdout, `contact_email = "user+tag@example.com"`) + require.Contains(t, res.stdout, `cudly_api_url = "https://cudly.example.com:8443/base"`) + require.Contains(t, res.stdout, "# Target GCP project : Acme Prod-1 (my-project)") +}