Server-components hardening — review-fix stream (5 blocking + second-pass fixes)#9
Open
IRus wants to merge 11 commits into
Open
Server-components hardening — review-fix stream (5 blocking + second-pass fixes)#9IRus wants to merge 11 commits into
IRus wants to merge 11 commits into
Conversation
Fixes the 9 findings from the server-components security review + triage.
Behavior is locked in by new/updated regression tests; 171 tests pass.
Runtime (framework API):
- toSafeHtml: add a tag allowlist (safeHtmlTagName) mirroring browserTagNameFor.
script/iframe/object/embed/svg/style/template now neutralize to <div ...>
instead of emitting a live element; abstract column/row map to div.
- KineticaServerActionDispatcher: fail closed by default — verifyCapabilityToken
and verifyCsrfToken default to { false } so a verifier-less dispatcher rejects
every action instead of silently accepting unauthenticated ones.
- Rename the FNV-1a integrity API to honest checksum framing: IntegrityHash ->
ChunkChecksum, SignedServerRenderChunk -> ChecksummedServerRenderChunk,
encode/decodeSignedChunk -> encode/decodeChecksummedChunk. KDoc states it
detects accidental corruption only, not tampering (it is keyless FNV).
HTTP servers (samples/server-components, docs/docs-site):
- 500 catch-all returns a generic body and logs the stack trace server-side only;
no more stack-trace/library-version/internal-path leaks to clients.
- Malformed action bodies (SerializationException) return 400 + generic message
instead of bubbling to the 500 catch-all. Request-body size is bounded
(readBoundedBody, 4 KB cap) -> 413 on overflow, so one POST cannot OOM the JVM.
- Fixed-size request thread pool (Executors.newFixedThreadPool) replaces the
single-thread blocking executor; cartCount is now an AtomicInteger so the
handler has no read-modify-write data race under concurrent threads.
- Security headers on every response (incl. static assets + 304s):
X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy,
strict Content-Security-Policy. Docs 404 no longer reflects the request path.
Docs:
- server-components.md: add "Hardening for production" section — schema validates
shape not range; per-session unguessable CSRF tokens required; reference
servers' hardening baseline documented.
Tests:
- safeHtmlEscapesTextAttributesKeysAndClientRefs: extended to assert
script/iframe/object/embed/svg/style/template neutralize to div.
- serverActionDispatcherFailsClosedWithoutExplicitVerifiers: new regression for
the fail-closed default.
- ServerComponentsDemoTest: flipped assertions that previously locked in the
stack-trace leak (malformed -> 400 generic; missing bundle -> 500 generic).
- RuntimeSmokeTest: updated to the renamed checksum API.
Both reviewers independently flagged the same residual issues; this commit fixes them. 8 of 9 findings were already FIXED; #9 (range validation) was PARTIAL (docs only) and is now enforced in code. - Add ServerActionRejection: a typed exception handlers throw to reject bad input with a client-safe message. serverActionStub.dispatch surfaces its message verbatim while all other throwables still return a generic "Server action failed." (no internal leak). - Add AddToCartInput.validationError() (shared): enforces productId length (1..128) and quantity in CART_QUANTITY_RANGE (1..999) — bounds the schema cannot express. - Both addToCart handlers now validate before mutating cartCount. - Tests: runtime covers ServerActionRejection surfacing; server-components covers negative/too-large quantity -> typed Failure + a valid call still succeeding. readBoundedBody off-by-one (codex): - Allocate ByteArray(limit), not limit+1. A body of exactly limit+1 bytes was previously accepted; now any byte beyond limit triggers 413. Added require (limit > 0). Boundary verified by the new oversized-body -> 413 test. Security headers (codex + claude): - Add frame-ancestors 'none' to the CSP on both servers (it does not inherit from default-src). - docs /favicon.ico 204 now goes through applySecurityHeaders(). Path leak (claude): - docs bundle-asset 404 no longer echoes the absolute $bundlesDir filesystem path; it logs server-side and returns a generic "Bundle asset not found." Doc/comment accuracy (codex + claude): - Html.kt: correct the "mirrors browserTagNameFor" comment — SSR is intentionally narrower (textInput/checkbox -> div, not input) since SSR does not hydrate; pages needing form controls use a ClientRef island. - deep-research-report.md: update the stale Russian "integrity hash" line to reflect checksum framing + handler-side validation + fail-closed defaults. 172 tests pass (runtime 152, server-components 7, shared 1, markdown 12).
The bounded body read, HTTP status mapping, and security headers were copy-pasted into both demo servers and had already drifted — the docs server's CSP dropped `style-src 'unsafe-inline'`, which the browser renderer's inline-style layout needs, silently breaking the hydrated demo island. Move the glue into the runtime so both demos, and adopters, inherit one correct implementation: - dispatchHttp(transport, body): ServerActionHttpResponse (common) — maps a request body to a status: null -> 413, undecodable envelope -> 400 (generic message, never the 500 catch-all), otherwise dispatched -> 200. - readBoundedRequestBody(input, limit) (jvm) — the bounded InputStream read. - KineticaSecurityHeaders (common) — the shared header + CSP baseline, keeping style-src 'unsafe-inline' with a KDoc note on why. - DEFAULT_MAX_SERVER_ACTION_BODY_BYTES (common) — the body cap, no longer duplicated per demo. Both servers now call these and their local copies are deleted; the 500 stays each server's outer catch-all (log + generic body). This also fixes the CSP divergence and removes the duplicated status mapping, headers, bounded read, and body-size constant flagged in review. Tests: RuntimeSmokeServerTest.dispatchHttpMapsRequestBodyToHttpStatusCodes (413/400/200 + dispatched-failure-stays-200) and ReadBoundedRequestBodyTest (boundary). Green: runtime 204 (+ JS build), server-components 7, docs-site 4. plan-security.md tracks the remaining review findings (unbounded /demo/api/stack POST, the compiler-generated verifier-less dispatcher, the SSR tag-allowlist snapshot breakage, the deferred-subtree error leak, and the type-invalid-payload 200-vs-400 gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…store SSR fidelity) The allowlist collapsed legitimate DSL/HTML tags (column/row/textInput/ select/textarea/video/canvas/dialog/custom elements) to <div>, breaking two existing SSR snapshot tests and regressing SSR fidelity. Replace it with a case-insensitive denylist of executable/document-mutating tags plus the existing char-gate: dangerous tags (incl. uppercase <SCRIPT>) still collapse to a neutral <div data-kinetica-tag=...>, everything well-formed renders verbatim. Adds case-insensitivity + legitimate-tag regression cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nvalid payloads as client errors Two runtime fixes: - toServerRenderStream streamed the raw exception message into the BoundaryError chunk, leaking JDBC/credential/stack detail to any /stream client. Now logs server-side and streams a generic 'Server render failed.' - serverActionStub decoded the payload inside the handler-guarding try, so a shape-valid but type-invalid payload (e.g. 3.5 for an Int) was swallowed as the opaque 'Server action failed.' server-fault message. Decode now happens before the handler try and returns a distinct 'Invalid server action payload.' client error, keeping CancellationException/ServerActionRejection semantics intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ded read) respondDemoStackSubmission read the POST body with an unbounded readAllBytes() on the public 0.0.0.0 docs server, so one large POST could exhaust the JVM heap — the sibling /actions endpoint was already bounded in this branch but this one was missed. Use readBoundedRequestBody and return 413 before parsing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…iers
The emitter produced a pre-built `val KineticaGeneratedServerActionDispatcher`
with no verifiers. Combined with this branch's new fail-closed {false}
verifier defaults, the generated dispatcher rejected every action with
'Invalid capability token.' and gave adopters no way to inject verifiers.
Emit a `kineticaGeneratedServerActionDispatcher(verifyCapabilityToken,
verifyCsrfToken)` factory instead: verifiers are now required parameters, so
'auth was never configured' can't silently become 'every action rejected',
and no secret is compiled into a global. Golden test pins the factory +
verifier forwarding; the annotated sample constructs through it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h; tidy rejection path Follow-up review fixes on the hardening: - The payload-decode catch was narrowed to SerializationException, but kotlinx does not wrap exceptions thrown by an @serializable init/require block. A shape-valid payload that violates such an invariant threw IllegalArgumentException out of dispatch() (a suspend fun contractually returning ServerActionResponse), bubbling to a 500 where the pre-hardening code returned a clean Failure. Broaden the decode catch to Exception (still rethrowing CancellationException) and classify as 'Invalid server action payload.' - ServerActionRejection.message is now a non-null override; collapse the handler-catch if/else (dead elvis) into one Failure. - Correct the validationError() KDoc: the per-call cap bounds one request's magnitude, it does not make the cumulative demo counter overflow-proof. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t CSP The hardening applies KineticaSecurityHeaders (script-src 'self', no 'unsafe-inline') to every served asset, including the static bench report. The report's inline <script> tooltip handler was therefore blocked by CSP, silently breaking hover tooltips. Move the IIFE into a same-origin bench/report/report.js referenced via <script src>, which script-src 'self' permits — keeping the CSP strict everywhere with no per-route exception. generate.mjs now emits report.js, the static bundler stages it, and index.html is regenerated (data unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s follow-up Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fresh kinetica run (main 13-op suite + scaling), merged with the stored same-machine parts (react/preact/vue/svelte/vanilla/compose-web). All ops within run-to-run noise of the prior mem-opt numbers; two runs done for verification, the warmer/tighter one kept. Headline: DOM geometric mean 1.22x the per-operation fastest (parity with React's 1.24x), startup 13.7ms, ~5.0MB after 1k rows, 86KB gzip bundle. Report regenerated with the CSP-externalized report.js (no inline script), so the strict CSP added in this PR still holds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Refreshed the benchmark report on this branch (commit c1b9cc5) — the committed Setup: M4 Max, system Chrome 150.0.7871.100 headless (same major as the stored parts' 150.0.7871.47), on AC power. Re-benched only kinetica (main 13-op suite + scaling curves) and merged with the same-machine stored parts for the other 6 frameworks. Two runs for verification; kept the warmer/tighter one. Headline (13-op DOM geometric mean, relative to per-op fastest — lower is better):
Kinetica sits at parity with React (marginally ahead). startup 13.7ms, ~5.0MB after 1k rows, 86KB gzip bundle — all within noise of the prior mem-opt figures. Report was regenerated through this PR's |
IRus
added a commit
that referenced
this pull request
Jul 16, 2026
Both reviewers independently flagged the same residual issues; this commit fixes them. 8 of 9 findings were already FIXED; #9 (range validation) was PARTIAL (docs only) and is now enforced in code. - Add ServerActionRejection: a typed exception handlers throw to reject bad input with a client-safe message. serverActionStub.dispatch surfaces its message verbatim while all other throwables still return a generic "Server action failed." (no internal leak). - Add AddToCartInput.validationError() (shared): enforces productId length (1..128) and quantity in CART_QUANTITY_RANGE (1..999) — bounds the schema cannot express. - Both addToCart handlers now validate before mutating cartCount. - Tests: runtime covers ServerActionRejection surfacing; server-components covers negative/too-large quantity -> typed Failure + a valid call still succeeding. readBoundedBody off-by-one (codex): - Allocate ByteArray(limit), not limit+1. A body of exactly limit+1 bytes was previously accepted; now any byte beyond limit triggers 413. Added require (limit > 0). Boundary verified by the new oversized-body -> 413 test. Security headers (codex + claude): - Add frame-ancestors 'none' to the CSP on both servers (it does not inherit from default-src). - docs /favicon.ico 204 now goes through applySecurityHeaders(). Path leak (claude): - docs bundle-asset 404 no longer echoes the absolute $bundlesDir filesystem path; it logs server-side and returns a generic "Bundle asset not found." Doc/comment accuracy (codex + claude): - Html.kt: correct the "mirrors browserTagNameFor" comment — SSR is intentionally narrower (textInput/checkbox -> div, not input) since SSR does not hydrate; pages needing form controls use a ClientRef island. - deep-research-report.md: update the stale Russian "integrity hash" line to reflect checksum framing + handler-side validation + fail-closed defaults. 172 tests pass (runtime 152, server-components 7, shared 1, markdown 12).
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.
Server-components hardening — review-fix stream
Hardens the server-components stack (SSR, server actions, HTTP dispatch) and closes the correctness/security gaps found while reviewing the initial hardening commits. Every fix landed red→green with a regression test; the branch's own
plan-security.mdnow tracks what's resolved vs. deferred.Blocking findings fixed (each with a test)
{ false }verifier defaults, the emitter's verifier-lessval KineticaGeneratedServerActionDispatcherreturnedInvalid capability token.for every action, with no way to inject verifiers. The emitter now emits akineticaGeneratedServerActionDispatcher(verifyCapabilityToken, verifyCsrfToken)factory that requires the verifiers — "auth never configured" can't silently become "all rejected," and no secret is compiled into client code. (compiler golden test + annotated sample)safeHtmlTagNameallowlist broke SSR + two snapshot tests. It collapsed legitimate DSL/HTML tags (column,row,textInput,select,textarea,video,canvas,dialog, custom elements) to<div>. Replaced with a case-insensitive denylist of executable/document-mutating tags + the char-gate: dangerous tags (incl.<SCRIPT>) still collapse to a neutral<div data-kinetica-tag=…>, everything well-formed renders verbatim./stream. A throwingServerRenderDeferredSubtreestreamed the rawerror.message(e.g. a JDBC/credential detail) into aBoundaryErrorchunk. Now logs server-side and streams a genericServer render failed./demo/api/stackPOST → heap-exhaustion DoS on the public0.0.0.0docs server. Now usesreadBoundedRequestBody(...) ?: 413, matching/actions.3.5for anInt) was swallowed as the opaqueServer action failed.server-fault message. Payload decode now runs before the handler try and returns a distinctInvalid server action payload.client error.Second-pass review fixes
@Serializableinit{}/constructor exceptions, so anIllegalArgumentExceptionescapeddispatch()(asuspend funcontractually returningServerActionResponse) → a 500. Broadened the decode catch toException(still rethrowingCancellationException).<script>.script-src 'self'(applied to every static asset) blocked the report's tooltip handler. Externalized it to a same-originbench/report/report.js(<script src>, whichscript-src 'self'permits) — CSP stays strict everywhere, no per-route exception.Failurein the handler catch (ServerActionRejection.messageis now a non-nulloverride); corrected thevalidationError()KDoc overflow overclaim.Deferred (tracked in
plan-security.md)Design/altitude follow-ups: unify the SSR/CSR tag tables, centralize error scrubbing, dedup the two reference-server helpers, and the low-priority residuals (413 no-drain is working-as-intended for the DoS goal).
Verification
Full local run of the CI suite — compiler + all JVM module tests, runtime/test-kit/browser JS tests, compiler-plugin samples (JVM+JS), bundle-size, and the JVM microbench smoke — all green.