initwd: require valid module name (#33745)

We install remote modules prior to showing any validation errors during init
so that we can show errors about the core version requirement before we do
anything else. Unfortunately, this means that we don't validate module names
until after remote modules have been installed, which may cause unexpected
problems if we can't convert the module name into a valid path.

Author: radditude

internal/initwd/module_install.go

@@ -16,7 +16,7 @@ import (
 	"github.com/apparentlymart/go-versions/versions"
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/hcl/v2"
-
+	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
 	"github.com/hashicorp/terraform/internal/configs/configload"
@@ -162,6 +162,13 @@ func (i *ModuleInstaller) moduleInstallWalker(ctx context.Context, manifest mods
 				return nil, nil, diags
 			}
 
+			if !hclsyntax.ValidIdentifier(req.Name) {
+				// A module with an invalid name shouldn't be installed at all. This is
+				// mostly a concern for remote modules, since we need to be able to convert
+				// the name to a valid path.
+				return nil, nil, diags
+			}
+
 			key := manifest.ModuleKey(req.Path)
 			instPath := i.packageInstallPath(req.Path)
 

internal/initwd/module_install_test.go

@@ -140,6 +140,27 @@ func TestModuleInstaller_emptyModuleName(t *testing.T) {
 	}
 }
 
+func TestModuleInstaller_invalidModuleName(t *testing.T) {
+	fixtureDir := filepath.Clean("testdata/invalid-module-name")
+	dir, done := tempChdir(t, fixtureDir)
+	defer done()
+
+	hooks := &testInstallHooks{}
+
+	modulesDir := filepath.Join(dir, ".terraform/modules")
+
+	loader, close := configload.NewLoaderForTests(t)
+	defer close()
+	inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil))
+	_, diags := inst.InstallModules(context.Background(), dir, "tests", false, false, hooks)
+
+	if !diags.HasErrors() {
+		t.Fatal("expected error")
+	} else {
+		assertDiagnosticSummary(t, diags, "Invalid module instance name")
+	}
+}
+
 func TestModuleInstaller_packageEscapeError(t *testing.T) {
 	fixtureDir := filepath.Clean("testdata/load-module-package-escape")
 	dir, done := tempChdir(t, fixtureDir)

internal/initwd/testdata/invalid-module-name/child/main.tf

@@ -0,0 +1,3 @@
+output "boop" {
+  value = "beep"
+}

internal/initwd/testdata/invalid-module-name/main.tf

@@ -0,0 +1,3 @@
+module "../invalid" {
+  source  = "./child"
+}