Skip to content

fix: resolve CodeQL alerts for logging, format strings, and ReDoS - #37

Merged
cursor[bot] merged 2 commits into
mainfrom
cursor/fix-codeql-alerts-b4c6
Aug 23, 2026
Merged

fix: resolve CodeQL alerts for logging, format strings, and ReDoS#37
cursor[bot] merged 2 commits into
mainfrom
cursor/fix-codeql-alerts-b4c6

Conversation

@Skiipy11

@Skiipy11 Skiipy11 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the open CodeQL alerts on main (#2#13): clear-text logging of secrets in error messages, format-string injection via console.error, polynomial ReDoS in the temporal resolver, and a hostname includes() check in a unit test.

Follow-up: CodeQL still flagged errorSummary() because Error fields inherit taint from GEMINI_API_KEY in request URLs. This PR now logs a SHA-256 fingerprint only, sends the Gemini key via x-goog-api-key, and omits URLs from timeout errors.

CodeRabbit reviewed HEAD (2ec7a9e) with no actionable comments. All CI checks passed.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have tested this locally
  • This does not introduce breaking changes (or they are documented above)
  • I have updated documentation if needed

API unit tests: 349 passed (includes log.test.js coverage for the format-string and fingerprint contracts).

Open in Web Open in Cursor 

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling to prevent sensitive details and credentials from appearing in logs.
    • Added safe, consistent error fingerprints for troubleshooting without exposing error contents.
    • Secured Gemini requests by sending API keys through headers instead of URLs.
    • Sanitized timeout messages and improved recognition of relative date phrases with extra spaces.
  • Tests
    • Added coverage for secure error summaries, logging, and relative date parsing.
    • Updated domain extraction assertions for more reliable validation.

Stop logging Error.message (pg/HTTP errors embed DSNs and API keys),
pass user-controlled values as console format arguments instead of
interpolating them into the format string, bound the relative-ago
regex, and use exact Set membership in the domain extraction test.

Co-authored-by: Skiipy <steven@expertlocal.ca>
@Skiipy11
Skiipy11 marked this pull request as ready for review August 23, 2026 06:09
@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

PR author is not in the allowed authors list.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: ZenSystemAI/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88bdeafe-d23c-4aef-a2b7-38d448ebf82d

📥 Commits

Reviewing files that changed from the base of the PR and between 1d7ef51 and 2ec7a9e.

📒 Files selected for processing (5)
  • api/src/lib/log.js
  • api/src/services/embedders/gemini.js
  • api/src/services/fetch-with-timeout.js
  • api/src/services/llm/gemini.js
  • api/tests/log.test.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds hashed error summaries, updates API and service logging call sites, moves Gemini credentials out of URLs, bounds relative-time parsing, and updates related tests.

Changes

Sanitized error logging

Layer / File(s) Summary
Error summary and logger contract
api/src/lib/log.js, api/tests/log.test.js
errorSummary returns a 12-character SHA-256 fingerprint. logError logs sanitized values with format-string-safe arguments. Tests cover stability, sensitive-data exclusion, request IDs, and nullish inputs.
Route error logging migration
api/src/routes/*.js
Route handlers pass complete errors to logError. Selected vector, import, and search logs use errorSummary.
Service and script logging migration
api/src/index.js, api/src/services/*.js, api/scripts/reembed.js
Startup, scheduled, batch, retrieval, embedding, and timeout logs use summarized errors or restricted error text.

Credential-safe Gemini requests

Layer / File(s) Summary
Gemini credential transport
api/src/services/embedders/gemini.js, api/src/services/llm/gemini.js
Gemini API keys move from URL query parameters to x-goog-api-key headers. JSON content headers are explicit.

Bounded temporal parsing

Layer / File(s) Summary
Relative-ago matcher bounds
api/src/services/temporal-resolver.js, api/tests/temporal-resolver.test.js
The matcher bounds numeric and whitespace repetitions. Tests cover extra spaces in relative-ago phrases.

Domain test assertion update

Layer / File(s) Summary
Domain membership assertion
api/tests/entities.test.js
The domain test uses a Set and Set.has() for membership checks.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 2ec7a

This change reduces secret exposure, format-string injection, and regular-expression denial-of-service risk without introducing a known merge-blocking issue; it is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main CodeQL fixes for logging, format-string safety, and ReDoS in the changeset.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/fix-codeql-alerts-b4c6

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread api/scripts/reembed.js Fixed
Comment thread api/scripts/reembed.js Fixed
Comment thread api/src/index.js Fixed
Comment thread api/src/lib/log.js Fixed
Comment thread api/src/routes/export.js Fixed
Comment thread api/src/routes/memory.js Fixed
Comment thread api/src/services/consolidation.js Fixed
Comment thread api/src/services/research-retrieval.js Fixed
CodeQL still flagged errorSummary() because Error.name/code inherit taint
from GEMINI_API_KEY in request URLs. Log a SHA-256 fingerprint instead,
send the Gemini key via x-goog-api-key, and omit URLs from timeout errors.

Co-authored-by: Skiipy <steven@expertlocal.ca>
@Skiipy11 Skiipy11 added the review:deep CodeRabbit plus independent deep review label Aug 23, 2026 — with Cursor
@cursor

cursor Bot commented Aug 23, 2026

Copy link
Copy Markdown

@coderabbitai review

@cursor
cursor Bot merged commit 2ec7a9e into main Aug 23, 2026
10 checks passed
@cursor
cursor Bot deleted the cursor/fix-codeql-alerts-b4c6 branch August 23, 2026 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:deep CodeRabbit plus independent deep review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants