Skip to content

Commit

Permalink
refactor(misconf): remove file filtering from parsers (#7289)
Browse files Browse the repository at this point in the history
Signed-off-by: nikpivkin <[email protected]>
  • Loading branch information
nikpivkin authored Aug 2, 2024
1 parent 2a0e529 commit e95152f
Show file tree
Hide file tree
Showing 26 changed files with 173 additions and 337 deletions.
11 changes: 8 additions & 3 deletions pkg/iac/detection/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,20 @@ func init() {
}

sniff := struct {
ContentType string `json:"contentType"`
Parameters map[string]any `json:"parameters"`
Resources []any `json:"resources"`
Schema string `json:"$schema"`
Parameters map[string]any `json:"parameters"`
Resources []any `json:"resources"`
}{}
metadata := types.NewUnmanagedMetadata()
if err := armjson.UnmarshalFromReader(r, &sniff, &metadata); err != nil {
return false
}

// schema is required https://learn.microsoft.com/en-us/azure/azure-resource-manager/templates/syntax
if !strings.HasPrefix(sniff.Schema, "https://schema.management.azure.com/schemas") {
return false
}

return (sniff.Parameters != nil && len(sniff.Parameters) > 0) ||
(sniff.Resources != nil && len(sniff.Resources) > 0)
}
Expand Down
64 changes: 64 additions & 0 deletions pkg/iac/detection/detect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,70 @@ rules:
FileTypeYAML,
},
},
{
name: "Azure ARM template with resources",
path: "test.json",
r: strings.NewReader(`
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"resources": [
{
"type": "Microsoft.Storage/storageAccounts",
"apiVersion": "2021-09-01",
"name": "{provide-unique-name}",
"location": "eastus",
"sku": {
"name": "Standard_LRS"
},
"kind": "StorageV2",
"properties": {
"supportsHttpsTrafficOnly": true
}
}
]
}
`),
expected: []FileType{
FileTypeJSON,
FileTypeAzureARM,
},
},
{
name: "Azure ARM template with parameters",
path: "test.json",
r: strings.NewReader(`
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"parameters": {
"storageName": {
"type": "string",
"minLength": 3,
"maxLength": 24
}
}
}
`),
expected: []FileType{
FileTypeJSON,
FileTypeAzureARM,
},
},
{
name: "empty Azure ARM template",
path: "test.json",
r: strings.NewReader(`
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"contentVersion": "1.0.0.0",
"resources": []
}
`),
expected: []FileType{
FileTypeJSON,
},
},
}

for _, test := range tests {
Expand Down
4 changes: 0 additions & 4 deletions pkg/iac/rego/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ func (s *Scanner) SetPolicyNamespaces(namespaces ...string) {
}
}

func (s *Scanner) SetSkipRequiredCheck(_ bool) {
// NOTE: Skip required option not applicable for rego.
}

func (s *Scanner) SetRegoErrorLimit(limit int) {
s.regoErrorLimit = limit
}
Expand Down
44 changes: 4 additions & 40 deletions pkg/iac/scanners/azure/arm/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"
"io"
"io/fs"
"path/filepath"
"strings"

"github.com/aquasecurity/trivy/pkg/iac/debug"
azure2 "github.com/aquasecurity/trivy/pkg/iac/scanners/azure"
Expand All @@ -17,19 +15,14 @@ import (
)

type Parser struct {
targetFS fs.FS
skipRequired bool
debug debug.Logger
targetFS fs.FS
debug debug.Logger
}

func (p *Parser) SetDebugWriter(writer io.Writer) {
p.debug = debug.New(writer, "azure", "arm")
}

func (p *Parser) SetSkipRequiredCheck(b bool) {
p.skipRequired = b
}

