Skip to content

feat: add Claude Code infrastructure adapted for deadmanssnitch-operator#334

Open
anispate wants to merge 1 commit into
openshift:masterfrom
anispate:add-claude-code-infrastructure
Open

feat: add Claude Code infrastructure adapted for deadmanssnitch-operator#334
anispate wants to merge 1 commit into
openshift:masterfrom
anispate:add-claude-code-infrastructure

Conversation

@anispate

@anispate anispate commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

What changed vs PR #316

Critical fixes

Major fixes

  • Mock path: pkg/util/test/generated/pkg/dmsclient/mock/mock_dmsclient.go (actual location)
  • Nonexistent file paths: Removed refs to docs/design.md, docs/how-to-test.md, test/e2e/, controllers/deadmanssnitchintegration/*_secret.go, pkg/handler/deployment.go, pkg/mypackage, pkg/newpackage
  • prek vs pre-commit: CONTRIBUTING.md now correctly uses prek throughout
  • settings.json: git commands now have * wildcard suffix (e.g. "Bash(git diff *)") so flags don't prompt; --force-with-lease moved from deny to ask
  • Worktree-compatible hook: session-start-prek-setup.sh uses git rev-parse --git-path hooks instead of hardcoded .git/hooks/pre-commit
  • xargs safety: stop-prek-validation.sh uses printf '%s\0' | xargs -0 for filenames with spaces
  • pre-edit.sh: Mock pattern updated to /mock/mock_* to match actual path
  • operator-sdk prerequisite: Removed — not needed for local development
  • Incomplete sentences: Fixed in DEVELOPMENT.md and docs-agent.md

Minor fixes

  • "Conflux" → "Konflux" in ci-agent.md
  • ocm-agent-token rule: Removed from .gitleaks.toml (OCM Agent is a different repo)
  • gitleaks allowlist: test/fixtures/ path removed (directory doesn't exist); testdata/ used instead
  • Python imports: Removed unused Path and urlparse imports from both Python scripts
  • ".claude/worktrees/" and ".work/" added to .gitignore
  • "Future Skills" section: Removed from skills README (aspirational, not implemented)

Test plan

  • make go-test passes
  • make go-check passes
  • prek run --all-files runs without error (after installing prek)
  • Shell scripts are executable and pass shellcheck
  • TESTING.md accurately reflects testify/GoMock patterns in the test files
  • DEVELOPMENT.md make targets all exist in make help output

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added automated CI failure analysis with artifact retrieval and failure pattern detection to streamline debugging
  • Documentation

    • Added comprehensive contributor guidelines and development setup instructions for new contributors
    • Added testing documentation with frameworks, best practices, and troubleshooting guidance
  • Chores

    • Enhanced pre-commit validation framework with integrated security scanning and code quality checks

Adds standardized Claude Code tooling (agents, hooks, skills, docs) with
corrections for the DMS operator codebase:

- TESTING.md: rewritten for testify/assert + go.uber.org/mock (not Ginkgo/Gomega)
- DEVELOPMENT.md: updated Go version (1.25.4), correct mock location
  (pkg/dmsclient/mock/), accurate make targets, removed operator-sdk prereq
- CONTRIBUTING.md: prek-based workflow (not pre-commit), testify/GoMock style
- .claude/settings.json: wildcard-suffixed git commands, removed nonexistent
  targets (make tools, ginkgo), force-with-lease moved from deny to ask
- .claude/hooks/session-start-prek-setup.sh: worktree-compatible .git detection
- .claude/hooks/stop-prek-validation.sh: null-delimited xargs for filenames
  with spaces; mapfile for safe array construction
- .claude/hooks/pre-edit.sh: mock path pattern updated to /mock/mock_*
- ci-agent.md: Konflux spelling fix, removed pkg/handler/deployment.go ref
- lint-agent.md: replaced template file path with actual DMS controller path
- docs-agent.md: removed nonexistent docs/design.md, fixed Ginkgo references
- security-agent.md: replaced nonexistent *_secret.go ref with actual controller
- test-agent.md: testify/GoMock examples, removed Ginkgo commands
- .gitleaks.toml: removed ocm-agent-token rule (not relevant), fixed allowlist
  paths (no test/fixtures/ in this repo), added dms-api-key rule
- .gitignore: add .claude/worktrees/ and .work/ entries
- Python scripts: removed unused imports (Path, urlparse)
- prek.toml / hack/prek.ci.toml: exclude pattern corrected (no test/e2e/)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 18, 2026
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Walkthrough

Adds a complete Claude Code AI assistant configuration to the deadmanssnitch-operator repository. This includes prek-based pre-commit and CI validation configs with gitleaks rules, three Claude Code lifecycle hook scripts wired via settings.json, five AI agent specification files (ci, lint, security, test, docs), a prow-ci skill with Python tooling for analyzing Prow job failures, and new contributor documentation files (CONTRIBUTING.md, DEVELOPMENT.md, TESTING.md).

Changes

Claude Code AI Tooling Setup

Layer / File(s) Summary
prek validation configs and gitleaks rules
prek.toml, hack/prek.ci.toml, hack/ci.sh, .gitleaks.toml, .prek-version, .gitignore
prek.toml defines full local validation (hygiene, InfoSec, gitleaks, golangci-lint, go build/mod-tidy, RBAC check). hack/prek.ci.toml is the CI-compatible subset used by hack/ci.sh. .gitleaks.toml adds an allowlist and four custom rules (DMS API key, pull secret, kubeconfig cert, PEM key). .prek-version pins v0.4.1; .gitignore excludes Claude runtime artifacts.
Claude Code hook scripts
.claude/hooks/stop-prek-validation.sh, .claude/hooks/session-start-prek-setup.sh, .claude/hooks/pre-edit.sh, .claude/hooks/README.md
stop-prek-validation.sh reads the Claude stop-hook JSON input, collects changed/untracked files, runs prek with hack/prek.ci.toml, and emits a JSON block response on failure. session-start-prek-setup.sh verifies prek is in PATH and runs prek install if the git pre-commit hook is not yet wired. pre-edit.sh validates file paths and blocks or requires TTY confirmation for generated files, mocks, vendor, boilerplate, CRDs, go.sum, high-risk security/CI paths, and files over 500 lines. hooks/README.md documents the full architecture.
Claude Code settings and permissions
.claude/settings.json
Defines Bash command allow, ask, and deny permission lists and registers SessionStart (async, session-start-prek-setup.sh) and Stop (stop-prek-validation.sh) lifecycle hooks.
Claude Code agent definitions
.claude/agents/ci-agent.md, .claude/agents/lint-agent.md, .claude/agents/security-agent.md, .claude/agents/test-agent.md, .claude/agents/docs-agent.md
Five agent specs: CI agent covers Tekton pipeline integrity, local/CI parity, failure playbooks, performance targets, and security rules. Lint agent defines a gofmt/golangci-lint validation flow with safe auto-fix criteria. Security agent covers gitleaks scanning, RBAC wildcard checks, FIPS compliance, and false-positive handling. Test agent defines incremental test selection from git diff, GoMock usage, failure triage, and escalation conditions. Docs agent specifies sync targets, auto-update patterns, and validation commands.
prow-ci skill: Python artifact fetcher and analyzer
.claude/skills/prow-ci/fetch_prow_artifacts.py, .claude/skills/prow-ci/analyze_failure.py, .claude/skills/prow-ci/SKILL.md, .claude/skills/README.md
fetch_prow_artifacts.py parses a Prow job URL, extracts the GCS base path and build ID, and downloads prowjob.json (optional) and build-log.txt (required) via gcloud storage cp. analyze_failure.py scans the build log with regexes for compilation errors, test failures, lint errors, timeouts, and OOM, then generates markdown/JSON/text reports. SKILL.md documents prerequisites, quick-start, and local reproduction commands. skills/README.md describes the skill system and creation steps.
Contributor, development, and testing documentation
CONTRIBUTING.md, DEVELOPMENT.md, TESTING.md
CONTRIBUTING.md covers quick start, PR checklist, workflow rules for human and AI contributors, code style, security do/don't guidance, and commit message format. DEVELOPMENT.md documents Go prerequisites, prek setup, Makefile targets, code generation steps, boilerplate container usage, and mock regeneration. TESTING.md covers testify/GoMock patterns, controller test setup, coverage commands, PKO template tests, debugging, CI expectations, and common issue remediation.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer / Claude Code
  participant pre_edit as pre-edit.sh
  participant stop_hook as stop-prek-validation.sh
  participant prek
  participant gitleaks
  participant golangci_lint

  rect rgba(70, 130, 180, 0.5)
    note over Dev,pre_edit: Before file edit
    Dev->>pre_edit: Edit request (file path)
    pre_edit->>pre_edit: Validate path, check block/warn rules
    alt Blocked (generated/vendor)
      pre_edit-->>Dev: Exit non-zero (block edit)
    else Requires confirmation (high-risk/CRD/boilerplate)
      pre_edit-->>Dev: Prompt on TTY
    else Allowed
      pre_edit-->>Dev: Exit 0
    end
  end

  rect rgba(60, 179, 113, 0.5)
    note over Dev,golangci_lint: Claude Code session stop
    Dev->>stop_hook: Stop event JSON
    stop_hook->>stop_hook: Read stop_hook_active, collect changed files
    stop_hook->>prek: prek run --config hack/prek.ci.toml --files <changed>
    prek->>gitleaks: Secret scan
    prek->>golangci_lint: Go lint
    alt All checks pass
      prek-->>stop_hook: Exit 0
      stop_hook-->>Dev: Allow (no output)
    else Check failure
      prek-->>stop_hook: Non-zero + output
      stop_hook-->>Dev: JSON block with prek output
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Important

Pre-merge checks failed

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

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error Two Python scripts log potentially sensitive data without redaction: (1) fetch_prow_artifacts.py:68 prints gcloud stderr which may contain API keys/tokens; (2) analyze_failure.py:46-51 extracts all... Redact sensitive patterns from stderr output in fetch_prow_artifacts.py and filter/redact credentials from build-log errors/warnings before including in analyze_failure.py's output report.
Docstring Coverage ⚠️ Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (13 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding Claude Code infrastructure (agents, hooks, skills, and related documentation) to the deadmanssnitch-operator repository.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR adds only documentation, config, and tooling files—no Go test files. Ginkgo not used in repo (uses testify/assert + GoMock instead). Check is not applicable.
Test Structure And Quality ✅ Passed PR adds no Ginkgo test code; changes are documentation/configuration/tooling only. TESTING.md explicitly documents testify/GoMock instead, correcting prior Ginkgo references.
Microshift Test Compatibility ✅ Passed PR does not add any Ginkgo e2e tests. All changes are documentation, configuration, and tooling scripts; check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests added in this PR. The change is purely infrastructure, documentation, and tooling—no test code with It(), Describe(), Context(), or When() patterns detected.
Topology-Aware Scheduling Compatibility ✅ Passed This PR contains only infrastructure, documentation, and configuration files (.claude/ agents/hooks/skills, prek configs, contributing/development/testing docs). No deployment manifests, operator c...
Ote Binary Stdout Contract ✅ Passed PR adds no Go code, test binaries, or OTE-compatible executables. Changes are documentation, configuration, and non-OTE utility scripts for manual Prow CI analysis.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests are added in this PR. The check is not applicable—only documentation, config, and utility scripts are added.
No-Weak-Crypto ✅ Passed No weak cryptographic patterns detected. The PR adds documentation and tooling files with no MD5, SHA1, DES, RC4, 3DES, Blowfish, or ECB mode usage; no custom crypto implementations; no insecure to...
Container-Privileges ✅ Passed PR adds documentation, scripts, and configuration files only—no Kubernetes manifests or container security configurations to review. Check is not applicable.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.28%. Comparing base (e33c4cf) to head (978fb3c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #334   +/-   ##
=======================================
  Coverage   43.28%   43.28%           
=======================================
  Files          11       11           
  Lines         834      834           
=======================================
  Hits          361      361           
  Misses        424      424           
  Partials       49       49           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-ci

openshift-ci Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@anispate: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/validate 978fb3c link true /test validate

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (1)
.claude/agents/docs-agent.md (1)

44-47: 💤 Low value

Improve make target validation regex for better accuracy.

The validation pattern at lines 45-46 uses grep -h 'make [a-z]' *.md which may miss:

  • Make targets with uppercase letters (e.g., make container-BUILD)
  • Targets referenced within markdown code blocks that aren't meant to be validated (e.g., examples with placeholder targets)
  • Targets with hyphens that match the pattern but aren't actual targets

Consider refining the pattern to extract only targets from properly formatted code blocks (lines starting with backtick-backtick-backtick bash followed by make):

# Better pattern: extract make targets only from bash code blocks
grep -A1 '```bash' *.md | grep 'make ' | grep -oP '`?make \K[a-z0-9_-]+' | sort -u

This is a minor suggestion for robustness.

🤖 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 44 - 47, The make target
extraction pattern in the while loop is too restrictive and captures targets
from markdown examples that shouldn't be validated. Refine the grep pattern to
specifically extract make targets only from bash code blocks by searching for
lines following the backtick-backtick-backtick bash marker, then use an improved
character set pattern that includes uppercase letters, numbers, underscores and
hyphens instead of just lowercase letters. This will reduce false positives from
examples while better capturing actual valid make targets for validation.
🤖 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/hooks/pre-edit.sh:
- Around line 35-39: The Python command in both the python3 and python fallback
blocks directly interpolates `$FILE` and `$REPO_ROOT` into the -c string without
escaping, creating a code injection vulnerability if these variables contain
quotes or special characters. Escape both variables using proper shell escaping
(such as printf %q) before passing them to the Python -c command, or
alternatively pass the file and repo root as arguments to Python using sys.argv
instead of direct string interpolation in the -c command.

In @.claude/settings.json:
- Around line 3-31: The allow permissions in the settings.json file are too
permissive and grant overly broad auto-execution access. Remove or relocate the
following patterns from the allow list to an ask configuration (or narrow them
to explicit safe invocations): Bash(cat *), Bash(find *), Bash(grep *), Bash(ls
*), Bash(make run), and Bash(boilerplate/_lib/container-make *). These patterns
currently permit silent broad filesystem reads and can trigger unintended side
effects like network or cluster actions without explicit user confirmation. Keep
only the narrowly scoped, genuinely safe build and validation commands in the
allow list.

In @.claude/skills/prow-ci/analyze_failure.py:
- Around line 185-188: The file write operation that uses args.output is not
protected against I/O exceptions such as missing parent directories or
permission denied errors, which can cause unhandled tracebacks. Wrap the entire
block containing the open() call and f.write() operation in a try-except block
to catch IOError or OSError, and when an exception occurs, print a descriptive
error message to the user instead of allowing the exception to propagate
uncaught. This ensures the tooling script fails gracefully with predictable
behavior.

In @.claude/skills/prow-ci/fetch_prow_artifacts.py:
- Around line 57-69: The exception handling in the try-except block only catches
subprocess.CalledProcessError, but when gcloud is not installed,
subprocess.run() raises FileNotFoundError instead, which causes an uncaught
crash. Extend the except clause to catch FileNotFoundError in addition to
subprocess.CalledProcessError, and when FileNotFoundError occurs, print a clear
operator-facing error message indicating that gcloud is not installed or not in
PATH before returning False.

In @.claude/skills/prow-ci/SKILL.md:
- Around line 110-115: The `make docker-build` command referenced in the
workflow documentation does not correspond to any existing Makefile target in
the repository, which will cause users following these instructions to encounter
an error. Either remove the line containing `make docker-build` from the
documentation, or replace it with a valid and existing Makefile target that
accomplishes the same purpose (such as `make container-test` or another
appropriate build target that actually exists in the project's Makefiles).

In @.gitleaks.toml:
- Around line 12-89: The gitleaks configuration is missing the required
`[extend]` stanza needed for gitleaks v8.x to inherit built-in default rules.
Currently, only the four custom rules defined with the `[[rules]]` sections
(dms-api-key, openshift-pull-secret, kubeconfig-embedded, and private-key-pem)
are active, creating detection gaps. Add a new `[extend]` section near the
beginning of the configuration file (after the title and before the
`[allowlist]` section) to explicitly inherit the default gitleaks rules,
ensuring comprehensive secret detection coverage alongside the custom rules.

In `@hack/prek.ci.toml`:
- Around line 19-29: Add gitleaks secret scanning to the hack/prek.ci.toml file
by including a new repository hook configuration for gitleaks, similar to what
is already configured in the local prek.toml. The gitleaks hook should be added
as a separate [[repos]] section that references the gitleaks repository and
includes the gitleaks hook with appropriate configuration to ensure secrets are
detected during CI runs, maintaining consistency with the documented security
requirements in CONTRIBUTING.md and the local development setup.

---

Nitpick comments:
In @.claude/agents/docs-agent.md:
- Around line 44-47: The make target extraction pattern in the while loop is too
restrictive and captures targets from markdown examples that shouldn't be
validated. Refine the grep pattern to specifically extract make targets only
from bash code blocks by searching for lines following the
backtick-backtick-backtick bash marker, then use an improved character set
pattern that includes uppercase letters, numbers, underscores and hyphens
instead of just lowercase letters. This will reduce false positives from
examples while better capturing actual valid make targets for validation.
🪄 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: 64d9b9f0-86f1-4ba9-afe4-6bf033fc3bb7

📥 Commits

Reviewing files that changed from the base of the PR and between e33c4cf and 978fb3c.

📒 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-version
  • CONTRIBUTING.md
  • DEVELOPMENT.md
  • TESTING.md
  • hack/ci.sh
  • hack/prek.ci.toml
  • prek.toml

Comment thread .claude/hooks/pre-edit.sh
Comment on lines +35 to +39
if command -v python3 >/dev/null 2>&1; then
CANONICAL=$(python3 -c "import os.path; print(os.path.relpath(os.path.normpath('$FILE'), '$REPO_ROOT'))" 2>/dev/null || echo "")
elif command -v python >/dev/null 2>&1; then
CANONICAL=$(python -c "import os.path; print(os.path.relpath(os.path.normpath('$FILE'), '$REPO_ROOT'))" 2>/dev/null || echo "")
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Escape path inputs in Python fallback to prevent code injection.

The Python fallback builds source code by interpolating $FILE/$REPO_ROOT directly into the -c string. A crafted path containing quotes can break out of the string and execute arbitrary Python when this fallback path is used.

🔧 Proposed fix
-    CANONICAL=$(python3 -c "import os.path; print(os.path.relpath(os.path.normpath('$FILE'), '$REPO_ROOT'))" 2>/dev/null || echo "")
+    CANONICAL=$(python3 -c 'import os.path, sys; print(os.path.relpath(os.path.normpath(sys.argv[1]), sys.argv[2]))' "$FILE" "$REPO_ROOT" 2>/dev/null || echo "")
   elif command -v python >/dev/null 2>&1; then
-    CANONICAL=$(python -c "import os.path; print(os.path.relpath(os.path.normpath('$FILE'), '$REPO_ROOT'))" 2>/dev/null || echo "")
+    CANONICAL=$(python -c 'import os.path, sys; print(os.path.relpath(os.path.normpath(sys.argv[1]), sys.argv[2]))' "$FILE" "$REPO_ROOT" 2>/dev/null || echo "")
   fi

As per coding guidelines: “**/{.claude,.vscode}/**/*: HIGH RISK … Review every change with a security and malware lens.”

🤖 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 35 - 39, The Python command in both
the python3 and python fallback blocks directly interpolates `$FILE` and
`$REPO_ROOT` into the -c string without escaping, creating a code injection
vulnerability if these variables contain quotes or special characters. Escape
both variables using proper shell escaping (such as printf %q) before passing
them to the Python -c command, or alternatively pass the file and repo root as
arguments to Python using sys.argv instead of direct string interpolation in the
-c command.

Source: Coding guidelines

Comment thread .claude/settings.json
Comment on lines +3 to +31
"allow": [
"Bash(make go-build)",
"Bash(make go-test)",
"Bash(make go-check)",
"Bash(make lint)",
"Bash(make run)",
"Bash(make generate)",
"Bash(make coverage)",
"Bash(make validate)",
"Bash(go build ./...)",
"Bash(go test ./...)",
"Bash(go test *)",
"Bash(go fmt ./...)",
"Bash(go mod tidy)",
"Bash(prek run)",
"Bash(prek run *)",
"Bash(prek install)",
"Bash(prek --version)",
"Bash(boilerplate/_lib/container-make)",
"Bash(boilerplate/_lib/container-make *)",
"Bash(git status *)",
"Bash(git diff *)",
"Bash(git log *)",
"Bash(git branch *)",
"Bash(grep *)",
"Bash(find *)",
"Bash(ls *)",
"Bash(cat *)"
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Tighten allow permissions; current patterns grant overly broad auto-execution.

Auto-allowing Bash(cat *), Bash(find *), Bash(grep *), Bash(ls *), plus side-effect commands like Bash(make run) and Bash(boilerplate/_lib/container-make *) is too permissive for default execution. Move these to ask (or narrow to explicit safe invocations) to avoid silent broad filesystem reads and unintended network/cluster actions.

🔧 Suggested hardening direction
   "allow": [
     "Bash(make go-build)",
     "Bash(make go-test)",
     "Bash(make go-check)",
     "Bash(make lint)",
-    "Bash(make run)",
     "Bash(make generate)",
@@
-    "Bash(boilerplate/_lib/container-make)",
-    "Bash(boilerplate/_lib/container-make *)",
@@
-    "Bash(grep *)",
-    "Bash(find *)",
-    "Bash(ls *)",
-    "Bash(cat *)"
+    "Bash(boilerplate/_lib/container-make generate)"
   ],
   "ask": [
+    "Bash(make run)",
+    "Bash(boilerplate/_lib/container-make)",
+    "Bash(boilerplate/_lib/container-make *)",
+    "Bash(grep *)",
+    "Bash(find *)",
+    "Bash(ls *)",
+    "Bash(cat *)",

As per coding guidelines: “Flag … any change that grants broad filesystem or network access.”

🤖 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/settings.json around lines 3 - 31, The allow permissions in the
settings.json file are too permissive and grant overly broad auto-execution
access. Remove or relocate the following patterns from the allow list to an ask
configuration (or narrow them to explicit safe invocations): Bash(cat *),
Bash(find *), Bash(grep *), Bash(ls *), Bash(make run), and
Bash(boilerplate/_lib/container-make *). These patterns currently permit silent
broad filesystem reads and can trigger unintended side effects like network or
cluster actions without explicit user confirmation. Keep only the narrowly
scoped, genuinely safe build and validation commands in the allow list.

Source: Coding guidelines

Comment on lines +185 to +188
if args.output:
with open(args.output, 'w') as f:
f.write(output)
print(f"Analysis saved to: {args.output}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard output file writes to prevent uncaught I/O exceptions.

Writing to args.output can fail (missing parent dir, permission denied), and currently this bubbles up as a traceback.

As per coding guidelines, tooling scripts in .claude should prioritize safe, predictable failure behavior.

Suggested fix
     if args.output:
-        with open(args.output, 'w') as f:
-            f.write(output)
-        print(f"Analysis saved to: {args.output}")
+        try:
+            output_parent = os.path.dirname(args.output)
+            if output_parent:
+                os.makedirs(output_parent, exist_ok=True)
+            with open(args.output, 'w', encoding='utf-8') as f:
+                f.write(output)
+            print(f"Analysis saved to: {args.output}")
+        except OSError as e:
+            print(f"Error: Could not write output file {args.output}: {e}", file=sys.stderr)
+            return 1
     else:
         print(output)
🤖 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/analyze_failure.py around lines 185 - 188, The file
write operation that uses args.output is not protected against I/O exceptions
such as missing parent directories or permission denied errors, which can cause
unhandled tracebacks. Wrap the entire block containing the open() call and
f.write() operation in a try-except block to catch IOError or OSError, and when
an exception occurs, print a descriptive error message to the user instead of
allowing the exception to propagate uncaught. This ensures the tooling script
fails gracefully with predictable behavior.

Source: Coding guidelines

Comment on lines +57 to +69
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle missing gcloud binary explicitly to avoid uncaught crashes.

If gcloud is not installed, subprocess.run(...) raises FileNotFoundError, which currently escapes and crashes the script with a traceback.

As per coding guidelines, .claude scripts are high-risk and should fail safely with clear operator-facing errors.

Suggested fix
 def download_from_gcs(gcs_path, local_path):
     """Download a file from GCS using gcloud storage cp."""
     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 in PATH. Install Google Cloud SDK first.", 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.43.0)

[error] 64-64: Command coming from incoming request
Context: subprocess.run(cmd, check=True, capture_output=True)
Note: [CWE-20].

(subprocess-from-request)


[error] 64-64: Use of unsanitized data to create processes
Context: subprocess.run(cmd, check=True, capture_output=True)
Note: [CWE-78].

(os-system-unsanitized-data)

🪛 Ruff (0.15.17)

[error] 65-65: 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 57 - 69, The
exception handling in the try-except block only catches
subprocess.CalledProcessError, but when gcloud is not installed,
subprocess.run() raises FileNotFoundError instead, which causes an uncaught
crash. Extend the except clause to catch FileNotFoundError in addition to
subprocess.CalledProcessError, and when FileNotFoundError occurs, print a clear
operator-facing error message indicating that gcloud is not installed or not in
PATH before returning False.

Source: Coding guidelines

Comment on lines +110 to +115
# For coverage (matches: pull-ci-...-coverage)
make coverage

# For container builds (Tekton pipelines)
make docker-build
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# List declared make targets from top-level Makefile and included mk files
fd -HI '^Makefile$|\.mk$' -t f | while read -r f; do
  echo "== $f =="
  rg -n '^[a-zA-Z0-9_.-]+:\s*(##.*)?$' "$f" || true
done

echo
echo "== Search for specific targets referenced in SKILL.md =="
rg -n '^(coverage|docker-build):' Makefile **/*.mk || true

Repository: openshift/deadmanssnitch-operator

Length of output: 1906


Remove or replace make docker-build — this target does not exist in the repository's Makefile.

The make coverage target exists (line 415 in boilerplate/openshift/golang-osd-operator/standard.mk), but make docker-build has no match in any Makefile or .mk file. Users following this workflow will encounter an error. Either remove the docker-build line or replace it with a valid target (e.g., make container-test or make build-push if those are the intended equivalents).

🤖 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 around lines 110 - 115, The `make
docker-build` command referenced in the workflow documentation does not
correspond to any existing Makefile target in the repository, which will cause
users following these instructions to encounter an error. Either remove the line
containing `make docker-build` from the documentation, or replace it with a
valid and existing Makefile target that accomplishes the same purpose (such as
`make container-test` or another appropriate build target that actually exists
in the project's Makefiles).

Source: Coding guidelines

Comment thread .gitleaks.toml
Comment on lines +12 to +89
title = "gitleaks config for deadmanssnitch-operator"

# =============================================================================
# GLOBAL ALLOWLIST
# =============================================================================

[allowlist]
description = "Global allowlist for deadmanssnitch-operator"

# Test fixtures with fake credentials, generated code, and vendored code
paths = [
'''testdata/.*\.go''',
'''pkg/.*/testdata/.*\.go''',
'''boilerplate/.*''',
'''vendor/.*''',
'''zz_generated\..*\.go''',
]

# Allow specific test values that look like secrets but aren't
regexes = [
'''(?i)fake[_-]?token''',
'''(?i)test[_-]?secret''',
'''(?i)example[_-]?key''',
'''(?i)dummy[_-]?password''',
'''(?i)placeholder''',
'''AKIAIOSFODNN7EXAMPLE''', # AWS example from docs
]

commits = []

stopwords = [
"example",
"test",
"fake",
"dummy",
"placeholder",
"sample",
"mock",
]

# =============================================================================
# CUSTOM RULES (deadmanssnitch-operator specific)
# =============================================================================

[[rules]]
id = "dms-api-key"
description = "Dead Man's Snitch API key"
regex = '''(?i)dms[_-]?api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}'''
tags = ["token", "dms", "critical"]

[[rules]]
id = "openshift-pull-secret"
description = "OpenShift pull secret"
regex = '''(?i)pull[_-]?secret.*auth.*[a-zA-Z0-9+/]{30,}={0,2}'''
tags = ["secret", "openshift", "high"]

[[rules]]
id = "kubeconfig-embedded"
description = "Embedded kubeconfig with credentials"
regex = '''client-certificate-data:\s*[a-zA-Z0-9+/]{30,}={0,2}'''
tags = ["kubeconfig", "certificate", "critical"]

[[rules]]
id = "private-key-pem"
description = "PEM-encoded private key"
regex = '''-----BEGIN\s+(RSA\s+)?PRIVATE KEY-----'''
tags = ["private-key", "pem", "critical"]

# =============================================================================
# NOTES
# =============================================================================

# 1. This config extends gitleaks default rules
# 2. False positives should be added to allowlist with justification
# 3. Never disable gitleaks entirely (security critical)
# 4. Review allowlist periodically for stale entries
# 5. All allowlist additions should be documented in PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For gitleaks v8.x, when --config points to a custom .gitleaks.toml, are default built-in rules automatically included, or must [extend] useDefault = true be set?

💡 Result:

In Gitleaks v8.x, when you specify a custom configuration file using the --config flag, the default built-in rules are not automatically included [1][2]. By default, Gitleaks will only use the rules defined in your custom configuration file [1][3]. To include the default built-in rules alongside your custom rules, you must explicitly enable them by adding the [extend] section to your.gitleaks.toml file and setting useDefault to true [1][2]: [extend] useDefault = true When this setting is used, Gitleaks loads the default ruleset and then applies your custom configuration on top of it [1][4]. If your custom configuration defines rules with the same IDs as those in the default configuration, your custom rules (or their modified attributes) will take precedence and override the default ones [1][4][5].

Citations:


Add explicit [extend] stanza to inherit gitleaks default rules.

Gitleaks v8.x does not automatically include default built-in rules when a custom config is used. Your config currently runs only the four custom rules defined here, creating significant detection gaps despite the comment on line 84 claiming defaults are extended. Add the required [extend] section:

Required change
 title = "gitleaks config for deadmanssnitch-operator"
+
+[extend]
+useDefault = true
🤖 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 12 - 89, The gitleaks configuration is missing
the required `[extend]` stanza needed for gitleaks v8.x to inherit built-in
default rules. Currently, only the four custom rules defined with the
`[[rules]]` sections (dms-api-key, openshift-pull-secret, kubeconfig-embedded,
and private-key-pem) are active, creating detection gaps. Add a new `[extend]`
section near the beginning of the configuration file (after the title and before
the `[allowlist]` section) to explicitly inherit the default gitleaks rules,
ensuring comprehensive secret detection coverage alongside the custom rules.

Comment thread hack/prek.ci.toml
Comment on lines +19 to +29
# golangci-lint static analysis
[[repos]]
repo = "https://github.com/golangci/golangci-lint"
rev = "v2.0.2"
hooks = [
{ id = "golangci-lint", args = [
"--config=boilerplate/openshift/golang-osd-operator/golangci.yml",
"--timeout=120s"
] },
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Verify CI config is missing gitleaks and CI entrypoint uses it:"
rg -n 'gitleaks|github.com/gitleaks' hack/prek.ci.toml
rg -n 'prek run .*hack/prek.ci.toml' hack/ci.sh .claude/hooks/stop-prek-validation.sh

Repository: openshift/deadmanssnitch-operator

Length of output: 141


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking hack/ci.sh for prek config reference ==="
cat -n hack/ci.sh | head -50

echo -e "\n=== Checking hack/prek.toml for gitleaks ==="
rg -A 5 'gitleaks' hack/prek.toml

echo -e "\n=== List key files in hack/ directory ==="
ls -la hack/ | grep -E '\.(toml|sh)$'

echo -e "\n=== Check if .gitleaks.toml exists ==="
find . -name '.gitleaks.toml' -o -name 'gitleaks.toml' 2>/dev/null | head -5

Repository: openshift/deadmanssnitch-operator

Length of output: 530


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Find all prek*.toml files ==="
find . -name 'prek*.toml' -type f

echo -e "\n=== Check for .gitleaks.toml ==="
find . -name '.gitleaks.toml' -type f

echo -e "\n=== Search entire codebase for gitleaks references ==="
rg 'gitleaks' --type toml --type sh --type md

echo -e "\n=== Check README or docs for CI/security info ==="
rg -i 'secret.*scan|gitleaks|prek.*ci' README.md CLAUDE.md .gitignore 2>/dev/null || true

Repository: openshift/deadmanssnitch-operator

Length of output: 1139


Add gitleaks secret scanning to CI prek config.

Local development (prek.toml) includes gitleaks secret detection, and CONTRIBUTING.md documents "Security scan: Automatic via prek (gitleaks)" as mandatory. However, the CI config (hack/prek.ci.toml) omits gitleaks entirely. This creates a security bypass where secrets can reach CI without detection, contradicting documented security requirements.

Suggested patch
 # golangci-lint static analysis
 [[repos]]
 repo = "https://github.com/golangci/golangci-lint"
 rev = "v2.0.2"
 hooks = [
   { id = "golangci-lint", args = [
     "--config=boilerplate/openshift/golang-osd-operator/golangci.yml",
     "--timeout=120s"
   ] },
 ]
+
+# Gitleaks secret scanning
+[[repos]]
+repo = "https://github.com/gitleaks/gitleaks"
+rev = "v8.18.0"
+hooks = [
+  { id = "gitleaks", args = ["--config=.gitleaks.toml"] },
+]
🤖 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 `@hack/prek.ci.toml` around lines 19 - 29, Add gitleaks secret scanning to the
hack/prek.ci.toml file by including a new repository hook configuration for
gitleaks, similar to what is already configured in the local prek.toml. The
gitleaks hook should be added as a separate [[repos]] section that references
the gitleaks repository and includes the gitleaks hook with appropriate
configuration to ensure secrets are detected during CI runs, maintaining
consistency with the documented security requirements in CONTRIBUTING.md and the
local development setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants