fix(web): type-safe peer_color blocks css inject#399
Merged
Conversation
`peer_color` took `&str` so future caller could pass attacker string into inline `style=`. Now takes `&EndpointId`. Add `peer_color_from_str` for legacy String call sites; falls back to neutral grey on parse fail. Add charset test locking output to digits + `hsl(),% `. Closes #191
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
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.
Why
peer_color(&str)build inlinestyle="color: hsl(...)". Output today only made of digits +hsl(),%so safe — but signature take any string. Future bad caller pass attacker text → CSS injection (data exfil, layout break, position:absolute overlay). Type signature lie about what input safe.Fix
Pick Option C: change
peer_color(&EndpointId) -> String. Bad string can't even reach func — compiler block it. Hash still over base32 form so colors stable for valid peer ids.Two old caller hold
String(member_list, profile_card). For them add thin wrapperpeer_color_from_str(&str) -> String— parse toEndpointId, fall back to neutral greyhsl(0, 0%, 70%)on garbage. Wrapper small + isolated; if thoseStringupstreams ever get retyped toEndpointId, drop the wrapper.Runner-up rejected: Option B (CSS custom property
--peer-hue). Need CSS edits across the styles, bigger diff, no extra type guarantee — just smaller injection slot. Option C kill the surface at the type, smaller blast radius.Test
peer_coloroutput (only digits +hsl(),%).id.to_string()→ same color as typed).Verify
cargo test -p willow-web --lib peer_color— 4 new tests pass.cargo clippy --workspace --all-targets -- -D warnings— clean.cargo fmt --check— clean.just check-wasm— clean.cargo test --workspaceclean exceptwillow-actor::ask_dead_actor_returns_closed, a pre-existing flake unrelated to web/CSS — passes in isolation.Closes #191
Generated by Claude Code