fix: P2 hardening — GUI apiKey, constant-time auth, pool byte accounting, /x lock, isGenerating refcount#56
Merged
Merged
Conversation
… byte accounting, awaited evictions, /x lock, isGenerating refcount - SRV-5: GUI-launched server now passes settings.serverAPIKey so BearerAuthMiddleware installs (was CLI-only) - SRV-6: bearer token compared constant-time over UTF-8 bytes (length leak accepted, content-position leak closed) - POOL-4: in-flight load bytes reserved before suspension; concurrent distinct-model loads can no longer overshoot maxBytes - POOL-5: evict returns victims and load() awaits their unload before the new engine allocates (no transient double residency); sweepIdle stays fire-and-forget (documented — TTL reclaim isn't racing an imminent allocation) - A2: /x/models/load + /x/models/unload wrapped in the throwing generation-lock discipline (manual swap waits for in-flight generation, same as the generation endpoints) - A3: isGenerating is a refcount (begin/end, clamped at zero) — concurrent GUI + server generations on one model can no longer clear each other's evict/sweep protection Tests: constant-time equality; concurrent-load budget with a LoadGate stub; refcount protect-until-balanced + underflow clamp.
An empty-string key would install BearerAuthMiddleware expecting the literal header "Bearer " — 401ing every request while providing no security. Empty now means "no auth", matching the nil default. Flagged by the P2 review as the one non-blocking finding (pre-existing, but SRV-5 newly routes the GUI server through this guard).
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The P2/backlog hardening cluster from the debug round (follows the P0/P1 wave in #51). Six items:
settings.serverAPIKey, soBearerAuthMiddlewareinstalls there too (was CLI-only). Plus review follow-up: empty-string key = no-auth (would otherwise install a middleware expecting"Bearer "and brick the server).pendingBytes), so concurrent loads of different models can no longer combine pastmaxBytes(OOM window closed). Reservation rolls back on failure — no silent budget shrink.evictdetaches victims synchronously andload()awaits their unload before the new engine allocates — no transient double residency.sweepIdlestays fire-and-forget (documented: TTL reclaim isn't racing an imminent allocation)./x/models/load+/x/models/unloadnow run under the throwing generation-lock discipline (manual swap waits for in-flight generation; exactly-once release, throw-from-acquire = not held).isGeneratingis a refcount (begin/end, clamped at 0): concurrent GUI + server generations on the same model no longer clear each other's evict/sweep protection.Review + verification
Adversarially reviewed (APPROVE): six targeted suspicions — constant-time correctness, reservation-leak lifecycle, victim double-unload, lock exactly-once, markInFlight pairing, begin/end stream-lifetime pairing — all REFUTED with file:line evidence; the one MEDIUM finding (empty-key guard) is fixed in this PR. Local:
** TEST SUCCEEDED **, 79 XCTest + 204 swift-testing, 0 failures; 4 new tests (srv6ConstantTimeEqualsRejectsAnyDifference,testConcurrentLoadsOfDifferentModelsRespectCombinedBudget,testGeneratingRefcountProtectsUntilBalanced,testEndGeneratingClampsAtZeroNoUnderflow) + all prior regression tests green. Branch includes the #55 CI gating fix via merge.