fix: health endpoint fails closed on remote bind when KEEPER_REGISTER_SECRET is unset

[Ayomisco]

Closes #358

What

Add an unauthenticated GET /health?probe=liveness endpoint and change the no-secret code path to fail closed.

Why

Railway's liveness probe cannot send a custom x-shared-secret header. Without an unauthenticated probe path, the container restarts in a loop.

The no-secret fallback was fail open: when KEEPER_SECRET is unset and the server is bound to a non-loopback address, any HTTP client received the full health payload. That posture was noted in a comment but not corrected.

Changes

• src/index.ts: added GET /health?probe=liveness before the auth middleware. Returns {alive:true} only, no operational data.
• src/index.ts lines 397-405: changed the no-secret branch from serving the full health response to returning 503. Loopback-only bind is still allowed without a secret (development use).

[coderabbitai]

Warning

Review limit reached

@Ayomisco, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 37 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8608b8be-4250-4c6c-a7d3-999576d081d7

📥 Commits

Reviewing files that changed from the base of the PR and between 8ee810d and 733f0c4.

📒 Files selected for processing (1)

• src/index.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dcccrypto]

Independent verification — not an approval (QA/Security own that). The security posture is right. One operational risk in the escape hatch is worth fixing before merge.

The posture is correct

Failing closed on a remote bind regardless of whether a secret is set is the right call. The previous fail-open behaviour exposed keeperWallet.solBalance, budget circuit-breaker state and stale-oracle market lists to any internet client when KEEPER_REGISTER_SECRET was unset — that's a real leak, and "operator forgot to set the secret" is exactly when you least want to be permissive.

Placing the liveness probe before the auth gate is also right, and it only returns {alive:true} — no operational data. Good separation of liveness from readiness.

The risk: the probe is an exact string match

if (req.url === "/health?probe=liveness" && req.method === "GET")

req.url is the raw request target, so this matches one byte-exact spelling. I ran the comparison rather than guessing:

exact-match behaviour:
  MATCH   /health?probe=liveness
  MISS ✗  /health?probe=liveness&t=1737556800     ← cache-buster
  MISS ✗  /health/?probe=liveness                  ← trailing slash
  MISS ✗  /health?t=1&probe=liveness               ← param order
  MISS ✗  /health?probe=liveness#

Why this matters more than usual here: this probe is the only escape hatch the fail-closed change leaves for a remote-bound keeper. If the health checker's URL differs by so much as an appended cache-buster, the request falls through to the authenticated branch, gets a 503, the platform marks the container unhealthy and restarts it — repeatedly. A hardening change would turn into a restart loop, and the symptom (keeper flapping) looks nothing like the cause.

Parsing instead makes all five spellings work:

const u = new URL(req.url ?? "", "http://localhost");
if (u.pathname.replace(/\/$/, "") === "/health"
    && u.searchParams.get("probe") === "liveness"
    && req.method === "GET") {

robust parse behaviour:
  MATCH  (all five cases above)

No tests

grep -rln "probe=liveness\|KEEPER_REGISTER_SECRET" tests/ returns nothing — this endpoint has no coverage at all, and this PR flips its security posture from fail-open to fail-closed.

The four that would pin it:

• remote bind + no secret → 503 (the fix)
• remote bind + correct secret → 200 with the operational payload
• remote bind + wrong secret → rejected
• ?probe=liveness with no secret → 200 {alive:true} — and ideally one variant spelling, so the brittleness above can't regress silently

Without these, a later refactor could restore the fail-open branch and the suite would stay green.

Not blocking on the posture — that part I'd take as-is. The URL matching is what I'd want changed first.