Skip to content

t2950: Fix quality-debt in setup.sh from PR #2936 review feedback #2994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 3 commits into from
Mar 6, 2026
Merged

t2950: Fix quality-debt in setup.sh from PR #2936 review feedback #2994

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dad309d
fix: t2950 remove error suppression from consent persistence and quot…
marcusquinn Mar 6, 2026
e633c8f
fix: address CodeRabbit CHANGES_REQUESTED review feedback on PR #2994
marcusquinn Mar 6, 2026
d420988
fix: resolve merge conflict — adopt main's _cron_escape, keep awk pip…
marcusquinn Mar 6, 2026
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
8 changes: 4 additions & 4 deletions setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ main() {
# Also check legacy .conf user override
if [[ -z "$_pulse_user_config" && -f "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" ]]; then
local _legacy_val
_legacy_val=$(grep -E '^supervisor_pulse=' "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" 2>/dev/null | tail -1 | cut -d= -f2)
_legacy_val=$(grep -E '^supervisor_pulse=' "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" | tail -1 | cut -d= -f2)
if [[ -n "$_legacy_val" ]]; then
_pulse_user_config="$_legacy_val"
fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -812,13 +812,13 @@ main() {
_do_install=true
# Record explicit consent
if type cmd_set &>/dev/null; then
cmd_set "orchestration.supervisor_pulse" "true" 2>/dev/null || true
cmd_set "orchestration.supervisor_pulse" "true" || true
fi
else
_do_install=false
# Record explicit decline so we never re-prompt on updates
if type cmd_set &>/dev/null; then
cmd_set "orchestration.supervisor_pulse" "false" 2>/dev/null || true
cmd_set "orchestration.supervisor_pulse" "false" || true
fi
print_info "Skipped. Enable later: aidevops config set orchestration.supervisor_pulse true && ./setup.sh"
fi
Expand Down Expand Up @@ -921,7 +921,7 @@ PLIST
# Remove old-style cron entries (direct opencode invocation)
(
crontab -l 2>/dev/null | grep -v 'aidevops: supervisor-pulse'
echo "*/2 * * * * OPENCODE_BIN=${opencode_bin} PULSE_DIR=${_aidevops_dir} /bin/bash ${wrapper_script} >> $HOME/.aidevops/logs/pulse-wrapper.log 2>&1 # aidevops: supervisor-pulse"
echo "*/2 * * * * OPENCODE_BIN='${opencode_bin}' PULSE_DIR='${_aidevops_dir}' /bin/bash '${wrapper_script}' >> '$HOME/.aidevops/logs/pulse-wrapper.log' 2>&1 # aidevops: supervisor-pulse"

@gemini-code-assist gemini-code-assist Bot Mar 6, 2026

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

While single-quoting variables is a good improvement to handle paths with spaces, this is still vulnerable if any of the path variables contain a single quote. This could lead to parsing issues or command injection in the crontab.

To make this fully robust, you should also escape any single quotes within the variables. The standard way to do this in shell is to replace each ' with '\''.

Suggested change
echo "*/2 * * * * OPENCODE_BIN='${opencode_bin}' PULSE_DIR='${_aidevops_dir}' /bin/bash '${wrapper_script}' >> '$HOME/.aidevops/logs/pulse-wrapper.log' 2>&1 # aidevops: supervisor-pulse"
echo "*/2 * * * * OPENCODE_BIN='${opencode_bin//\'/'\''}' PULSE_DIR='${_aidevops_dir//\'/'\''}' /bin/bash '${wrapper_script//\'/'\''}' >> '${HOME//\'/'\''}/.aidevops/logs/pulse-wrapper.log' 2>&1 # aidevops: supervisor-pulse"

) | crontab - 2>/dev/null || true
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if crontab -l 2>/dev/null | grep -qF "aidevops: supervisor-pulse"; then
print_info "Supervisor pulse enabled (cron, every 2 min). Disable: crontab -e and remove the supervisor-pulse line"
Expand Down
Loading