superseded: inert rate-limit wiring prototype

[OnlineChef]

Implements structure/rate-limit-runtime-wiring.md on current dev.

The token-bucket policy-transition fix is already present on dev as f06bffe and independently verified by Greptile/T-Rex across low→high, high→low, burst-clamp, and shared-overflow transitions.

This PR remains draft until the runtime activation is complete in isolated gates:

1. strict canonical config/types/safe DTO and focused tests;
2. typed auth principals without header reparsing outside auth internals;
3. one synchronous runtime admission coordinator and exact route mapping;
4. HTTP 429 integration after successful auth;
5. WebSocket token charge, concurrency reservation, rollback and idempotent lifecycle release;
6. aggregate-only bounded metrics;
7. docs, focused security/privacy tests, and full Linux/macOS/Windows validation.

Default-off behavior must remain byte-compatible. Origin is never identity or bypass. Raw credentials, fingerprints, addresses, provider/model/account/request/conversation fields, prompts, and errors must not escape auth/admission internals or appear in metrics/logs/responses.

Greptile Summary

This change introduces the rate-limit configuration, principal derivation, admission primitives, route mapping, and endpoint-specific 429 response helpers. A real server check showed that enabling the configuration does not yet limit protected traffic: after a one-request model-discovery budget was exhausted, two GET /v1/models requests still returned 200 without rate-limit headers. The limiter works when invoked directly; src/server/index.ts still needs to connect it to HTTP and WebSocket request handling.

Confidence Score: 4/5

Not safe to merge until the configured rate limiter is enforced by the server runtime.

The server accepted repeated protected requests after an enabled one-request budget had been exhausted, while the same configuration correctly denied the second request when the admission helper was called directly.

Files Needing Attention: src/server/index.ts

Security Review

The rate limiter is intended to protect server resources, but enabled settings currently do not constrain production HTTP or WebSocket traffic. Operators could enable the feature believing abusive or accidental request bursts are bounded while protected endpoints continue accepting requests. Wire authentication-aware admission, denial responses, and WebSocket reservation lifecycle handling into src/server/index.ts before merging.

T-Rex Logs

What T-Rex did

• T-Rex produced and published a finding-comment-proof for a posted P1 finding.
• T-Rex produced a second finding-comment-proof for another posted P1 finding.
• T-Rex executed a general-contract-validation-proof by running the rate-limit-wiring-check.ts script, capturing the before/after logs, starting the server, and observing runtime statuses of 200 with no rate-limit headers.

_{Ran code and verified through T-Rex}

Comments Outside Diff (3)

1. General comment

   Supported parameterized live WebSocket routes bypass live surface mapping

   • Bug
     • rateLimitSurfaceForRequest returns null for GET /v1/live/{callId} and GET /v1/realtime/calls/{callId} when webSocket is true, even though the live sideband parser recognizes both routes and the server dispatches them for WebSocket upgrades.
   • Cause
     • The WebSocket branch at src/server/rate-limit-admission.ts:55 compares only the three literal paths /v1/live, /v1/realtime, and /v1/realtime/calls; it has no parameterized-path match.
   • Fix
     • Extend the WebSocket live route mapping to recognize the validated parameterized route shapes, ideally by sharing route recognition with parseLiveSidebandTarget or using equivalent strict path matching so unsupported prefixes remain uncharged.

   _{Ran code and verified through T-Rex}

