fix(creds): allow periods in TF_TOKEN_... credentials vars

internal/command/cliconfig/credentials.go

@@ -114,45 +114,77 @@ func (c *Config) credentialsSource(helperType string, helper svcauth.Credentials
 	}
 }
 
+func collectCredentialsFromEnv() map[svchost.Hostname]string {
+	const prefix = "TF_TOKEN_"
+
+	ret := make(map[svchost.Hostname]string)
+	for _, ev := range os.Environ() {
+		eqIdx := strings.Index(ev, "=")
+		if eqIdx < 0 {
+			continue
+		}
+		name := ev[:eqIdx]
+		value := ev[eqIdx+1:]
+		if !strings.HasPrefix(name, prefix) {
+			continue
+		}
+		rawHost := name[len(prefix):]
+
+		// We accept double underscores in place of hyphens because hyphens are not valid
+		// identifiers in most shells and are therefore hard to set.
+		// This is unambiguous with replacing single underscores below because
+		// hyphens are not allowed at the beginning or end of a label and therefore
+		// odd numbers of underscores will not appear together in a valid variable name.
+		rawHost = strings.ReplaceAll(rawHost, "__", "-")
+
+		// We accept underscores in place of dots because dots are not valid
+		// identifiers in most shells and are therefore hard to set.
+		// Underscores are not valid in hostnames, so this is unambiguous for
+		// valid hostnames.
+		rawHost = strings.ReplaceAll(rawHost, "_", ".")
+
+		// Because environment variables are often set indirectly by OS
+		// libraries that might interfere with how they are encoded, we'll
+		// be tolerant of them being given either directly as UTF-8 IDNs
+		// or in Punycode form, normalizing to Punycode form here because
+		// that is what the Terraform credentials helper protocol will
+		// use in its requests.
+		//
+		// Using ForDisplay first here makes this more liberal than Terraform
+		// itself would usually be in that it will tolerate pre-punycoded
+		// hostnames that Terraform normally rejects in other contexts in order
+		// to ensure stored hostnames are human-readable.
+		dispHost := svchost.ForDisplay(rawHost)
+		hostname, err := svchost.ForComparison(dispHost)
+		if err != nil {
+			// Ignore invalid hostnames
+			continue
+		}
+
+		ret[hostname] = value
+	}
+
+	return ret
+}
+
 // hostCredentialsFromEnv returns a token credential by searching for a hostname-specific
 // environment variable. The host parameter is expected to be in the "comparison" form,
 // for example, hostnames containing non-ASCII characters like "café.fr"
 // should be expressed as "xn--caf-dma.fr". If the variable based on the hostname is not
-// defined, nil is returned. Variable names must have dot characters translated to
-// underscores, which are not allowed in DNS names. For example, token credentials
-// for app.terraform.io should be set in the variable named TF_TOKEN_app_terraform_io.
+// defined, nil is returned.
 //
-// Hyphen characters are allowed in environment variable names, but are not valid POSIX
-// variable names. Usually, it's still possible to set variable names with hyphens using
-// utilities like env or docker. But, as a fallback, host names may encode their
-// hyphens as double underscores in the variable name. For the example "café.fr",
-// the variable name "TF_TOKEN_xn____caf__dma_fr" or "TF_TOKEN_xn--caf-dma_fr"
-// may be used.
+// Hyphen and period characters are allowed in environment variable names, but are not valid POSIX
+// variable names. However, it's still possible to set variable names with these characters using
+// utilities like env or docker. Variable names may have periods translated to underscores and
+// hyphens translated to double underscores in the variable name.
+// For the example "café.fr", you may use the variable names "TF_TOKEN_xn____caf__dma_fr",
+// "TF_TOKEN_xn--caf-dma_fr", or "TF_TOKEN_xn--caf-dma.fr"
 func hostCredentialsFromEnv(host svchost.Hostname) svcauth.HostCredentials {
-	if len(host) == 0 {
+	token, ok := collectCredentialsFromEnv()[host]
+	if !ok {
 		return nil
 	}
-
-	// Convert dots to underscores when looking for environment configuration for a specific host.
-	// DNS names do not allow underscore characters so this is unambiguous.
-	translated := strings.ReplaceAll(host.String(), ".", "_")
-
-	if token, ok := os.LookupEnv(fmt.Sprintf("TF_TOKEN_%s", translated)); ok {
-		return svcauth.HostCredentialsToken(token)
-	}
-
-	if strings.ContainsRune(translated, '-') {
-		// This host name contains a hyphen. Replace hyphens with double underscores as a fallback
-		// (see godoc above for details)
-		translated = strings.ReplaceAll(host.String(), "-", "__")
-		translated = strings.ReplaceAll(translated, ".", "_")
-
-		if token, ok := os.LookupEnv(fmt.Sprintf("TF_TOKEN_%s", translated)); ok {
-			return svcauth.HostCredentialsToken(token)
-		}
-	}
-
-	return nil
+	return svcauth.HostCredentialsToken(token)
 }
 
 // CredentialsSource is an implementation of svcauth.CredentialsSource

internal/command/cliconfig/credentials_test.go

@@ -156,6 +156,56 @@ func TestCredentialsForHost(t *testing.T) {
 			t.Errorf("wrong result\ngot: %s\nwant: %s", got, expectedToken)
 		}
 	})
+
+	t.Run("periods are ok", func(t *testing.T) {
+		envName := "TF_TOKEN_configured.example.com"
+		expectedToken := "configured-by-env"
+		t.Cleanup(func() {
+			os.Unsetenv(envName)
+		})
+
+		os.Setenv(envName, expectedToken)
+
+		hostname, _ := svchost.ForComparison("configured.example.com")
+		creds, err := credSrc.ForHost(hostname)
+
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+
+		if creds == nil {
+			t.Fatal("no credentials found")
+		}
+
+		if got := creds.Token(); got != expectedToken {
+			t.Errorf("wrong result\ngot: %s\nwant: %s", got, expectedToken)
+		}
+	})
+
+	t.Run("casing is insensitive", func(t *testing.T) {
+		envName := "TF_TOKEN_CONFIGUREDUPPERCASE_EXAMPLE_COM"
+		expectedToken := "configured-by-env"
+
+		os.Setenv(envName, expectedToken)
+		t.Cleanup(func() {
+			os.Unsetenv(envName)
+		})
+
+		hostname, _ := svchost.ForComparison("configureduppercase.example.com")
+		creds, err := credSrc.ForHost(hostname)
+
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+
+		if creds == nil {
+			t.Fatal("no credentials found")
+		}
+
+		if got := creds.Token(); got != expectedToken {
+			t.Errorf("wrong result\ngot: %s\nwant: %s", got, expectedToken)
+		}
+	})
 }
 
 func TestCredentialsStoreForget(t *testing.T) {