Review
Deterministic escaping pass
Adds emission-time escaping and the injection fixture suite.
- Files changed
- 12
- Additions
- 418
- Deletions
- 96
- Review cards
- 4
Changed files
| Path | Status | Additions | Deletions |
|---|---|---|---|
src/escape.ts | added | +88 | −0 |
src/emit.ts | modified | +140 | −61 |
tests/security/injection.test.ts | added | +190 | −0 |
docs/adr/0001-page-spec-compiler-boundary.md | modified | +0 | −35 |
Escaping strategy
Before
- Escape applied while parsing
- Context assumed safe after validation
- Attribute values escaped once
After
- Escape applied at emission per context
- URL schemes allowlisted per action
- Embedded JSON serialized for script context
Risk assessment
| Area | Impact | Likelihood | Note |
|---|---|---|---|
| Escaping regression | high | low | Covered by the injection fixture suite. |
| Output byte drift | medium | medium | Expected once; snapshots update in the same commit. |
| Performance | low | low | Escaping is linear in output size. |
Review cards
good
Emission-time escaping
Context is explicit at the call site, which removes a whole class of ordering bugs.
concern
Snapshot churn
Every escaping change rewrites snapshots; keep the diff reviewable per block type.
question
URL policy source
Confirm the allowlist lives in one module and is reused by actions and widgets.