Skip to content

t3657: address PR #1688 review feedback — path resolution, sqlite busy_timeout, shared token extraction #4706

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
alex-solovyev merged 1 commit into from
Mar 14, 2026
Merged

t3657: address PR #1688 review feedback — path resolution, sqlite busy_timeout, shared token extraction #4706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .agents/scripts/archived/pattern-tracker-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ cmd_record() {
[[ -n "$tokens_out" ]] && sql_tokens_out="$tokens_out"
[[ -n "$estimated_cost" ]] && sql_estimated_cost="$estimated_cost"

sqlite3 "$MEMORY_DB" "INSERT OR REPLACE INTO pattern_metadata (id, strategy, quality, failure_mode, tokens_in, tokens_out, estimated_cost) VALUES ('$mem_id', '$sql_strategy', $sql_quality, $sql_failure_mode, $sql_tokens_in, $sql_tokens_out, $sql_estimated_cost);" 2>/dev/null || log_warn "Failed to store pattern metadata for $mem_id"
sqlite3 -cmd ".timeout 5000" "$MEMORY_DB" "INSERT OR REPLACE INTO pattern_metadata (id, strategy, quality, failure_mode, tokens_in, tokens_out, estimated_cost) VALUES ('$mem_id', '$sql_strategy', $sql_quality, $sql_failure_mode, $sql_tokens_in, $sql_tokens_out, $sql_estimated_cost);" 2>/dev/null || log_warn "Failed to store pattern metadata for $mem_id"
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 14, 2026

Choose a reason for hiding this comment

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

security-critical critical

This line is vulnerable to SQL injection. The variables $mem_id and $sql_strategy are directly embedded into the SQL query string. If their values contain a single quote, an attacker could break out of the string and execute arbitrary SQL commands, which is a critical security risk.

For sqlite3 in shell scripts, the most robust way to prevent SQL injection is to use parameterized queries with .param set within a helper function. This separates the SQL command from the data, eliminating injection risks. As an immediate fix for this line, you must escape single quotes within these variables before they are used in the query. The standard way to escape a single quote in SQL is to double it (''). You can achieve this in bash using parameter expansion: ${variable//'/'/''}.

Suggested change
sqlite3 -cmd ".timeout 5000" "$MEMORY_DB" "INSERT OR REPLACE INTO pattern_metadata (id, strategy, quality, failure_mode, tokens_in, tokens_out, estimated_cost) VALUES ('$mem_id', '$sql_strategy', $sql_quality, $sql_failure_mode, $sql_tokens_in, $sql_tokens_out, $sql_estimated_cost);" 2>/dev/null || log_warn "Failed to store pattern metadata for $mem_id"
sqlite3 -cmd ".timeout 5000" "$MEMORY_DB" "INSERT OR REPLACE INTO pattern_metadata (id, strategy, quality, failure_mode, tokens_in, tokens_out, estimated_cost) VALUES ('${mem_id//'/'/''}', '${sql_strategy//'/'/''}', $sql_quality, $sql_failure_mode, $sql_tokens_in, $sql_tokens_out, $sql_estimated_cost);" 2>/dev/null || log_warn "Failed to store pattern metadata for $mem_id"
References
  1. Employ a defense-in-depth strategy for handling user-provided input. Sanitize input at the entry point using a strict allowlist, and also apply context-specific escaping or safe handling mechanisms (e.g., parameterized queries for SQL, --arg for jq) at each point of use.
  2. To prevent SQL injection in shell scripts using sqlite3, create a helper function that uses .param set for safe parameterized bindings instead of direct string interpolation.

fi

log_success "Recorded $outcome pattern: $description"
Expand Down
39 changes: 39 additions & 0 deletions .agents/scripts/supervisor-archived/_common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,42 @@ portable_timeout() {
timeout_sec "$@"
return $?
}

#######################################
# Extract token counts from a worker log file (t1114)
# Supports camelCase (opencode JSON) and snake_case (claude CLI JSON) formats.
# Results are stored in module-level globals _EXTRACT_TOKENS_IN and
# _EXTRACT_TOKENS_OUT. Callers copy these into their own local variables.
#
# Usage:
# local tokens_in="" tokens_out=""
# extract_tokens_from_log "$log_file"
# tokens_in="$_EXTRACT_TOKENS_IN"
# tokens_out="$_EXTRACT_TOKENS_OUT"
#
# $1: log_file path (may be empty or non-existent — handled gracefully)
#######################################
_EXTRACT_TOKENS_IN=""
_EXTRACT_TOKENS_OUT=""
extract_tokens_from_log() {
local log_file="$1"
_EXTRACT_TOKENS_IN=""
_EXTRACT_TOKENS_OUT=""

if [[ -z "$log_file" || ! -f "$log_file" ]]; then
return 0
fi

local raw_in raw_out
raw_in=$(grep -oE '"inputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
raw_out=$(grep -oE '"outputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
if [[ -z "$raw_in" ]]; then
raw_in=$(grep -oE '"input_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
if [[ -z "$raw_out" ]]; then
raw_out=$(grep -oE '"output_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
Comment on lines +250 to +258
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 14, 2026

Choose a reason for hiding this comment

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

medium

The current implementation for extracting token counts can be inefficient, as it may read the log file from disk up to four separate times. For large log files, this can lead to a noticeable performance degradation.

You can significantly improve efficiency by reading the file only once to gather all potential matches. Furthermore, instead of repeated grep | cut pipelines, consider using a single awk command or a while IFS='=' read -r key value loop to parse the extracted matches, which is more efficient and readable for processing multiple key-value pairs from a single source. Also, the initial grep | sed pipeline can be consolidated into a single sed command for further optimization.

Suggested change
local raw_in raw_out
raw_in=$(grep -oE '"inputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
raw_out=$(grep -oE '"outputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
if [[ -z "$raw_in" ]]; then
raw_in=$(grep -oE '"input_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
if [[ -z "$raw_out" ]]; then
raw_out=$(grep -oE '"output_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
local matches raw_in raw_out
matches=$(sed -nE 's/.*"(inputTokens|input_tokens|outputTokens|output_tokens)":([0-9]+).*/\1:\2/p' "$log_file" 2>/dev/null || true)
if [[ -n "$matches" ]]; then
read -r raw_in raw_out <<< "$(echo "$matches" | awk -F: '
/^(inputTokens|input_tokens):/ { last_in=$2 }
/^(outputTokens|output_tokens):/ { last_out=$2 }
END { print last_in, last_out }
')"
fi
References
  1. Optimize shell script pipelines by replacing 'grep | sed' combinations with a single, more efficient 'sed' command where possible to improve performance.
  2. When parsing multiple key-value pairs from a single source in shell scripts, use a single 'while IFS='=' read -r key value' loop instead of repeated 'grep | cut' pipelines to improve efficiency and readability.

[[ -n "$raw_in" ]] && _EXTRACT_TOKENS_IN="$raw_in"
[[ -n "$raw_out" ]] && _EXTRACT_TOKENS_OUT="$raw_out"
return 0
}
23 changes: 8 additions & 15 deletions .agents/scripts/supervisor-archived/evaluate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,10 @@ record_evaluation_metadata() {
local quality_score="${5:-0}"
local ai_evaluated="${6:-false}"

local pattern_helper="${SCRIPT_DIR}/pattern-tracker-helper.sh"
local pattern_helper="${SCRIPT_DIR}/../pattern-tracker-helper.sh"
if [[ ! -x "$pattern_helper" ]]; then
pattern_helper="${SCRIPT_DIR}/pattern-tracker-helper.sh"
fi
if [[ ! -x "$pattern_helper" ]]; then
pattern_helper="$HOME/.aidevops/agents/scripts/pattern-tracker-helper.sh"
fi
Expand Down Expand Up @@ -847,21 +850,11 @@ record_evaluation_metadata() {
fi

# Extract token counts from worker log for cost tracking (t1114, t1117)
# Supports camelCase (opencode JSON) and snake_case (claude CLI JSON) formats.
# Shared extraction logic lives in supervisor-archived/_common.sh (extract_tokens_from_log).
local tokens_in="" tokens_out=""
if [[ -n "$task_log_file" && -f "$task_log_file" ]]; then
local raw_in raw_out
raw_in=$(grep -oE '"inputTokens":[0-9]+' "$task_log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
raw_out=$(grep -oE '"outputTokens":[0-9]+' "$task_log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
if [[ -z "$raw_in" ]]; then
raw_in=$(grep -oE '"input_tokens":[0-9]+' "$task_log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
if [[ -z "$raw_out" ]]; then
raw_out=$(grep -oE '"output_tokens":[0-9]+' "$task_log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
[[ -n "$raw_in" ]] && tokens_in="$raw_in"
[[ -n "$raw_out" ]] && tokens_out="$raw_out"
fi
extract_tokens_from_log "$task_log_file"
tokens_in="$_EXTRACT_TOKENS_IN"
tokens_out="$_EXTRACT_TOKENS_OUT"

# Look up task type from DB tags if available, fallback to "unknown"
# TODO(t1096): extract real task type from TODO.md tags or DB metadata
Expand Down
22 changes: 8 additions & 14 deletions .agents/scripts/supervisor-archived/memory-integration.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,20 +227,11 @@ store_success_pattern() {

# Extract token counts from worker log for cost tracking (t1114)
# opencode/claude --format json logs emit usage stats in the final JSON entry.
# Shared extraction logic lives in supervisor-archived/_common.sh (extract_tokens_from_log).
local tokens_in="" tokens_out=""
if [[ -n "$log_file" && -f "$log_file" ]]; then
local raw_in raw_out
raw_in=$(grep -oE '"inputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
raw_out=$(grep -oE '"outputTokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
if [[ -z "$raw_in" ]]; then
raw_in=$(grep -oE '"input_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
if [[ -z "$raw_out" ]]; then
raw_out=$(grep -oE '"output_tokens":[0-9]+' "$log_file" 2>/dev/null | tail -1 | grep -oE '[0-9]+' || true)
fi
[[ -n "$raw_in" ]] && tokens_in="$raw_in"
[[ -n "$raw_out" ]] && tokens_out="$raw_out"
fi
extract_tokens_from_log "$log_file"
tokens_in="$_EXTRACT_TOKENS_IN"
tokens_out="$_EXTRACT_TOKENS_OUT"

# Build tags with model and duration info for pattern-tracker queries
local tags="supervisor,pattern,$task_id,complete"
Expand All @@ -252,7 +243,10 @@ store_success_pattern() {
tags="$tags,quality:${quality_score},failure_mode:NONE"

# Use pattern-tracker-helper.sh directly when available for richer metadata (t1114)
local pattern_helper="${SCRIPT_DIR}/pattern-tracker-helper.sh"
local pattern_helper="${SCRIPT_DIR}/../pattern-tracker-helper.sh"
if [[ ! -x "$pattern_helper" ]]; then
pattern_helper="${SCRIPT_DIR}/pattern-tracker-helper.sh"
fi
if [[ ! -x "$pattern_helper" ]]; then
pattern_helper="$HOME/.aidevops/agents/scripts/pattern-tracker-helper.sh"
fi
Expand Down
Loading