Repository navigation
feat: sanitized rich text field labels - #89
Merged
Merged
Conversation
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.
What this does
Lets a form field label hold formatted text (headings, paragraphs, lists, bold, links), so long agreements like CHECK's STEPS agreement (form 25, "Please Read and Sign Student Name") are readable. Every rich label is sanitized, and plain labels render exactly as before.
Support\FieldLabelholds the label rules.symfony/html-sanitizer. It keeps safe elements and http, https, mailto, tel and relative links. It dropsscript,style,iframe,form,img, event handlers,styleattributes, andjavascript:anddata:URLs, and addsrel="noopener noreferrer"to links.fi-prose, so Filament's typography styles them.Show) usesFieldLabel::toHtml()for field and repeater sub-field labels.new HtmlString()unsanitized.labelcolumn, so there's no migration.fieldas the raw label. Apps that match entries against fields or rubrics by label (CHECK grades tests this way) keep working.symfony/html-sanitizer: ^7.0|^8.0torequire.Blast radius
new HtmlString($label)). Tests cover this across text, textarea, select, checkbox, checkbox list, radio, toggle, date, rich editor and heading fields, with labels that use&, entities, quotes, unicode and multiple lines.<strong>,<em>,<b>,<i>,<u>,<br />) render unchanged.<that looks like a tag (for example<name>) is now treated as HTML.labelcolumn, entry JSON, rubric/test matching in host apps: unchanged.How it was tested
vendor/bin/pest: 126 passed. The new tests are intests/FieldLabelTest.php:vendor/bin/pint: clean.vendor/bin/phpstan: clean. 4.x already fails PHPStan in CI on one larastan error (FormSubmissionNotificationpasses the namespaced view toContentas a plain string, not aview-string). This PR adds that error tophpstan-baseline.neonand makes the PHPStan workflow run when the baseline changes.Human test steps
rel="noopener noreferrer".<p onclick="alert(1)">x</p><script>alert(1)</script>and load the form. No script runs, and onlyxrenders.Bug replication
I ran this PR's
tests/FieldLabelTest.phpagainst4.x, with none of the fix. These tests push each XSS payload throughShow::getFormSchema()and read the label that gets rendered. On 4.x the payload comes through untouched, so all 24 cases fail: 12 payloads, each with a TEXT field and a CHECKBOX field.strips scripts, event handlers and dangerous links from labels in the form schema:Expecting 'name<script>alert(1)</script>' not to contain '<script'Expecting '<img src=x onerror="alert(1)"> name' not to contain 'onerror'Expecting '<svg onload="alert(1)"></svg>name' not to contain 'onload'Expecting '<a href="javascript:alert(1)"…ck</a>' not to contain 'javascript:'Expecting '<a href="data:text/html;base6…ck</a>' not to contain 'data:'Expecting '<iframe src="https://evil.tes…e>name' not to contain '<iframe'Expecting '<p onclick="alert(1)">name</p>' not to contain 'onclick'sanitizes rich labels in the form schema: expected<div class="fi-prose"><h2>Agreement</h2><p>Sign below</p></div>, got<h2>Agreement</h2><p>Sign below<script>alert(1)</script></p>.The plain-label schema tests pass on 4.x, as they should: plain labels render exactly as before. The full suite passes on this branch, 138 tests.