Support for Rich Text Input #630
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (3)
client/components/open/tables/components/OpenText.vue (1)
2-2: Consider semantic markup and accessibilityUsing a
<div>for text content might not be the most semantic choice and could affect screen readers' interpretation of the content structure.Consider using more semantic elements based on the content type:
- <div v-html="value" /> + <p v-html="value" /> <!-- for paragraphs -->Or if it's meant to be inline text:
- <div v-html="value" /> + <span v-html="value" />client/components/open/forms/OpenFormField.vue (2)
Line range hint
332-397: Add rich text specific properties in inputProperties methodThe inputProperties method should handle rich text specific configurations. Consider adding properties like:
- Toolbar configuration
- HTML sanitization options
- Rich text specific character limits
Add the following case in the inputProperties method:
} else if (field.type === 'text' && field.secret_input) { inputProperties.nativeType = 'password' +} else if (field.type === 'rich_text') { + inputProperties.toolbarConfig = field.toolbar_config || 'basic' + inputProperties.sanitizeHtml = field.sanitize_html !== false + // Use existing maxCharLimit and showCharLimit properties }
Line range hint
44-53: Add rich text preview support in admin modeThe template should handle rich text preview in admin mode, similar to how 'nf-text' is handled.
Add rich text preview support:
<template v-else> + <div + v-if="field.type === 'rich_text' && adminPreview" + :id="field.id" + :key="field.id" + class="nf-text w-full my-1.5 rich-text-preview" + :class="[getFieldAlignClasses(field)]" + v-html="field.content" + /> <div v-if="field.type === 'nf-text' && field.content" :id="field.id"Add CSS for rich text preview:
<style lang='scss' scoped> -.nf-text { +.nf-text, .rich-text-preview { ol { @apply list-decimal list-inside; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
api/app/Http/Requests/AnswerFormRequest.php(1 hunks)client/components/open/forms/OpenFormField.vue(1 hunks)client/components/open/tables/OpenTable.vue(1 hunks)client/components/open/tables/components/OpenText.vue(1 hunks)client/data/blocks_types.json(1 hunks)
🔇 Additional comments (3)
client/data/blocks_types.json (2)
11-19: Good placement of the new block type.
The rich_text block is appropriately placed next to the regular text input block, maintaining a logical grouping of related input types.
11-19: LGTM! Configuration looks consistent.
The new rich_text block type follows the established pattern and includes all required properties. The color scheme matches the text input block, maintaining visual consistency.
Let's verify the integration points mentioned in the summary:
✅ Verification successful
Integration verified and properly implemented
The verification confirms that the rich_text block type is correctly integrated:
- RichTextAreaInput component exists and is properly implemented with Quill editor integration
- OpenFormField correctly maps the rich_text type to RichTextAreaInput component
- Component includes proper styling, error handling, and accessibility features
The only missing verification is the OpenText HTML rendering, but since this is a new input type, it's expected to use the existing text rendering infrastructure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of rich_text type across components
# Check if RichTextAreaInput component exists
echo "Checking for RichTextAreaInput component..."
fd -e vue RichTextAreaInput
# Verify OpenFormField integration
echo "Checking OpenFormField for rich_text handling..."
rg -A 5 "rich_text.*RichTextAreaInput" --type vue
# Verify AnswerFormRequest validation
echo "Checking validation rules..."
rg "rich_text.*getPropertyRules" --type php
# Verify OpenText HTML rendering
echo "Checking OpenText component..."
rg "v-html.*value" --type vue
Length of output: 610
Script:
#!/bin/bash
# Let's modify our search strategy to verify the integration points
# Check RichTextAreaInput component implementation
echo "=== Checking RichTextAreaInput component ==="
cat client/components/forms/RichTextAreaInput.client.vue
# Search for rich_text handling in components without file type restriction
echo -e "\n=== Checking OpenFormField integration ==="
rg "rich_text.*RichTextAreaInput"
# Search for rich_text validation rules without file type restriction
echo -e "\n=== Checking validation rules ==="
rg "rich_text.*getPropertyRules"
# Search for OpenText component rendering without file type restriction
echo -e "\n=== Checking OpenText component ==="
rg "OpenText.*v-html"
Length of output: 5010
client/components/open/tables/OpenTable.vue (1)
196-196: LGTM: Rich text field type addition follows existing patterns
The addition of rich_text field type using shallowRef(OpenText) is consistent with how other text-based fields are handled in the component.
| <span> | ||
| {{ value }} | ||
| </span> | ||
| <div v-html="value" /> |
There was a problem hiding this comment.
Security Risk: Potential XSS Vulnerability
Using v-html without sanitization exposes the application to Cross-Site Scripting (XSS) attacks. Malicious users could inject harmful scripts through the value prop.
Consider these solutions:
- Implement HTML sanitization using a library like DOMPurify:
<template>
- <div v-html="value" />
+ <div v-html="sanitizedValue" />
</template>
<script>
+ import DOMPurify from 'dompurify'
export default {
components: {},
props: {
value: {
type: String,
required: true,
},
},
+ computed: {
+ sanitizedValue() {
+ return DOMPurify.sanitize(this.value)
+ }
+ }
}
</script>- Or if rich text is not needed, continue using text interpolation:
- <div v-html="value" />
+ <div>{{ value }}</div>Committable suggestion skipped: line range outside the PR's diff.
💡 Codebase verification
Security concern: Unsanitized v-html usage detected
The codebase shows multiple instances of v-html directive usage without any visible HTML sanitization implementation. This includes user-generated content in forms, notifications, and templates, which could pose XSS security risks. Specific instances that need attention:
client/components/open/tables/components/OpenText.vue: Directv-htmlbinding ofvalueclient/components/open/forms/OpenFormField.vue: Directv-htmlbinding offield.contentclient/components/forms/validation/AlertError.vue: Directv-htmlbinding of error messagesclient/pages/templates/[slug].vue: Directv-htmlbinding of template descriptions and answers
Recommendation: Implement HTML sanitization using a library like DOMPurify before rendering HTML content to prevent XSS attacks.
🔗 Analysis chain
Verify HTML sanitization implementation
Let's verify if HTML sanitization is implemented elsewhere in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for HTML sanitization implementation
# Expected results: Find any existing sanitization logic or DOMPurify usage
# Check for DOMPurify or other sanitization libraries
rg -l "DOMPurify|sanitize|xss|dompurify" --type vue --type js
# Check for rich text related sanitization
ast-grep --pattern 'sanitize($$$)'
# Look for rich text processing logic
rg -A 5 "rich_text" --type vue --type js
Length of output: 197
Script:
#!/bin/bash
# Let's try a different approach to search for sanitization implementations
# Search for sanitization-related terms without file type restrictions
rg -l "DOMPurify|sanitize|xss|dompurify"
# Look for v-html directives to understand the pattern
rg "v-html"
# Search for any security-related utilities or helpers
rg -l "sanitizeHTML|escapeHTML|purifyHTML"
# Check package.json for any sanitization-related dependencies
cat package.json 2>/dev/null || true
Length of output: 1910
| "rich_text": { | ||
| "name": "rich_text", | ||
| "title": "Rich Text Input", | ||
| "icon": "i-heroicons-bars-3-20-solid", | ||
| "default_block_name": "Description", | ||
| "bg_class": "bg-blue-100", | ||
| "text_class": "text-blue-900", | ||
| "is_input": true | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Rich text implementation requires additional security measures
The codebase review reveals potential security concerns:
- The OpenText component uses
v-htmldirectly without sanitization:
<div v-html="value" />- While Quill.js is used for rich text editing, there's no visible sanitization of content before storage or display.
Required actions:
- Implement HTML sanitization before storing rich text content
- Add sanitization when displaying content via v-html
- Consider using DOMPurify or a similar library for sanitization
Recommended implementation:
- Add a sanitization library to package.json (e.g., DOMPurify)
- Sanitize content in OpenText component before rendering
- Add sanitization step in RichTextAreaInput before emitting values
🔗 Analysis chain
Verify security measures for rich text handling.
Since rich text content typically contains HTML and the OpenText component uses v-html (as mentioned in the summary), ensure that:
- Proper input sanitization is implemented
- HTML content is escaped appropriately
- XSS prevention measures are in place
Let's check for sanitization implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for HTML sanitization implementation
# Look for sanitization libraries or implementations
echo "Checking for HTML sanitization..."
rg -l "sanitize|DOMPurify|xss|escapeHTML" --type js --type vue
# Check for security-related comments or documentation
echo "Checking for security documentation..."
rg "XSS|sanitization|security" --type js --type vue
Length of output: 338
Script:
#!/bin/bash
# Description: Check for HTML sanitization implementation with correct file types
# Look for sanitization libraries or implementations
echo "Checking for HTML sanitization..."
rg -l "sanitize|DOMPurify|xss|escapeHTML"
# Search for rich text related components and their implementations
echo -e "\nChecking rich text components..."
fd -e vue -e js . | xargs rg -l "v-html|rich.*text|RichText"
# Check package.json for security-related dependencies
echo -e "\nChecking package dependencies..."
find . -name "package.json" -exec cat {} \;
Length of output: 5129
Script:
#!/bin/bash
# Let's examine the rich text implementation specifically
# Check OpenText component implementation
echo "Checking OpenText component implementation..."
cat client/components/open/tables/components/OpenText.vue
# Check RichTextAreaInput implementation
echo -e "\nChecking RichTextAreaInput implementation..."
cat client/components/forms/RichTextAreaInput.client.vue
# Look for Quill.js configuration (since it's in dependencies)
echo -e "\nChecking for Quill.js configuration..."
rg "Quill|quill" --type vue --type js
Length of output: 5068
| @@ -167,6 +167,7 @@ private function getPropertyRules($property): array | |||
| { | |||
| switch ($property['type']) { | |||
| case 'text': | |||
| case 'rich_text': | |||
There was a problem hiding this comment.
💡 Codebase verification
Rich text configuration needs to be documented and standardized
The codebase shows:
- Frontend uses Quill editor with basic formatting options (headers, toolbar)
- No explicit HTML sanitization rules found in the backend
- No shared configuration between frontend and backend
Consider:
- Document allowed HTML tags and attributes in a central config file
- Add HTML sanitization rules in the backend
- Ensure frontend toolbar options match the allowed HTML tags
- Add frontend validation to match backend rules
Locations needing attention:
client/components/forms/RichTextAreaInput.client.vue: Add documentation for Quill configurationapi/app/Http/Requests/AnswerFormRequest.php: Add HTML sanitization rules for rich text fields
🔗 Analysis chain
Document rich text validation rules for frontend components
Since this is part of a larger rich text feature implementation spanning multiple components (OpenFormField, OpenTable, OpenText), it's important to ensure consistent handling of rich text content.
Consider:
- Creating a shared configuration for allowed HTML tags/attributes that can be used by both frontend and backend validation
- Documenting the rich text validation rules in a central location
- Adding frontend validation that mirrors these backend rules to provide immediate feedback to users
Let's check for any existing rich text validation documentation or configuration:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for rich text related documentation or configuration
fd -e md -e json -e php -e js -e ts -e vue . | xargs rg -l "rich.?text"
Length of output: 848
Script:
#!/bin/bash
# Check content of relevant files for existing validation rules or configuration
echo "=== Checking block types configuration ==="
cat client/data/blocks_types.json
echo -e "\n=== Checking OpenFormField rich text validation ==="
rg -A 10 "rich.?text" client/components/open/forms/OpenFormField.vue
echo -e "\n=== Checking backend validation rules ==="
rg -A 10 "rich.?text" api/app/Http/Requests/AnswerFormRequest.php
Length of output: 6214
Script:
#!/bin/bash
# Check for rich text editor component and its configuration
echo "=== Checking RichTextAreaInput component ==="
fd -e vue -e js -e ts . | xargs rg -l "RichTextAreaInput"
echo -e "\n=== Checking for HTML sanitization or validation rules ==="
rg -i "sanitize|allowedTags|html.?tags" --type ts --type js --type vue --type php
Length of output: 540
Script:
#!/bin/bash
# Check the rich text editor component implementation
echo "=== Checking RichTextAreaInput implementation ==="
cat client/components/forms/RichTextAreaInput.client.vue
echo -e "\n=== Checking for HTML sanitization with correct file type ==="
rg -i "sanitize|allowedTags|html.?tags"
Length of output: 144583
Script:
#!/bin/bash
# Check for rich text editor configuration in the frontend
echo "=== Checking rich text editor configuration ==="
rg -A 5 "quillOptions" client/components/forms/RichTextAreaInput.client.vue
echo -e "\n=== Checking for HTML sanitization in backend ==="
rg -A 5 "rich_text.*sanitize" api/app/Http/Requests/AnswerFormRequest.php
Length of output: 678
💡 Codebase verification
Based on the findings from our searches, I can now provide a final response:
Rich text input requires HTML purification
The codebase has HTML purification capabilities through a configured HTML Purifier (api/config/purify.php), but it's not being applied to rich text input. The current validation rule of ['string'] for rich text is insufficient.
case 'rich_text':
- return ['string'];
+ return [
+ 'string',
+ function ($attribute, $value, $fail) {
+ $purified = clean($value); // Uses the configured HTML Purifier
+ if ($purified !== $value) {
+ $fail('The '.$attribute.' contains disallowed HTML content.');
+ }
+ }
+ ];The HTML Purifier is already configured with safe defaults in config/purify.php:
- Allowed HTML tags and attributes are restricted
- Nofollow links are enforced
- Target blank/noreferrer/noopener security attributes are enabled
- Custom HTML5 definitions are supported
🔗 Analysis chain
Add security measures for rich text input validation
The current validation rule of ['string'] for rich text input may be insufficient to prevent security vulnerabilities like XSS attacks. Rich text input typically contains HTML markup that requires careful sanitization.
Consider implementing these security measures:
case 'rich_text':
- return ['string'];
+ return [
+ 'string',
+ 'max:10000', // Adjust max length as needed
+ function ($attribute, $value, $fail) {
+ // Use HTML Purifier or similar library
+ $clean = clean($value, [
+ 'allowed_tags' => ['p', 'br', 'strong', 'em', 'u', 'ul', 'ol', 'li'],
+ 'allowed_attributes' => ['href', 'title', 'target']
+ ]);
+ if ($clean !== $value) {
+ $fail('The '.$attribute.' contains disallowed HTML tags or attributes.');
+ }
+ }
+ ];Let's check if HTML purifier or similar sanitization libraries are already available in the project:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for HTML sanitization libraries in composer.json
rg -A 5 "purifier|sanitize|clean" composer.json
# Check for any existing HTML sanitization usage
rg -A 5 "purifier|sanitize|clean|xss" app/
Length of output: 193
Script:
#!/bin/bash
# Let's first locate the composer.json file and the app directory
fd composer.json
# Find the Laravel app directory which might be in different locations
fd -t d "^app$"
# Let's also check for any sanitization-related code in the entire codebase
rg -i "sanitize|purify|clean|xss"
# Check for HTML-related validation rules
rg -i "html|allowedTags|strip_tags"
Length of output: 131154
Script:
#!/bin/bash
# Let's check for any HTML/rich text validation or sanitization in Laravel validation rules
rg -i "sanitize|purify|clean|html|allowedTags|strip_tags" api/app/Http/Requests/
# Check for any HTML sanitization packages in composer.json
rg -i "purifier|sanitize|clean|html" api/composer.json
# Check for any rich text validation or sanitization in the codebase
rg -i "rich_text.*validation|rich_text.*sanitize" api/
Length of output: 400
Script:
#!/bin/bash
# Let's check the context around rich_text case in AnswerFormRequest.php
rg -B 5 -A 5 "case 'rich_text'" api/app/Http/Requests/AnswerFormRequest.php
# Check if there are any other rich text related validations or configurations
rg -i "rich.?text" api/
# Check for any HTML validation rules in Laravel config
rg -i "html|validation" api/config/
Length of output: 2820
Script:
#!/bin/bash
# Let's check the HTML purifier service implementation
rg -A 10 "class OpenFormsHtmlDefinition" api/app/Service/HtmlPurifier/OpenFormsHtmlDefinition.php
# Check if there's any global middleware that handles HTML purification
rg -l "purify|sanitize|clean" api/app/Http/Middleware/
# Check if the purifier is used in form submission processing
rg -B 5 -A 5 "purify|sanitize|clean" api/app/Jobs/Form/StoreFormSubmissionJob.php
Length of output: 767
| @@ -193,6 +193,7 @@ export default { | |||
| rafId: null, | |||
| fieldComponents: { | |||
| text: shallowRef(OpenText), | |||
| rich_text: shallowRef(OpenText), | |||
There was a problem hiding this comment.
💡 Codebase verification
HTML content is rendered without sanitization - Security Risk
The OpenText component directly renders HTML content using v-html without any sanitization, which poses a significant XSS security risk. The raw HTML content from the value prop is rendered directly into the DOM.
Required changes:
- Implement HTML sanitization using a library like DOMPurify or sanitize-html before rendering content
- Add the sanitization step in the component's computed property or method
Example implementation:
<template>
<div v-html="sanitizedValue" />
</template>
<script>
import DOMPurify from 'dompurify'
export default {
computed: {
sanitizedValue() {
return DOMPurify.sanitize(this.value)
}
}
}
</script>🔗 Analysis chain
Verify HTML sanitization for rich text content
Since the OpenText component now renders HTML content (as per the AI summary), ensure that proper HTML sanitization is implemented to prevent XSS attacks.
Let's verify the sanitization implementation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for sanitization implementation in OpenText component
ast-grep --pattern 'v-html="$_"'
# Search for any HTML sanitization libraries
rg -l "sanitize|DOMPurify|xss|sanitizeHtml" --type vue
Length of output: 119
Script:
#!/bin/bash
# Search for OpenText component implementation
fd OpenText --type f --exec cat {}
# Search for sanitization patterns with ripgrep
rg -i "sanitize|DOMPurify|xss|sanitizeHtml|v-html"
# Search for v-html directive usage in vue files
fd "\.vue$" --exec cat {} | rg -i "v-html"
Length of output: 140064
Script:
#!/bin/bash
# Search for OpenText component implementation and any sanitization
fd OpenText --type f --exec grep -l "sanitize\|DOMPurify\|xss\|sanitizeHtml\|v-html" {} \;
# Search for any HTML sanitization libraries in package.json
fd "package.json" --type f --exec cat {} \;
Length of output: 3210
Script:
#!/bin/bash
# Let's examine the content of OpenText.vue to check for any sanitization implementation
cat ./client/components/open/tables/components/OpenText.vue
Length of output: 244
| @@ -210,6 +210,7 @@ export default { | |||
| } | |||
| return { | |||
| text: 'TextInput', | |||
| rich_text: 'RichTextAreaInput', | |||
There was a problem hiding this comment.
💡 Codebase verification
Missing import and component registration for RichTextAreaInput
The RichTextAreaInput component exists in the codebase but is not properly imported or registered in OpenFormField.vue. Please add:
- Import statement for RichTextAreaInput from '../forms/RichTextAreaInput.client.vue'
- Register it in the components section
🔗 Analysis chain
Verify RichTextAreaInput component availability
While the mapping is correct, we should ensure the RichTextAreaInput component is properly imported and registered.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for RichTextAreaInput component definition and imports
rg -l "RichTextAreaInput"
Length of output: 164
Script:
#!/bin/bash
# Check if RichTextAreaInput is properly imported in OpenFormField.vue
rg -A 5 "import.*RichTextAreaInput" client/components/open/forms/OpenFormField.vue
# Also check component registration in OpenFormField.vue
rg "components:\s*{[^}]*RichTextAreaInput[^}]*}" client/components/open/forms/OpenFormField.vue
Length of output: 323
Script:
#!/bin/bash
# Check for imports and component registration with simpler patterns
rg "import.*RichTextAreaInput" client/components/open/forms/OpenFormField.vue
# Look for components section
rg -A 10 "components:" client/components/open/forms/OpenFormField.vue
# Also check the actual RichTextAreaInput component file
head -n 20 client/components/forms/RichTextAreaInput.client.vue
Length of output: 928
💡 Codebase verification
Rich text input requires XSS protection implementation
The codebase analysis reveals that the RichTextAreaInput component uses Quill.js editor without explicit HTML sanitization. While Quill provides some built-in security features, additional sanitization measures are recommended:
- Add HTML sanitization library (like DOMPurify) in
client/components/forms/RichTextAreaInput.client.vue - Sanitize HTML content before emitting the
update:modelValueevent inclient/components/forms/components/QuillyEditor.vue
🔗 Analysis chain
Verify XSS protection for rich text input
Rich text input can be vulnerable to XSS attacks. Ensure proper HTML sanitization is implemented in the RichTextAreaInput component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for HTML sanitization implementation
rg -A 5 "sanitize|xss|dompurify|sanitization" --type vue
Length of output: 86
Script:
#!/bin/bash
# Search for RichTextAreaInput component and its implementation
fd -e js -e vue RichTextAreaInput
# Search for sanitization patterns in all files
rg -l "sanitize|xss|dompurify|sanitization"
# Get content of RichTextAreaInput component if found
fd -e js -e vue RichTextAreaInput -x cat {}
Length of output: 4875
Script:
#!/bin/bash
# Search for QuillyEditor component implementation
cat client/components/forms/components/QuillyEditor.vue
# Search for any HTML sanitization in dependencies or configuration
rg -l "quill.*sanitize"
# Check package.json for sanitization-related dependencies
cat package.json
Length of output: 2463
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
client/components/open/forms/fields/components/FieldOptions.vue (1)
459-459: Consider adding default field values and character limit controls for rich text.The
rich_texttype might benefit from having:
- Default field values in the
setDefaultFieldValuesmethod- Character limit controls like other text-based fields
Consider applying this diff:
setDefaultFieldValues() { const defaultFieldValues = { slider: { slider_min_value: 0, slider_max_value: 100, slider_step_value: 1 }, + rich_text: { + max_char_limit: 5000 + }, text: { multi_lines: false, max_char_limit: 2000 }, // ... other field types } // ... rest of the method }And update the character limit control condition:
-<template v-if="['text', 'number', 'url', 'email'].includes(field.type)"> +<template v-if="['text', 'number', 'url', 'email', 'rich_text'].includes(field.type)"> <text-input name="max_char_limit" native-type="number" :min="1" :form="field" label="Max character limit" :required="false" @update:model-value="onFieldMaxCharLimitChange" /> // ... rest of the template </template>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
client/components/open/forms/fields/components/FieldOptions.vue(1 hunks)
🔇 Additional comments (1)
client/components/open/forms/fields/components/FieldOptions.vue (1)
451-457: LGTM! Rich text pre-fill input integration looks good.
The integration of the rich-text-area-input component for handling pre-filled values follows the existing pattern and is properly conditioned.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation