fix(iac): keep "}" and JSON metacharacters in 8 more deploy scripts' contact email

internal/api/handler_federation_azure_escape_test.go

@@ -63,7 +63,7 @@ func evalShellWord(t *testing.T, bashPath, script string, env ...string) (out, d
 }
 
 var (
-	// registrationBlockRe matches deploy-azure.sh from the CONTACT_EMAIL
+	// registrationBlockRe matches a deploy script from the CONTACT_EMAIL
 	// assignment through the end of the PAYLOAD heredoc: the part that
 	// turns the pre-filled email into the registration JSON, stopping
 	// before the curl call.
@@ -83,12 +83,20 @@ type registration struct {
 // CLI, no network; curl is past the end of the block). It returns the
 // resulting CONTACT_EMAIL and the PAYLOAD heredoc parsed as JSON.
 func evalRegistration(t *testing.T, bashPath, script string, env ...string) registration {
+	t.Helper()
+	return evalRegistrationWith(t, bashPath,
+		"az() { printf '%s\\n' 00000000-0000-0000-0000-0000000000aa; }\n", script, env...)
+}
+
+// evalRegistrationWith is evalRegistration with the caller's prelude (stub
+// functions and the variables the block reads) in place of the az stub, so
+// the other generated scripts' registration blocks can run the same way.
+func evalRegistrationWith(t *testing.T, bashPath, prelude, script string, env ...string) registration {
 	t.Helper()
 	block := registrationBlockRe.FindString(script)
-	require.NotEmpty(t, block, "deploy-azure.sh must build the registration payload")
-	stub := "set -euo pipefail\naz() { printf '%s\\n' 00000000-0000-0000-0000-0000000000aa; }\n"
+	require.NotEmpty(t, block, "the script must build the registration payload")
 	out, dir := evalShellWord(t, bashPath,
-		stub+block+"\nprintf '%s\\0%s' \"$CONTACT_EMAIL\" \"$PAYLOAD\"", env...)
+		"set -euo pipefail\n"+prelude+block+"\nprintf '%s\\0%s' \"$CONTACT_EMAIL\" \"$PAYLOAD\"", env...)
 	email, payload, ok := strings.Cut(out, "\x00")
 	require.True(t, ok, "sandbox output must hold CONTACT_EMAIL and PAYLOAD: %q", out)
 	var fields map[string]string

internal/api/handler_federation_registration_escape_test.go

@@ -0,0 +1,206 @@
+package api
+
+import (
+	"bytes"
+	"context"
+	"encoding/base64"
+	"io/fs"
+	"net/mail"
+	"os/exec"
+	"path"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/iacfiles"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Issue #1436: the #958 fix (CONTACT_EMAIL default on its own line, JSON
+// escaping of the registration payload) ported to the other eight generated
+// scripts that self-register with CUDly.
+
+// registrationScriptPrelude stubs the cloud CLIs and sets the variables the
+// registration blocks read, so each block runs under set -u with no CLI, no
+// cloud and no network (curl is past the end of the block).
+const registrationScriptPrelude = `aws() { printf '%s\n' stub-aws-output; }
+az() { printf '%s\n' 00000000-0000-0000-0000-0000000000aa; }
+PROFILE_ARG= REGION=us-east-1 STACK_NAME=stub-stack ROLE_ARN=stub-role EXTERNAL_ID=stub-external-id
+SUBSCRIPTION_ID=00000000-0000-0000-0000-0000000000aa TENANT_ID=00000000-0000-0000-0000-0000000000bb APP_ID=00000000-0000-0000-0000-0000000000cc
+PROJECT_ID=stub-project SERVICE_ACCOUNT_EMAIL=stub-sa@stub-project.iam.gserviceaccount.com WIF_AUDIENCE=stub-audience
+`
+
+// registrationScript is one generated script that self-registers with CUDly,
+// and how a download produces it.
+type registrationScript struct {
+	template               string // under internal/iacfiles/templates
+	target, source, format string // the GET /api/federation/iac query
+	zipEntry               string // the script's name in a zip download
+}
+
+// registrationScripts lists every shell template with a registration block
+// except the Bicep/ARM deploy script, which #958 covers. Each is fetched
+// through getFederationIaC, the real download path, except
+// azure-wif-deploy.sh.tmpl: no download format ships it today, so it is
+// rendered the way every shell template is (shellEscapeData, then
+// renderTemplate).
+var registrationScripts = []registrationScript{
+	{template: "aws-wif-cli.sh.tmpl", target: "aws", source: "gcp", format: "cli"},
+	{template: "aws-cross-account-cli.sh.tmpl", target: "aws", source: "aws", format: "cli"},
+	{template: "azure-wif-cli.sh.tmpl", target: "azure", source: "gcp", format: "cli"},
+	{template: "gcp-wif-cli.sh.tmpl", target: "gcp", source: "aws", format: "cli"},
+	{template: "gcp-sa-impersonation-cli.sh.tmpl", target: "gcp", source: "gcp", format: "cli"},
+	{template: "aws-cfn-deploy.sh.tmpl", target: "aws", source: "gcp", format: "cfn", zipEntry: "cloudformation/deploy-cfn.sh"},
+	{template: "aws-cross-account-cfn-deploy.sh.tmpl", target: "aws", source: "aws", format: "cfn", zipEntry: "cloudformation/deploy-cfn.sh"},
+	{template: "azure-wif-deploy.sh.tmpl", target: "azure", source: "gcp"},
+}
+
+// renderRegistrationScript returns the script sc produces for a session whose
+// email is email.
+func renderRegistrationScript(t *testing.T, sc registrationScript, email string) string {
+	t.Helper()
+	if sc.format == "" {
+		data := buildGenericIaCData(sc.target, sc.source, "https://cudly.example.com")
+		data.ContactEmail = email
+		script, err := renderTemplate("templates/"+sc.template, shellEscapeData(data))
+		require.NoError(t, err)
+		return script
+	}
+	// Self-sourced targets need CUDly deployed on that cloud (#42).
+	if sc.target == sc.source {
+		t.Setenv("CUDLY_SOURCE_CLOUD", sc.source)
+	} else {
+		t.Setenv("CUDLY_SOURCE_CLOUD", "gcp")
+	}
+	h := federationHandlerWithEmail(t, email)
+	res, err := h.getFederationIaC(context.Background(), federationReqWithDomain(map[string]string{
+		"target": sc.target, "source": sc.source, "format": sc.format,
+	}))
+	require.NoError(t, err)
+	if sc.zipEntry == "" {
+		return res.Content
+	}
+	zipBytes, err := base64.StdEncoding.DecodeString(res.Content)
+	require.NoError(t, err)
+	return azureZipEntry(t, zipBytes, sc.zipEntry)
+}
+
+// TestRegistrationScripts_KeepContactEmailAndPayloadIntact is the regression
+// test for #1436. For every hostile email (a "}", quotes, backslashes, a tab,
+// command substitutions; all accepted by mail.ParseAddress) each script must
+// parse, its registration block must leave CONTACT_EMAIL equal to the email,
+// put it into valid JSON unchanged, and run nothing.
+func TestRegistrationScripts_KeepContactEmailAndPayloadIntact(t *testing.T) {
+	bashPath, err := exec.LookPath("bash")
+	if err != nil {
+		t.Skip("bash not available")
+	}
+	for _, sc := range registrationScripts {
+		for _, email := range hostileContactEmails {
+			_, err := mail.ParseAddress(email)
+			require.NoError(t, err, "fixture %q must be an address the auth service accepts", email)
+			t.Run(sc.template+"/"+email, func(t *testing.T) {
+				script := renderRegistrationScript(t, sc, email)
+
+				syntax := exec.Command(bashPath, "-n")
+				syntax.Stdin = bytes.NewReader([]byte(script))
+				out, err := syntax.CombinedOutput()
+				require.NoError(t, err, "%s must parse: %s", sc.template, out)
+
+				got := evalRegistrationWith(t, bashPath, registrationScriptPrelude, script)
+				assert.Equal(t, email, got.contactEmail, "CONTACT_EMAIL must evaluate to the email verbatim")
+				assert.Equal(t, email, got.payload["contact_email"], "the payload must carry the email verbatim")
+				assert.NoFileExists(t, filepath.Join(got.dir, "pwned"),
+					"a command substitution in the email must not run")
+			})
+		}
+	}
+}
+
+// TestRegistrationScripts_EnvOverridesAndAccountName pins the env-override
+// semantics of the restructured CONTACT_EMAIL assignment (a set, non-empty
+// CUDLY_CONTACT_EMAIL wins; unset or empty falls back to the pre-filled
+// email, as ${VAR:-default} did) and that an operator-supplied
+// CUDLY_ACCOUNT_NAME holding '"', '\' and a tab reaches the payload intact.
+func TestRegistrationScripts_EnvOverridesAndAccountName(t *testing.T) {
+	bashPath, err := exec.LookPath("bash")
+	if err != nil {
+		t.Skip("bash not available")
+	}
+	const prefilled = `"pre}filled"@example.com`
+	const name = "Ops \"prod\" \\ team\tB"
+	for _, sc := range registrationScripts {
+		t.Run(sc.template, func(t *testing.T) {
+			script := renderRegistrationScript(t, sc, prefilled)
+			for _, tc := range []struct {
+				name, wantEmail string
+				env             []string
+			}{
+				{"unset", prefilled, nil},
+				{"empty", prefilled, []string{"CUDLY_CONTACT_EMAIL="}},
+				{"set", `"o}p\s"@example.com`, []string{`CUDLY_CONTACT_EMAIL="o}p\s"@example.com`}},
+			} {
+				got := evalRegistrationWith(t, bashPath, registrationScriptPrelude, script, tc.env...)
+				assert.Equal(t, tc.wantEmail, got.contactEmail, tc.name)
+				assert.Equal(t, tc.wantEmail, got.payload["contact_email"], tc.name)
+			}
+			got := evalRegistrationWith(t, bashPath, registrationScriptPrelude, script, "CUDLY_ACCOUNT_NAME="+name)
+			assert.Equal(t, name, got.payload["account_name"], "account_name must be carried verbatim as JSON")
+		})
+	}
+}
+
+var (
+	// contactEmailInDefaultRe matches the pre-#958 shape: the pre-filled
+	// email as the default of a ${VAR:-...} expansion, where a "}" in it
+	// closes the expansion early.
+	contactEmailInDefaultRe = regexp.MustCompile(`\$\{[A-Za-z_][A-Za-z0-9_]*:-[^}\n]*\{\{\s*\.ContactEmail\s*\}\}`)
+	// rawPayloadFieldRe matches a payload line that puts CONTACT_EMAIL or
+	// ACCOUNT_NAME into the JSON without json_escape.
+	rawPayloadFieldRe = regexp.MustCompile(`"(contact_email|account_name)": "\$\{?(CONTACT_EMAIL|ACCOUNT_NAME)\}?"`)
+	// jsonEscapeFuncRe matches the json_escape helper definition.
+	jsonEscapeFuncRe = regexp.MustCompile(`(?m)^json_escape\(\) \{\n.*\n\}$`)
+)
+
+// TestRegistrationTemplates_NoDrift keeps the shell templates from drifting
+// back: every one that registers with CUDly must be covered by the tests
+// above (or be the Bicep script #958 covers), must not put the contact email
+// inside a ${VAR:-default}, must JSON-escape the email and account name, and
+// must carry the same json_escape helper as the Bicep script, so a fix to one
+// copy cannot miss the others. A new registering template fails here until it
+// is added to registrationScripts.
+func TestRegistrationTemplates_NoDrift(t *testing.T) {
+	covered := map[string]bool{"azure-wif-bicep-deploy.sh.tmpl": true}
+	for _, sc := range registrationScripts {
+		covered[sc.template] = true
+	}
+
+	bicep, err := iacfiles.Templates.ReadFile("templates/azure-wif-bicep-deploy.sh.tmpl")
+	require.NoError(t, err)
+	wantHelper := jsonEscapeFuncRe.FindString(string(bicep))
+	require.NotEmpty(t, wantHelper, "the Bicep deploy script must define json_escape")
+
+	names, err := fs.Glob(iacfiles.Templates, "templates/*.sh.tmpl")
+	require.NoError(t, err)
+	sort.Strings(names)
+	var registering []string
+	for _, name := range names {
+		b, err := iacfiles.Templates.ReadFile(name)
+		require.NoError(t, err)
+		src, base := string(b), path.Base(name)
+		if !strings.Contains(src, "/api/register") {
+			continue
+		}
+		registering = append(registering, base)
+		assert.True(t, covered[base], "%s registers with CUDly but is not in registrationScripts", base)
+		assert.NotRegexp(t, contactEmailInDefaultRe, src, "%s puts the contact email inside a ${VAR:-default}", base)
+		assert.NotRegexp(t, rawPayloadFieldRe, src, "%s puts an unescaped value into the registration JSON", base)
+		assert.Contains(t, src, `"contact_email": "${CONTACT_EMAIL_JSON}"`, base)
+		assert.Contains(t, src, `"account_name": "${ACCOUNT_NAME_JSON}"`, base)
+		assert.Equal(t, wantHelper, jsonEscapeFuncRe.FindString(src), "%s must carry the Bicep script's json_escape", base)
+	}
+	assert.Len(t, registering, len(covered), "registering templates: %v", registering)
+}

internal/api/handler_federation_test.go

@@ -1424,15 +1424,15 @@ func TestGetFederationIaC_CLIMatrix(t *testing.T) {
 			// PII guard: the session email must not appear raw in the script
 			// body outside the CONTACT_EMAIL env-var assignment. The email
 			// is legitimately present as the default value for CONTACT_EMAIL
-			// (e.g. CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-admin@example.com}"),
+			// (e.g. [ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="admin@example.com"),
 			// but it must not be repeated elsewhere (e.g. embedded in a URL,
 			// a hardcoded JSON field, or a curl argument).
 			//
 			// Strategy: strip the CONTACT_EMAIL assignment line and verify
 			// the email does not appear in the remaining content.
 			sessionEmail := "admin@example.com"
 			scriptWithoutAssignment := strings.ReplaceAll(res.Content,
-				`CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-`+sessionEmail+`}"`,
+				`[ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="`+sessionEmail+`"`,
 				"CONTACT_EMAIL_ASSIGNMENT_REMOVED")
 			assert.NotContains(t, scriptWithoutAssignment, sessionEmail,
 				"session email must not appear in CLI script body outside CONTACT_EMAIL assignment (PII leak)")

internal/iacfiles/templates/aws-cfn-deploy.sh.tmpl

@@ -53,7 +53,10 @@ aws cloudformation describe-stacks \
 {{- if .CUDlyAPIURL}}
 echo ""
 echo "Registering account with CUDly..."
-CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-{{.ContactEmail}}}"
+# The pre-filled email is assigned on its own line, not as a ${VAR:-default},
+# so a "}" in it cannot close the expansion early (#958, #1436).
+CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-}"
+[ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="{{.ContactEmail}}"
 # shellcheck disable=SC2086
 TARGET_ACCOUNT_ID=$(aws sts get-caller-identity $PROFILE_ARG --query Account --output text)
 ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-AWS ${TARGET_ACCOUNT_ID}}"
@@ -64,12 +67,21 @@ ROLE_ARN=$(aws cloudformation describe-stacks \
   --stack-name "$STACK_NAME" \
   --query "Stacks[0].Outputs[?OutputKey=='RoleArn'].OutputValue" \
   --output text)
+# json_escape prints $1 escaped for use inside a JSON string: \ and " are
+# backslash-escaped and a tab becomes \t. CR and LF are not handled: the
+# email cannot hold them (net/mail rejects them), and CUDLY_ACCOUNT_NAME is
+# set by the operator running this script.
+json_escape() {
+  printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e "s/$(printf '\t')/\\\\t/g"
+}
+CONTACT_EMAIL_JSON=$(json_escape "$CONTACT_EMAIL")
+ACCOUNT_NAME_JSON=$(json_escape "$ACCOUNT_NAME")
 read -r -d '' PAYLOAD <<JSON || true
 {
   "provider": "aws",
   "external_id": "${TARGET_ACCOUNT_ID}",
-  "account_name": "${ACCOUNT_NAME}",
-  "contact_email": "${CONTACT_EMAIL}",
+  "account_name": "${ACCOUNT_NAME_JSON}",
+  "contact_email": "${CONTACT_EMAIL_JSON}",
   "aws_auth_mode": "workload_identity_federation",
   "aws_role_arn": "${ROLE_ARN}"
 }

internal/iacfiles/templates/aws-cross-account-cfn-deploy.sh.tmpl

@@ -53,7 +53,10 @@ aws cloudformation describe-stacks \
 {{- if .CUDlyAPIURL}}
 echo ""
 echo "Registering account with CUDly..."
-CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-{{.ContactEmail}}}"
+# The pre-filled email is assigned on its own line, not as a ${VAR:-default},
+# so a "}" in it cannot close the expansion early (#958, #1436).
+CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-}"
+[ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="{{.ContactEmail}}"
 # shellcheck disable=SC2086
 TARGET_ACCOUNT_ID=$(aws sts get-caller-identity $PROFILE_ARG --query Account --output text)
 ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-AWS ${TARGET_ACCOUNT_ID}}"
