diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index d3485289..36d206a7 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -12,3 +12,5 @@ github.com/spf13/pflag,https://github.com/spf13/pflag,BSD-3-Clause,"Copyright (c golang.org/x/net,https://github.com/golang/net,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sync,https://github.com/golang/sync,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. golang.org/x/sys,https://github.com/golang/sys,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. +golang.org/x/mod,https://github.com/golang/mod,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. +golang.org/x/tools,https://github.com/golang/tools,BSD-3-Clause,Copyright (c) 2009 The Go Authors. All rights reserved. diff --git a/allowedsymbols/analyzer.go b/allowedsymbols/analyzer.go new file mode 100644 index 00000000..6afb25ea --- /dev/null +++ b/allowedsymbols/analyzer.go @@ -0,0 +1,207 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package allowedsymbols provides a go/analysis.Analyzer that enforces +// symbol-level import restrictions on Go source files. +// +// The analyzer checks that every imported symbol is in a given allowlist, that +// no permanently banned packages are imported, and that every symbol in the +// allowlist is actually used. It reports violations with file:line:col +// diagnostics. +package allowedsymbols + +import ( + "fmt" + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" +) + +// AnalyzerConfig configures a single instance of the allowed-symbols analyzer. +type AnalyzerConfig struct { + // Symbols is the allowlist to enforce (e.g. builtinAllowedSymbols). + // Each entry must be in "importpath.Symbol" form. + Symbols []string + // ExemptImport returns true for import paths that are auto-allowed and + // should not be checked against the allowlist (e.g. same-module imports). + ExemptImport func(importPath string) bool + // ListName is used in diagnostic messages (e.g. "builtinAllowedSymbols"). + ListName string +} + +// NewAnalyzer returns a go/analysis.Analyzer that enforces the symbol-level +// import restrictions described by cfg. Violations are reported via +// pass.Reportf and appear as diagnostics with proper file:line:col positions. +// +// NewAnalyzer panics if any entry in cfg.Symbols is malformed (no dot +// separator), matching the behaviour of the test-harness variant. +func NewAnalyzer(cfg AnalyzerConfig) *analysis.Analyzer { + for _, entry := range cfg.Symbols { + if strings.LastIndexByte(entry, '.') <= 0 { + panic(fmt.Sprintf("allowedsymbols.NewAnalyzer: malformed allowlist entry (no dot): %q", entry)) + } + } + + run := func(pass *analysis.Pass) (any, error) { + allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) + usedSymbols := make(map[string]bool, len(cfg.Symbols)) + + for _, f := range pass.Files { + localToPath := checkFileImports(f, allowedPkgs, cfg.ExemptImport, + func(pos token.Pos, format string, args ...any) { + pass.Reportf(pos, format, args...) + }) + + checkFileSelectors(f, localToPath, allowedSyms, usedSymbols, + func(pos token.Pos, format string, args ...any) { + pass.Reportf(pos, format, args...) + }) + } + + if len(pass.Files) > 0 { + reportUnused(cfg.Symbols, usedSymbols, func(entry string) { + pass.Reportf(pass.Files[0].Package, + "allowlist symbol %q is not used by any file — remove it from %s", + entry, cfg.ListName) + }) + } + + return nil, nil + } + + return &analysis.Analyzer{ + Name: "allowedsymbols", + Doc: "enforces symbol-level import restrictions via an allowlist", + Run: run, + } +} + +// buildAllowlistSets constructs lookup maps from a flat allowlist. +// It returns (allowedSymbols, allowedPackages) where allowedSymbols maps +// "pkg.Symbol" → true and allowedPackages maps "importpath" → true. +func buildAllowlistSets(symbols []string) (map[string]bool, map[string]bool) { + allowedSyms := make(map[string]bool, len(symbols)) + allowedPkgs := make(map[string]bool) + for _, entry := range symbols { + dot := strings.LastIndexByte(entry, '.') + if dot <= 0 { + continue + } + allowedSyms[entry] = true + allowedPkgs[entry[:dot]] = true + } + return allowedSyms, allowedPkgs +} + +// checkFileImports validates each import in f against the permanently banned +// list, the exempt predicate, and allowedPkgs. It calls report for each +// violation and returns a localName→importPath map for the file's valid, +// non-exempt imports. +// +// report is called with a token.Pos and a pre-formatted message. Callers that +// use the file:line string form (e.g. fileLineReporter) set pos to +// token.NoPos; callers that surface diagnostics via pass.Reportf pass the +// actual source position. +func checkFileImports( + f *ast.File, + allowedPkgs map[string]bool, + exemptImport func(string) bool, + report func(pos token.Pos, format string, args ...any), +) map[string]string { + localToPath := make(map[string]string) + + for _, imp := range f.Imports { + importPath := strings.Trim(imp.Path.Value, `"`) + + banned := false + for key, reason := range permanentlyBanned { + if strings.HasSuffix(key, "/") { + if strings.HasPrefix(importPath, key) { + report(imp.Pos(), "import of %q is permanently banned (%s)", importPath, reason) + banned = true + break + } + } else if importPath == key { + report(imp.Pos(), "import of %q is permanently banned (%s)", importPath, reason) + banned = true + break + } + } + if banned { + continue + } + + if exemptImport != nil && exemptImport(importPath) { + continue + } + + var localName string + if imp.Name != nil { + localName = imp.Name.Name + } else { + parts := strings.Split(importPath, "/") + localName = parts[len(parts)-1] + } + + if localName == "_" || localName == "." { + report(imp.Pos(), "blank/dot import of %q is not allowed", importPath) + continue + } + + if !allowedPkgs[importPath] { + report(imp.Pos(), "import of %q is not in the allowlist", importPath) + continue + } + + localToPath[localName] = importPath + } + + return localToPath +} + +// checkFileSelectors walks all ast.SelectorExpr nodes in f and checks each +// package-qualified symbol against allowedSyms. Allowed symbols are recorded +// in usedSymbols. Violations are sent to report. +func checkFileSelectors( + f *ast.File, + localToPath map[string]string, + allowedSyms map[string]bool, + usedSymbols map[string]bool, + report func(pos token.Pos, format string, args ...any), +) { + ast.Inspect(f, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return true + } + importPath, ok := localToPath[ident.Name] + if !ok { + return true // not a package-level selector + } + key := importPath + "." + sel.Sel.Name + if !allowedSyms[key] { + report(sel.Pos(), "%s is not in the allowlist", key) + } else { + usedSymbols[key] = true + } + return true + }) +} + +// reportUnused calls onUnused for each symbol in symbols that is not present +// in usedSymbols. +func reportUnused(symbols []string, usedSymbols map[string]bool, onUnused func(entry string)) { + for _, entry := range symbols { + if !usedSymbols[entry] { + onUnused(entry) + } + } +} diff --git a/allowedsymbols/analyzer_test.go b/allowedsymbols/analyzer_test.go new file mode 100644 index 00000000..e192822c --- /dev/null +++ b/allowedsymbols/analyzer_test.go @@ -0,0 +1,86 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package allowedsymbols + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +// TestNewAnalyzer_Metadata verifies that the returned Analyzer has the correct +// name and does not declare unnecessary framework dependencies. +func TestNewAnalyzer_Metadata(t *testing.T) { + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + if a.Name != "allowedsymbols" { + t.Errorf("Name = %q, want \"allowedsymbols\"", a.Name) + } + if len(a.Requires) != 0 { + t.Errorf("Requires has %d entries, want 0", len(a.Requires)) + } +} + +// TestNewAnalyzer_MalformedEntry verifies that NewAnalyzer panics immediately +// when given an allowlist entry with no dot separator, matching the test-harness +// behaviour (t.Fatalf on the same condition). +func TestNewAnalyzer_MalformedEntry(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic for malformed allowlist entry, got none") + } + }() + NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"nodot"}, + ListName: "test", + }) +} + +// TestNewAnalyzer_AllowedImport runs the analyzer against a package that uses +// only allowlisted symbols; no diagnostics should be emitted. +func TestNewAnalyzer_AllowedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "good") +} + +// TestNewAnalyzer_BannedImport verifies that a permanently banned import is +// reported at the import site. +func TestNewAnalyzer_BannedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "bannedimport") +} + +// TestNewAnalyzer_DisallowedImport verifies that an import not in the +// allowlist is reported at the import site. +func TestNewAnalyzer_DisallowedImport(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "disallowedimport") +} + +// TestNewAnalyzer_UnusedSymbol verifies that an allowlisted symbol that is +// never referenced in the package is reported at the package declaration. +func TestNewAnalyzer_UnusedSymbol(t *testing.T) { + testdata := analysistest.TestData() + a := NewAnalyzer(AnalyzerConfig{ + Symbols: []string{"fmt.Println", "fmt.Sprintf"}, + ListName: "test", + }) + analysistest.Run(t, testdata, a, "unusedsymbol") +} diff --git a/allowedsymbols/symbols_test.go b/allowedsymbols/symbols_test.go index 550611b1..3a1c6342 100644 --- a/allowedsymbols/symbols_test.go +++ b/allowedsymbols/symbols_test.go @@ -7,7 +7,6 @@ package allowedsymbols import ( "fmt" - "go/ast" "go/parser" "go/token" "os" @@ -45,20 +44,23 @@ type allowedSymbolsConfig struct { // Go source files. It verifies that every imported symbol is in the allowlist, // that no permanently banned packages are imported, and that every symbol in // the allowlist is actually used. +// +// The core checking logic is provided by the analyzer helpers in analyzer.go +// (checkFileImports, checkFileSelectors, reportUnused) — this function handles +// file discovery, AST parsing, and test-framework integration. func checkAllowedSymbols(t *testing.T, cfg allowedSymbolsConfig) { t.Helper() // Build lookup sets from the allowlist. - allowedSymbols := make(map[string]bool, len(cfg.Symbols)) + allowedSyms, allowedPkgs := buildAllowlistSets(cfg.Symbols) usedSymbols := make(map[string]bool, len(cfg.Symbols)) - allowedPackages := make(map[string]bool) + + // Validate allowlist entries are well-formed. for _, entry := range cfg.Symbols { dot := strings.LastIndexByte(entry, '.') if dot <= 0 { t.Fatalf("malformed allowlist entry (no dot): %q", entry) } - allowedSymbols[entry] = true - allowedPackages[entry[:dot]] = true } // reportErr collects errors into cfg.Errors when set, otherwise calls t.Errorf. @@ -102,90 +104,22 @@ func checkAllowedSymbols(t *testing.T, cfg allowedSymbolsConfig) { continue } - // Build a map from local package name → import path and validate each import. - localToPath := make(map[string]string) - for _, imp := range f.Imports { - importPath := strings.Trim(imp.Path.Value, `"`) - - banned := false - for key, reason := range permanentlyBanned { - if strings.HasSuffix(key, "/") { - if strings.HasPrefix(importPath, key) { - reportErr("%s: import of %q is permanently banned (%s)", rel, importPath, reason) - banned = true - break - } - } else if importPath == key { - reportErr("%s: import of %q is permanently banned (%s)", rel, importPath, reason) - banned = true - break - } - } - if banned { - continue - } - - if cfg.ExemptImport(importPath) { - continue - } - - // Determine the local name used to reference this package. - var localName string - if imp.Name != nil { - localName = imp.Name.Name - } else { - parts := strings.Split(importPath, "/") - localName = parts[len(parts)-1] - } - - if localName == "_" || localName == "." { - reportErr("%s: blank/dot import of %q is not allowed", rel, importPath) - continue - } - - if !allowedPackages[importPath] { - reportErr("%s: import of %q is not in the allowlist", rel, importPath) - continue - } - - localToPath[localName] = importPath - } - - // Walk all selector expressions and verify each pkg.Symbol is allowed. - ast.Inspect(f, func(n ast.Node) bool { - sel, ok := n.(*ast.SelectorExpr) - if !ok { - return true - } - ident, ok := sel.X.(*ast.Ident) - if !ok { - return true - } - importPath, ok := localToPath[ident.Name] - if !ok { - return true // not a package-level selector - } - key := importPath + "." + sel.Sel.Name - if !allowedSymbols[key] { - pos := fset.Position(sel.Pos()) - reportErr("%s:%d: %s is not in the allowlist", rel, pos.Line, key) - } else { - usedSymbols[key] = true - } - return true - }) + // Use analyzer helpers for import checking and selector walking. + reporter := fileLineReporter(fset, rel, reportErr) + localToPath := checkFileImports(f, allowedPkgs, cfg.ExemptImport, reporter) + checkFileSelectors(f, localToPath, allowedSyms, usedSymbols, reporter) } + if checked < cfg.MinFiles { t.Fatalf("expected at least %d files in %s, found %d", cfg.MinFiles, cfg.TargetDir, checked) } // Verify every symbol in the allowlist is actually used by at least one // file. Unused entries should be removed to keep the allowlist minimal. - for _, entry := range cfg.Symbols { - if !usedSymbols[entry] { - reportErr("allowlist symbol %q is not used by any file in %s — remove it from %s", entry, cfg.TargetDir, cfg.ListName) - } - } + reportUnused(cfg.Symbols, usedSymbols, func(entry string) { + reportErr("allowlist symbol %q is not used by any file in %s — remove it from %s", + entry, cfg.TargetDir, cfg.ListName) + }) } // perBuiltinConfig holds the configuration for checkPerBuiltinAllowedSymbols. @@ -363,3 +297,20 @@ func collectFlatGoFiles(dir string) ([]string, error) { } return files, nil } + +// fileLineReporter returns a report function suitable for use with +// checkFileImports and checkFileSelectors in the test harness. It translates +// token.Pos into file:line strings using fset and forwards messages via errorf. +// When pos is token.NoPos, only the format+args message is emitted without a +// location prefix. +func fileLineReporter(fset *token.FileSet, relPath string, errorf func(string, ...any)) func(token.Pos, string, ...any) { + return func(pos token.Pos, format string, args ...any) { + msg := fmt.Sprintf(format, args...) + if pos != token.NoPos && fset != nil { + p := fset.Position(pos) + errorf("%s:%d: %s", relPath, p.Line, msg) + } else { + errorf("%s: %s", relPath, msg) + } + } +} diff --git a/allowedsymbols/testdata/src/bannedimport/bannedimport.go b/allowedsymbols/testdata/src/bannedimport/bannedimport.go new file mode 100644 index 00000000..d088b546 --- /dev/null +++ b/allowedsymbols/testdata/src/bannedimport/bannedimport.go @@ -0,0 +1,18 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package bannedimport imports a permanently banned package. +package bannedimport + +import ( + "fmt" + "os/exec" // want `import of "os/exec" is permanently banned` +) + +// Run uses a banned import. +func Run() { + fmt.Println("run") + _ = exec.Command("ls") +} diff --git a/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go new file mode 100644 index 00000000..1e25f753 --- /dev/null +++ b/allowedsymbols/testdata/src/disallowedimport/disallowedimport.go @@ -0,0 +1,18 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package disallowedimport imports a package not in the allowlist. +package disallowedimport + +import ( + "bufio" // want `import of "bufio" is not in the allowlist` + "fmt" +) + +// Read uses an import that is not allowlisted. +func Read() { + fmt.Println("read") + _ = bufio.NewScanner(nil) +} diff --git a/allowedsymbols/testdata/src/good/good.go b/allowedsymbols/testdata/src/good/good.go new file mode 100644 index 00000000..52e1ca46 --- /dev/null +++ b/allowedsymbols/testdata/src/good/good.go @@ -0,0 +1,14 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package good uses only allowlisted symbols; no diagnostics expected. +package good + +import "fmt" + +// Hello prints a greeting. +func Hello() { + fmt.Println("hello") +} diff --git a/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go new file mode 100644 index 00000000..13bd6b3e --- /dev/null +++ b/allowedsymbols/testdata/src/unusedsymbol/unusedsymbol.go @@ -0,0 +1,14 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +// Package unusedsymbol has an allowlisted symbol that is never used. +package unusedsymbol // want `allowlist symbol "fmt.Sprintf" is not used` + +import "fmt" + +// Hello uses only fmt.Println; fmt.Sprintf is in the allowlist but unused. +func Hello() { + fmt.Println("hello") +} diff --git a/go.mod b/go.mod index 42a88c05..7be42efe 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 golang.org/x/sys v0.42.0 + golang.org/x/tools v0.43.0 gopkg.in/yaml.v3 v3.0.1 mvdan.cc/sh/v3 v3.13.0 ) @@ -17,6 +18,7 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/net v0.49.0 // indirect - golang.org/x/sync v0.19.0 // indirect + golang.org/x/mod v0.34.0 // indirect + golang.org/x/net v0.52.0 // indirect + golang.org/x/sync v0.20.0 // indirect ) diff --git a/go.sum b/go.sum index 930144f8..a8069899 100644 --- a/go.sum +++ b/go.sum @@ -28,12 +28,16 @@ github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3A github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= +golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= +golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= +golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= +golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=