Add post-edit lint shielding#260
Conversation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Ruff-based "lint shield" that runs after ChangesPost-Edit Lint Shield and Advisory Merge
Sequence Diagram(s)sequenceDiagram
rect rgba(135, 206, 235, 0.5)
note over AgentEvent,postEditOutput: Post-Edit Hook Entry
AgentEvent->>runner: Write/Edit/MultiEdit event
runner->>postEditOutput: event
end
rect rgba(144, 238, 144, 0.5)
note over postEditOutput,ruff: Lint Shield Execution
postEditOutput->>computeLintShieldState: file paths from event
computeLintShieldState->>computeLintShieldState: SHA-256 snapshot before
computeLintShieldState->>ruff: ruff format --quiet <files>
ruff-->>computeLintShieldState: exit 0 / error / timeout
computeLintShieldState->>ruff: ruff check --fix --quiet --output-format json
ruff-->>computeLintShieldState: JSON diagnostics
computeLintShieldState->>computeLintShieldState: SHA-256 snapshot after → changed_files
computeLintShieldState-->>postEditOutput: postEditLintShieldResult
end
rect rgba(255, 200, 120, 0.5)
note over postEditOutput,runner: Signal Gating and Context Emission
postEditOutput->>postEditHasActionableSignal: lintShieldState + lint signals
postEditHasActionableSignal-->>postEditOutput: bool
alt fully remediated and no other signals
postEditOutput-->>runner: nil HookSpecificOutput
else diagnostics remain or shield error
postEditOutput->>buildPostEditContext: lintShieldState + lint results
buildPostEditContext-->>postEditOutput: context with lint_shield section
postEditOutput-->>runner: HookSpecificOutput with AdditionalContext
end
runner-->>AgentEvent: merged advisory output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a post-edit lint shielding feature that automatically runs Ruff formatting and safe autofixes on edited Python files, updating the post-edit context and documentation. It also refactors the hook runner to merge multiple advisory hook outputs. Feedback on these changes highlights a security vulnerability in postEditLintShieldState, where input file paths are processed without validating whether they escape the repository root, potentially allowing directory traversal.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func postEditLintShieldState(event Event) postEditLintShieldResult { | ||
| files := existingPythonPostEditFiles(event.Cwd, event.Files()) | ||
| if len(files) == 0 { | ||
| return postEditLintShieldResult{} | ||
| } |
There was a problem hiding this comment.
The postEditLintShieldState function processes files provided by the event without validating if they escape the repository root (event.Cwd). This can lead to directory traversal vulnerabilities where the linter is executed on arbitrary files outside the repository. We should normalize and validate the paths to ensure they do not escape the repository root, using a separator-aware check to prevent directory traversal while avoiding false positives on directory names starting with "..".
func postEditLintShieldState(event Event) postEditLintShieldResult {
files := existingPythonPostEditFiles(event.Cwd, event.Files())
if len(files) == 0 {
return postEditLintShieldResult{}
}
safeFiles := make([]string, 0, len(files))
for _, file := range files {
path := file
if event.Cwd != "" && !filepath.IsAbs(path) {
path = filepath.Join(event.Cwd, path)
}
rel, err := filepath.Rel(event.Cwd, filepath.Clean(path))
if err != nil || rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../") {
continue
}
safeFiles = append(safeFiles, file)
}
files = safeFiles
if len(files) == 0 {
return postEditLintShieldResult{}
}References
- When validating if a path escapes a repository root, normalize both relative and absolute paths against the repository root and perform a separator-aware check (e.g., checking if the path is exactly ".." or starts with ".." followed by the path separator) to prevent directory traversal vulnerabilities and avoid false positives on directory names starting with ".." (such as "..foo").
![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 `@go/internal/hooks/post_edit_context.go`:
- Around line 241-262: The postEditLintShieldState function calls
runPostEditRuff which spawns ruff check --fix, and then postEditFastLintState
later spawns another ruff check, causing redundant subprocess calls.
Additionally, existingPythonPostEditFiles is recomputed in multiple functions.
To fix this, modify postEditLintShieldResult to include the JSON diagnostics
from the ruff check --fix output, refactor postEditLintShieldState to capture
and return these diagnostics, update postEditFastLintState to accept the cached
diagnostics and skip re-invoking ruff check when available, and move the
existingPythonPostEditFiles call to postEditOutput so the file list is computed
once and threaded through to all functions that need it.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 935ee002-0a77-4230-a498-2c77b4d5f775
📒 Files selected for processing (5)
README.mddocs/AGENT_PROXY.mdgo/internal/hooks/post_edit_context.gogo/internal/hooks/runner.gogo/internal/hooks/runner_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Coding Ethos SARIF Gate / Coding Ethos SARIF Gate
- GitHub Check: Unified lint
- GitHub Check: Test (Python 3.13)
- GitHub Check: Test (Python 3.11)
- GitHub Check: Validate GitHub workflows
- GitHub Check: Go coverage
- GitHub Check: CodeQL (go)
- GitHub Check: Go fuzz smoke
🧰 Additional context used
📓 Path-based instructions (1)
**/{hooks,policies,lint-capture,runtime}/**/*.{go,sh,py}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer compiled Go for hook, policy, lint-capture, and runtime glue; treat shell and Python as transitional unless they are clearly the right interface
Files:
go/internal/hooks/runner.gogo/internal/hooks/post_edit_context.gogo/internal/hooks/runner_test.go
🧠 Learnings (1)
📚 Learning: 2026-06-23T12:26:47.373Z
Learnt from: paudley
Repo: paudley/coding-ethos PR: 258
File: go/internal/geminiprompts/render.go:125-132
Timestamp: 2026-06-23T12:26:47.373Z
Learning: In the paudley/coding-ethos repo, the documented guideline “Gemini prompt pack should be generated in go/internal/geminiprompts/” refers to the generator implementation location (Go source), not the location of the generated prompt-pack artifact. Always treat PromptPackPath as the enforced output artifact path `.coding-ethos/gemini/prompt-pack.json` (e.g., render_test.go asserts PromptPackPath, and compiler_policies.go / hookrunner lookup / README document the expected path). Do not flag a code review issue just because PromptPackPath points to `.coding-ethos/gemini/prompt-pack.json`; only flag violations if the artifact output path logic breaks that enforced contract.
Applied to files:
go/internal/hooks/runner.gogo/internal/hooks/post_edit_context.gogo/internal/hooks/runner_test.go
🔇 Additional comments (7)
README.md (1)
689-691: LGTM!docs/AGENT_PROXY.md (1)
441-457: LGTM!go/internal/hooks/post_edit_context.go (3)
34-70: LGTM!Also applies to: 72-82, 98-143
241-320: LGTM!Also applies to: 322-374
563-586: LGTM!go/internal/hooks/runner.go (1)
293-295: LGTM!Also applies to: 314-390
go/internal/hooks/runner_test.go (1)
5120-5148: LGTM!Also applies to: 5863-5976, 6021-6082
|
Stage 2 update. Addressed review gaps:
Validation:
Latest pushed commit: |
paudley
deleted the
paudley/60-agent-proxy-lint-shielding-auto-remediation
branch
Summary
Write,Edit, andMultiEditevents.Closes #60
Validation
go test -buildvcs=false ./internal/hooksgo test -buildvcs=false ./internal/hooks -run 'TestRunInjectsContextAdviceOnSessionStartWhenThresholdCrossed|TestRunKeepsSessionStartLifecycleContextWhenMemoryImportFails|TestRunImportsMemoriesOnSessionStart|TestRunInjectsRepoMapOnSessionStart' -count=1 -vgo test -buildvcs=false ./internal/hooks -run 'TestRunReportsPostEditLintShieldError|TestRunKeepsSessionStartLifecycleContextWhenMemoryImportFails|TestRunSilentlyAppliesPostEditLintShieldForCodex|TestRunReportsAppliedPostEditLintShield' -count=1bin/golangci-lint run ./go/internal/hooks/...make buildmake check(passes with existing non-blockingtesting.go_coverage_goalwarning)bin/coding-ethos-run policy-git diff --checkNotes