t1402: Sanitize process names in log output to prevent log injection#2908
t1402: Sanitize process names in log output to prevent log injection#2908marcusquinn merged 1 commit intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Mar 5 06:17:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
PR has been open 8.5+ hours with green CI but zero formal reviews. Waiting for a review before merge can proceed. CodeRabbit shows 'pass' in the CI gate but has not posted a formal review comment on this PR. |
|
Permission check failed for this PR (HTTP 403 from collaborator permission API). Unable to determine if @alex-solovyev is a maintainer or external contributor. A maintainer must review and merge this PR manually. This is a fail-closed safety measure — the pulse will not auto-merge until the permission API succeeds. |
…1402) Add _sanitize_log_field() helper that strips control characters (ASCII 0x00-0x1F and 0x7F) from untrusted strings before writing to log files. Applied to cmd_base in guard_child_processes() where process names from ps output are logged during termination. Addresses the medium-severity log injection findings from Gemini review on PR #2881. The two critical findings (grep -v $$ self-exclusion bypass) were already fixed by prior PRs that rewrote the process guard to use awk descendant-tree walking. Closes #2892
marcusquinn
force-pushed
the
fix/pulse-wrapper-log-sanitization
branch
from
0601ec8 to
45cb57b
Compare
3 participants
Summary
_sanitize_log_field()helper that strips control characters (ASCII 0x00-0x1F, 0x7F) from untrusted strings before writing to log filescmd_baseinguard_child_processes()where process names frompsoutput are logged during terminationIssue Analysis
Issue #2892 reported 4 findings from Gemini review on PR #2881:
grep -v "$$"self-exclusion bypass (~line 2232)guard_child_processesnow usesawkdescendant-tree walkgrep -v "$$"self-exclusion bypass (~line 2272)guard_child_processescleanup_orphansonly logs counts, not process namesThe two critical findings were resolved by PRs #2879, #2881, #2885, and #2886 which rewrote the process guard to use proper descendant-tree walking. This PR addresses the remaining medium-severity log injection finding.
Testing
_sanitize_log_field(): normal input, newline injection, tab stripping, CR stripping, empty input, path-like input — all passCloses #2892