Escape all CSDL attribute values on emit; decode entity references on parse (ARN-237)

[NALLSUR]

Summary

Emitter half of ARN-237: CSDL metadata emitted by emit_csdl_xml applied XML escaping to only 4 of ~30 attribute interpolation sites, so agent- or user-influenced identifiers (entity/property names, namespaces, DefaultValue, navigation-binding Path/Target, annotation terms, action/function references) could close their attribute and inject arbitrary markup — the same defect class as ARN-172. A property named Name"/><Property Name="Smuggled produced a second property element; a smuggled HasStream="true" altered the typed model on re-parse.

Two commits, each independently green (bisect-safe):

1. Parser: decode XML entity references in attr_str (crates/temper-spec/src/csdl/parser/xml.rs) — attr_str returned the raw, still-escaped attribute bytes, so &/"/
 surfaced as literal escape text in the typed model. This is load-bearing for commit 2, not incidental: the old emitter and parser bugs were compensating, and fixing emitter escaping alone would double-escape on every parse/emit/parse cycle (& → & → &). No fixture in the repo contains an entity reference, so this commit is a behavioral no-op for the existing corpus; it is separated purely to keep the shared read-path change reviewable on its own.
2. Emitter: escape every attribute value (crates/temper-spec/src/csdl/emit.rs) — every interpolated string now routes through xml_escape; the only remaining raw interpolations are numeric types (i64, u32, f64, bool), which cannot produce a metacharacter. xml_escape additionally escapes tab/LF/CR as character references (XML attribute-value normalization otherwise silently rewrites them to spaces on re-parse) and is now a single pass instead of five chained replace calls.

Tests

• adversarial_identifiers_do_not_inject_markup — builds a document whose namespace, entity-type name, key, property name, DefaultValue, annotation term, container name, entity-set name/type, navigation-binding path/target, and action/function imports all carry "><Injected/>-style payloads; asserts no live markup is emitted, the output re-parses, and the typed model survives the round trip byte-identical (including that the smuggled HasStream="true" does not take effect). Satisfies the ARN-237 acceptance line "parse/emit/parse round trips preserve the typed model for adversarial XML characters".
• whitespace_in_attribute_values_round_trips — pins the tab/LF/CR character-reference behavior.

Verification

• cargo test -p temper-spec: 269 + 3 integration, green; verified each commit passes independently
• cargo test -p temper-server (primary CSDL consumer, ~20 test targets incl. DST suites): ~750 passed, 0 new failures. The 2 failures in spec_validate_endpoint reproduce on unmodified main — root-caused to observe feature gating and filed as ARN-281
• cargo fmt --check and cargo clippy --all-targets clean
• Mandatory code review: PASS WITH FINDINGS; the test-coverage finding was addressed by extending the adversarial fixture, the other rides with the deferred work below

Deliberately deferred — strict-parser half of ARN-237

Rejecting truncated schemas (EOF-as-success), mandatory attributes, exact numeric parsing, unknown-structure rejection, and reference validation are not in this PR. Strictness strictly increases the set of persisted schemas that fail to load, which interacts directly with ARN-190 (one corrupt tenant CSDL aborts the entire registry restore). Sequencing it after ARN-190 avoids turning currently-booting servers into non-booting ones. One reviewer finding rides with it: unescape_value().ok() silently drops an optional attribute whose value contains a malformed entity reference; mandatory attributes still fail loudly via required_attr. The proper fix (attr_str → Result) is the strict-parser work.

Tradeoff, plainly: until the strict half lands, malformed CSDL still parses permissively — this PR only guarantees that what we emit is well-formed and injection-free, and that legal escaped content round-trips faithfully.

🤖 Generated with Claude Code

Greptile Summary

The PR hardens CSDL XML round trips by:

• decoding XML entity and character references when parsing attribute values
• escaping every string interpolated into emitted XML attributes
• preserving tab, newline, and carriage-return characters through attribute normalization
• adding adversarial injection and whitespace round-trip coverage

Confidence Score: 3/5

The parser's silent handling of malformed entity references needs to be fixed before merging because it can erase optional CSDL data.

The new attribute decoding path converts unescape errors to absence, so malformed values silently select defaults or drop annotations instead of preserving or reporting the input error.

crates/temper-spec/src/csdl/parser/xml.rs

Important Files Changed

Filename	Overview
crates/temper-spec/src/csdl/emit.rs	Routes all emitted string attributes through centralized XML escaping while preserving XML-normalized whitespace with character references.
crates/temper-spec/src/csdl/parser/xml.rs	Decodes legal XML references, but converts decoding errors into absent attributes and thereby permits silent typed-model changes.
crates/temper-spec/src/csdl/emit_test.rs	Moves existing emitter tests into a dedicated module and adds broad injection and whitespace round-trip regression coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[CsdlDocument] --> B[emit_csdl_xml]
    B --> C[xml_escape all string attributes]
    C --> D[Well-formed CSDL XML]
    D --> E[parse_csdl]
    E --> F[attr_str unescapes references]
    F --> G[Round-tripped CsdlDocument]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/temper-spec/src/csdl/parser/xml.rs:51
**Malformed entities erase optional attributes**

When an optional attribute contains a malformed entity reference, converting `unescape_value()` errors to `None` makes the parser treat the present attribute as absent, causing annotations to be dropped or values such as nullability, defaults, paths, and targets to silently revert to fallback values.

_{Reviews (1): Last reviewed commit: "Escape every attribute value emitted in ..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Context used - CLAUDE.md (source)
• Knowledge Base — Temper spec format, codegen, and verification

[greptile-apps]

Malformed entities erase optional attributes

When an optional attribute contains a malformed entity reference, converting unescape_value() errors to None makes the parser treat the present attribute as absent, causing annotations to be dropped or values such as nullability, defaults, paths, and targets to silently revert to fallback values.

Context Used: CLAUDE.md (source)

Knowledge Base Used: Temper spec format, codegen, and verification

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/temper-spec/src/csdl/parser/xml.rs
Line: 51

Comment:
**Malformed entities erase optional attributes**

When an optional attribute contains a malformed entity reference, converting `unescape_value()` errors to `None` makes the parser treat the present attribute as absent, causing annotations to be dropped or values such as nullability, defaults, paths, and targets to silently revert to fallback values.

**Context Used:** CLAUDE.md ([source](https://github.com/nerdsane/temper/blob/main/CLAUDE.md))

**Knowledge Base Used:** [Temper spec format, codegen, and verification](https://app.greptile.com/arni-labs/-/custom-context/knowledge-base/nerdsane/temper/-/docs/temper-spec.md)

How can I resolve this? If you propose a fix, please make it concise.