no-jira: coderabbit: add supply chain and malware detection rules#10613
Open
rna-afk wants to merge 1 commit into
Open
no-jira: coderabbit: add supply chain and malware detection rules#10613rna-afk wants to merge 1 commit into
rna-afk wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates ChangesCodeRabbit Security Policy Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 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. Comment |
rna-afk
force-pushed
the
coderabbit_security_changes
branch
from
e5010de to
4decc4b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
98-114: ⚡ Quick winMake the path-filter protection enforceable, not comment-only.
The security note is useful, but it can be bypassed by a future edit. Add a pre-merge custom check so protected paths cannot be excluded.
Suggested policy hardening
reviews: + pre_merge_checks: + custom_checks: + - mode: error + name: "Disallow exclusions for security-sensitive paths" + instructions: | + Fail if `.coderabbit.yaml` adds `path_filters` exclusions for: + `.coderabbit.yaml`, `.github/**`, `.pre-commit-config.yaml`, + `**/.claude/**`, `**/.cursor/**`, `**/.gemini/**`, `**/.windsurf/**`, + `**/.vscode/**`, `**/.devcontainer/**`, `**/.gitattributes`, + `**/.npmrc`, `**/.yarnrc`, `**/.yarnrc.yml`, `**/.pypirc`.🤖 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 98 - 114, Add an enforceable validation that prevents protected paths from being added to path_filters exclusions by implementing a pre-merge check that inspects changes to .coderabbit.yaml (the path_instructions key) and fails the merge/CI job if any of the protected entries (".claude/", ".cursor/", ".gemini/", ".windsurf/", ".vscode/", ".devcontainer/", ".github/", ".rules/", ".coderabbit.yaml", ".pre-commit-config.yaml", ".gitattributes", ".npmrc", ".yarnrc", "renovate.json") are present in an exclusion list; update the repo's CI/pre-merge hook or CodeRabbit validation to run this check and return a non-zero exit status when violations are detected so the protection is enforceable rather than comment-only.
🤖 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 67-68: The config enables summaries via the high_level_summary and
high_level_summary_placeholder keys but conflicts with the project's “no
summaries” policy; update the configuration by disabling or removing
high_level_summary (set high_level_summary to false or delete the key) and
remove the high_level_summary_placeholder entry so no summary generation is
requested; ensure changes touch the high_level_summary and
high_level_summary_placeholder keys only.
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 98-114: Add an enforceable validation that prevents protected
paths from being added to path_filters exclusions by implementing a pre-merge
check that inspects changes to .coderabbit.yaml (the path_instructions key) and
fails the merge/CI job if any of the protected entries (".claude/", ".cursor/",
".gemini/", ".windsurf/", ".vscode/", ".devcontainer/", ".github/", ".rules/",
".coderabbit.yaml", ".pre-commit-config.yaml", ".gitattributes", ".npmrc",
".yarnrc", "renovate.json") are present in an exclusion list; update the repo's
CI/pre-merge hook or CodeRabbit validation to run this check and return a
non-zero exit status when violations are detected so the protection is
enforceable rather than comment-only.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7a860d80-c988-4b4f-b8d5-9e82e3d2fd5c
📒 Files selected for processing (1)
.coderabbit.yaml
rna-afk
force-pushed
the
coderabbit_security_changes
branch
2 times, most recently
from
6f3415b to
636d6a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
607-766: 💤 Low valueGood coverage; consider consolidating duplicate blocks.
The instructions are thorough. Note: requirements*.txt/constraints*.txt (lines 649-691) and pyproject.toml/setup.py (lines 693-747) contain identical content. Lower priority than the IDE config duplication, but YAML anchors would reduce maintenance burden here too.
🤖 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 607 - 766, The YAML contains duplicated instruction blocks for requirements*.txt and constraints*.txt and for pyproject.toml and setup.py; to fix, consolidate the repeated content by creating a single canonical instruction block (e.g., a YAML anchor) and reference it from the duplicate paths (use an anchor like &supply_chain_rules and alias *supply_chain_rules), or merge the duplicate path entries into one shared instruction entry; update the entries for "requirements*.txt", "constraints*.txt", "pyproject.toml", and "setup.py" to reference the consolidated instructions so maintenance is centralized and duplicates are removed.
🤖 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 220-224: Replace the ML/AI-specific Makefile checks under the
"**/Makefile" rule with installer-relevant validations: remove "training jobs"
and "storage backends" wording and instead enforce (1) no hardcoded credentials
or registry URLs in Makefiles, (2) verification that any downloaded tools (e.g.,
controller-gen, golangci-lint) are pinned to explicit versions, and (3)
detection/flagging of unsafe "curl | bash" (or similar pipe-to-shell) patterns
in build scripts; update the instruction text in the existing rule block so it
references these installer-focused checks and retains the CWE/security
rationale.
- Around line 278-510: The YAML repeats the same 38-line instruction block for
each path (the entries currently listed under the keys ".claude/**",
".cursor/**", ".gemini/**", ".windsurf/**", ".vscode/**", ".devcontainer/**");
refactor by extracting the repeated instruction text into a single YAML anchor
(e.g. &agent_injection) and replace each duplicated block with an alias
reference (e.g. *agent_injection) so the content is defined once and reused for
each path, reducing duplication and preventing drift while preserving the
existing keys and semantics.
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 607-766: The YAML contains duplicated instruction blocks for
requirements*.txt and constraints*.txt and for pyproject.toml and setup.py; to
fix, consolidate the repeated content by creating a single canonical instruction
block (e.g., a YAML anchor) and reference it from the duplicate paths (use an
anchor like &supply_chain_rules and alias *supply_chain_rules), or merge the
duplicate path entries into one shared instruction entry; update the entries for
"requirements*.txt", "constraints*.txt", "pyproject.toml", and "setup.py" to
reference the consolidated instructions so maintenance is centralized and
duplicates are removed.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7c722273-b417-4852-8801-d127d03979a5
📒 Files selected for processing (1)
.coderabbit.yaml
rna-afk
force-pushed
the
coderabbit_security_changes
branch
from
636d6a7 to
24d3f30
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.coderabbit.yaml (2)
355-424: 💤 Low valueConsider YAML anchors for
dist/**andout/**instructions.These two blocks (lines 355-375 and 404-424) are identical. The
build/**block differs (has APPLICABILITY clause), so it correctly stays separate.♻️ Optional refactor using anchors
- path: "**/dist/**" - instructions: | + instructions: &prebuilt-artifact-instructions | SUPPLY CHAIN: PRE-BUILT ARTIFACT INJECTION (CWE-829) ... - path: "**/out/**" - instructions: | - SUPPLY CHAIN: PRE-BUILT ARTIFACT INJECTION (CWE-829) - ... + instructions: *prebuilt-artifact-instructions🤖 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 355 - 424, The two identical rule blocks for the path patterns "**/dist/**" and "**/out/**" should be deduplicated by extracting their shared multi-line instructions into a YAML anchor (e.g., &prebuilt_instructions) and replacing each block body with the corresponding alias (*prebuilt_instructions); keep the distinct "**/build/**" block unchanged since it has the APPLICABILITY clause. Update the "**/dist/**" and "**/out/**" entries to reference the anchor so future edits only need to change one place.
468-586: 💤 Low valueAdditional duplication candidates for YAML anchors.
requirements*.txt(468-489) andconstraints*.txt(490-510) are identicalpyproject.toml(512-538) andsetup.py(540-566) are identicalSame anchor pattern used for IDE configs would work here. Lower priority since these are 2 copies each, not 6.
🤖 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 468 - 586, The two identical rule blocks for path "**/requirements*.txt" and "**/constraints*.txt" and the two identical blocks for path "**/pyproject.toml" and "**/setup.py" should be deduplicated using YAML anchors: extract the shared instructions into a named anchor (e.g. &python_requirements and &python_build_config) and replace the duplicate blocks with aliases (*python_requirements, *python_build_config) so the rules remain identical but defined once; update the duplicated entries in the .coderabbit.yaml to reference those anchors and remove the repeated instruction text in the aliased blocks.
🤖 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.
Nitpick comments:
In @.coderabbit.yaml:
- Around line 355-424: The two identical rule blocks for the path patterns
"**/dist/**" and "**/out/**" should be deduplicated by extracting their shared
multi-line instructions into a YAML anchor (e.g., &prebuilt_instructions) and
replacing each block body with the corresponding alias (*prebuilt_instructions);
keep the distinct "**/build/**" block unchanged since it has the APPLICABILITY
clause. Update the "**/dist/**" and "**/out/**" entries to reference the anchor
so future edits only need to change one place.
- Around line 468-586: The two identical rule blocks for path
"**/requirements*.txt" and "**/constraints*.txt" and the two identical blocks
for path "**/pyproject.toml" and "**/setup.py" should be deduplicated using YAML
anchors: extract the shared instructions into a named anchor (e.g.
&python_requirements and &python_build_config) and replace the duplicate blocks
with aliases (*python_requirements, *python_build_config) so the rules remain
identical but defined once; update the duplicated entries in the
.coderabbit.yaml to reference those anchors and remove the repeated instruction
text in the aliased blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 675b3bdb-17e1-4ff4-8ecc-581eb554081b
📒 Files selected for processing (1)
.coderabbit.yaml
Member
|
/retest-required |
Member
tthvo
reviewed
Jun 9, 2026
|
|
||
| reviews: | ||
| profile: chill | ||
| instructions: |
Member
There was a problem hiding this comment.
Do you think these instructions/path-instructions should be enforced organization-wide in https://github.com/openshift/coderabbit?
Our coderabbit config inherits the fields from there.
Contributor
Author
There was a problem hiding this comment.
yep my question exactly. I'll keep looking for a PR in that repo or elsewhere. There's also a mention of 4 options that only come with something else called skillsaw which I have no idea of.
Member
There was a problem hiding this comment.
Looks like the openshift-org repo defines some security instructions like .coderabbit.yaml#L532-L537. We probably need to double-check and just extend instead of overriding them 👀
Member
|
/approve |
rna-afk
changed the title
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@rna-afk: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
needs-rebase
Member
|
@rna-afk there is a merge conflict, can you rebase? |
Contributor
Author
|
@coderabbitai configuration |
|
Configuration used: Repository YAML (base), Central YAML (inherited) CodeRabbit configuration# Source: Repository YAML (base)
language: en-US
# Source: Repository YAML (base)
tone_instructions: Be concise and direct. Focus on correctness, security, and backward compatibility. This is infrastructure software that provisions cloud resources - mistakes can be costly and hard to reverse.
# Source: Central YAML (inherited)
early_access: false
# Source: Central YAML (inherited)
enable_free_tier: false
# Source: Repository YAML (base)
inheritance: true
reviews:
# Source: Repository YAML (base)
profile: chill
# Source: Repository YAML (base)
request_changes_workflow: false
# Source: Repository YAML (base)
high_level_summary: true
# Source: defaults
high_level_summary_instructions: ''
# Source: Repository YAML (base)
high_level_summary_placeholder: '`@coderabbitai` summary'
# Source: defaults
high_level_summary_in_walkthrough: false
# Source: defaults
auto_title_placeholder: '`@coderabbitai`'
# Source: defaults
auto_title_instructions: ''
# Source: Repository YAML (base)
review_status: true
# Source: defaults
review_details: false
# Source: Repository YAML (base)
commit_status: true
# Source: defaults
fail_commit_status: false
# Source: Repository YAML (base)
collapse_walkthrough: true
# Source: Repository YAML (base)
changed_files_summary: true
# Source: Repository YAML (base)
sequence_diagrams: false
# Source: Repository YAML (base)
estimate_code_review_effort: true
# Source: defaults
assess_linked_issues: true
# Source: defaults
related_issues: true
# Source: defaults
related_prs: true
# Source: defaults
suggested_labels: true
# Source: defaults
labeling_instructions: []
# Source: defaults
mutually_exclusive_groups: {}
# Source: defaults
auto_apply_labels: false
# Source: Repository YAML (base)
suggested_reviewers: false
# Source: defaults
auto_assign_reviewers: false
# Source: defaults
suggested_reviewers_instructions: []
# Source: Central YAML (inherited)
in_progress_fortune: false
# Source: Repository YAML (base)
poem: false
# Source: defaults
enable_prompt_for_ai_agents: true
# Source: Repository YAML (base), Central YAML (inherited)
path_filters:
- '!vendor/**'
- '!**/vendor/**'
- '!**/zz_generated*'
- '!data/data/install.openshift.io_installconfigs.yaml'
- '!boilerplate/**'
- '!node_modules/**'
- '!dist/**'
- '!*.min.js'
- '!yarn.lock'
# Source: Repository YAML (base), Central YAML (inherited)
path_instructions:
- path: pkg/types/**/validation/**
instructions: This is install-config validation code. Ensure validation errors are clear and actionable for the end user. Check that new validations do not break existing valid configs (backward compatibility). Verify error field paths match the YAML/JSON structure users provide.
- path: pkg/types/**/defaults/**
instructions: This is defaulting logic for install-config fields. Ensure defaults are only applied when the field is zero-valued. Verify platform- specific defaults don't conflict with cross-platform defaults.
- path: pkg/asset/**
instructions: This code follows the asset dependency-graph architecture. Verify that Dependencies() returns all required assets and that Generate() properly consumes them. Check that file names and paths are correct for the target output.
- path: pkg/destroy/**
instructions: This is cluster teardown code. Be extra careful - bugs here can leave orphaned cloud resources that cost money. Verify proper error handling and that all resources created during install are cleaned up.
- path: pkg/infrastructure/**
instructions: Infrastructure provisioning code. Verify cloud API calls handle errors and retries properly. Check for resource leaks on failure paths. Ensure tags/labels are applied consistently.
- path: cluster-api/providers/**
instructions: Cluster API infrastructure providers. Ensure changes are compatible with the CAPI contract and that controller reconciliation is idempotent.
- path: data/data/**
instructions: Static data files embedded in the installer binary. For bootimage metadata (rhcos.json), verify URLs and SHA256 digests. For manifests, ensure YAML is valid and API versions are correct.
- path: hack/**
instructions: Build and CI scripts. Ensure scripts are POSIX-compatible where possible, use set -e, and handle errors. Check for hardcoded paths that may not work in all environments.
- path: upi/**
instructions: User-Provisioned Infrastructure templates (ARM, CloudFormation, etc.). These are used directly by customers. Ensure backward compatibility and that parameter defaults are sensible.
- path: '**/*_test.go'
instructions: Test files. Verify edge cases are covered, especially for validation and defaulting logic. Check that test names follow Go conventions and are descriptive. Ensure mocks are used appropriately.
- path: images/**
instructions: Container image definitions. Check for pinned base image versions and unnecessary layers. Verify multi-stage builds minimize final image size.
- path: docs/**
instructions: User-facing documentation. Ensure accuracy, check for broken links, and verify command examples actually work. Flag outdated platform references.
- path: '**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}'
instructions: |
Injection prevention (prodsec-skills):
- SQL: parameterized queries only; no string concatenation
- Command: no shell=True, os.system, or backtick exec with user input
- LDAP/XPath: escape special characters in filters
- Path traversal: canonicalize paths, reject ../
- Deserialization: no pickle/yaml.load()/eval on untrusted data
- Prototype pollution: no recursive merge of untrusted objects
- Validate at trust boundaries with allow-lists, not deny-lists
- Normalize Unicode and anchor regexes (^$); watch for ReDoS
- path: '**/*.{html,jsx,tsx,vue,svelte}'
instructions: |
Web security (prodsec-skills):
- No dangerouslySetInnerHTML or v-html with user data
- CSP: no unsafe-inline, no unsafe-eval
- CSRF tokens on state-changing requests
- Cookies: Secure, HttpOnly, SameSite=Strict
- No document.write, eval, new Function with user input
- GraphQL: depth/complexity limits, disable introspection in prod
- File uploads: validate by content magic, cap size, server-generate names
- XML: disable external entities (XXE), reject DTDs from untrusted sources
- path: '**/*{crypt,cipher,sign,hash,tls,ssl,cert,key,token}*'
instructions: |
Cryptographic security (prodsec-skills):
- Banned: MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode
- Symmetric: AES-256-GCM or ChaCha20-Poly1305
- Passwords: Argon2id (not bcrypt/scrypt for new code)
- Signing: Ed25519 or ECDSA P-256+
- Key exchange: X25519 or ECDH P-256+
- Constant-time comparison for all secret/token data
- Zeroize key material after use (no garbage-collector reliance)
- No custom crypto; use vetted libraries only
- Post-quantum: flag if protecting long-lived secrets
- path: '**/{Dockerfile,Containerfile}*'
instructions: |
Container security (prodsec-skills):
- Base image: UBI minimal or distroless from catalog.redhat.com
- Red Hat images: use floating tags (Red Hat manages updates);
non-RH images: pin by digest
- Multi-stage builds; no build tools in final image
- USER non-root; never run as root
- COPY specific files, not entire context
- No secrets in ENV, ARG, or COPY
- Read-only rootfs where possible
- No package manager cache in final layer
- HEALTHCHECK defined
- path: '**/*.{yaml,yml}'
instructions: |
If this is a Kubernetes/OpenShift manifest or Helm template:
- securityContext: runAsNonRoot, readOnlyRootFilesystem,
allowPrivilegeEscalation: false
- Drop ALL capabilities, add only what is required
- Resource limits (cpu, memory) on every container
- No hostPID, hostNetwork, hostIPC, privileged: true
- NetworkPolicy defined for the namespace
- OpenShift: SCC must be restricted or custom-scoped
- Liveness + readiness probes defined
- automountServiceAccountToken: false unless needed
- RBAC: least privilege; no cluster-admin for workloads
- Helm: no .Values interpolation in shell commands
- path: '**/{requirements*.txt,Pipfile*,pyproject.toml,package*.json,go.mod,go.sum,Cargo.toml,Gemfile*,pom.xml,build.gradle*}'
instructions: |
Supply chain security (prodsec-skills):
- New deps: justify need, check license compatibility
- Pin exact versions; verify hashes where supported
- Flag known CVEs (cross-ref osv.dev)
- No pre-release or yanked versions in production
- SBOM: ensure build produces provenance attestations
- Signing: artifacts signed with Sigstore/cosign
- path: .github/workflows/**/*
instructions: |
CI/CD security (prodsec-skills):
- Pin actions by full SHA, not tag
- No secrets in logs; mask sensitive outputs
- Least privilege: minimize GITHUB_TOKEN permissions
- No pull_request_target with checkout of PR head
- SAST/SCA steps in pipeline
- Sign artifacts with Sigstore/cosign
- Agentic CI actions: audit for prompt injection via
issue/PR title/body flowing into LLM prompts
- path: '**/{.claude,.vscode}/**/*'
instructions: |
HIGH RISK — IDE and AI tool configuration (prodsec-skills):
These directories can be supply chain attack vectors.
Review every change with a security and malware lens.
Flag the following for close scrutiny:
- .claude: hook commands, MCP server definitions, or
permissive tool-use policies in settings.json
- .claude: any scripts or binaries (e.g., .mjs, .sh)
- .vscode: tasks.json or launch.json entries that execute
code on folder open, build, or save
- .vscode: settings.json entries that auto-run formatters,
linters, or extensions with broad permissions
- Any change that grants broad filesystem or network access
- Obfuscated content or suspiciously large files
- Changes without a clear, legitimate purpose
- path: '**/*.go'
instructions: |
Go security (prodsec-skills):
- Never ignore error returns
- database/sql with placeholders; no fmt.Sprintf in queries
- Use stdlib crypto/* and golang.org/x/crypto (Go team maintained);
avoid third-party crypto libraries
- Integer overflow: bounds-check user-supplied sizes
- context.Context for cancellation and timeouts
- path: '**/*.{c,cpp,cc,h,hpp}'
instructions: |
C/C++ security (prodsec-skills):
- Banned: gets, sprintf, strcpy, strcat, strtok
- Use strlcpy, snprintf, bounded APIs
- Compile: -fstack-protector-strong -fPIE -pie
-D_FORTIFY_SOURCE=2 -Wformat-security
- Nullify pointers after free; no use-after-free
- Integer overflow: check arithmetic on untrusted sizes
- path: '**/{db,database,redis,cache,storage}/**/*'
instructions: |
Data store security (prodsec-skills):
- Auth: no default credentials; use IAM or IdP tokens
- Encryption: TLS in transit, encryption at rest
- Least privilege: app user has minimal grants
- Redis/ElastiCache: AUTH required, no KEYS in prod,
rename dangerous commands (FLUSHALL, CONFIG)
- Connection strings: no embedded credentials
# Source: defaults
abort_on_close: true
# Source: defaults
disable_cache: false
slop_detection:
# Source: defaults
enabled: true
auto_review:
# Source: Repository YAML (base)
enabled: true
# Source: defaults
description_keyword: ''
# Source: Repository YAML (base)
auto_incremental_review: true
# Source: defaults
auto_pause_after_reviewed_commits: 5
# Source: Repository YAML (base)
ignore_title_keywords:
- WIP
- DO NOT MERGE
- DNM
# Source: defaults
labels: []
# Source: Repository YAML (base)
drafts: false
# Source: Repository YAML (base)
base_branches:
- main
- release-4.*
- release-5.*
# Source: defaults
ignore_usernames: []
finishing_touches:
docstrings:
# Source: defaults
enabled: true
unit_tests:
# Source: defaults
enabled: true
simplify:
# Source: defaults
enabled: false
# Source: defaults
custom: []
pre_merge_checks:
# Source: defaults
override_requested_reviewers_only: false
docstrings:
# Source: defaults
mode: warning
# Source: defaults
threshold: 80
title:
# Source: defaults
mode: warning
# Source: defaults
requirements: ''
description:
# Source: defaults
mode: warning
issue_assessment:
# Source: defaults
mode: warning
# Source: Central YAML (inherited)
custom_checks:
- mode: error
name: Stable and Deterministic Test Names
instructions: |
Ginkgo test names MUST be stable and deterministic. They must never contain dynamic
information that changes between runs.
Flag any test title (It(), Describe(), Context(), When(), etc.) that includes:
- Pod names with generated suffixes (e.g., "test-pod-abc123")
- Timestamps or dates
- Random UUIDs or generated identifiers
- Node names
- Namespace names with random suffixes
- IP addresses
- Any value that could change between test runs
Additionally, flag overly-specific test titles likely to change in
later development iterations, to avoid the need to specify mappings
after changing the name.
Test names should use descriptive, static strings that clearly indicate what
the test validates.
❌ Bad examples:
- `It("should create pod test-pod-xyz123 with custom security context")`
- `It(fmt.Sprintf("should run on node %s", nodeName))`
- `It("should create namespace " + ns.Name)`
- `It("should complete initialization within 30s")`
✅ Good examples:
- `It("should create a pod with custom security context")`
- `It("should schedule workloads to labeled nodes")`
- `It("should enforce network policy between namespaces")`
- `It("should complete initialization quickly")`
Dynamic values belong in test BODIES (assertions, setup), never in test TITLES.
- mode: warning
name: Test Structure and Quality
instructions: |
Review Ginkgo test code for these quality requirements:
1. **Single responsibility**: Each test (It block) should test one specific behavior.
Flag tests that assert multiple unrelated behaviors.
2. **Setup and cleanup**: Tests should use BeforeEach/AfterEach for setup and cleanup.
Flag tests that create resources without cleanup, especially cluster-scoped resources.
3. **Timeouts**: Operations that interact with the cluster (pod creation, deployments,
waiting for conditions) must include appropriate timeouts. Flag indefinite waits
or missing timeouts on Eventually/Consistently calls.
4. **Assertion messages**: Assertions should include meaningful failure messages
that help diagnose what went wrong.
❌ `Expect(err).NotTo(HaveOccurred())`
✅ `Expect(err).NotTo(HaveOccurred(), "failed to create test pod")`
5. **Consistency with codebase**: Tests should follow existing patterns in the
repository for how fixtures are created, how clients are obtained, and how
waits are structured.
- mode: warning
name: MicroShift Test Compatibility
instructions: |
When new Ginkgo e2e tests are added (It(), Describe(), Context(), When(), etc.),
check whether they use any APIs or features that are NOT available on MicroShift.
MicroShift is a single-node, minimal OpenShift distribution and does not support
all standard OpenShift APIs and features.
Note: The only OpenShift kube APIs available on MicroShift are Route and
SecurityContextConstraints. All other OpenShift-specific APIs are unavailable.
IMPORTANT: Do NOT flag a test if it is already protected from running on
MicroShift by any of these mechanisms:
- The test name includes a `[Skipped:MicroShift]` label
- The test name includes an `[apigroup:...]` tag for an API group not available
on MicroShift (e.g., `[apigroup:config.openshift.io]`,
`[apigroup:machine.openshift.io]`). The MicroShift CI jobs automatically skip
tests whose apigroup tag references an API group not served by MicroShift.
- The test body contains an `exutil.IsMicroShiftCluster()` check with `g.Skip()`
- The test is wrapped in a Describe/Context that already has one of the above
Flag the test if it references ANY of the following unavailable APIs or resources:
- Project / ProjectRequest (project.openshift.io) — use plain Namespaces instead
- Build / BuildConfig (build.openshift.io)
- DeploymentConfig (apps.openshift.io) — use Deployments instead
- ClusterOperator / ClusterOperators (config.openshift.io/v1)
- ClusterVersion / ClusterVersions (config.openshift.io/v1)
- Etcd operator or etcd pods (etcd.operator.openshift.io, openshift-etcd namespace)
- ClusterServiceVersion (CSV) / OLM resources (operators.coreos.com)
- MachineSet / Machine / MachineHealthCheck (machine.openshift.io)
- ClusterAutoscaler / MachineAutoscaler
- Console (console.openshift.io, openshift-console namespace)
- Monitoring stack components (prometheus-k8s, alertmanager, thanos-querier in openshift-monitoring)
- ImageRegistry operator (imageregistry.operator.openshift.io, openshift-image-registry namespace)
- Samples operator (samples.operator.openshift.io, openshift-cluster-samples-operator namespace)
- OperatorHub / CatalogSource / PackageManifest (operators.coreos.com, marketplace.redhat.com)
- CloudCredential / CredentialsRequest (cloudcredential.openshift.io)
- Storage operator (operator.openshift.io/v1 storage, openshift-cluster-storage-operator namespace)
- Network operator CRDs (operator.openshift.io/v1 network, openshift-network-operator namespace)
- Any other OpenShift API group besides Route (route.openshift.io) and
SecurityContextConstraints (security.openshift.io)
Flag the test if it references ANY of the following namespaces that do not exist on MicroShift:
- openshift-kube-apiserver
- openshift-kube-controller-manager
- openshift-kube-scheduler
Flag the test if it makes ANY of the following unsupported assumptions:
- Multi-node or HA assumptions (e.g., expecting multiple master/control-plane nodes,
pod anti-affinity across nodes, leader election across replicas)
- FeatureGate resources or TechPreviewNoUpgrade / CustomNoUpgrade feature sets
- Upgrade or update workflows (ClusterVersion-based upgrades)
- Node scaling (expecting nodes to be added or removed)
- Multi-replica deployments of control-plane components
If a test is flagged, recommend the following:
> **MicroShift compatibility notice:** This test uses APIs or features that are
> not available on MicroShift. If this repository's presubmit CI does not already
> include MicroShift jobs, please verify your test works on MicroShift by running
> an additional CI job:
>
> For parallel tests:
> `/payload-job periodic-ci-openshift-microshift-release-4.22-periodics-e2e-aws-ovn-ocp-conformance`
>
> For serial tests (test name contains `[Serial]`):
> `/payload-job periodic-ci-openshift-microshift-release-4.22-periodics-e2e-aws-ovn-ocp-conformance-serial`
>
> If the test is intentionally not applicable to MicroShift, there are
> several options:
>
> **Option 1 (preferred for API-specific tests):** Add an `[apigroup:...]` tag
> to the test name for the OpenShift API group being used. MicroShift CI jobs
> automatically skip tests whose apigroup is not served by MicroShift:
>
> ```go
> g.It("should report cluster operator status [apigroup:config.openshift.io]", func() { ... })
> ```
>
> **Option 2:** Add a `[Skipped:MicroShift]` label to the test name:
>
> ```go
> g.It("should do something [Skipped:MicroShift]", func() { ... })
> ```
>
> **Option 3:** Guard the test with a runtime platform check. In the
> `openshift/origin` repository, the common pattern is:
>
> ```go
> isMicroShift, err := exutil.IsMicroShiftCluster(oc.AdminKubeClient())
> o.Expect(err).NotTo(o.HaveOccurred())
> if isMicroShift {
> g.Skip("Not supported on MicroShift")
> }
> ```
- mode: warning
name: Single Node OpenShift (SNO) Test Compatibility
instructions: |
When new Ginkgo e2e tests are added (It(), Describe(), Context(), When(), etc.),
check whether they make assumptions about multi-node or HA clusters. Single Node
OpenShift (SNO) runs the full OpenShift control plane and worker components on a
single node. Unlike MicroShift, SNO includes all standard OpenShift operators and
APIs, but any test that assumes multiple nodes will fail.
IMPORTANT: Do NOT flag a test if it is already protected from running on
SNO by any of these mechanisms:
- The test name includes a `[Skipped:SingleReplicaTopology]` label
- The test body contains an `exutil.IsSingleNode()` check with `g.Skip()`
- The test body contains a `skipOnSingleNodeTopology()` call
- The test body checks `infrastructure.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode`
and skips accordingly
- The test is wrapped in a Describe/Context that already has one of the above
Flag the test if it makes ANY of the following multi-node or HA assumptions:
- Expects multiple control-plane/master nodes (e.g., counting master nodes > 1)
- Expects multiple worker nodes or schedules pods across distinct nodes
- Uses pod anti-affinity or topology spread constraints requiring multiple nodes
- Tests node-to-node communication patterns that require separate hosts
- Assumes leader election failover across multiple replicas on different nodes
- Expects pod rescheduling to a different node after node drain or failure
- Tests node scaling operations (adding or removing nodes)
- Assumes separate infra/worker/master node roles on different hosts
- Validates rolling updates that require scheduling to other nodes
- Tests ingress or load balancing behavior that depends on multiple endpoints
on different nodes
Do NOT flag tests that:
- Use OpenShift APIs and operators (these are all available on SNO)
- Run multiple pods on the same node
- Test single-pod behavior, even with multiple replicas (replicas can coexist on one node)
If a test is flagged, recommend the following:
> **Single Node OpenShift (SNO) compatibility notice:** This test assumes a
> multi-node cluster and may fail on Single Node OpenShift deployments. Please
> verify your test works on SNO by running an additional CI job:
>
> For parallel tests:
> `/payload-job periodic-ci-openshift-release-master-ci-4.22-e2e-aws-upgrade-ovn-single-node`
>
> For serial tests (test name contains `[Serial]`):
> `/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-aws-ovn-single-node-serial`
>
> If the test is intentionally not applicable to SNO, there are several
> options:
>
> **Option 1:** Add a `[Skipped:SingleReplicaTopology]` label to the test name.
> SNO CI jobs automatically skip tests with this label:
>
> ```go
> g.It("should schedule pods across nodes [Skipped:SingleReplicaTopology]", func() { ... })
> ```
>
> **Option 2:** Guard the test with a runtime topology check. In the
> `openshift/origin` repo, use the `exutil.IsSingleNode()` utility:
>
> ```go
> isSingleNode, err := exutil.IsSingleNode(context.Background(), oc.AdminConfigClient())
> o.Expect(err).NotTo(o.HaveOccurred())
> if isSingleNode {
> g.Skip("Test requires multiple nodes and does not apply to single-node topologies")
> }
> ```
>
> This checks `infrastructure.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode`
> which is the canonical way to detect SNO clusters.
>
> **Option 3:** Some test packages define a local `skipOnSingleNodeTopology()` helper:
>
> ```go
> func skipOnSingleNodeTopology(oc *exutil.CLI) {
> infra, err := oc.AdminConfigClient().ConfigV1().Infrastructures().Get(
> context.Background(), "cluster", metav1.GetOptions{})
> o.Expect(err).NotTo(o.HaveOccurred())
> if infra.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode {
> e2eskipper.Skipf("This test does not apply to single-node topologies")
> }
> }
> ```
>
> You can also use the `single_node.GetTopologies()` helper from
> `test/extended/single_node/topology.go` to get both control plane and
> infrastructure topology modes.
- mode: warning
name: Topology-Aware Scheduling Compatibility
instructions: |
When deployment manifests, operator code, or controllers are added or modified,
check whether they introduce scheduling constraints that assume a standard
highly-available (HA) topology with 3+ control-plane nodes and dedicated worker
nodes. OpenShift supports several topologies where these assumptions break:
- **Single Node OpenShift (SNO):** One node runs everything.
`ControlPlaneTopology = SingleReplica`, 1 schedulable node.
- **Two-Node Fixed (TNF):** Two control-plane nodes, no dedicated workers.
`ControlPlaneTopology = DualReplica`, 2 schedulable nodes.
Operators should run 2 replicas (not 1 as on SNO) to maintain HA.
- **Two-Node with Arbiter (TNA):** Two control-plane nodes plus a
resource-constrained arbiter node. The arbiter is NOT a master or worker
— it is a distinct role with label `node-role.kubernetes.io/arbiter` and
taint `node-role.kubernetes.io/arbiter:NoSchedule`. It only runs etcd
for quorum plus essential infrastructure (kubelet, SDN/OVN, MCD), and
may be as small as 2 cores / 8 GiB RAM. `ControlPlaneTopology =
HighlyAvailableArbiter`, but only 2 fully schedulable nodes.
- **External Control Plane (HyperShift):** Control plane runs outside the
hosted cluster. `ControlPlaneTopology = External`. There are NO nodes with
`node-role.kubernetes.io/master` or `node-role.kubernetes.io/control-plane`
labels in the hosted cluster.
The `ControlPlaneTopology` values are defined in `openshift/api` as the
`TopologyMode` type. Availability of each value depends on the release
version and enabled feature gates — check `openshift/api/features/features.go`
and the `FeatureGateAwareEnum` annotations on the `ControlPlaneTopology`
field for the target release:
- `HighlyAvailable` (default)
- `SingleReplica`
- `DualReplica` (feature gate: `DualReplica`)
- `HighlyAvailableArbiter`
- `External`
Do NOT flag changes that already check `ControlPlaneTopology`, node counts,
or topology labels before applying scheduling constraints.
Flag changes that introduce ANY of the following without topology-awareness:
- **Required pod anti-affinity with `maxUnavailable: 0`.**
(`requiredDuringSchedulingIgnoredDuringExecution` with
`topologyKey: kubernetes.io/hostname` combined with
`maxUnavailable: 0` in the rolling update strategy).
This combination deadlocks on ANY topology where replicas == schedulable
nodes (SNO, TNF, TNA, and even HA with 3 replicas on 3 nodes): the surge
pod cannot schedule (anti-affinity blocks it) and no old pod can be deleted
(maxUnavailable: 0 prevents it). Required anti-affinity is SAFE when paired
with `maxUnavailable >= 1` — this is the pattern used by most HA operators
(oauth-openshift, openshift-apiserver, image-registry, monitoring).
Preferred anti-affinity is NOT a safe alternative — it allows pods to
co-locate on the same node, defeating HA (see OCPBUGS-65984).
- **Pod topology spread constraints** with `whenUnsatisfiable: DoNotSchedule`
and a hostname topology key. Breaks on SNO and is problematic when the spread
`maxSkew` exceeds the number of schedulable nodes (TNF, TNA).
- **Replica count derived from node count** (e.g., counting control-plane nodes
and setting replicas to match) without considering that SNO and TNF have
fewer than 3 control-plane nodes, and TNA's arbiter node should not run
general workloads.
- **nodeSelector or node affinity targeting control-plane nodes**
(`node-role.kubernetes.io/master` or `node-role.kubernetes.io/control-plane`).
On HyperShift, no nodes carry these labels — pods will remain Pending
indefinitely.
- **Scheduling workloads to all control-plane nodes equally** without excluding
arbiter nodes. On TNA, the arbiter node has a separate taint
(`node-role.kubernetes.io/arbiter:NoSchedule`) that blocks general
workloads. However, operators that use **broad or wildcard tolerations**
(e.g., tolerating all `NoSchedule` taints) will inadvertently schedule to
the resource-constrained arbiter. Only etcd and essential infrastructure
pods (kubelet, SDN/OVN, machine-config-daemon) should run on the arbiter.
- **Assuming dedicated worker nodes exist.** On SNO and TNF, all workloads
run on control-plane nodes. Code that targets only worker nodes via
`node-role.kubernetes.io/worker` nodeSelector may need to also consider
nodes that carry both control-plane and worker roles.
- **PodDisruptionBudgets designed for 3+ nodes.** On TNF and TNA, only 2
nodes are fully schedulable. PDBs with `minAvailable: 2` on a 2-replica
deployment will block all voluntary disruptions (drains, upgrades).
PDBs should be reviewed for topology-appropriate values. Note: PDBs only
protect against the eviction API (`kubectl drain`). They do NOT protect
against `TaintManagerEviction` (node fencing, unreachable taints), which
directly deletes pods regardless of PDB settings.
If a change is flagged, recommend the following:
> **Topology-aware scheduling notice:** This change introduces scheduling
> constraints that may not work on all supported OpenShift topologies
> (SNO, Two-Node, HyperShift).
>
> OpenShift clusters vary in topology:
> | Topology | `ControlPlaneTopology` | Schedulable nodes |
> |---|---|---|
> | HA | `HighlyAvailable` | 3+ CP + N workers |
> | SNO | `SingleReplica` | 1 (all roles) |
> | Two-Node Fixed | `DualReplica` | 2 CP (no workers) |
> | Two-Node + Arbiter | `HighlyAvailableArbiter` | 2 CP + 1 arbiter (resource-limited) |
> | HyperShift | `External` | N workers (no CP nodes) |
>
> Please consider:
> - Checking `infrastructure.Status.ControlPlaneTopology` to detect
> `SingleReplica`, `DualReplica`, `HighlyAvailableArbiter`, or
> `External` topologies
> - Using required anti-affinity with `maxUnavailable >= 1` (not
> `maxUnavailable: 0`, which deadlocks). Preferred anti-affinity
> allows pods to co-locate, defeating HA.
> - Capping replica counts to the number of schedulable nodes
> - Excluding arbiter nodes (`node-role.kubernetes.io/arbiter`) from
> workload scheduling on TNA clusters
> - Avoiding `node-role.kubernetes.io/master` nodeSelectors on HyperShift,
> where no control-plane nodes exist in-cluster
> - For operators using the library-go `DeploymentController`, consider
> `WithTopologyAwareReplicasHook`, `WithTopologyAwareSchedulingHook`,
> and `WithControlPlaneNodeSelectorHook` from
> `library-go/pkg/operator/deploymentcontroller/`
>
> Verify with topology-specific CI jobs before merging:
> ```
> /payload-job periodic-ci-openshift-release-main-ci-4.22-e2e-aws-upgrade-ovn-single-node
> /payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-hypershift
> /payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-arbiter
> /payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-fencing-techpreview
> ```
- mode: error
name: OTE Binary Stdout Contract
instructions: |
OpenShift Tests Extension (OTE) binaries communicate with `openshift-tests` via
JSON on stdout. Any non-JSON stdout from the main binary process corrupts the
test listing and breaks CI. Individual test cases (It, BeforeEach, AfterEach) are
fine — their stdout is intercepted by the framework.
Only flag stdout writes in process-level code: main(), init(), TestMain(),
BeforeSuite(), AfterSuite(), SynchronizedBeforeSuite(), RunSpecs() setup, or
top-level var/const initializers.
Common violations:
- klog writing to stdout (default behavior — must redirect to stderr via
`klog.SetOutput(os.Stderr)` or `klog.LogToStderr(true)`)
- fmt.Print/Println/Printf to stdout in main or suite setup
- Ginkgo v2 suite configuration that emits warnings to stdout
- log.SetOutput not set to stderr before any logging
Do NOT flag:
- Writes inside It(), BeforeEach(), AfterEach(), JustBeforeEach() blocks
- Writes to explicitly-opened files or buffers
- Writes to os.Stderr or GinkgoWriter
- mode: warning
name: IPv6 and Disconnected Network Test Compatibility
instructions: |
When new Ginkgo e2e tests are added (It(), Describe(), Context(), When(), etc.),
check whether they make assumptions about IPv4 networking or require connectivity
to external/public internet services. IPv6-only CI jobs run in disconnected
environments with no public internet access.
Flag the test if it contains ANY of the following IPv4 assumptions:
- Hardcoded IPv4 addresses (e.g., "10.0.0.1", "192.168.1.1", "172.16.0.0/12")
- Hardcoded IPv4 localhost ("127.0.0.1") where "::1" would also be needed
- Parsing or validating IPs assuming IPv4 format only (e.g., splitting on "." to parse octets)
- Creating Service or Endpoint objects with hardcoded IPv4 CIDRs or addresses
- Using net.ParseIP() or net.ParseCIDR() with hardcoded IPv4 values only
- Assuming pod or node IPs will be IPv4 (e.g., checking ip.To4() != nil without fallback)
- Hardcoded IPv4-only network policies (ipBlock with IPv4 CIDRs only)
- Building URLs by interpolating a variable host/IP without brackets for IPv6
(e.g., `fmt.Sprintf("http://%s:%d/path", host, port)` or `"http://" + host + ":" + port`).
Use `net.JoinHostPort(host, port)` instead, which adds brackets automatically for IPv6.
Flag the test if it requires ANY of the following external connectivity:
- Connections to public internet hosts (e.g., google.com, github.com, quay.io, registry.redhat.io)
- Pulling images from public registries without using a mirror or internal registry
- Downloading content from external URLs (curl, wget to public endpoints)
- DNS resolution of public hostnames
- Connections to external APIs or services outside the cluster
Do NOT flag tests that:
- Use cluster-internal service DNS names (e.g., service.namespace.svc.cluster.local)
- Use the cluster's own registry or image streams
- Dynamically detect IP family and adapt accordingly
If a test is flagged, recommend the following:
> **IPv6 and disconnected network compatibility notice:** This test may contain
> IPv4 assumptions or external connectivity requirements that will fail in IPv6-only
> disconnected environments. Please verify your test works on IPv6 by running
> an additional CI job:
>
> For parallel tests:
> `/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-metal-ipi-ovn-ipv6`
>
> For serial tests (test name contains `[Serial]`):
> `/payload-job periodic-ci-openshift-release-master-nightly-4.22-e2e-metal-ipi-serial-ovn-ipv6`
>
> In the `openshift/origin` repo, use `GetIPAddressFamily()` to detect the
> cluster's IP family and adapt accordingly:
>
> ```go
> hasIPv4, hasIPv6, err := GetIPAddressFamily(oc)
> o.Expect(err).NotTo(o.HaveOccurred())
> if !hasIPv4 {
> // Use IPv6 addresses and CIDRs instead
> }
> ```
>
> Or use `GetIPFamilyForCluster()` to check the pod network IP family:
>
> ```go
> ipFamily := GetIPFamilyForCluster(oc.KubeFramework())
> if ipFamily == IPv6 {
> g.Skip("Test requires IPv4 connectivity")
> }
> ```
>
> You can also use the `InIPv4ClusterContext()` wrapper to automatically skip
> tests that only apply to IPv4 clusters:
>
> ```go
> InIPv4ClusterContext(oc, func() {
> // Test body - only runs on IPv4 clusters
> })
> ```
>
> For CIDRs, use `correctCIDRFamily()` to select the right CIDR for the cluster:
>
> ```go
> cidr := correctCIDRFamily(oc, "10.128.0.0/14", "fd01::/48")
> ```
>
> If the test requires external internet connectivity and cannot be adapted for
> disconnected environments, add `[Skipped:Disconnected]` to the test name to
> automatically skip it on disconnected clusters:
>
> ```go
> g.It("should fetch external content [Skipped:Disconnected]", func() { ... })
> ```
- mode: error
name: no-weak-crypto
instructions: |
Flag MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode usage.
Flag custom crypto implementations. Flag non-constant-time
comparison of secrets or tokens.
- mode: error
name: container-privileges
instructions: |
Flag privileged: true, hostPID, hostNetwork, hostIPC,
SYS_ADMIN capability, running as root without justification,
allowPrivilegeEscalation: true in container/K8s manifests.
- mode: error
name: no-sensitive-data-in-logs
instructions: |
Flag logging that may expose passwords, tokens, API keys,
PII (email, SSN, credit card), session IDs, internal
hostnames, or customer data.
tools:
ast-grep:
# Source: defaults
rule_dirs: []
# Source: defaults
util_dirs: []
# Source: Central YAML (inherited)
essential_rules: true
# Source: defaults
packages: []
shellcheck:
# Source: Repository YAML (base)
enabled: true
ruff:
# Source: defaults
enabled: true
markdownlint:
# Source: defaults
enabled: true
github-checks:
# Source: defaults
enabled: true
# Source: defaults
timeout_ms: 90000
languagetool:
# Source: defaults
enabled: true
# Source: defaults
enabled_rules: []
# Source: defaults
disabled_rules: []
# Source: defaults
enabled_categories: []
# Source: defaults
disabled_categories: []
# Source: defaults
enabled_only: false
# Source: defaults
level: default
biome:
# Source: defaults
enabled: true
hadolint:
# Source: Repository YAML (base)
enabled: true
swiftlint:
# Source: defaults
enabled: true
phpstan:
# Source: defaults
enabled: true
# Source: defaults
level: default
phpmd:
# Source: defaults
enabled: true
phpcs:
# Source: defaults
enabled: true
golangci-lint:
# Source: Repository YAML (base)
enabled: true
yamllint:
# Source: Repository YAML (base)
enabled: true
gitleaks:
# Source: Repository YAML (base)
enabled: true
trufflehog:
# Source: defaults
enabled: true
checkov:
# Source: Repository YAML (base)
enabled: true
tflint:
# Source: defaults
enabled: true
detekt:
# Source: defaults
enabled: true
eslint:
# Source: defaults
enabled: true
flake8:
# Source: defaults
enabled: true
fbinfer:
# Source: defaults
enabled: true
# Source: defaults
enable_java: false
fortitudeLint:
# Source: defaults
enabled: true
rubocop:
# Source: defaults
enabled: true
buf:
# Source: defaults
enabled: true
regal:
# Source: defaults
enabled: true
actionlint:
# Source: Central YAML (inherited)
enabled: true
zizmor:
# Source: defaults
enabled: true
pmd:
# Source: defaults
enabled: true
clang:
# Source: defaults
enabled: true
cppcheck:
# Source: defaults
enabled: true
opengrep:
# Source: defaults
enabled: true
semgrep:
# Source: Repository YAML (base)
enabled: true
circleci:
# Source: defaults
enabled: true
clippy:
# Source: defaults
enabled: true
sqlfluff:
# Source: defaults
enabled: true
trivy:
# Source: Central YAML (inherited)
enabled: true
prismaLint:
# Source: defaults
enabled: true
pylint:
# Source: defaults
enabled: true
oxc:
# Source: defaults
enabled: true
shopifyThemeCheck:
# Source: defaults
enabled: true
luacheck:
# Source: defaults
enabled: true
brakeman:
# Source: defaults
enabled: true
dotenvLint:
# Source: defaults
enabled: true
htmlhint:
# Source: defaults
enabled: true
stylelint:
# Source: defaults
enabled: true
checkmake:
# Source: defaults
enabled: true
osvScanner:
# Source: Central YAML (inherited)
enabled: true
oasdiff:
# Source: defaults
enabled: true
reactDoctor:
# Source: defaults
enabled: true
presidio:
# Source: defaults
enabled: true
blinter:
# Source: defaults
enabled: true
smartyLint:
# Source: defaults
enabled: true
emberTemplateLint:
# Source: defaults
enabled: true
skillspector:
# Source: defaults
enabled: true
psscriptanalyzer:
# Source: defaults
enabled: true
chat:
# Source: Central YAML (inherited)
art: false
# Source: defaults
allow_non_org_members: true
# Source: Repository YAML (base)
auto_reply: true
integrations:
jira:
# Source: defaults
usage: auto
linear:
# Source: defaults
usage: auto
knowledge_base:
# Source: defaults
opt_out: false
web_search:
# Source: defaults
enabled: true
code_guidelines:
# Source: Central YAML (inherited)
enabled: true
# Source: Repository YAML (base)
filePatterns:
- docs/*-guidelines.md
- '**/AGENTS.md'
- '**/CLAUDE.md'
- '**/CONTRIBUTING.md'
- '**/REDHAT.md'
learnings:
# Source: Repository YAML (base)
scope: auto
# Source: defaults
approval_delay: 0
issues:
# Source: Repository YAML (base)
scope: auto
jira:
# Source: defaults
usage: auto
# Source: defaults
project_keys: []
linear:
# Source: defaults
usage: auto
# Source: defaults
team_keys: []
pull_requests:
# Source: Repository YAML (base)
scope: auto
mcp:
# Source: defaults
usage: auto
# Source: defaults
disabled_servers: []
# Source: defaults
automatic_repository_linking: false
# Source: defaults
linked_repositories: []
code_generation:
docstrings:
# Source: defaults
path_instructions: []
unit_tests:
# Source: defaults
path_instructions: []
issue_enrichment:
auto_enrich:
# Source: defaults
enabled: false
planning:
# Source: defaults
enabled: true
auto_planning:
# Source: defaults
enabled: true
# Source: defaults
labels: []
labeling:
# Source: defaults
labeling_instructions: []
# Source: defaults
auto_apply_labels: false
|
rna-afk
force-pushed
the
coderabbit_security_changes
branch
from
24d3f30 to
a76c0dd
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rochacbruno The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
60-83: 🔒 Security & Privacy | 🟠 MajorForce-include supply-chain paths correctly
CodeRabbit automatically skips
dist/**, lockfiles, and minified JS. To review these, you must list them as positive patterns inpath_filters—the previous suggestion used an incorrect!exclusion prefix which would not force their inclusion.Corrected diff
path_filters: + # Force-review supply-chain surfaces skipped by default + - "**/dist/**" + - "**/package-lock.json" + - "**/*.min.js" # Exclude vendored dependencies - "!vendor/**" # Exclude generated files - "!**/zz_generated*" # Exclude generated CRD manifests - "!data/data/install.openshift.io_installconfigs.yaml"🤖 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 60 - 83, The path_filters block is using exclusion patterns where the review comment expects force-inclusion of supply-chain-sensitive paths, so update the .coderabbit.yaml configuration to add the needed directories/files as positive path_filters entries instead of negated ones. Make sure the change is applied in the path_filters section and preserves the existing security-sensitive exclusions/notes while allowing the intended paths to be reviewed.
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
654-704: 🔒 Security & Privacy | 🔵 TrivialExpand Renovate configuration detection to cover JSON5 and alternate locations.
Current coverage is incomplete. Renovate supports
renovate.json5,renovate.jsonc,.renovaterc.json5, and configurations within.github/and.gitlab/directories. Missing these paths allows maliciouspostUpgradeTasksto evade supply chain security checks.Consolidate and expand the path filters to include all valid variants.
🤖 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 654 - 704, The Renovate config path filters are incomplete and miss valid variants that can hide malicious settings. Update the glob patterns in the Renovate detection rules to cover renovate.json5, renovate.jsonc, .renovaterc.json5, and config files under .github/ and .gitlab/, keeping the existing checks in the same instruction blocks for the renovate config entries. Consolidate the matching logic in the existing path rules so all supported Renovate locations are covered consistently.
🤖 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 18-23: The secret-detection rule is too permissive because it
fully exempts tests and documentation, which can hide real credentials in
examples. Tighten the logic in the secret-flagging configuration so the
test/docs exemption only applies to obvious placeholder values like changeme or
xxx, while still flagging high-entropy or valid-looking API keys, tokens,
passwords, private keys, base64 strings, and embedded-credential URLs. Update
the relevant rule definitions in the secret detection config to preserve
detection behavior outside placeholder samples.
- Around line 623-651: Add the same CWE-74 prompt-injection protection used for
the .rules/ entry to the AGENTS.md, CLAUDE.md, CONTRIBUTING.md, and REDHAT.md
ingestion paths in the .coderabbit.yaml configuration. Update the relevant
configuration block and the ingestion definition so these files are explicitly
marked high-risk review context, with instructions to flag attempts to ignore,
downgrade, skip, or approve security, RBAC, CI/CD, dependency, or supply-chain
changes, plus encoded content, external commands, or org-level security
overrides. Use the existing .rules/ pattern and the review-context path entries
as the reference points for the change.
---
Outside diff comments:
In @.coderabbit.yaml:
- Around line 60-83: The path_filters block is using exclusion patterns where
the review comment expects force-inclusion of supply-chain-sensitive paths, so
update the .coderabbit.yaml configuration to add the needed directories/files as
positive path_filters entries instead of negated ones. Make sure the change is
applied in the path_filters section and preserves the existing
security-sensitive exclusions/notes while allowing the intended paths to be
reviewed.
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 654-704: The Renovate config path filters are incomplete and miss
valid variants that can hide malicious settings. Update the glob patterns in the
Renovate detection rules to cover renovate.json5, renovate.jsonc,
.renovaterc.json5, and config files under .github/ and .gitlab/, keeping the
existing checks in the same instruction blocks for the renovate config entries.
Consolidate the matching logic in the existing path rules so all supported
Renovate locations are covered consistently.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0bb7601d-c1e8-4d41-904b-f7fb579a1971
📒 Files selected for processing (1)
.coderabbit.yaml
rna-afk
force-pushed
the
coderabbit_security_changes
branch
from
a76c0dd to
41fab10
Compare
Add security-first review instructions and supply chain attack surface monitoring based on opendatahub-io/security-config patterns. Includes named security rules (hardcoded secrets, weak crypto, injection vectors, privileged containers, sensitive data in logs), path instructions for IDE/AI agent config injection (Miasma worm defense), diff hiding (Glassworm defense), dependency manifest tampering, CI/CD workflow security, and registry config monitoring. Enables settings-dangerous, hooks-dangerous, mcp-prohibited, and hooks-prohibited tools. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rna-afk
force-pushed
the
coderabbit_security_changes
branch
from
41fab10 to
a194285
Compare
Contributor
|
@rna-afk: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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
4 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.
Add security-first review instructions and supply chain attack surface monitoring based on opendatahub-io/security-config patterns. Includes named security rules (hardcoded secrets, weak crypto, injection vectors, privileged containers, sensitive data in logs), path instructions for IDE/AI agent config injection (Miasma worm defense), diff hiding (Glassworm defense), dependency manifest tampering, CI/CD workflow security, and registry config monitoring.
Summary by CodeRabbit
Summary by CodeRabbit