func New(targetFS fs.FS, opts ...options.ParserOption) *Parser {
p := &Parser{
targetFS: targetFS,
Expand All @@ -56,14 +49,13 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) ([]azure2.Deployment,
if entry.IsDir() {
return nil
}
if !p.Required(path) {
return nil
}

f, err := p.targetFS.Open(path)
if err != nil {
return err
}
defer f.Close()

deployment, err := p.parseFile(f, path)
if err != nil {
return err
Expand All @@ -77,34 +69,6 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) ([]azure2.Deployment,
return deployments, nil
}

func (p *Parser) Required(path string) bool {
if p.skipRequired {
return true
}
if !strings.HasSuffix(path, ".json") {
return false
}
data, err := fs.ReadFile(p.targetFS, path)
if err != nil {
return false
}
var template Template
root := types.NewMetadata(
types.NewRange(filepath.Base(path), 0, 0, "", p.targetFS),
"",
)
if err := armjson.Unmarshal(data, &template, &root); err != nil {
p.debug.Log("Error scanning %s: %s", path, err)
return false
}

if template.Schema.Kind != azure2.KindString {
return false
}

return strings.HasPrefix(template.Schema.AsString(), "https://schema.management.azure.com")
}

func (p *Parser) parseFile(r io.Reader, filename string) (*azure2.Deployment, error) {
var template Template
data, err := io.ReadAll(r)
Expand Down
5 changes: 0 additions & 5 deletions pkg/iac/scanners/azure/arm/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,6 @@ func TestParser_Parse(t *testing.T) {
want func() azure2.Deployment
wantDeployment bool
}{
{
name: "invalid code",
input: `blah`,
wantDeployment: false,
},
{
name: "basic param",
input: `{
Expand Down
12 changes: 4 additions & 8 deletions pkg/iac/scanners/azure/arm/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,19 @@ import (
var _ scanners.FSScanner = (*Scanner)(nil)
var _ options.ConfigurableScanner = (*Scanner)(nil)

type Scanner struct { // nolint: gocritic
type Scanner struct {
mu sync.Mutex
scannerOptions []options.ScannerOption
parserOptions []options.ParserOption
debug debug.Logger
frameworks []framework.Framework
skipRequired bool
regoOnly bool
loadEmbeddedPolicies bool
loadEmbeddedLibraries bool
policyDirs []string
policyReaders []io.Reader
regoScanner *rego.Scanner
spec string
sync.Mutex
}

func (s *Scanner) SetIncludeDeprecatedChecks(b bool) {}
Expand Down Expand Up @@ -73,9 +72,6 @@ func (s *Scanner) SetPolicyDirs(dirs ...string) {
s.policyDirs = dirs
}

func (s *Scanner) SetSkipRequiredCheck(skipRequired bool) {
s.skipRequired = skipRequired
}
func (s *Scanner) SetPolicyReaders(readers []io.Reader) {
s.policyReaders = readers
}
Expand Down Expand Up @@ -106,8 +102,8 @@ func (s *Scanner) SetPolicyNamespaces(...string) {}
func (s *Scanner) SetRegoErrorLimit(_ int) {}

func (s *Scanner) initRegoScanner(srcFS fs.FS) error {
s.Lock()
defer s.Unlock()
s.mu.Lock()
defer s.mu.Unlock()
if s.regoScanner != nil {
return nil
}
Expand Down
29 changes: 0 additions & 29 deletions pkg/iac/scanners/cloudformation/parser/parser.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package parser

import (
"bytes"
"context"
"encoding/json"
"errors"
Expand All @@ -15,7 +14,6 @@ import (
"gopkg.in/yaml.v3"

"github.com/aquasecurity/trivy/pkg/iac/debug"
"github.com/aquasecurity/trivy/pkg/iac/detection"
"github.com/aquasecurity/trivy/pkg/iac/ignore"
"github.com/aquasecurity/trivy/pkg/iac/scanners/options"
)
Expand All @@ -24,7 +22,6 @@ var _ options.ConfigurableParser = (*Parser)(nil)

type Parser struct {
debug debug.Logger
skipRequired bool
parameterFiles []string
parameters map[string]any
overridedParameters Parameters
Expand Down Expand Up @@ -59,10 +56,6 @@ func (p *Parser) SetDebugWriter(writer io.Writer) {
p.debug = debug.New(writer, "cloudformation", "parser")
}

func (p *Parser) SetSkipRequiredCheck(b bool) {
p.skipRequired = b
}

func New(opts ...options.ParserOption) *Parser {
p := &Parser{}
for _, option := range opts {
Expand All @@ -86,11 +79,6 @@ func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, dir string) (FileConte
return nil
}

if !p.Required(fsys, path) {
p.debug.Log("not a CloudFormation file, skipping %s", path)
return nil
}

c, err := p.ParseFile(ctx, fsys, path)
if err != nil {
p.debug.Log("Error parsing file '%s': %s", path, err)
Expand All @@ -104,23 +92,6 @@ func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, dir string) (FileConte
return contexts, nil
}

func (p *Parser) Required(fsys fs.FS, path string) bool {
if p.skipRequired {
return true
}

f, err := fsys.Open(filepath.ToSlash(path))
if err != nil {
return false
}
defer func() { _ = f.Close() }()
if data, err := io.ReadAll(f); err == nil {
return detection.IsType(path, bytes.NewReader(data), detection.FileTypeCloudFormation)
}
return false

}

func (p *Parser) ParseFile(ctx context.Context, fsys fs.FS, path string) (fctx *FileContext, err error) {
defer func() {
if e := recover(); e != nil {
Expand Down
15 changes: 5 additions & 10 deletions pkg/iac/scanners/cloudformation/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,20 @@ func WithConfigsFS(fsys fs.FS) options.ScannerOption {
var _ scanners.FSScanner = (*Scanner)(nil)
var _ options.ConfigurableScanner = (*Scanner)(nil)

type Scanner struct { // nolint: gocritic
type Scanner struct {
mu sync.Mutex
debug debug.Logger
policyDirs []string
policyReaders []io.Reader
parser *parser.Parser
regoScanner *rego.Scanner
skipRequired bool
regoOnly bool
loadEmbeddedPolicies bool
loadEmbeddedLibraries bool
options []options.ScannerOption
parserOptions []options.ParserOption
frameworks []framework.Framework
spec string
sync.Mutex
}

func (s *Scanner) SetIncludeDeprecatedChecks(bool) {}
Expand Down Expand Up @@ -98,12 +97,9 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) {
s.policyReaders = readers
}

func (s *Scanner) SetSkipRequiredCheck(skip bool) {
s.skipRequired = skip
}

func (s *Scanner) SetDebugWriter(writer io.Writer) {
s.debug = debug.New(writer, "cloudformation", "scanner")
s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer))
}

func (s *Scanner) SetPolicyDirs(dirs ...string) {
Expand Down Expand Up @@ -132,14 +128,13 @@ func New(opts ...options.ScannerOption) *Scanner {
for _, opt := range opts {
opt(s)
}
s.addParserOptions(options.ParserWithSkipRequiredCheck(s.skipRequired))
s.parser = parser.New(s.parserOptions...)
return s
}

func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) {
s.Lock()
defer s.Unlock()
s.mu.Lock()
defer s.mu.Unlock()
if s.regoScanner != nil {
return s.regoScanner, nil
}
Expand Down
Loading

0 comments on commit e95152f

Please sign in to comment.