fix(dynamic-field): escape number/date/datetime input value (stored XSS) - #1027
Open
mmcintosh wants to merge 3 commits into
Open
fix(dynamic-field): escape number/date/datetime input value (stored XSS)#1027mmcintosh wants to merge 3 commits into
mmcintosh wants to merge 3 commits into
Conversation
The number/date/datetime cases in dynamic-field.template.ts emit `value="${value}"`
UNESCAPED, while every other field type escapes it (escapeHtml(value)). Content field
values are stored raw by the write path, then rendered back into the content edit form,
so an authenticated content writer could persist `"><script>…` in a number/date/datetime
field and have it execute in another admin's edit form. type="date"/"number" doesn't
help — the payload breaks out of the tag during HTML parsing, before the input validates
its value.
Escape at the sink: `escapeHtml(String(value ?? ''))`. A valid number/date is unchanged;
the String() coercion is required because escapeHtml returns '' for non-strings (a numeric
value would otherwise vanish).
…e value escaping Covers the stored-XSS fix from issue #1026 — verifies payload is escaped at render time and valid values are preserved unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three independent root causes: 1. Module-scoped column/index caches in document-scalar-schema.ts carry over between test files — a fresh in-memory SQLite DB sees cached column names from a prior test's DB and skips ALTER TABLE, leaving q_* columns absent. Fix: export resetScalarSchemaCache() and call it in createTestD1() and migrations-d45.test.ts beforeEach. Fixes: q_tst_rating, q_tst_sort_order, q_media_folder, q_apikey_hash, q_apikey_user_id, q_blog_difficulty missing-column failures. 2. _pluginStatusCache (module-scoped Map) in plugin-middleware.ts persists across tests — first test caches 'my-plugin' → true, later tests hit the cache instead of the mock. Fix: call invalidatePluginStatusCache() in beforeEach of affected describe blocks. 3. c.executionCtx getter throws 'This context has no ExecutionContext' in Hono test env (app.request()), crashing the cache-enabled path in api.ts GET /:collection before scheduleKvWrite could guard on !globalKv. Fix: safeCtx(c) helper wraps the getter in try/catch; scheduleKvWrite accepts null/undefined ctx and returns early. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
2 participants
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.
Description
The
number,date, anddatetimecases indynamic-field.template.tsrender the stored value unescaped (value="${value}") while every other field type escapes it. Content field values are stored raw by the write path, so an authenticated content writer can persist"><script>…in one of these fields and have it execute in another admin's content-edit form (type="date"/"number"doesn't help — the payload breaks out of the tag during HTML parsing, before the input validates its value).Fixes #1026
Changes
number/date/datetimecases:value="${escapeHtml(String(value ?? ''))}"— matching theescapeHtml(...)every other field type already uses. TheString(...)coercion is required becauseescapeHtmlreturns''for non-strings, so a numeric value survives (a valid42stays"42"; a valid date stays unchanged).dynamic-field.test.ts: XSS payload → escaped; valid numeric/date/datetime values → preserved; null/undefined → empty string.Testing
tsc --noEmitclean. Regression tests added in-repo covering:Unit Tests
q_apikey_*columns is pre-existing onmain, unrelated to this change)E2E Tests
Checklist