Skip to content

NO-ISSUE: Reduce false positives - #4

Merged
sk-ilya merged 2 commits into
mainfrom
checks-false-positives
Jun 22, 2026
Merged

NO-ISSUE: Reduce false positives#4
sk-ilya merged 2 commits into
mainfrom
checks-false-positives

Conversation

@sk-ilya

@sk-ilya sk-ilya commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Chores
    • Added automated YAML linting to the CI/CD pipeline for code quality enforcement
    • Refined code review configuration to adjust security checks and file exclusions

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Refines .coderabbit.yaml prodsec custom-check rules to exclude Keycloak realm exports from review and add carve-outs for Keycloak admin credentials, RSA 2048, non-cryptographic hash CLI tools, HmacSHA1 (TOTP/OTP), and jq local-file interpolation. Introduces a .yamllint.yml config and a GitHub Actions workflow that lints all YAML files in strict mode.

Changes

CodeRabbit Prodsec Rule Refinements

Layer / File(s) Summary
Keycloak realm.json path exclusion
.coderabbit.yaml
Adds **/realm.json to reviews.path_filters so Keycloak realm export files are excluded from code review.
Hardcoded secrets, weak crypto, and injection vector carve-outs
.coderabbit.yaml
Extends no-hardcoded-secrets to permit Keycloak admin/admin in local scripts; extends no-weak-crypto with RSA 2048, md5sum/sha1sum (non-cryptographic), and HmacSHA1 (RFC 6238 TOTP); adds no-injection-vectors exception for jq string interpolation sourced from a prior jq extraction on the same local file.

YAML Linting CI Setup

Layer / File(s) Summary
yamllint config and CI workflow
.yamllint.yml, .github/workflows/yamllint.yml
Creates .yamllint.yml with extends: default and overrides for indentation, truthy values, comment spacing, brace/bracket spacing, trailing spaces, and empty lines. Adds a Lint YAML GitHub Actions workflow triggered on push/pull_request to main that installs and runs yamllint --strict ..

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🔑 No realm.json peeking through,
Admin/admin gets a pass—just for you.
HmacSHA1 hums its TOTP song,
jq plucks from itself—nothing wrong.
YAML lints strictly, the CI stands tall,
Config refined once and for all! 🎉


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (3 errors, 1 inconclusive)

Check name Status Explanation Resolution
No-Hardcoded-Secrets ❌ Error Exception for admin/admin credentials lacks path/host constraints; vague "maintenance scripts" wording can suppress real credential findings outside dev environments. Restrict exception to explicit dev paths (e.g., scripts/dev/, test/) and replace "maintenance scripts" with scope-specific language like "development-only token request scripts."
No-Weak-Crypto ❌ Error PR adds exceptions to no-weak-crypto check (md5sum/sha1sum, HmacSHA1) not in specification. Custom check requires flagging "MD5, SHA1" with only RSA 2048 exception. Limit no-weak-crypto exceptions to only "Do NOT flag RSA 2048" per specification, or update custom check instructions to document md5sum/HmacSHA1 carve-outs.
No-Injection-Vectors ❌ Error The no-injection-vectors check instructions match the spec, but the jq exception lacks trust-boundary constraints to prevent attacker-controlled file contents from causing injection vulnerabilities. Strengthen the jq exception to require source files be repository-controlled/static and explicitly restrict interpolation to data-only contexts, not filter logic.
Title check ❓ Inconclusive The title 'NO-ISSUE: Reduce false positives' is vague and generic, using non-descriptive language that doesn't clearly convey what was actually changed in the changeset. Provide a more specific title that describes the actual changes, such as 'Update linting and security checks configuration to reduce false positives' or 'Add yamllint workflow and refine coderabbit security rules'.
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Container-Privileges ✅ Passed PR contains only configuration files (.coderabbit.yaml, .github/workflows/yamllint.yml, .yamllint.yml) and no container or Kubernetes manifests with privileged settings.
No-Sensitive-Data-In-Logs ✅ Passed No logging statements in the PR expose passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data. The files contain configuration and check definitions, not logging code.
Ai-Attribution ✅ Passed AI tools (Claude) were used and properly attributed with "Assisted-by: Claude noreply@anthropic.com" trailer in commit 44a0316, following Red Hat attribution conventions.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch checks-false-positives

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 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 @.coderabbit.yaml:
- Around line 349-353: The EXCEPTION rule for admin/admin credentials in the
Keycloak section is too broad by permitting generic "maintenance scripts"
without constraints. Restrict the exception to explicitly reference local-dev
contexts only, adding path-based constraints (e.g., specific docker-compose or
local setup files) and host constraints (e.g., localhost/127.0.0.1) to prevent
the rule from suppressing real credential findings in production or non-local
scripts.
- Around line 374-376: The jq filter string interpolation exception rule in the
EXCEPTION block needs to strengthen its trust-boundary validation. Currently it
allows interpolation when the variable originates from a prior jq extraction on
the same local file, but this is insufficient since file contents can be
attacker-controlled. Update the exception to additionally verify that the source
file is repository-controlled or static (not dynamically loaded or
user-supplied), and enforce that interpolation remains strictly data-only
without allowing arbitrary filter expressions. This ensures the exception only
applies when both the extraction source and the interpolated value are from
trusted, non-malleable origins.
- Around line 361-364: The EXCEPTION text for weak-crypto detection is too
permissive and allows insecure SHA1/MD5 usage to bypass checks if labeled as
checksums or cache logic. Enhance the exception text in the weak-crypto
carve-out section to explicitly prohibit the use of these algorithms for
authentication, digital signatures, password hashing, token integrity
verification, and processing untrusted input, ensuring clear security boundaries
that prevent insecure patterns from being overlooked.
- Around line 29-31: The exclusion pattern `!**/realm.json` on line 31 is too
broad and excludes realm.json files from everywhere in the repository, which can
hide security-relevant configuration changes from code review. Replace this
global wildcard pattern with explicit path exclusions that target only the known
local and dev fixture directories where realm.json exports are typically stored
(for example, paths like dev fixtures or local export directories). This ensures
that only intentional non-reviewable files are excluded while allowing
security-relevant realm configuration changes in other locations to be reviewed.

