Improve security-related code comments with explanations

[sgaunet]

Problem

The comment at internal/app/client.go:276 states "Use string concatenation instead of fmt.Sprintf for security" but doesn't explain WHY this choice is more secure, making it unclear to future maintainers.

Current comment:

// Use string concatenation instead of fmt.Sprintf for security
actualCountQuery := `SELECT COUNT(*) FROM "` + schema + `"."` + table + `"`

Issues

1. Unclear reasoning: Why is concatenation more secure than fmt.Sprintf?
2. Missing context: What security vulnerability is being prevented?
3. No reference: Doesn't mention that identifiers are quoted for escaping
4. Future confusion: Maintainers might "fix" this to use fmt.Sprintf not understanding the intent

Context

The actual security mechanism here is the double-quote escaping of identifiers, not the choice of concatenation vs fmt.Sprintf. Both approaches would be equally secure/insecure without proper escaping.

Proposed Solution

Update the comment to clearly explain the security approach:

// Build query with proper identifier escaping using double quotes.
// Schema and table names are wrapped in double quotes to prevent SQL injection
// via malicious identifiers. For example, a table named `users"; DROP TABLE users--`
// becomes: SELECT COUNT(*) FROM "public"."users""; DROP TABLE users--"
// which is safe because the semicolon and comment are treated as part of the
// identifier name, not SQL commands.
//
// Note: Consider using pq.QuoteIdentifier() for more robust escaping.
actualCountQuery := `SELECT COUNT(*) FROM "` + schema + `"."` + table + `"`

Or if switching to pq.QuoteIdentifier() (recommended):

// Use pq.QuoteIdentifier() for SQL-safe identifier escaping.
// This properly handles special characters, quotes, and potential injection
// attempts in schema and table names.
actualCountQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s.%s",
    pq.QuoteIdentifier(schema),
    pq.QuoteIdentifier(table))

Other Comments to Improve

Search for other security-related comments that lack explanation:

git grep -n "security" --include="*.go"
git grep -n "SQL injection" --include="*.go"
git grep -n "TODO.*security" --include="*.go"

Best Practices for Security Comments

Security-related comments should include:

1. What: What security mechanism is being used
2. Why: What vulnerability is being prevented
3. How: How the mechanism works
4. Example: Example of what would happen without the protection
5. References: Links to documentation or security advisories

Good example:

// Validate that queries start with SELECT or WITH to prevent destructive operations.
// This prevents SQL injection attacks like: "SELECT * FROM users; DROP TABLE users--"
// where a semicolon allows execution of additional statements.
// See: https://owasp.org/www-community/attacks/SQL_Injection

Impact

• Severity: LOW
• Type: Documentation, Code Quality
• Location: internal/app/client.go:276 and potentially other locations
• Benefits: Better code maintainability, clearer security model

Checklist

• Update comment at client.go:276 with detailed explanation
• Search for other security-related comments lacking context
• Add references to OWASP or PostgreSQL security documentation
• Document the overall security model in CLAUDE.md or docs/
• Add examples of prevented attacks in comments
• Consider adding a SECURITY.md file with security guidelines