mirror of
https://github.com/kubevela/kubevela.git
synced 2026-08-18 20:17:04 +00:00
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.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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,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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user