feat(PDRIVE-687): add mypy-based pre-commit hook for SafeCmdString ty… - #123
feat(PDRIVE-687): add mypy-based pre-commit hook for SafeCmdString ty…#123hoberger-rh wants to merge 1 commit into
Conversation
…pe checking Add a pre-commit hook that runs mypy to catch SafeCmdString type violations at commit time. Uses mypy.ini to filter out all non-SafeCmdString errors. - Add mypy.ini configuration tuned for SafeCmdString-only checking - Add safecmdstring-mypy-check hook to .pre-commit-config.yaml - Add mypy to dev dependencies Assisted-by: Claude Code (Claude Opus 4.6) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds mypy development tooling, configures scoped SafeCmdString type checking for Python 3.12, and registers a local pre-commit hook for the ChangesSafeCmdString mypy validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🤖 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 `@mypy.ini`:
- Around line 6-7: Update the mypy configuration’s disable_error_code list to
stop suppressing assignment and return-value errors globally, while retaining
the other existing suppressions. Keep assignment and return-value diagnostics
enabled so SafeCmdString type violations remain visible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2477a6a1-ad49-47ec-9912-eaea0530a9e5
📒 Files selected for processing (3)
.pre-commit-config.yamlmypy.inipyproject.toml
| # Only report SafeCmdString type errors | ||
| disable_error_code = assignment,var-annotated,no-untyped-def,return-value,import-untyped,no-any-return,attr-defined |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- mypy.ini ---\n'
cat -n mypy.ini
printf '\n--- SafeCmdString references ---\n'
rg -n "SafeCmdString|run_rsh_cmd|SafeCmd" -S .
printf '\n--- candidate files containing run_rsh_cmd or SafeCmdString ---\n'
python3 - <<'PY'
import os
matches = []
for root, dirs, files in os.walk('.'):
if '.git' in dirs:
dirs.remove('.git')
for f in files:
if f.endswith(('.py', '.pyi', '.ini', '.toml', '.md', '.yaml', '.yml')):
path = os.path.join(root, f)
try:
with open(path, 'r', encoding='utf-8', errors='ignore') as fh:
txt = fh.read()
if 'SafeCmdString' in txt or 'run_rsh_cmd' in txt:
matches.append(path)
except Exception:
pass
print('\n'.join(sorted(set(matches))))
PYRepository: RedHatInsights/incluster-checks
Length of output: 260
🌐 Web query:
mypy error code assignment return-value disable suppress incompatible assignment return type documentation
💡 Result:
To suppress or disable specific mypy error codes, such as those related to assignment or return values, you can use inline comments or configuration file settings [1][2]. To suppress errors on a specific line, append an inline comment with the relevant error code in brackets [3]: # type: ignore[assignment] # type: ignore[return-value] Using specific codes is recommended over a bare # type: ignore, as it prevents you from accidentally silencing other, unrelated errors on the same line [4][3]. To identify the exact error code being triggered, you can enable them in your output by setting show_error_codes = True in your mypy configuration file (mypy.ini or pyproject.toml) [4]. If you wish to disable an error code for an entire file, you can place a special comment at the top of the file [1][5]: # mypy: disable-error-code="assignment, return-value" You can also manage error codes globally or per-module via your configuration file [1][2]: [mypy] disable_error_code = assignment, return-value [mypy-my_module.*] disable_error_code = assignment For reference, common error codes include: - [assignment]: Used for incompatible types in assignment [6]. - [return-value]: Used when a function returns a value incompatible with its return type annotation [6]. - [method-assign]: A subcode of [assignment] that triggers when an assignment target is a method [6]. If an error code is a subcode of a broader category, ignoring the broader code (e.g., [assignment]) will typically suppress the narrower subcode as well [1][3].
Citations:
- 1: https://mypy.readthedocs.io/en/latest/error_codes.html
- 2: https://mypy.readthedocs.io/en/stable/error_codes.html
- 3: https://mypy.readthedocs.io/en/stable/error_codes.html?highlight=type%3A+ignore
- 4: https://stackoverflow.com/questions/54860432/can-i-suppress-mypy-errors-in-line
- 5: https://mypy.readthedocs.io/en/stable/common_issues.html
- 6: https://mypy.readthedocs.io/en/stable/error_code_list.html?highlight=return
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- mypy.ini ---'
cat -n mypy.ini
echo
echo '--- SafeCmdString / run_rsh_cmd references ---'
rg -n "SafeCmdString|run_rsh_cmd|disable_error_code|show_error_codes" -S . || true
echo
echo '--- likely source files containing SafeCmdString ---'
python3 - <<'PY'
import os
paths=[]
for root, dirs, files in os.walk('.'):
if '.git' in dirs:
dirs.remove('.git')
for fn in files:
if fn.endswith(('.py','.pyi','.md','.ini','.toml','.yaml','.yml')):
p=os.path.join(root,fn)
try:
txt=open(p,'r',encoding='utf-8',errors='ignore').read()
except Exception:
continue
if 'SafeCmdString' in txt or 'run_rsh_cmd' in txt:
paths.append(p)
for p in sorted(set(paths)):
print(p)
PYRepository: RedHatInsights/incluster-checks
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/in_cluster_checks/utils/safe_cmd_string.py ---'
wc -l src/in_cluster_checks/utils/safe_cmd_string.py
cat -n src/in_cluster_checks/utils/safe_cmd_string.py | sed -n '1,260p'
echo
echo '--- src/in_cluster_checks/utils/oc_api_utils.py (run_rsh_cmd area) ---'
cat -n src/in_cluster_checks/utils/oc_api_utils.py | sed -n '460,520p'
echo
echo '--- any mypy plugin / strict config references ---'
rg -n "plugin|plugins|SafeCmdString|disable_error_code|assignment|return-value" mypy.ini pyproject.toml setup.cfg tox.ini .pre-commit-config.yaml src tests -S | sed -n '1,220p'Repository: RedHatInsights/incluster-checks
Length of output: 46068
Keep assignment and return-value enabled for SafeCmdString
These global suppressions also hide plain str flowing through SafeCmdString-annotated assignments and returns. Narrow the suppression instead of disabling those codes project-wide.
🤖 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 `@mypy.ini` around lines 6 - 7, Update the mypy configuration’s
disable_error_code list to stop suppressing assignment and return-value errors
globally, while retaining the other existing suppressions. Keep assignment and
return-value diagnostics enabled so SafeCmdString type violations remain
visible.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
=======================================
Coverage ? 86.30%
=======================================
Files ? 57
Lines ? 6565
Branches ? 0
=======================================
Hits ? 5666
Misses ? 899
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 participants
Add a pre-commit hook that runs mypy and filters output to only report SafeCmdString type violations, catching command injection issues at commit time.
Assisted-by: Claude Code (Claude Opus 4.6) noreply@anthropic.com
Summary by CodeRabbit