Merge pull request #430 from warmbly/fix/bootstrap-and-tracking-domain-check

fix: two aws-bootstrap defects, and a check for tracking on your own brand

docs/content/docs/development/instance-health.mdx

@@ -217,6 +217,12 @@ No worker has checked in for more than five minutes while mailboxes are assigned
 
 Check the worker process is running, then check `ENCRYPTED_KEYS_BACKEND_URL` and `ENCRYPTED_KEYS_WORKER_TOKEN`: an empty value lets the worker start, subscribe and never register, with no log line to tell you. See [workers](/development/configuration/#workers).
 
+### tracking_domain_shares_brand
+
+`TRACKING_DOMAIN` is on the same registered domain as `APP_URL` or `API_PUBLIC_URL`. Every campaign's links and unsubscribe pages carry that host, so complaints against any one workspace can get it listed on a URL blocklist, and the listing reaches the domain that also serves your marketing site and your platform mail.
+
+Use a separate registered domain for tracking, and move workspaces onto their own verified tracking domains so their link reputation is theirs rather than shared.
+
 ### fleet_infra_unreachable
 
 Nodes are checking in from more than one machine, and `NATS_URL`, `REDIS` or `ENCRYPTED_KEYS_BACKEND_URL` still names a host that only resolves here, such as a compose service name or loopback. A joining node inherits those values verbatim, so it enrols, keeps heartbeating over HTTP, and reaches neither the bus nor the cache. The fleet looks healthy the whole time, which is why this is a check rather than a log line. See [split deployment](/development/split-deployment/).

internal/app/instancecheck/checks_urls.go

@@ -27,6 +27,7 @@ func urlChecks() []check {
 		{id: "websocket_unreachable", run: checkWebsocketUnreachable},
 		{id: "tracking_domain_unset", run: checkTrackingDomainUnset},
 		{id: "tracking_domain_unreachable", run: checkTrackingDomainUnreachable},
+		{id: "tracking_domain_shares_brand", run: checkTrackingDomainSharesBrand},
 		{id: "app_origin_wildcard", run: checkAppOriginWildcard},
 	}
 }
@@ -181,6 +182,35 @@ func checkTrackingDomainUnset(ctx context.Context, d Deps, in Input) *Finding {
 		docsDelivery)
 }
 
+// checkTrackingDomainSharesBrand catches a tracking host on the same
+// registered domain as the product itself. Every campaign's links and opt-out
+// pages carry that host, so one customer's complaints can get the domain listed
+// on a URL blocklist, and the listing lands on the domain that also serves the
+// marketing site and the platform's own mail.
+func checkTrackingDomainSharesBrand(_ context.Context, _ Deps, _ Input) *Finding {
+	tracking := hostOnly(env("TRACKING_DOMAIN"))
+	if tracking == "" || isLoopbackHost(tracking) {
+		return nil
+	}
+	for _, key := range []string{"APP_URL", "API_PUBLIC_URL"} {
+		other := hostOf(env(key))
+		if other == "" {
+			continue
+		}
+		if registrableDomain(tracking) != registrableDomain(other) {
+			continue
+		}
+		return result(CategoryURLs, SeverityWarning, "The tracking domain is on your own brand's domain",
+			fmt.Sprintf("TRACKING_DOMAIN is %s, which shares the registered domain %s with %s. Every campaign's "+
+				"links and unsubscribe pages carry that host, so complaints against any one workspace can get it "+
+				"listed on a URL blocklist, and the listing reaches the domain serving your site and your platform "+
+				"mail as well. Use a separate registered domain for tracking, and move workspaces onto their own "+
+				"verified tracking domains.", tracking, registrableDomain(tracking), key),
+			docsDelivery)
+	}
+	return nil
+}
+
 func checkTrackingDomainUnreachable(ctx context.Context, d Deps, in Input) *Finding {
 	domain := env("TRACKING_DOMAIN")
 	if domain == "" {

internal/app/instancecheck/checks_urls_tracking_test.go

@@ -0,0 +1,30 @@
+package instancecheck
+
+import "testing"
+
+// The exact misconfiguration this exists for: a tracking host on the brand's
+// own registered domain, where a URL blocklisting earned by one workspace's
+// complaints reaches the site and the platform mail too.
+func TestTrackingDomainSharesBrand(t *testing.T) {
+	t.Setenv("APP_URL", "https://app.warmbly.com")
+	t.Setenv("API_PUBLIC_URL", "https://api.warmbly.com")
+
+	t.Setenv("TRACKING_DOMAIN", "track.warmbly.com")
+	if f := checkTrackingDomainSharesBrand(t.Context(), Deps{}, Input{}); f == nil {
+		t.Error("a tracking host on the brand's registered domain was not reported")
+	}
+
+	// A separate registered domain is the whole point, and must stay silent.
+	t.Setenv("TRACKING_DOMAIN", "t.emberbly.com")
+	if f := checkTrackingDomainSharesBrand(t.Context(), Deps{}, Input{}); f != nil {
+		t.Errorf("a separate tracking domain was reported: %s", f.Message)
+	}
+
+	// Unset is a different finding's job, and localhost is development.
+	for _, v := range []string{"", "localhost:3000"} {
+		t.Setenv("TRACKING_DOMAIN", v)
+		if f := checkTrackingDomainSharesBrand(t.Context(), Deps{}, Input{}); f != nil {
+			t.Errorf("TRACKING_DOMAIN=%q was reported: %s", v, f.Message)
+		}
+	}
+}

scripts/aws-bootstrap.sh

@@ -116,8 +116,20 @@ create_kms() {
 create_bucket() {
   BUCKET="$PREFIX-blobs-$ACCOUNT_ID"
   if aws s3api head-bucket --bucket "$BUCKET" >/dev/null 2>&1; then
-    log "S3: $BUCKET already exists"
-    return 0
+    # Bucket names are global, so head-bucket says nothing about WHERE it is.
+    # Taking that as "already done" left the blobs in the region of a previous
+    # run while everything else moved, which nothing reported.
+    actual=$(aws s3api get-bucket-location --bucket "$BUCKET" \
+      --query 'LocationConstraint' --output text 2>/dev/null)
+    # S3 reports us-east-1 as the literal null, its original default.
+    [ "$actual" = "None" ] && actual="us-east-1"
+    if [ "$actual" = "$REGION" ]; then
+      log "S3: $BUCKET already exists in $REGION"
+      return 0
+    fi
+    die "$BUCKET exists in $actual, not $REGION. A bucket cannot move, and
+       deleting one is not this script's call. Empty and delete it, then re-run;
+       or pass a different --prefix so this region gets its own bucket."
   fi
   log "S3: creating $BUCKET"
   if [ "$DRY_RUN" = "true" ]; then
@@ -267,8 +279,13 @@ create_db() {
     --publicly-accessible \
     --no-multi-az \
     --region "$REGION" >/dev/null
-  log "RDS: creating. The master password is printed once, below."
-  DB_PASSWORD_PRINTED="$DB_PASSWORD"
+  # Never stdout: that is a terminal, a CI log or an agent transcript, and the
+  # value cannot be rotated back out of any of them.
+  pwfile="./${PREFIX}-db-password.txt"
+  ( umask 077; printf '%s\n' "$DB_PASSWORD" > "$pwfile" )
+  chmod 600 "$pwfile"
+  DB_PASSWORD_FILE="$pwfile"
+  log "RDS: creating. The master password was written to $pwfile (mode 0600)."
 }
 
 # ---- SES -------------------------------------------------------------------
@@ -308,10 +325,11 @@ summary() {
   log "  4. the DKIM records above, in DNS"
   log "  5. SES production access; a sandboxed account only delivers to verified"
   log "     addresses"
-  if [ -n "${DB_PASSWORD_PRINTED:-}" ]; then
+  if [ -n "${DB_PASSWORD_FILE:-}" ]; then
     log ""
-    log "  The database master password, shown once:"
-    log "    $DB_PASSWORD_PRINTED"
+    log "  The database master password is in $DB_PASSWORD_FILE."
+    log "  Move it into your secret store and delete the file; this script"
+    log "  cannot show it again, and it was never written to this output."
   fi
   log ""
 }