Non-blocking IO, various polish#140
Open
UnknownJoe796 wants to merge 23 commits into
Open
Conversation
Map handler, routing, timeout, and compression exceptions to HTTP responses *inside* the compiled interceptor chain instead of outside it, so error responses still receive interceptor post-processing. The motivating case is CORS: without this, 4xx/5xx responses shipped without Access-Control-Allow-Origin, and browsers masked the real error as a generic CORS failure. An outer catch remains as a safety net for exceptions thrown by an interceptor itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full-repo design review complementing todo.md: critique by area (core, typed/SDK, auth/sessions, engines/deployment, data layer boundary, build/docs/test health) plus a four-phase roadmap prioritizing security hardening, cross-engine conformance testing, AWS reliability, and maintainability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add SecurityHeadersInterceptor (core), installed by default as the outermost interceptor at the single top-level composition point in ServerDefinition. It adds X-Content-Type-Options: nosniff to all responses and Strict-Transport-Security (max-age=3600, per expectations.md) to https responses only, per the HSTS spec. Existing headers set by a handler are left untouched so endpoints can override. Closes the expectations.md security-header gap for all engines at once, since it lives in shared request handling rather than per-engine code. HSTS max-age is a constructor parameter; production deployments may prefer a longer value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The OAuth client flow previously round-tripped a caller-supplied `state` through the provider without storing or validating it (TODO acknowledged in-code), and used no PKCE. - state/CSRF: the value sent to the provider is now an opaque 256-bit single-use nonce. At flow start a FlowRecord (caller state + PKCE verifier) is stored in the cache keyed by the nonce with a short TTL; the callback validates and consumes it via getAndRemove BEFORE any token exchange, so unknown/expired/replayed callbacks are rejected. The caller's app STATE is preserved server-side and still delivered to onAccess; it no longer leaks to the provider. - PKCE (RFC 7636): S256 code_challenge on the auth redirect and code_verifier on token exchange, gated by a per-provider supportsPkce flag (default true). Verified against the RFC Appendix B test vector. - Confirmed the redirect_uri (fixed callback URL) and final UI redirect (driven only by a server-signed Proof) are not attacker-controllable; documented rather than adding an unnecessary whitelist. Also fixes two latent bugs uncovered here: a NPE on code.state!! and an onError result that was computed then discarded (execution fell through). Requires a cache: OauthCallbackEndpoint/OauthProofEndpoints take a Runtime<Cache> and loginUrl is now suspend (source-breaking on the 5.x line). Docs note the shared-cache requirement for multi-instance deployments and the residual login-CSRF limitation of cookie-less state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EventBridge can deliver a scheduled trigger more than once and to overlapping Lambda invocations, so the same tick could run concurrently. Acquire a distributed cache lock (same key scheme and 1h TTL as LocalEngine) before executing a scheduled tick; a losing invocation returns the same benign 200 and does no work. The lock is released in a finally under NonCancellable so a Lambda timeout can't leak it. Adds an optional engine-cache setting on AwsAdapter (RAM default) mirror- ing LocalEngine. As with LocalEngine, cross-process (cross-Lambda) coordination requires pointing this at a shared cache (DynamoDB/Redis); documented on the setting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
constrainAttemptRate was a flat counter+block window: once a block lapsed, the counter TTL expired and an attacker got a fresh full batch of attempts (slow "popcorn" brute force). Now each time the limit is hit the block doubles (blocked * 2^level) up to a maxBlocked cap (default 3h), and the strike level is persisted under "$cacheKey-level" with a memory far longer than any single block window. A successful action clears both the counter and the level, so legitimate users are never penalized. First-offense behavior (level 0) is identical to before, and the exponent is capped so the Duration math can't overflow. Follow-ups noted for later: password/TOTP/backup-code limiters key off un-normalized input (case/whitespace can dodge the limit), and per-IP limiting needs sourceIp plumbed into typed handler implementations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Masquerade is a privilege-sensitive action but was previously silent. Log an AUDIT line on both the granted (info) and denied (warn) paths, recording actor principal/id and the requested target, so impersonation is traceable. Also document that the masqueraded session inherits the actor's scopes verbatim (not the target's) and that narrowing this is an app-specific decision to gate in permitMasquerade — left unchanged here because the correct subset depends on intended masquerade semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ServerSettings.get() transformed settings into a plain LinkedHashMap via a non-atomic containsKey-then-register, so concurrent first-access from multiple threads could double-transform a setting or corrupt the map. Back `goal` with a ConcurrentHashMap and resolve under a single reentrant lock: a lock-free fast path serves already-resolved values (the hot path after ready() pre-warms everything), and first-time resolution double-checks under the lock. A single lock (not per-setting locks) is deliberate — a getter may re-enter get() on the same thread to resolve dependencies, and per-setting locks could deadlock two mutually dependent settings resolved concurrently. The per-thread resolving ThreadLocal circular-dependency check still runs inside the lock. A NULL_RESULT sentinel represents resolved-null in the null-hostile map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A flattened module's internal/external serializersModule getter was evaluated lazily, so a getter that throws surfaced only when the first request hit it, with no indication of which module was at fault. Route every getter (including the previously-bypassed single-module early-return path — a latent gap) through a guard that rethrows a clear error naming the kind (internal/external) and module location, with the original exception as the cause. The combined module stays cached. Note: evaluation still happens at first materialization rather than at build(), because module serializers getters may legitimately depend on the ServerRuntime, which does not exist at build time. Moving the force to startup (ready()) is a separate, engine-wide change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
toNettyResponse omitted Content-Length for bodyless responses (307 redirects, etc.). Without a Content-Length, a keep-alive HTTP/1.1 client cannot tell the response is complete and stalls until the idle timeout closes the connection. Always emit the length (0 when empty). Ktor and JDK already did this; Netty was the outlier. Surfaced by the new cross-engine conformance suite: the trailing-slash 307 redirect test took 120s on Netty (idle timeout) versus <1s on the other engines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Ktor, Netty, and JDK engines each hand-roll request/response translation with no shared test asserting they behave identically against the expectations.md contract. Add one reusable suite in engine-local test fixtures (EngineHttpConformanceSuite) that each engine runs via a ~30-line subclass supplying only how to start itself; the shared HTTP client is the JDK's java.net.http.HttpClient so no new test deps are needed. Checks (all pass on all three engines): nosniff on success and error responses, HEAD fallback, trailing-slash 307 with Location, CORS origin reflection, OPTIONS preflight, HSTS absent over http, timeout 408, and oversized-body 413 (first 413 coverage for Netty). Known gaps are asserted honestly rather than faked: expectations.md's static OPTIONS headers (Allow/Accept-Post/Accept-Patch/Accept-Ranges) and Range/Accept-Ranges are not implemented at the engine level and are documented in the suite as unmet expectations, not passing assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite docs/files.md to the v5 API: single UploadEarlyEndpoint with the upload-then-reference flow, jail/ready scanner quarantine, ServerFile in models, signed URLs, and the one-instance-per-server footgun. Replace the v4 HttpContent/FileObject examples with the TypedData/PublicFileSystem API and drop the removed Azure backend. Replace the docs/websockets.md TODO stub with real docs: WebSocketHandler lifecycle, pub/sub topics, MultiplexWebSocketHandler, typed ApiWebsocketHandler, and local vs AWS execution models. Add docs/migration-v4-to-v5.md covering the service-abstractions extraction, ServerBuilder / definition-vs-runtime split, dependency and import moves, and common pitfalls, deferring KiteUI client details to the ls5-kui7-migration skill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Typed endpoints declare errorCases (LSError with status/detail/message), but only the success response was emitted. Emit each declared error case as a documented response grouped by HTTP status, using the LSError schema and an example, mirroring the success-response emission. Adds a test confirming error responses and path parameters appear in the spec (path parameters were already emitted at path-item level; the test locks that in). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Several auto-CRUD endpoints threw BadRequestException(detail="unique") on unique-constraint violations and NotFoundException on upsert/replace without declaring them, producing W6 "undeclared error" warnings at boot. Add shared notFoundError (404) and uniqueViolationError (400, "unique") LSError constants to the errorCases of every endpoint that throws them, so the errors are documented and the warnings stop. Read-only endpoints that never throw keep empty error lists. A test asserts each throwing endpoint declares the error it can raise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The committed golden TS snapshots had drifted from the current generator. Regenerating disambiguates nested type names (Mode -> UpdateRestrictions Mode, Part -> UpdateRestrictionsPart), matching the generator's intended output. Changes are confined to the type definitions and their imports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PasswordProofEndpoints, TimeBasedOTPProofEndpoints, and BackupCodeEndpoints built their constrainAttemptRate cache key from the raw input value while normalization happened only inside the rate-limited action. An attacker could therefore vary case/whitespace of an identifier to get a fresh limiter bucket per variant, dodging the limiter and its new exponential backoff. Resolve the principal handler and normalize the value before constructing the key, matching PinBasedProofEndpoints. Credential-matching behavior is unchanged. Tests assert case variants of one identifier share a single bucket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers the complete set of changes to move a consumer from 5.1.x to 5.2.0, all verified against a real downstream app (USBE) that then ran locally and served its KiteUI web client unchanged: - Bump service-abstractions to the version 5.2 was built against (1.2.0-1-b2f7bd67); a mismatch compiles but throws NoSuchMethodError at runtime (SettingContext.getOpenTelemetry()). - New required AuthEndpoints/SessionManager `cache` parameter. - Telemetry moved from OpenTelemetrySettings to TelemetryBackend.Settings (both the code setting and the settings.json `"telemetry"` entry, which was previously `null`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uur4dHcoa1a259iiTzvin3
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.
No description provided.