2. src/server/index.ts, line 255 (link)

   Configured rate limiter is never enforced

   The new configuration, principal, admission, and 429-rendering helpers are not connected to the server request or WebSocket lifecycle. With rateLimit.enabled: true and a model-discovery policy capped at one request, two real GET /v1/models requests both return 200 without rate-limit headers. This leaves the resource-protection feature inert in production. Construct the coordinator during server startup, derive a principal after successful authentication, admit each mapped HTTP route before downstream work, and return the endpoint-specific 429 response on denial. WebSocket upgrades also need token charging, reservation, failed-upgrade rollback, and release hooks.

   Artifacts

   
   

   Command output from the check

   • Executed the configured admission helper twice and captured allowed then denied behavior, showing the limiter itself can enforce the burst. Takeaway: helper-level limiting works.

   Command output from the check

   • Started the real Bun server with persisted enabled rate-limit configuration and made two protected model-discovery requests; both returned 200 without rate-limit headers. Takeaway: production HTTP routing is not wired to enforcement.

   Evidence from the check

   • The executed TypeScript validation script writes isolated enabled configuration, verifies helper exhaustion, starts the server, and requests the protected route twice. Takeaway: the captured runtime comparison is reproducible.

   

   _{Ran code and verified through T-Rex}

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/server/index.ts
   Line: 255
   
   Comment:
   **Configured rate limiter is never enforced**
   
   The new configuration, principal, admission, and 429-rendering helpers are not connected to the server request or WebSocket lifecycle. With `rateLimit.enabled: true` and a `model-discovery` policy capped at one request, two real `GET /v1/models` requests both return `200` without rate-limit headers. This leaves the resource-protection feature inert in production. Construct the coordinator during server startup, derive a principal after successful authentication, admit each mapped HTTP route before downstream work, and return the endpoint-specific 429 response on denial. WebSocket upgrades also need token charging, reservation, failed-upgrade rollback, and release hooks.
   
   ---
   
   For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

   

3. General comment

   Configured rate limiter is never wired into HTTP or WebSocket server runtime

   • Bug
     • With persisted rateLimit.enabled=true and model-discovery set to one request per minute with burst one, two actual GET /v1/models requests returned 200 and no X-RateLimit-*/Retry-After headers. The same configuration passed directly to RuntimeRateLimitAdmission returned allowed then denied, proving the limiter itself can exhaust but production routing does not invoke it.
   • Cause
     • startServer loads configuration and handles every HTTP/WebSocket route in src/server/index.ts:255-976, but does not resolve config.rateLimit, instantiate RuntimeRateLimitAdmission, derive an authenticated principal, call admit/reserveWebSocket, render rateLimitResponse, or release WebSocket reservations. The new modules therefore have no runtime call path.
   • Fix
     • At server startup, resolve validated rate-limit config and create one process-local admission coordinator. After each route's successful authentication and before downstream work, derive a principal and charge the mapped surface; return the appropriate 429 response on denial. For Responses/live WebSocket upgrades, charge and reserve before server.upgrade, release on failed upgrade, retain the release function in WsData, and release it on every close/error path. Wire management metrics/snapshots as specified by the runtime-wiring contract.

   _{Ran code and verified through T-Rex}

Prompt To Fix All With AI

### Issue 1
src/server/index.ts:255
**Configured rate limiter is never enforced**

The new configuration, principal, admission, and 429-rendering helpers are not connected to the server request or WebSocket lifecycle. With `rateLimit.enabled: true` and a `model-discovery` policy capped at one request, two real `GET /v1/models` requests both return `200` without rate-limit headers. This leaves the resource-protection feature inert in production. Construct the coordinator during server startup, derive a principal after successful authentication, admit each mapped HTTP route before downstream work, and return the endpoint-specific 429 response on denial. WebSocket upgrades also need token charging, reservation, failed-upgrade rollback, and release hooks.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (2): Last reviewed commit: "test(ratelimit): cover canonical live si..." | Re-trigger Greptile}

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9aa726cd-fa78-43c1-81ea-c528d687ca25

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

[github-actions]

✅ Target branch corrected

This pull request now targets dev.

The [WRONG BRANCH] title prefix has been removed. Its existing draft status has been preserved.

[OnlineChef]

[code]smith (@codesmith-bot) Implement the runtime activation contract in structure/rate-limit-runtime-wiring.md on the existing branch feat/rate-limit-runtime-wiring only. Do not create another branch or PR.

Use the contract as authoritative. Work in coherent commits:

1. canonical OcxConfig type + Zod/load/save/safe-DTO validation and focused config tests;
2. additive typed data-plane and management auth results that return only opaque RateLimitPrincipal outside auth internals while preserving all existing response helpers;
3. a single synchronous runtime admission coordinator with exact route mapping and API-specific 429 envelopes;
4. HTTP route integration after successful auth and before expensive work;
5. Responses and live-sideband WebSocket token charge + concurrency reserve/rollback/idempotent release on all lifecycle paths, updating WsData types;
6. aggregate-only projection into existing metrics exports;
7. focused security/privacy/lifecycle tests and docs.

Default-off behavior must remain byte-compatible. Never use Origin as bypass/identity, never persist buckets/secrets, never expose fingerprints or dynamic identity labels, and never touch provider routing/account selection. Keep the PR draft and push incremental commits after each focused typecheck/test gate.

[blacksmith-sh]

Got it, I'm reviewing the rate-limit runtime wiring contract first.

[OnlineChef]

[code]smith (@codesmith-bot) The PR now targets dev and satisfies the repository branch rule. Continue the implementation on feat/rate-limit-runtime-wiring using structure/rate-limit-runtime-wiring.md; do not modify PR #48.

[OnlineChef]

Implement the contract incrementally. This first review item is intentionally limited to canonical configuration and focused tests; do not touch auth or server routing in the same commit.

[OnlineChef]

Implement configuration gate 1 now on this branch. Add OcxRateLimitConfig/related types to src/types.ts, the strict optional rateLimit object to the existing canonical Zod schema and load/save validation in src/config.ts, and safe management DTO/candidate validation through existing paths. Default absent/disabled must preserve configs exactly. Use complete documented bounded defaults for all existing RateLimitSurface policies when enabled, or require complete policies—do not permit silently unprotected surfaces. Reject unknown surfaces, invalid rates/bursts/caps, and contradictory WebSocket settings. Add a focused tests/ratelimit-config.test.ts covering absent config, valid round-trip, unknown surface, invalid values, enabled completeness/defaults, and no secret/runtime fields. Run typecheck plus the focused config and existing ratelimit tests, then push one coherent commit. Do not modify auth, server/index.ts, WebSocket data, or metrics in this commit.

[OnlineChef]

[code]smith (@codesmith-bot) The strict validator/defaults/tests now exist and the branch is synced with current dev at b40ad8f. Complete this finding with only the three existing-file connections: type-only rateLimit?: RateLimitConfigInput on OcxConfig; rateLimit: rateLimitConfigSchema.optional() in the existing top-level configSchema; explicit safeConfigDTO/management candidate projection plus round-trip tests. Do not create new schemas/defaults and do not touch runtime routes/auth/metrics.

[OnlineChef]

Security note for the later typed-auth commit: on loopback, do not fingerprint an arbitrary Authorization, x-api-key, or upstream credential merely because auth is optional. Use admission-key only when isDataPlaneAdmissionSecret confirms the presented value is an OpenCodex admission secret. Otherwise use Bun's trustworthy socket address when available, or the shared anonymous principal. Responses transport must continue treating ordinary Authorization as upstream Codex Direct auth, never limiter identity.

[OnlineChef]

[code]smith (@codesmith-bot) The branch is now clean on current dev (f06bffe) and only the contract file remains. Execute the existing inline configuration finding at structure/rate-limit-runtime-wiring.md:17 now. Push only the strict canonical config/types/safe-DTO/tests commit; no auth/server/metrics changes.

[OnlineChef]

[code]smith (@codesmith-bot) Gate 1 foundation is now on head 605592e: src/ratelimit/config.ts defines complete fixed defaults and resolveValidatedRateLimitConfig; tests/ratelimit-config-defaults.test.ts covers default-off/completeness/override/freeze. Continue only with canonical integration: add the persisted type to OcxConfig, a strict Zod rateLimit schema in existing src/config.ts, explicit safeConfigDTO projection/candidate validation, focused invalid-input/round-trip tests, and docs. Reuse this module; do not create a second defaults or validation implementation. No auth/server/metrics changes yet.

[OnlineChef]

greptile-apps Review current head 0311f4b only for the new pure/inert layers: src/ratelimit/config.ts, src/server/rate-limit-admission.ts, their exports and focused tests. Reproduce: default-off allocates no state; all 9 surfaces have complete bounded defaults; partial overrides cannot leave a surface unlimited; exact route mapping does not broaden through prefixes/methods; principal snapshots leak no fingerprint; WebSocket principal/global caps, duplicate release and reset-generation safety hold. The module is not imported by existing runtime routes yet. Report concrete defects only; do not assess the still-pending canonical Zod/auth/server integration as implemented.

[OnlineChef]

Triage 2026-08-02 ADR (OCX → Sofie / Pi-Rust)

Classification: SUPERSEDE → close in favor of #45

Why

• Same goal as feat(ratelimit): wire admission-aware rate limiting into the server #45 (wire feat(ratelimit): add admission limiter primitives #44 admission primitives into the OCX server), but this branch is still an inert/partial coordinator:
  • adds src/server/rate-limit-admission.ts, rate-limit-auth-principal.ts, src/ratelimit/config.ts, contract doc
  • does not wire src/server/index.ts / live HTTP+WS enforcement (gates 4–7 still open per PR body)
• feat(ratelimit): wire admission-aware rate limiting into the server #45 already has the full opt-in integration path (src/server/rate-limit.ts + server/auth/ws hooks + docs + admission e2e).
• Sofie @ f06bffeb already has feat(metrics): add bounded Prometheus observability #42/feat(ratelimit): add admission limiter primitives #44/fix(ratelimit): preserve bucket policy across runtime changes #47 primitives; neither feat(ratelimit): wire admission-aware rate limiting into the server #45 nor superseded: inert rate-limit wiring prototype #49 is deployed as wiring yet — keep one wiring donor (feat(ratelimit): wire admission-aware rate limiting into the server #45).

Salvage (optional, into #45 if missing)

• structure/rate-limit-runtime-wiring.md contract wording
• any stricter defaults from src/ratelimit/config.ts not already covered by feat(ratelimit): wire admission-aware rate limiting into the server #45’s config/types

Closing as superseded. No merge.

[OnlineChef]

Superseded by #45 (complete admission wiring donor). See triage comment.

[OnlineChef]

greptile-apps Re-run the focused live WebSocket validation on current head 96e084c. Admission now calls the same parseLiveSidebandTarget(pathname, searchParams) used by dispatch, requires GET, and fails malformed routes closed. Verify all three supported forms (/v1/live/{id}, /v1/realtime/calls/{id}, /v1/realtime?call_id={id}) plus invalid IDs/lookalike paths. The prior P1 thread is resolved as outdated.

[OnlineChef]

[code]smith (@codesmith-bot) Gate 1 is now reduced to three existing-file edits. Reuse rateLimitConfigSchema from src/ratelimit/schema.ts and RateLimitConfigInput from src/ratelimit/config.ts:

1. Add rateLimit?: RateLimitConfigInput to top-level OcxConfig in src/types.ts via a type-only import.
2. Import rateLimitConfigSchema in src/config.ts and add exactly rateLimit: rateLimitConfigSchema.optional() inside the existing top-level configSchema object. Do not add another validator/default layer.
3. Explicitly project rateLimit in safeConfigDTO/existing management candidate path so it round-trips as config but no runtime state appears.
4. Add focused load/save/safe-DTO round-trip tests using the already-tested strict subschema.

Push one coherent commit to this branch. No auth/router/WebSocket/metrics changes.

[OnlineChef]

Superseded by PR #45, which contains the complete canonical config/auth/HTTP/WebSocket integration on the current ChefVault/provider-security base. Salvage retained for #45: auth-first 401/403 precedence, Cache-Control on 429s, stricter numeric config caps, and live-sideband concurrency lifecycle. The parameterized-route P1 in this PR was fixed before closure, but #45 already classifies sideband routes through the canonical dispatcher. No merge from #49.

[blacksmith-sh]

OnlineChef

[code]smith (@codesmith-bot) Implement the runtime activation contract in structure/rate-limit-runtime-wiring.md on the existing branch feat/rate-limit-runtime-wiring only. Do not create another branch or PR.

...

Todo list reconciled — those three items were finished earlier in the session: the WebSocket lifecycle commit (reserve and release WebSocket admission across all lifecycle paths), the metrics projection commit (project aggregate rate-limit statistics into both exports), and the tests+docs commit (cover admission security, lifecycle, and privacy end to end) are all in the pushed stack at 777a5ff, gated by typecheck, privacy scan, and the full suite.

The open question from my last message stands: PR #49 was closed as superseded by PR #45, and the "inert" review finding predates the wiring push. Let me know whether to (a) reopen #49 with a correction, (b) port my branch's deltas onto #45, or (c) just leave a pointer comment on #49 for the maintainer.

[blacksmith-sh]

This session has hit its $100 spend limit, so [code]smith paused work here. To keep going, open the session in the dashboard (https://app.blacksmith.sh/OnlineChefGroep/sessions/019fc1f2-6e6a-73a5-a375-06e666dcf80d) and press Continue, or start a new session.