From 48ba606fffbadb80946cf6bf35ade7e3d5ed020a Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Mon, 13 Jul 2026 14:49:25 +0200 Subject: [PATCH 1/6] Add the hosted Terraform module manifest and discovery contract. --- cmd/scanner/list_modules.go | 173 ++++++++++++++++++ cmd/scanner/list_modules_test.go | 50 +++++ cmd/scanner/main.go | 1 + cmd/scanner/scan.go | 15 +- pkg/model/fingerprint.go | 17 ++ pkg/model/fingerprint_test.go | 22 +++ .../modules/modulegraph/modulegraph_test.go | 153 +++++++++++++++- .../terraform/modules/resolver/prefetched.go | 12 +- .../modules/resolver/prefetched_test.go | 31 ++++ pkg/scan/remote_modules.go | 11 +- pkg/scan/remote_modules_test.go | 40 +++- 11 files changed, 505 insertions(+), 20 deletions(-) create mode 100644 cmd/scanner/list_modules.go create mode 100644 cmd/scanner/list_modules_test.go diff --git a/cmd/scanner/list_modules.go b/cmd/scanner/list_modules.go new file mode 100644 index 000000000..19e9e2c62 --- /dev/null +++ b/cmd/scanner/list_modules.go @@ -0,0 +1,173 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "strings" + + "github.com/DataDog/datadog-iac-scanner/pkg/model" + tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + cli "github.com/urfave/cli/v3" +) + +var listModulesAction = &cli.Command{ + Name: "list-modules", + Usage: "Prints Terraform module calls as JSON", + Flags: []cli.Flag{ + &cli.StringSliceFlag{ + Name: "path", + Aliases: []string{"p"}, + Usage: "names of files or directories to inspect", + Required: true, + }, + &cli.BoolFlag{ + Name: "all", + Usage: "include local module calls", + Value: false, + }, + }, + Action: listModules, +} + +func listModules(ctx context.Context, c *cli.Command) error { + paths, err := getAbsolutePaths(c.StringSlice("path")) + if err != nil { + return err + } + files, err := collectTerraformFiles(paths) + if err != nil { + return fmt.Errorf("collecting Terraform files: %w", err) + } + parsed, err := tfmodules.ParseTerraformModulesFromFiles(ctx, nil, files, allowedModuleFiles(paths, files)) + if err != nil { + return fmt.Errorf("parsing Terraform modules: %w", err) + } + encoder := json.NewEncoder(os.Stdout) + encoder.SetIndent("", " ") + return encoder.Encode(tfmodules.ListModuleEntries(parsed, c.Bool("all"))) +} + +func allowedModuleFiles(paths []string, files model.FileMetadatas) map[string]bool { + allowed := make(map[string]bool) + for _, rootPath := range paths { + info, err := os.Stat(rootPath) + if err != nil { + continue + } + if info.IsDir() { + for _, file := range files { + if pathContainsFile(rootPath, file.FilePath) { + allowed[file.FilePath] = true + } + } + continue + } + if strings.EqualFold(filepath.Ext(rootPath), ".tf") { + allowed[rootPath] = true + } + } + if len(allowed) == 0 { + return nil + } + return allowed +} + +func pathContainsFile(root, filePath string) bool { + rel, err := filepath.Rel(filepath.Clean(root), filepath.Clean(filePath)) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) +} + +func collectTerraformFiles(paths []string) (model.FileMetadatas, error) { + var files model.FileMetadatas + for _, rootPath := range paths { + info, err := os.Stat(rootPath) + if err != nil { + return nil, fmt.Errorf("stat path %q: %w", rootPath, err) + } + if !info.IsDir() { + if !strings.EqualFold(filepath.Ext(rootPath), ".tf") { + continue + } + dirFiles, err := collectTopLevelTerraformFiles(filepath.Dir(rootPath)) + if err != nil { + return nil, err + } + files = append(files, dirFiles...) + continue + } + dirFiles, err := walkTerraformDir(rootPath) + if err != nil { + return nil, err + } + files = append(files, dirFiles...) + } + return files, nil +} + +func walkTerraformDir(rootPath string) (model.FileMetadatas, error) { + root, err := os.OpenRoot(rootPath) + if err != nil { + return nil, fmt.Errorf("open root %q: %w", rootPath, err) + } + + var files model.FileMetadatas + walkErr := fs.WalkDir(root.FS(), ".", func(rel string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() { + if rel != "." { + switch path.Base(rel) { + case ".terraform", ".git", "vendor": + return fs.SkipDir + } + } + return nil + } + if !strings.EqualFold(path.Ext(rel), ".tf") { + return nil + } + data, err := fs.ReadFile(root.FS(), rel) + if err != nil { + return err + } + files = append(files, &model.FileMetadata{ + FilePath: filepath.Join(rootPath, filepath.FromSlash(rel)), + OriginalData: string(data), + }) + return nil + }) + closeErr := root.Close() + if walkErr != nil { + return nil, walkErr + } + return files, closeErr +} + +func collectTopLevelTerraformFiles(dir string) (model.FileMetadatas, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + var files model.FileMetadatas + for _, entry := range entries { + if entry.IsDir() || !strings.EqualFold(filepath.Ext(entry.Name()), ".tf") { + continue + } + filePath := filepath.Join(dir, entry.Name()) + data, err := os.ReadFile(filepath.Clean(filePath)) + if err != nil { + return nil, err + } + files = append(files, &model.FileMetadata{ + FilePath: filePath, + OriginalData: string(data), + }) + } + return files, nil +} diff --git a/cmd/scanner/list_modules_test.go b/cmd/scanner/list_modules_test.go new file mode 100644 index 000000000..437fb6652 --- /dev/null +++ b/cmd/scanner/list_modules_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "testing" + + tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + "github.com/stretchr/testify/require" +) + +func TestCollectTerraformFilesForModuleListing(t *testing.T) { + root := t.TempDir() + child := filepath.Join(root, "modules", "child") + cache := filepath.Join(root, ".terraform", "modules", "cached") + require.NoError(t, os.MkdirAll(child, 0o755)) + require.NoError(t, os.MkdirAll(cache, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "main.tf"), []byte(` +module "remote" { + source = "registry.example.com/acme/network/aws" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(child, "main.tf"), []byte(` +module "local" { + source = "../other" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(cache, "main.tf"), []byte(` +module "ignored" { + source = "registry.example.com/acme/ignored/aws" +} +`), 0o644)) + + files, err := collectTerraformFiles([]string{root}) + require.NoError(t, err) + require.Len(t, files, 2) + + modules, err := tfmodules.ParseTerraformModulesFromFiles( + context.Background(), + nil, + files, + allowedModuleFiles([]string{root}, files), + ) + require.NoError(t, err) + entries := tfmodules.ListModuleEntries(modules, true) + require.Len(t, entries, 2) + require.Equal(t, "local", entries[0].Name) + require.Equal(t, "remote", entries[1].Name) +} diff --git a/cmd/scanner/main.go b/cmd/scanner/main.go index c41bc7841..3e971f750 100644 --- a/cmd/scanner/main.go +++ b/cmd/scanner/main.go @@ -36,6 +36,7 @@ func main() { customAction, listPlatformsAction, listQueriesAction, + listModulesAction, showConfigAction, testRulesAction, serveAction, diff --git a/cmd/scanner/scan.go b/cmd/scanner/scan.go index a9e2381d3..a69ae4d50 100644 --- a/cmd/scanner/scan.go +++ b/cmd/scanner/scan.go @@ -333,11 +333,9 @@ func runScan(ctx context.Context, c *cli.Command) error { if err := validateReportFormats(reportFormats); err != nil { return errorWithExitCode(err, constants.InvalidConfigErrorCode) } - if !c.Bool("x-remote-modules") && - (c.String("x-remote-modules-manifest") != "" || - len(c.StringSlice("x-remote-modules-allowed-host")) > 0) { + if !c.Bool("x-remote-modules") && len(c.StringSlice("x-remote-modules-allowed-host")) > 0 { return errorWithExitCode( - errors.New("remote module resolver options require --x-remote-modules"), + errors.New("remote module host allowlist requires --x-remote-modules"), constants.InvalidConfigErrorCode, ) } @@ -383,7 +381,7 @@ func runScan(ctx context.Context, c *cli.Command) error { MaxResolverDepth: c.Int("max-resolver-depth"), PayloadPath: payloadPath, SCIInfo: model.SCIInfo{RepositoryDir: repoDir, RepositoryCommitInfo: *repoInfo}, - FlagEvaluator: getFeatureFlagEvaluator(c), + FlagEvaluator: getFeatureFlagEvaluator(c, inputPaths), Config: *cfg, ShouldScanTfPlans: c.Bool("x-terraform-plan"), DisableRuleIsolation: c.Bool("x-disable-rule-isolation"), @@ -609,10 +607,13 @@ func selectPlatforms(platforms []string) []string { return out } -func getFeatureFlagEvaluator(c *cli.Command) featureflags.FlagEvaluator { +func getFeatureFlagEvaluator(c *cli.Command, scanPaths []string) featureflags.FlagEvaluator { overrides := map[string]bool{ featureflags.IaCEnableKicsParallelFileParsing: c.Bool("x-parallelparsing"), - featureflags.IacEnableLocalModuleEval: c.Bool("x-local-module-eval") || c.Bool("x-remote-modules"), + featureflags.IacEnableLocalModuleEval: c.Bool("x-local-module-eval") || + c.Bool("x-remote-modules") || + c.String("x-remote-modules-manifest") != "" || + scan.HasTerraformModuleCache(scanPaths), } return featureflags.NewLocalEvaluatorWithOverrides(overrides) } diff --git a/pkg/model/fingerprint.go b/pkg/model/fingerprint.go index 705655683..993b15da4 100644 --- a/pkg/model/fingerprint.go +++ b/pkg/model/fingerprint.go @@ -23,6 +23,9 @@ const ( func GetDatadogFingerprintHash( sciInfo SCIInfo, filePath, platform, resourceType, resourceName, ruleID, vulnLine, moduleCallChain string, ) string { + if platform == terraformPlatform { + filePath = stripTerraformRegistryModuleVersion(filePath) + } segments := []string{sciInfo.RepositoryCommitInfo.RepositoryUrl, filePath, resourceType, resourceName, ruleID} switch platform { @@ -38,6 +41,20 @@ func GetDatadogFingerprintHash( return stringToHash(strings.Join(segments, "|")) } +func stripTerraformRegistryModuleVersion(filePath string) string { + slashed := strings.ReplaceAll(filePath, `\`, "/") + parts := strings.Split(slashed, "/") + if len(parts) < 5 || !strings.Contains(parts[0], ".") { + return filePath + } + provider, _, hasVersion := strings.Cut(parts[3], "@") + if !hasVersion || provider == "" { + return filePath + } + parts[3] = provider + return strings.Join(parts, "/") +} + func stringToHash(str string) string { hash := sha256.Sum256([]byte(str)) return hex.EncodeToString(hash[:]) diff --git a/pkg/model/fingerprint_test.go b/pkg/model/fingerprint_test.go index 3e72b3c68..13626e1e8 100644 --- a/pkg/model/fingerprint_test.go +++ b/pkg/model/fingerprint_test.go @@ -11,6 +11,28 @@ import ( "github.com/stretchr/testify/require" ) +func TestGetDatadogFingerprintHashRemoteModuleVersionStripping(t *testing.T) { + sci := SCIInfo{RepositoryCommitInfo: RepositoryCommitInfo{RepositoryUrl: "repo"}} + + v1 := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws@1.0.0/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + v2 := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws@2.0.0/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + require.Equal(t, v1, v2) + + constraint := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws@~> 3.0/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + noVersion := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + require.Equal(t, v1, constraint) + require.Equal(t, v1, noVersion) + + nestedAt := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws@1.0.0/templates/@scope/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + nestedAtNoVersion := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/vpc/aws/templates/@scope/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + require.Equal(t, nestedAtNoVersion, nestedAt) + + other := GetDatadogFingerprintHash(sci, "registry.terraform.io/hashicorp/eks/aws@1.0.0/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + local := GetDatadogFingerprintHash(sci, "modules/vpc@1.0.0/main.tf", terraformPlatform, "aws_vpc", "this", "rule-1", "", "") + require.NotEqual(t, v1, other) + require.NotEqual(t, v1, local) +} + // Empty chain adds no segment; a chain changes the hash for Terraform and is ignored elsewhere. func TestGetDatadogFingerprintHash_ModuleCallChain(t *testing.T) { sci := SCIInfo{RepositoryCommitInfo: RepositoryCommitInfo{RepositoryUrl: "repo"}} diff --git a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go index ae8009853..8405028d1 100644 --- a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go +++ b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go @@ -14,11 +14,15 @@ import ( type stubResolver struct { resolution resolver.Resolution + resolve func(*tfmodules.ParsedModule) (resolver.Resolution, error) } func (r stubResolver) Resolve( - _ context.Context, _ *tfmodules.ParsedModule, + _ context.Context, module *tfmodules.ParsedModule, ) (resolver.Resolution, error) { + if r.resolve != nil { + return r.resolve(module) + } return r.resolution, nil } @@ -90,6 +94,153 @@ func TestResolveTraversesLocalModuleToRemoteModule(t *testing.T) { require.Equal(t, wrapperDir, result.Modules[0].CallerRoot) } +func TestResolveRestrictsRepositoryCallsToDiscoveryPaths(t *testing.T) { + root := t.TempDir() + allowedDir := filepath.Join(root, "allowed") + excludedDir := filepath.Join(root, "excluded") + require.NoError(t, os.MkdirAll(allowedDir, 0o755)) + require.NoError(t, os.MkdirAll(excludedDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(allowedDir, "main.tf"), []byte(`resource "x" "allowed" {}`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(excludedDir, "main.tf"), []byte(`resource "x" "excluded" {}`), 0o644)) + allowedFile := filepath.Join(root, "main.tf") + excludedFile := filepath.Join(root, "experimental.tf") + require.NoError(t, os.WriteFile(allowedFile, []byte(`module "allowed" { source = "allowed" }`), 0o644)) + require.NoError(t, os.WriteFile(excludedFile, []byte(`module "excluded" { source = "excluded" }`), 0o644)) + + result := Resolve(context.Background(), &Request{ + RootPaths: []string{root}, + DiscoveryPaths: []string{allowedFile}, + Resolver: stubResolver{resolve: func(module *tfmodules.ParsedModule) (resolver.Resolution, error) { + if module.Source == "allowed" { + return resolver.Resolution{LocalPath: allowedDir}, nil + } + return resolver.Resolution{LocalPath: excludedDir}, nil + }}, + MaxDepth: 1, + }) + + require.Equal(t, []string{filepath.Join(allowedDir, "main.tf")}, result.ScanPaths) +} + +func TestResolveSkipsFilteredLocalModuleTrees(t *testing.T) { + root := t.TempDir() + wrapperDir := filepath.Join(root, "modules", "wrapper") + remoteDir := filepath.Join(root, "remote") + require.NoError(t, os.MkdirAll(wrapperDir, 0o755)) + require.NoError(t, os.MkdirAll(remoteDir, 0o755)) + rootFile := filepath.Join(root, "main.tf") + require.NoError(t, os.WriteFile(rootFile, []byte(`module "wrapper" { source = "./modules/wrapper" }`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(wrapperDir, "main.tf"), []byte(`module "remote" { source = "remote" }`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "main.tf"), []byte(`resource "x" "remote" {}`), 0o644)) + + result := Resolve(context.Background(), &Request{ + RootPaths: []string{root}, + DiscoveryPaths: []string{rootFile}, + Resolver: stubResolver{resolution: resolver.Resolution{LocalPath: remoteDir}}, + MaxDepth: 2, + }) + + require.Empty(t, result.ScanPaths) +} + +func TestResolveDeduplicatesEquivalentGitCalls(t *testing.T) { + root := t.TempDir() + remoteDir := filepath.Join(root, "remote") + require.NoError(t, os.MkdirAll(remoteDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(remoteDir, "main.tf"), []byte(`resource "x" "remote" {}`), 0o644)) + rootFile := filepath.Join(root, "main.tf") + require.NoError(t, os.WriteFile(rootFile, []byte(` +module "a" { source = "git::https://example.com/mod.git?ref=main" } +module "b" { source = "git::https://example.com/mod.git?ref=main" } +`), 0o644)) + var calls atomic.Int32 + + result := Resolve(context.Background(), &Request{ + RootPaths: []string{rootFile}, + DiscoveryPaths: []string{rootFile}, + Resolver: stubResolver{resolve: func(*tfmodules.ParsedModule) (resolver.Resolution, error) { + calls.Add(1) + return resolver.Resolution{LocalPath: remoteDir}, nil + }}, + MaxDepth: 1, + }) + + require.Equal(t, int32(1), calls.Load()) + require.Equal(t, []string{filepath.Join(remoteDir, "main.tf")}, result.ScanPaths) + require.Len(t, result.Modules, 2) +} + +func TestResolveKeepsUnversionedRegistryCallsDistinctByName(t *testing.T) { + root := t.TempDir() + remoteA := filepath.Join(root, "remote-a") + remoteB := filepath.Join(root, "remote-b") + for _, dir := range []string{remoteA, remoteB} { + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "main.tf"), []byte(`resource "x" "remote" {}`), 0o644)) + } + rootFile := filepath.Join(root, "main.tf") + require.NoError(t, os.WriteFile(rootFile, []byte(` +module "a" { source = "same/source/aws" } +module "b" { source = "same/source/aws" } +`), 0o644)) + + result := Resolve(context.Background(), &Request{ + RootPaths: []string{rootFile}, + DiscoveryPaths: []string{rootFile}, + Resolver: stubResolver{resolve: func(module *tfmodules.ParsedModule) (resolver.Resolution, error) { + if module.Name == "a" { + return resolver.Resolution{LocalPath: remoteA}, nil + } + return resolver.Resolution{LocalPath: remoteB}, nil + }}, + MaxDepth: 1, + }) + + require.ElementsMatch(t, []string{ + filepath.Join(remoteA, "main.tf"), + filepath.Join(remoteB, "main.tf"), + }, result.ScanPaths) + require.Len(t, result.Modules, 2) +} + +func TestResolveCountsLocalHopsTowardDepth(t *testing.T) { + root, wrapperDir, remoteDir := writeNestedModuleGraphFixture(t) + + result := Resolve(context.Background(), &Request{ + RootPaths: []string{root}, + DiscoveryPaths: []string{ + filepath.Join(root, "main.tf"), + filepath.Join(wrapperDir, "main.tf"), + }, + Resolver: stubResolver{resolution: resolver.Resolution{LocalPath: remoteDir}}, + MaxDepth: 1, + }) + + require.Empty(t, result.ScanPaths) +} + +func TestCanonicalGitModuleSource(t *testing.T) { + require.Equal( + t, + "git::https://github.com/org/repo//sub?ref=v1", + canonicalGitModuleSource(" git::https://github.com/org/repo.git//sub?ref=v1 "), + ) + require.Equal( + t, + canonicalModuleURL("git::https://github.com/org/repo.git//sub", "v1.2.3"), + canonicalModuleURL("git::https://github.com/org/repo//sub", "v1.2.3"), + ) + require.Equal( + t, + remoteResolveIdentity(&tfmodules.ParsedModule{ + Source: "git::https://github.com/org/repo.git//sub?ref=v1", + }), + remoteResolveIdentity(&tfmodules.ParsedModule{ + Source: "git::ssh://git@github.com/org/repo//sub?ref=v1", + }), + ) +} + func writeModuleGraphFixture(t *testing.T) (string, string) { t.Helper() base := t.TempDir() diff --git a/pkg/parser/terraform/modules/resolver/prefetched.go b/pkg/parser/terraform/modules/resolver/prefetched.go index ad67f0f77..3a165b06c 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched.go +++ b/pkg/parser/terraform/modules/resolver/prefetched.go @@ -14,6 +14,7 @@ import ( "strings" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + goversion "github.com/hashicorp/go-version" ) // ManifestEntry is one row in a --modules-manifest JSON file. @@ -96,7 +97,7 @@ func (r *PrefetchedResolver) Resolve(_ context.Context, mod *tfmodules.ParsedMod Reason: fmt.Sprintf("module %q not found in manifest", mod.Source), } } - if entry.Version != "" && mod.Version != "" && entry.Version != mod.Version { + if entry.Version != "" && mod.Version != "" && !versionMatchesConstraint(entry.Version, mod.Version) { return Resolution{}, &tfmodules.UnresolvedError{ Reason: fmt.Sprintf("module %q version %q not found in manifest", mod.Source, mod.Version), } @@ -104,6 +105,15 @@ func (r *PrefetchedResolver) Resolve(_ context.Context, mod *tfmodules.ParsedMod return Resolution{LocalPath: entry.LocalPath}, nil } +func versionMatchesConstraint(version, constraint string) bool { + constraints, err := goversion.NewConstraint(constraint) + if err != nil { + return version == constraint + } + resolved, err := goversion.NewVersion(version) + return err == nil && constraints.Check(resolved) +} + func manifestModuleKey(source, version string) string { if version == "" { return source diff --git a/pkg/parser/terraform/modules/resolver/prefetched_test.go b/pkg/parser/terraform/modules/resolver/prefetched_test.go index ca179ff20..8d0b8f412 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched_test.go +++ b/pkg/parser/terraform/modules/resolver/prefetched_test.go @@ -67,3 +67,34 @@ func TestPrefetchedResolverUsesManifest(t *testing.T) { require.NoError(t, err) require.Equal(t, moduleDir, got.LocalPath) } + +func TestPrefetchedResolverValidatesResolvedVersionAgainstConstraint(t *testing.T) { + dir := t.TempDir() + moduleDir := filepath.Join(dir, "vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + resolver := NewPrefetchedResolver(&Manifest{ + Dir: dir, + Modules: map[string]ManifestEntry{ + "terraform-aws-modules/vpc/aws@~> 5.0": { + LocalPath: moduleDir, + Version: "5.1.2", + }, + }, + }) + + _, err := resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Source: "terraform-aws-modules/vpc/aws", + Version: "~> 5.0", + }) + require.NoError(t, err) + + resolver.manifest.Modules["terraform-aws-modules/vpc/aws@~> 6.0"] = ManifestEntry{ + LocalPath: moduleDir, + Version: "5.1.2", + } + _, err = resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Source: "terraform-aws-modules/vpc/aws", + Version: "~> 6.0", + }) + require.Error(t, err) +} diff --git a/pkg/scan/remote_modules.go b/pkg/scan/remote_modules.go index 7bda812ae..bd973d599 100644 --- a/pkg/scan/remote_modules.go +++ b/pkg/scan/remote_modules.go @@ -77,18 +77,19 @@ func (c *Client) resolveTerraformModulesForScan( } func (c *Client) shouldPreScanTerraformModules(scanPaths []string) bool { - if !c.ScanParams.EnableRemoteModules { - return false - } - if c.ScanParams.RemoteModulesManifestPath != "" { + if c.ScanParams.EnableRemoteModules || c.ScanParams.RemoteModulesManifestPath != "" { return true } + return HasTerraformModuleCache(scanPaths) +} + +func HasTerraformModuleCache(scanPaths []string) bool { for _, root := range dotTerraformRootDirs(scanPaths) { if hasTerraformModulesManifest(root) { return true } } - return true + return false } func (c *Client) buildModuleResolverChain(ctx context.Context, moduleDiscoveryPaths []string) *tfresolver.ChainResolver { diff --git a/pkg/scan/remote_modules_test.go b/pkg/scan/remote_modules_test.go index 2f5d016c3..4281788de 100644 --- a/pkg/scan/remote_modules_test.go +++ b/pkg/scan/remote_modules_test.go @@ -18,22 +18,21 @@ import ( func TestRemoteModulesDisabledByDefault(t *testing.T) { root := t.TempDir() - _, manifestPath := writeRemoteModuleFixture(t, root) + writeRemoteModuleFixture(t, root) params := remoteModuleScanParams(root) - params.RemoteModulesManifestPath = manifestPath params.EnableRemoteModules = false results := executeRemoteModuleScan(t, params) require.Empty(t, results.Results) } -func TestRemoteModulesManifestEnablesModuleInstantiation(t *testing.T) { +func TestRemoteModulesManifestEnablesOfflineModuleInstantiation(t *testing.T) { root := t.TempDir() moduleDir, manifestPath := writeRemoteModuleFixture(t, root) params := remoteModuleScanParams(root) - params.EnableRemoteModules = true + params.EnableRemoteModules = false params.RemoteModulesManifestPath = manifestPath results := executeRemoteModuleScan(t, params) @@ -88,6 +87,35 @@ func TestRemoteModuleFilesBypassPrebuiltInventoryFilters(t *testing.T) { require.ElementsMatch(t, []string{filepath.ToSlash(rootFile), filepath.ToSlash(moduleFile)}, paths) } +func TestShouldPreScanTerraformModules(t *testing.T) { + t.Run("disabled", func(t *testing.T) { + root := t.TempDir() + client := &Client{ScanParams: &Parameters{}} + require.False(t, client.shouldPreScanTerraformModules([]string{root})) + }) + + t.Run("network resolution", func(t *testing.T) { + root := t.TempDir() + client := &Client{ScanParams: &Parameters{EnableRemoteModules: true}} + require.True(t, client.shouldPreScanTerraformModules([]string{root})) + }) + + t.Run("prefetched manifest", func(t *testing.T) { + root := t.TempDir() + client := &Client{ScanParams: &Parameters{RemoteModulesManifestPath: filepath.Join(root, "modules.json")}} + require.True(t, client.shouldPreScanTerraformModules([]string{root})) + }) + + t.Run("terraform cache", func(t *testing.T) { + root := t.TempDir() + manifestDir := filepath.Join(root, ".terraform", "modules") + require.NoError(t, os.MkdirAll(manifestDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(manifestDir, "modules.json"), []byte(`{}`), 0o644)) + client := &Client{ScanParams: &Parameters{}} + require.True(t, client.shouldPreScanTerraformModules([]string{root})) + }) +} + func writeRemoteModuleFixture(t *testing.T, root string) (string, string) { t.Helper() moduleDir := filepath.Join(filepath.Dir(root), "downloaded-vpc") @@ -103,7 +131,7 @@ resource "aws_vpc" "this" { require.NoError(t, os.WriteFile(rootFile, []byte(` module "vpc" { source = "terraform-aws-modules/vpc/aws" - version = "5.0.0" + version = "~> 5.0" cidr = "10.0.0.0/16" } `), 0o644)) @@ -111,7 +139,7 @@ module "vpc" { manifestPath := filepath.Join(root, "modules.json") manifest := resolver.Manifest{ Modules: map[string]resolver.ManifestEntry{ - "terraform-aws-modules/vpc/aws@5.0.0": {LocalPath: moduleDir, Version: "5.0.0"}, + "terraform-aws-modules/vpc/aws@~> 5.0": {LocalPath: moduleDir, Version: "5.1.2"}, }, } data, err := json.Marshal(manifest) From fa7feb39d327f9a1e6939409ec063ded5bac7e6e Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Mon, 13 Jul 2026 15:30:56 +0200 Subject: [PATCH 2/6] Fail scans on invalid manifests and prefer explicit manifests over the Terraform cache. --- pkg/scan/remote_modules.go | 24 ++++++++++------ pkg/scan/remote_modules_test.go | 51 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/pkg/scan/remote_modules.go b/pkg/scan/remote_modules.go index bd973d599..3eea00577 100644 --- a/pkg/scan/remote_modules.go +++ b/pkg/scan/remote_modules.go @@ -7,6 +7,7 @@ package scan import ( "context" + "fmt" "os" "path/filepath" "strings" @@ -37,7 +38,10 @@ func (c *Client) resolveTerraformModulesForScan( if err != nil { return nil, nil, nil, err } - chain := c.buildModuleResolverChain(ctx, moduleDiscoveryPaths) + chain, err := c.buildModuleResolverChain(ctx, moduleDiscoveryPaths) + if err != nil { + return nil, nil, nil, err + } if c.ScanParams.EnableRemoteModules { contextLogger.Info().Msg("Resolving remote Terraform modules...") @@ -92,25 +96,27 @@ func HasTerraformModuleCache(scanPaths []string) bool { return false } -func (c *Client) buildModuleResolverChain(ctx context.Context, moduleDiscoveryPaths []string) *tfresolver.ChainResolver { +func (c *Client) buildModuleResolverChain( + ctx context.Context, moduleDiscoveryPaths []string, +) (*tfresolver.ChainResolver, error) { contextLogger := logger.FromContext(ctx) resolvers := []tfresolver.Resolver{ tfresolver.LocalResolver{}, - &tfresolver.DotTerraformResolver{RootDirs: dotTerraformRootDirs(moduleDiscoveryPaths)}, } if c.ScanParams.RemoteModulesManifestPath != "" { manifest, err := tfresolver.LoadManifest(c.ScanParams.RemoteModulesManifestPath) if err != nil { - contextLogger.Warn().Err(err). - Msgf("Failed to load modules manifest %q; remote modules from manifest will be unresolved", - c.ScanParams.RemoteModulesManifestPath) - } else { - resolvers = append(resolvers, tfresolver.NewPrefetchedResolver(manifest)) + return nil, fmt.Errorf("loading modules manifest %q: %w", c.ScanParams.RemoteModulesManifestPath, err) } + resolvers = append(resolvers, tfresolver.NewPrefetchedResolver(manifest)) } + resolvers = append(resolvers, &tfresolver.DotTerraformResolver{ + RootDirs: dotTerraformRootDirs(moduleDiscoveryPaths), + }) + if c.ScanParams.EnableRemoteModules { resolvers = append(resolvers, tfresolver.NewLocalGitRefResolver(dotTerraformRootDirs(moduleDiscoveryPaths), ""), @@ -137,7 +143,7 @@ func (c *Client) buildModuleResolverChain(ctx context.Context, moduleDiscoveryPa } resolvers = append(resolvers, tfresolver.NewGoGetterResolver(ggCfg)) - return tfresolver.NewChainResolver(resolvers...) + return tfresolver.NewChainResolver(resolvers...), nil } func dotTerraformRootDirs(paths []string) []string { diff --git a/pkg/scan/remote_modules_test.go b/pkg/scan/remote_modules_test.go index 4281788de..e9a0328ca 100644 --- a/pkg/scan/remote_modules_test.go +++ b/pkg/scan/remote_modules_test.go @@ -11,6 +11,7 @@ import ( "github.com/DataDog/datadog-iac-scanner/pkg/engine/source" "github.com/DataDog/datadog-iac-scanner/pkg/featureflags" "github.com/DataDog/datadog-iac-scanner/pkg/model" + tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules/resolver" consolePrinter "github.com/DataDog/datadog-iac-scanner/pkg/printer" "github.com/stretchr/testify/require" @@ -116,6 +117,56 @@ func TestShouldPreScanTerraformModules(t *testing.T) { }) } +func TestBuildModuleResolverChainFailsOnInvalidManifest(t *testing.T) { + client := &Client{ScanParams: &Parameters{ + RemoteModulesManifestPath: filepath.Join(t.TempDir(), "missing.json"), + }} + + _, err := client.buildModuleResolverChain(context.Background(), []string{t.TempDir()}) + + require.Error(t, err) + require.Contains(t, err.Error(), "loading modules manifest") +} + +func TestBuildModuleResolverChainPrefersExplicitManifestOverTerraformCache(t *testing.T) { + root := t.TempDir() + resolvedRoot, err := filepath.EvalSymlinks(root) + require.NoError(t, err) + staleDir := filepath.Join(resolvedRoot, "stale") + preferredDir := filepath.Join(resolvedRoot, "preferred") + for _, dir := range []string{staleDir, preferredDir} { + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "main.tf"), []byte(`resource "x" "y" {}`), 0o644)) + } + + tfModulesDir := filepath.Join(resolvedRoot, ".terraform", "modules") + require.NoError(t, os.MkdirAll(tfModulesDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tfModulesDir, "modules.json"), []byte(`{ + "Modules": [{"Key":"m","Source":"terraform-aws-modules/vpc/aws","Version":"1.0.0","Dir":"stale"}] +}`), 0o644)) + + manifestPath := filepath.Join(resolvedRoot, "modules.json") + manifestData, err := json.Marshal(resolver.Manifest{ + Dir: resolvedRoot, + Modules: map[string]resolver.ManifestEntry{ + "terraform-aws-modules/vpc/aws@1.0.0": {LocalPath: preferredDir, Version: "1.0.0"}, + }, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, manifestData, 0o644)) + + client := &Client{ScanParams: &Parameters{RemoteModulesManifestPath: manifestPath}} + chain, err := client.buildModuleResolverChain(context.Background(), []string{resolvedRoot}) + require.NoError(t, err) + + got, err := chain.Resolve(context.Background(), &tfmodules.ParsedModule{ + Source: "terraform-aws-modules/vpc/aws", + Version: "1.0.0", + }) + require.NoError(t, err) + require.Equal(t, preferredDir, got.LocalPath) +} + func writeRemoteModuleFixture(t *testing.T, root string) (string, string) { t.Helper() moduleDir := filepath.Join(filepath.Dir(root), "downloaded-vpc") From 7584da473f1660dfe386cbf1e6489034c575952d Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Tue, 14 Jul 2026 20:05:18 +0200 Subject: [PATCH 3/6] Harden hosted manifest resolution with caller attribution, authoritative offline mode, and scanner hot-path optimizations. --- cmd/scanner/list_modules.go | 53 +++- internal/storage/memory.go | 21 +- internal/storage/memory_test.go | 101 ++++++-- pkg/analyzer/analyzer.go | 44 +++- pkg/analyzer/analyzer_test.go | 33 +++ pkg/detector/search_line_detector.go | 233 +++++++++++------ pkg/detector/search_line_detector_test.go | 50 ++++ pkg/engine/module_resolve.go | 72 ++++-- pkg/engine/module_resolve_test.go | 35 +++ pkg/engine/provider/filesystem.go | 3 + pkg/engine/provider/filesystem_test.go | 16 ++ pkg/model/summary.go | 41 ++- pkg/model/summary_test.go | 58 +++-- pkg/parser/terraform/modules/list_modules.go | 38 +++ .../terraform/modules/list_modules_test.go | 24 ++ .../modules/modulegraph/modulegraph.go | 139 +++++++--- .../modules/modulegraph/modulegraph_test.go | 94 ++++++- .../terraform/modules/resolver/prefetched.go | 244 +++++++++++++++--- .../modules/resolver/prefetched_test.go | 173 ++++++++++++- .../terraform/modules/resolver/resolver.go | 10 +- pkg/scan/remote_modules.go | 50 ++-- pkg/scan/remote_modules_test.go | 53 ++++ 22 files changed, 1330 insertions(+), 255 deletions(-) diff --git a/cmd/scanner/list_modules.go b/cmd/scanner/list_modules.go index 19e9e2c62..13df013a7 100644 --- a/cmd/scanner/list_modules.go +++ b/cmd/scanner/list_modules.go @@ -49,7 +49,58 @@ func listModules(ctx context.Context, c *cli.Command) error { } encoder := json.NewEncoder(os.Stdout) encoder.SetIndent("", " ") - return encoder.Encode(tfmodules.ListModuleEntries(parsed, c.Bool("all"))) + return encoder.Encode(tfmodules.ListModuleEntriesRelativeTo( + parsed, c.Bool("all"), moduleRepositoryRoot(paths), + )) +} + +func moduleRepositoryRoot(paths []string) string { + if len(paths) == 0 { + return "" + } + start := paths[0] + if info, err := os.Stat(start); err == nil && !info.IsDir() { + start = filepath.Dir(start) + } + for dir := filepath.Clean(start); ; dir = filepath.Dir(dir) { + if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil && pathsWithinRoot(dir, paths) { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + break + } + } + return commonPathRoot(paths) +} + +func pathsWithinRoot(root string, paths []string) bool { + for _, candidate := range paths { + if !pathContainsFile(root, candidate) { + return false + } + } + return true +} + +func commonPathRoot(paths []string) string { + root := filepath.Clean(paths[0]) + if info, err := os.Stat(root); err == nil && !info.IsDir() { + root = filepath.Dir(root) + } + for _, candidate := range paths[1:] { + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + candidate = filepath.Dir(candidate) + } + for !pathContainsFile(root, candidate) { + parent := filepath.Dir(root) + if parent == root { + return root + } + root = parent + } + } + return root } func allowedModuleFiles(paths []string, files model.FileMetadatas) map[string]bool { diff --git a/internal/storage/memory.go b/internal/storage/memory.go index ac9cda5fe..c8a8c369a 100644 --- a/internal/storage/memory.go +++ b/internal/storage/memory.go @@ -14,43 +14,40 @@ import ( "github.com/rs/zerolog/log" ) -var ( - memoryMu sync.Mutex -) - // MemoryStorage is scans' results representation type MemoryStorage struct { vulnerabilities []model.Vulnerability allFiles model.FileMetadatas + mu sync.RWMutex } // SaveFile adds a new file metadata to files collection func (m *MemoryStorage) SaveFile(_ context.Context, metadata *model.FileMetadata) error { - memoryMu.Lock() - defer memoryMu.Unlock() + m.mu.Lock() + defer m.mu.Unlock() m.allFiles = append(m.allFiles, metadata) return nil } // GetFiles returns a collection of files saved on MemoryStorage func (m *MemoryStorage) GetFiles(_ context.Context, _ string) (model.FileMetadatas, error) { - memoryMu.Lock() - defer memoryMu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() return m.allFiles, nil } // SaveVulnerabilities adds a list of vulnerabilities to vulnerabilities collection func (m *MemoryStorage) SaveVulnerabilities(_ context.Context, vulnerabilities []model.Vulnerability) error { - defer memoryMu.Unlock() - memoryMu.Lock() + m.mu.Lock() + defer m.mu.Unlock() m.vulnerabilities = append(m.vulnerabilities, vulnerabilities...) return nil } // GetVulnerabilities returns a collection of vulnerabilities saved on MemoryStorage func (m *MemoryStorage) GetVulnerabilities(_ context.Context, _ string) ([]model.Vulnerability, error) { - memoryMu.Lock() - defer memoryMu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() return m.getUniqueVulnerabilities(), nil } diff --git a/internal/storage/memory_test.go b/internal/storage/memory_test.go index a2630d728..d36d93396 100644 --- a/internal/storage/memory_test.go +++ b/internal/storage/memory_test.go @@ -9,7 +9,9 @@ import ( "context" "fmt" "reflect" + "sync" "testing" + "time" "github.com/stretchr/testify/require" @@ -91,15 +93,15 @@ func TestMemoryStorage(t *testing.T) { //nolint fields: fields{ vulnerabilities: []model.Vulnerability{ { - ID: 0, - ScanID: "scan_id", - FileID: "file_id", - FileName: "file_name", - QueryID: "query_id", - QueryName: "query_name", - Line: 1, - SearchKey: "search_key", - Output: "-", + ID: 0, + ScanID: "scan_id", + FileID: "file_id", + FileName: "file_name", + QueryID: "query_id", + QueryName: "query_name", + Line: 1, + SearchKey: "search_key", + Output: "-", }, }, allFiles: model.FileMetadatas{ @@ -128,15 +130,15 @@ func TestMemoryStorage(t *testing.T) { //nolint }, vulnerabilities: []model.Vulnerability{ { - ID: 0, - ScanID: "scan_id", - FileID: "file_id", - FileName: "file_name", - QueryID: "query_id", - QueryName: "query_name", - Line: 1, - SearchKey: "search_key", - Output: "-", + ID: 0, + ScanID: "scan_id", + FileID: "file_id", + FileName: "file_name", + QueryID: "query_id", + QueryName: "query_name", + Line: 1, + SearchKey: "search_key", + Output: "-", }, }, }, @@ -196,15 +198,15 @@ func TestMemoryStorage_SaveVulnerabilities(t *testing.T) { in0: nil, vulnerabilities: []model.Vulnerability{ { - ID: 0, - ScanID: "scan_id", - FileID: "file_id", - FileName: "file_name", - QueryID: "query_id", - QueryName: "query_name", - Line: 1, - SearchKey: "search_key", - Output: "-", + ID: 0, + ScanID: "scan_id", + FileID: "file_id", + FileName: "file_name", + QueryID: "query_id", + QueryName: "query_name", + Line: 1, + SearchKey: "search_key", + Output: "-", }, }, }, @@ -247,3 +249,48 @@ func TestNewMemoryStorage(t *testing.T) { }) } } + +func TestMemoryStorageInstancesDoNotBlockEachOther(t *testing.T) { + first := NewMemoryStorage() + second := NewMemoryStorage() + first.mu.Lock() + defer first.mu.Unlock() + + done := make(chan error, 1) + go func() { + done <- second.SaveFile(context.Background(), &model.FileMetadata{ID: "second"}) + }() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("an independent storage instance was blocked") + } +} + +func TestMemoryStorageConcurrentInstances(t *testing.T) { + storages := []*MemoryStorage{NewMemoryStorage(), NewMemoryStorage()} + var wg sync.WaitGroup + for storageIndex, storage := range storages { + for item := 0; item < 100; item++ { + wg.Add(1) + go func() { + defer wg.Done() + id := fmt.Sprintf("%d-%d", storageIndex, item) + _ = storage.SaveFile(context.Background(), &model.FileMetadata{ID: id}) + _ = storage.SaveVulnerabilities(context.Background(), []model.Vulnerability{{ + FileName: id, + QueryID: id, + }}) + }() + } + } + wg.Wait() + + for _, storage := range storages { + files, err := storage.GetFiles(context.Background(), "") + require.NoError(t, err) + require.Len(t, files, 100) + } +} diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 1741f30e1..36cac084d 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -376,11 +376,17 @@ func Analyze(ctx context.Context, a *Analyzer) (model.AnalyzedPaths, error) { return returnAnalyzedPaths, fmt.Errorf("failed to expand only-paths: %w", err) } // get all the files inside the given paths - for _, path := range a.Paths { + for _, path := range deduplicateAnalyzerRoots(a.Paths) { + if err := ctx.Err(); err != nil { + return returnAnalyzedPaths, err + } if _, err := os.Stat(path); err != nil { return returnAnalyzedPaths, errors.Wrap(err, "failed to analyze path") } if err := filepath.WalkDir(path, func(path string, d os.DirEntry, err error) error { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } if err != nil { return err } @@ -455,6 +461,9 @@ func Analyze(ctx context.Context, a *Analyzer) (model.AnalyzedPaths, error) { files = append(files, filepath.ToSlash(path)) return nil }); err != nil { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return returnAnalyzedPaths, err + } contextLogger.Error().Msgf("failed to analyze path %s: %s", path, err) } } @@ -543,6 +552,39 @@ func Analyze(ctx context.Context, a *Analyzer) (model.AnalyzedPaths, error) { return returnAnalyzedPaths, nil } +func deduplicateAnalyzerRoots(paths []string) []string { + roots := append([]string(nil), paths...) + sort.SliceStable(roots, func(i, j int) bool { + return len(filepath.Clean(roots[i])) < len(filepath.Clean(roots[j])) + }) + result := make([]string, 0, len(roots)) + for _, root := range roots { + cleanRoot := filepath.Clean(root) + covered := false + for _, parent := range result { + info, err := os.Stat(parent) + if err == nil && info.IsDir() && pathWithinRoot(parent, cleanRoot) { + covered = true + break + } + } + if !covered { + result = append(result, cleanRoot) + } + } + return result +} + +func pathWithinRoot(root, path string) bool { + absoluteRoot, rootErr := filepath.Abs(root) + absolutePath, pathErr := filepath.Abs(path) + if rootErr != nil || pathErr != nil { + return root == path + } + rel, err := filepath.Rel(absoluteRoot, absolutePath) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) +} + // worker determines the type of the file by ext (dockerfile and terraform)/content and // writes the answer to the results channel // if no types were found, the worker will write the path of the file in the unwanted channel diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index fcf6b8c1a..89fbfcf6c 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -476,6 +476,39 @@ func TestAnalyze_ValidSymlink(t *testing.T) { require.Empty(t, got.Exc) } +func TestAnalyzeDeduplicatesOverlappingRoots(t *testing.T) { + dir := t.TempDir() + nested := filepath.Join(dir, "nested") + require.NoError(t, os.MkdirAll(nested, 0o755)) + file := filepath.Join(nested, "main.tf") + require.NoError(t, os.WriteFile(file, []byte("resource \"aws_s3_bucket\" \"b\" {}\n"), 0o600)) + + got, err := Analyze(context.Background(), &Analyzer{ + RepoPath: dir, + Paths: []string{nested, dir, nested}, + Types: []string{""}, + MaxFileSize: -1, + }) + + require.NoError(t, err) + require.Equal(t, 1, got.ExpectedLOC) + require.Equal(t, []string{filepath.ToSlash(file)}, got.Inventory) +} + +func TestAnalyzeCanceledBeforeWalk(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := Analyze(ctx, &Analyzer{ + RepoPath: t.TempDir(), + Paths: []string{t.TempDir()}, + Types: []string{""}, + MaxFileSize: -1, + }) + + require.ErrorIs(t, err, context.Canceled) +} + func Test_checkHelm(t *testing.T) { helm := filepath.FromSlash("../../test/fixtures/analyzer_test/helm") tests := []struct { diff --git a/pkg/detector/search_line_detector.go b/pkg/detector/search_line_detector.go index e44dfeff5..7344e41b8 100644 --- a/pkg/detector/search_line_detector.go +++ b/pkg/detector/search_line_detector.go @@ -6,115 +6,196 @@ package detector import ( - "encoding/json" + "reflect" "strconv" "strings" "github.com/DataDog/datadog-iac-scanner/pkg/model" - "github.com/tidwall/gjson" ) -// searchLineDetector is the struct used to get the line from the payload with lines information -// content - payload with line information -// resolvedPath - string created from pathComponents, used to create gjson paths -// resolvedArrayPath - string created from pathComponents containing an array used to create gjson paths -// targetObj - key of the interface{}, we want the line from -type searchLineDetector struct { - content []byte - resolvedPath string - resolvedArrayPath string - targetObj string -} - -// GetLineBySearchLine makes use of the gjson pkg to find the line of a key in the original file -// with it's path given by a slice of strings +// GetLineBySearchLine finds a key's source line from the parser's line metadata. func GetLineBySearchLine(pathComponents []string, file *model.FileMetadata) (int, error) { - content, err := json.Marshal(file.LineInfoDocument) - if err != nil { - return -1, err + if len(pathComponents) == 0 { + return 1, nil } - detector := &searchLineDetector{ - content: content, + target := pathComponents[len(pathComponents)-1] + objectPath := pathComponents[:len(pathComponents)-1] + targetKey := "_dd_" + target + if line := lineAtJoinedPath(file.LineInfoDocument, objectPath, []string{"_dd_lines", targetKey, "_dd_line"}); line > 0 { + return line, nil + } + if line := lineAtJoinedPath(file.LineInfoDocument, objectPath, []string{target, "_dd_lines", "_dd__default", "_dd_line"}); line > 0 { + return line, nil } - - return detector.preparePath(pathComponents), nil -} - -// preparePath resolves the path components and retrives important information -// for the creation of the paths to search -func (d *searchLineDetector) preparePath(pathItems []string) int { - if len(pathItems) == 0 { - return 1 - } - // Escaping '.' in path components so it doesn't conflict with gjson pkg - objPath := strings.ReplaceAll(pathItems[0], ".", "\\.") - ArrPath := strings.ReplaceAll(pathItems[0], ".", "\\.") - - obj := pathItems[len(pathItems)-1] arrayObject := "" - - // Iterate reversely through the path components and get the key of the last array in the path - // needed for cases where the fields in the array are <"key": "value"> type and not foundArrayIdx := false - for i := len(pathItems) - 1; i >= 0; i-- { - if _, err := strconv.Atoi(pathItems[i]); err == nil { + for i := len(pathComponents) - 1; i >= 0; i-- { + if _, err := strconv.Atoi(pathComponents[i]); err == nil { foundArrayIdx = true continue } if foundArrayIdx { - arrayObject = pathItems[i] + arrayObject = pathComponents[i] break } } + arrayPath := make([]string, 1, len(pathComponents)*3) + arrayPath[0] = pathComponents[0] + if arrayObject == pathComponents[0] { + arrayPath = append(arrayPath[:0], "_dd_lines", "_dd_"+arrayObject, "_dd_arr") + } + if len(pathComponents) > 2 { + for _, pathItem := range pathComponents[1 : len(pathComponents)-1] { + if pathItem == arrayObject { + arrayPath = append(arrayPath, "_dd_lines", "_dd_"+pathItem, "_dd_arr") + } else { + arrayPath = append(arrayPath, pathItem) + } + } + } - if arrayObject == objPath { - ArrPath = "_dd_lines._dd_" + arrayObject + "._dd_arr" + if line := lineAtJoinedPath(file.LineInfoDocument, arrayPath, []string{target, "_dd__default", "_dd_line"}); line > 0 { + return line, nil } + if line := lineAtJoinedPath(file.LineInfoDocument, arrayPath, []string{targetKey, "_dd_line"}); line > 0 { + return line, nil + } + return -1, nil +} - var treatedPathItems []string - if len(pathItems) > 1 { - treatedPathItems = pathItems[1 : len(pathItems)-1] +func lineAtJoinedPath(root interface{}, prefix, suffix []string) int { + value := root + for _, path := range [2][]string{prefix, suffix} { + for _, component := range path { + var ok bool + value, ok = pathComponent(value, component) + if !ok { + return -1 + } + } } + return positiveInt(indirectValue(reflect.ValueOf(value))) +} - // Create a string based on the path components so it can be later transformed in a gjson path - for _, pathItem := range treatedPathItems { - // In case of an array present - if pathItem == arrayObject { - ArrPath += "._dd_lines._dd_" + strings.ReplaceAll(pathItem, ".", "\\.") + "._dd_arr" - } else { - ArrPath += "." + strings.ReplaceAll(pathItem, ".", "\\.") +func pathComponent(value interface{}, component string) (interface{}, bool) { + switch current := value.(type) { + case map[string]interface{}: + next, ok := current[component] + return next, ok + case model.Document: + next, ok := current[component] + return next, ok + case []interface{}: + index, ok := pathIndex(component, len(current)) + if !ok { + return nil, false + } + return current[index], true + case map[string]*model.LineObject: + next, ok := current[component] + return next, ok + case *model.LineObject: + if current == nil { + return nil, false + } + switch component { + case "_dd_line": + return current.Line, true + case "_dd_arr": + return current.Arr, true } - objPath += "." + strings.ReplaceAll(pathItem, ".", "\\.") + return nil, false + case []map[string]*model.LineObject: + index, ok := pathIndex(component, len(current)) + if !ok { + return nil, false + } + return current[index], true + default: + return reflectedPathComponent(value, component) } +} - d.resolvedPath = objPath - d.resolvedArrayPath = ArrPath - d.targetObj = obj +func pathIndex(component string, length int) (int, bool) { + index, err := strconv.Atoi(component) + return index, err == nil && index >= 0 && index < length +} - return d.getResult() +func reflectedPathComponent(current interface{}, component string) (interface{}, bool) { + value := indirectValue(reflect.ValueOf(current)) + if !value.IsValid() { + return nil, false + } + switch value.Kind() { + case reflect.Map: + if value.Type().Key().Kind() != reflect.String { + return nil, false + } + value = value.MapIndex(reflect.ValueOf(component).Convert(value.Type().Key())) + case reflect.Slice, reflect.Array: + index, ok := pathIndex(component, value.Len()) + if !ok { + return nil, false + } + value = value.Index(index) + case reflect.Struct: + value = structJSONField(value, component) + default: + return nil, false + } + value = indirectValue(value) + if !value.IsValid() || !value.CanInterface() { + return nil, false + } + return value.Interface(), true } -// getResult creates the paths to be used by gjson pkg to find the line in the content -func (d *searchLineDetector) getResult() int { - // Escape '.' like preparePath does for every other path segment, so a dotted - // targetObj isn't misread by gjson as nested path segments. - targetObj := strings.ReplaceAll(d.targetObj, ".", "\\.") - pathObjects := []string{ - d.resolvedPath + "._dd_lines._dd_" + targetObj + "._dd_line", - d.resolvedPath + "." + targetObj + "._dd_lines._dd__default._dd_line", - d.resolvedArrayPath + "." + targetObj + "._dd__default._dd_line", - d.resolvedArrayPath + "._dd_" + targetObj + "._dd_line", +func indirectValue(value reflect.Value) reflect.Value { + for value.IsValid() && (value.Kind() == reflect.Interface || value.Kind() == reflect.Pointer) { + if value.IsNil() { + return reflect.Value{} + } + value = value.Elem() } + return value +} - result := -1 - // run gjson pkg - for _, pathItem := range pathObjects { - if tmpResult := gjson.GetBytes(d.content, pathItem); int(tmpResult.Int()) > 0 { - result = int(tmpResult.Int()) - break +func structJSONField(value reflect.Value, name string) reflect.Value { + typ := value.Type() + for i := 0; i < typ.NumField(); i++ { + tag := strings.Split(typ.Field(i).Tag.Get("json"), ",")[0] + if tag == name { + return value.Field(i) + } + } + return reflect.Value{} +} + +func positiveInt(value reflect.Value) int { + if !value.IsValid() { + return -1 + } + var line int64 + switch value.Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + line = value.Int() + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + line = int64(value.Uint()) + case reflect.Float32, reflect.Float64: + line = int64(value.Float()) + case reflect.String: + parsed, err := strconv.ParseInt(value.String(), 10, 64) + if err != nil { + return -1 } + line = parsed + default: + return -1 + } + if line <= 0 { + return -1 } - return result + return int(line) } diff --git a/pkg/detector/search_line_detector_test.go b/pkg/detector/search_line_detector_test.go index eb6f555d8..c53a3f45f 100644 --- a/pkg/detector/search_line_detector_test.go +++ b/pkg/detector/search_line_detector_test.go @@ -6,6 +6,7 @@ package detector import ( + "strconv" "testing" "github.com/DataDog/datadog-iac-scanner/pkg/model" @@ -291,6 +292,25 @@ func TestGetLineBySearchLine(t *testing.T) { //nolint want: 7, wantErr: false, }, + { + name: "test typed line objects", + args: args{ + pathComponents: []string{"resource", "aws_instance", "name"}, + file: &model.FileMetadata{ + LineInfoDocument: map[string]interface{}{ + "resource": map[string]interface{}{ + "aws_instance": map[string]interface{}{ + "_dd_lines": map[string]*model.LineObject{ + "_dd_name": {Line: 12}, + }, + "name": "example", + }, + }, + }, + }, + }, + want: 12, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -305,3 +325,33 @@ func TestGetLineBySearchLine(t *testing.T) { //nolint }) } } + +func BenchmarkGetLineBySearchLineFindingCardinality(b *testing.B) { + file := &model.FileMetadata{ + LineInfoDocument: map[string]interface{}{ + "resource": map[string]interface{}{ + "aws_instance": map[string]interface{}{ + "_dd_lines": map[string]interface{}{ + "_dd_name": map[string]interface{}{"_dd_line": 42}, + }, + "name": "example", + }, + }, + }, + } + path := []string{"resource", "aws_instance", "name"} + + for _, count := range []int{10, 100, 500} { + b.Run(strconv.Itoa(count), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + for range count { + line, err := GetLineBySearchLine(path, file) + if err != nil || line != 42 { + b.Fatalf("line = %d, err = %v", line, err) + } + } + } + }) + } +} diff --git a/pkg/engine/module_resolve.go b/pkg/engine/module_resolve.go index f9ff0d526..9c9ecd07f 100644 --- a/pkg/engine/module_resolve.go +++ b/pkg/engine/module_resolve.go @@ -275,6 +275,12 @@ type moduleRefEntry struct { attrs interface{} } +type moduleCallScope struct { + entries []moduleRefEntry + refs map[string]interface{} + digest string +} + // instantiatedDocs emits one synthetic OPA doc per resolved module resource instance. // Identical callers are recorded in extras for post-eval finding cloning. func instantiatedDocs( @@ -285,21 +291,33 @@ func instantiatedDocs( extras map[string][]extraCallerInfo, ) (docs []model.Document, synthetic []*model.FileMetadata) { // Pass 1: index resources per call chain. - cckRefs := make(map[string][]moduleRefEntry) + callScopes := make(map[string]*moduleCallScope) + resourceAttrs := make(map[*tfeval.ResolvedResource]map[string]interface{}) for i := range resources { r := &resources[i] if r.ModuleAddress == "" { continue } cck := callChainKey(r, repoPath) - cckRefs[cck] = append(cckRefs[cck], moduleRefEntry{ + scope := callScopes[cck] + if scope == nil { + scope = &moduleCallScope{} + callScopes[cck] = scope + } + attrs := tfeval.AttributesToDocument(r) + resourceAttrs[r] = attrs + scope.entries = append(scope.entries, moduleRefEntry{ typ: r.Type, name: r.Name, definedIn: absPath(r.DefinedIn, repoPath), defLine: strconv.Itoa(r.DefLine), - attrs: tfeval.AttributesToDocument(r), + attrs: attrs, }) } + for _, scope := range callScopes { + scope.refs = buildCallRefsMap(scope.entries) + scope.digest = moduleCallScopeDigest(scope.entries) + } // Pass 2: emit one doc per instance. for i := range resources { @@ -317,21 +335,22 @@ func instantiatedDocs( docID := fm.ID + "\x00" + cck + "\x00" + r.Name // Identical resolved attributes dedupe to one OPA doc; extras get cloned findings after OPA. - contentKey := moduleCallContentKey(r, cckRefs[cck], repoPath) + scope := callScopes[cck] + contentKey := moduleCallContentKey(r, scope.digest, repoPath) if primaryDocID, dup := seen[contentKey]; dup { extras[primaryDocID] = append(extras[primaryDocID], extraCallerInfo{callChain: cck, docID: docID}) continue } seen[contentKey] = docID - refsMap := buildRefsMap(r, cckRefs[cck]) + refsMap := buildRefsMap(r, scope.refs) doc := model.Document{ "id": docID, "file": fm.FilePath, "resource": map[string]interface{}{ r.Type: map[string]interface{}{ - tfeval.ResourceBaseName(r.Name): tfeval.AttributesToDocument(r), + tfeval.ResourceBaseName(r.Name): resourceAttrs[r], }, }, } @@ -344,8 +363,9 @@ func instantiatedDocs( return docs, synthetic } -// moduleCallContentKey hashes the resource and every resolved resource in its module call. -func moduleCallContentKey(self *tfeval.ResolvedResource, allInCall []moduleRefEntry, repoPath string) string { +func moduleCallScopeDigest(allInCall []moduleRefEntry) string { + const fieldsPerRow = 5 + type row struct{ typ, name, definedIn, defLine, attrKey string } rows := make([]row, 0, len(allInCall)) for _, e := range allInCall { @@ -366,23 +386,26 @@ func moduleCallContentKey(self *tfeval.ResolvedResource, allInCall []moduleRefEn } return false }) + parts := make([]string, 0, len(rows)*fieldsPerRow) + for _, r := range rows { + parts = append(parts, r.typ, r.name, r.definedIn, r.defLine, r.attrKey) + } + return strings.Join(parts, "\x00") +} + +// moduleCallContentKey combines resource identity with its precomputed call-scope digest. +func moduleCallContentKey(self *tfeval.ResolvedResource, scopeDigest, repoPath string) string { parts := []string{ self.ModuleAddress, self.Type, self.Name, absPath(self.DefinedIn, repoPath), strconv.Itoa(self.DefLine), - } - for _, r := range rows { - parts = append(parts, r.typ, r.name, r.definedIn, r.defLine, r.attrKey) + scopeDigest, } return strings.Join(parts, "\x00") } -// buildRefsMap returns sibling resources in the same module call (for walk-based rules). -func buildRefsMap(self *tfeval.ResolvedResource, allInCall []moduleRefEntry) map[string]interface{} { +func buildCallRefsMap(allInCall []moduleRefEntry) map[string]interface{} { refs := make(map[string]interface{}) for _, e := range allInCall { - if e.typ == self.Type && e.name == self.Name { - continue - } inner, _ := refs[e.typ].(map[string]interface{}) if inner == nil { inner = make(map[string]interface{}) @@ -393,6 +416,23 @@ func buildRefsMap(self *tfeval.ResolvedResource, allInCall []moduleRefEntry) map return refs } +// buildRefsMap excludes self while sharing unaffected resource-type maps. +func buildRefsMap(self *tfeval.ResolvedResource, allRefs map[string]interface{}) map[string]interface{} { + refs := maps.Clone(allRefs) + selfType, _ := allRefs[self.Type].(map[string]interface{}) + if len(selfType) == 0 { + return refs + } + selfType = maps.Clone(selfType) + delete(selfType, self.Name) + if len(selfType) == 0 { + delete(refs, self.Type) + } else { + refs[self.Type] = selfType + } + return refs +} + // newInstanceFileMetadata clones fm for a synthetic doc (empty Document so Combine skips it). func newInstanceFileMetadata(fm *model.FileMetadata, id, callChain string) *model.FileMetadata { clone := *fm diff --git a/pkg/engine/module_resolve_test.go b/pkg/engine/module_resolve_test.go index ca1367c2f..a629250e9 100644 --- a/pkg/engine/module_resolve_test.go +++ b/pkg/engine/module_resolve_test.go @@ -9,11 +9,14 @@ import ( "context" "os" "path/filepath" + "strconv" "strings" "testing" "github.com/DataDog/datadog-iac-scanner/pkg/model" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/tfeval" + "github.com/zclconf/go-cty/cty" ) // docFileID is the prefix of synthetic doc ids: fileID\x00callChain. @@ -39,6 +42,38 @@ func fileMeta(id, path string) *model.FileMetadata { return &model.FileMetadata{ID: id, FilePath: path, Document: model.Document{}} } +func BenchmarkInstantiatedDocsCardinality(b *testing.B) { + for _, count := range []int{10, 100, 500} { + resources := make([]tfeval.ResolvedResource, count) + byAbsPath := make(map[string]*model.FileMetadata, count) + for i := range resources { + path := filepath.Join("/repo/module", "resource-"+strconv.Itoa(i)+".tf") + resources[i] = tfeval.ResolvedResource{ + Type: "resource_type_" + strconv.Itoa(i), + Name: "resource_" + strconv.Itoa(i), + Attributes: map[string]cty.Value{"value": cty.StringVal("value")}, + DefinedIn: path, + DefLine: i + 1, + ModuleAddress: "module.test", + } + byAbsPath[path] = fileMeta("file-"+strconv.Itoa(i), path) + } + + b.Run(strconv.Itoa(count), func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + instantiatedDocs( + resources, + byAbsPath, + "/repo", + make(map[string]string), + make(map[string][]extraCallerInfo), + ) + } + }) + } +} + func TestBuildRemoteResolverUsesVersion(t *testing.T) { repoPath := t.TempDir() callerFile := filepath.Join(repoPath, "stack", "main.tf") diff --git a/pkg/engine/provider/filesystem.go b/pkg/engine/provider/filesystem.go index 232f3473b..9da9924b1 100644 --- a/pkg/engine/provider/filesystem.go +++ b/pkg/engine/provider/filesystem.go @@ -458,6 +458,9 @@ func (s *FileSystemSourceProvider) walkDirectory(ctx context.Context, scanPath s onFile func(ctx context.Context, path string) error) error { var resolvedChartPaths []string return filepath.Walk(scanPath, func(path string, info os.FileInfo, err error) error { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } if err != nil { return err } diff --git a/pkg/engine/provider/filesystem_test.go b/pkg/engine/provider/filesystem_test.go index 908949c68..66b0b5309 100644 --- a/pkg/engine/provider/filesystem_test.go +++ b/pkg/engine/provider/filesystem_test.go @@ -77,6 +77,22 @@ func TestNewFileSystemSourceProvider(t *testing.T) { } } +func TestWalkDirectoryStopsOnCanceledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + provider := &FileSystemSourceProvider{excludes: make(map[string][]os.FileInfo)} + + err := provider.walkDirectory( + ctx, + t.TempDir(), + model.Extensions{}, + func(context.Context, string, *[]string) error { return nil }, + func(context.Context, string) error { return nil }, + ) + + require.ErrorIs(t, err, context.Canceled) +} + // TestFileSystemSourceProvider_GetSources tests the functions [GetSources()] and all the methods called by them func TestFileSystemSourceProvider_GetSources(t *testing.T) { //nolint if err := test.ChangeCurrentDir("datadog-iac-scanner"); err != nil { diff --git a/pkg/model/summary.go b/pkg/model/summary.go index 0df07b1ce..d617ee3a5 100644 --- a/pkg/model/summary.go +++ b/pkg/model/summary.go @@ -7,6 +7,7 @@ package model import ( "context" + "os" "path/filepath" "regexp" "sort" @@ -190,22 +191,36 @@ func getRelativePath(basePath, filePath string) string { } func replaceIfTemporaryPath(filePath string, pathExtractionMap map[string]ExtractedPathObject) string { - prettyPath := filePath + cleanFilePath := filepath.Clean(filePath) + matchedRoot := "" + var matchedValue ExtractedPathObject + matchedSuffix := "" for key, val := range pathExtractionMap { - if strings.Contains(filePath, key) { - splittedPath := strings.Split(filePath, key) - if !val.LocalPath { - // remove authentication information from the URL - sanitizedURL := removeURLCredentials(val.Path) - // remove query parameters '?key=value&key2=value' - return filepath.FromSlash(queryRegex.ReplaceAllString(sanitizedURL, "") + splittedPath[1]) - } - prettyPath = filepath.FromSlash(filepath.Base(val.Path) + splittedPath[1]) - } else { - prettyPath = filePath + cleanRoot := filepath.Clean(key) + relative, err := filepath.Rel(cleanRoot, cleanFilePath) + if err != nil || relative == ".." || + strings.HasPrefix(relative, ".."+string(os.PathSeparator)) { + continue + } + if len(cleanRoot) > len(matchedRoot) || + (len(cleanRoot) == len(matchedRoot) && cleanRoot < matchedRoot) { + matchedRoot = cleanRoot + matchedValue = val + matchedSuffix = relative } } - return prettyPath + if matchedRoot == "" { + return filePath + } + suffix := "" + if matchedSuffix != "." { + suffix = "/" + filepath.ToSlash(matchedSuffix) + } + if !matchedValue.LocalPath { + sanitizedURL := removeURLCredentials(matchedValue.Path) + return filepath.FromSlash(queryRegex.ReplaceAllString(sanitizedURL, "") + suffix) + } + return filepath.FromSlash(filepath.Base(matchedValue.Path) + suffix) } func removeAllURLCredentials(pathExtractionMap map[string]ExtractedPathObject) []string { diff --git a/pkg/model/summary_test.go b/pkg/model/summary_test.go index 85c7f8a4c..ff90e4595 100644 --- a/pkg/model/summary_test.go +++ b/pkg/model/summary_test.go @@ -18,17 +18,17 @@ import ( func TestCreateSummary(t *testing.T) { vulnerabilities := []Vulnerability{ { - ID: 1, - ScanID: "scanID", - FileID: "fileId", - FileName: "fileName", - QueryID: "QueryID", - CWE: "22", - QueryName: "query_name", - Severity: SeverityHigh, - Line: 1, - SearchKey: "searchKey", - Output: "-", + ID: 1, + ScanID: "scanID", + FileID: "fileId", + FileName: "fileName", + QueryID: "QueryID", + CWE: "22", + QueryName: "query_name", + Severity: SeverityHigh, + Line: 1, + SearchKey: "searchKey", + Output: "-", }, } @@ -92,11 +92,11 @@ func TestCreateSummary(t *testing.T) { CWE: "22", Files: []VulnerableFile{ { - FileName: "fileName", - Fingerprint: GetDatadogFingerprintHash(SCIInfo{}, "fileName", "", "", "", "QueryID", "", ""), - Line: 1, - SearchKey: "searchKey", - Value: nil, + FileName: "fileName", + Fingerprint: GetDatadogFingerprintHash(SCIInfo{}, "fileName", "", "", "", "QueryID", "", ""), + Line: 1, + SearchKey: "searchKey", + Value: nil, }, }, }, @@ -107,6 +107,32 @@ func TestCreateSummary(t *testing.T) { }) } +func TestReplaceIfTemporaryPathUsesLongestBoundaryPrefix(t *testing.T) { + root := filepath.Join(string(os.PathSeparator), "tmp", "modules") + nested := filepath.Join(root, "network") + mappings := map[string]ExtractedPathObject{ + root: { + Path: "https://example.com/root?token=redacted", + LocalPath: false, + }, + nested: { + Path: "https://example.com/network?token=redacted", + LocalPath: false, + }, + } + + require.Equal( + t, + filepath.FromSlash("https://example.com/network/main.tf"), + replaceIfTemporaryPath(filepath.Join(nested, "main.tf"), mappings), + ) + require.Equal( + t, + filepath.Join(root+"-other", "main.tf"), + replaceIfTemporaryPath(filepath.Join(root+"-other", "main.tf"), mappings), + ) +} + func TestModel_resolvePath(t *testing.T) { pwd, err := os.Getwd() if err != nil { diff --git a/pkg/parser/terraform/modules/list_modules.go b/pkg/parser/terraform/modules/list_modules.go index cdf1d3e90..f22a25e4a 100644 --- a/pkg/parser/terraform/modules/list_modules.go +++ b/pkg/parser/terraform/modules/list_modules.go @@ -1,7 +1,11 @@ package tfmodules import ( + "crypto/sha256" + "encoding/hex" + "path/filepath" "sort" + "strconv" "strings" ) @@ -15,10 +19,19 @@ type ListModuleEntry struct { RegistryScope string `json:"registry_scope,omitempty"` FileName string `json:"file_name"` DefLine int `json:"def_line"` + CallerPath string `json:"caller_path,omitempty"` + CallID string `json:"call_id,omitempty"` } // ListModuleEntries converts parsed modules to list-modules JSON rows. func ListModuleEntries(modules map[string]ParsedModule, includeLocal bool) []ListModuleEntry { + return ListModuleEntriesRelativeTo(modules, includeLocal, "") +} + +// ListModuleEntriesRelativeTo includes stable repository-relative call attribution. +func ListModuleEntriesRelativeTo( + modules map[string]ParsedModule, includeLocal bool, repositoryRoot string, +) []ListModuleEntry { entries := make([]ListModuleEntry, 0, len(modules)) for src := range modules { mod := modules[src] @@ -28,6 +41,7 @@ func ListModuleEntries(modules map[string]ParsedModule, includeLocal bool) []Lis if strings.HasPrefix(mod.Source, "__") || mod.Source == "" { continue } + callerPath := stableCallerPath(repositoryRoot, mod.FileName) entries = append(entries, ListModuleEntry{ Name: mod.Name, Source: mod.Source, @@ -36,6 +50,8 @@ func ListModuleEntries(modules map[string]ParsedModule, includeLocal bool) []Lis RegistryScope: mod.RegistryScope, FileName: mod.FileName, DefLine: mod.DefLine, + CallerPath: callerPath, + CallID: ModuleCallID(callerPath, &mod), }) } sort.Slice(entries, func(i, j int) bool { @@ -49,3 +65,25 @@ func ListModuleEntries(modules map[string]ParsedModule, includeLocal bool) []Lis }) return entries } + +func stableCallerPath(repositoryRoot, fileName string) string { + cleanFile := filepath.Clean(fileName) + if repositoryRoot != "" { + if rel, err := filepath.Rel(filepath.Clean(repositoryRoot), cleanFile); err == nil && + rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return filepath.ToSlash(rel) + } + } + return filepath.ToSlash(cleanFile) +} + +func ModuleCallID(callerPath string, mod *ParsedModule) string { + sum := sha256.Sum256([]byte(strings.Join([]string{ + callerPath, + strconv.Itoa(mod.DefLine), + mod.Name, + mod.Source, + mod.Version, + }, "\x00"))) + return hex.EncodeToString(sum[:]) +} diff --git a/pkg/parser/terraform/modules/list_modules_test.go b/pkg/parser/terraform/modules/list_modules_test.go index 0fcc8372a..90cb3e9ec 100644 --- a/pkg/parser/terraform/modules/list_modules_test.go +++ b/pkg/parser/terraform/modules/list_modules_test.go @@ -2,6 +2,7 @@ package tfmodules import ( "encoding/json" + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -36,6 +37,8 @@ func TestListModuleEntriesJSONShape(t *testing.T) { require.Equal(t, "public", row["registry_scope"]) require.Equal(t, "/repo/main.tf", row["file_name"]) require.Equal(t, float64(12), row["def_line"]) + require.Equal(t, "/repo/main.tf", row["caller_path"]) + require.NotEmpty(t, row["call_id"]) } func TestListModuleEntriesSkipsLocalByDefault(t *testing.T) { @@ -50,3 +53,24 @@ func TestListModuleEntriesSkipsLocalByDefault(t *testing.T) { require.Len(t, entries, 1) require.Equal(t, "local", entries[0].Name) } + +func TestListModuleEntriesUsesStableRepositoryRelativeCallID(t *testing.T) { + module := ParsedModule{ + Name: "vpc", + Source: "terraform-aws-modules/vpc/aws", + Version: "~> 5.0", + FileName: filepath.Join("/checkout-a", "infra", "main.tf"), + DefLine: 12, + } + first := ListModuleEntriesRelativeTo( + map[string]ParsedModule{"a": module}, false, "/checkout-a", + ) + module.FileName = filepath.Join("/checkout-b", "infra", "main.tf") + second := ListModuleEntriesRelativeTo( + map[string]ParsedModule{"a": module}, false, "/checkout-b", + ) + + require.Equal(t, "infra/main.tf", first[0].CallerPath) + require.Equal(t, first[0].CallerPath, second[0].CallerPath) + require.Equal(t, first[0].CallID, second[0].CallID) +} diff --git a/pkg/parser/terraform/modules/modulegraph/modulegraph.go b/pkg/parser/terraform/modules/modulegraph/modulegraph.go index 4b472ac8b..a1df2c6d7 100644 --- a/pkg/parser/terraform/modules/modulegraph/modulegraph.go +++ b/pkg/parser/terraform/modules/modulegraph/modulegraph.go @@ -7,6 +7,8 @@ package modulegraph import ( "context" + "errors" + "fmt" "os" "path/filepath" "runtime" @@ -28,20 +30,27 @@ const ( ) type Request struct { - RootPaths []string - DiscoveryPaths []string - Resolver resolver.Resolver - MaxDepth int - FS vfs.FS + RootPaths []string + DiscoveryPaths []string + Resolver resolver.Resolver + MaxDepth int + FS vfs.FS + CallScopedResolution bool + FailOnUnresolved bool } type ResolvedModule struct { - CallerRoot string - Source string - Version string - Name string - LocalPath string - CanonicalSource string + CallerRoot string + Source string + Version string + RequestedVersion string + ResolvedVersion string + Name string + LocalPath string + CanonicalSource string + ContentDigest string + Provenance string + Outcome string } type Result struct { @@ -49,6 +58,7 @@ type Result struct { Modules []ResolvedModule SourceMappings map[string]string Cleanup func() + Error error } type resolvedEntry struct { @@ -66,6 +76,7 @@ type walkerSnapshot struct { modules []ResolvedModule sourceMappings map[string]string cleanups []func() + errors []error } type visitedSet struct { @@ -84,6 +95,7 @@ type resultCollector struct { modules []ResolvedModule sourceMappings map[string]string cleanups []func() + errors []error } type moduleParseCache struct { @@ -100,6 +112,8 @@ type walker struct { resolver resolver.Resolver maxDepth int fsys vfs.FS + callScoped bool + failOnError bool } func Resolve(ctx context.Context, request *Request) Result { @@ -124,9 +138,11 @@ func Resolve(ctx context.Context, request *Request) Result { parseCache: moduleParseCache{ entries: make(map[string]map[string]tfmodules.ParsedModule), }, - resolver: request.Resolver, - maxDepth: request.MaxDepth, - fsys: request.FS, + resolver: request.Resolver, + maxDepth: request.MaxDepth, + fsys: request.FS, + callScoped: request.CallScopedResolution, + failOnError: request.FailOnUnresolved, } seedGroups, repositoryGroups := w.seedGroups(ctx, request.RootPaths, request.DiscoveryPaths) @@ -144,6 +160,10 @@ func Resolve(ctx context.Context, request *Request) Result { result.ScanPaths = snapshot.paths result.Modules = snapshot.modules result.SourceMappings = snapshot.sourceMappings + sort.Slice(snapshot.errors, func(i, j int) bool { + return snapshot.errors[i].Error() < snapshot.errors[j].Error() + }) + result.Error = errors.Join(snapshot.errors...) sort.Strings(result.ScanPaths) sort.Slice(result.Modules, func(i, j int) bool { @@ -217,19 +237,42 @@ func (c *resultCollector) addPaths(paths ...string) { c.paths = append(c.paths, paths...) } -func (c *resultCollector) addResolvedModule(mod *tfmodules.ParsedModule, localPath string) { +func (c *resultCollector) addResolvedModule(mod *tfmodules.ParsedModule, resolution *resolver.Resolution) { c.mu.Lock() defer c.mu.Unlock() + resolvedVersion := resolution.ResolvedVersion + version := resolvedVersion + if version == "" { + version = mod.Version + } + canonicalSource := resolution.CanonicalSource + if canonicalSource == "" { + canonicalSource = canonicalModuleURL(mod.Source, version) + } c.modules = append(c.modules, ResolvedModule{ - CallerRoot: moduleCallRoot(mod), - Source: mod.Source, - Version: mod.Version, - Name: mod.Name, - LocalPath: localPath, - CanonicalSource: canonicalModuleURL(mod.Source, mod.Version), + CallerRoot: moduleCallRoot(mod), + Source: mod.Source, + Version: version, + RequestedVersion: mod.Version, + ResolvedVersion: resolvedVersion, + Name: mod.Name, + LocalPath: resolution.LocalPath, + CanonicalSource: canonicalSource, + ContentDigest: resolution.ContentDigest, + Provenance: resolution.Provenance, + Outcome: resolution.Outcome, }) } +func (c *resultCollector) addError(err error) { + if err == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + c.errors = append(c.errors, err) +} + func (c *resultCollector) addSourceMapping(localPath, source string) { c.mu.Lock() defer c.mu.Unlock() @@ -254,6 +297,7 @@ func (c *resultCollector) snapshot() walkerSnapshot { modules: append([]ResolvedModule(nil), c.modules...), sourceMappings: sourceMappings, cleanups: append([]func(){}, c.cleanups...), + errors: append([]error(nil), c.errors...), } } @@ -264,10 +308,10 @@ func (c *resolutionCache) get(resolveID string) (resolvedEntry, bool) { return entry, ok } -func (c *resolutionCache) set(resolveID string, entry resolvedEntry) { +func (c *resolutionCache) set(resolveID string, entry *resolvedEntry) { c.mu.Lock() defer c.mu.Unlock() - c.entries[resolveID] = entry + c.entries[resolveID] = *entry } func (c *moduleParseCache) get(key string) (map[string]tfmodules.ParsedModule, bool) { @@ -286,7 +330,7 @@ func (c *moduleParseCache) set(key string, mods map[string]tfmodules.ParsedModul func (w *walker) resolveRemote( ctx context.Context, mod *tfmodules.ParsedModule, ) (resolver.Resolution, bool, error) { - resolveID := remoteResolveIdentity(mod) + resolveID := remoteResolveIdentity(mod, w.callScoped) if entry, ok := w.resolutions.get(resolveID); ok { return entry.res, true, entry.err @@ -298,7 +342,7 @@ func (w *walker) resolveRemote( } resolution, resolveErr := w.resolver.Resolve(ctx, mod) - w.resolutions.set(resolveID, resolvedEntry{res: resolution, err: resolveErr}) + w.resolutions.set(resolveID, &resolvedEntry{res: resolution, err: resolveErr}) return resolution, resolveErr }) if err != nil { @@ -446,12 +490,21 @@ func (w *walker) traverseRemoteModules( if mod.IsLocal { continue } - id := remoteResolveIdentity(&mod) + id := remoteResolveIdentity(&mod, w.callScoped) cached, hit := w.resolutions.get(id) if hit { if cached.err == nil && cached.res.LocalPath != "" { - w.results.addResolvedModule(&mod, cached.res.LocalPath) + w.results.addResolvedModule(&mod, &cached.res) + } else if w.failOnError { + resolveErr := cached.err + if resolveErr == nil { + resolveErr = errors.New("resolver returned an empty local path") + } + w.results.addError(fmt.Errorf( + "resolving remote module %q from %s: %w", + mod.Source, mod.FileName, resolveErr, + )) } continue } @@ -491,6 +544,12 @@ func (w *walker) traverseRemoteModuleGroup( contextLogger.Debug().Msgf("Fetching remote Terraform module %q", representative.Source) resolution, shared, err := w.resolveRemote(ctx, representative) if err != nil { + if w.failOnError { + w.results.addError(fmt.Errorf( + "resolving remote module %q from %s: %w", + representative.Source, representative.FileName, err, + )) + } if !shared { contextLogger.Debug().Err(err). Msgf("Failed to resolve remote Terraform module %q", representative.Source) @@ -498,6 +557,12 @@ func (w *walker) traverseRemoteModuleGroup( return } if resolution.LocalPath == "" { + if w.failOnError { + w.results.addError(fmt.Errorf( + "resolving remote module %q from %s: resolver returned an empty local path", + representative.Source, representative.FileName, + )) + } if !shared { contextLogger.Debug(). Msgf("Resolved remote Terraform module %q without a local path", representative.Source) @@ -509,7 +574,7 @@ func (w *walker) traverseRemoteModuleGroup( } for _, mod := range group.callers { - w.results.addResolvedModule(mod, resolution.LocalPath) + w.results.addResolvedModule(mod, &resolution) } if !w.visited.tryAdd(resolution.LocalPath) { return @@ -518,10 +583,15 @@ func (w *walker) traverseRemoteModuleGroup( w.results.addCleanup(resolution.Cleanup) } w.results.addPaths(flatTerraformFilePaths(resolution.LocalPath)...) - w.results.addSourceMapping( - resolution.LocalPath, - canonicalModuleURL(representative.Source, representative.Version), - ) + canonicalSource := resolution.CanonicalSource + if canonicalSource == "" { + version := resolution.ResolvedVersion + if version == "" { + version = representative.Version + } + canonicalSource = canonicalModuleURL(representative.Source, version) + } + w.results.addSourceMapping(resolution.LocalPath, canonicalSource) w.traverse(ctx, resolution.LocalPath, nil, repoAllowedDirs, depth+1, true) } @@ -532,7 +602,10 @@ func moduleCallRoot(mod *tfmodules.ParsedModule) string { return filepath.Clean(filepath.Dir(mod.FileName)) } -func remoteResolveIdentity(mod *tfmodules.ParsedModule) string { +func remoteResolveIdentity(mod *tfmodules.ParsedModule, callScoped bool) string { + if callScoped { + return callKey(moduleCallRoot(mod), mod.Source, mod.Version, mod.Name) + } sourceType, _ := tfmodules.DetectModuleSourceType(mod.Source) if sourceType == sourceTypeRegistry && mod.Version == "" { return callKey(moduleCallRoot(mod), mod.Source, mod.Version, mod.Name) diff --git a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go index 8405028d1..f1100a1bc 100644 --- a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go +++ b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go @@ -2,6 +2,7 @@ package modulegraph import ( "context" + "errors" "os" "path/filepath" "sync/atomic" @@ -43,15 +44,60 @@ func TestResolveAssemblesModuleMetadataAndPaths(t *testing.T) { moduleDir: "git::https://github.com/acme/network//modules/vpc?ref=v1@1.2.3", }, result.SourceMappings) require.Equal(t, []ResolvedModule{{ - CallerRoot: root, - Source: "git::https://git@github.com/acme/network.git//modules/vpc?ref=v1", - Version: "1.2.3", - Name: "network", - LocalPath: moduleDir, - CanonicalSource: "git::https://github.com/acme/network//modules/vpc?ref=v1@1.2.3", + CallerRoot: root, + Source: "git::https://git@github.com/acme/network.git//modules/vpc?ref=v1", + Version: "1.2.3", + RequestedVersion: "1.2.3", + Name: "network", + LocalPath: moduleDir, + CanonicalSource: "git::https://github.com/acme/network//modules/vpc?ref=v1@1.2.3", }}, result.Modules) } +func TestResolveReportsUnresolvedModulesWhenRequired(t *testing.T) { + root, _ := writeModuleGraphFixture(t) + result := Resolve(t.Context(), &Request{ + RootPaths: []string{root}, + DiscoveryPaths: []string{filepath.Join(root, "main.tf")}, + Resolver: stubResolver{resolve: func(*tfmodules.ParsedModule) (resolver.Resolution, error) { + return resolver.Resolution{}, errors.New("missing hosted artifact") + }}, + MaxDepth: 2, + FailOnUnresolved: true, + }) + + require.ErrorContains(t, result.Error, "missing hosted artifact") +} + +func TestResolvePreservesConcreteResolutionMetadata(t *testing.T) { + root, moduleDir := writeModuleGraphFixture(t) + result := Resolve(context.Background(), &Request{ + RootPaths: []string{root}, + DiscoveryPaths: []string{filepath.Join(root, "main.tf")}, + Resolver: stubResolver{resolution: resolver.Resolution{ + LocalPath: moduleDir, + RequestedVersion: "1.2.3", + ResolvedVersion: "1.2.4", + CanonicalSource: "https://example.com/acme/network@1.2.4", + ContentDigest: "sha256:abc", + Provenance: "prefetched", + Outcome: "resolved", + }}, + MaxDepth: 2, + }) + + require.Len(t, result.Modules, 1) + module := result.Modules[0] + require.Equal(t, "1.2.4", module.Version) + require.Equal(t, "1.2.3", module.RequestedVersion) + require.Equal(t, "1.2.4", module.ResolvedVersion) + require.Equal(t, "https://example.com/acme/network@1.2.4", module.CanonicalSource) + require.Equal(t, "sha256:abc", module.ContentDigest) + require.Equal(t, "prefetched", module.Provenance) + require.Equal(t, "resolved", module.Outcome) + require.Equal(t, module.CanonicalSource, result.SourceMappings[moduleDir]) +} + func TestResolveCleanupIsIdempotent(t *testing.T) { root, moduleDir := writeModuleGraphFixture(t) var cleanupCalls atomic.Int32 @@ -170,6 +216,38 @@ module "b" { source = "git::https://example.com/mod.git?ref=main" } require.Len(t, result.Modules, 2) } +func TestResolveKeepsHostedCallsDistinct(t *testing.T) { + root := t.TempDir() + remoteA, remoteB := filepath.Join(root, "remote-a"), filepath.Join(root, "remote-b") + for _, dir := range []string{remoteA, remoteB} { + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "main.tf"), []byte(`resource "x" "remote" {}`), 0o644)) + } + rootFile := filepath.Join(root, "main.tf") + require.NoError(t, os.WriteFile(rootFile, []byte(` +module "a" { source = "git::https://example.com/mod.git?ref=main" } +module "b" { source = "git::https://example.com/mod.git?ref=main" } +`), 0o644)) + + result := Resolve(t.Context(), &Request{ + RootPaths: []string{rootFile}, + DiscoveryPaths: []string{rootFile}, + Resolver: stubResolver{resolve: func(module *tfmodules.ParsedModule) (resolver.Resolution, error) { + if module.Name == "a" { + return resolver.Resolution{LocalPath: remoteA}, nil + } + return resolver.Resolution{LocalPath: remoteB}, nil + }}, + MaxDepth: 1, + CallScopedResolution: true, + }) + + require.ElementsMatch(t, []string{ + filepath.Join(remoteA, "main.tf"), + filepath.Join(remoteB, "main.tf"), + }, result.ScanPaths) +} + func TestResolveKeepsUnversionedRegistryCallsDistinctByName(t *testing.T) { root := t.TempDir() remoteA := filepath.Join(root, "remote-a") @@ -234,10 +312,10 @@ func TestCanonicalGitModuleSource(t *testing.T) { t, remoteResolveIdentity(&tfmodules.ParsedModule{ Source: "git::https://github.com/org/repo.git//sub?ref=v1", - }), + }, false), remoteResolveIdentity(&tfmodules.ParsedModule{ Source: "git::ssh://git@github.com/org/repo//sub?ref=v1", - }), + }, false), ) } diff --git a/pkg/parser/terraform/modules/resolver/prefetched.go b/pkg/parser/terraform/modules/resolver/prefetched.go index 3a165b06c..418989c3a 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched.go +++ b/pkg/parser/terraform/modules/resolver/prefetched.go @@ -11,6 +11,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" @@ -19,15 +20,27 @@ import ( // ManifestEntry is one row in a --modules-manifest JSON file. type ManifestEntry struct { - LocalPath string `json:"local_path"` - Version string `json:"version,omitempty"` - Origin string `json:"origin,omitempty"` + LocalPath string `json:"local_path,omitempty"` + Source string `json:"source,omitempty"` + Name string `json:"name,omitempty"` + CallerPath string `json:"caller_path,omitempty"` + CallID string `json:"call_id,omitempty"` + Version string `json:"version,omitempty"` + Origin string `json:"origin,omitempty"` + RequestedVersion string `json:"requested_version,omitempty"` + ResolvedVersion string `json:"resolved_version,omitempty"` + CanonicalSource string `json:"canonical_source,omitempty"` + ContentDigest string `json:"content_digest,omitempty"` + Provenance string `json:"provenance,omitempty"` + Outcome string `json:"outcome,omitempty"` } // Manifest is validated JSON: optional root Dir plus source or source@version → ManifestEntry. type Manifest struct { - Dir string `json:"dir"` - Modules map[string]ManifestEntry `json:"modules"` + Version int `json:"version,omitempty"` + SchemaVersion int `json:"schema_version,omitempty"` + Dir string `json:"dir"` + Modules map[string]ManifestEntry `json:"modules"` } // LoadManifest parses and validates manifest JSON from path. @@ -47,30 +60,83 @@ func LoadManifest(path string) (*Manifest, error) { } func (m *Manifest) validate() error { - for src, entry := range m.Modules { - if !filepath.IsAbs(entry.LocalPath) { - return fmt.Errorf("entry %q: local_path must be absolute, got %q", src, entry.LocalPath) - } - if m.Dir != "" { - rel, err := filepath.Rel(m.Dir, entry.LocalPath) - if err != nil || pathEscapesDir(rel) { - return fmt.Errorf("entry %q: local_path %q is not confined to dir %q", src, entry.LocalPath, m.Dir) - } - } - resolved, err := filepath.EvalSymlinks(entry.LocalPath) - if err != nil { - return fmt.Errorf("entry %q: cannot resolve symlinks for %q: %w", src, entry.LocalPath, err) - } - if m.Dir != "" { - rel, err := filepath.Rel(m.Dir, resolved) - if err != nil || pathEscapesDir(rel) { - return fmt.Errorf("entry %q: resolved path %q escapes dir %q", src, resolved, m.Dir) - } + if err := validateManifestVersions(m.Version, m.SchemaVersion); err != nil { + return err + } + resolvedDir, err := validateManifestDir(m.Dir) + if err != nil { + return err + } + for src := range m.Modules { + entry := m.Modules[src] + if err := validateManifestEntry(src, &entry, m.Dir, resolvedDir); err != nil { + return err } } return nil } +func validateManifestVersions(version, schemaVersion int) error { + if version < 0 || version > 2 { + return fmt.Errorf("unsupported manifest version %d", version) + } + if schemaVersion < 0 || schemaVersion > 2 { + return fmt.Errorf("unsupported schema_version %d", schemaVersion) + } + if version != 0 && schemaVersion != 0 && version != schemaVersion { + return fmt.Errorf("manifest version %d does not match schema_version %d", version, schemaVersion) + } + return nil +} + +func validateManifestDir(dir string) (string, error) { + if dir == "" { + return "", nil + } + if !filepath.IsAbs(dir) { + return "", fmt.Errorf("manifest dir must be absolute, got %q", dir) + } + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + return "", fmt.Errorf("cannot resolve manifest dir %q: %w", dir, err) + } + return resolved, nil +} + +func validateManifestEntry(src string, entry *ManifestEntry, dir, resolvedDir string) error { + if entry.Version != "" && entry.ResolvedVersion != "" && entry.Version != entry.ResolvedVersion { + return fmt.Errorf( + "entry %q: version %q does not match resolved_version %q", + src, entry.Version, entry.ResolvedVersion, + ) + } + if !manifestEntryResolved(entry.Outcome) { + return nil + } + if entry.LocalPath == "" { + return fmt.Errorf("entry %q: resolved module requires local_path", src) + } + if !filepath.IsAbs(entry.LocalPath) { + return fmt.Errorf("entry %q: local_path must be absolute, got %q", src, entry.LocalPath) + } + if dir != "" && !pathConfinedTo(dir, entry.LocalPath) { + return fmt.Errorf("entry %q: local_path %q is not confined to dir %q", src, entry.LocalPath, dir) + } + resolved, err := filepath.EvalSymlinks(entry.LocalPath) + if err != nil { + return fmt.Errorf("entry %q: cannot resolve symlinks for %q: %w", src, entry.LocalPath, err) + } + if resolvedDir != "" && !pathConfinedTo(resolvedDir, resolved) { + return fmt.Errorf("entry %q: resolved path %q escapes dir %q", src, resolved, dir) + } + return nil +} + +func pathConfinedTo(dir, path string) bool { + rel, err := filepath.Rel(dir, path) + return err == nil && !pathEscapesDir(rel) +} + func pathEscapesDir(rel string) bool { return rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) } @@ -88,21 +154,137 @@ func (r *PrefetchedResolver) Resolve(_ context.Context, mod *tfmodules.ParsedMod if mod.IsLocal { return Resolution{}, &tfmodules.UnresolvedError{Reason: "local modules are handled by LocalResolver"} } - entry, ok := r.manifest.Modules[manifestModuleKey(mod.Source, mod.Version)] - if !ok { - entry, ok = r.manifest.Modules[mod.Source] - } + entry, ok := r.findEntry(mod) if !ok { return Resolution{}, &tfmodules.UnresolvedError{ Reason: fmt.Sprintf("module %q not found in manifest", mod.Source), } } - if entry.Version != "" && mod.Version != "" && !versionMatchesConstraint(entry.Version, mod.Version) { + if !manifestEntryResolved(entry.Outcome) { return Resolution{}, &tfmodules.UnresolvedError{ - Reason: fmt.Sprintf("module %q version %q not found in manifest", mod.Source, mod.Version), + Reason: fmt.Sprintf("module %q manifest outcome is %q", mod.Source, entry.Outcome), + } + } + if entry.RequestedVersion != "" && + strings.TrimSpace(entry.RequestedVersion) != strings.TrimSpace(mod.Version) { + return Resolution{}, &tfmodules.UnresolvedError{ + Reason: fmt.Sprintf( + "module %q requested version %q does not match manifest request %q", + mod.Source, mod.Version, entry.RequestedVersion, + ), + } + } + resolvedVersion := entry.Version + if resolvedVersion == "" { + resolvedVersion = entry.ResolvedVersion + } + if resolvedVersion != "" && mod.Version != "" && !versionMatchesConstraint(resolvedVersion, mod.Version) { + return Resolution{}, &tfmodules.UnresolvedError{ + Reason: fmt.Sprintf( + "module %q resolved version %q does not satisfy requested constraint %q", + mod.Source, resolvedVersion, mod.Version, + ), + } + } + return Resolution{ + LocalPath: entry.LocalPath, + RequestedVersion: mod.Version, + ResolvedVersion: resolvedVersion, + CanonicalSource: entry.CanonicalSource, + ContentDigest: entry.ContentDigest, + Provenance: firstNonEmpty(entry.Provenance, entry.Origin), + Outcome: firstNonEmpty(entry.Outcome, "resolved"), + }, nil +} + +func (r *PrefetchedResolver) findEntry(mod *tfmodules.ParsedModule) (ManifestEntry, bool) { + keys := make([]string, 0, len(r.manifest.Modules)) + for key := range r.manifest.Modules { + keys = append(keys, key) + } + sort.Strings(keys) + if entry, found, scoped := r.findScopedEntry(mod, keys); found || scoped { + return entry, found + } + return r.findLegacyEntry(mod, keys) +} + +func (r *PrefetchedResolver) findScopedEntry( + mod *tfmodules.ParsedModule, + keys []string, +) (ManifestEntry, bool, bool) { + hasScopedEntries := false + for _, key := range keys { + entry := r.manifest.Modules[key] + if entry.CallID == "" || entry.CallerPath == "" { + continue + } + if entry.Source != "" && entry.Source != mod.Source { + continue + } + if entry.RequestedVersion != "" && + strings.TrimSpace(entry.RequestedVersion) != strings.TrimSpace(mod.Version) { + continue + } + hasScopedEntries = true + if !callerPathMatches(mod.FileName, entry.CallerPath) { + continue + } + if entry.CallID == tfmodules.ModuleCallID(entry.CallerPath, mod) { + return entry, true, true + } + } + return ManifestEntry{}, false, hasScopedEntries +} + +func (r *PrefetchedResolver) findLegacyEntry( + mod *tfmodules.ParsedModule, + keys []string, +) (ManifestEntry, bool) { + for _, key := range []string{manifestModuleKey(mod.Source, mod.Version), mod.Source} { + if entry, ok := r.manifest.Modules[key]; ok { + return entry, true + } + } + for _, key := range keys { + entry := r.manifest.Modules[key] + if entry.CallID != "" { + continue + } + if entry.CanonicalSource == mod.Source && + (entry.RequestedVersion == "" || + strings.TrimSpace(entry.RequestedVersion) == strings.TrimSpace(mod.Version)) { + return entry, true + } + } + return ManifestEntry{}, false +} + +func callerPathMatches(fileName, callerPath string) bool { + fileName = filepath.ToSlash(filepath.Clean(fileName)) + callerPath = filepath.ToSlash(filepath.Clean(callerPath)) + if filepath.IsAbs(callerPath) { + return fileName == callerPath + } + return fileName == callerPath || strings.HasSuffix(fileName, "/"+callerPath) +} + +func manifestEntryResolved(outcome string) bool { + switch strings.ToLower(strings.TrimSpace(outcome)) { + case "", "resolved", "success": + return true + default: + return false + } +} + +func firstNonEmpty(values ...string) string { + for _, value := range values { + if value != "" { + return value } } - return Resolution{LocalPath: entry.LocalPath}, nil + return "" } func versionMatchesConstraint(version, constraint string) bool { diff --git a/pkg/parser/terraform/modules/resolver/prefetched_test.go b/pkg/parser/terraform/modules/resolver/prefetched_test.go index 8d0b8f412..35bf1e3e7 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched_test.go +++ b/pkg/parser/terraform/modules/resolver/prefetched_test.go @@ -19,12 +19,19 @@ func TestLoadManifestJSONShape(t *testing.T) { manifestPath := filepath.Join(resolvedDir, "modules.json") manifestData, err := json.Marshal(map[string]any{ - "dir": resolvedDir, + "version": 2, + "dir": resolvedDir, "modules": map[string]any{ "terraform-aws-modules/vpc/aws@5.0.0": map[string]any{ - "local_path": moduleDir, - "version": "5.0.0", - "origin": "registry", + "local_path": moduleDir, + "version": "5.0.0", + "origin": "registry", + "requested_version": "~> 5.0", + "resolved_version": "5.0.0", + "canonical_source": "registry.terraform.io/terraform-aws-modules/vpc/aws", + "content_digest": "sha256:abc", + "provenance": "prefetched", + "outcome": "resolved", }, }, }) @@ -34,6 +41,7 @@ func TestLoadManifestJSONShape(t *testing.T) { m, err := LoadManifest(manifestPath) require.NoError(t, err) require.Equal(t, resolvedDir, m.Dir) + require.Equal(t, 2, m.Version) data, err := json.Marshal(m) require.NoError(t, err) @@ -45,6 +53,32 @@ func TestLoadManifestJSONShape(t *testing.T) { require.Equal(t, moduleDir, entry["local_path"]) require.Equal(t, "5.0.0", entry["version"]) require.Equal(t, "registry", entry["origin"]) + require.Equal(t, "~> 5.0", entry["requested_version"]) + require.Equal(t, "5.0.0", entry["resolved_version"]) + require.Equal(t, "sha256:abc", entry["content_digest"]) +} + +func TestLoadManifestAcceptsV1Fields(t *testing.T) { + moduleDir := t.TempDir() + manifestPath := filepath.Join(t.TempDir(), "modules.json") + data, err := json.Marshal(map[string]any{ + "version": 1, + "modules": map[string]any{ + "terraform-aws-modules/vpc/aws@5.0.0": map[string]any{ + "local_path": moduleDir, + "version": "5.0.0", + "origin": "registry", + }, + }, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + manifest, err := LoadManifest(manifestPath) + + require.NoError(t, err) + require.Equal(t, 1, manifest.Version) + require.Equal(t, "5.0.0", manifest.Modules["terraform-aws-modules/vpc/aws@5.0.0"].Version) } func TestPrefetchedResolverUsesManifest(t *testing.T) { @@ -98,3 +132,134 @@ func TestPrefetchedResolverValidatesResolvedVersionAgainstConstraint(t *testing. }) require.Error(t, err) } + +func TestPrefetchedResolverReturnsResolutionMetadata(t *testing.T) { + moduleDir := t.TempDir() + resolver := NewPrefetchedResolver(&Manifest{ + SchemaVersion: 2, + Modules: map[string]ManifestEntry{ + "legacy-key": { + LocalPath: moduleDir, + Version: "5.1.2", + RequestedVersion: "~> 5.0", + ResolvedVersion: "5.1.2", + CanonicalSource: "registry.terraform.io/terraform-aws-modules/vpc/aws", + ContentDigest: "sha256:abc", + Provenance: "hosted-cache", + Outcome: "resolved", + }, + }, + }) + + got, err := resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Source: "registry.terraform.io/terraform-aws-modules/vpc/aws", + Version: "~> 5.0", + }) + + require.NoError(t, err) + require.Equal(t, "5.1.2", got.ResolvedVersion) + require.Equal(t, "~> 5.0", got.RequestedVersion) + require.Equal(t, "sha256:abc", got.ContentDigest) + require.Equal(t, "hosted-cache", got.Provenance) + require.Equal(t, "resolved", got.Outcome) +} + +func TestPrefetchedResolverRejectsManifestRequestedVersionMismatch(t *testing.T) { + resolver := NewPrefetchedResolver(&Manifest{ + Modules: map[string]ManifestEntry{ + "terraform-aws-modules/vpc/aws": { + LocalPath: t.TempDir(), + Version: "5.1.2", + RequestedVersion: "~> 5.0", + }, + }, + }) + + _, err := resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Source: "terraform-aws-modules/vpc/aws", + Version: "~> 6.0", + }) + + require.ErrorContains(t, err, "does not match manifest request") +} + +func TestPrefetchedResolverSelectsCallerSpecificEntry(t *testing.T) { + source := "terraform-aws-modules/vpc/aws" + version := "~> 5.0" + first := &tfmodules.ParsedModule{ + Name: "vpc", Source: source, Version: version, FileName: "/checkout/stacks/first/main.tf", DefLine: 3, + } + second := &tfmodules.ParsedModule{ + Name: "vpc", Source: source, Version: version, FileName: "/checkout/stacks/second/main.tf", DefLine: 3, + } + firstPath, secondPath := t.TempDir(), t.TempDir() + resolver := NewPrefetchedResolver(&Manifest{ + Modules: map[string]ManifestEntry{ + manifestModuleKey(source, version): { + LocalPath: firstPath, Source: source, Version: "5.1.0", + }, + "first": { + LocalPath: firstPath, Source: source, Name: first.Name, + CallerPath: "stacks/first/main.tf", + CallID: tfmodules.ModuleCallID("stacks/first/main.tf", first), + Version: "5.1.0", + }, + "second": { + LocalPath: secondPath, Source: source, Name: second.Name, + CallerPath: "stacks/second/main.tf", + CallID: tfmodules.ModuleCallID("stacks/second/main.tf", second), + Version: "5.2.0", + }, + }, + }) + + firstResolution, err := resolver.Resolve(t.Context(), first) + require.NoError(t, err) + secondResolution, err := resolver.Resolve(t.Context(), second) + require.NoError(t, err) + require.Equal(t, firstPath, firstResolution.LocalPath) + require.Equal(t, secondPath, secondResolution.LocalPath) + + _, err = resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Name: "vpc", Source: source, Version: version, FileName: "/checkout/stacks/third/main.tf", DefLine: 3, + }) + require.ErrorContains(t, err, "not found in manifest") +} + +func TestPrefetchedResolverLeavesLegacyCanonicalSourceUnset(t *testing.T) { + resolver := NewPrefetchedResolver(&Manifest{ + Modules: map[string]ManifestEntry{ + "terraform-aws-modules/vpc/aws@5.0.0": { + LocalPath: t.TempDir(), + Version: "5.0.0", + }, + }, + }) + + resolution, err := resolver.Resolve(t.Context(), &tfmodules.ParsedModule{ + Source: "terraform-aws-modules/vpc/aws", Version: "5.0.0", + }) + + require.NoError(t, err) + require.Empty(t, resolution.CanonicalSource) +} + +func TestLoadManifestAcceptsSymlinkedRoot(t *testing.T) { + realRoot := t.TempDir() + linkRoot := filepath.Join(t.TempDir(), "modules") + require.NoError(t, os.Symlink(realRoot, linkRoot)) + moduleDir := filepath.Join(linkRoot, "vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + manifestPath := filepath.Join(realRoot, "modules.json") + data, err := json.Marshal(Manifest{ + Dir: linkRoot, + Modules: map[string]ManifestEntry{ + "module": {LocalPath: moduleDir}, + }, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + _, err = LoadManifest(manifestPath) + require.NoError(t, err) +} diff --git a/pkg/parser/terraform/modules/resolver/resolver.go b/pkg/parser/terraform/modules/resolver/resolver.go index 383b62656..f2d266bc2 100644 --- a/pkg/parser/terraform/modules/resolver/resolver.go +++ b/pkg/parser/terraform/modules/resolver/resolver.go @@ -15,8 +15,14 @@ import ( // Resolution holds a resolved module directory and optional cleanup. type Resolution struct { - LocalPath string - Cleanup func() // optional post-scan cleanup + LocalPath string + RequestedVersion string + ResolvedVersion string + CanonicalSource string + ContentDigest string + Provenance string + Outcome string + Cleanup func() // optional post-scan cleanup } // Resolver maps one module call to disk; errors should wrap *tfmodules.UnresolvedError when appropriate. diff --git a/pkg/scan/remote_modules.go b/pkg/scan/remote_modules.go index 3eea00577..f611f7edd 100644 --- a/pkg/scan/remote_modules.go +++ b/pkg/scan/remote_modules.go @@ -49,13 +49,20 @@ func (c *Client) resolveTerraformModulesForScan( contextLogger.Debug().Msg("Resolving Terraform modules from local, manifest, or .terraform/modules sources only") } + hostedMode := c.ScanParams.RemoteModulesManifestPath != "" result := modulegraph.Resolve(ctx, &modulegraph.Request{ - RootPaths: extractedPaths.Path, - DiscoveryPaths: moduleDiscoveryPaths, - Resolver: chain, - MaxDepth: c.ScanParams.ModuleMaxDepth, - FS: c.fsys, + RootPaths: extractedPaths.Path, + DiscoveryPaths: moduleDiscoveryPaths, + Resolver: chain, + MaxDepth: c.ScanParams.ModuleMaxDepth, + FS: c.fsys, + CallScopedResolution: hostedMode, + FailOnUnresolved: hostedMode, }) + if result.Error != nil { + result.Cleanup() + return nil, nil, nil, fmt.Errorf("resolving modules from manifest: %w", result.Error) + } if len(result.ScanPaths) > 0 { contextLogger.Info().Msgf("Adding %d remote module file(s) to scan", len(result.ScanPaths)) } @@ -66,16 +73,26 @@ func (c *Client) resolveTerraformModulesForScan( } } remoteSourceDirs = make(map[string]string, len(result.Modules)*3) - for _, module := range result.Modules { + for i := range result.Modules { + module := &result.Modules[i] + requestedVersion := module.RequestedVersion + if requestedVersion == "" { + requestedVersion = module.Version + } remoteSourceDirs[engine.RemoteModuleKey( - module.CallerRoot, module.Source, module.Version, + module.CallerRoot, module.Source, requestedVersion, )] = module.LocalPath remoteSourceDirs[engine.RemoteModuleCallKey( - module.CallerRoot, module.Source, module.Version, module.Name, + module.CallerRoot, module.Source, requestedVersion, module.Name, )] = module.LocalPath remoteSourceDirs[engine.RemoteModuleCallKey( module.CallerRoot, module.Source, "", module.Name, )] = module.LocalPath + if module.ResolvedVersion != "" && module.ResolvedVersion != requestedVersion { + remoteSourceDirs[engine.RemoteModuleKey( + module.CallerRoot, module.Source, module.ResolvedVersion, + )] = module.LocalPath + } } return result.Cleanup, result.ScanPaths, remoteSourceDirs, nil } @@ -111,13 +128,14 @@ func (c *Client) buildModuleResolverChain( return nil, fmt.Errorf("loading modules manifest %q: %w", c.ScanParams.RemoteModulesManifestPath, err) } resolvers = append(resolvers, tfresolver.NewPrefetchedResolver(manifest)) + } else { + resolvers = append(resolvers, &tfresolver.DotTerraformResolver{ + RootDirs: dotTerraformRootDirs(moduleDiscoveryPaths), + }) } - resolvers = append(resolvers, &tfresolver.DotTerraformResolver{ - RootDirs: dotTerraformRootDirs(moduleDiscoveryPaths), - }) - - if c.ScanParams.EnableRemoteModules { + hostedMode := c.ScanParams.RemoteModulesManifestPath != "" + if c.ScanParams.EnableRemoteModules && !hostedMode { resolvers = append(resolvers, tfresolver.NewLocalGitRefResolver(dotTerraformRootDirs(moduleDiscoveryPaths), ""), tfresolver.NewBareGitResolver("", c.ScanParams.RemoteModulesHostAllowlist...), @@ -125,7 +143,7 @@ func (c *Client) buildModuleResolverChain( } ggCfg := tfresolver.NewGoGetterConfig() - ggCfg.Disabled = !c.ScanParams.EnableRemoteModules + ggCfg.Disabled = !c.ScanParams.EnableRemoteModules || hostedMode if t := c.ScanParams.ModuleFetchTimeout; t > 0 { ggCfg.FetchTimeout = t ggCfg.RegistryCache = tfresolver.NewRegistryCache(ggCfg.FetchTimeout) @@ -142,7 +160,9 @@ func (c *Client) buildModuleResolverChain( } } - resolvers = append(resolvers, tfresolver.NewGoGetterResolver(ggCfg)) + if !hostedMode { + resolvers = append(resolvers, tfresolver.NewGoGetterResolver(ggCfg)) + } return tfresolver.NewChainResolver(resolvers...), nil } diff --git a/pkg/scan/remote_modules_test.go b/pkg/scan/remote_modules_test.go index e9a0328ca..db379a9af 100644 --- a/pkg/scan/remote_modules_test.go +++ b/pkg/scan/remote_modules_test.go @@ -167,6 +167,59 @@ func TestBuildModuleResolverChainPrefersExplicitManifestOverTerraformCache(t *te require.Equal(t, preferredDir, got.LocalPath) } +func TestBuildModuleResolverChainDoesNotFallBackToTerraformCache(t *testing.T) { + root := t.TempDir() + staleDir := filepath.Join(root, ".terraform", "modules", "stale") + require.NoError(t, os.MkdirAll(staleDir, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(root, ".terraform", "modules", "modules.json"), + []byte(`{"Modules":[{"Key":"m","Source":"terraform-aws-modules/vpc/aws","Version":"1.0.0","Dir":"stale"}]}`), + 0o644, + )) + manifestPath := filepath.Join(root, "hosted-modules.json") + data, err := json.Marshal(resolver.Manifest{Modules: map[string]resolver.ManifestEntry{}}) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + client := &Client{ScanParams: &Parameters{RemoteModulesManifestPath: manifestPath}} + chain, err := client.buildModuleResolverChain(t.Context(), []string{root}) + require.NoError(t, err) + + _, err = chain.Resolve(t.Context(), &tfmodules.ParsedModule{ + Name: "m", Source: "terraform-aws-modules/vpc/aws", Version: "1.0.0", + FileName: filepath.Join(root, "main.tf"), + }) + require.ErrorContains(t, err, "not found in manifest") +} + +func TestBuildModuleResolverChainIsNetworklessWithManifest(t *testing.T) { + root := t.TempDir() + manifestPath := filepath.Join(root, "modules.json") + data, err := json.Marshal(resolver.Manifest{ + SchemaVersion: 2, + Modules: map[string]resolver.ManifestEntry{}, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + client := &Client{ScanParams: &Parameters{ + EnableRemoteModules: true, + RemoteModulesManifestPath: manifestPath, + }} + chain, err := client.buildModuleResolverChain(context.Background(), []string{root}) + require.NoError(t, err) + + _, err = chain.Resolve(context.Background(), &tfmodules.ParsedModule{ + Source: "git::https://127.0.0.1:1/acme/network.git?ref=v1", + Version: "1.0.0", + FileName: filepath.Join(root, "main.tf"), + }) + + require.ErrorContains(t, err, "not found in manifest") + require.NotContains(t, err.Error(), "fetch failed") + require.NotContains(t, err.Error(), "BareGitResolver") +} + func writeRemoteModuleFixture(t *testing.T, root string) (string, string) { t.Helper() moduleDir := filepath.Join(filepath.Dir(root), "downloaded-vpc") From c080eaf20f5b08fa6ba06f1ffdb9bd0bd29c9cf8 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 15 Jul 2026 00:59:11 +0200 Subject: [PATCH 4/6] Keep hosted Terraform scans offline and resilient to unresolved Git modules. --- .../modules/internal/modulesource/source.go | 33 ++++++++ pkg/parser/terraform/modules/modules.go | 4 +- pkg/parser/terraform/modules/modules_test.go | 8 +- .../terraform/modules/resolver/baregit.go | 66 ++------------- pkg/scan/remote_modules.go | 1 - pkg/scan/remote_modules_test.go | 83 +++++++++++++++++++ 6 files changed, 130 insertions(+), 65 deletions(-) create mode 100644 pkg/parser/terraform/modules/internal/modulesource/source.go diff --git a/pkg/parser/terraform/modules/internal/modulesource/source.go b/pkg/parser/terraform/modules/internal/modulesource/source.go new file mode 100644 index 000000000..ed53d01b4 --- /dev/null +++ b/pkg/parser/terraform/modules/internal/modulesource/source.go @@ -0,0 +1,33 @@ +package modulesource + +import "strings" + +// NormalizeGit normalizes unambiguous Git source forms. +func NormalizeGit(source string) (string, bool) { + source = strings.TrimSpace(source) + if source == "" { + return "", false + } + if strings.HasPrefix(source, "git::") { + return source, true + } + if strings.HasPrefix(source, "ssh://") { + return "git::" + source, true + } + if !strings.Contains(source, "::") { + if at := strings.IndexByte(source, '@'); at > 0 { + rest := source[at+1:] + slash := strings.IndexByte(rest, '/') + if colon := strings.IndexByte(rest, ':'); colon > 0 && + (slash < 0 || colon < slash) && + colon < len(rest)-1 { + return "git::ssh://" + source[:at] + "@" + rest[:colon] + "/" + rest[colon+1:], true + } + } + if strings.HasPrefix(source, "github.com/") && + (strings.Contains(source, "//") || strings.Contains(source, "ref=")) { + return "git::ssh://git@" + source, true + } + } + return source, false +} diff --git a/pkg/parser/terraform/modules/modules.go b/pkg/parser/terraform/modules/modules.go index a2786f30a..6de56ad87 100644 --- a/pkg/parser/terraform/modules/modules.go +++ b/pkg/parser/terraform/modules/modules.go @@ -16,6 +16,7 @@ import ( "github.com/DataDog/datadog-iac-scanner/pkg/hclexpr" "github.com/DataDog/datadog-iac-scanner/pkg/logger" "github.com/DataDog/datadog-iac-scanner/pkg/model" + "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules/internal/modulesource" "github.com/DataDog/datadog-iac-scanner/pkg/utils" "github.com/DataDog/datadog-iac-scanner/pkg/vfs" "github.com/cespare/xxhash/v2" @@ -606,8 +607,7 @@ func DetectModuleSourceType(source string) (sourceType, registryScope string) { return "data_ref", "" } - // Recognize git-based sources - if strings.HasPrefix(source, "git::") { + if _, ok := modulesource.NormalizeGit(source); ok { return "git", "" } diff --git a/pkg/parser/terraform/modules/modules_test.go b/pkg/parser/terraform/modules/modules_test.go index 2d83d5a44..6f50a4989 100644 --- a/pkg/parser/terraform/modules/modules_test.go +++ b/pkg/parser/terraform/modules/modules_test.go @@ -425,13 +425,19 @@ func TestDetectModuleSourceTypeWithScope(t *testing.T) { wantScope string }{ {"./module", "local", ""}, - {"git::./mod", "git", ""}, + {"../modules/network", "local", ""}, + {"git@github.com:DataDog/repo//module?ref=v1", "git", ""}, + {"ssh://git@github.com/DataDog/repo//module?ref=v1", "git", ""}, + {"git::https://github.com/DataDog/repo.git//module?ref=v1", "git", ""}, + {"git::ssh://git@github.com/DataDog/repo//module?ref=v1", "git", ""}, + {"github.com/DataDog/repo//module?ref=v1", "git", ""}, {"registry.terraform.io/org/vpc/aws", "registry", "public"}, {"terraform-aws-modules/vpc/aws", "registry", "public"}, {"terraform-aws-modules/eks/aws//modules/karpenter", "registry", "public"}, {"company.internal.io/infra/mod/aws", "registry", "private"}, {"company.internal.io/infra/mod/aws//child", "registry", "private"}, {"https://github.com/org/repo", "unknown", ""}, + {"https://example.com/modules/network.tar.gz", "unknown", ""}, {"data_ref:aws_s3.bucket.id", "data_ref", ""}, {"", "unknown", ""}, } diff --git a/pkg/parser/terraform/modules/resolver/baregit.go b/pkg/parser/terraform/modules/resolver/baregit.go index 599582030..cf169cc06 100644 --- a/pkg/parser/terraform/modules/resolver/baregit.go +++ b/pkg/parser/terraform/modules/resolver/baregit.go @@ -24,6 +24,7 @@ import ( "github.com/DataDog/datadog-iac-scanner/pkg/logger" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules/internal/modulesource" ) // dirPerm is the permission mode used for all directories created by resolvers. @@ -483,58 +484,6 @@ func (r *BareGitResolver) Resolve(ctx context.Context, mod *tfmodules.ParsedModu return Resolution{LocalPath: dir}, nil } -// normalizeSCPGitSource converts SCP-form git sources to git::ssh://git@... so -// BareGitResolver can clone them using the same SSH credentials that would be -// used by a native git client. HTTPS is intentionally avoided here because SCP -// sources typically point at private repositories that require SSH auth. -// -// "git@github.com:org/repo//subdir?ref=tag" -// → "git::ssh://git@github.com/org/repo//subdir?ref=tag", true -// -// Returns ("", false) if source is not SCP form. -func normalizeSCPGitSource(source string) (string, bool) { - // Must not already carry a getter scheme prefix. - if strings.Contains(source, "::") { - return "", false - } - // SCP form: [user@]host:path — @ precedes a colon that comes before any slash. - atIdx := strings.IndexByte(source, '@') - if atIdx < 0 { - return "", false - } - user := source[:atIdx] // e.g. "git" - rest := source[atIdx+1:] // host:org/repo//subdir?ref=tag - colonIdx := strings.IndexByte(rest, ':') - if colonIdx < 0 { - return "", false - } - // Ensure the colon is a host separator, not a path colon (no slash before it). - if slashIdx := strings.IndexByte(rest, '/'); slashIdx >= 0 && slashIdx < colonIdx { - return "", false - } - host := rest[:colonIdx] - path := rest[colonIdx+1:] // org/repo//subdir?ref=tag - return "git::ssh://" + user + "@" + host + "/" + path, true -} - -// normalizeImplicitGitHubSource converts Terraform's implicit GitHub module paths to git::ssh://. -// -// "github.com/org/repo//subdir?ref=tag" -// → "git::ssh://git@github.com/org/repo//subdir?ref=tag", true -func normalizeImplicitGitHubSource(source string) (string, bool) { - if strings.Contains(source, "::") { - return "", false - } - if !strings.HasPrefix(source, "github.com/") { - return "", false - } - // Require a subdir marker or ref= so registry short forms are not matched. - if !strings.Contains(source, "//") && !strings.Contains(source, "ref=") { - return "", false - } - return "git::ssh://git@" + source, true -} - // normalizeHTTPGitToSSH rewrites git::http(s)://host/... to git::ssh://git@host/... // so git clones use SSH credentials instead of unauthenticated HTTPS. func normalizeHTTPGitToSSH(source string) (string, bool) { @@ -557,16 +506,11 @@ func normalizeHTTPGitToSSH(source string) (string, bool) { // normalizeGitModuleSourceForGetter applies SCP and implicit GitHub normalization // without rewriting git::https sources to SSH (go-getter should keep HTTPS). func normalizeGitModuleSourceForGetter(source string) (string, bool) { - if source == "" { - return "", false + normalized, ok := modulesource.NormalizeGit(source) + if !ok || normalized == strings.TrimSpace(source) { + return source, false } - original := source - if normalized, ok := normalizeSCPGitSource(source); ok { - source = normalized - } else if normalized, ok := normalizeImplicitGitHubSource(source); ok { - source = normalized - } - return source, source != original + return normalized, true } // normalizeGitModuleSource applies all git source normalizations used by resolvers. diff --git a/pkg/scan/remote_modules.go b/pkg/scan/remote_modules.go index f611f7edd..7ca58bd5a 100644 --- a/pkg/scan/remote_modules.go +++ b/pkg/scan/remote_modules.go @@ -57,7 +57,6 @@ func (c *Client) resolveTerraformModulesForScan( MaxDepth: c.ScanParams.ModuleMaxDepth, FS: c.fsys, CallScopedResolution: hostedMode, - FailOnUnresolved: hostedMode, }) if result.Error != nil { result.Cleanup() diff --git a/pkg/scan/remote_modules_test.go b/pkg/scan/remote_modules_test.go index db379a9af..70d3fc86f 100644 --- a/pkg/scan/remote_modules_test.go +++ b/pkg/scan/remote_modules_test.go @@ -3,8 +3,11 @@ package scan import ( "context" "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" + "sync/atomic" "testing" engineprovider "github.com/DataDog/datadog-iac-scanner/pkg/engine/provider" @@ -41,6 +44,86 @@ func TestRemoteModulesManifestEnablesOfflineModuleInstantiation(t *testing.T) { require.Equal(t, filepath.ToSlash(filepath.Join(moduleDir, "main.tf")), filepath.ToSlash(results.Results[0].FileName)) } +func TestRemoteModulesManifestSkipsMissingEntryWithoutNetworkFallback(t *testing.T) { + root := t.TempDir() + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + requests.Add(1) + })) + t.Cleanup(server.Close) + + rootFile := filepath.Join(root, "main.tf") + require.NoError(t, os.WriteFile(rootFile, []byte(` +resource "aws_vpc" "repository" { + cidr_block = "10.0.0.0/16" +} +module "missing" { + source = "`+server.URL+`/module.zip" +} +`), 0o644)) + manifestPath := filepath.Join(root, "modules.json") + data, err := json.Marshal(resolver.Manifest{Modules: map[string]resolver.ManifestEntry{}}) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + params := remoteModuleScanParams(root) + params.EnableRemoteModules = true + params.RemoteModulesManifestPath = manifestPath + results := executeRemoteModuleScan(t, params) + + require.Len(t, results.Results, 1) + require.Equal(t, filepath.ToSlash(rootFile), filepath.ToSlash(results.Results[0].FileName)) + require.Zero(t, requests.Load()) +} + +func TestRemoteModulesManifestSkipsUnresolvedEntryAndIncludesResolvedSibling(t *testing.T) { + base := t.TempDir() + root := filepath.Join(base, "repository") + require.NoError(t, os.MkdirAll(root, 0o755)) + moduleDir := filepath.Join(base, "resolved-vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, []byte(` +resource "aws_vpc" "resolved" { + cidr_block = "10.0.0.0/16" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, "main.tf"), []byte(` +module "resolved" { + source = "terraform-aws-modules/vpc/aws" + version = "1.0.0" +} +module "unresolved" { + source = "terraform-aws-modules/subnets/aws" + version = "1.0.0" +} +`), 0o644)) + + manifestPath := filepath.Join(base, "modules.json") + data, err := json.Marshal(resolver.Manifest{ + Dir: base, + Modules: map[string]resolver.ManifestEntry{ + "terraform-aws-modules/vpc/aws@1.0.0": { + LocalPath: moduleDir, + Version: "1.0.0", + Outcome: "resolved", + }, + "terraform-aws-modules/subnets/aws@1.0.0": { + Outcome: "unresolved", + }, + }, + }) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + params := remoteModuleScanParams(root) + params.RemoteModulesManifestPath = manifestPath + results := executeRemoteModuleScan(t, params) + + require.Len(t, results.Results, 1) + require.Equal(t, filepath.ToSlash(moduleFile), filepath.ToSlash(results.Results[0].FileName)) +} + func TestRemoteModuleMaxDepthZeroDisablesTraversal(t *testing.T) { root := t.TempDir() _, manifestPath := writeRemoteModuleFixture(t, root) From 45b18a230371b3ee272a1b68c719547661e6be38 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 15 Jul 2026 01:30:11 +0200 Subject: [PATCH 5/6] Update Git source normalization tests to use the shared helper. --- pkg/parser/terraform/modules/resolver/baregit_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/parser/terraform/modules/resolver/baregit_test.go b/pkg/parser/terraform/modules/resolver/baregit_test.go index 3692e4645..da3eb5eed 100644 --- a/pkg/parser/terraform/modules/resolver/baregit_test.go +++ b/pkg/parser/terraform/modules/resolver/baregit_test.go @@ -11,6 +11,7 @@ import ( "testing" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" + "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules/internal/modulesource" ) func TestBareGitResolverRejectsDisallowedHost(t *testing.T) { @@ -78,7 +79,7 @@ func TestParseGitGetterSourceHTTPSGitHubToSSH(t *testing.T) { func TestNormalizeSCPGitSource(t *testing.T) { in := "git@github.com:DataDog/vault-platform//terraform/aws/external-iam?ref=v1.8.2-17" - got, ok := normalizeSCPGitSource(in) + got, ok := modulesource.NormalizeGit(in) if !ok { t.Fatal("expected ok") } @@ -129,7 +130,7 @@ func TestNormalizeHTTPGitToSSH(t *testing.T) { func TestNormalizeImplicitGitHubSource(t *testing.T) { in := "github.com/oracle-quickstart/terraform-oci-cis-landing-zone-iam//identity-domains?ref=release-0.3.0" - got, ok := normalizeImplicitGitHubSource(in) + got, ok := modulesource.NormalizeGit(in) if !ok { t.Fatal("expected ok") } @@ -137,7 +138,7 @@ func TestNormalizeImplicitGitHubSource(t *testing.T) { if got != want { t.Errorf("got %q, want %q", got, want) } - _, ok = normalizeImplicitGitHubSource("aws-ia/eks-blueprints-addon/aws") + _, ok = modulesource.NormalizeGit("aws-ia/eks-blueprints-addon/aws") if ok { t.Error("registry short form should not match") } From 907f3c1c0cca1295cca4ee6ad967e3161ca601f8 Mon Sep 17 00:00:00 2001 From: Chakib Hamie Date: Wed, 15 Jul 2026 16:13:51 +0200 Subject: [PATCH 6/6] Reuse complete hosted module discovery manifests during Terraform scans. --- cmd/scanner/list_modules.go | 121 ++++++++- cmd/scanner/list_modules_test.go | 124 ++++++++- pkg/parser/terraform/modules/list_modules.go | 42 ++-- .../terraform/modules/list_modules_test.go | 2 + .../modules/modulegraph/modulegraph.go | 124 +++++++++ .../modules/modulegraph/modulegraph_test.go | 83 ++++++ .../terraform/modules/resolver/prefetched.go | 238 +++++++++++++++++- .../modules/resolver/prefetched_test.go | 133 ++++++++++ pkg/scan/remote_modules.go | 99 ++++++-- pkg/scan/remote_modules_test.go | 117 +++++++++ 10 files changed, 1032 insertions(+), 51 deletions(-) diff --git a/cmd/scanner/list_modules.go b/cmd/scanner/list_modules.go index 13df013a7..ce7f0866a 100644 --- a/cmd/scanner/list_modules.go +++ b/cmd/scanner/list_modules.go @@ -8,6 +8,7 @@ import ( "os" "path" "path/filepath" + "sort" "strings" "github.com/DataDog/datadog-iac-scanner/pkg/model" @@ -30,6 +31,11 @@ var listModulesAction = &cli.Command{ Usage: "include local module calls", Value: false, }, + &cli.BoolFlag{ + Name: "direct", + Usage: "inspect top-level Terraform files and reachable local modules only", + Value: false, + }, }, Action: listModules, } @@ -39,11 +45,11 @@ func listModules(ctx context.Context, c *cli.Command) error { if err != nil { return err } - files, err := collectTerraformFiles(paths) + files, allowedFiles, err := collectModuleDiscoveryFiles(ctx, paths, c.Bool("direct")) if err != nil { return fmt.Errorf("collecting Terraform files: %w", err) } - parsed, err := tfmodules.ParseTerraformModulesFromFiles(ctx, nil, files, allowedModuleFiles(paths, files)) + parsed, err := tfmodules.ParseTerraformModulesFromFiles(ctx, nil, files, allowedFiles) if err != nil { return fmt.Errorf("parsing Terraform modules: %w", err) } @@ -54,6 +60,108 @@ func listModules(ctx context.Context, c *cli.Command) error { )) } +func collectModuleDiscoveryFiles( + ctx context.Context, paths []string, direct bool, +) (model.FileMetadatas, map[string]bool, error) { + files, err := collectTerraformFiles(paths, direct) + if err != nil { + return nil, nil, err + } + allowed := allowedModuleFiles(paths, files) + if !direct { + return files, allowed, nil + } + return collectReachableModuleDiscoveryFiles(ctx, paths, files, allowed) +} + +func collectReachableModuleDiscoveryFiles( + ctx context.Context, + paths []string, + files model.FileMetadatas, + allowed map[string]bool, +) (model.FileMetadatas, map[string]bool, error) { + return collectReachableModuleDiscoveryFilesWithParser( + paths, + files, + allowed, + func(frontier model.FileMetadatas, frontierAllowed map[string]bool) (map[string]tfmodules.ParsedModule, error) { + return tfmodules.ParseTerraformModulesFromFiles(ctx, nil, frontier, frontierAllowed) + }, + ) +} + +func collectReachableModuleDiscoveryFilesWithParser( + paths []string, + files model.FileMetadatas, + allowed map[string]bool, + parse func(model.FileMetadatas, map[string]bool) (map[string]tfmodules.ParsedModule, error), +) (model.FileMetadatas, map[string]bool, error) { + repositoryRoot := moduleRepositoryRoot(paths) + resolvedRoot, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return nil, nil, fmt.Errorf("resolving repository root: %w", err) + } + seenDirs := make(map[string]bool) + for _, file := range files { + dir, resolveErr := filepath.EvalSymlinks(filepath.Dir(file.FilePath)) + if resolveErr != nil { + return nil, nil, resolveErr + } + seenDirs[dir] = true + } + frontierFiles := make(model.FileMetadatas, 0, len(files)) + for _, file := range files { + if allowed == nil || allowed[file.FilePath] { + frontierFiles = append(frontierFiles, file) + } + } + for len(frontierFiles) > 0 { + frontierAllowed := make(map[string]bool, len(frontierFiles)) + for _, file := range frontierFiles { + frontierAllowed[file.FilePath] = true + } + modules, parseErr := parse(frontierFiles, frontierAllowed) + if parseErr != nil { + return nil, nil, parseErr + } + var childDirs []string + for key := range modules { + module := modules[key] + childDir := filepath.Clean(module.AbsSource) + if !module.IsLocal || childDir == "." { + continue + } + resolvedChild, resolveErr := filepath.EvalSymlinks(childDir) + if resolveErr != nil || seenDirs[resolvedChild] || + !pathContainsFile(resolvedRoot, resolvedChild) { + continue + } + seenDirs[resolvedChild] = true + childDirs = append(childDirs, resolvedChild) + } + if len(childDirs) == 0 { + return files, allowed, nil + } + sort.Strings(childDirs) + frontierFiles = nil + for _, childDir := range childDirs { + childFiles, collectErr := collectTopLevelTerraformFiles(childDir) + if collectErr != nil { + return nil, nil, collectErr + } + files = append(files, childFiles...) + frontierFiles = append(frontierFiles, childFiles...) + if allowed == nil { + allowed = make(map[string]bool) + } + for _, file := range childFiles { + allowed[file.FilePath] = true + } + } + } + return files, allowed, nil +} + func moduleRepositoryRoot(paths []string) string { if len(paths) == 0 { return "" @@ -133,7 +241,7 @@ func pathContainsFile(root, filePath string) bool { return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) } -func collectTerraformFiles(paths []string) (model.FileMetadatas, error) { +func collectTerraformFiles(paths []string, direct bool) (model.FileMetadatas, error) { var files model.FileMetadatas for _, rootPath := range paths { info, err := os.Stat(rootPath) @@ -151,7 +259,12 @@ func collectTerraformFiles(paths []string) (model.FileMetadatas, error) { files = append(files, dirFiles...) continue } - dirFiles, err := walkTerraformDir(rootPath) + var dirFiles model.FileMetadatas + if direct { + dirFiles, err = collectTopLevelTerraformFiles(rootPath) + } else { + dirFiles, err = walkTerraformDir(rootPath) + } if err != nil { return nil, err } diff --git a/cmd/scanner/list_modules_test.go b/cmd/scanner/list_modules_test.go index 437fb6652..1a5853b3d 100644 --- a/cmd/scanner/list_modules_test.go +++ b/cmd/scanner/list_modules_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/DataDog/datadog-iac-scanner/pkg/model" tfmodules "github.com/DataDog/datadog-iac-scanner/pkg/parser/terraform/modules" "github.com/stretchr/testify/require" ) @@ -32,7 +33,7 @@ module "ignored" { } `), 0o644)) - files, err := collectTerraformFiles([]string{root}) + files, err := collectTerraformFiles([]string{root}, false) require.NoError(t, err) require.Len(t, files, 2) @@ -48,3 +49,124 @@ module "ignored" { require.Equal(t, "local", entries[0].Name) require.Equal(t, "remote", entries[1].Name) } + +func TestDirectModuleDiscoveryFollowsOnlyReachableLocalModules(t *testing.T) { + root := t.TempDir() + reachable := filepath.Join(root, "modules", "reachable") + unreachable := filepath.Join(root, "examples", "unreachable") + testModule := filepath.Join(root, "test", "fixture") + for _, dir := range []string{reachable, unreachable, testModule} { + require.NoError(t, os.MkdirAll(dir, 0o755)) + } + require.NoError(t, os.WriteFile(filepath.Join(root, "main.tf"), []byte(` +module "reachable" { + source = "./modules/reachable" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(reachable, "main.tf"), []byte(` +module "remote_child" { + source = "registry.example.com/acme/child/aws" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(unreachable, "main.tf"), []byte(` +module "example_remote" { + source = "registry.example.com/acme/example/aws" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(testModule, "main.tf"), []byte(` +module "test_remote" { + source = "registry.example.com/acme/test/aws" +} +`), 0o644)) + + files, allowed, err := collectModuleDiscoveryFiles(t.Context(), []string{root}, true) + require.NoError(t, err) + require.Len(t, files, 2) + + modules, err := tfmodules.ParseTerraformModulesFromFiles(t.Context(), nil, files, allowed) + require.NoError(t, err) + entries := tfmodules.ListModuleEntries(modules, true) + require.Equal(t, []string{"reachable", "remote_child"}, []string{entries[0].Name, entries[1].Name}) +} + +func TestDirectModuleDiscoveryHandlesLocalCycles(t *testing.T) { + root := t.TempDir() + child := filepath.Join(root, "child") + require.NoError(t, os.MkdirAll(child, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(root, "main.tf"), + []byte(`module "child" { source = "./child" }`), + 0o644, + )) + require.NoError(t, os.WriteFile( + filepath.Join(child, "main.tf"), + []byte(`module "root" { source = ".." }`), + 0o644, + )) + + files, _, err := collectModuleDiscoveryFiles(t.Context(), []string{root}, true) + require.NoError(t, err) + require.Len(t, files, 2) +} + +func TestDirectModuleDiscoveryParsesEachFrontierOnce(t *testing.T) { + root := t.TempDir() + child := filepath.Join(root, "child") + grandchild := filepath.Join(child, "grandchild") + require.NoError(t, os.MkdirAll(grandchild, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(root, "main.tf"), + []byte(`module "child" { source = "./child" }`), + 0o644, + )) + require.NoError(t, os.WriteFile( + filepath.Join(child, "main.tf"), + []byte(`module "grandchild" { source = "./grandchild" }`), + 0o644, + )) + require.NoError(t, os.WriteFile(filepath.Join(grandchild, "main.tf"), []byte("# leaf"), 0o644)) + files, err := collectTerraformFiles([]string{root}, true) + require.NoError(t, err) + allowed := allowedModuleFiles([]string{root}, files) + var frontierSizes []int + + discovered, _, err := collectReachableModuleDiscoveryFilesWithParser( + []string{root}, + files, + allowed, + func(frontier model.FileMetadatas, frontierAllowed map[string]bool) (map[string]tfmodules.ParsedModule, error) { + frontierSizes = append(frontierSizes, len(frontier)) + return tfmodules.ParseTerraformModulesFromFiles(t.Context(), nil, frontier, frontierAllowed) + }, + ) + + require.NoError(t, err) + require.Len(t, discovered, 3) + require.Equal(t, []int{1, 1, 1}, frontierSizes) +} + +func TestDirectModuleDiscoveryRejectsSymlinkedModulesOutsideRoot(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(outside, "main.tf"), + []byte(`module "escaped" { source = "registry.example.com/acme/escaped/aws" }`), + 0o644, + )) + require.NoError(t, os.Symlink(outside, filepath.Join(root, "escaped"))) + require.NoError(t, os.WriteFile( + filepath.Join(root, "main.tf"), + []byte(`module "outside" { source = "./escaped" }`), + 0o644, + )) + + files, allowed, err := collectModuleDiscoveryFiles(t.Context(), []string{root}, true) + + require.NoError(t, err) + require.Len(t, files, 1) + modules, err := tfmodules.ParseTerraformModulesFromFiles(t.Context(), nil, files, allowed) + require.NoError(t, err) + entries := tfmodules.ListModuleEntries(modules, true) + require.Len(t, entries, 1) + require.Equal(t, "outside", entries[0].Name) +} diff --git a/pkg/parser/terraform/modules/list_modules.go b/pkg/parser/terraform/modules/list_modules.go index f22a25e4a..f5e47ed7a 100644 --- a/pkg/parser/terraform/modules/list_modules.go +++ b/pkg/parser/terraform/modules/list_modules.go @@ -12,15 +12,16 @@ import ( // ListModuleEntry is the JSON shape emitted by `datadog-iac-scanner list-modules`. // Field names are stable integration contracts for hosted scan orchestration. type ListModuleEntry struct { - Name string `json:"name"` - Source string `json:"source"` - Version string `json:"version,omitempty"` - SourceType string `json:"source_type"` - RegistryScope string `json:"registry_scope,omitempty"` - FileName string `json:"file_name"` - DefLine int `json:"def_line"` - CallerPath string `json:"caller_path,omitempty"` - CallID string `json:"call_id,omitempty"` + Name string `json:"name"` + Source string `json:"source"` + Version string `json:"version,omitempty"` + SourceType string `json:"source_type"` + RegistryScope string `json:"registry_scope,omitempty"` + FileName string `json:"file_name"` + DefLine int `json:"def_line"` + CallerPath string `json:"caller_path,omitempty"` + CallerPathBase string `json:"caller_path_base,omitempty"` + CallID string `json:"call_id,omitempty"` } // ListModuleEntries converts parsed modules to list-modules JSON rows. @@ -42,16 +43,21 @@ func ListModuleEntriesRelativeTo( continue } callerPath := stableCallerPath(repositoryRoot, mod.FileName) + callerPathBase := "" + if repositoryRoot != "" { + callerPathBase = filepath.Clean(repositoryRoot) + } entries = append(entries, ListModuleEntry{ - Name: mod.Name, - Source: mod.Source, - Version: mod.Version, - SourceType: mod.SourceType, - RegistryScope: mod.RegistryScope, - FileName: mod.FileName, - DefLine: mod.DefLine, - CallerPath: callerPath, - CallID: ModuleCallID(callerPath, &mod), + Name: mod.Name, + Source: mod.Source, + Version: mod.Version, + SourceType: mod.SourceType, + RegistryScope: mod.RegistryScope, + FileName: mod.FileName, + DefLine: mod.DefLine, + CallerPath: callerPath, + CallerPathBase: callerPathBase, + CallID: ModuleCallID(callerPath, &mod), }) } sort.Slice(entries, func(i, j int) bool { diff --git a/pkg/parser/terraform/modules/list_modules_test.go b/pkg/parser/terraform/modules/list_modules_test.go index 90cb3e9ec..63a0d4477 100644 --- a/pkg/parser/terraform/modules/list_modules_test.go +++ b/pkg/parser/terraform/modules/list_modules_test.go @@ -71,6 +71,8 @@ func TestListModuleEntriesUsesStableRepositoryRelativeCallID(t *testing.T) { ) require.Equal(t, "infra/main.tf", first[0].CallerPath) + require.Equal(t, "/checkout-a", first[0].CallerPathBase) + require.Equal(t, "/checkout-b", second[0].CallerPathBase) require.Equal(t, first[0].CallerPath, second[0].CallerPath) require.Equal(t, first[0].CallID, second[0].CallID) } diff --git a/pkg/parser/terraform/modules/modulegraph/modulegraph.go b/pkg/parser/terraform/modules/modulegraph/modulegraph.go index a1df2c6d7..2f7ae15cb 100644 --- a/pkg/parser/terraform/modules/modulegraph/modulegraph.go +++ b/pkg/parser/terraform/modules/modulegraph/modulegraph.go @@ -182,6 +182,130 @@ func Resolve(ctx context.Context, request *Request) Result { return result } +func FromManifestDiscovery(discovery *resolver.ManifestDiscovery, repositoryRoot string, maxDepth int) Result { + result := Result{ + SourceMappings: make(map[string]string), + Cleanup: func() {}, + } + if discovery == nil || !discovery.Complete { + return result + } + calls := make(map[string]resolver.ManifestDiscoveryCall, len(discovery.Calls)) + for _, call := range discovery.Calls { + calls[call.CallID] = call + } + depths := manifestCallDepths(calls) + mappingDepths := make(map[string]int) + for i := range discovery.ModuleMappings { + mapping := &discovery.ModuleMappings[i] + if !manifestMappingResolved(mapping.Outcome) { + continue + } + if existing := mappingDepths[mapping.LocalPath]; existing == 0 || depths[mapping.CallID] < existing { + mappingDepths[mapping.LocalPath] = depths[mapping.CallID] + } + if depths[mapping.CallID] > maxDepth { + continue + } + call := calls[mapping.CallID] + resolvedVersion := mapping.ResolvedVersion + if resolvedVersion == "" { + resolvedVersion = mapping.Version + } + version := resolvedVersion + if version == "" { + version = call.RequestedVersion + } + canonicalSource := mapping.CanonicalSource + if canonicalSource == "" { + canonicalSource = discovery.SourceMappings[mapping.LocalPath] + } + result.Modules = append(result.Modules, ResolvedModule{ + CallerRoot: manifestCallerRoot(call.CallerPath, repositoryRoot), + Source: call.Source, + Version: version, + RequestedVersion: call.RequestedVersion, + ResolvedVersion: resolvedVersion, + Name: call.Name, + LocalPath: mapping.LocalPath, + CanonicalSource: canonicalSource, + ContentDigest: mapping.ContentDigest, + Provenance: mapping.Provenance, + Outcome: mapping.Outcome, + }) + } + for localPath, source := range discovery.SourceMappings { + if manifestPathWithinDepth(localPath, mappingDepths, maxDepth) { + result.SourceMappings[localPath] = source + } + } + for _, scanPath := range discovery.ScanPaths { + if manifestPathWithinDepth(scanPath, mappingDepths, maxDepth) { + result.ScanPaths = append(result.ScanPaths, scanPath) + } + } + sort.Strings(result.ScanPaths) + sort.Slice(result.Modules, func(i, j int) bool { + left, right := result.Modules[i], result.Modules[j] + return strings.Join([]string{left.CallerRoot, left.Source, left.Version, left.Name}, "\x00") < + strings.Join([]string{right.CallerRoot, right.Source, right.Version, right.Name}, "\x00") + }) + return result +} + +func manifestCallDepths(calls map[string]resolver.ManifestDiscoveryCall) map[string]int { + depths := make(map[string]int, len(calls)) + var depth func(string) int + depth = func(callID string) int { + if existing := depths[callID]; existing != 0 { + return existing + } + parent := calls[callID].ParentCallID + if parent == "" { + depths[callID] = 1 + } else { + depths[callID] = depth(parent) + 1 + } + return depths[callID] + } + for callID := range calls { + depth(callID) + } + return depths +} + +func manifestPathWithinDepth(candidate string, roots map[string]int, maxDepth int) bool { + bestLength := -1 + bestDepth := 0 + for root, depth := range roots { + relative, err := filepath.Rel(root, candidate) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + continue + } + if len(root) > bestLength { + bestLength = len(root) + bestDepth = depth + } + } + return bestLength >= 0 && bestDepth <= maxDepth +} + +func manifestCallerRoot(callerPath, repositoryRoot string) string { + if !filepath.IsAbs(callerPath) { + callerPath = filepath.Join(repositoryRoot, filepath.FromSlash(callerPath)) + } + return filepath.Clean(filepath.Dir(callerPath)) +} + +func manifestMappingResolved(outcome string) bool { + switch strings.ToLower(strings.TrimSpace(outcome)) { + case "", "resolved", "success": + return true + default: + return false + } +} + func (w *walker) parseModulesInDir( ctx context.Context, dir string, allowedFiles map[string]bool, ) map[string]tfmodules.ParsedModule { diff --git a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go index f1100a1bc..a58806abb 100644 --- a/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go +++ b/pkg/parser/terraform/modules/modulegraph/modulegraph_test.go @@ -297,6 +297,89 @@ func TestResolveCountsLocalHopsTowardDepth(t *testing.T) { require.Empty(t, result.ScanPaths) } +func TestFromManifestDiscoveryUsesExactSnapshotWithoutResolving(t *testing.T) { + repositoryRoot := t.TempDir() + moduleRoot := filepath.Join(t.TempDir(), "modules", "vpc") + moduleFile := filepath.Join(moduleRoot, "main.tf") + discovery := &resolver.ManifestDiscovery{ + Complete: true, + ScanPaths: []string{moduleFile}, + Calls: []resolver.ManifestDiscoveryCall{{ + CallID: "vpc-call", + CallerPath: "stacks/network/main.tf", + Name: "vpc", + Source: "terraform-aws-modules/vpc/aws", + RequestedVersion: "~> 5.0", + }}, + SourceMappings: map[string]string{ + moduleRoot: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + }, + ModuleMappings: []resolver.ManifestModuleMapping{{ + CallID: "vpc-call", + LocalPath: moduleRoot, + ResolvedVersion: "5.1.2", + CanonicalSource: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + ContentDigest: "sha256:abc", + Provenance: "prefetched", + Outcome: "resolved", + }}, + } + + result := FromManifestDiscovery(discovery, repositoryRoot, 1) + + require.NoError(t, result.Error) + require.Equal(t, []string{moduleFile}, result.ScanPaths) + require.Equal(t, discovery.SourceMappings, result.SourceMappings) + require.Equal(t, []ResolvedModule{{ + CallerRoot: filepath.Join(repositoryRoot, "stacks", "network"), + Source: "terraform-aws-modules/vpc/aws", + Version: "5.1.2", + RequestedVersion: "~> 5.0", + ResolvedVersion: "5.1.2", + Name: "vpc", + LocalPath: moduleRoot, + CanonicalSource: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + ContentDigest: "sha256:abc", + Provenance: "prefetched", + Outcome: "resolved", + }}, result.Modules) +} + +func TestFromManifestDiscoveryHonorsPositiveDepth(t *testing.T) { + root := t.TempDir() + parentRoot := filepath.Join(root, "parent") + childRoot := filepath.Join(root, "child") + require.NoError(t, os.MkdirAll(parentRoot, 0o755)) + require.NoError(t, os.MkdirAll(childRoot, 0o755)) + parentFile := filepath.Join(parentRoot, "main.tf") + childFile := filepath.Join(childRoot, "main.tf") + require.NoError(t, os.WriteFile(parentFile, nil, 0o644)) + require.NoError(t, os.WriteFile(childFile, nil, 0o644)) + discovery := &resolver.ManifestDiscovery{ + Complete: true, + ScanPaths: []string{childFile, parentFile}, + Calls: []resolver.ManifestDiscoveryCall{ + {CallID: "parent", CallerPath: "main.tf", Name: "parent", Source: "parent"}, + { + CallID: "child", ParentCallID: "parent", CallerPath: filepath.Join(parentRoot, "main.tf"), + Name: "child", Source: "child", + }, + }, + SourceMappings: map[string]string{parentRoot: "canonical-parent", childRoot: "canonical-child"}, + ModuleMappings: []resolver.ManifestModuleMapping{ + {CallID: "parent", LocalPath: parentRoot, Outcome: "resolved"}, + {CallID: "child", LocalPath: childRoot, Outcome: "resolved"}, + }, + } + + result := FromManifestDiscovery(discovery, root, 1) + + require.Equal(t, []string{parentFile}, result.ScanPaths) + require.Equal(t, map[string]string{parentRoot: "canonical-parent"}, result.SourceMappings) + require.Len(t, result.Modules, 1) + require.Equal(t, "parent", result.Modules[0].Name) +} + func TestCanonicalGitModuleSource(t *testing.T) { require.Equal( t, diff --git a/pkg/parser/terraform/modules/resolver/prefetched.go b/pkg/parser/terraform/modules/resolver/prefetched.go index 418989c3a..c7a3bb426 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched.go +++ b/pkg/parser/terraform/modules/resolver/prefetched.go @@ -35,12 +35,41 @@ type ManifestEntry struct { Outcome string `json:"outcome,omitempty"` } +type ManifestDiscoveryCall struct { + CallID string `json:"call_id"` + ParentCallID string `json:"parent_call_id,omitempty"` + CallerPath string `json:"caller_path"` + Name string `json:"name"` + Source string `json:"source"` + RequestedVersion string `json:"requested_version,omitempty"` +} + +type ManifestModuleMapping struct { + CallID string `json:"call_id"` + LocalPath string `json:"local_path,omitempty"` + Version string `json:"version,omitempty"` + ResolvedVersion string `json:"resolved_version,omitempty"` + CanonicalSource string `json:"canonical_source,omitempty"` + ContentDigest string `json:"content_digest,omitempty"` + Provenance string `json:"provenance,omitempty"` + Outcome string `json:"outcome,omitempty"` +} + +type ManifestDiscovery struct { + Complete bool `json:"complete,omitempty"` + Calls []ManifestDiscoveryCall `json:"calls,omitempty"` + ScanPaths []string `json:"scan_paths,omitempty"` + SourceMappings map[string]string `json:"source_mappings,omitempty"` + ModuleMappings []ManifestModuleMapping `json:"module_mappings,omitempty"` +} + // Manifest is validated JSON: optional root Dir plus source or source@version → ManifestEntry. type Manifest struct { Version int `json:"version,omitempty"` SchemaVersion int `json:"schema_version,omitempty"` Dir string `json:"dir"` Modules map[string]ManifestEntry `json:"modules"` + Discovery *ManifestDiscovery `json:"discovery,omitempty"` } // LoadManifest parses and validates manifest JSON from path. @@ -73,14 +102,17 @@ func (m *Manifest) validate() error { return err } } + if err := m.validateDiscovery(resolvedDir); err != nil { + return err + } return nil } func validateManifestVersions(version, schemaVersion int) error { - if version < 0 || version > 2 { + if version < 0 || version > 3 { return fmt.Errorf("unsupported manifest version %d", version) } - if schemaVersion < 0 || schemaVersion > 2 { + if schemaVersion < 0 || schemaVersion > 3 { return fmt.Errorf("unsupported schema_version %d", schemaVersion) } if version != 0 && schemaVersion != 0 && version != schemaVersion { @@ -89,6 +121,208 @@ func validateManifestVersions(version, schemaVersion int) error { return nil } +func (m *Manifest) HasCompleteDiscovery() bool { + return m != nil && m.Discovery != nil && m.Discovery.Complete +} + +func (m *Manifest) validateDiscovery(resolvedDir string) error { + if m.Discovery == nil || !m.Discovery.Complete { + return nil + } + if m.SchemaVersion != 3 { + return fmt.Errorf("complete discovery requires schema_version 3") + } + if m.Dir == "" { + return fmt.Errorf("complete discovery requires dir") + } + + calls, err := validateDiscoveryCalls(m.Discovery.Calls) + if err != nil { + return err + } + if err := validateDiscoveryPaths(m.Discovery, m.Dir, resolvedDir); err != nil { + return err + } + if err := validateDiscoveryMappings(m.Discovery, calls, m.Dir, resolvedDir); err != nil { + return err + } + sort.Slice(m.Discovery.Calls, func(i, j int) bool { + return m.Discovery.Calls[i].CallID < m.Discovery.Calls[j].CallID + }) + sort.Strings(m.Discovery.ScanPaths) + sort.Slice(m.Discovery.ModuleMappings, func(i, j int) bool { + return m.Discovery.ModuleMappings[i].CallID < m.Discovery.ModuleMappings[j].CallID + }) + return nil +} + +func validateDiscoveryCalls(entries []ManifestDiscoveryCall) (map[string]ManifestDiscoveryCall, error) { + calls := make(map[string]ManifestDiscoveryCall, len(entries)) + for i := range entries { + call := entries[i] + if call.CallID == "" { + return nil, fmt.Errorf("discovery call %d: call_id is required", i) + } + if _, exists := calls[call.CallID]; exists { + return nil, fmt.Errorf("discovery call %q is duplicated", call.CallID) + } + if call.CallerPath == "" || call.Name == "" || call.Source == "" { + return nil, fmt.Errorf("discovery call %q requires caller_path, name, and source", call.CallID) + } + calls[call.CallID] = call + } + for _, call := range calls { + if call.ParentCallID != "" { + if _, exists := calls[call.ParentCallID]; !exists { + return nil, fmt.Errorf( + "discovery call %q references missing parent %q", + call.CallID, call.ParentCallID, + ) + } + } + } + if err := validateDiscoveryCycles(calls); err != nil { + return nil, err + } + return calls, nil +} + +func validateDiscoveryPaths(discovery *ManifestDiscovery, dir, resolvedDir string) error { + seenScanPaths := make(map[string]bool, len(discovery.ScanPaths)) + for _, scanPath := range discovery.ScanPaths { + if seenScanPaths[scanPath] { + return fmt.Errorf("discovery scan_path %q is duplicated", scanPath) + } + seenScanPaths[scanPath] = true + if err := validateDiscoveryFile(scanPath, dir, resolvedDir); err != nil { + return fmt.Errorf("discovery scan_path %q: %w", scanPath, err) + } + } + for localPath, source := range discovery.SourceMappings { + if source == "" { + return fmt.Errorf("discovery source_mapping %q has an empty source", localPath) + } + if err := validateDiscoveryPath(localPath, dir, resolvedDir); err != nil { + return fmt.Errorf("discovery source_mapping %q: %w", localPath, err) + } + } + for _, scanPath := range discovery.ScanPaths { + if !pathCoveredBySourceMapping(scanPath, discovery.SourceMappings) { + return fmt.Errorf("discovery scan_path %q has no source_mapping", scanPath) + } + } + return nil +} + +func validateDiscoveryMappings( + discovery *ManifestDiscovery, + calls map[string]ManifestDiscoveryCall, + dir, resolvedDir string, +) error { + mappedCalls := make(map[string]bool, len(discovery.ModuleMappings)) + for i := range discovery.ModuleMappings { + mapping := discovery.ModuleMappings[i] + call, exists := calls[mapping.CallID] + if !exists { + return fmt.Errorf("discovery module_mapping references missing call %q", mapping.CallID) + } + if mappedCalls[mapping.CallID] { + return fmt.Errorf("discovery module_mapping for call %q is duplicated", mapping.CallID) + } + mappedCalls[mapping.CallID] = true + entry := ManifestEntry{ + LocalPath: mapping.LocalPath, + Version: mapping.Version, + ResolvedVersion: mapping.ResolvedVersion, + RequestedVersion: call.RequestedVersion, + Outcome: mapping.Outcome, + } + if err := validateManifestEntry(mapping.CallID, &entry, dir, resolvedDir); err != nil { + return fmt.Errorf("discovery module_mapping: %w", err) + } + if manifestEntryResolved(mapping.Outcome) { + if _, exists := discovery.SourceMappings[mapping.LocalPath]; !exists { + return fmt.Errorf( + "discovery module_mapping for call %q has no source_mapping for %q", + mapping.CallID, mapping.LocalPath, + ) + } + } + } + return nil +} + +func validateDiscoveryCycles(calls map[string]ManifestDiscoveryCall) error { + const ( + visiting = 1 + visited = 2 + ) + state := make(map[string]int, len(calls)) + var visit func(string) error + visit = func(callID string) error { + switch state[callID] { + case visiting: + return fmt.Errorf("discovery calls contain a cycle at %q", callID) + case visited: + return nil + } + state[callID] = visiting + if parent := calls[callID].ParentCallID; parent != "" { + if err := visit(parent); err != nil { + return err + } + } + state[callID] = visited + return nil + } + for callID := range calls { + if err := visit(callID); err != nil { + return err + } + } + return nil +} + +func validateDiscoveryFile(filePath, dir, resolvedDir string) error { + if !strings.EqualFold(filepath.Ext(filePath), ".tf") { + return fmt.Errorf("path must identify a Terraform file") + } + info, err := os.Stat(filePath) + if err != nil { + return fmt.Errorf("cannot stat path: %w", err) + } + if info.IsDir() { + return fmt.Errorf("path identifies a directory") + } + return validateDiscoveryPath(filePath, dir, resolvedDir) +} + +func validateDiscoveryPath(candidate, dir, resolvedDir string) error { + if !filepath.IsAbs(candidate) { + return fmt.Errorf("path must be absolute") + } + if !pathConfinedTo(dir, candidate) { + return fmt.Errorf("path is not confined to dir %q", dir) + } + resolved, err := filepath.EvalSymlinks(candidate) + if err != nil { + return fmt.Errorf("cannot resolve symlinks: %w", err) + } + if !pathConfinedTo(resolvedDir, resolved) { + return fmt.Errorf("resolved path escapes dir %q", dir) + } + return nil +} + +func pathCoveredBySourceMapping(scanPath string, mappings map[string]string) bool { + for localPath := range mappings { + if pathConfinedTo(localPath, scanPath) { + return true + } + } + return false +} + func validateManifestDir(dir string) (string, error) { if dir == "" { return "", nil diff --git a/pkg/parser/terraform/modules/resolver/prefetched_test.go b/pkg/parser/terraform/modules/resolver/prefetched_test.go index 35bf1e3e7..cb626f817 100644 --- a/pkg/parser/terraform/modules/resolver/prefetched_test.go +++ b/pkg/parser/terraform/modules/resolver/prefetched_test.go @@ -263,3 +263,136 @@ func TestLoadManifestAcceptsSymlinkedRoot(t *testing.T) { _, err = LoadManifest(manifestPath) require.NoError(t, err) } + +func TestLoadManifestAcceptsCompleteDiscoverySnapshot(t *testing.T) { + root := t.TempDir() + moduleDir := filepath.Join(root, "vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, []byte(`resource "x" "y" {}`), 0o644)) + manifest := completeDiscoveryManifest(root, moduleDir, moduleFile) + manifest.Discovery.Calls = append(manifest.Discovery.Calls, ManifestDiscoveryCall{ + CallID: "a", CallerPath: "child/main.tf", Name: "child", Source: "./child", + }) + manifest.Discovery.ScanPaths = []string{moduleFile} + + data, err := json.Marshal(manifest) + require.NoError(t, err) + manifestPath := filepath.Join(root, "modules.json") + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + loaded, err := LoadManifest(manifestPath) + + require.NoError(t, err) + require.True(t, loaded.HasCompleteDiscovery()) + require.Equal(t, []string{"a", "root"}, []string{ + loaded.Discovery.Calls[0].CallID, + loaded.Discovery.Calls[1].CallID, + }) +} + +func TestCompleteDiscoveryValidationRejectsBrokenGraphs(t *testing.T) { + root := t.TempDir() + moduleDir := filepath.Join(root, "vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, nil, 0o644)) + + tests := []struct { + name string + mutate func(*Manifest) + match string + }{ + { + name: "missing parent", + mutate: func(manifest *Manifest) { + manifest.Discovery.Calls[0].ParentCallID = "missing" + }, + match: "references missing parent", + }, + { + name: "cycle", + mutate: func(manifest *Manifest) { + manifest.Discovery.Calls = append(manifest.Discovery.Calls, ManifestDiscoveryCall{ + CallID: "child", + ParentCallID: "root", + CallerPath: "vpc/main.tf", + Name: "child", + Source: "./child", + }) + manifest.Discovery.Calls[0].ParentCallID = "child" + }, + match: "contain a cycle", + }, + { + name: "missing mapping call", + mutate: func(manifest *Manifest) { + manifest.Discovery.ModuleMappings[0].CallID = "missing" + }, + match: "references missing call", + }, + { + name: "unmapped scan path", + mutate: func(manifest *Manifest) { + manifest.Discovery.SourceMappings = nil + }, + match: "has no source_mapping", + }, + { + name: "path escape", + mutate: func(manifest *Manifest) { + outside := filepath.Join(t.TempDir(), "outside.tf") + require.NoError(t, os.WriteFile(outside, nil, 0o644)) + manifest.Discovery.ScanPaths = []string{outside} + }, + match: "not confined", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manifest := completeDiscoveryManifest(root, moduleDir, moduleFile) + tt.mutate(manifest) + require.ErrorContains(t, manifest.validate(), tt.match) + }) + } +} + +func TestCompleteDiscoveryRequiresSchemaVersionThree(t *testing.T) { + root := t.TempDir() + moduleDir := filepath.Join(root, "vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, nil, 0o644)) + manifest := completeDiscoveryManifest(root, moduleDir, moduleFile) + manifest.SchemaVersion = 2 + + require.ErrorContains(t, manifest.validate(), "requires schema_version 3") +} + +func completeDiscoveryManifest(root, moduleDir, moduleFile string) *Manifest { + return &Manifest{ + SchemaVersion: 3, + Dir: root, + Modules: map[string]ManifestEntry{}, + Discovery: &ManifestDiscovery{ + Complete: true, + Calls: []ManifestDiscoveryCall{{ + CallID: "root", + CallerPath: "main.tf", + Name: "vpc", + Source: "terraform-aws-modules/vpc/aws", + RequestedVersion: "~> 5.0", + }}, + ScanPaths: []string{moduleFile}, + SourceMappings: map[string]string{ + moduleDir: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + }, + ModuleMappings: []ManifestModuleMapping{{ + CallID: "root", + LocalPath: moduleDir, + ResolvedVersion: "5.1.2", + CanonicalSource: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + Outcome: "resolved", + }}, + }, + } +} diff --git a/pkg/scan/remote_modules.go b/pkg/scan/remote_modules.go index 7ca58bd5a..c732a037e 100644 --- a/pkg/scan/remote_modules.go +++ b/pkg/scan/remote_modules.go @@ -30,34 +30,16 @@ func (c *Client) resolveTerraformModulesForScan( } contextLogger := logger.FromContext(ctx) - filteredFilesSource, err := c.getFileSystemSourceProvider(ctx, extractedPaths.Path) - if err != nil { - return nil, nil, nil, err - } - moduleDiscoveryPaths, err := filteredFilesSource.TerraformFiles(ctx) - if err != nil { - return nil, nil, nil, err - } - chain, err := c.buildModuleResolverChain(ctx, moduleDiscoveryPaths) - if err != nil { - return nil, nil, nil, err - } - if c.ScanParams.EnableRemoteModules { contextLogger.Info().Msg("Resolving remote Terraform modules...") } else { contextLogger.Debug().Msg("Resolving Terraform modules from local, manifest, or .terraform/modules sources only") } - hostedMode := c.ScanParams.RemoteModulesManifestPath != "" - result := modulegraph.Resolve(ctx, &modulegraph.Request{ - RootPaths: extractedPaths.Path, - DiscoveryPaths: moduleDiscoveryPaths, - Resolver: chain, - MaxDepth: c.ScanParams.ModuleMaxDepth, - FS: c.fsys, - CallScopedResolution: hostedMode, - }) + result, err := c.resolveTerraformModuleGraph(ctx, extractedPaths.Path) + if err != nil { + return nil, nil, nil, err + } if result.Error != nil { result.Cleanup() return nil, nil, nil, fmt.Errorf("resolving modules from manifest: %w", result.Error) @@ -96,6 +78,59 @@ func (c *Client) resolveTerraformModulesForScan( return result.Cleanup, result.ScanPaths, remoteSourceDirs, nil } +func (c *Client) resolveTerraformModuleGraph( + ctx context.Context, rootPaths []string, +) (modulegraph.Result, error) { + hostedMode := c.ScanParams.RemoteModulesManifestPath != "" + var manifest *tfresolver.Manifest + if hostedMode { + loaded, err := tfresolver.LoadManifest(c.ScanParams.RemoteModulesManifestPath) + if err != nil { + return modulegraph.Result{}, fmt.Errorf( + "loading modules manifest %q: %w", + c.ScanParams.RemoteModulesManifestPath, err, + ) + } + manifest = loaded + } + if manifest.HasCompleteDiscovery() { + if c.ScanParams.ModuleMaxDepth <= 0 { + return modulegraph.Result{SourceMappings: make(map[string]string), Cleanup: func() {}}, nil + } + return modulegraph.FromManifestDiscovery( + manifest.Discovery, + c.ScanParams.RepoPath, + c.ScanParams.ModuleMaxDepth, + ), nil + } + + filteredFilesSource, err := c.getFileSystemSourceProvider(ctx, rootPaths) + if err != nil { + return modulegraph.Result{}, err + } + moduleDiscoveryPaths, err := filteredFilesSource.TerraformFiles(ctx) + if err != nil { + return modulegraph.Result{}, err + } + var chain *tfresolver.ChainResolver + if hostedMode { + chain, err = c.buildModuleResolverChainWithManifest(ctx, moduleDiscoveryPaths, manifest) + } else { + chain, err = c.buildModuleResolverChain(ctx, moduleDiscoveryPaths) + } + if err != nil { + return modulegraph.Result{}, err + } + return modulegraph.Resolve(ctx, &modulegraph.Request{ + RootPaths: rootPaths, + DiscoveryPaths: moduleDiscoveryPaths, + Resolver: chain, + MaxDepth: c.ScanParams.ModuleMaxDepth, + FS: c.fsys, + CallScopedResolution: hostedMode, + }), nil +} + func (c *Client) shouldPreScanTerraformModules(scanPaths []string) bool { if c.ScanParams.EnableRemoteModules || c.ScanParams.RemoteModulesManifestPath != "" { return true @@ -114,6 +149,22 @@ func HasTerraformModuleCache(scanPaths []string) bool { func (c *Client) buildModuleResolverChain( ctx context.Context, moduleDiscoveryPaths []string, +) (*tfresolver.ChainResolver, error) { + var manifest *tfresolver.Manifest + if c.ScanParams.RemoteModulesManifestPath != "" { + loaded, err := tfresolver.LoadManifest(c.ScanParams.RemoteModulesManifestPath) + if err != nil { + return nil, fmt.Errorf("loading modules manifest %q: %w", c.ScanParams.RemoteModulesManifestPath, err) + } + manifest = loaded + } + return c.buildModuleResolverChainWithManifest(ctx, moduleDiscoveryPaths, manifest) +} + +func (c *Client) buildModuleResolverChainWithManifest( + ctx context.Context, + moduleDiscoveryPaths []string, + manifest *tfresolver.Manifest, ) (*tfresolver.ChainResolver, error) { contextLogger := logger.FromContext(ctx) @@ -122,10 +173,6 @@ func (c *Client) buildModuleResolverChain( } if c.ScanParams.RemoteModulesManifestPath != "" { - manifest, err := tfresolver.LoadManifest(c.ScanParams.RemoteModulesManifestPath) - if err != nil { - return nil, fmt.Errorf("loading modules manifest %q: %w", c.ScanParams.RemoteModulesManifestPath, err) - } resolvers = append(resolvers, tfresolver.NewPrefetchedResolver(manifest)) } else { resolvers = append(resolvers, &tfresolver.DotTerraformResolver{ diff --git a/pkg/scan/remote_modules_test.go b/pkg/scan/remote_modules_test.go index 70d3fc86f..f230652c4 100644 --- a/pkg/scan/remote_modules_test.go +++ b/pkg/scan/remote_modules_test.go @@ -44,6 +44,86 @@ func TestRemoteModulesManifestEnablesOfflineModuleInstantiation(t *testing.T) { require.Equal(t, filepath.ToSlash(filepath.Join(moduleDir, "main.tf")), filepath.ToSlash(results.Results[0].FileName)) } +func TestCompleteManifestUsesExactExternalScanPaths(t *testing.T) { + root := t.TempDir() + moduleDir := filepath.Join(t.TempDir(), "downloaded-vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, []byte(` +resource "aws_vpc" "included" { + cidr_block = "10.0.0.0/16" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(moduleDir, "example.tf"), []byte(` +resource "aws_vpc" "excluded" { + cidr_block = "10.1.0.0/16" +} +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(root, "main.tf"), []byte(` +module "vpc" { + source = "terraform-aws-modules/vpc/aws" + version = "~> 5.0" +} +`), 0o644)) + manifestPath := writeCompleteRemoteManifest(t, root, moduleDir, moduleFile) + + params := remoteModuleScanParams(root) + params.RemoteModulesManifestPath = manifestPath + results := executeRemoteModuleScan(t, params) + + require.Len(t, results.Results, 1) + require.Equal(t, filepath.ToSlash(moduleFile), filepath.ToSlash(results.Results[0].FileName)) +} + +func TestCompleteManifestBypassesRepositoryDiscovery(t *testing.T) { + repositoryRoot := filepath.Join(t.TempDir(), "removed-repository") + moduleDir := filepath.Join(t.TempDir(), "downloaded-vpc") + require.NoError(t, os.MkdirAll(moduleDir, 0o755)) + moduleFile := filepath.Join(moduleDir, "main.tf") + require.NoError(t, os.WriteFile(moduleFile, nil, 0o644)) + manifestPath := writeCompleteRemoteManifest(t, repositoryRoot, moduleDir, moduleFile) + params := &Parameters{ + RepoPath: repositoryRoot, + RemoteModulesManifestPath: manifestPath, + ModuleMaxDepth: DefaultRemoteModuleMaxDepth, + } + client := &Client{ScanParams: params} + extracted := engineprovider.ExtractedPath{ + Path: []string{repositoryRoot}, + ExtractionMap: make(map[string]model.ExtractedPathObject), + } + + cleanup, scanPaths, mappings, err := client.resolveTerraformModulesForScan( + t.Context(), []string{"Terraform"}, &extracted, + ) + + require.NoError(t, err) + require.NotNil(t, cleanup) + require.Equal(t, []string{moduleFile}, scanPaths) + require.NotEmpty(t, mappings) +} + +func TestIncompleteDiscoveryManifestFallsBackToScannerTraversal(t *testing.T) { + root := t.TempDir() + moduleDir, manifestPath := writeRemoteModuleFixture(t, root) + data, err := os.ReadFile(manifestPath) + require.NoError(t, err) + var manifest resolver.Manifest + require.NoError(t, json.Unmarshal(data, &manifest)) + manifest.SchemaVersion = 3 + manifest.Discovery = &resolver.ManifestDiscovery{Complete: false} + data, err = json.Marshal(manifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + + params := remoteModuleScanParams(root) + params.RemoteModulesManifestPath = manifestPath + results := executeRemoteModuleScan(t, params) + + require.NotEmpty(t, results.Results) + require.Equal(t, filepath.ToSlash(filepath.Join(moduleDir, "main.tf")), filepath.ToSlash(results.Results[0].FileName)) +} + func TestRemoteModulesManifestSkipsMissingEntryWithoutNetworkFallback(t *testing.T) { root := t.TempDir() var requests atomic.Int32 @@ -335,6 +415,43 @@ module "vpc" { return moduleDir, manifestPath } +func writeCompleteRemoteManifest(t *testing.T, repositoryRoot, moduleDir, moduleFile string) string { + t.Helper() + manifestRoot := filepath.Dir(moduleDir) + manifest := resolver.Manifest{ + SchemaVersion: 3, + Dir: manifestRoot, + Modules: map[string]resolver.ManifestEntry{}, + Discovery: &resolver.ManifestDiscovery{ + Complete: true, + Calls: []resolver.ManifestDiscoveryCall{{ + CallID: "vpc-call", + CallerPath: filepath.Join(repositoryRoot, "main.tf"), + Name: "vpc", + Source: "terraform-aws-modules/vpc/aws", + RequestedVersion: "~> 5.0", + }}, + ScanPaths: []string{moduleFile}, + SourceMappings: map[string]string{ + moduleDir: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + }, + ModuleMappings: []resolver.ManifestModuleMapping{{ + CallID: "vpc-call", + LocalPath: moduleDir, + ResolvedVersion: "5.1.2", + CanonicalSource: "registry.terraform.io/terraform-aws-modules/vpc/aws@5.1.2", + Provenance: "prefetched", + Outcome: "resolved", + }}, + }, + } + data, err := json.Marshal(manifest) + require.NoError(t, err) + manifestPath := filepath.Join(manifestRoot, "complete-modules.json") + require.NoError(t, os.WriteFile(manifestPath, data, 0o644)) + return manifestPath +} + func remoteModuleScanParams(root string) *Parameters { return &Parameters{ Path: []string{root},