Skip to content

Warn at startup when PUBLIC_HOST/DB connection env vars are missing - #301

Open
birme wants to merge 1 commit into
mainfrom
security/268-startup-warnings-env
Open

Warn at startup when PUBLIC_HOST/DB connection env vars are missing#301
birme wants to merge 1 commit into
mainfrom
security/268-startup-warnings-env

Conversation

@birme

@birme birme commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds startup Log().warn(...) warnings when PUBLIC_HOST is not set (falls back to localhost default) and when neither DB_CONNECTION_STRING nor MONGODB_CONNECTION_STRING is set (uses localhost MongoDB default).
  • Matches the style/placement of the existing SMB_ADDRESS startup check. Minimal addition only; no restructuring.

Test plan

  • npm run typecheck clean
  • npm test — all 243 tests pass ("worker process has failed to exit gracefully" warning is expected/pre-existing)
  • npm run lint clean on changed file

Closes #268

…rs are missing

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@birme

birme commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

code-reviewer verdict: Needs Changes

Code Review

Summary: PR #301 (#268) adds two startup Log().warn(...) calls for missing PUBLIC_HOST and missing DB_CONNECTION_STRING/MONGODB_CONNECTION_STRING. Conditions correctly mirror the existing ?? fallbacks and don't break startup. Style clean, no npm-hygiene issues. Held back on required test coverage + a message-accuracy nit.

Blocking: None.

Warnings:

  • src/server.ts (no test) — Criteria require a unit test covering missing-vs-present env branches (mocking ./log). No server.test.ts exists/was added.
  • src/server.ts (testability) — The warning logic is top-level in a file whose startServer() IIFE runs on import (and connects to the DB), so it's not unit-testable as written. Extract into an exported pure warnMissingEnv(env, log).
  • src/server.ts:23-25 — Message names only DB_CONNECTION_STRING though the guard also depends on MONGODB_CONNECTION_STRING; name both to avoid operator confusion.

3 Warnings with no Blocking → Needs Changes. Moving back to Ready to add the extracted helper + test.

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.

Security: Add startup warnings for missing PUBLIC_HOST and DB_CONNECTION_STRING environment variables

2 participants