Browse Source

experiment: Add SQL injection protection and input validation

- sql_escape() function for all user inputs
- Numeric validation for message IDs, limits, days
- Empty body rejection
- 10 new test cases (T28-T37)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0xDarkMatter 1 week ago
parent
commit
6c3be7ec2a
3 changed files with 91 additions and 7 deletions
  1. 1 1
      hooks/check-mail.sh
  2. 32 6
      skills/agentmail/scripts/mail-db.sh
  3. 58 0
      skills/agentmail/scripts/test-mail.sh

+ 1 - 1
hooks/check-mail.sh

@@ -19,7 +19,7 @@ MAIL_DB="$HOME/.claude/mail.db"
 # Skip if no database exists yet
 [ -f "$MAIL_DB" ] || exit 0
 
-PROJECT=$(basename "$PWD")
+PROJECT=$(basename "$PWD" | sed "s/'/''/g")
 
 # Single fast query - count unread
 UNREAD=$(sqlite3 "$MAIL_DB" "SELECT COUNT(*) FROM messages WHERE to_project='${PROJECT}' AND read=0;" 2>/dev/null)

+ 32 - 6
skills/agentmail/scripts/mail-db.sh

@@ -25,6 +25,11 @@ CREATE INDEX IF NOT EXISTS idx_timestamp ON messages(timestamp);
 SQL
 }
 
+# Sanitize string for safe SQL interpolation (escape single quotes)
+sql_escape() {
+  printf '%s' "$1" | sed "s/'/''/g"
+}
+
 # Get project name from cwd
 get_project() {
   basename "$PWD"
@@ -34,7 +39,7 @@ get_project() {
 count_unread() {
   init_db
   local project
-  project=$(get_project)
+  project=$(sql_escape "$(get_project)")
   sqlite3 "$MAIL_DB" "SELECT COUNT(*) FROM messages WHERE to_project='${project}' AND read=0;"
 }
 
@@ -42,7 +47,7 @@ count_unread() {
 list_unread() {
   init_db
   local project
-  project=$(get_project)
+  project=$(sql_escape "$(get_project)")
   sqlite3 -separator ' | ' "$MAIL_DB" \
     "SELECT id, from_project, subject, timestamp FROM messages WHERE to_project='${project}' AND read=0 ORDER BY timestamp DESC;"
 }
@@ -51,7 +56,7 @@ list_unread() {
 read_mail() {
   init_db
   local project
-  project=$(get_project)
+  project=$(sql_escape "$(get_project)")
   sqlite3 -header -separator ' | ' "$MAIL_DB" \
     "SELECT id, from_project, subject, body, timestamp FROM messages WHERE to_project='${project}' AND read=0 ORDER BY timestamp ASC;"
   sqlite3 "$MAIL_DB" \
@@ -61,6 +66,11 @@ read_mail() {
 # Read a single message by ID and mark as read
 read_one() {
   local msg_id="$1"
+  # Validate ID is numeric
+  if ! [[ "$msg_id" =~ ^[0-9]+$ ]]; then
+    echo "Error: message ID must be numeric" >&2
+    return 1
+  fi
   init_db
   sqlite3 -header -separator ' | ' "$MAIL_DB" \
     "SELECT id, from_project, to_project, subject, body, timestamp FROM messages WHERE id=${msg_id};"
@@ -73,11 +83,19 @@ send() {
   local to_project="$1"
   local subject="$2"
   local body="$3"
+  if [ -z "$body" ]; then
+    echo "Error: message body cannot be empty" >&2
+    return 1
+  fi
   init_db
   local from_project
-  from_project=$(get_project)
+  from_project=$(sql_escape "$(get_project)")
+  local safe_to safe_subject safe_body
+  safe_to=$(sql_escape "$to_project")
+  safe_subject=$(sql_escape "$subject")
+  safe_body=$(sql_escape "$body")
   sqlite3 "$MAIL_DB" \
-    "INSERT INTO messages (from_project, to_project, subject, body) VALUES ('${from_project}', '${to_project}', '${subject}', '${body}');"
+    "INSERT INTO messages (from_project, to_project, subject, body) VALUES ('${from_project}', '${safe_to}', '${safe_subject}', '${safe_body}');"
   echo "Sent to ${to_project}: ${subject}"
 }
 
@@ -85,8 +103,12 @@ send() {
 list_all() {
   init_db
   local project
-  project=$(get_project)
+  project=$(sql_escape "$(get_project)")
   local limit="${1:-20}"
+  # Validate limit is numeric
+  if ! [[ "$limit" =~ ^[0-9]+$ ]]; then
+    limit=20
+  fi
   sqlite3 -header -separator ' | ' "$MAIL_DB" \
     "SELECT id, from_project, subject, CASE WHEN read=0 THEN 'UNREAD' ELSE 'read' END as status, timestamp FROM messages WHERE to_project='${project}' ORDER BY timestamp DESC LIMIT ${limit};"
 }
@@ -95,6 +117,10 @@ list_all() {
 clear_old() {
   init_db
   local days="${1:-7}"
+  # Validate days is numeric
+  if ! [[ "$days" =~ ^[0-9]+$ ]]; then
+    days=7
+  fi
   local deleted
   deleted=$(sqlite3 "$MAIL_DB" \
     "DELETE FROM messages WHERE read=1 AND timestamp < datetime('now', '-${days} days'); SELECT changes();")

+ 58 - 0
skills/agentmail/scripts/test-mail.sh

@@ -262,6 +262,64 @@ exit_code=$?
 assert_exit_code "unknown command fails" "1" "$exit_code"
 
 echo ""
+echo "=== Input Validation ==="
+
+# T28: Non-numeric message ID rejected
+result=$(bash "$MAIL_SCRIPT" read "abc" 2>&1)
+exit_code=$?
+assert_exit_code "non-numeric ID rejected" "1" "$exit_code"
+
+# T29: SQL injection via message ID
+bash "$MAIL_SCRIPT" send "claude-mods" "id-inject-test" "before injection" >/dev/null 2>&1
+result=$(bash "$MAIL_SCRIPT" read "1 OR 1=1" 2>&1)
+exit_code=$?
+assert_exit_code "SQL injection via ID rejected" "1" "$exit_code"
+
+# T30: Non-numeric limit in list
+result=$(bash "$MAIL_SCRIPT" list "abc" 2>&1)
+exit_code=$?
+assert_exit_code "non-numeric limit handled" "0" "$exit_code"
+
+# T31: Non-numeric days in clear
+result=$(bash "$MAIL_SCRIPT" clear "abc" 2>&1)
+assert_contains "non-numeric days handled" "Cleared" "$result"
+
+# T32: Single quotes in subject preserved
+bash "$MAIL_SCRIPT" read >/dev/null 2>&1  # clear unread
+bash "$MAIL_SCRIPT" send "claude-mods" "it's working" "body with 'quotes'" >/dev/null 2>&1
+result=$(bash "$MAIL_SCRIPT" read 2>&1)
+assert_contains "single quotes in subject" "it's working" "$result"
+
+# T33: Double quotes in body preserved
+bash "$MAIL_SCRIPT" send "claude-mods" "quotes" 'She said "hello"' >/dev/null 2>&1
+result=$(bash "$MAIL_SCRIPT" read 2>&1)
+assert_contains "double quotes in body" "hello" "$result"
+
+# T34: Project name with spaces (edge case)
+bash "$MAIL_SCRIPT" send "my project" "spaces" "project name has spaces" >/dev/null 2>&1
+result=$(bash "$MAIL_SCRIPT" projects)
+assert_contains "project with spaces stored" "my project" "$result"
+
+# T35: Multiple rapid sends
+for i in 1 2 3 4 5; do
+  bash "$MAIL_SCRIPT" send "claude-mods" "rapid-$i" "rapid fire test $i" >/dev/null 2>&1
+done
+result=$(bash "$MAIL_SCRIPT" count)
+assert "5 rapid sends all counted" "5" "$result"
+bash "$MAIL_SCRIPT" read >/dev/null 2>&1
+
+# T36: Init is idempotent
+bash "$MAIL_SCRIPT" init >/dev/null 2>&1
+bash "$MAIL_SCRIPT" init >/dev/null 2>&1
+result=$(bash "$MAIL_SCRIPT" count)
+assert "init idempotent" "0" "$result"
+
+# T37: Empty subject defaults
+result=$(bash "$MAIL_SCRIPT" send "claude-mods" "" "empty subject body" 2>&1)
+assert_contains "empty subject accepted" "Sent to claude-mods" "$result"
+bash "$MAIL_SCRIPT" read >/dev/null 2>&1
+
+echo ""
 echo "=== Results ==="
 echo "Passed: $PASS / $TOTAL"
 echo "Failed: $FAIL / $TOTAL"