In @.github/workflows/yamllint.yml:
- Around line 21-23: The Checkout repository step using actions/checkout does
not disable GitHub token credential persistence, which increases token exposure
to subsequent steps. Add the persist-credentials property set to false in the
step configuration to minimize GitHub token exposure and follow least privilege
security principles for workflow credentials.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: f9cb17d2-15d3-4d41-963c-aa93c5a06a7e

📥 Commits

Reviewing files that changed from the base of the PR and between 88d50a6 and 44a0316.

📒 Files selected for processing (3)
  • .coderabbit.yaml
  • .github/workflows/yamllint.yml
  • .yamllint.yml

Comment thread .coderabbit.yaml
Comment on lines +29 to +31
# Keycloak realm exports: config data with dev client secrets and
# RFC 6238 TOTP defaults, not reviewable code.
- "!**/realm.json"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Narrow the realm.json exclusion to explicit dev fixture paths.

Line 31 currently excludes every realm.json in the repo, which can hide security-relevant realm configuration changes from review. Scope this to known local/dev export directories instead of a global wildcard.

🤖 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 @.coderabbit.yaml around lines 29 - 31, The exclusion pattern
`!**/realm.json` on line 31 is too broad and excludes realm.json files from
everywhere in the repository, which can hide security-relevant configuration
changes from code review. Replace this global wildcard pattern with explicit
path exclusions that target only the known local and dev fixture directories
where realm.json exports are typically stored (for example, paths like dev
fixtures or local export directories). This ensures that only intentional
non-reviewable files are excluded while allowing security-relevant realm
configuration changes in other locations to be reviewed.

Comment thread .coderabbit.yaml
Comment on lines +349 to 353
EXCEPTION: Do NOT flag default admin/admin credentials in
developer setup or maintenance scripts (e.g., Keycloak token
requests with username=admin, password=admin). These are
standard defaults for local development environments.
mode: "error"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

The admin/admin exception is too broad for “maintenance scripts.”

This wording can suppress real credential findings in scripts that may run outside localhost workflows. Restrict the carve-out to explicit local-dev contexts (path + host constraints), not generic maintenance usage.

🤖 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 @.coderabbit.yaml around lines 349 - 353, The EXCEPTION rule for admin/admin
credentials in the Keycloak section is too broad by permitting generic
"maintenance scripts" without constraints. Restrict the exception to explicitly
reference local-dev contexts only, adding path-based constraints (e.g., specific
docker-compose or local setup files) and host constraints (e.g.,
localhost/127.0.0.1) to prevent the rule from suppressing real credential
findings in production or non-local scripts.

Comment thread .coderabbit.yaml
Comment on lines +361 to +364
EXCEPTION: Do NOT flag RSA 2048. Do NOT flag md5sum or
sha1sum when used for non-cryptographic purposes (file change
detection, checksums, cache keys). Do NOT flag HmacSHA1 for
TOTP/OTP configuration (RFC 6238 default algorithm).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Constrain weak-crypto carve-outs with explicit non-security boundaries.

The current exception text can let insecure SHA1/MD5 usage slip through if labeled as checksum/cache logic. Add explicit prohibition for auth, signatures, password hashing, token integrity, and untrusted-input paths.

🤖 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 @.coderabbit.yaml around lines 361 - 364, The EXCEPTION text for weak-crypto
detection is too permissive and allows insecure SHA1/MD5 usage to bypass checks
if labeled as checksums or cache logic. Enhance the exception text in the
weak-crypto carve-out section to explicitly prohibit the use of these algorithms
for authentication, digital signatures, password hashing, token integrity
verification, and processing untrusted input, ensuring clear security boundaries
that prevent insecure patterns from being overlooked.

Comment thread .coderabbit.yaml
Comment on lines +374 to +376
EXCEPTION: Do NOT flag jq filter string interpolation when
the variable originates from a prior jq extraction on the
same local file, not from user input.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

jq interpolation exception needs a stronger trust-boundary check.

“Prior jq extraction from the same local file” is insufficient when file contents can be attacker-controlled. Require the source file to be repository-controlled/static and keep interpolation strictly data-only.

🤖 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 @.coderabbit.yaml around lines 374 - 376, The jq filter string interpolation
exception rule in the EXCEPTION block needs to strengthen its trust-boundary
validation. Currently it allows interpolation when the variable originates from
a prior jq extraction on the same local file, but this is insufficient since
file contents can be attacker-controlled. Update the exception to additionally
verify that the source file is repository-controlled or static (not dynamically
loaded or user-supplied), and enforce that interpolation remains strictly
data-only without allowing arbitrary filter expressions. This ensures the
exception only applies when both the extraction source and the interpolated
value are from trusted, non-malleable origins.

Comment thread .github/workflows/yamllint.yml
Assisted-by: Claude <noreply@anthropic.com>
@sk-ilya
sk-ilya force-pushed the checks-false-positives branch from 44a0316 to 184d84f Compare June 22, 2026 12:52
@sk-ilya
sk-ilya merged commit ebd5a71 into main Jun 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant