[Backport release-1.9] Fix: prevent unbounded read in Terraform remote configuration loader (GHSA-fmgp-q6jx-gg3x) (#7192)

* Merge commit from fork * fix: prevent unbounded read in Terraform remote configuration loader (GHSA-fmgp-q6jx-gg3x) * fix: bound remote Terraform clone and invalidate cache on rejection Follow-up hardening for GHSA-fmgp-q6jx-gg3x. Bound the clone of the attacker-supplied repository: shallow Depth:1, a 2-minute fetch timeout via PlainCloneContext, and post-clone caps on the retained tree size (64 MiB) and file count, rejecting and removing a clone that exceeds them. Invalidate the clone cache: re-clone when the recorded remote URL changes, and remove the cache on a failed clone or a rejected read so a corrected repository is re-fetched instead of a poisoned or stale tree being reused. Validate the module name before building the cache path, and log clone, rejection, and eviction events. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: resolve gosec G304 lint failure in Terraform module cache check (#7190) Wrap the cache remote marker read in filepath.Clean, the same pattern other os.ReadFile call sites in this repo use to satisfy gosec. The path is built from filepath.Join and a constant suffix, with the module name validated beforehand, so behavior is unchanged. The finding surfaced on master after the GHSA-fmgp-q6jx-gg3x merge because the advisory workflow did not run the full lint job. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Chore: bump actions/cache to v4 in CI workflows GitHub has closed down actions/cache v1 and v2. Any job pinned to the old SHA (704facf57e6136b1bc63b828d79edcd491f0ee84) is now automatically failed at job setup, before any step runs. On this branch that broke check-diff, check-windows, and unit-tests (each failing in a few seconds with no logs). Bump the three references in go.yml and unit-test.yml to actions/cache@v4, matching the version already used on master, so these jobs can run again. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Chore: install envtest binaries via setup-envtest in unit tests The RyanSiu1995/kubebuilder-action step fetched kube-apiserver and kubectl from the kubernetes-release and kubebuilder-tools storage buckets, which have since been retired. Those downloads now return small 404 error pages that get saved as the binaries, so envtest cannot start the control plane (exec format error) and the unit-test BeforeSuite panics. Replace that step with the official prebuilt setup-envtest, which pulls the matching envtest bundle (etcd, kube-apiserver, kubectl) for Kubernetes 1.26.1 from the current controller-runtime release index and exports its path through KUBEBUILDER_ASSETS. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Chore: pin cache action and harden envtest install per review Address automated review feedback on the CI changes: - Pin actions/cache to a commit SHA (v4.3.0, 0057852) in go.yml and unit-test.yml instead of the mutable v4 tag, matching how every other action in these workflows is pinned. - Add curl -f to the setup-envtest download so an HTTP error fails the step immediately instead of saving an error page as the binary. - Verify the downloaded setup-envtest against a known SHA-256 before running it. - Capture the envtest asset path into a variable and fail fast when it is empty or not a directory, rather than letting a failed command substitution slip through and surface later as a confusing make test error. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: load 0.2.0 helm test chart from local file instead of dead repo The helm helper test fixture pointed version 0.2.0 at https://charts.kubevela.net/example/autoscalertrait-0.1.0.tgz, but that host no longer resolves. Once the unit-test suite could run again, "Test getValues from chart" failed with "cannot load chart from chart repo". Point the 0.2.0 entry at the local autoscalertrait-0.2.0.tgz that already ships in testdata, matching how master resolves this chart, so the test no longer depends on an external network endpoint. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: point addon CLI tests at the live KubeVela registry The addon listing and status tests registered https://addons.kubevela.net, which no longer resolves, so the three "addon in the registry" cases failed once the suite could run again. Point them at https://kubevela.github.io/catalog/official, the registry that master already uses for these same tests. The only difference between this file and master was this URL (the assertions are identical), and that host still serves the addons the assertions expect. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Chore: re-trigger CI for stuck e2e jobs Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: run release-1.9 e2e jobs on GitHub-hosted runners The e2e-tests and e2e-multi-cluster-tests jobs targeted self-hosted runners, which GitHub does not assign to pull requests from forks. The jobs sat queued until the 24h ceiling and were cancelled, so they never ran on this PR (the original run shows no runner assigned and zero steps executed). Switch both to ubuntu-22.04, the GitHub-hosted runner that release-1.10 and master already use for these jobs, where the same fork-based backport runs them successfully. These workflows are self-contained (they install their own tools and create the kind cluster inline), so no other change is needed. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: wait for flux controllers before enabling the terraform e2e addon The e2e post-hook enabled the terraform addon right after fluxcd. The terraform addon's controller is a flux HelmRelease that needs the flux source-controller and helm-controller pods running to reconcile, but flux readiness was only checked after every addon had been enabled. On the GitHub-hosted runner (slower than the self-hosted one this hook was written for) the terraform addon enable timed out after 600s waiting on a reconcile that could not happen until flux was up. Add the flux-system readiness checks before the terraform addon enable so flux is reconciling before the addon that depends on it is applied. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: enable terraform e2e addon with terraform-controller 0.8.0 The terraform addon enable timed out after 600s on the GitHub-hosted runner. A previous attempt that waited for the flux controllers to be Ready before enabling the addon did not help: the run logs confirm source-controller and helm-controller were Ready and the terraform addon still timed out, so flux readiness was not the cause. Revert that wait. The real difference from master, where this addon enables cleanly, is the terraform-controller chart: release-1.9 pinned 0.2.11 from charts.kubevela.net, while master uses 0.8.0 from kubevela.github.io/charts with the ghcr image. Backport that chart version and image override so the addon controller becomes healthy in time. Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> * Fix: do not log or persist credentials embedded in Terraform module remote URLs GetTerraformConfigurationFromRemote logged the raw remote URL and wrote it to the .remote-url cache marker. An authenticated Git URL such as https://user:token@host/repo.git embeds credentials in its userinfo, so the raw value leaked secrets into controller logs and onto disk. Strip any embedded userinfo with a new redactURLCredentials helper before the URL is logged or recorded, and compare against the stripped form when checking the cache marker so cache reuse still works. scp-style SSH URLs authenticate with keys and carry no secret, so they pass through unchanged. See GHSA-fmgp-q6jx-gg3x. Signed-off-by: Ayush Kumar <65535504+roguepikachu@users.noreply.github.com> * Fix: discover traits from a reachable registry in the e2e raw-url test The e2e registry test discovered traits from oss://registry.kubevela.net, whose TLS certificate expired in May 2025, so e2e-tests failed with a certificate verification error. The default-registry listing test hits the same expired endpoint. Match release-1.10: point raw-url discovery at the GitHub-hosted registry, and disable the default-registry listing until the default registry is updated. Signed-off-by: Ayush Kumar <65535504+roguepikachu@users.noreply.github.com> --------- Signed-off-by: Ayush Kumar <ayushshyamkumar888@gmail.com> Signed-off-by: Ayush Kumar <65535504+roguepikachu@users.noreply.github.com>
2026-08-18 03:56:36 +00:00 · 2026-06-25 20:50:35 -07:00
parent 02e13f9dc3
commit 7a4e59b295
12 changed files with 649 additions and 42 deletions
@@ -39,7 +39,7 @@ jobs:
        continue-on-error: true

  e2e-multi-cluster-tests:
-    runs-on: self-hosted
+    runs-on: ubuntu-22.04
    needs: [ detect-noop ]
    if: needs.detect-noop.outputs.noop != 'true'
    strategy:
@@ -39,7 +39,7 @@ jobs:
        continue-on-error: true

  e2e-tests:
-    runs-on: self-hosted
+    runs-on: ubuntu-22.04
    needs: [ detect-noop ]
    if: needs.detect-noop.outputs.noop != 'true'
    strategy:
@@ -109,7 +109,7 @@ jobs:
          node-version: '14'

      - name: Cache Go Dependencies
-        uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
        with:
          path: .work/pkg
          key: ${{ runner.os }}-pkg-${{ hashFiles('**/go.sum') }}
@@ -149,7 +149,7 @@ jobs:
          go-version: ${{ env.GO_VERSION }}

      - name: Cache Go Dependencies
-        uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
        with:
          path: .work/pkg
          key: ${{ runner.os }}-pkg-${{ hashFiles('**/go.sum') }}
@@ -53,7 +53,7 @@ jobs:
          submodules: true

      - name: Cache Go Dependencies
-        uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
        with:
          path: .work/pkg
          key: ${{ runner.os }}-pkg-${{ hashFiles('**/go.sum') }}
@@ -70,12 +70,17 @@ jobs:
          go install sigs.k8s.io/kind@v0.19.0
          kind create cluster

-      - name: install Kubebuilder
-        uses: RyanSiu1995/kubebuilder-action@7170cb0476187070ae04cbb6cee305e809de2693
-        with:
-          version: 3.9.1
-          kubebuilderOnly: false
-          kubernetesVersion: v1.26.2
+      - name: Install envtest binaries
+        run: |
+          curl -fsSLo setup-envtest https://github.com/kubernetes-sigs/controller-runtime/releases/download/v0.19.7/setup-envtest-linux-amd64
+          echo "1b8a93325f5dfe539bb8d6175fe25587fb63121509c2114f670d66849abf24c9  setup-envtest" | sha256sum -c -
+          chmod +x setup-envtest
+          assets_path=$(./setup-envtest use 1.26.1 -p path)
+          if [ -z "$assets_path" ] || [ ! -d "$assets_path" ]; then
+            echo "failed to resolve envtest binary path"
+            exit 1
+          fi
+          echo "KUBEBUILDER_ASSETS=$assets_path" >> "$GITHUB_ENV"

      - name: Run Make test
        run: make test
@@ -24,7 +24,11 @@ spec:
      type: helm
      properties:
        repoType: helm
-        url: https://charts.kubevela.net/addons
+        url: https://kubevela.github.io/charts
        chart: terraform-controller
-        version: 0.2.11
+        version: 0.8.0
+        values:
+          image:
+            repository: ghcr.io/kubevela/oamdev/terraform-controller
+            tag: v0.8.0

@@ -87,7 +87,8 @@ var _ = Describe("test registry and trait/comp command", func() {
 			Expect(output).To(ContainSubstring("pvc"))
 			Expect(output).To(ContainSubstring("[deployments.apps]"))
 		})
-		It("list trait from default registry", func() {
+		// TODO: enable this test after the default registry has been updated
+		XIt("list trait from default registry", func() {
 			cli := "vela trait --discover"
 			output, err := e2e.Exec(cli)
 			Expect(err).NotTo(HaveOccurred())
@@ -100,10 +101,10 @@ var _ = Describe("test registry and trait/comp command", func() {
 		})

 		It("test list trait in raw url", func() {
-			cli := "vela trait --discover --url=oss://registry.kubevela.net"
+			cli := "vela trait --discover --url=https://github.com/kubevela/kubevela/tree/master/vela-templates/registry/auto-gen/"
 			output, err := e2e.Exec(cli)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(output).To(SatisfyAll(ContainSubstring("Showing trait definition from url"), ContainSubstring("oss://registry.kubevela.net")))
+			Expect(output).To(SatisfyAll(ContainSubstring("Showing trait definition from url"), ContainSubstring("https://github.com/kubevela/kubevela/tree/master/vela-templates/registry/auto-gen/")))
 		})

 	})
@@ -20,9 +20,12 @@ package utils
 import (
 	"context"
 	"fmt"
+	"io"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
+	"time"

 	"github.com/getkin/kin-openapi/openapi3"
 	"github.com/go-git/go-git/v5"
@@ -205,41 +208,266 @@ func GetOpenAPISchemaFromTerraformComponentDefinition(configuration string) ([]b

 // GetTerraformConfigurationFromRemote gets Terraform Configuration(HCL)
 func GetTerraformConfigurationFromRemote(name, remoteURL, remotePath string, sshPublicKey *gitssh.PublicKeys) (string, error) {
+	if err := validateModuleName(name); err != nil {
+		return "", err
+	}
 	userHome, err := os.UserHomeDir()
 	if err != nil {
 		return "", err
 	}
 	cachePath := filepath.Join(userHome, ".vela", "terraform", name)
-	// Check if the directory exists. If yes, remove it.
-	entities, err := os.ReadDir(cachePath)
-	if err != nil || len(entities) == 0 {
-		fmt.Printf("loading terraform module %s into %s from %s\n", name, cachePath, remoteURL)
-		cloneOptions := &git.CloneOptions{
-			URL:      remoteURL,
-			Progress: os.Stdout,
-		}
-		if sshPublicKey != nil {
-			cloneOptions.Auth = sshPublicKey
-		}
-		if _, err = git.PlainClone(cachePath, false, cloneOptions); err != nil {
+
+	// Reuse the cache only when it is populated AND was cloned from the same URL.
+	// Otherwise (empty, a changed URL, or a missing marker) re-clone, so a
+	// definition repointed at a different repository does not keep serving the old
+	// tree. See GHSA-fmgp-q6jx-gg3x.
+	if !cacheMatchesRemote(cachePath, remoteURL) {
+		_ = os.RemoveAll(cachePath)
+		klog.InfoS("cloning remote Terraform module", "module", name, "url", redactURLCredentials(remoteURL), "cache", cachePath)
+		if err = cloneTerraformModule(cachePath, remoteURL, sshPublicKey); err != nil {
+			// Do not leave a partial or oversized clone behind for the next reconcile.
+			klog.ErrorS(err, "failed to clone remote Terraform module", "module", name, "url", redactURLCredentials(remoteURL))
+			_ = os.RemoveAll(cachePath)
 			return "", err
 		}
+		recordCacheRemote(cachePath, remoteURL)
 	}
 	sshKnownHostsPath := os.Getenv("SSH_KNOWN_HOSTS")
 	_ = os.Remove(sshKnownHostsPath)

-	tfPath := filepath.Join(cachePath, remotePath, "variables.tf")
-	if _, err := os.Stat(tfPath); err != nil {
-		tfPath = filepath.Join(cachePath, remotePath, "main.tf")
-		if _, err := os.Stat(tfPath); err != nil {
-			return "", errors.Wrap(err, "failed to find main.tf or variables.tf in Terraform configurations of the remote repository")
+	conf, err := readTerraformConfigFromDir(cachePath, remotePath)
+	if err != nil {
+		// Drop the cached clone on any rejection or read failure so a corrected or
+		// replaced repository is re-cloned next time, rather than the controller
+		// re-reading a poisoned or stale tree forever. Wiping on a transient read
+		// error only costs one bounded re-clone, which beats risking a stuck poison.
+		klog.InfoS("evicting Terraform module cache after a failed read", "module", name, "cache", cachePath, "err", err.Error())
+		_ = os.RemoveAll(cachePath)
+		return "", err
+	}
+	return conf, nil
+}
+
+// cacheRemoteMarkerSuffix names the sibling file that records which remote URL a
+// cached Terraform module was cloned from, used to detect a changed URL. The URL is
+// stored credential-stripped so the marker never persists secrets.
+const cacheRemoteMarkerSuffix = ".remote-url"
+
+// validateModuleName rejects names that could steer the cache path (and the
+// os.RemoveAll that cleans it) outside the module cache directory. Callers pass a
+// Kubernetes object name, but the check keeps that guarantee local and explicit.
+func validateModuleName(name string) error {
+	if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\`) {
+		return errors.Errorf("invalid Terraform module name %q", name)
+	}
+	return nil
+}
+
+// cacheMatchesRemote reports whether cachePath holds a populated clone recorded
+// as coming from remoteURL. The marker stores a credential-stripped URL, so the
+// comparison strips remoteURL the same way before matching.
+func cacheMatchesRemote(cachePath, remoteURL string) bool {
+	entities, err := os.ReadDir(cachePath)
+	if err != nil || len(entities) == 0 {
+		return false
+	}
+	recorded, err := os.ReadFile(filepath.Clean(cachePath + cacheRemoteMarkerSuffix))
+	if err != nil {
+		return false
+	}
+	return string(recorded) == redactURLCredentials(remoteURL)
+}
+
+// recordCacheRemote records the credential-stripped remote URL a freshly cloned
+// module came from, so the marker never persists secrets to disk.
+func recordCacheRemote(cachePath, remoteURL string) {
+	if err := os.WriteFile(cachePath+cacheRemoteMarkerSuffix, []byte(redactURLCredentials(remoteURL)), 0600); err != nil {
+		klog.ErrorS(err, "failed to record Terraform module remote URL", "cache", cachePath)
+	}
+}
+
+// redactURLCredentials strips any embedded userinfo (a username, password, or token)
+// from a URL so it is safe to log and persist. Authenticated HTTPS Git URLs can carry
+// credentials in the userinfo; scp-style SSH URLs ("git@host:path") fail url.Parse but
+// authenticate with keys, so they carry no secret. The same stripped form is used for
+// logging and for the cache marker, so record and match stay consistent.
+func redactURLCredentials(raw string) string {
+	if u, err := url.Parse(raw); err == nil {
+		u.User = nil
+		return u.String()
+	}
+	// url.Parse failed (typically scp-style SSH). Strip a leading "user@" defensively
+	// in case a malformed URL still embeds credentials before the host.
+	if _, after, found := strings.Cut(raw, "@"); found {
+		return after
+	}
+	return raw
+}
+
+const (
+	// cloneTimeout bounds the network fetch of a remote Terraform module clone, so a
+	// slow or oversized attacker-supplied repository cannot hang the controller. It
+	// covers the git transport only; the worktree checkout that follows is not bound
+	// by this deadline.
+	cloneTimeout = 2 * time.Minute
+	// maxCloneBytes and maxCloneEntries cap the on-disk footprint of a cloned remote
+	// Terraform module (modules are small). They run AFTER the clone completes, so
+	// they bound the RETAINED clone (an oversized clone is rejected and removed by
+	// the caller) rather than peak transient disk during transfer/checkout; Depth:1
+	// keeps that transient footprint to a single shallow commit. Fully bounding peak
+	// transfer disk (NoCheckout plus object-store reads, or an instrumented
+	// filesystem) is a follow-up. See GHSA-fmgp-q6jx-gg3x.
+	maxCloneBytes int64 = 64 << 20 // 64 MiB
+	// maxCloneEntries counts every filesystem entry (files, directories, and
+	// symlinks), so a tree of many empty directories or symlinks cannot bypass the
+	// bound and exhaust inodes.
+	maxCloneEntries = 50000
+)
+
+// cloneTerraformModule shallow-clones remoteURL into cachePath under a fetch
+// timeout, then rejects an oversized clone. Depth:1 drops history, the timeout
+// bounds the network fetch, and the post-clone caps reject a clone whose retained
+// size or entry count is too large (the caller removes the rejected clone).
+func cloneTerraformModule(cachePath, remoteURL string, sshPublicKey *gitssh.PublicKeys) error {
+	ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout)
+	defer cancel()
+
+	cloneOptions := &git.CloneOptions{
+		URL:      remoteURL,
+		Progress: os.Stdout,
+		Depth:    1,
+	}
+	if sshPublicKey != nil {
+		cloneOptions.Auth = sshPublicKey
+	}
+	if _, err := git.PlainCloneContext(ctx, cachePath, false, cloneOptions); err != nil {
+		return errors.Wrap(err, "failed to clone remote Terraform configuration")
+	}
+	return ensureDirWithinLimits(cachePath, maxCloneBytes, maxCloneEntries)
+}
+
+// ensureDirWithinLimits returns an error as soon as the entries under root exceed
+// maxEntries in count or the regular files exceed maxBytes in total size. Every
+// entry (regular files, directories, and symlinks) counts toward maxEntries, so a
+// tree of many directories or symlinks cannot slip past the bound and exhaust
+// inodes; only regular files contribute to the byte total. It counts everything
+// under root (including .git, by design) and does not follow symlinks. The caps
+// bound the retained clone, not peak disk during the clone (see maxCloneBytes).
+func ensureDirWithinLimits(root string, maxBytes int64, maxEntries int) error {
+	var (
+		total   int64
+		entries int
+	)
+	return filepath.WalkDir(root, func(_ string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		entries++
+		if entries > maxEntries {
+			return errors.Errorf("remote Terraform repository exceeds the maximum allowed entry count of %d", maxEntries)
+		}
+		if !d.Type().IsRegular() {
+			return nil
+		}
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+		total += info.Size()
+		if total > maxBytes {
+			return errors.Errorf("remote Terraform repository exceeds the maximum allowed size of %d bytes", maxBytes)
+		}
+		return nil
+	})
+}
+
+// maxTerraformConfigBytes caps how much of a remote Terraform configuration
+// file the loader will read. HCL configurations are small, so the cap stops a
+// malicious or compromised repository from steering the read at an unbounded
+// source (for example a variables.tf symlinked to /dev/zero) and OOM-killing
+// the controller. See GHSA-fmgp-q6jx-gg3x.
+const maxTerraformConfigBytes int64 = 1 << 20 // 1 MiB
+
+// errTerraformConfigTooLarge builds the error returned when a candidate
+// configuration file exceeds maxTerraformConfigBytes.
+func errTerraformConfigTooLarge(path string, maxSize int64) error {
+	return errors.Errorf("refusing to read Terraform configuration %q: exceeds the maximum allowed size of %d bytes", path, maxSize)
+}
+
+// readTerraformConfigFromDir reads variables.tf, or main.tf as a fallback, from
+// the cloned remote repository at cacheRoot/remotePath. It refuses any candidate
+// that is not a regular file contained within cacheRoot (defeating symlink and
+// path-traversal escapes) and any file larger than maxTerraformConfigBytes.
+func readTerraformConfigFromDir(cacheRoot, remotePath string) (string, error) {
+	baseDir := filepath.Join(cacheRoot, remotePath)
+	for _, name := range []string{"variables.tf", "main.tf"} {
+		content, found, err := readContainedRegularFile(cacheRoot, filepath.Join(baseDir, name), maxTerraformConfigBytes)
+		if err != nil {
+			return "", err
+		}
+		if found {
+			return content, nil
 		}
 	}
-	conf, err := os.ReadFile(filepath.Clean(tfPath))
+	return "", errors.New("failed to find main.tf or variables.tf in Terraform configurations of the remote repository")
+}
+
+// readContainedRegularFile reads path when it is a regular file whose real
+// location stays inside root and whose size is within maxSize. found is false
+// only when path does not exist, so callers can try a fallback name; any other
+// problem (a symlink or traversal escape, an irregular file, or an oversize
+// file) is a hard error rather than a silent fallback.
+func readContainedRegularFile(root, path string, maxSize int64) (content string, found bool, err error) {
+	info, err := os.Lstat(path)
 	if err != nil {
-		return "", errors.Wrap(err, "failed to read Terraform configuration")
+		if os.IsNotExist(err) {
+			return "", false, nil
+		}
+		return "", false, errors.Wrap(err, "failed to stat Terraform configuration")
 	}
-	return string(conf), nil
+	// Reject any symlink at the target outright, even one that resolves inside
+	// the cache. This is intentionally stricter than the containment check
+	// below: variables.tf/main.tf are expected to be regular files, so a blanket
+	// rejection is the simplest defense against the GHSA-fmgp-q6jx-gg3x
+	// symlink-to-/dev/zero vector. The containment check still guards a symlinked
+	// parent directory whose leaf is itself a regular file.
+	if info.Mode()&os.ModeSymlink != 0 {
+		return "", true, errors.Errorf("refusing to read symlinked Terraform configuration %q", path)
+	}
+
+	realRoot, err := filepath.EvalSymlinks(root)
+	if err != nil {
+		return "", true, errors.Wrap(err, "failed to resolve Terraform cache directory")
+	}
+	realPath, err := filepath.EvalSymlinks(path)
+	if err != nil {
+		return "", true, errors.Wrap(err, "failed to resolve Terraform configuration path")
+	}
+	if realPath != realRoot && !strings.HasPrefix(realPath, realRoot+string(os.PathSeparator)) {
+		return "", true, errors.Errorf("refusing to read Terraform configuration outside the cache directory: %q", path)
+	}
+
+	if !info.Mode().IsRegular() {
+		return "", true, errors.Errorf("refusing to read non-regular Terraform configuration %q", path)
+	}
+	if info.Size() > maxSize {
+		return "", true, errTerraformConfigTooLarge(path, maxSize)
+	}
+
+	f, err := os.Open(filepath.Clean(path))
+	if err != nil {
+		return "", true, errors.Wrap(err, "failed to read Terraform configuration")
+	}
+	defer func() { _ = f.Close() }()
+
+	buf, err := io.ReadAll(io.LimitReader(f, maxSize+1))
+	if err != nil {
+		return "", true, errors.Wrap(err, "failed to read Terraform configuration")
+	}
+	if int64(len(buf)) > maxSize {
+		return "", true, errTerraformConfigTooLarge(path, maxSize)
+	}
+	return string(buf), true, nil
 }

 func parseOtherProperties4TerraformDefinition() map[string]*openapi3.Schema {
@@ -104,14 +104,21 @@ variable "aaa" {
 		t.Run(name, func(t *testing.T) {
 			home, _ := os.UserHomeDir()
 			path := filepath.Join(home, ".vela", "terraform")
+			cacheRoot := filepath.Join(path, tc.args.name)
 			tmpPath := filepath.Join(path, tc.args.name, tc.args.path)
 			if len(tc.args.data) > 0 {
 				err := os.MkdirAll(tmpPath, os.ModePerm)
 				assert.NoError(t, err)
 				err = os.WriteFile(filepath.Clean(filepath.Join(tmpPath, tc.args.variableFile)), tc.args.data, 0644)
 				assert.NoError(t, err)
+				// Record the remote URL so the populated cache is reused. URL-keyed
+				// reuse was added for GHSA-fmgp-q6jx-gg3x; without the marker the
+				// loader would treat the cache as stale and attempt a real clone.
+				err = os.WriteFile(cacheRoot+cacheRemoteMarkerSuffix, []byte(tc.args.url), 0600)
+				assert.NoError(t, err)
 			}
-			defer os.RemoveAll(tmpPath)
+			defer os.RemoveAll(cacheRoot)
+			defer os.Remove(cacheRoot + cacheRemoteMarkerSuffix)

 			conf, err := GetTerraformConfigurationFromRemote(tc.args.name, tc.args.url, tc.args.path, nil)
 			if tc.want.errMsg != "" {
@@ -0,0 +1,47 @@
+//go:build unix
+
+/*
+ Copyright 2026 The KubeVela Authors.
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+
+*/
+
+package utils
+
+import (
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestReadTerraformConfigFromDirNonRegular guards the non-regular-file branch
+// of the GHSA-fmgp-q6jx-gg3x fix: a non-symlink irregular file (a FIFO here)
+// must be refused before any read, since reading it could block or stream
+// without bound. This branch is what catches a non-symlink path to an
+// unbounded source, complementing the symlink rejection.
+func TestReadTerraformConfigFromDirNonRegular(t *testing.T) {
+	dir := t.TempDir()
+	fifo := filepath.Join(dir, "variables.tf")
+	if err := syscall.Mkfifo(fifo, 0600); err != nil {
+		t.Skipf("mkfifo unsupported on this platform: %v", err)
+	}
+	defer func() { _ = os.Remove(fifo) }()
+
+	_, err := readTerraformConfigFromDir(dir, "")
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "non-regular")
+}
@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"

@@ -535,3 +536,317 @@ func TestGetGitSSHPublicKey(t *testing.T) {
 		})
 	}
 }
+
+// TestReadTerraformConfigFromDir guards the remote Terraform loader against
+// GHSA-fmgp-q6jx-gg3x: a variables.tf (or a parent directory) that symlinks
+// outside the clone cache, and an oversized configuration file. The loader
+// must read normal regular files but refuse anything that escapes the cache
+// or exceeds the size cap, before any content is returned.
+func TestReadTerraformConfigFromDir(t *testing.T) {
+	// A sentinel file outside the clone cache. A successful escape would leak
+	// its content; the guards must prevent that.
+	outside := t.TempDir()
+	secretPath := filepath.Join(outside, "secret.txt")
+	assert.NoError(t, os.WriteFile(secretPath, []byte("top-secret-host-data"), 0600))
+
+	t.Run("reads variables.tf", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "variables.tf"), []byte(`variable "x" {}`), 0600))
+		got, err := readTerraformConfigFromDir(dir, "")
+		assert.NoError(t, err)
+		assert.Equal(t, `variable "x" {}`, got)
+	})
+
+	t.Run("falls back to main.tf", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "main.tf"), []byte(`resource "x" "y" {}`), 0600))
+		got, err := readTerraformConfigFromDir(dir, "")
+		assert.NoError(t, err)
+		assert.Equal(t, `resource "x" "y" {}`, got)
+	})
+
+	t.Run("errors when neither file is present", func(t *testing.T) {
+		dir := t.TempDir()
+		_, err := readTerraformConfigFromDir(dir, "")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "failed to find")
+	})
+
+	t.Run("refuses a variables.tf that symlinks outside the cache", func(t *testing.T) {
+		dir := t.TempDir()
+		// This is the advisory's attack: variables.tf -> a path outside the clone.
+		assert.NoError(t, os.Symlink(secretPath, filepath.Join(dir, "variables.tf")))
+		got, err := readTerraformConfigFromDir(dir, "")
+		assert.Error(t, err)
+		assert.NotContains(t, got, "top-secret-host-data")
+	})
+
+	t.Run("refuses a symlinked subdirectory that escapes the cache", func(t *testing.T) {
+		dir := t.TempDir()
+		outDir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(outDir, "variables.tf"), []byte("secret-config"), 0600))
+		assert.NoError(t, os.Symlink(outDir, filepath.Join(dir, "evil")))
+		got, err := readTerraformConfigFromDir(dir, "evil")
+		assert.Error(t, err)
+		assert.NotContains(t, got, "secret-config")
+	})
+
+	t.Run("refuses a file larger than the size cap", func(t *testing.T) {
+		dir := t.TempDir()
+		big := make([]byte, maxTerraformConfigBytes+1024)
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "variables.tf"), big, 0600))
+		_, err := readTerraformConfigFromDir(dir, "")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "exceeds the maximum")
+	})
+
+	t.Run("accepts a file exactly at the size cap", func(t *testing.T) {
+		dir := t.TempDir()
+		atCap := make([]byte, maxTerraformConfigBytes)
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "variables.tf"), atCap, 0600))
+		got, err := readTerraformConfigFromDir(dir, "")
+		assert.NoError(t, err)
+		assert.Len(t, got, int(maxTerraformConfigBytes))
+	})
+
+	t.Run("refuses a file one byte over the size cap", func(t *testing.T) {
+		dir := t.TempDir()
+		overCap := make([]byte, maxTerraformConfigBytes+1)
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "variables.tf"), overCap, 0600))
+		_, err := readTerraformConfigFromDir(dir, "")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "exceeds the maximum")
+	})
+
+	t.Run("refuses lexical ../ traversal via remotePath with no symlink", func(t *testing.T) {
+		dir := t.TempDir()
+		// A real variables.tf in a sibling of the cache dir, reached purely by a
+		// cleaned ".." in remotePath, with no symlink anywhere. The containment
+		// check (not the symlink check) must reject this.
+		sibling := filepath.Join(filepath.Dir(dir), "sibling-"+filepath.Base(dir))
+		assert.NoError(t, os.MkdirAll(sibling, 0700))
+		defer func() { _ = os.RemoveAll(sibling) }()
+		assert.NoError(t, os.WriteFile(filepath.Join(sibling, "variables.tf"), []byte("escaped-config"), 0600))
+		got, err := readTerraformConfigFromDir(dir, "../"+filepath.Base(sibling))
+		assert.Error(t, err)
+		assert.NotContains(t, got, "escaped-config")
+	})
+
+	t.Run("refuses even a contained in-cache symlink", func(t *testing.T) {
+		dir := t.TempDir()
+		// Pins the deliberate reject-all-symlinks policy: a symlink whose target
+		// is a regular file inside the cache is still refused.
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "real.tf"), []byte(`variable "x" {}`), 0600))
+		assert.NoError(t, os.Symlink(filepath.Join(dir, "real.tf"), filepath.Join(dir, "variables.tf")))
+		got, err := readTerraformConfigFromDir(dir, "")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "symlinked")
+		assert.NotContains(t, got, "variable")
+	})
+}
+
+// TestEnsureDirWithinLimits covers the clone caps that bound how large (bytes)
+// and how many files a cloned remote Terraform module may retain on disk
+// (GHSA-fmgp-q6jx-gg3x).
+func TestEnsureDirWithinLimits(t *testing.T) {
+	t.Run("passes when under the limits", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "a"), make([]byte, 1024), 0600))
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "b"), make([]byte, 1024), 0600))
+		assert.NoError(t, ensureDirWithinLimits(dir, 4096, 100))
+	})
+
+	t.Run("passes when cumulative size is exactly at the limit", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "a"), make([]byte, 4096), 0600))
+		assert.NoError(t, ensureDirWithinLimits(dir, 4096, 100))
+	})
+
+	t.Run("fails when cumulative size is one byte over the limit", func(t *testing.T) {
+		dir := t.TempDir()
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "a"), make([]byte, 4097), 0600))
+		err := ensureDirWithinLimits(dir, 4096, 100)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "exceeds the maximum allowed size")
+	})
+
+	t.Run("sums regular files across nested subdirectories", func(t *testing.T) {
+		dir := t.TempDir()
+		sub := filepath.Join(dir, "modules", "vpc")
+		assert.NoError(t, os.MkdirAll(sub, 0700))
+		assert.NoError(t, os.WriteFile(filepath.Join(dir, "main.tf"), make([]byte, 3000), 0600))
+		assert.NoError(t, os.WriteFile(filepath.Join(sub, "vars.tf"), make([]byte, 2000), 0600))
+		assert.Error(t, ensureDirWithinLimits(dir, 4096, 100))
+	})
+
+	t.Run("fails when the entry count exceeds the limit", func(t *testing.T) {
+		dir := t.TempDir()
+		for i := 0; i < 5; i++ {
+			assert.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%d", i)), []byte("x"), 0600))
+		}
+		err := ensureDirWithinLimits(dir, 4096, 3)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "entry count")
+	})
+
+	t.Run("counts directories toward the entry limit (inode-exhaustion guard)", func(t *testing.T) {
+		dir := t.TempDir()
+		// Many empty directories carry no regular-file bytes and no regular files,
+		// but each is an inode, so they must still trip the entry cap.
+		for i := 0; i < 10; i++ {
+			assert.NoError(t, os.MkdirAll(filepath.Join(dir, fmt.Sprintf("d%d", i)), 0700))
+		}
+		err := ensureDirWithinLimits(dir, 4096, 3)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "entry count")
+	})
+
+	t.Run("does not follow or count symlinks", func(t *testing.T) {
+		dir := t.TempDir()
+		outside := t.TempDir()
+		big := filepath.Join(outside, "big")
+		assert.NoError(t, os.WriteFile(big, make([]byte, 8192), 0600))
+		assert.NoError(t, os.Symlink(big, filepath.Join(dir, "link")))
+		// The symlink is skipped (not a regular file), so the 8192-byte target is
+		// neither followed nor counted against the 4096 cap.
+		assert.NoError(t, ensureDirWithinLimits(dir, 4096, 100))
+	})
+
+	t.Run("returns an error when the root does not exist", func(t *testing.T) {
+		assert.Error(t, ensureDirWithinLimits(filepath.Join(t.TempDir(), "missing"), 4096, 100))
+	})
+}
+
+// TestCacheMatchesRemote verifies the URL-keyed cache reuse decision.
+func TestCacheMatchesRemote(t *testing.T) {
+	populated := func(t *testing.T, markerURL string, withMarker bool) string {
+		t.Helper()
+		cache := filepath.Join(t.TempDir(), "mod")
+		assert.NoError(t, os.MkdirAll(cache, 0700))
+		assert.NoError(t, os.WriteFile(filepath.Join(cache, "variables.tf"), []byte("x"), 0600))
+		if withMarker {
+			assert.NoError(t, os.WriteFile(cache+cacheRemoteMarkerSuffix, []byte(markerURL), 0600))
+		}
+		return cache
+	}
+
+	t.Run("matches when populated and the URL marker matches", func(t *testing.T) {
+		cache := populated(t, "git://example/repo.git", true)
+		assert.True(t, cacheMatchesRemote(cache, "git://example/repo.git"))
+	})
+
+	t.Run("does not match when the URL changed", func(t *testing.T) {
+		cache := populated(t, "git://example/old.git", true)
+		assert.False(t, cacheMatchesRemote(cache, "git://example/new.git"))
+	})
+
+	t.Run("does not match when the marker is missing", func(t *testing.T) {
+		cache := populated(t, "", false)
+		assert.False(t, cacheMatchesRemote(cache, "git://example/repo.git"))
+	})
+
+	t.Run("does not match when the cache is empty", func(t *testing.T) {
+		assert.False(t, cacheMatchesRemote(t.TempDir(), "git://example/repo.git"))
+	})
+}
+
+// TestGetTerraformConfigurationFromRemoteInvalidatesCacheOnRejection exercises
+// the cache-reuse path with no network: a populated cache whose URL marker
+// matches skips the clone, so a poisoned cached variables.tf (a symlink escaping
+// the cache) is rejected AND the cache removed (GHSA-fmgp-q6jx-gg3x).
+func TestGetTerraformConfigurationFromRemoteInvalidatesCacheOnRejection(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+
+	outside := t.TempDir()
+	secret := filepath.Join(outside, "secret.txt")
+	assert.NoError(t, os.WriteFile(secret, []byte("top-secret-host-data"), 0600))
+
+	name := "poisoned-module"
+	url := "git://unused.invalid/repo.git"
+	cachePath := filepath.Join(home, ".vela", "terraform", name)
+	assert.NoError(t, os.MkdirAll(cachePath, 0700))
+	assert.NoError(t, os.Symlink(secret, filepath.Join(cachePath, "variables.tf")))
+	// Matching URL marker so the populated cache is reused and the clone is skipped.
+	assert.NoError(t, os.WriteFile(cachePath+cacheRemoteMarkerSuffix, []byte(url), 0600))
+
+	got, err := GetTerraformConfigurationFromRemote(name, url, "", nil)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "symlinked")
+	assert.NotContains(t, got, "top-secret-host-data")
+
+	_, statErr := os.Stat(cachePath)
+	assert.True(t, os.IsNotExist(statErr), "cache should be removed after a rejected read")
+}
+
+// TestGetTerraformConfigurationFromRemoteCleansUpOnCloneFailure verifies that a
+// failed clone (here a non-existent local repo) leaves no cache directory behind
+// for the next reconcile to reuse (GHSA-fmgp-q6jx-gg3x).
+func TestGetTerraformConfigurationFromRemoteCleansUpOnCloneFailure(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+
+	name := "clone-fail-module"
+	bogus := filepath.Join(t.TempDir(), "does-not-exist.git")
+
+	_, err := GetTerraformConfigurationFromRemote(name, bogus, "", nil)
+	assert.Error(t, err)
+
+	cachePath := filepath.Join(home, ".vela", "terraform", name)
+	_, statErr := os.Stat(cachePath)
+	assert.True(t, os.IsNotExist(statErr), "cache should be removed after a failed clone")
+}
+
+// TestGetTerraformConfigurationFromRemoteRejectsInvalidName ensures a name that
+// could steer the cache path outside the cache directory is rejected up front.
+func TestGetTerraformConfigurationFromRemoteRejectsInvalidName(t *testing.T) {
+	for _, name := range []string{"", ".", "..", "a/b", "../evil"} {
+		_, err := GetTerraformConfigurationFromRemote(name, "git://example/repo.git", "", nil)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "invalid Terraform module name")
+	}
+}
+
+// TestRedactURLCredentials verifies credentials embedded in a remote URL are stripped
+// before the URL is logged or written to the cache marker (GHSA-fmgp-q6jx-gg3x).
+func TestRedactURLCredentials(t *testing.T) {
+	cases := map[string]struct {
+		in   string
+		want string
+	}{
+		"https with user and password": {"https://user:s3cr3t@github.com/org/repo.git", "https://github.com/org/repo.git"},
+		"https with token as user":     {"https://ghp_TOKEN123@github.com/org/repo.git", "https://github.com/org/repo.git"},
+		"https without credentials":    {"https://github.com/org/repo.git", "https://github.com/org/repo.git"},
+		"ssh url with user":            {"ssh://git@github.com/org/repo.git", "ssh://github.com/org/repo.git"},
+		"scp-style ssh":                {"git@github.com:org/repo.git", "github.com:org/repo.git"},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := redactURLCredentials(tc.in)
+			assert.Equal(t, tc.want, got)
+			assert.NotContains(t, got, "s3cr3t")
+			assert.NotContains(t, got, "ghp_TOKEN123")
+		})
+	}
+}
+
+// TestRecordCacheRemoteRedactsCredentials verifies the persisted cache marker never
+// contains credentials from an authenticated remote URL, while the populated cache is
+// still reused for the same repository (GHSA-fmgp-q6jx-gg3x).
+func TestRecordCacheRemoteRedactsCredentials(t *testing.T) {
+	cache := filepath.Join(t.TempDir(), "mod")
+	assert.NoError(t, os.MkdirAll(cache, 0700))
+	assert.NoError(t, os.WriteFile(filepath.Join(cache, "variables.tf"), []byte("x"), 0600))
+
+	recordCacheRemote(cache, "https://user:s3cr3t@github.com/org/repo.git")
+
+	marker, err := os.ReadFile(filepath.Clean(cache + cacheRemoteMarkerSuffix))
+	assert.NoError(t, err)
+	assert.NotContains(t, string(marker), "s3cr3t")
+	assert.NotContains(t, string(marker), "user")
+	assert.Equal(t, "https://github.com/org/repo.git", string(marker))
+
+	assert.True(t, cacheMatchesRemote(cache, "https://user:s3cr3t@github.com/org/repo.git"))
+	assert.True(t, cacheMatchesRemote(cache, "https://github.com/org/repo.git"))
+	assert.False(t, cacheMatchesRemote(cache, "https://github.com/org/other.git"))
+}
@@ -19,7 +19,7 @@ entries:
    name: autoscalertrait
    type: application
    urls:
-    - https://charts.kubevela.net/example/autoscalertrait-0.1.0.tgz
+    - autoscalertrait-0.2.0.tgz
    version: 0.2.0
  - apiVersion: v2
    appVersion: 1.16.0
@@ -61,7 +61,7 @@ var _ = Describe("Output of listing addons tests", func() {
 		reg := &pkgaddon.Registry{
 			Name: "KubeVela",
 			Helm: &pkgaddon.HelmSource{
-				URL: "https://addons.kubevela.net",
+				URL: "https://kubevela.github.io/catalog/official",
 			},
 		}
 		ds := pkgaddon.NewRegistryDataStore(k8sClient)
@@ -155,7 +155,7 @@ var _ = Describe("Addon status or info", func() {
 				reg := &pkgaddon.Registry{
 					Name: "KubeVela",
 					Helm: &pkgaddon.HelmSource{
-						URL: "https://addons.kubevela.net",
+						URL: "https://kubevela.github.io/catalog/official",
 					},
 				}
 				ds := pkgaddon.NewRegistryDataStore(k8sClient)
@@ -218,7 +218,7 @@ var _ = Describe("Addon status or info", func() {
 				reg := &pkgaddon.Registry{
 					Name: "KubeVela",
 					Helm: &pkgaddon.HelmSource{
-						URL: "https://addons.kubevela.net",
+						URL: "https://kubevela.github.io/catalog/official",
 					},
 				}
 				ds := pkgaddon.NewRegistryDataStore(k8sClient)
@@ -319,7 +319,7 @@ var _ = Describe("Addon status or info", func() {
 				reg := &pkgaddon.Registry{
 					Name: "KubeVela",
 					Helm: &pkgaddon.HelmSource{
-						URL: "https://addons.kubevela.net",
+						URL: "https://kubevela.github.io/catalog/official",
 					},
 				}
 				ds := pkgaddon.NewRegistryDataStore(k8sClient)