diff --git a/.agents/scripts/ai-judgment-helper.sh b/.agents/scripts/ai-judgment-helper.sh index 814d57e69..722d36109 100755 --- a/.agents/scripts/ai-judgment-helper.sh +++ b/.agents/scripts/ai-judgment-helper.sh @@ -905,9 +905,10 @@ ${user_message}" if [[ -n "$score" ]]; then # Determine pass/fail using awk for float comparison local passed - passed=$(awk "BEGIN { print ($score >= $threshold) ? \"true\" : \"false\" }") + passed=$(awk -v s="$score" -v t="$threshold" 'BEGIN { print (s >= t) ? "true" : "false" }') - local result_json="{\"evaluator\": \"${eval_type}\", \"score\": ${score}, \"passed\": ${passed}, \"details\": \"${details}\"}" + local result_json + result_json=$(jq -cn --arg type "$eval_type" --argjson score "${score:-null}" --argjson passed "$passed" --arg details "$details" '{evaluator: $type, score: $score, passed: $passed, details: $details}') # Cache the result cache_judgment "$cache_key" "$result_json" "" "haiku" @@ -919,7 +920,8 @@ ${user_message}" fi # Deterministic fallback: API unavailable - local fallback_json="{\"evaluator\": \"${eval_type}\", \"score\": null, \"passed\": null, \"details\": \"API unavailable, using fallback\"}" + local fallback_json + fallback_json=$(jq -cn --arg type "$eval_type" '{evaluator: $type, score: null, passed: null, details: "API unavailable, using fallback"}') echo "$fallback_json" return 0 } diff --git a/.agents/scripts/stuck-detection-helper.sh b/.agents/scripts/stuck-detection-helper.sh index f2e461c33..faa34a38c 100755 --- a/.agents/scripts/stuck-detection-helper.sh +++ b/.agents/scripts/stuck-detection-helper.sh @@ -147,7 +147,7 @@ _sd_write_state() { # Atomic write via temp file + mv local tmp_file="${state_file}.tmp.$$" - if ! printf '%s\n' "$state_json" >"$tmp_file" 2>/dev/null; then + if ! printf '%s\n' "$state_json" >"$tmp_file"; then _sd_log_warn "failed to write temp state file: $tmp_file" rm -f "$tmp_file" 2>/dev/null || true return 1 @@ -261,14 +261,32 @@ cmd_label_stuck() { local suggested_actions="$6" local repo_slug="${7:-}" - if [[ -z "$issue_number" || -z "$milestone_min" || -z "$confidence" ]]; then + if [[ -z "$issue_number" || -z "$milestone_min" || -z "$elapsed_min" || -z "$confidence" ]]; then _sd_log_error "usage: label-stuck [--repo ]" return 1 fi + # Validate numeric parameters before any side effects (GitHub ops, state mutations). + # milestone_min/elapsed_min are integers used in jq --argjson and comment interpolation. + # confidence is a float used in awk comparison — non-numeric strings cause + # lexicographic semantics (e.g., "high" >= "0.7" is true). + if ! [[ "$milestone_min" =~ ^[0-9]+$ ]] || ! [[ "$elapsed_min" =~ ^[0-9]+$ ]]; then + _sd_log_error "milestone_min and elapsed_min must be positive integers (got milestone=${milestone_min}, elapsed=${elapsed_min})" + return 1 + fi + if ! [[ "$confidence" =~ ^[0-9]*\.?[0-9]+$ ]]; then + _sd_log_error "confidence must be a number, got: ${confidence}" + return 1 + fi + # Check confidence threshold local above_threshold - above_threshold=$(awk "BEGIN { print ($confidence >= $STUCK_CONFIDENCE_THRESHOLD) ? 1 : 0 }" 2>/dev/null) || above_threshold="0" + if ! [[ "$confidence" =~ ^([0-9]+([.][0-9]+)?|[.][0-9]+)$ ]] || + ! [[ "$STUCK_CONFIDENCE_THRESHOLD" =~ ^([0-9]+([.][0-9]+)?|[.][0-9]+)$ ]]; then + _sd_log_error "confidence values must be numeric (got confidence=${confidence}, threshold=${STUCK_CONFIDENCE_THRESHOLD})" + return 1 + fi + above_threshold=$(awk -v c="$confidence" -v t="$STUCK_CONFIDENCE_THRESHOLD" 'BEGIN { print (c >= t) ? 1 : 0 }') || above_threshold="0" if [[ "$above_threshold" -ne 1 ]]; then _sd_log_info "confidence $confidence below threshold $STUCK_CONFIDENCE_THRESHOLD for issue #$issue_number — not labeling" @@ -307,7 +325,7 @@ cmd_label_stuck() { # Apply label gh issue edit "$issue_number" --repo "$repo_slug" \ - --add-label "$STUCK_LABEL" 2>/dev/null || { + --add-label "$STUCK_LABEL" || { _sd_log_warn "failed to add label to issue #$issue_number" return 1 } @@ -330,12 +348,15 @@ ${suggested_actions} --- *This is an advisory notification only. No automated action has been taken. The worker continues running. The \`${STUCK_LABEL}\` label will be automatically removed if the task completes successfully.*" + local comment_failed=0 gh issue comment "$issue_number" --repo "$repo_slug" \ - --body "$comment_body" 2>/dev/null || { + --body "$comment_body" || { _sd_log_warn "failed to comment on issue #$issue_number" + comment_failed=1 } - # Record milestone and labeled issue in state + # Record milestone and labeled issue in state (regardless of comment success, + # since the label was applied — skipping state would cause re-labeling). _sd_record_milestone "$issue_number" "$milestone_min" "$repo_slug" || true local state @@ -345,13 +366,13 @@ ${suggested_actions} --arg issue "$issue_number" \ --arg repo "$repo_slug" \ --arg now "$now" \ - '.labeled_issues = ((.labeled_issues // []) + [{"issue": $issue, "repo": $repo, "labeled_at": $now}] | unique_by(.issue + .repo))') || true + '.labeled_issues = ((.labeled_issues // []) + [{"issue": $issue, "repo": $repo, "labeled_at": $now}] | unique_by([.issue, .repo]))') || true if [[ -n "$new_state" ]]; then _sd_write_state "$new_state" || true fi _sd_log_warn "labeled issue #$issue_number as stuck (confidence: $confidence, milestone: ${milestone_min}min)" - return 0 + return "$comment_failed" } # Remove stuck-detection label from a GitHub issue on task success. @@ -394,7 +415,7 @@ cmd_label_clear() { # Remove the label gh issue edit "$issue_number" --repo "$repo_slug" \ - --remove-label "$STUCK_LABEL" 2>/dev/null || { + --remove-label "$STUCK_LABEL" || { _sd_log_warn "failed to remove label from issue #$issue_number" return 1 } @@ -416,7 +437,8 @@ cmd_label_clear() { new_state=$(printf '%s' "$state" | jq \ --arg key "$issue_key" \ --arg issue "$issue_number" \ - 'del(.milestones_checked[$key]) | .labeled_issues = [.labeled_issues[] | select(.issue != $issue)]') || true + --arg repo "$repo_slug" \ + 'del(.milestones_checked[$key]) | .labeled_issues = [(.labeled_issues // [])[] | select((.issue != $issue) or (.repo != $repo))]') || true if [[ -n "$new_state" ]]; then _sd_write_state "$new_state" || true fi diff --git a/.agents/scripts/worker-token-helper.sh b/.agents/scripts/worker-token-helper.sh index f004f5ac3..25412f13a 100755 --- a/.agents/scripts/worker-token-helper.sh +++ b/.agents/scripts/worker-token-helper.sh @@ -367,6 +367,11 @@ cmd_create() { ;; --ttl | -t) ttl="$2" + # Validate TTL is numeric to prevent arithmetic injection + if ! [[ "$ttl" =~ ^[0-9]+$ ]]; then + log_token "ERROR" "TTL must be a positive integer: ${ttl}" + return 1 + fi if ((ttl > MAX_TTL)); then log_token "WARN" "TTL capped at ${MAX_TTL}s (requested ${ttl}s)" ttl=$MAX_TTL @@ -398,14 +403,14 @@ cmd_create() { # Strategy 1: GitHub App installation token (best — enforced by GitHub) local token_file - token_file=$(create_app_token "$repo" "$permissions" "$ttl" 2>/dev/null) && { + token_file=$(create_app_token "$repo" "$permissions" "$ttl") && { log_token "INFO" "Strategy: GitHub App installation token (enforced scoping)" printf '%s' "$token_file" return 0 } # Strategy 2: Delegated token (fallback — advisory scoping) - token_file=$(create_delegated_token "$repo" "$permissions" "$ttl" 2>/dev/null) && { + token_file=$(create_delegated_token "$repo" "$permissions" "$ttl") && { log_token "INFO" "Strategy: Delegated token (advisory scoping)" printf '%s' "$token_file" return 0 @@ -437,6 +442,22 @@ cmd_validate() { return 1 fi + # Validate token file path is within TOKEN_DIR to prevent path traversal + # Canonicalize both paths with realpath to handle symlinked home directories + local real_path token_dir_real + token_dir_real=$(realpath "$TOKEN_DIR" 2>/dev/null) || { + log_token "ERROR" "Cannot resolve token directory: ${TOKEN_DIR}" + return 1 + } + real_path=$(realpath "$token_file" 2>/dev/null) || { + log_token "ERROR" "Cannot resolve token file path: ${token_file}" + return 1 + } + if [[ "$real_path" != "${token_dir_real}/"* ]]; then + log_token "ERROR" "Token file must be within ${TOKEN_DIR}: ${token_file}" + return 1 + fi + if [[ ! -f "$token_file" ]]; then log_token "ERROR" "Token file not found: ${token_file}" return 1 @@ -505,6 +526,24 @@ cmd_revoke() { return 1 fi + # Validate token file path is within TOKEN_DIR to prevent path traversal + # Resolve parent directory instead of the file itself — the token file may + # already be deleted (only .meta remains), and realpath fails on missing files. + local token_dir_real token_parent_real token_base + token_dir_real=$(realpath "$TOKEN_DIR" 2>/dev/null) || { + log_token "ERROR" "Cannot resolve token directory: ${TOKEN_DIR}" + return 1 + } + token_parent_real=$(realpath "$(dirname "$token_file")" 2>/dev/null) || { + log_token "ERROR" "Cannot resolve token file path: ${token_file}" + return 1 + } + token_base=$(basename "$token_file") + if [[ "$token_parent_real" != "$token_dir_real" || "$token_base" != *.token ]]; then + log_token "ERROR" "Token file must be within ${TOKEN_DIR}: ${token_file}" + return 1 + fi + local meta_file="${token_file%.token}.meta" local strategy="" local repo=""