Skip to content

fix: safeFetch timer leak, env quote stripping, source count#84

Open
bloxi-bot wants to merge 1 commit intocalesthio:masterfrom
bloxi-bot:fix/audit-hardening
Open

fix: safeFetch timer leak, env quote stripping, source count#84
bloxi-bot wants to merge 1 commit intocalesthio:masterfrom
bloxi-bot:fix/audit-hardening

Conversation

@bloxi-bot
Copy link
Copy Markdown

Audit Fixes

Found these while doing a full code review of the repo. All low-risk, surgical changes.

Changes

1. safeFetch() timer leak on error path (apis/utils/fetch.mjs)

  • The AbortController timeout timer was only cleared on the success path. If fetch() throws (network error, DNS failure, etc.), the timer kept running until it fired — one leaked timer per retry attempt.
  • Fix: moved controller/timer creation outside try, added clearTimeout(timer) in catch.

2. .env parser doesn't strip quotes (apis/utils/env.mjs)

  • Values like FRED_API_KEY="abc123" or FRED_API_KEY='abc123' would include the surrounding quotes as part of the value, causing silent auth failures.
  • Fix: strip matching surrounding quotes (single or double), matching standard dotenv behavior.

3. Startup banner says 26 sources (server.mjs)

  • briefing.mjs actually runs 29 runSource() calls. The README says 27. The banner said 26.
  • Fix: updated banner to 29 to match the actual source count.

Not Changed (noted for future)

  • Telegram/Discord bot command handlers are duplicated (~80 lines each) — could be extracted into shared functions, but it's a larger refactor
  • SSE endpoint has Access-Control-Allow-Origin: * — fine for local use, but crucix.live should restrict it
  • No security headers (helmet/CSP) on the Express server

- Fix safeFetch() abort timer not cleared on error path (leaked per retry)
- Strip surrounding quotes in .env parser (matches dotenv behavior)
- Fix startup banner source count: 26 → 29 (matches actual briefing.mjs)
@bloxi-bot bloxi-bot requested a review from calesthio as a code owner March 29, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant