Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ lint error when query is used both as check and data query #1380

Merged
merged 3 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions internal/bundle/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
queryTitle = "query-name"
queryUidUnique = "query-uid-unique"
queryUnassigned = "query-unassigned"
queryUsedAsDifferentTypes = "query-used-as-different-types"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you need to update the LinterRules var with this entry too so that it shows in the sarif report

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! I missed that

)

type Rule struct {
Expand Down Expand Up @@ -118,6 +119,11 @@ var LinterRules = []Rule{
Name: "Unassigned query",
Description: "The query is not assigned to any policy",
},
{
ID: queryUsedAsDifferentTypes,
Name: "Query used as a check and data query",
Description: "The query is used both as a check and a data query",
},
}

type Results struct {
Expand Down Expand Up @@ -229,6 +235,8 @@ func lintFile(file string) (*Results, error) {
// index global queries that are not embedded
globalQueriesUids := map[string]int{}
globalQueriesByUid := map[string]*Mquery{}
checks := map[string]struct{}{}
dataQueries := map[string]struct{}{}
// child to parent mapping
variantMapping := map[string]string{}
for i := range policyBundle.Queries {
Expand Down Expand Up @@ -414,6 +422,20 @@ func lintFile(file string) (*Results, error) {
uid := check.Uid
updateAssignedQueries(check, assignedQueries, globalQueriesByUid)

checks[check.Uid] = struct{}{}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only check by uid since this is under user control. It is still possible to cause weirdness when the MQL is identical for a query and a check but that's a bug in the resolved policy. It's not something to be caught by the linter

if _, ok := dataQueries[check.Uid]; ok {
res.Entries = append(res.Entries, Entry{
RuleID: queryUsedAsDifferentTypes,
Message: fmt.Sprintf("query %s is used as a check and data query", uid),
Level: levelError,
Location: []Location{{
File: file,
Line: group.FileContext.Line,
Column: group.FileContext.Column,
}},
})
}

// check if the query is embedded
if isEmbeddedQuery(check) {
// NOTE: embedded queries do not need a uid
Expand Down Expand Up @@ -442,6 +464,20 @@ func lintFile(file string) (*Results, error) {
uid := query.Uid
updateAssignedQueries(query, assignedQueries, globalQueriesByUid)

dataQueries[query.Uid] = struct{}{}
if _, ok := checks[query.Uid]; ok {
res.Entries = append(res.Entries, Entry{
RuleID: queryUsedAsDifferentTypes,
Message: fmt.Sprintf("query %s is used as a check and data query", uid),
Level: levelError,
Location: []Location{{
File: file,
Line: group.FileContext.Line,
Column: group.FileContext.Column,
}},
})
}

// check if the query is embedded
if isEmbeddedQuery(query) {
// NOTE: embedded queries do not need a uid
Expand Down
14 changes: 14 additions & 0 deletions internal/bundle/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,17 @@ func TestLintFail(t *testing.T) {
require.NoError(t, err)
assert.True(t, len(data) > 0)
}

func TestLintFail_MixQueries(t *testing.T) {
file := "./testdata/mixing-queries.mql.yaml"
results, err := bundle.Lint(schema, file)
require.NoError(t, err)

assert.Equal(t, 1, len(results.BundleLocations))
assert.Equal(t, 1, len(results.Entries))
assert.True(t, results.HasError())

entry := results.Entries[0]
assert.Equal(t, "query-used-as-different-types", entry.RuleID)
assert.Equal(t, "query sshd-sshd-01 is used as a check and data query", entry.Message)
}
22 changes: 22 additions & 0 deletions internal/bundle/testdata/mixing-queries.mql.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
policies:
- uid: data-queries-mix
name: Test data SSH Policy
version: "1.0.0"
owner_mrn: ""
is_public: true
authors:
- name: Mondoo, Inc.
email: [email protected]
groups:
- title: group 01
checks:
- uid: sshd-sshd-01
queries:
- uid: sshd-sshd-01
filters: |
asset.family.contains(_ == 'unix')

queries:
- uid: sshd-sshd-01
title: Asset name is "test"
query: asset.name == "test"
Loading