fix(security): promote unknown bare elements for element bindings - #490
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f428e5a7e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Brooooooklyn
force-pushed
the
fix/element-binding-ns-promotion
branch
from
September 22, 2026 17:22
f428e5a to
69726e0
Compare
Upstream routes element bindings through `calcPossibleSecurityContexts` (`binding_parser.ts`), which promotes a bare element missing from the DOM schema to its `:svg:`/`:math:` form — the same promotion our host-binding path already applies for selectors. Our element-binding path looked up the verbatim name, so `<animate [attr.to]>` missed `:svg:animate|to` and emitted no `ɵɵvalidateAttribute`. `security_context` now shares `collect_namespaced_contexts` / `collect_bare_contexts` via `element_security_context_for`, which keeps the numerically lowest context — matching upstream's `securityContexts[0]` after its enum sort. That preserves `NONE` for `tagName === null` selectorless hosts, which expand over every known element upstream. Upstream quirks preserved: - Promotion exists only alongside the namespaced schema (19.2.23 / 20.3.22 / 21.2.14 / v22). On earlier versions the whole selector goes through `CssSelector.parse` verbatim, so `:svg:animate` still parses to element `animate` and hits the bare `animate|to` key — `<svg><animate [attr.to]>` keeps `ɵɵvalidateAttribute` at 21.2.7. - `:xml:iframe` on pre-namespaced versions likewise parses to `iframe` and hits `iframe|src`; only namespaced versions split the prefix, and only normalizer versions (19.2.23 / 20.3.22 / 21.2.15+) strip it back — 21.2.14 alone misses, matching upstream.
Brooooooklyn
force-pushed
the
fix/element-binding-ns-promotion
branch
from
September 23, 2026 02:24
69726e0 to
6d45cec
Compare
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.
Summary
Follow-up to #334. A post-merge adversarial review found one real divergence from the vendored v22.0.0 baseline: element bindings skip the unknown-element promotion that
calcPossibleSecurityContextsapplies.Upstream
createBoundElementProperty(binding_parser.ts) computes security viacalcPossibleSecurityContexts(registry, elementSelector, …), which rewrites a bare element missing from the DOM schema to its:svg:/:math:form (animate→:svg:animate) whenever the namespaced schema exists (19.2.23 / 20.3.22 / 21.2.14 / v22+). Our element-binding path passed the verbatim resolved name to the schema lookup, so<animate [attr.to]>resolvedanimate|to→ miss →None, where upstream resolves:svg:animate|to→AttributeNoBinding→ɵɵvalidateAttribute.Changes
element_security_context_for()routes element bindings through the same selector machinery as host bindings (collect_namespaced_contexts/collect_bare_contexts), so both paths share the promotion,:ns:splitting, and:nothandling.Upstream quirks preserved
splitNsName: the whole selector goes throughCssSelector.parse, so:svg:animateparses to elementanimateand hits the bareanimate|tokey —<svg><animate [attr.to]>keepsɵɵvalidateAttributeat 21.2.7.:xml:iframeon pre-namespaced versions likewise parses toiframeand hitsiframe|src. Namespaced versions split the prefix; only normalizer versions (19.2.23 / 20.3.22 / 21.2.15+) strip it back — 21.2.14 alone looks up:xml:iframe|srcverbatim and misses.Tests
test_element_binding_promotes_unknown_bare_element(promotion on/off by version,:math:local-name promotion, known-element passthrough).test_svg_animation_attr_binding_validates_on_namespaced_schemaasserting emittedɵɵvalidateAttributefor<animate [attr.to]>on v22 and 21.2.14, and its absence on 21.0.1.Out of scope (post-v22.0.0 upstream changes)
The same review flagged upstream changes that landed after our vendored v22.0.0 and are tracked separately:
credentiallessiframe sink (22.0.2), namespaced→default schema fallback (22.0.2), MathML*|hrefwildcard (22.0.1), concrete-host scan incalcHostBindingSecurityContexts(22.1.0), i18non*rejection (22.1.0),:math:scriptstripping (22.1.2), bare→svg fallback incheckSecurityContext(22.1.5). None apply to the v22.0.0 baseline this repo vendors; covering them would need finer-grainedangular_versiongating and is a separate decision.