fix(server): restore getClientIp helper for secret-proxy#694
Conversation
#672 deleted getClientIp from rate-limiter.ts as dead code; #692 (hardening sweep) then added an import + call to it in secret-proxy.ts. The gateway fails to boot from source with "SyntaxError: ...does not provide an export named 'getClientIp'", and the prod bundle would silently call undefined() on the legacy header-swap path. Restore the helper. Also fix biome noEmptyBlockStatements lint errors in command-registry.test.ts (10 mock(async () => {}) bodies, all from #685) — these blocked the pre-commit hook on main.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a4e41f94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const forwarded = headers.forwardedFor?.split(",")[0]?.trim().toLowerCase(); | ||
| if (forwarded) { | ||
| return forwarded; |
There was a problem hiding this comment.
Do not trust worker-supplied X-Forwarded-For
When the legacy secret-swap path has no /a/{agentId} in the URL, secret-proxy uses this value as the source for ResolutionFailureLimiter; but worker requests can supply arbitrary x-forwarded-for values, and the gateway HTTP proxy copies request headers through unchanged (forwardHeaders = { ...req.headers }). In that scenario, a compromised worker probing bogus lobu_secret_* placeholders can rotate this header to get a fresh bucket each time and avoid the 20-failure throttle entirely, so the restored helper should not key security limits on an untrusted forwarded header unless it is set/validated by a trusted proxy layer.
Useful? React with 👍 / 👎.
PR #694 restored the helper, but the merge of #693 left an earlier copy intact — esbuild errors out on `Multiple exports with the same name "getClientIp"` in src/gateway/utils/rate-limiter.ts, breaking `make build-packages` on main (typecheck + integration CI jobs cascade from there). Drop the older un-documented copy; keep the JSDoc'd one. No behavior change — the two implementations were functionally identical.
PR #694 restored the helper, but the merge of #693 left an earlier copy intact — esbuild errors out on `Multiple exports with the same name "getClientIp"` in src/gateway/utils/rate-limiter.ts, breaking `make build-packages` on main (typecheck + integration CI jobs cascade from there). Drop the older un-documented copy; keep the JSDoc'd one. No behavior change — the two implementations were functionally identical.
Summary
getClientIpfrompackages/server/src/gateway/utils/rate-limiter.tsas dead code; sec: hardening sweep — webhook sigs, SSRF, nix injection, secret-proxy, token revocation, egress timeout, sandbox, encryption-key #692 (hardening sweep) then imported it + called it frompackages/server/src/gateway/proxy/secret-proxy.ts.mainships a dangling import.make dev,lobu runfrom a checkout) crashes at boot withSyntaxError: The requested module '../utils/rate-limiter.js' does not provide an export named 'getClientIp'. The prod bundle isn't crashlooping today only because the currently-deployed image (20260513-130030) was built from242b5bf0— i.e. before sec: hardening sweep — webhook sigs, SSRF, nix injection, secret-proxy, token revocation, egress timeout, sandbox, encryption-key #692 landed. The next deploy would either fail to start or silently callundefined()on the legacy non-provider secret-swap path (esbuild emits a warning, not an error, on the missing export).getClientIpinrate-limiter.ts(verbatim from pre-refactor: simplification sweep #672).command-registry.test.ts(added in test(runtime+core): harden agent-worker, lobu.toml schema, guardrails, secret-proxy #685 today) has 10mock(async () => {})bodies that tripbiome/lint/suspicious/noEmptyBlockStatements, blocking the pre-commit hook on main. Added/* noop */to each so any subsequent fix PR isn't blocked.Test plan
bun run typecheckcleanbun run check:fixcleanmake devboots and/dev/devicesrenders +GET /api/me/devicesreturns 200 with the bootstrap PATapp.lobu.ai/<owner>/devicesloads