Skip to content

feat(server): add startup security checks for secrets and origins - #317

Merged
anurag629 merged 2 commits into
codercops:devfrom
varun-pahuja:fix/231-startup-security-checks
Oct 23, 2026
Merged

anurag629 merged 2 commits into
codercops:devfrom
varun-pahuja:fix/231-startup-security-checks

Conversation

@varun-pahuja

Copy link
Copy Markdown
Contributor

What does this change?

Adds startup security checks for session secrets and allowed origins as a new module `packages/server/src/security/startup-checks.ts`. Reports errors for: secrets < 32 bytes (UTF-8), secrets that match public env vars (NEXT_PUBLIC_, VITE_, PUBLIC_, NUXT_PUBLIC_, EXPO_PUBLIC_, REACT_APP_), origins not starting with http(s)://, origins with path/query/trailing slash, and `*` combined with other origins. Warns for: duplicate secrets, http:// origins not localhost/127.0.0.1. Not exported from `@chatcops/server` yet (will be called by handler in follow-up).

Closes #231

How I tested this

  • `pnpm -r typecheck` — passes
  • `pnpm test` — 575 passed, 10 skipped (Node 22, happy-dom works)
  • `pnpm test:coverage` — passes thresholds (server/src/** ≥ 90% lines, 85% branches)
  • `pnpm lint` — passes (pre-existing warnings only)
  • Unit tests: one test per rule (7 rules), clean config returns [], secret-never-in-message test, UTF-8 byte counting, all public prefixes covered

AI assistance

Claude Code — wrote the implementation, tests, and changeset. I reviewed and ran all gates.

Checklist

  • This PR targets `dev`
  • I was assigned the linked issue
  • `pnpm -r build`, `pnpm test` and `pnpm -r typecheck` pass locally
  • Tests added or updated where it makes sense
  • For a bug fix: the new test fails without my change
  • Changeset added (`pnpm changeset`) if this changes `core`, `widget` or `server`
  • Docs updated if behavior or options changed

Adds checkSecuritySettings() in packages/server/src/security/startup-checks.ts
and tests. Reports errors/warnings for:
- session secrets < 32 bytes (UTF-8)
- secret matches NEXT_PUBLIC_, VITE_, PUBLIC_, NUXT_PUBLIC_, EXPO_PUBLIC_, REACT_APP_ env vars
- allowed origin not http(s):// or *
- origin with path/query/trailing slash
- * combined with other origins
- duplicate secrets (warn)
- http:// origin not localhost/127.0.0.1 (warn)

Not exported from @chatcops/server yet.

Closes codercops#231

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @varun-pahuja, this does everything #231 lists. There's one test per rule, a clean config returns [], and a test shows the secret never shows up in a message. Bytes are counted in UTF-8, env falls back safely where there's no process, nothing is exported yet, and the changeset is empty. I also tried a few origins the issue didn't name: https://*.acme.com and https://acme.com:8443 pass as they should. Locally the server suite passes (332 tests, 96% lines), and lint is unchanged from dev.

A small note on the PR description: it lists * together with other origins as an error, but the code reports it as a warning, which is what the issue asked for. The code is right.

Two things I'll handle when I wire this into the startup log (T4), so nothing to change here:

  • The origin messages say "Allowed origin 2" rather than showing the origin. Origins aren't secret, so I'll put the value in the message so it's easier to act on.
  • https://acme.com#x passes. A fragment is never part of an origin either, so I'll count it like a path.

@anurag629
anurag629 merged commit e4b6f02 into codercops:dev Oct 23, 2026
5 checks passed
@anurag629 anurag629 added the hacktoberfest-accepted PR counts for Hacktoberfest label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest-accepted PR counts for Hacktoberfest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add startup checks that catch weak or leaked security settings

2 participants