fix(config): empty allowed-hosts guard, fail-fast port, 0600 secrets, no request-path panics#16
Conversation
… no request-path panics - MCP_SSH_ALLOWED_HOSTS set-but-empty (or allowed_hosts = [] / blank entries in the file) fed rmcp an empty allowlist, which it reads as allow-ALL hosts — the DNS-rebinding guard silently OFF, worse than unset. Empty now behaves exactly like unset: loopback default + warn. - Invalid MCP_SSH_PORT (8080x, 70000) was silently ignored, leaving the server on the default port while the operator believed it moved. Now fails fast like an invalid MCP_SSH_BIND. - set-auth created the config file umask-default (0644) with the plaintext password and only chmodded after — and a chmod failure was silently discarded, leaving it world-readable forever. Now created 0600 from the first byte; the tighten-existing chmod propagates errors. - config::db_path() swallowed read/parse errors (unwrap_or_default), silently pointing admin commands at the DEFAULT database while the server used the configured one. Errors now propagate. - Db::call had expect() on the request path: one panicking closure poisoned the mutex and turned every later call (all bearer validation, all job writes) into a panic until restart. Poisoned locks are recovered; a closure panic returns an error to its caller alone. - Admin job kill cast the stored pgid i64 -> u32 with as, wrapping a corrupt/negative row into a real (wrong) process group to signal. Now rejected with a message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai approve |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
✅ Action performedComments resolved and changes approved. |
Version bump for the v1.3.0 release: GlitchTip error tracking (#12) + bug-fix PRs #13–#16. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Config/startup/robustness fixes from the bug hunt, all with tests:
MCP_SSH_ALLOWED_HOSTSdisabled the host guard entirely.Environment=MCP_SSH_ALLOWED_HOSTS=in a unit file (orallowed_hosts = []) produced an empty allowlist, which rmcp treats as allow all hosts — DNS-rebinding guard silently off, worse than unset. Empty-after-trim now behaves exactly like unset: loopback default plus the warning.MCP_SSH_PORTsilently ignored.8080x/70000fell through.ok()and the server bound the default port while the operator believed they'd moved it. Now fails fast, consistent withMCP_SSH_BIND.set-authwrote the plaintext password world-readable first.fs::writecreated the file umask-default (0644), chmod 600 only afterwards — and a chmod failure was discarded (let _), leaving it 0644 forever. The file is now created 0600 from the first byte and the tighten-existing chmod propagates errors.db_path()swallowed config errors. An unreadable/broken config file mademcp-ssh jobs/job kill/sessionssilently operate on the default DB path while the live server used the configured one ("no jobs", kills that do nothing). Errors now propagate.Db::callcould turn one bug into a full outage.expect()on the lock + join meant a single panicking closure poisoned the mutex and every subsequent call — all bearer validation and job persistence — panicked until restart. Poisoned locks are recovered and a closure panic returns an error to its caller alone.pgid as u32wrapped a corrupt/negative row value into a real (wrong) process group; now rejected viatry_from.🤖 Generated with Claude Code