Skip to content

fix: MQ Wave 1 security hardening and code review fixes#288

Merged
jongio merged 1 commit into
mainfrom
mq/security-hardening-wave1
May 20, 2026
Merged

fix: MQ Wave 1 security hardening and code review fixes#288
jongio merged 1 commit into
mainfrom
mq/security-hardening-wave1

Conversation

@jongio
Copy link
Copy Markdown
Owner

@jongio jongio commented May 20, 2026

Summary

MQ Wave 1 identified 1 CRITICAL, 4 HIGH, and 5 MEDIUM findings. This PR fixes all of them.

Security Hardening

  • RPC session token auth - server generates a crypto/rand token at startup, validated on every Connect-RPC call
  • Healthcheck SSRF prevention - isLocalhostURL() blocks non-local URLs
  • Log secret masking - regex-based masking of key=value secrets and JWT tokens in log file output

Code Review Fixes

  • CR-001 CRITICAL: Operator precedence bug in isFileLockingError
  • CR-002 HIGH: time.Sleep replaced with context-aware select in retry loop
  • CR-005 HIGH: stderr double-wrapping on retry simplified
  • CR-003: Context propagation through hook execution chain
  • CR-004: Nil-check for LogsStoreFuncs in NewLogsHandler
  • CR-006: Extracted updateRegistryWithRetry helper (dedup ~40 lines)
  • CR-010: filepath.Base() sanitization on serviceName in logbuffer

Verification

  • go build ./... pass
  • golangci-lint run ./... 0 issues
  • pnpm run build (dashboard) pass
  • All package tests pass

Security hardening:
- Add RPC session token authentication (X-Session-Token header)
- Restrict healthcheck URLs to localhost only (SSRF prevention)
- Add log secret masking for file output (key=value, JWT patterns)
- Dashboard fetches and injects session token via Connect interceptor

Code review fixes:
- Fix operator precedence in isFileLockingError (gate on Windows first)
- Replace time.Sleep with context-aware select in retry loop
- Fix stderr double-wrapping on installer retry
- Propagate context through hook execution chain
- Add nil-check for LogsStoreFuncs in NewLogsHandler
- Extract updateRegistryWithRetry helper (dedup ~40 lines)
- Sanitize serviceName with filepath.Base in logbuffer

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jongio jongio merged commit 96ca37a into main May 20, 2026
4 of 6 checks passed
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