Merge branch 'main' into dependabot/github_actions/cicd-83381794f3

Author: mergify[bot]

pkg/config/git_root.go

@@ -9,6 +9,12 @@ import (
 	u "github.com/cloudposse/atmos/pkg/utils"
 )
 
+// gitRootResolver resolves the Git repository root for a "!repo-root" style tag.
+// Overridden in tests to force the error path in applyGitRootBasePath, since
+// u.ProcessTagGitRoot's own default-value fallback otherwise swallows every
+// underlying git-detection failure when called with a non-empty default.
+var gitRootResolver = u.ProcessTagGitRoot
+
 // applyGitRootBasePath automatically sets the base path to the Git repository root
 // when base_path is empty or set to the default value.
 //
@@ -56,7 +62,7 @@ func applyGitRootBasePath(atmosConfig *schema.AtmosConfiguration) error {
 	}
 
 	// Resolve git root.
-	gitRoot, err := u.ProcessTagGitRoot("!repo-root .")
+	gitRoot, err := gitRootResolver("!repo-root .")
 	if err != nil {
 		log.Trace("Git root detection failed", "error", err)
 		return err

pkg/config/load_config_args.go

@@ -68,6 +68,16 @@ func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAnd
 	preserveCaseSensitiveMaps(v, atmosConfig)
 	restoreCaseSensitiveEnvMaps(atmosConfig)
 
+	// Apply git root discovery for default base path (same as the main LoadConfig
+	// auto-discovery flow, load.go). Without this, a config loaded via --config/
+	// --config-path with an empty (or ".") base_path never resolves to the git
+	// repository root, breaking component/stack path resolution that the exact
+	// same atmos.yaml would get right via plain auto-discovery (cloudposse/atmos#2863).
+	if err := applyGitRootBasePath(atmosConfig); err != nil {
+		log.Debug("Failed to apply git root base path", "error", err)
+		// Don't fail config loading if this step fails, just log it (mirrors load.go).
+	}
+
 	atmosConfig.CliConfigPath = connectPaths(configPaths)
 	return nil
 }

pkg/config/load_config_args_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -134,6 +135,114 @@ components:
 	require.ErrorIs(t, err, edition.ErrInvalidEdition)
 }
 
+// TestLoadConfigFromCLIArgs_AppliesGitRootBasePath reproduces cloudposse/atmos#2863:
+// loading config via --config/--config-path with an empty (or ".") base_path skipped
+// git-root discovery, because loadConfigFromCLIArgs never called applyGitRootBasePath,
+// unlike the main LoadConfig auto-discovery flow (load.go). This asserts the desired
+// end state -- BasePath resolved to the (mocked) git root -- for every CLI input
+// (--config file vs --config-path directory) and default base_path spelling ("" vs
+// "."), since applyGitRootBasePath treats both spellings as "unset" and either CLI
+// input can carry that config.
+func TestLoadConfigFromCLIArgs_AppliesGitRootBasePath(t *testing.T) {
+	tests := []struct {
+		name     string
+		basePath string
+		useDir   bool // true = AtmosConfigDirsFromArg, false = AtmosConfigFilesFromArg
+	}{
+		{name: "config file, empty base_path", basePath: `""`, useDir: false},
+		{name: "config file, dot base_path", basePath: `"."`, useDir: false},
+		{name: "config dir, empty base_path", basePath: `""`, useDir: true},
+		{name: "config dir, dot base_path", basePath: `"."`, useDir: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// The fixture atmos.yaml lives in its own temp dir, separate from the process
+			// cwd: hasLocalAtmosConfig only inspects the cwd, never the --config file's
+			// directory, so the two must be kept apart to exercise the real code path.
+			configDir := t.TempDir()
+			configFile := filepath.Join(configDir, "atmos.yaml")
+
+			configContent := fmt.Sprintf(`
+base_path: %s
+stacks:
+  base_path: "stacks"
+components:
+  terraform:
+    base_path: "components/terraform"
+`, tt.basePath)
+			require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0o644))
+
+			// cwd must be a separate, atmos-config-free directory so hasLocalAtmosConfig(cwd)
+			// returns false and git-root discovery is not skipped.
+			cwd := t.TempDir()
+			t.Chdir(cwd)
+
+			// Mock git-root discovery deterministically (see pkg/utils/git.go ProcessTagGitRoot,
+			// which short-circuits to TEST_GIT_ROOT when set, bypassing real git detection).
+			t.Setenv("TEST_GIT_ROOT", "/mock/git/repo/root")
+
+			v := viper.New()
+			v.SetConfigType("yaml")
+
+			configAndStacksInfo := &schema.ConfigAndStacksInfo{}
+			if tt.useDir {
+				configAndStacksInfo.AtmosConfigDirsFromArg = []string{configDir}
+			} else {
+				configAndStacksInfo.AtmosConfigFilesFromArg = []string{configFile}
+			}
+
+			var atmosConfig schema.AtmosConfiguration
+			err := loadConfigFromCLIArgs(v, configAndStacksInfo, &atmosConfig)
+			require.NoError(t, err)
+
+			// Desired end state: BasePath resolves to the git root, exactly like the main
+			// LoadConfig auto-discovery path does for the same atmos.yaml content.
+			assert.Equal(t, "/mock/git/repo/root", atmosConfig.BasePath)
+		})
+	}
+}
+
+// TestLoadConfigFromCLIArgs_GitRootBasePathErrorIsNonFatal verifies that a git-root
+// discovery failure is logged and swallowed rather than failing config loading, per
+// the "Don't fail config loading if this step fails, just log it" contract.
+func TestLoadConfigFromCLIArgs_GitRootBasePathErrorIsNonFatal(t *testing.T) {
+	original := gitRootResolver
+	var resolverCalled bool
+	gitRootResolver = func(string) (string, error) {
+		resolverCalled = true
+		return "", errors.New("simulated git root detection failure")
+	}
+	defer func() { gitRootResolver = original }()
+
+	configDir := t.TempDir()
+	configFile := filepath.Join(configDir, "atmos.yaml")
+	require.NoError(t, os.WriteFile(configFile, []byte(`
+base_path: ""
+stacks:
+  base_path: "stacks"
+components:
+  terraform:
+    base_path: "components/terraform"
+`), 0o644))
+
+	cwd := t.TempDir()
+	t.Chdir(cwd)
+
+	v := viper.New()
+	v.SetConfigType("yaml")
+
+	configAndStacksInfo := &schema.ConfigAndStacksInfo{
+		AtmosConfigFilesFromArg: []string{configFile},
+	}
+
+	var atmosConfig schema.AtmosConfiguration
+	err := loadConfigFromCLIArgs(v, configAndStacksInfo, &atmosConfig)
+	require.NoError(t, err, "a git-root resolution failure must not fail config loading")
+	assert.True(t, resolverCalled, "gitRootResolver must have been invoked for this test to prove anything")
+	assert.Empty(t, atmosConfig.BasePath, "base_path should remain unset when git-root discovery fails")
+}
+
 func TestLoadConfigFromCLIArgs_InvalidConfigDir(t *testing.T) {
 	v := viper.New()
 	v.SetConfigType("yaml")

pkg/toolchain/pr_artifact.go

@@ -471,38 +471,65 @@ func extractZipFile(zipPath, destDir string) error {
 	cleanDestDir := filepath.Clean(destDir) + string(os.PathSeparator)
 
 	for _, f := range r.File {
+		// Reject any archive entry name containing "..", guarding the raw tainted value
+		// directly (the shape CodeQL's go/zipslip query itself documents as the fix,
+		// see the query's help text) rather than only a derived/split-component check --
+		// sanitizeZipPath's per-component comparison isn't recognized as a sanitizer by
+		// the query's dataflow model, so this guard is what actually clears the taint.
+		if strings.Contains(f.Name, "..") {
+			return fmt.Errorf(errZipSlipFormat, ErrPRArtifactExtractFailed, f.Name)
+		}
+
 		// Sanitize path to prevent Zip Slip attacks.
 		destPath, err := sanitizeZipPath(f.Name, cleanDestDir)
 		if err != nil {
 			return err
 		}
 
-		// Verify path stays within destination (redundant with sanitizeZipPath but satisfies CodeQL).
-		if rel, relErr := filepath.Rel(strings.TrimSuffix(cleanDestDir, string(os.PathSeparator)), destPath); relErr != nil || strings.HasPrefix(rel, "..") {
-			return fmt.Errorf("%w: path escapes destination: %s", ErrPRArtifactExtractFailed, f.Name)
-		}
-
 		// Create parent directories.
 		if f.FileInfo().IsDir() {
+			// Guard placed immediately adjacent to the sink (rather than relying solely on
+			// sanitizeZipPath's earlier check) so CodeQL's go/zipslip query, which only
+			// credits a containment check that directly guards the sink, recognizes it.
+			if err := verifyWithinDestDir(destPath, cleanDestDir, f.Name); err != nil {
+				return err
+			}
 			if err := os.MkdirAll(destPath, dirPermissions); err != nil {
 				return fmt.Errorf("%w: failed to create dir: %w", ErrPRArtifactExtractFailed, err)
 			}
 			continue
 		}
 
-		if err := os.MkdirAll(filepath.Dir(destPath), dirPermissions); err != nil {
+		parentDir := filepath.Dir(destPath)
+		if err := verifyWithinDestDir(parentDir, cleanDestDir, f.Name); err != nil {
+			return err
+		}
+		if err := os.MkdirAll(parentDir, dirPermissions); err != nil {
 			return fmt.Errorf("%w: failed to create parent dir: %w", ErrPRArtifactExtractFailed, err)
 		}
 
 		// Extract file.
-		if err := extractZipEntry(f, destPath); err != nil {
+		if err := extractZipEntry(f, destPath, cleanDestDir); err != nil {
 			return err
 		}
 	}
 
 	return nil
 }
 
+// verifyWithinDestDir re-validates that path is contained within cleanDestDir immediately
+// before a filesystem-mutating operation. This duplicates the check sanitizeZipPath already
+// performed, deliberately placed adjacent to each sink (MkdirAll/Create) rather than relying
+// on a guarantee computed earlier in the caller, since that's what the go/zipslip scanner needs
+// to recognize the path as sanitized at the point of use.
+func verifyWithinDestDir(path, cleanDestDir, entryName string) error {
+	rel, err := filepath.Rel(strings.TrimSuffix(cleanDestDir, string(os.PathSeparator)), path)
+	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
+		return fmt.Errorf("%w: path escapes destination: %s", ErrPRArtifactExtractFailed, entryName)
+	}
+	return nil
+}
+
 // sanitizeZipPath validates that a zip entry path is safe and returns the full destination path.
 // This prevents Zip Slip attacks where malicious zip files contain paths like "../../../etc/passwd".
 func sanitizeZipPath(entryName, cleanDestDir string) (string, error) {
@@ -537,7 +564,14 @@ func sanitizeZipPath(entryName, cleanDestDir string) (string, error) {
 }
 
 // extractZipEntry extracts a single ZIP entry to the destination path.
-func extractZipEntry(f *zip.File, destPath string) error {
+func extractZipEntry(f *zip.File, destPath, cleanDestDir string) error {
+	// Guard immediately before the sink: extractZipFile already validated destPath, but this
+	// function must not trust that a caller-computed guarantee still holds without checking it
+	// again itself (and it's what lets CodeQL's go/zipslip query see the sanitizer at the sink).
+	if err := verifyWithinDestDir(destPath, cleanDestDir, f.Name); err != nil {
+		return err
+	}
+
 	rc, err := f.Open()
 	if err != nil {
 		return fmt.Errorf("%w: failed to open ZIP entry: %w", ErrPRArtifactExtractFailed, err)

pkg/toolchain/pr_artifact_test.go

@@ -451,6 +451,132 @@ func TestExtractZipFile_WithDirectory(t *testing.T) {
 	assert.Equal(t, "nested content", string(content))
 }
 
+func TestExtractZipFile_RawTraversalName(t *testing.T) {
+	tempDir := t.TempDir()
+	zipPath := filepath.Join(tempDir, "evil.zip")
+	extractDir := filepath.Join(tempDir, "extract")
+	require.NoError(t, os.MkdirAll(extractDir, 0o755))
+
+	// Entry name contains ".." directly; the raw substring guard in extractZipFile
+	// must reject it before sanitizeZipPath is even consulted.
+	createTestZip(t, zipPath, map[string][]byte{
+		"../evil.txt": []byte("malicious"),
+	})
+
+	err := extractZipFile(zipPath, extractDir)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrPRArtifactExtractFailed)
+	assert.Contains(t, err.Error(), "Zip Slip")
+}
+
+func TestExtractZipFile_ParentDirCreationFails(t *testing.T) {
+	tempDir := t.TempDir()
+	zipPath := filepath.Join(tempDir, "test.zip")
+	extractDir := filepath.Join(tempDir, "extract")
+	require.NoError(t, os.MkdirAll(extractDir, 0o755))
+
+	// Pre-create a regular file where the entry's parent directory needs to go,
+	// forcing os.MkdirAll(parentDir, ...) to fail with "not a directory".
+	blockedPath := filepath.Join(extractDir, "blocked")
+	require.NoError(t, os.WriteFile(blockedPath, []byte("i am a file, not a dir"), 0o644))
+
+	createTestZip(t, zipPath, map[string][]byte{
+		"blocked/file.txt": []byte("content"),
+	})
+
+	err := extractZipFile(zipPath, extractDir)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrPRArtifactExtractFailed)
+	assert.Contains(t, err.Error(), "failed to create parent dir")
+}
+
+func TestExtractZipFile_EntryCreationFails(t *testing.T) {
+	tempDir := t.TempDir()
+	zipPath := filepath.Join(tempDir, "test.zip")
+	extractDir := filepath.Join(tempDir, "extract")
+	require.NoError(t, os.MkdirAll(extractDir, 0o755))
+
+	// Pre-create a directory at the exact destination path of the file entry,
+	// forcing os.Create(destPath) inside extractZipEntry to fail.
+	collidingPath := filepath.Join(extractDir, "file.txt")
+	require.NoError(t, os.MkdirAll(collidingPath, 0o755))
+
+	createTestZip(t, zipPath, map[string][]byte{
+		"file.txt": []byte("content"),
+	})
+
+	err := extractZipFile(zipPath, extractDir)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrPRArtifactExtractFailed)
+}
+
+func TestVerifyWithinDestDir(t *testing.T) {
+	baseDir := t.TempDir()
+	cleanDestDir := filepath.Clean(baseDir) + string(os.PathSeparator)
+
+	tests := []struct {
+		name    string
+		path    string
+		wantErr bool
+	}{
+		{
+			name:    "path within dest dir",
+			path:    filepath.Join(baseDir, "sub", "file.txt"),
+			wantErr: false,
+		},
+		{
+			name:    "path exactly at dest dir root",
+			path:    filepath.Clean(baseDir),
+			wantErr: false,
+		},
+		{
+			name:    `path is parent of dest dir (rel == "..")`,
+			path:    filepath.Dir(filepath.Clean(baseDir)),
+			wantErr: true,
+		},
+		{
+			name:    `path is sibling of dest dir (rel starts with "../")`,
+			path:    filepath.Join(filepath.Dir(filepath.Clean(baseDir)), "sibling", "file.txt"),
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := verifyWithinDestDir(tt.path, cleanDestDir, "entry.txt")
+
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.ErrorIs(t, err, ErrPRArtifactExtractFailed)
+				assert.Contains(t, err.Error(), "escapes destination")
+				return
+			}
+			assert.NoError(t, err)
+		})
+	}
+}
+
+func TestExtractZipEntry_PathEscape(t *testing.T) {
+	tempDir := t.TempDir()
+	zipPath := filepath.Join(tempDir, "test.zip")
+	createTestZip(t, zipPath, map[string][]byte{"file.txt": []byte("content")})
+
+	r, err := zip.OpenReader(zipPath)
+	require.NoError(t, err)
+	defer r.Close()
+	require.Len(t, r.File, 1)
+
+	// destPath is outside cleanDestDir even though the entry name itself is benign,
+	// forcing extractZipEntry's own adjacent-to-sink guard to reject it.
+	cleanDestDir := filepath.Join(tempDir, "extract") + string(os.PathSeparator)
+	outsidePath := filepath.Join(tempDir, "outside", "file.txt")
+
+	err = extractZipEntry(r.File[0], outsidePath, cleanDestDir)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, ErrPRArtifactExtractFailed)
+	assert.Contains(t, err.Error(), "escapes destination")
+}
+
 func TestCopyFile_DestDirNotFound(t *testing.T) {
 	tempDir := t.TempDir()
 	srcPath := filepath.Join(tempDir, "source.txt")

website/package.json

@@ -106,7 +106,7 @@
       "brace-expansion@^1": "1.1.18",
       "brace-expansion@^2": "2.1.4",
       "dompurify@^3": "^3.4.12",
-      "fast-uri@^3": "^3.1.4",
+      "fast-uri@^3": "^3.1.5",
       "follow-redirects@^1": "^1.16.0",
       "http-proxy-middleware@^2": "^2.0.10",
       "joi@^17": "^17.13.4",