@@ -64,12 +67,21 @@ ROLE_ARN=$(aws cloudformation describe-stacks \
   --stack-name "$STACK_NAME" \
   --query "Stacks[0].Outputs[?OutputKey=='RoleArn'].OutputValue" \
   --output text)
+# json_escape prints $1 escaped for use inside a JSON string: \ and " are
+# backslash-escaped and a tab becomes \t. CR and LF are not handled: the
+# email cannot hold them (net/mail rejects them), and CUDLY_ACCOUNT_NAME is
+# set by the operator running this script.
+json_escape() {
+  printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e "s/$(printf '\t')/\\\\t/g"
+}
+CONTACT_EMAIL_JSON=$(json_escape "$CONTACT_EMAIL")
+ACCOUNT_NAME_JSON=$(json_escape "$ACCOUNT_NAME")
 read -r -d '' PAYLOAD <<JSON || true
 {
   "provider": "aws",
   "external_id": "${TARGET_ACCOUNT_ID}",
-  "account_name": "${ACCOUNT_NAME}",
-  "contact_email": "${CONTACT_EMAIL}",
+  "account_name": "${ACCOUNT_NAME_JSON}",
+  "contact_email": "${CONTACT_EMAIL_JSON}",
   "aws_auth_mode": "role_arn",
   "aws_role_arn": "${ROLE_ARN}",
   "aws_external_id": "${EXTERNAL_ID}"

internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl

@@ -90,16 +90,28 @@ echo "In CUDly, set aws_auth_mode = role_arn and paste the role_arn above."
 {{- if .CUDlyAPIURL}}
 echo ""
 echo "Registering account with CUDly..."
-CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-{{.ContactEmail}}}"
+# The pre-filled email is assigned on its own line, not as a ${VAR:-default},
+# so a "}" in it cannot close the expansion early (#958, #1436).
+CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-}"
+[ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="{{.ContactEmail}}"
 # shellcheck disable=SC2086
 TARGET_ACCOUNT_ID=$(aws sts get-caller-identity $PROFILE_ARG --query Account --output text)
 ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-AWS ${TARGET_ACCOUNT_ID}}"
