Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .agents/scripts/.shellcheckrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# ShellCheck configuration for .agents/scripts/
#
# GH#2915: The bash language server spawns ShellCheck with --external-sources
# on every edit. With 463 scripts cross-sourcing each other, this causes
# exponential AST expansion (observed: 11 GB RSS, kernel panics).
#
# LIMITATION: This .shellcheckrc is effective when ShellCheck is invoked with
# a file path (it walks up from the file's directory to find .shellcheckrc).
# However, the bash language server pipes content via stdin (-), so ShellCheck
# has no file path to walk from and CANNOT discover this file automatically.
# The primary fix is shellcheck-wrapper.sh (strips --external-sources from
# the language server's invocation). This .shellcheckrc is a secondary defense
# for direct ShellCheck invocations on files in this directory.
#
# Framework-invoked ShellCheck (linters-local.sh, pulse-wrapper.sh) already
# uses --norc and explicit flags, so this file doesn't affect them.

# Disable external source following — prevents recursive expansion
external-sources=false

# Inherit the same disable rules as root .shellcheckrc
# (ShellCheck only reads ONE .shellcheckrc — the first found — so we must
# duplicate the disables here rather than inheriting from root)
disable=SC2329
disable=SC2317
disable=SC2034
disable=SC2001
disable=SC2059
disable=SC2012
disable=SC2030
disable=SC2031
disable=SC2015
disable=SC2129
disable=SC2153
disable=SC2004
disable=SC2009
58 changes: 57 additions & 1 deletion .agents/scripts/memory-pressure-monitor.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
# processes (ShellCheck at 5.7 GB, zombie pulses, session accumulation), not by
# generic OS memory pressure.
#
# Auto-kill (GH#2915): ShellCheck processes that hit CRITICAL RSS or exceed
# runtime limits are automatically killed. This is safe because the bash
# language server respawns them, and the .shellcheckrc in .agents/scripts/
# now prevents the recursive source-chain expansion that caused the bloat.
# The auto-kill is a safety net for cases where ShellCheck is invoked without
# the .shellcheckrc (e.g., different source-path, --norc flag).
#
# kern.memorystatus_level is a secondary/informational signal only. macOS runs
# fine with compression + swap; aggressive thresholds on that metric cause false
# alarms. The primary signals are process-level.
Expand Down Expand Up @@ -35,6 +42,7 @@
# TOOL_RUNTIME_MAX Other tool max runtime in seconds (default: 1800)
# SESSION_COUNT_WARN Interactive session warning threshold (default: 5)
# AGGREGATE_RSS_WARN_MB Total aidevops RSS warning (default: 8192)
# AUTO_KILL_SHELLCHECK Auto-kill runaway ShellCheck (default: true)
# MEMORY_COOLDOWN_SECS Notification cooldown per category (default: 300)
# MEMORY_NOTIFY Set to "false" to disable notifications (log only)
# MEMORY_LOG_DIR Override log directory
Expand All @@ -44,7 +52,7 @@ set -euo pipefail
# --- Configuration -----------------------------------------------------------

readonly SCRIPT_NAME="memory-pressure-monitor"
readonly SCRIPT_VERSION="1.0.0"
readonly SCRIPT_VERSION="1.1.0"

# Per-process RSS thresholds (MB)
PROCESS_RSS_WARN_MB="${PROCESS_RSS_WARN_MB:-2048}"
Expand All @@ -58,6 +66,9 @@ TOOL_RUNTIME_MAX="${TOOL_RUNTIME_MAX:-1800}" # 30 min
SESSION_COUNT_WARN="${SESSION_COUNT_WARN:-5}"
AGGREGATE_RSS_WARN_MB="${AGGREGATE_RSS_WARN_MB:-8192}" # 8 GB total

# Auto-kill: ShellCheck processes are safe to kill (language server respawns them)
readonly AUTO_KILL_SHELLCHECK="${AUTO_KILL_SHELLCHECK:-true}"

# Notification
readonly COOLDOWN_SECS="${MEMORY_COOLDOWN_SECS:-300}"
readonly NOTIFY_ENABLED="${MEMORY_NOTIFY:-true}"
Expand Down Expand Up @@ -391,6 +402,36 @@ _get_os_memory_info() {
return 0
}

# --- Auto-Kill ----------------------------------------------------------------

# Kill a runaway process and log the action.
# Arguments: $1=PID, $2=reason (human-readable)
# Returns: 0 on success, 1 if process not found or kill failed
_auto_kill_process() {
local pid="$1"
local reason="$2"

# Verify process still exists before killing
if ! kill -0 "$pid" 2>/dev/null; then
log_msg "INFO" "Auto-kill: PID ${pid} already gone (${reason})"
return 1
fi

# SIGTERM first (graceful), then SIGKILL after 2 seconds if still alive
log_msg "CRITICAL" "Auto-kill: sending SIGTERM to PID ${pid} (${reason})"
kill -TERM "$pid" 2>/dev/null || true
sleep 2

if kill -0 "$pid" 2>/dev/null; then
log_msg "CRITICAL" "Auto-kill: PID ${pid} survived SIGTERM, sending SIGKILL"
kill -KILL "$pid" 2>/dev/null || true
fi

log_msg "CRITICAL" "Auto-kill: PID ${pid} terminated (${reason})"
notify "Process Killed" "ShellCheck PID ${pid} killed: ${reason}" "critical"
return 0
}

# --- Core Logic ---------------------------------------------------------------

# Evaluate all monitored processes and generate alerts
Expand Down Expand Up @@ -418,6 +459,10 @@ do_check() {
if [[ "$rss_mb" -ge "$PROCESS_RSS_CRIT_MB" ]]; then
findings+=("CRITICAL|rss|${pid}|${cmd_name} using ${rss_mb} MB RSS (limit: ${PROCESS_RSS_CRIT_MB} MB)")
has_critical=true
# Auto-kill ShellCheck at CRITICAL RSS — safe, language server respawns
if [[ "$cmd_name" == "shellcheck" && "$AUTO_KILL_SHELLCHECK" == "true" ]]; then
_auto_kill_process "$pid" "RSS ${rss_mb} MB exceeds ${PROCESS_RSS_CRIT_MB} MB limit"
fi
elif [[ "$rss_mb" -ge "$PROCESS_RSS_WARN_MB" ]]; then
findings+=("WARNING|rss|${pid}|${cmd_name} using ${rss_mb} MB RSS (limit: ${PROCESS_RSS_WARN_MB} MB)")
has_warning=true
Expand All @@ -436,6 +481,10 @@ do_check() {
limit_duration=$(_format_duration "$runtime_limit")
findings+=("WARNING|runtime|${pid}|${cmd_name} running for ${duration} (limit: ${limit_duration})")
has_warning=true
# Auto-kill ShellCheck exceeding runtime — stuck in source chain expansion
if [[ "$cmd_name" == "shellcheck" && "$AUTO_KILL_SHELLCHECK" == "true" ]]; then
_auto_kill_process "$pid" "runtime ${duration} exceeds ${limit_duration} limit"
fi
fi
done <<<"$processes"

Expand Down Expand Up @@ -606,6 +655,7 @@ cmd_status() {
echo " Tool runtime max: $(_format_duration "$TOOL_RUNTIME_MAX")"
echo " Session count warning: ${SESSION_COUNT_WARN}"
echo " Aggregate RSS warning: ${AGGREGATE_RSS_WARN_MB} MB"
echo " Auto-kill ShellCheck: ${AUTO_KILL_SHELLCHECK}"
echo " Notification cooldown: ${COOLDOWN_SECS}s"
echo " Notifications: ${NOTIFY_ENABLED}"

Expand Down Expand Up @@ -736,13 +786,19 @@ Process-level thresholds (primary):
Session count: warning >= ${SESSION_COUNT_WARN}
Aggregate RSS: warning >= ${AGGREGATE_RSS_WARN_MB}MB

Auto-kill (GH#2915):
ShellCheck processes are auto-killed when they hit CRITICAL RSS (>${PROCESS_RSS_CRIT_MB}MB)
or exceed runtime limit (>$(_format_duration "$SHELLCHECK_RUNTIME_MAX")).
Safe because the language server respawns them. Disable: AUTO_KILL_SHELLCHECK=false

Environment variables:
PROCESS_RSS_WARN_MB Per-process RSS warning (default: 2048)
PROCESS_RSS_CRIT_MB Per-process RSS critical (default: 4096)
SHELLCHECK_RUNTIME_MAX ShellCheck max runtime in seconds (default: 600)
TOOL_RUNTIME_MAX Other tool max runtime in seconds (default: 1800)
SESSION_COUNT_WARN Interactive session warning threshold (default: 5)
AGGREGATE_RSS_WARN_MB Total aidevops RSS warning (default: 8192)
AUTO_KILL_SHELLCHECK Auto-kill runaway ShellCheck (default: true)
MEMORY_COOLDOWN_SECS Notification cooldown per category (default: 300)
MEMORY_NOTIFY Set to "false" to disable notifications
MEMORY_LOG_DIR Override log directory
Expand Down
105 changes: 105 additions & 0 deletions .agents/scripts/shellcheck-wrapper.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#!/usr/bin/env bash
# Safe ShellCheck wrapper for language servers (shellcheck-wrapper.sh)
#
# The bash language server hardcodes --external-sources in every ShellCheck
# invocation (bash-language-server/out/shellcheck/index.js:82). Combined with
# --source-path pointing to a directory with 463+ cross-sourcing scripts, this
# causes exponential AST expansion (observed: 11 GB RSS, kernel panics).
#
# This wrapper strips --external-sources from the arguments before passing them
# to the real ShellCheck binary. It also enforces a memory limit via ulimit.
#
# Usage:
# Set SHELLCHECK_PATH to this script's path, or place it earlier on PATH as
# "shellcheck". The bash language server will use it instead of the real binary.
#
# Environment variables:
# SHELLCHECK_REAL_PATH — Path to the real shellcheck binary (auto-detected)
# SHELLCHECK_VMEM_MB — Virtual memory limit in MB (default: 2048)
#
# GH#2915: https://github.com/marcusquinn/aidevops/issues/2915

set -uo pipefail

# --- Find the real ShellCheck binary ---
_find_real_shellcheck() {
local real_path="${SHELLCHECK_REAL_PATH:-}"

if [[ -n "$real_path" && -x "$real_path" ]]; then
printf '%s' "$real_path"
return 0
fi

# Search PATH, skipping this wrapper script
local self
self="$(realpath "${BASH_SOURCE[0]}" 2>/dev/null || readlink -f "${BASH_SOURCE[0]}" 2>/dev/null || echo "${BASH_SOURCE[0]}")"

local IFS=':'

Check warning on line 37 in .agents/scripts/shellcheck-wrapper.sh

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

.agents/scripts/shellcheck-wrapper.sh#L37

The special variable IFS affects how splitting takes place when expanding unquoted variables.

Check warning on line 37 in .agents/scripts/shellcheck-wrapper.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove the unused local variable 'IFS'.

See more on https://sonarcloud.io/project/issues?id=marcusquinn_aidevops&issues=AZy-dLUwHcf8S1HlWEvM&open=AZy-dLUwHcf8S1HlWEvM&pullRequest=2918
local dir
for dir in $PATH; do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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

local candidate="${dir}/shellcheck"
if [[ -x "$candidate" ]]; then
local resolved
resolved="$(realpath "$candidate" 2>/dev/null || readlink -f "$candidate" 2>/dev/null || echo "$candidate")"
if [[ "$resolved" != "$self" ]]; then
printf '%s' "$candidate"
return 0
fi
fi
done

# Common locations
local loc
for loc in /opt/homebrew/bin/shellcheck /usr/local/bin/shellcheck /usr/bin/shellcheck; do
if [[ -x "$loc" ]]; then
local resolved
resolved="$(realpath "$loc" 2>/dev/null || readlink -f "$loc" 2>/dev/null || echo "$loc")"
if [[ "$resolved" != "$self" ]]; then
printf '%s' "$loc"
return 0
fi
fi
done

echo "shellcheck-wrapper: ERROR: cannot find real shellcheck binary" >&2
return 1
}

# --- Filter arguments ---
_filter_args() {
local args=()
while [[ $# -gt 0 ]]; do
case "$1" in
--external-sources | -x)
# Strip this flag — it causes unbounded source chain expansion
;;
*)
args+=("$1")
;;
esac
shift
done
printf '%s\n' "${args[@]}"
}

# --- Main ---
main() {
local real_shellcheck
real_shellcheck="$(_find_real_shellcheck)" || exit 1

# Read filtered args into array
local filtered_args=()
while IFS= read -r arg; do
filtered_args+=("$arg")
done < <(_filter_args "$@")
Comment on lines +91 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-medium medium

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.

Suggested change
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


# Enforce memory limit (soft limit — ShellCheck can still be killed by the
# memory pressure monitor if it exceeds this, but this prevents the worst case)
local vmem_mb="${SHELLCHECK_VMEM_MB:-2048}"
local vmem_kb=$((vmem_mb * 1024))
ulimit -v "$vmem_kb" 2>/dev/null || true

exec "$real_shellcheck" "${filtered_args[@]}"
}

main "$@"
80 changes: 80 additions & 0 deletions setup-modules/shell-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,86 @@ add_local_bin_to_path() {
return 0
}

# GH#2915: Configure SHELLCHECK_PATH to use the safe wrapper that strips
# --external-sources. The bash language server hardcodes --external-sources
# in every ShellCheck invocation, causing exponential memory growth (11 GB+)
# when source chains span 463+ scripts. The wrapper intercepts this.
#
# Uses launchctl setenv (macOS) for GUI-launched apps + shell rc for terminals.
# This ensures all processes — regardless of shell — see the wrapper.
setup_shellcheck_wrapper() {
local wrapper_path="$HOME/.aidevops/agents/scripts/shellcheck-wrapper.sh"

# Verify the wrapper exists and is executable
if [[ ! -x "$wrapper_path" ]]; then
if [[ -f "$wrapper_path" ]]; then
chmod +x "$wrapper_path"
else
print_warning "ShellCheck wrapper not found at $wrapper_path (will be available after deploy)"
return 0
fi
fi

# Verify the wrapper actually works (can find real shellcheck)
if ! "$wrapper_path" --version >/dev/null 2>&1; then
print_warning "ShellCheck wrapper cannot find real shellcheck binary — skipping"
return 0
fi

local env_line
# shellcheck disable=SC2016 # env_line is written to rc files; must expand at shell startup
env_line='export SHELLCHECK_PATH="$HOME/.aidevops/agents/scripts/shellcheck-wrapper.sh"'
local added_to=""
local already_in=""

# Layer 1: launchctl setenv (macOS) — affects all GUI-launched processes
if [[ "$PLATFORM_MACOS" == "true" ]]; then
if launchctl setenv SHELLCHECK_PATH "$wrapper_path" 2>/dev/null; then
print_info "Set SHELLCHECK_PATH via launchctl (GUI processes)"
fi
Comment on lines +495 to +497

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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
  1. 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.

fi

# Layer 2: Shell rc files — affects terminal sessions
local rc_file
while IFS= read -r rc_file; do
[[ -z "$rc_file" ]] && continue

if [[ ! -f "$rc_file" ]]; then
mkdir -p "$(dirname "$rc_file")"
touch "$rc_file"
fi

# Check if already added
if grep -q 'SHELLCHECK_PATH' "$rc_file" 2>/dev/null; then
already_in="${already_in:+$already_in, }$rc_file"
continue
fi

echo "" >>"$rc_file"
echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file"
echo "$env_line" >>"$rc_file"
added_to="${added_to:+$added_to, }$rc_file"
done < <(get_all_shell_rcs)

if [[ -n "$added_to" ]]; then
print_success "Configured SHELLCHECK_PATH wrapper in: $added_to"
fi

if [[ -n "$already_in" ]]; then
print_info "SHELLCHECK_PATH already configured in: $already_in"
fi

if [[ -z "$added_to" && -z "$already_in" && "$PLATFORM_MACOS" != "true" ]]; then
print_warning "Could not configure SHELLCHECK_PATH automatically"
print_info "Add this to your shell config: $env_line"
fi

# Also export for current session
export SHELLCHECK_PATH="$wrapper_path"

return 0
}

setup_aliases() {
print_info "Setting up shell aliases..."

Expand Down
2 changes: 2 additions & 0 deletions setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ main() {
validate_opencode_config
deploy_aidevops_agents
sync_agent_sources
setup_shellcheck_wrapper
if is_feature_enabled safety_hooks 2>/dev/null; then
setup_safety_hooks
fi
Expand Down Expand Up @@ -662,6 +663,7 @@ main() {
confirm_step "Check OpenCode prompt drift" && check_opencode_prompt_drift
confirm_step "Deploy aidevops agents to ~/.aidevops/agents/" && deploy_aidevops_agents
confirm_step "Sync agents from private repositories" && sync_agent_sources
setup_shellcheck_wrapper
confirm_step "Install Claude Code safety hooks (block destructive commands)" && setup_safety_hooks
confirm_step "Initialize settings.json (canonical config file)" && init_settings_json
confirm_step "Setup multi-tenant credential storage" && setup_multi_tenant_credentials
Expand Down
Loading