feat: NAUT-1300 schema-only projection lint + 19 rule declarations#30
Merged
Conversation
Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
Each rule with profileDependency Required (0) or Optional (1) now declares which profile surfaces it queries. Declarations derived from helper call arguments and co-located literal guards in each rule expression. Signed-off-by: Ben <ben@armosec.io>
Adds a 'Lint projection declarations' step after go test in CI so missing or malformed profileDataRequired blocks fail the build. Adds profileDataRequired authoring guidance to README. Signed-off-by: Ben <ben@armosec.io>
📝 WalkthroughWalkthroughAdds a new CLI command Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI
participant FS as Filesystem
participant Lint as lint-projection
participant YAML as YAML Parser
participant CI as CI Runner
CLI->>FS: walk directories (walkYAMLs)
FS-->>CLI: list of .yaml/.yml files
CLI->>Lint: lintFiles(file list)
Lint->>YAML: read & unmarshal files (capture yaml.Node)
YAML-->>Lint: rule docs + node positions
Lint->>Lint: validate profileDependency vs profileDataRequired
Lint-->>CLI: findings (file:line: ruleID: severity: ...)
CLI->>CI: exit code (1 if any SeverityError, 0 otherwise)
CI-->>User: job result (pass/fail)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
248-250: Document C4 warning behavior explicitly in this section.Consider adding one line that
profileDependency: NotRequired (2)with non-emptyprofileDataRequiredproduces a warning, so authors see all lint outcomes in one place.📝 Suggested docs patch
The lint at `cmd/lint-projection/` enforces that declarations are present and schema-valid. +If `profileDependency` is `NotRequired` (2) and `profileDataRequired` is set, lint emits C4 as a warning. Run it locally with `make lint-projection`. Getting the patterns right is the rule author's responsibility — runtime metrics in node-agent catch drift after deployment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 248 - 250, Add a single explicit sentence to the README section describing lint behavior: state that the lint in cmd/lint-projection will emit a warning when profileDependency is set to NotRequired (2) but profileDataRequired is non-empty; place this sentence near the existing sentence about the lint enforcement so authors can see all possible lint outcomes in one place and use the exact identifiers profileDependency, NotRequired (2), and profileDataRequired to match the rule names used by the linter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/lint-projection/check.go`:
- Around line 78-82: The C4 warning currently only triggers when pdr != nil &&
!pdr.IsEmpty(), which misses an explicit empty declaration like
profileDataRequired: {}; update the condition in the armotypes.NotRequired
branch to warn whenever pdr is non-nil (remove the IsEmpty() check) so any
declared profileDataRequired—even empty—emits the Finding; the change should be
applied in the case armotypes.NotRequired block that constructs the Finding
(using pdr and Finding) so the message and SeverityWarn remain unchanged.
- Around line 57-58: Replace the non-strict yaml.Unmarshal call so unknown
fields are not silently dropped: use a strict decoder (e.g.,
yaml.NewDecoder(bytes.NewReader(data)) with dec.KnownFields(true) and
dec.Decode(&doc) or yaml.UnmarshalStrict) instead of yaml.Unmarshal; update the
error return in the same block that currently references yaml.Unmarshal, keeping
the Finding creation (doc variable, File/path, RuleID, Severity, Check) but
reporting errors from the strict decode so misspelled or unknown mapping keys
cause a failure.
---
Nitpick comments:
In `@README.md`:
- Around line 248-250: Add a single explicit sentence to the README section
describing lint behavior: state that the lint in cmd/lint-projection will emit a
warning when profileDependency is set to NotRequired (2) but profileDataRequired
is non-empty; place this sentence near the existing sentence about the lint
enforcement so authors can see all possible lint outcomes in one place and use
the exact identifiers profileDependency, NotRequired (2), and
profileDataRequired to match the rule names used by the linter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25948c05-aeca-4942-a5f3-19fbc0945302
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (34)
.github/workflows/test.ymlMakefileREADME.mdcmd/lint-projection/check.gocmd/lint-projection/check_test.gocmd/lint-projection/main.gocmd/lint-projection/testdata/error-bad-pattern-multikey.yamlcmd/lint-projection/testdata/error-field-empty.yamlcmd/lint-projection/testdata/error-required-empty.yamlcmd/lint-projection/testdata/error-required-missing.yamlcmd/lint-projection/testdata/valid-notrequired-no-decl.yamlcmd/lint-projection/testdata/valid-optional-all.yamlcmd/lint-projection/testdata/valid-required.yamlcmd/lint-projection/testdata/warn-notrequired-with-decl.yamlgo.modpkg/rules/r0001-unexpected-process-launched/unexpected-process-launched.yamlpkg/rules/r0002-unexpected-file-access/unexpected-file-access.yamlpkg/rules/r0003-unexpected-system-call/unexpected-system-call.yamlpkg/rules/r0004-unexpected-capability-used/unexpected-capability-used.yamlpkg/rules/r0005-unexpected-domain-request/unexpected-domain-request.yamlpkg/rules/r0006-unexpected-service-account-token-access/unexpected-service-account-token-access.yamlpkg/rules/r0007-kubernetes-client-executed/kubernetes-client-executed.yamlpkg/rules/r0008-read-environment-variables-procfs/read-environment-variables-procfs.yamlpkg/rules/r0009-ebpf-program-load/ebpf-program-load.yamlpkg/rules/r0010-unexpected-sensitive-file-access/unexpected-sensitive-file-access.yamlpkg/rules/r0011-unexpected-egress-network-traffic/unexpected-egress-network-traffic.yamlpkg/rules/r1001-exec-binary-not-in-base-image/exec-binary-not-in-base-image.yamlpkg/rules/r1003-malicious-ssh-connection/malicious-ssh-connection.yamlpkg/rules/r1004-exec-from-mount/exec-from-mount.yamlpkg/rules/r1009-crypto-mining-related-port/crypto-mining-related-port.yamlpkg/rules/r1010-symlink-created-over-sensitive-file/symlink-created-over-sensitive-file.yamlpkg/rules/r1011-ld-preload-hook/ld-preload-hook.yamlpkg/rules/r1012-hardlink-created-over-sensitive-file/hardlink-created-over-sensitive-file.yamlpkg/rules/r1030-unexpected-io_uring-operation/unexpected-io_uring-operation.yaml
The pre-existing CI failure was caused by three dependency issues: - node-agent@v0.2.387 used undefined apitypes.AlertSourcePlatformK8s - The matthyx inspektor-gadget fork lacked LostSampleCount() and Release(Data) APIs - No replace directive for cilium/ebpf -> matthyx/ebpf Pin to node-agent@4aefa6517e0a (between creator.go fix and AP/NN→CP migration) which carries the correct matthyx fork replace directives. Mirror both replaces in kubescape-rulelibrary. Also tighten C4 (NotRequired+declaration) check: warn on any non-nil profileDataRequired, including empty structs, matching C1 semantics. Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
matthyx
reviewed
Apr 29, 2026
Per Matthias's review: a YAML file could contain an out-of-range integer for profileDependency; the switch default now emits a C5 error instead of silently producing no findings. Signed-off-by: Ben <ben@armosec.io>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
go.mod (2)
267-269: Fork-based replace directives require ongoing maintenance.Both
inspektor-gadgetandcilium/ebpfare replaced with personal forks (github.com/matthyx/...). While this addresses CI build issues as noted in the PR objectives, consider:
- Documenting why these forks are necessary (e.g., in a comment or README)
- Tracking upstream issues/PRs to eventually remove these replacements
- The pseudo-versions (
20260421...) tie to specific commits—ensure these are periodically rebased on upstreamThis is acceptable for unblocking the PR but should be tracked for future cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 267 - 269, Add a short explanatory comment in go.mod immediately above the two replace directives (the lines replacing github.com/inspektor-gadget/inspektor-gadget and github.com/cilium/ebpf with github.com/matthyx/... pseudo-versions) explaining why the forks are used, linking to the upstream issue/PR numbers or a tracking ticket, and note that the pseudo-versions are tied to specific commits and must be reviewed/rebased periodically; also add a brief entry in the repo README or CONTRIBUTING that references the same rationale and a TODO to remove these replaces once upstream fixes land so future maintainers can find and clean up the replacements.
3-3: Consider updating Go version to 1.26.2. The specified version (1.25.8) is outdated; the current stable release is 1.26.2. If 1.25.8 is required for specific compatibility reasons with the CI environment, this should be documented.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 3, Update the Go version in the go.mod module directive: replace the current 'go 1.25.8' entry with 'go 1.26.2' to use the current stable release (or, if 1.25.8 is required for CI compatibility, add a brief comment in the repository documentation explaining why that older version is pinned); locate the 'go 1.25.8' line in go.mod and change it to 'go 1.26.2' (or document the exception) and ensure any CI config (e.g., workflows or toolchain files) is updated to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@go.mod`:
- Around line 267-269: Add a short explanatory comment in go.mod immediately
above the two replace directives (the lines replacing
github.com/inspektor-gadget/inspektor-gadget and github.com/cilium/ebpf with
github.com/matthyx/... pseudo-versions) explaining why the forks are used,
linking to the upstream issue/PR numbers or a tracking ticket, and note that the
pseudo-versions are tied to specific commits and must be reviewed/rebased
periodically; also add a brief entry in the repo README or CONTRIBUTING that
references the same rationale and a TODO to remove these replaces once upstream
fixes land so future maintainers can find and clean up the replacements.
- Line 3: Update the Go version in the go.mod module directive: replace the
current 'go 1.25.8' entry with 'go 1.26.2' to use the current stable release
(or, if 1.25.8 is required for CI compatibility, add a brief comment in the
repository documentation explaining why that older version is pinned); locate
the 'go 1.25.8' line in go.mod and change it to 'go 1.26.2' (or document the
exception) and ensure any CI config (e.g., workflows or toolchain files) is
updated to match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1dd196c-203d-4314-bfba-3ccefe988cac
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
README.mdcmd/lint-projection/check.gocmd/lint-projection/check_test.gocmd/lint-projection/testdata/error-bad-dependency-value.yamlgo.mod
✅ Files skipped from review due to trivial changes (2)
- cmd/lint-projection/testdata/error-bad-dependency-value.yaml
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/lint-projection/check_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmd/lint-projection/: schema-only lint enforcing C1 (declaration required whenprofileDependencyis Required/Optional), C2 (Validate()from armoapi-go), C3 (YAML unmarshal), C4 (warn: declaration on NotRequired rule).armoapi-goto v0.0.707 which introducedProfileDataRequired,ProfileDataField, andProfileDataPattern.profileDataRequiredblocks to all 19 rules withprofileDependency != NotRequired, derived from helper call arguments and co-located literal guards in each rule expression.make lint-projectionand wires it into CI as a hard gate aftergo test ./....Test plan
go test ./cmd/lint-projection -v— 8 golden fixtures, all C1/C2/C4 checksmake lint-projectionexits 0 overpkg/rules/Related: NAUT-1300 / NAUT-1295
Summary by CodeRabbit
New Features
Documentation
Tests
Chores