+# json_escape prints $1 escaped for use inside a JSON string: \ and " are
+# backslash-escaped and a tab becomes \t. CR and LF are not handled: the
+# email cannot hold them (net/mail rejects them), and CUDLY_ACCOUNT_NAME is
+# set by the operator running this script.
+json_escape() {
+  printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e "s/$(printf '\t')/\\\\t/g"
+}
+CONTACT_EMAIL_JSON=$(json_escape "$CONTACT_EMAIL")
+ACCOUNT_NAME_JSON=$(json_escape "$ACCOUNT_NAME")
 read -r -d '' PAYLOAD <<JSON || true
 {
   "provider": "aws",
   "external_id": "${TARGET_ACCOUNT_ID}",
-  "account_name": "${ACCOUNT_NAME}",
-  "contact_email": "${CONTACT_EMAIL}",
+  "account_name": "${ACCOUNT_NAME_JSON}",
+  "contact_email": "${CONTACT_EMAIL_JSON}",
   "aws_auth_mode": "role_arn",
   "aws_role_arn": "${ROLE_ARN}",
   "aws_external_id": "${EXTERNAL_ID}"

internal/iacfiles/templates/aws-wif-cli.sh.tmpl

@@ -158,16 +158,28 @@ echo "In CUDly, set aws_auth_mode = workload_identity_federation and paste the r
 {{- if .CUDlyAPIURL}}
 echo ""
 echo "Registering account with CUDly..."
-CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-{{.ContactEmail}}}"
+# The pre-filled email is assigned on its own line, not as a ${VAR:-default},
+# so a "}" in it cannot close the expansion early (#958, #1436).
+CONTACT_EMAIL="${CUDLY_CONTACT_EMAIL:-}"
+[ -n "$CONTACT_EMAIL" ] || CONTACT_EMAIL="{{.ContactEmail}}"
 # shellcheck disable=SC2086
 TARGET_ACCOUNT_ID=$(aws sts get-caller-identity $PROFILE_ARG --query Account --output text)
 ACCOUNT_NAME="${CUDLY_ACCOUNT_NAME:-AWS ${TARGET_ACCOUNT_ID}}"
+# json_escape prints $1 escaped for use inside a JSON string: \ and " are
+# backslash-escaped and a tab becomes \t. CR and LF are not handled: the
+# email cannot hold them (net/mail rejects them), and CUDLY_ACCOUNT_NAME is
+# set by the operator running this script.
+json_escape() {
+  printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e "s/$(printf '\t')/\\\\t/g"
+}
+CONTACT_EMAIL_JSON=$(json_escape "$CONTACT_EMAIL")
+ACCOUNT_NAME_JSON=$(json_escape "$ACCOUNT_NAME")
 read -r -d '' PAYLOAD <<JSON || true
 {
   "provider": "aws",
   "external_id": "${TARGET_ACCOUNT_ID}",
-  "account_name": "${ACCOUNT_NAME}",
-  "contact_email": "${CONTACT_EMAIL}",
+  "account_name": "${ACCOUNT_NAME_JSON}",
+  "contact_email": "${CONTACT_EMAIL_JSON}",
   "aws_auth_mode": "workload_identity_federation",
   "aws_role_arn": "${ROLE_ARN}"
 }