GH#2915: fix ShellCheck memory explosion from language server (11 GB RSS, kernel panics)#2918
Conversation
The bash language server hardcodes --external-sources in every ShellCheck invocation. With 463 scripts cross-sourcing each other in .agents/scripts/, this causes exponential AST expansion (observed: 11 GB RSS, kernel panics). Three-layer defense: 1. shellcheck-wrapper.sh (root cause fix): Strips --external-sources from args before passing to real ShellCheck. Set as SHELLCHECK_PATH so the language server uses it instead of the real binary. 2. .agents/scripts/.shellcheckrc (secondary): Disables external-sources for direct file invocations. Limited effectiveness for stdin-piped content (language server pattern). 3. memory-pressure-monitor.sh auto-kill (safety net): ShellCheck processes hitting CRITICAL RSS (>4 GB) or exceeding runtime (>10 min) are now automatically killed. Safe because the language server respawns them. Setup integration: - setup.sh calls setup_shellcheck_wrapper() after agent deployment - Uses launchctl setenv (macOS GUI processes) + shell rc files (terminals) - Idempotent, runs in both interactive and non-interactive modes Closes #2915
🔍 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 14:43:09 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
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 (5)
✨ 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical memory explosion issue with ShellCheck when used by the bash language server, which was hardcoding the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 provides a robust, multi-layered solution to a critical memory explosion issue caused by ShellCheck's --external-sources flag, introducing a wrapper script, a dedicated .shellcheckrc, and an enhanced memory monitor to auto-kill runaway processes. The setup scripts are also updated to deploy and configure this new mechanism. A critical vulnerability was identified in the argument filtering logic of shellcheck-wrapper.sh, which has been addressed with a code suggestion. Additionally, suggestions to improve the robustness of PATH parsing and error handling for launchctl setenv have been provided, aligning with best practices for shell scripting.
| while IFS= read -r arg; do | ||
| filtered_args+=("$arg") | ||
| done < <(_filter_args "$@") |
There was a problem hiding this comment.
The script uses printf '%s\n' to serialize arguments and a while read loop to deserialize them. This is vulnerable to argument splitting if any argument contains a newline character. An attacker could use this to bypass the stripping of --external-sources or -x flags, which the wrapper is specifically designed to prevent to avoid Denial of Service (memory explosion). Bypassing this control directly re-introduces the risk of 11 GB RSS usage and kernel panics as described in the PR summary.
| while IFS= read -r arg; do | |
| filtered_args+=("$arg") | |
| done < <(_filter_args "$@") | |
| local filtered_args=() | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --external-sources | -x) | |
| # Strip this flag | |
| ;; | |
| *) | |
| filtered_args+=("$1") | |
| ;; | |
| esac | |
| shift | |
| done |
| local IFS=':' | ||
| local dir | ||
| for dir in $PATH; do |
There was a problem hiding this comment.
The current method of iterating over PATH using for dir in $PATH is not safe as it is subject to word splitting. This will cause issues if any directory in the PATH contains spaces. Using read -a to populate an array is a more robust approach that correctly handles such edge cases.
| local IFS=':' | |
| local dir | |
| for dir in $PATH; do | |
| local -a path_dirs | |
| IFS=':' read -r -a path_dirs <<< "$PATH" | |
| for dir in "${path_dirs[@]}"; do |
| if launchctl setenv SHELLCHECK_PATH "$wrapper_path" 2>/dev/null; then | ||
| print_info "Set SHELLCHECK_PATH via launchctl (GUI processes)" | ||
| fi |
There was a problem hiding this comment.
Suppressing error output from launchctl setenv with 2>/dev/null can hide configuration failures. If the command fails (e.g., when running setup via SSH), the user isn't notified that GUI applications will lack the SHELLCHECK_PATH setting. It's better to report the failure to the user, providing more context on why it might have occurred.
| if launchctl setenv SHELLCHECK_PATH "$wrapper_path" 2>/dev/null; then | |
| print_info "Set SHELLCHECK_PATH via launchctl (GUI processes)" | |
| fi | |
| if launchctl setenv SHELLCHECK_PATH "$wrapper_path"; then | |
| print_info "Set SHELLCHECK_PATH via launchctl (GUI processes)" | |
| else | |
| print_warning "Failed to set SHELLCHECK_PATH via launchctl. GUI apps may not use the wrapper. This can happen if not in a GUI session (e.g. SSH)." | |
| fi |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
Use 'while read -d :' instead of 'local IFS=:' to iterate PATH entries. Functionally identical but avoids Codacy's SC-style warning about IFS affecting variable expansion.
🔍 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 14:48:26 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
Follow-up: root cause removed in PR #2939 This PR contained the wrapper, but the root cause ( Upstream hazard note: The bash-language-server still hardcodes |
|
fatal: couldn't find remote ref bugfix/shellcheck-memory-explosion |
…oot survival Two changes to prevent kernel panics from runaway processes: 1. Pulse plist: RunAtLoad=true so the supervisor pulse restarts after reboot/panic. Previously RunAtLoad=false meant the pulse never restarted, leaving the system unmonitored after crashes. 2. Process guard: Added to setup.sh so all users get it on install/update. Previously the guard plist was manually created and not managed by setup. Now installs automatically: - macOS: launchd plist, 30s interval (was 120s), RunAtLoad=true - Linux: cron entry, every minute (cron minimum granularity) - No consent needed (safety net, not autonomous action) - Kills ShellCheck processes >512MB RSS or >120s runtime - Kills other AI processes >8GB RSS or >2h runtime Context: 5 kernel panics in one day from watchdog timeouts caused by ShellCheck memory bloat (2GB+ per instance) exhausting the compressor segment limit. The shellcheck-wrapper.sh (PR #2918) fixes the root cause; this ensures the guard catches any remaining edge cases.
…oot survival (#2940) * fix: add process guard to setup.sh and enable pulse RunAtLoad for reboot survival Two changes to prevent kernel panics from runaway processes: 1. Pulse plist: RunAtLoad=true so the supervisor pulse restarts after reboot/panic. Previously RunAtLoad=false meant the pulse never restarted, leaving the system unmonitored after crashes. 2. Process guard: Added to setup.sh so all users get it on install/update. Previously the guard plist was manually created and not managed by setup. Now installs automatically: - macOS: launchd plist, 30s interval (was 120s), RunAtLoad=true - Linux: cron entry, every minute (cron minimum granularity) - No consent needed (safety net, not autonomous action) - Kills ShellCheck processes >512MB RSS or >120s runtime - Kills other AI processes >8GB RSS or >2h runtime Context: 5 kernel panics in one day from watchdog timeouts caused by ShellCheck memory bloat (2GB+ per instance) exhausting the compressor segment limit. The shellcheck-wrapper.sh (PR #2918) fixes the root cause; this ensures the guard catches any remaining edge cases. * fix: address review feedback — dynamic PATH, HOME env, quoted cron paths, remove stderr suppression - Use dynamic ${PATH} and add ${HOME} to guard plist env vars for consistency with the pulse plist pattern (CodeRabbit, Gemini) - Remove 2>/dev/null from launchctl unload/load — || true handles errors, stderr aids debugging (Gemini) - Quote paths in cron entry to handle spaces in paths (Gemini) - Always regenerate cron entry to pick up config changes, matching macOS plist behavior (CodeRabbit)
1 participant
Summary
--external-sourcesin every ShellCheck invocation. With 463 cross-sourcing scripts, this causes exponential AST expansion (11 GB RSS, kernel panics)--external-sources; memory monitor auto-kills runaway processes; setup.sh deploys and configures automaticallysetup.sh/aidevops updatesetsSHELLCHECK_PATHvialaunchctl setenv(macOS GUI) + shell rc files (all terminals)Changes
.agents/scripts/shellcheck-wrapper.sh--external-sources, enforces ulimit, finds real shellcheck on PATH.agents/scripts/.shellcheckrcexternal-sources=falsefor direct file invocations.agents/scripts/memory-pressure-monitor.shsetup-modules/shell-env.shsetup_shellcheck_wrapper()— configures SHELLCHECK_PATH cross-shellsetup.shsetup_shellcheck_wrapper()in both interactive and non-interactive modesVerification
--statusand--helpoutput updated with auto-kill documentationPost-merge
After merge, run
aidevops update(or./setup.sh) to deploy the wrapper and setSHELLCHECK_PATH. Restart terminal/OpenCode for the env var to take effect.Closes #2915