From 065ca8fb3b8db07037ec1c52eb5ec8e01cc714de Mon Sep 17 00:00:00 2001
From: Elbaz <eranelbaz97+github@gmail.com>
Date: Sun, 10 Sep 2023 12:29:40 +0300
Subject: [PATCH] Backport from 1.5.x: Require valid module name (#373)

---
 CHANGELOG.md                                  |  1 +
 internal/initwd/module_install.go             |  8 ++++++++
 internal/initwd/module_install_test.go        | 20 +++++++++++++++++++
 .../invalid-module-name/child/main.tf         |  3 +++
 .../testdata/invalid-module-name/main.tf      |  3 +++
 5 files changed, 35 insertions(+)
 create mode 100644 internal/initwd/testdata/invalid-module-name/child/main.tf
 create mode 100644 internal/initwd/testdata/invalid-module-name/main.tf

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61de1bd844..370dafc627 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@ BUG FIXES:
 * Transitive dependencies were lost during apply when the referenced resource expanded into zero instances ([#33403](https://github.com/hashicorp/terraform/issues/33403))
 * OpenTF will no longer override SSH settings in local git configuration when installing modules. ([#33592](https://github.com/hashicorp/terraform/issues/33592))
 * Handle file-operation errors in `internal/states/statemgr`. ([#278](https://github.com/opentffoundation/opentf/issues/278))
+* `opentf init`: OpenTF will no longer allow downloading remote modules to invalid paths. ([#356](https://github.com/opentffoundation/opentf/issues/356))
 
 ## Previous Releases
 
diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go
index 0fa799a22b..4c9209c9ae 100644
--- a/internal/initwd/module_install.go
+++ b/internal/initwd/module_install.go
@@ -16,6 +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/placeholderplaceholderplaceholder/opentf/internal/addrs"
 	"github.com/placeholderplaceholderplaceholder/opentf/internal/configs"
@@ -162,6 +163,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)
 
diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go
index fda90d2073..15de83d24a 100644
--- a/internal/initwd/module_install_test.go
+++ b/internal/initwd/module_install_test.go
@@ -139,6 +139,26 @@ 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)
diff --git a/internal/initwd/testdata/invalid-module-name/child/main.tf b/internal/initwd/testdata/invalid-module-name/child/main.tf
new file mode 100644
index 0000000000..347fbbc13b
--- /dev/null
+++ b/internal/initwd/testdata/invalid-module-name/child/main.tf
@@ -0,0 +1,3 @@
+output "boop" {
+  value = "beep"
+}
\ No newline at end of file
diff --git a/internal/initwd/testdata/invalid-module-name/main.tf b/internal/initwd/testdata/invalid-module-name/main.tf
new file mode 100644
index 0000000000..fcc5b8d1c9
--- /dev/null
+++ b/internal/initwd/testdata/invalid-module-name/main.tf
@@ -0,0 +1,3 @@
+module "../invalid" {
+  source  = "./child"
+}
\ No newline at end of file