fix: use parameterized query in cleanup_old_results (CWE-89)

[spidershield-contrib]

Summary

Fixes #3898 — The cleanup_old_results method used an f-string to embed the days_old parameter directly into SQL, allowing SQL injection if the parameter bypasses type checking.

Changes

• Replaced the f-string SQL interpolation with a parameterized query using ? placeholder
• Used CAST(? AS TEXT) with string concatenation in the datetime() function to safely pass the parameter

CWE Reference

• CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
• Severity: Medium

Testing

• Verify that cleanup_old_results(days_old=30) still correctly deletes results older than 30 days
• Verify that cleanup_old_results(days_old=0) works correctly
• Verify that the parameterized query produces the same results as the original for valid integer inputs

---

Found by SpiderShield security scanner

[msureshkumar88]

Thank You for Identifying This Security Issue! 🙏

@spidershield-contrib, thank you for identifying and reporting the SQL injection vulnerability in cleanup_old_results() (CWE-89). Your contribution through SpiderShield's security scanner is valuable to the project's security posture.

Decision: Proceeding with PR #3944

After reviewing both PRs, we've decided to proceed with PR #3944 instead of this one. Here's why:

Comparison

Aspect	PR #3899 (This PR)	PR #3944
SQL Injection Fix	✅ Identical fix	✅ Identical fix
Additional Security Fixes	—	✅ Command injection in run_mutmut.py
Test Coverage	None	✅ 269 lines (14 comprehensive tests)
Deny-Path Testing	—	✅ SQL injection attempt tests
Static Regression Checks	—	✅ AST-based verification
Opened	2026-03-27	2026-03-31

Why PR #3944?

While your PR was opened first and correctly identifies the vulnerability, PR #3944 provides:

1. Broader Security Scope: Fixes 2 vulnerabilities instead of 1

   • Your SQL injection fix in results_store.py
   • Additional command injection fix in run_mutmut.py (replacing os.system() with shutil.rmtree())

2. Comprehensive Test Coverage: 269 lines of tests including:

   • Functional correctness tests
   • Edge case handling
   • Critical deny-path tests that verify the SQL injection fix actually blocks attacks like "0 days') OR 1=1 --"
   • Static regression checks using AST parsing to prevent reintroduction of os.system() calls

3. Better Long-term Maintenance: The extensive test suite ensures:

   • The vulnerability won't be reintroduced
   • Future refactoring won't break the security fix
   • Other developers understand why the parameterized query is important

Your Contribution Matters

Your identification of this issue through SpiderShield is valuable! The SQL injection fix in PR #3944 is identical to yours, which validates your analysis. We want to ensure you receive proper credit:

• Your issue report (Security: SQL Injection in cleanup_old_results (CWE-89) #3898) will be closed by PR security: Replace os.system() with shutil.rmtree() in run_mutmut.py #3944
• We'll acknowledge your contribution in the merge commit
• Consider this a successful security finding that led to a comprehensive fix

Next Steps

We'll be closing this PR in favor of #3944, but your security scanning work is appreciated. If you'd like to contribute more:

1. Continue running SpiderShield scans on the codebase
2. Consider adding test coverage to future security PRs
3. Check out our testing guidelines for writing security tests

Thank you again for helping make ContextForge more secure! 🛡️

---

Duplicate of #3944
If you have any questions or concerns about this decision, please feel free to comment below or reach out to the maintainers.