Add Claude Code hooks, agents, skills, and developer docs#447
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anispate 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (23)
✅ Files skipped from review due to trivial changes (12)
🚧 Files skipped from review as they are similar to previous changes (8)
WalkthroughAdds Claude Code hooks and permissions, pre-commit/CI configuration, agent instructions, a Prow CI analysis skill, and repository documentation for development, testing, and contribution workflows. ChangesClaude Code governance and repo tooling
Sequence Diagram(s)sequenceDiagram
participant ClaudeCode
participant session-start-prek-setup.sh
participant pre-edit.sh
participant stop-prek-validation.sh
participant prek
participant fetch_prow_artifacts.py
participant analyze_failure.py
participant GCS
ClaudeCode->>session-start-prek-setup.sh: SessionStart
session-start-prek-setup.sh->>prek: check/install hook wiring
ClaudeCode->>pre-edit.sh: edit target file
pre-edit.sh->>pre-edit.sh: canonicalize path and gate edit
ClaudeCode->>stop-prek-validation.sh: Stop
stop-prek-validation.sh->>prek: run validation on changed files
ClaudeCode->>fetch_prow_artifacts.py: analyze Prow failure URL
fetch_prow_artifacts.py->>GCS: download prowjob.json and build-log.txt
fetch_prow_artifacts.py->>analyze_failure.py: hand off artifacts
analyze_failure.py-->>ClaudeCode: report output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0).claude/skills/prow-ci/fetch_prow_artifacts.py┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.20][ERROR]: unable to find a config; path 🔧 markdownlint-cli2 (0.22.1).claude/agents/lint-agent.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) TESTING.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) .claude/skills/README.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #447 +/- ##
=======================================
Coverage 72.44% 72.44%
=======================================
Files 11 11
Lines 704 704
=======================================
Hits 510 510
Misses 173 173
Partials 21 21 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 @.claude/agents/ci-agent.md:
- Around line 40-76: Update the Pre-commit ↔ CI Mapping section documentation to
align with the current prek contract. Replace all references to
`.pre-commit-config.yaml` with `prek.toml` or `hack/prek.ci.toml` in the version
validation grep commands. Replace the outdated `prek run --all` command with the
correct invocation using `--all-files` flag. Update all example commands and
paths throughout the "Pre-commit ↔ CI Mapping" and "Running Full CI Locally"
sections to reflect the current repository's prek configuration structure and
hook script conventions.
In @.claude/agents/docs-agent.md:
- Around line 33-35: The docs-agent.md file contains references to nonexistent
documentation paths (docs/design.md, docs/development.md) at lines 33-35, 93-95,
and 163-164. Replace these stale file path references with either actual
existing documentation files that are currently available in the repository, or
use more generic/flexible wording that doesn't depend on specific doc paths.
This will prevent guidance drift and align with the cleanup of broken
documentation references.
In @.claude/agents/security-agent.md:
- Around line 208-210: The documentation incorrectly claims that Tekton CI runs
both gitleaks and gosec, but the actual CI configuration only runs gosec via
golangci-lint. Correct the bullet point that currently states "**CI**: Tekton
runs gitleaks and gosec" to accurately reflect that only gosec runs in the
Tekton CI pipeline, while gitleaks only runs locally as a pre-commit hook.
Remove the reference to gitleaks from the CI section to ensure the documentation
accurately represents the actual security controls and scanning tools executed
in the pipeline.
In @.claude/agents/test-agent.md:
- Around line 31-39: The script does not handle the case when no `.go` files are
changed, which causes the package extraction and test execution to fail. Add a
guard clause after the CHANGED_FILES variable assignment to check if the result
is empty, and if so, exit cleanly with a success message. This prevents the
downstream PACKAGES extraction and the for loop from attempting to process empty
data.
In @.claude/hooks/pre-edit.sh:
- Around line 142-165: The HIGH_RISK_PATTERNS array in the pre-edit.sh script
does not include patterns to protect critical Claude automation configuration
files. Add three new patterns to the HIGH_RISK_PATTERNS array to flag changes to
.claude directory files: one pattern for .claude/settings.json, one pattern for
.claude/hooks/* files, and one pattern for .claude/agents/* files. These
patterns will ensure that modifications to Claude IDE configuration, hook
scripts, and agent definitions trigger the high-risk warning and require user
confirmation before proceeding.
In @.claude/hooks/stop-prek-validation.sh:
- Around line 25-29: The jq command is being used in the error handling branch
when REPO_ROOT is empty (when not in a git repository), but jq may not be
available on all machines. Either add a check to verify jq is installed before
using it, or replace the jq call in this error branch with a plain printf
statement to output the JSON decision object. The error path for "Not in a git
repository" should not depend on external tools like jq being present.
- Around line 80-89: The git commands and xargs invocation in the conditional
block (checking CHANGED_FILES and passing to prek run) do not properly handle
filenames with spaces or special characters. Modify the git diff and git
ls-files commands to use the -z flag for null-delimited output, and update the
xargs invocation to use the -0 flag to handle null-delimited input. This will
ensure that filenames with spaces, newlines, or other special characters are
correctly passed to the prek run command without being incorrectly split.
In @.claude/skills/prow-ci/fetch_prow_artifacts.py:
- Around line 63-75: The current exception handling in the try block only
catches subprocess.CalledProcessError, but when the gcloud binary is not
installed, subprocess.run raises FileNotFoundError instead, causing an unhandled
exception. Add an additional except clause after the existing
subprocess.CalledProcessError handler to catch FileNotFoundError (or OSError
more broadly) and print a user-friendly error message indicating that the gcloud
binary is not installed, then return False to maintain consistent error handling
behavior.
- Around line 84-89: The exception handling in the JSON file loading block only
catches json.JSONDecodeError, but filesystem errors from the open() call (such
as FileNotFoundError, PermissionError, or IOError) are not handled and will
cause the script to abort. Expand the exception handling to catch
filesystem-related errors in addition to JSONDecodeError. Either catch multiple
specific exceptions (FileNotFoundError, PermissionError, IOError, etc.) or use a
broader exception type like OSError or Exception to handle all potential errors
that could occur when opening and reading the local_path file, ensuring the
function gracefully returns None when any error occurs.
In @.claude/skills/prow-ci/SKILL.md:
- Line 214: The linting command documented in the SKILL.md file uses make
go-check, but this is inconsistent with the repository's actual declared linting
target. Replace the make go-check command with make lint to match the documented
lint target and ensure contributors following this workflow will use the correct
command.
In @.gitleaks.toml:
- Around line 50-59: The global stopwords array in the .gitleaks.toml
configuration is too permissive and can suppress legitimate secret detections by
matching overly common terms. Remove the entire stopwords array definition that
contains example, test, fake, dummy, placeholder, sample, and mock, and instead
implement path-specific or rule-specific allowlists for known false positives to
maintain tighter security controls that won't accidentally mask real credential
leaks.
🪄 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: e0180f3d-8ece-47a0-8ec5-c543dd1cd448
📒 Files selected for processing (23)
.claude/agents/ci-agent.md.claude/agents/docs-agent.md.claude/agents/lint-agent.md.claude/agents/security-agent.md.claude/agents/test-agent.md.claude/hooks/README.md.claude/hooks/pre-edit.sh.claude/hooks/session-start-prek-setup.sh.claude/hooks/stop-prek-validation.sh.claude/settings.json.claude/skills/README.md.claude/skills/prow-ci/SKILL.md.claude/skills/prow-ci/analyze_failure.py.claude/skills/prow-ci/fetch_prow_artifacts.py.gitignore.gitleaks.toml.prek-versionCONTRIBUTING.mdDEVELOPMENT.mdTESTING.mdhack/ci.shhack/prek.ci.tomlprek.toml
| ### Pre-commit ↔ CI Mapping | ||
|
|
||
| | Pre-commit Hook | CI Equivalent | Purpose | | ||
| |----------------|---------------|---------| | ||
| | `go-build` | Tekton compile check | Ensure code compiles | | ||
| | `golangci-lint` | Tekton lint job | Static analysis | | ||
| | `gitleaks` | Tekton security scan | Secret detection | | ||
| | `go-mod-tidy` | CI dependency check | No uncommitted go.mod/sum | | ||
| | `rbac-wildcard-check` | CI security policy | No wildcard RBAC | | ||
|
|
||
| **Parity validation:** | ||
| ```bash | ||
| # Check pre-commit uses same golangci-lint version as CI | ||
| grep "rev:" .pre-commit-config.yaml | grep golangci-lint | ||
| # Should match version in boilerplate pipeline | ||
|
|
||
| # Check gitleaks version | ||
| grep "rev:" .pre-commit-config.yaml | grep gitleaks | ||
| ``` | ||
|
|
||
| ### Running Full CI Locally | ||
|
|
||
| ```bash | ||
| # Lint (same as CI) | ||
| make go-check | ||
|
|
||
| # Tests (same environment as CI) | ||
| boilerplate/_lib/container-make go-test | ||
|
|
||
| # Build (same as CI) | ||
| make docker-build | ||
|
|
||
| # Full validation | ||
| prek run --all | ||
| make go-test | ||
| make go-build | ||
| ``` |
There was a problem hiding this comment.
Update the examples to match the repo's prek contract.
These snippets still grep .pre-commit-config.yaml and use prek run --all, but the repo now standardizes on prek.toml / hack/prek.ci.toml and the hook scripts use --all-files. Please align this walkthrough so the agent doesn't learn stale commands.
Suggested updates
-# Check pre-commit uses same golangci-lint version as CI
-grep "rev:" .pre-commit-config.yaml | grep golangci-lint
+# Check the pinned prek config used by CI
+grep -n "golangci-lint" prek.toml-prek run --all
+prek run --all-files🤖 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 @.claude/agents/ci-agent.md around lines 40 - 76, Update the Pre-commit ↔ CI
Mapping section documentation to align with the current prek contract. Replace
all references to `.pre-commit-config.yaml` with `prek.toml` or
`hack/prek.ci.toml` in the version validation grep commands. Replace the
outdated `prek run --all` command with the correct invocation using
`--all-files` flag. Update all example commands and paths throughout the
"Pre-commit ↔ CI Mapping" and "Running Full CI Locally" sections to reflect the
current repository's prek configuration structure and hook script conventions.
| - **API types changed**: Update `docs/design.md` | ||
| - **Test framework changes**: Update `TESTING.md` | ||
| - **New dependencies**: Update `docs/development.md` |
There was a problem hiding this comment.
Remove stale references to nonexistent docs paths.
This agent still points to docs/design.md / docs/development.md, which conflicts with the stated cleanup of broken doc references. Please replace with existing files (or generic wording) to avoid guidance drift.
Also applies to: 93-95, 163-164
🤖 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 @.claude/agents/docs-agent.md around lines 33 - 35, The docs-agent.md file
contains references to nonexistent documentation paths (docs/design.md,
docs/development.md) at lines 33-35, 93-95, and 163-164. Replace these stale
file path references with either actual existing documentation files that are
currently available in the repository, or use more generic/flexible wording that
doesn't depend on specific doc paths. This will prevent guidance drift and align
with the cleanup of broken documentation references.
| - **Pre-commit**: gitleaks runs automatically | ||
| - **CI**: Tekton runs gitleaks and gosec | ||
| - **RBAC check**: Custom make target |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== CI entrypoints mentioning prek/gitleaks/gosec =="
rg -n -C2 'prek|gitleaks|gosec|ci\.sh|prek\.ci\.toml' \
--iglob '*.yaml' --iglob '*.yml' --iglob '*.sh' --iglob '*.md'
echo
echo "== Tekton/Prow style configs (if present) =="
fd -i 'tekton|prow|pipeline|task' .
echo
echo "== Security agent claim lines =="
rg -n -C1 'CI.*gitleaks|gosec' .claude/agents/security-agent.mdRepository: openshift/splunk-forwarder-operator
Length of output: 8170
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "=== hack/prek.ci.toml content ==="
if [ -f "hack/prek.ci.toml" ]; then
cat hack/prek.ci.toml
else
echo "File not found"
fi
echo
echo "=== Prow config structure ==="
fd -t f . ./boilerplate/openshift/golang-osd-operator/prow-config/ 2>/dev/null | head -20
echo
echo "=== Tekton pipeline files in repo ==="
find . -maxdepth 3 -type f \( -name "*.yaml" -o -name "*.yml" \) -path "*tekton*" -o -path "*pipeline*" | grep -E "(tekton|pipeline)" | head -20
echo
echo "=== Check for Tekton PipelineRun or Task definitions ==="
rg -l "kind:\s*(PipelineRun|Pipeline|Task)" --max-count=10 2>/dev/null || echo "No Tekton definitions found via rg"Repository: openshift/splunk-forwarder-operator
Length of output: 1971
Remove or correct the false claim that CI runs gitleaks.
The CI configuration (hack/prek.ci.toml) used by hack/ci.sh explicitly excludes gitleaks—it only includes golangci-lint (which runs gosec) and custom Go/RBAC checks. While gitleaks is defined in .pre-commit-config.yaml for local pre-commit hooks, it does not run in the Tekton CI pipeline. Line 209's claim that "CI: Tekton runs gitleaks and gosec" is inaccurate: CI runs gosec (via golangci-lint) but not gitleaks.
High-risk file per coding guidelines: Correct this documentation to avoid misleading developers about actual CI security controls.
🤖 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 @.claude/agents/security-agent.md around lines 208 - 210, The documentation
incorrectly claims that Tekton CI runs both gitleaks and gosec, but the actual
CI configuration only runs gosec via golangci-lint. Correct the bullet point
that currently states "**CI**: Tekton runs gitleaks and gosec" to accurately
reflect that only gosec runs in the Tekton CI pipeline, while gitleaks only runs
locally as a pre-commit hook. Remove the reference to gitleaks from the CI
section to ensure the documentation accurately represents the actual security
controls and scanning tools executed in the pipeline.
Source: Coding guidelines
| CHANGED_FILES=$(git diff --name-only HEAD | grep "\.go$") | ||
|
|
||
| # Extract packages | ||
| PACKAGES=$(echo "$CHANGED_FILES" | xargs -n1 dirname | sort -u | tr '\n' ' ') | ||
|
|
||
| # Run targeted tests | ||
| for pkg in $PACKAGES; do | ||
| go test -v ./$pkg/... | ||
| done |
There was a problem hiding this comment.
Handle empty changed-file sets in test selection logic.
When no .go files are changed, this flow can error during package extraction. Add a guard so the agent exits cleanly instead of failing.
Proposed fix
CHANGED_FILES=$(git diff --name-only HEAD | grep "\.go$")
+if [ -z "${CHANGED_FILES}" ]; then
+ echo "No changed Go files; skipping targeted tests."
+ exit 0
+fi
# Extract packages
PACKAGES=$(echo "$CHANGED_FILES" | xargs -n1 dirname | sort -u | tr '\n' ' ')🤖 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 @.claude/agents/test-agent.md around lines 31 - 39, The script does not
handle the case when no `.go` files are changed, which causes the package
extraction and test execution to fail. Add a guard clause after the
CHANGED_FILES variable assignment to check if the result is empty, and if so,
exit cleanly with a success message. This prevents the downstream PACKAGES
extraction and the for loop from attempting to process empty data.
| HIGH_RISK_PATTERNS=( | ||
| "*/rbac.go" | ||
| "*/auth*.go" | ||
| "*_rbac.yaml" | ||
| "*/networkpolicy*.go" | ||
| "*[Cc]luster[Rr]ole*.yaml" | ||
| ".tekton/*.yaml" | ||
| "build/Dockerfile" | ||
| ) | ||
|
|
||
| for pattern in "${HIGH_RISK_PATTERNS[@]}"; do | ||
| # shellcheck disable=SC2053 | ||
| if [[ "$FILE" == $pattern ]]; then | ||
| echo "⚠️ HIGH-RISK FILE: $FILE" | ||
| echo " This file affects security or CI/CD." | ||
| echo " Changes require:" | ||
| echo " - Careful review" | ||
| echo " - Test coverage" | ||
| echo " - Security validation" | ||
| echo "" | ||
| confirm_or_exit " Continue? (y/N)" | ||
| break | ||
| fi | ||
| done |
There was a problem hiding this comment.
Protect the Claude automation files too.
As per coding guidelines, .claude is a HIGH RISK — IDE and AI tool configuration (prodsec-skills). This allowlist never warns on .claude/settings.json, .claude/hooks/*, or .claude/agents/*, so the files that control the agent itself can still be edited without confirmation.
Suggested pattern additions
HIGH_RISK_PATTERNS=(
+ ".claude/settings.json"
+ ".claude/hooks/*"
+ ".claude/agents/*"
"*/rbac.go"📝 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.
| HIGH_RISK_PATTERNS=( | |
| "*/rbac.go" | |
| "*/auth*.go" | |
| "*_rbac.yaml" | |
| "*/networkpolicy*.go" | |
| "*[Cc]luster[Rr]ole*.yaml" | |
| ".tekton/*.yaml" | |
| "build/Dockerfile" | |
| ) | |
| for pattern in "${HIGH_RISK_PATTERNS[@]}"; do | |
| # shellcheck disable=SC2053 | |
| if [[ "$FILE" == $pattern ]]; then | |
| echo "⚠️ HIGH-RISK FILE: $FILE" | |
| echo " This file affects security or CI/CD." | |
| echo " Changes require:" | |
| echo " - Careful review" | |
| echo " - Test coverage" | |
| echo " - Security validation" | |
| echo "" | |
| confirm_or_exit " Continue? (y/N)" | |
| break | |
| fi | |
| done | |
| HIGH_RISK_PATTERNS=( | |
| ".claude/settings.json" | |
| ".claude/hooks/*" | |
| ".claude/agents/*" | |
| "*/rbac.go" | |
| "*/auth*.go" | |
| "*_rbac.yaml" | |
| "*/networkpolicy*.go" | |
| "*[Cc]luster[Rr]ole*.yaml" | |
| ".tekton/*.yaml" | |
| "build/Dockerfile" | |
| ) | |
| for pattern in "${HIGH_RISK_PATTERNS[@]}"; do | |
| # shellcheck disable=SC2053 | |
| if [[ "$FILE" == $pattern ]]; then | |
| echo "⚠️ HIGH-RISK FILE: $FILE" | |
| echo " This file affects security or CI/CD." | |
| echo " Changes require:" | |
| echo " - Careful review" | |
| echo " - Test coverage" | |
| echo " - Security validation" | |
| echo "" | |
| confirm_or_exit " Continue? (y/N)" | |
| break | |
| fi | |
| done |
🤖 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 @.claude/hooks/pre-edit.sh around lines 142 - 165, The HIGH_RISK_PATTERNS
array in the pre-edit.sh script does not include patterns to protect critical
Claude automation configuration files. Add three new patterns to the
HIGH_RISK_PATTERNS array to flag changes to .claude directory files: one pattern
for .claude/settings.json, one pattern for .claude/hooks/* files, and one
pattern for .claude/agents/* files. These patterns will ensure that
modifications to Claude IDE configuration, hook scripts, and agent definitions
trigger the high-risk warning and require user confirmation before proceeding.
Source: Coding guidelines
| CHANGED_FILES=$(git diff --name-only --diff-filter=d HEAD; git ls-files --others --exclude-standard) | ||
| if [[ -z "$CHANGED_FILES" ]]; then | ||
| # No files changed, but we're here because git status showed changes | ||
| # Fall back to --all-files to catch any edge cases | ||
| PREK_OUTPUT=$(prek run --all-files --config hack/prek.ci.toml 2>&1) | ||
| else | ||
| # Pass changed files explicitly to prek | ||
| PREK_OUTPUT=$(echo "$CHANGED_FILES" | xargs prek run --config hack/prek.ci.toml --files 2>&1) | ||
| fi | ||
| PREK_EXIT=$? |
There was a problem hiding this comment.
Preserve filenames when passing them to prek.
xargs will split valid paths on spaces/newlines, so the hook can validate the wrong files or fail on otherwise valid edits. Use a NUL-delimited path list (git diff -z / git ls-files -z) and pass it through a filename-safe transport.
🤖 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 @.claude/hooks/stop-prek-validation.sh around lines 80 - 89, The git commands
and xargs invocation in the conditional block (checking CHANGED_FILES and
passing to prek run) do not properly handle filenames with spaces or special
characters. Modify the git diff and git ls-files commands to use the -z flag for
null-delimited output, and update the xargs invocation to use the -0 flag to
handle null-delimited input. This will ensure that filenames with spaces,
newlines, or other special characters are correctly passed to the prek run
command without being incorrectly split.
| try: | ||
| os.makedirs(os.path.dirname(local_path), exist_ok=True) | ||
| cmd = [ | ||
| 'gcloud', 'storage', 'cp', | ||
| gcs_path, | ||
| local_path, | ||
| '--no-user-output-enabled' | ||
| ] | ||
| subprocess.run(cmd, check=True, capture_output=True) | ||
| return True | ||
| except subprocess.CalledProcessError as e: | ||
| print(f"Warning: Could not download {gcs_path}: {e.stderr.decode()}", file=sys.stderr) | ||
| return False |
There was a problem hiding this comment.
Handle missing gcloud binary explicitly.
If gcloud is not installed, subprocess.run raises FileNotFoundError and the script exits with a traceback instead of a clean error.
Suggested fix
try:
os.makedirs(os.path.dirname(local_path), exist_ok=True)
cmd = [
'gcloud', 'storage', 'cp',
gcs_path,
local_path,
'--no-user-output-enabled'
]
subprocess.run(cmd, check=True, capture_output=True)
return True
+ except FileNotFoundError:
+ print("Error: gcloud CLI not found. Install Google Cloud SDK and retry.", file=sys.stderr)
+ return False
except subprocess.CalledProcessError as e:
print(f"Warning: Could not download {gcs_path}: {e.stderr.decode()}", file=sys.stderr)
return False🧰 Tools
🪛 ast-grep (0.44.0)
[error] 70-70: Command coming from incoming request
Context: subprocess.run(cmd, check=True, capture_output=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 70-70: Use of unsanitized data to create processes
Context: subprocess.run(cmd, check=True, capture_output=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
🪛 Ruff (0.15.18)
[error] 71-71: subprocess call: check for execution of untrusted input
(S603)
🤖 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 @.claude/skills/prow-ci/fetch_prow_artifacts.py around lines 63 - 75, The
current exception handling in the try block only catches
subprocess.CalledProcessError, but when the gcloud binary is not installed,
subprocess.run raises FileNotFoundError instead, causing an unhandled exception.
Add an additional except clause after the existing subprocess.CalledProcessError
handler to catch FileNotFoundError (or OSError more broadly) and print a
user-friendly error message indicating that the gcloud binary is not installed,
then return False to maintain consistent error handling behavior.
| try: | ||
| with open(local_path, 'r') as f: | ||
| return json.load(f) | ||
| except json.JSONDecodeError as e: | ||
| print(f"Error: Could not parse JSON from {local_path}: {e}", file=sys.stderr) | ||
| return None |
There was a problem hiding this comment.
Guard optional prowjob.json reads against filesystem errors.
This block treats prowjob.json as optional, but open() failures (e.g., permissions/partial writes) aren’t caught and can abort the script.
Suggested fix
- except json.JSONDecodeError as e:
+ except (json.JSONDecodeError, OSError) as e:
print(f"Error: Could not parse JSON from {local_path}: {e}", file=sys.stderr)
return None🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 84-84: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(local_path, 'r')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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 @.claude/skills/prow-ci/fetch_prow_artifacts.py around lines 84 - 89, The
exception handling in the JSON file loading block only catches
json.JSONDecodeError, but filesystem errors from the open() call (such as
FileNotFoundError, PermissionError, or IOError) are not handled and will cause
the script to abort. Expand the exception handling to catch filesystem-related
errors in addition to JSONDecodeError. Either catch multiple specific exceptions
(FileNotFoundError, PermissionError, IOError, etc.) or use a broader exception
type like OSError or Exception to handle all potential errors that could occur
when opening and reading the local_path file, ensuring the function gracefully
returns None when any error occurs.
| make go-test | ||
|
|
||
| # For linting (matches: pull-ci-...-lint) | ||
| make go-check |
There was a problem hiding this comment.
Use the repository’s documented lint target here.
make go-check appears inconsistent with this repo’s declared commands and will likely fail for contributors following this workflow.
Suggested doc fix
- make go-check
+ make lintAs per coding guidelines, the documented linting command is make lint.
📝 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.
| make go-check | |
| make lint |
🤖 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 @.claude/skills/prow-ci/SKILL.md at line 214, The linting command documented
in the SKILL.md file uses make go-check, but this is inconsistent with the
repository's actual declared linting target. Replace the make go-check command
with make lint to match the documented lint target and ensure contributors
following this workflow will use the correct command.
Source: Coding guidelines
| # Stopwords that appear in code but aren't secrets | ||
| stopwords = [ | ||
| "example", | ||
| "test", | ||
| "fake", | ||
| "dummy", | ||
| "placeholder", | ||
| "sample", | ||
| "mock", | ||
| ] |
There was a problem hiding this comment.
Narrow global stopwords to avoid suppressing real secret leaks.
These global stopwords are too broad and can mask valid detections (for example, real credentials containing test/example/sample). Keep only tightly scoped false-positive controls (path- or rule-specific allowlists) instead of global suppression.
Suggested hardening
stopwords = [
- "example",
- "test",
- "fake",
- "dummy",
- "placeholder",
- "sample",
- "mock",
+ "AKIAIOSFODNN7EXAMPLE",
]Based on learnings: Never commit API keys, tokens, passwords, AWS credentials, kubeconfig files, private keys, certificates, .env files with secrets, or debug statements printing sensitive data.
📝 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.
| # Stopwords that appear in code but aren't secrets | |
| stopwords = [ | |
| "example", | |
| "test", | |
| "fake", | |
| "dummy", | |
| "placeholder", | |
| "sample", | |
| "mock", | |
| ] | |
| # Stopwords that appear in code but aren't secrets | |
| stopwords = [ | |
| "AKIAIOSFODNN7EXAMPLE", | |
| ] |
🤖 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 @.gitleaks.toml around lines 50 - 59, The global stopwords array in the
.gitleaks.toml configuration is too permissive and can suppress legitimate
secret detections by matching overly common terms. Remove the entire stopwords
array definition that contains example, test, fake, dummy, placeholder, sample,
and mock, and instead implement path-specific or rule-specific allowlists for
known false positives to maintain tighter security controls that won't
accidentally mask real credential leaks.
Source: Learnings
Adds standardized Claude Code tooling (agents, hooks, skills, settings) and developer documentation (CONTRIBUTING.md, DEVELOPMENT.md, TESTING.md, prek configs, gitleaks config) adapted from the ocm-agent-operator reference implementation. Fixes vs closed PR openshift#437: - Go version corrected to 1.25+ (was 1.22.7) - Removed nonexistent make targets (tools, run, run-verbose, help) - Replaced all pre-commit references with prek equivalents - Removed OCM copy-paste artifacts (ocm_agent_token rule, fleet notification example, pkg/handler/deployment.go path references) - Fixed command injection in pre-edit.sh python3 fallback (sys.argv) - Narrowed Bash(find *) permission to Bash(find . -name/type *) - Fixed nested code block in TESTING.md - Corrected operator name to "Splunk Forwarder Operator" throughout - Removed links to nonexistent docs/ files Related: ROSAENG-60008 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
anispate
force-pushed
the
ROSAENG-60008-claude-code-hooks-and-docs
branch
from
b2848ed to
619376c
Compare
|
@anispate: 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. |
2 participants
Summary
.claude/settings.jsonCONTRIBUTING.md,DEVELOPMENT.md,TESTING.mdprek.toml,hack/prek.ci.toml,.gitleaks.toml,.prek-version,hack/ci.shFixes vs closed PR #437
All issues identified in ROSAENG-60008 have been addressed:
Major:
1.25+(was1.22.7)make tools,make run,make run-verbosetargets; replaced with correct commandspre-commitreferences withprekequivalents throughout docs and agentsocm_agent_tokengitleaks rule replaced withsplunk-auth-token,fleet notification filteringexample updated,pkg/handler/deployment.gopath examples replaced with real pathspre-edit.shpython3 fallback: shell variables no longer interpolated into-cstrings; usessys.argvinsteadMinor:
Bash(find *)permission insettings.jsontoBash(find . -name *)andBash(find . -type *)TESTING.mddocs/design.mdanddocs/how-to-test.mdpkg/util/test/generated/pathCloses ROSAENG-60008
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores