Add supply-chain and secrets access checks#1214
Conversation
Add 4 new security checks: cluster-wide-secrets-access (detects ClusterRoleBindings granting broad secrets access), image-not-pinned-by-digest (flags mutable image tags), externalname-service-redirect (flags ExternalName services as potential SSRF vectors), and configmap-with-credentials (detects credential-like keys in ConfigMaps).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a template-backed ChangesBuiltin check additions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/builtinchecks/yamls/cluster-wide-secrets-access.yaml`:
- Around line 16-17: The cluster-wide secrets access check only matches the
literal secrets resource, so it misses ClusterRoles that grant access through
wildcard resources. Update the matching logic in the cluster-wide secrets access
rule to treat resources like "*" as secrets access when paired with the existing
verbs, using the same check definition that currently matches resources and
verbs so broad grants are detected correctly.
In `@pkg/builtinchecks/yamls/configmap-with-credentials.yaml`:
- Around line 15-20: The ConfigMap credential check only scans object.data with
lowercase substring matches, so it misses mixed-case keys and any sensitive
entries under binaryData. Update the rule in the existing
yamls/configmap-with-credentials check to inspect both object.data and
object.binaryData, and compare keys in a case-insensitive way by normalizing the
key names before testing for password, secret, token, api_key, apikey,
private_key, credential, and similar variants. Ensure the logic still returns
the same warning message when either map contains credential-like keys.
In `@pkg/builtinchecks/yamls/externalname-service-redirect.yaml`:
- Around line 15-17: Restrict the ExternalName redirect check so it only applies
to Service objects, since scope.objectKinds: Any can match unrelated resources
that happen to have spec.type set. Update the expression in
externalname-service-redirect to first verify object.kind == 'Service' and add a
has(object.spec) guard before accessing object.spec.type or
object.spec.externalName, keeping the existing ExternalName condition unchanged.
In `@pkg/templates/imagedigest/template.go`:
- Around line 46-49: The digest check in the image-pinning diagnostic is too
weak because `strings.Contains(container.Image, "`@sha256`:")` accepts malformed
references; update the logic in `template.go` to validate a full digest
reference at the end of the image string, or parse the image reference before
deciding to suppress the diagnostic. Keep the diagnostic emission in the
existing container/image check path, and adjust the `dont-fire` fixture in
`tests/checks/image-not-pinned-by-digest.yml` so it uses a syntactically valid
digest reference.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 62f2f137-b071-4fa6-87d9-93a7f138a1e7
📒 Files selected for processing (12)
pkg/builtinchecks/yamls/cluster-wide-secrets-access.yamlpkg/builtinchecks/yamls/configmap-with-credentials.yamlpkg/builtinchecks/yamls/externalname-service-redirect.yamlpkg/builtinchecks/yamls/image-not-pinned-by-digest.yamlpkg/templates/all/all.gopkg/templates/imagedigest/internal/params/gen-params.gopkg/templates/imagedigest/internal/params/params.gopkg/templates/imagedigest/template.gotests/checks/cluster-wide-secrets-access.ymltests/checks/configmap-with-credentials.ymltests/checks/externalname-service-redirect.ymltests/checks/image-not-pinned-by-digest.yml
| has(object.data) && | ||
| object.data.exists(k, | ||
| k.contains('password') || k.contains('secret') || k.contains('token') || | ||
| k.contains('api_key') || k.contains('apikey') || k.contains('private_key') || | ||
| k.contains('credential') | ||
| ) ? 'ConfigMap contains credential-like keys — use Secrets instead' : '' |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and any CEL-related implementation/tests.
git ls-files 'pkg/builtinchecks/yamls/*' 'pkg/**' | sed -n '1,200p'
printf '\n--- target file outline ---\n'
ast-grep outline pkg/builtinchecks/yamls/configmap-with-credentials.yaml --view expanded || true
printf '\n--- search for matches()/regex usage in yamls checks ---\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
"matches\(|contains\(|binaryData|api[_-]?key|private[_-]?key|credential" pkg/builtinchecks/yamls pkg 2>/dev/null | sed -n '1,240p'Repository: stackrox/kube-linter
Length of output: 15758
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant YAML file in a narrow slice with line numbers.
wc -l pkg/builtinchecks/yamls/configmap-with-credentials.yaml
printf '\n--- file excerpt ---\n'
cat -n pkg/builtinchecks/yamls/configmap-with-credentials.yaml | sed -n '1,120p'Repository: stackrox/kube-linter
Length of output: 1155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect CEL template/runtime wiring and any tests around supported functions.
ast-grep outline pkg/templates/cel/template.go --view expanded || true
printf '\n--- cel template excerpt ---\n'
cat -n pkg/templates/cel/template.go | sed -n '1,260p'
printf '\n--- search for cel-go dependency and matches support references ---\n'
rg -n "cel-go|google/cel|matches\(|startsWith\(|endsWith\(|contains\(" go.mod go.sum pkg test .github 2>/dev/null | sed -n '1,260p'Repository: stackrox/kube-linter
Length of output: 5375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the CEL template and tests for expression evaluation behavior.
wc -l pkg/templates/cel/template.go pkg/templates/cel/template_test.go
printf '\n--- template.go excerpt ---\n'
cat -n pkg/templates/cel/template.go | sed -n '1,260p'
printf '\n--- template_test.go excerpt ---\n'
cat -n pkg/templates/cel/template_test.go | sed -n '1,260p'Repository: stackrox/kube-linter
Length of output: 9252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the CEL environment construction and any custom function declarations.
rg -n "cel\.NewEnv|cel\.EnvOption|functions\.|matches|strings\." pkg go.mod go.sum 2>/dev/null | sed -n '1,260p'Repository: stackrox/kube-linter
Length of output: 30628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate CEL environment creation and the exact expression evaluator used by built-in checks.
rg -n "cel\.NewEnv|cel\.Compile|cel\.Program|Evaluate|cel-expression|NewProgram|EnvOption|FunctionDecl|matches\(" pkg | sed -n '1,260p'Repository: stackrox/kube-linter
Length of output: 662
🌐 Web query:
cel-go v0.28.1 string.matches regex semantics CEL standard library string method matches
💡 Result:
In the Common Expression Language (CEL), the string method matches is part of the standard library and is used to test whether a string matches a specified regular expression [1][2][3]. Regex Semantics: CEL uses RE2 syntax for its regular expressions [4][5][6]. A matches operation succeeds if the provided regular expression matches any substring of the target string [4][5][6]. To perform a full-string match—where the pattern must match the entire input—you must explicitly anchor the regular expression using the ^ (start of string) and $ (end of string) anchors [4][5][6]. Usage in cel-go v0.28.1: The method is invoked using member syntax: targetString.matches(regexPattern) [1]. The targetString is the string being evaluated, and regexPattern is the string containing the RE2-compliant regular expression [7][1]. Key distinctions: 1. Standard Library vs. Extensions: The matches function is a core part of the CEL standard library and is always available [3]. It is distinct from the additional regex operations (such as find or findAll) provided by the optional cel-go regex extension library, which may require explicit registration in your CEL environment [2]. 2. Versioning: As of cel-go v0.28.1, the core behavior of matches remains consistent with the CEL specification, following RE2 syntax and substring matching semantics [4][8]. Recent updates to cel-go (including v0.28.1) have focused on areas such as cost estimation and performance tracking for regex operations, but they have not altered the fundamental substring-matching behavior of the matches method [8][9].
Citations:
- 1: https://docs.kuadrant.io/1.4.x/kuadrant-operator/doc/cel/standard/
- 2: https://kubernetes.io/docs/reference/using-api/cel/
- 3: https://kro.run/docs/concepts/rgd/cel-libraries/
- 4: https://github.com/google/cel-spec/blob/v0.13.0/doc/langdef.md
- 5: https://github.com/cel-expr/cel-spec/blob/master/doc/langdef.md
- 6: https://github.com/google/cel-spec/blob/master/doc/langdef.md
- 7: https://github.com/google/cel-go/blob/master/common/types/string.go
- 8: https://github.com/google/cel-go/releases/tag/v0.28.1
- 9: https://github.com/google/cel-go/releases
🌐 Web query:
cel-go v0.28.1 string.matches regex semantics CEL standard library string method matches
💡 Result:
In cel-go v0.28.1, the string.matches(regex) method is part of the CEL standard library [1][2]. Its semantics are defined by the CEL language specification and implemented in Go using the RE2 regular expression engine [1][2][3]. Key technical details regarding the matches method include: Regex Syntax: CEL uses RE2 syntax for regular expressions [1][3]. You can reference the official RE2 syntax documentation for details on supported patterns. Matching Behavior: A matches call evaluates to true if the regular expression matches anywhere within the target string (a substring match) [1][3]. To perform a full-string match, you must explicitly use anchors such as ^ (start of string) and $ (end of string) in your regex pattern [1][3]. Implementation: In the cel-go runtime, the matches method on a string type is implemented to invoke Go's standard regexp package (which uses the RE2 engine) to validate the pattern against the input string [4]. Best Practices: - Anchors: Use ^ and $ to ensure the regex matches the entire string rather than just a substring [1][3]. - Optimization: In modern cel-go versions, if a regex pattern is provided as a static string constant, it can be type-checked and pre-compiled at compile time rather than being recompiled at runtime for every evaluation [5]. - Error Handling: If an invalid regex pattern is provided, the function will return an error [4][6]. When constructing CEL expressions dynamically in Go, ensure that external input strings used as regex patterns are properly escaped (e.g., using regexp.QuoteMeta) to avoid syntax errors or injection issues [6].
Citations:
- 1: https://github.com/cel-expr/cel-spec/blob/master/doc/langdef.md
- 2: https://github.com/google/cel-spec/blob/master/doc/langdef.md
- 3: https://celbyexample.com/strings
- 4: https://github.com/google/cel-go/blob/master/common/types/string.go
- 5: Type check and pre-compile fixed regex strings cel-expr/cel-go#496
- 6: Newbie Question: parser crashed when using .matches cel-expr/cel-go#535
Expand the check to inspect binaryData and case-insensitive key names.
object.data only catches lower-case keys; PASSWORD, API_KEY, clientSecret, and any credential-like keys in binaryData still bypass this rule.
Suggested change
- has(object.data) &&
- object.data.exists(k,
- k.contains('password') || k.contains('secret') || k.contains('token') ||
- k.contains('api_key') || k.contains('apikey') || k.contains('private_key') ||
- k.contains('credential')
- ) ? 'ConfigMap contains credential-like keys — use Secrets instead' : ''
+ (
+ (has(object.data) && object.data.exists(k,
+ k.matches('(?i).*(password|secret|token|api[_-]?key|private[_-]?key|credential).*')
+ )) ||
+ (has(object.binaryData) && object.binaryData.exists(k,
+ k.matches('(?i).*(password|secret|token|api[_-]?key|private[_-]?key|credential).*')
+ ))
+ ) ? 'ConfigMap contains credential-like keys — use Secrets instead' : ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| has(object.data) && | |
| object.data.exists(k, | |
| k.contains('password') || k.contains('secret') || k.contains('token') || | |
| k.contains('api_key') || k.contains('apikey') || k.contains('private_key') || | |
| k.contains('credential') | |
| ) ? 'ConfigMap contains credential-like keys — use Secrets instead' : '' | |
| ( | |
| (has(object.data) && object.data.exists(k, | |
| k.matches('(?i).*(password|secret|token|api[_-]?key|private[_-]?key|credential).*') | |
| )) || | |
| (has(object.binaryData) && object.binaryData.exists(k, | |
| k.matches('(?i).*(password|secret|token|api[_-]?key|private[_-]?key|credential).*') | |
| )) | |
| ) ? 'ConfigMap contains credential-like keys — use Secrets instead' : '' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/builtinchecks/yamls/configmap-with-credentials.yaml` around lines 15 -
20, The ConfigMap credential check only scans object.data with lowercase
substring matches, so it misses mixed-case keys and any sensitive entries under
binaryData. Update the rule in the existing yamls/configmap-with-credentials
check to inspect both object.data and object.binaryData, and compare keys in a
case-insensitive way by normalizing the key names before testing for password,
secret, token, api_key, apikey, private_key, credential, and similar variants.
Ensure the logic still returns the same warning message when either map contains
credential-like keys.
- Add wildcard resource match to cluster-wide-secrets-access - Add Service kind guard to externalname-service-redirect - Add binaryData scanning to configmap-with-credentials - Use proper digest regex validation in image-not-pinned-by-digest template - Update test fixture with valid 64-char hex digest - Regenerate docs/generated/checks.md and templates.md
- Add bats test entries for cluster-wide-secrets-access, configmap-with-credentials, externalname-service-redirect, and image-not-pinned-by-digest in correct alphabetical order - Regenerate gen-params.go with correct indentation via go generate - Regenerate docs
1 participant
Summary
image-not-pinned-by-digest) following thelatesttagpatternaccess-to-resources,cel-expression)New Checks
cluster-wide-secrets-accessimage-not-pinned-by-digestexternalname-service-redirectconfigmap-with-credentialsTest plan
go build ./...passesgo test ./...passeslatesttagpattern exactly