From f6a64398b5e0065c57c3a0fb6765dd3dc48c749d Mon Sep 17 00:00:00 2001 From: Ayush Kumar <65535504+roguepikachu@users.noreply.github.com> Date: Wed, 10 Jun 2026 21:54:29 +0530 Subject: [PATCH] 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. --- pkg/controller/utils/capability.go | 246 ++++++++++++++-- .../utils/capability_config_test.go | 9 +- pkg/controller/utils/capability_fifo_test.go | 47 +++ pkg/controller/utils/capability_test.go | 271 ++++++++++++++++++ 4 files changed, 552 insertions(+), 21 deletions(-) create mode 100644 pkg/controller/utils/capability_fifo_test.go diff --git a/pkg/controller/utils/capability.go b/pkg/controller/utils/capability.go index 7ae23e35c..03ea5499d 100644 --- a/pkg/controller/utils/capability.go +++ b/pkg/controller/utils/capability.go @@ -20,9 +20,11 @@ package utils import ( "context" "fmt" + "io" "os" "path/filepath" "strings" + "time" "github.com/getkin/kin-openapi/openapi3" "github.com/go-git/go-git/v5" @@ -205,41 +207,245 @@ 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", 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", 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. +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. +func cacheMatchesRemote(cachePath, remoteURL string) bool { + entities, err := os.ReadDir(cachePath) + if err != nil || len(entities) == 0 { + return false + } + recorded, err := os.ReadFile(cachePath + cacheRemoteMarkerSuffix) + if err != nil { + return false + } + return string(recorded) == remoteURL +} + +// recordCacheRemote records the remote URL a freshly cloned module came from. +func recordCacheRemote(cachePath, remoteURL string) { + if err := os.WriteFile(cachePath+cacheRemoteMarkerSuffix, []byte(remoteURL), 0600); err != nil { + klog.ErrorS(err, "failed to record Terraform module remote URL", "cache", cachePath) + } +} + +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 { diff --git a/pkg/controller/utils/capability_config_test.go b/pkg/controller/utils/capability_config_test.go index 0e1fb1c48..9b38beede 100644 --- a/pkg/controller/utils/capability_config_test.go +++ b/pkg/controller/utils/capability_config_test.go @@ -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 != "" { diff --git a/pkg/controller/utils/capability_fifo_test.go b/pkg/controller/utils/capability_fifo_test.go new file mode 100644 index 000000000..6c7145348 --- /dev/null +++ b/pkg/controller/utils/capability_fifo_test.go @@ -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") +} diff --git a/pkg/controller/utils/capability_test.go b/pkg/controller/utils/capability_test.go index 79b7a1279..d92a4f441 100644 --- a/pkg/controller/utils/capability_test.go +++ b/pkg/controller/utils/capability_test.go @@ -21,6 +21,7 @@ import ( "context" "fmt" "os" + "path/filepath" "strings" "testing" @@ -535,3 +536,273 @@ 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") + } +}