[ES-2597] [ES-2651] [ES-2708] [ES-2699] [ES-2724] Json-form-builder UI Enhancement to support date validation, radio-buttons, textarea and other UI fixes. #126
Conversation
…ent to support date validation, radio-buttons, textarea and other UI fixes Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
WalkthroughAdds textarea, radio, and file-upload controls; refactors Date and Password components; introduces inline SVG icon constants and MIME helpers; expands multilingual validation and file/base64 handling; adds submit-button gating, recaptcha handling, isFormInitialized flag, and renames maxUploadFileSize → maxFileSizeMB; updates types, styles, exports, and README. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as "File Upload UI"
participant Input as "Hidden File Input"
participant Reader as "FileReader"
participant Validator as "Validator"
participant State as "Form State"
User->>UI: Drop file / Click to upload
UI->>Input: populate FileList
Input->>Reader: read file -> base64
Reader-->>Validator: deliver file (mime, size, data)
alt file valid
Validator->>State: store {value: base64, format, docType?, refId?}
State->>UI: render preview / update UI
UI->>User: show preview
else invalid
Validator->>UI: return error message
UI->>User: display error state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (9)
json-form-builder/README.md (1)
126-128: Consider using more realistic example values for age validation.The
minAge: 2andmaxAge: 3values would restrict the date of birth to people aged 2-3 years old. For documentation purposes, consider using more realistic values likeminAge: 18andmaxAge: 120to better illustrate the intended use case.json-form-builder/src/components/RadioComponent.ts (3)
41-42: Hardcoded language mapping may not scale.The language map
{ en: "eng", km: "khm" }is hardcoded. If additional languages are added, this will need updates. Consider moving this mapping to a shared constant or deriving it fromstate.languageMapfor consistency.
103-104:dataset.originalValueis never set.
selected.dataset.originalValuewill always beundefinedsince the radio element never has this attribute assigned. The fallback toselected.valueworks correctly, but this check is dead code.- const originalKey = selected.dataset.originalValue || selected.value; + const originalKey = selected.value;
104-104: Avoidas anytype assertion.The cast
(options as any)[originalKey]bypasses type checking. Consider properly typingoptionsor using a more specific type assertion to maintain type safety.json-form-builder/src/components/TextareaComponent.ts (1)
58-60: Redundant|| falsein conditional.The expression
field.disabled || falseis equivalent to justfield.disabledsincedisableFieldwon't be called iffield.disabledis falsy anyway.- if (field.disabled || false) { + if (field.disabled) { disableField(textarea); }json-form-builder/src/components/FileUploadComponent.ts (1)
17-24: fileToBase64 lacks error context and may swallow FileReader errors.The Promise rejection passes the raw error object without context. Consider wrapping with a descriptive message. Also,
reader.result!uses a non-null assertion which could be problematic ifonloadfires with a null result (edge case).async function fileToBase64(file: File): Promise<string> { return new Promise((resolve, reject) => { const reader = new FileReader(); - reader.onload = () => resolve(reader.result!.toString().split(",")[1]); - reader.onerror = reject; + reader.onload = () => { + if (reader.result) { + resolve(reader.result.toString().split(",")[1]); + } else { + reject(new Error(`Failed to read file: ${file.name}`)); + } + }; + reader.onerror = () => reject(new Error(`Failed to read file: ${file.name}`)); reader.readAsDataURL(file); }); }json-form-builder/src/JsonFormBuilder.ts (3)
680-682: Inconsistent units:maxFileSizeMBcomment says bytes but name implies MB.The comment states "Default to 5MB given as bytes" with value
5242880, but the property is namedmaxFileSizeMB. This is confusing. The value 5242880 bytes equals 5 MB, so the name is correct but the comment is misleading.- maxFileSizeMB: config.maxFileSizeMB || 5242880, // Default to 5MB given as bytes + maxFileSizeMB: config.maxFileSizeMB || 5, // Default to 5 MBNote: Verify the expected unit in
config.maxFileSizeMBand inFileUploadComponent.tswhere it's used.
827-847: Live validation listeners may cause excessive re-renders.Every
inputevent on any form element triggersrefreshLabels(state), which iterates through all schema fields and updates the DOM. This could cause performance issues in forms with many fields.Consider debouncing the validation or only refreshing the affected field:
+const debounce = (fn: Function, delay: number) => { + let timeoutId: ReturnType<typeof setTimeout>; + return (...args: any[]) => { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => fn(...args), delay); + }; +}; const attachLiveValidationListeners = (state: FormState): void => { + const debouncedRefresh = debounce(() => refreshLabels(state), 150); const inputs = state.container.querySelectorAll( "input, select, textarea, input[type='checkbox'], input[type='radio']" ); inputs.forEach((input) => { input.addEventListener("input", () => { - refreshLabels(state); + debouncedRefresh(); }); input.addEventListener("change", () => { - refreshLabels(state); + debouncedRefresh(); }); }); };
797-800: Non-null assertion on form button query could throw.Line 797 uses
!assertion assuming the button always exists, but line 798 then checksif (!formButton) return;. The assertion is unnecessary and the null check handles the case.- const formButton: HTMLButtonElement = form.querySelector('button[type="submit"]')!; + const formButton = form.querySelector('button[type="submit"]') as HTMLButtonElement | null; if (!formButton) return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
json-form-builder/README.md(3 hunks)json-form-builder/src/JsonFormBuilder.ts(18 hunks)json-form-builder/src/components/DateComponent.ts(3 hunks)json-form-builder/src/components/FileUploadComponent.ts(1 hunks)json-form-builder/src/components/PasswordComponent.ts(11 hunks)json-form-builder/src/components/RadioComponent.ts(1 hunks)json-form-builder/src/components/TextareaComponent.ts(1 hunks)json-form-builder/src/components/index.ts(2 hunks)json-form-builder/src/types.ts(4 hunks)json-form-builder/src/utils/constants.ts(1 hunks)json-form-builder/src/utils/icons.ts(1 hunks)json-form-builder/src/utils/responsive-style.ts(16 hunks)json-form-builder/src/utils/utils.ts(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
json-form-builder/src/components/DateComponent.ts (1)
json-form-builder/src/utils/icons.ts (1)
calendarIconSvg(16-20)
json-form-builder/src/components/FileUploadComponent.ts (4)
json-form-builder/src/utils/utils.ts (10)
mimeToLabel(694-694)getLabelText(677-677)createInfoIcon(682-682)getMultiLangText(678-678)getAcceptString(693-693)emptyInvalidFn(692-692)disableField(685-685)createErrorContainer(687-687)appendError(679-679)handleRequiredValidation(680-680)json-form-builder/src/components/index.ts (1)
createFileUploadField(24-24)json-form-builder/src/types.ts (2)
FormState(130-157)FormField(7-37)json-form-builder/src/utils/icons.ts (3)
uploadIconSvg(1-5)fileIconSvg(11-14)trashIconSvg(7-9)
json-form-builder/src/components/PasswordComponent.ts (1)
json-form-builder/src/utils/icons.ts (2)
eyeIconSvg(27-31)eyeCrossedIconSvg(33-36)
json-form-builder/src/utils/utils.ts (2)
json-form-builder/src/utils/icons.ts (2)
errorIconSvg(22-25)infoIconSvg(38-42)json-form-builder/src/types.ts (1)
Label(5-5)
🪛 ast-grep (0.40.0)
json-form-builder/src/components/RadioComponent.ts
[warning] 28-28: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 28-28: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/components/DateComponent.ts
[warning] 40-40: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: calendarIcon.innerHTML = calendarIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 40-40: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: calendarIcon.innerHTML = calendarIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/components/TextareaComponent.ts
[warning] 32-32: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 32-32: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 48-48: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 82-82: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 85-85: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">Click to upload</span> or drag & drop
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 132-143: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: item.innerHTML = <div class="file-preview-left"> <div class="file-icon">${fileIconSvg}</div> <div class="file-meta"> <div class="file-name">${fileName}</div> <div class="file-size">${fileSizeMB} MB</div> </div> </div> <button type="button" class="file-delete-btn"> ${trashIconSvg} </button>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 48-48: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 82-82: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 85-85: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">Click to upload</span> or drag & drop
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 132-143: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: item.innerHTML = <div class="file-preview-left"> <div class="file-icon">${fileIconSvg}</div> <div class="file-meta"> <div class="file-name">${fileName}</div> <div class="file-size">${fileSizeMB} MB</div> </div> </div> <button type="button" class="file-delete-btn"> ${trashIconSvg} </button>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/JsonFormBuilder.ts
[warning] 334-334: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 364-364: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: labelElement.innerHTML = translatedValue
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 334-334: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 364-364: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: labelElement.innerHTML = translatedValue
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 800-800: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 820-820: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 820-820: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/components/PasswordComponent.ts
[warning] 33-33: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: eyeIconSpan.innerHTML = eyeCrossedIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 36-36: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: eyeIconSpan.innerHTML = eyeIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 30-30: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 33-33: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = eyeCrossedIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 36-36: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = eyeIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/utils/utils.ts
[warning] 98-98: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 98-98: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 242-242: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 250-250: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 242-242: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 250-250: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (30)
json-form-builder/src/utils/responsive-style.ts (2)
62-103: LGTM! Well-structured calendar and error message styling.The calendar icon positioning, date display input styling, and error message layout are properly structured with appropriate use of flexbox and positioning.
110-140: Native CSS nesting is broadly supported in modern browsers as of 2024–2025.Native CSS nesting is now supported in Chrome 120+, Firefox 117+, and Safari 17.2+. However, verify that your project's target browser support aligns with these minimums. If you need to support older browsers (pre-v112/v117/v17.2), configure your build pipeline to transpile nesting syntax using PostCSS or Sass.
json-form-builder/src/types.ts (4)
16-19: LGTM! Control type union properly expanded.The new control types
"radio","textarea", and"fileupload"align with the constants defined inconstants.tsand the new component creators exported from the components index.
33-36: LGTM! New field properties for enhanced form controls.The additions of
minAge,maxAgefor date validation,rowsfor textarea, andmaxFileSizeMBfor file upload are appropriate optional properties that support the new component features.
156-156: LGTM! Form initialization tracking added.The
isFormInitializedproperty enables proper form state tracking, which is useful for conditional rendering and validation timing.
90-90: The form builder already provides a sensible default value formaxFileSizeMBduring initialization. InFormBuilder.ts(line 24), the state is initialized withmaxFileSizeMB: config.maxFileSizeMB ?? DEFAULT_FILE_SIZE_MB, whereDEFAULT_FILE_SIZE_MBis set to 5 MB inconstants.ts. The mismatch between the optional property inFormConfigand the required property inFormStateis properly handled—no action needed.Likely an incorrect or invalid review comment.
json-form-builder/src/components/index.ts (1)
9-11: LGTM! Public API properly extended.The new field creators for textarea, radio, and file upload are correctly imported and exported, maintaining the established barrel file pattern and extending the public API surface consistently.
Also applies to: 22-24
json-form-builder/src/utils/constants.ts (1)
3-11: LGTM! New control types properly added.The additions of
TEXTAREAandRADIOconstants align with the expanded control type union intypes.ts. TheFILEvalue change from"file"to"fileupload"should be verified to ensure all existing usages have been updated across the codebase.json-form-builder/src/components/RadioComponent.ts (2)
29-29: innerHTML usage for label rendering.The static analyzer flags this as a potential XSS risk. However, since
getLabelTextappears to be an internal utility that constructs label HTML from controlled schema data (not user input), this is likely a false positive. If the label content could ever come from untrusted sources, consider using DOMPurify or textContent instead.
18-136: LGTM overall structure and functionality.The radio component correctly handles multi-language support, validation, and form data updates. The DOM structure with wrapper, label, radio group, and error container follows established patterns.
json-form-builder/src/components/TextareaComponent.ts (1)
22-107: Component structure looks good.The textarea component correctly implements validation, error handling, and form data updates. The pattern is consistent with other form components.
json-form-builder/src/utils/icons.ts (1)
1-42: Good centralization of SVG icons.Moving SVG icons to a dedicated module improves maintainability and ensures consistency across components. Since these are static string literals (not user input), they are safe to use with innerHTML.
json-form-builder/src/components/DateComponent.ts (4)
156-158:realInputis appended twice to different parents.
realInputis first appended toinputWrapper(line 65), then appended again directly towrapper(line 157). Since DOM elements can only have one parent, this moves the input frominputWrappertowrapper. If this is intentional for styling/layout purposes, consider adding a comment to clarify. Otherwise, remove the redundant append at line 65.
95-97: Good browser compatibility handling forshowPicker().The try/catch around
showPicker()gracefully handles browsers that don't support this API.
14-160: Overall implementation is well-structured.The composite date picker with read-only display input and hidden native input provides a good UX. The validation, date range constraints, and formatting are handled correctly.
70-88: ClarifyminAge/maxAgesemantics: verify if these values represent years or days.The variable names suggest age in years, but the code uses
addDays(). IfminAgerepresents "minimum age in years" (e.g., 18), the calculation should useaddYears()from date-fns and the logic may need adjustment. If these are intentionally in days, rename them tominDays/maxDaysfor clarity. Check the field type definition to confirm the intended units.json-form-builder/src/components/PasswordComponent.ts (3)
28-38: innerHTML for controlled SVG icons is acceptable.The static analyzer flags these innerHTML assignments, but since
eyeIconSvgandeyeCrossedIconSvgare static string constants from the icons module (not user input), this is safe. The pattern is consistent with other components in this PR.
188-222: Confirm password validation logic is correct.The validation properly handles three cases: required but empty, value mismatch, and valid input. The early return for untouched fields during initial render prevents premature error display.
72-91: Good wrapper structure for input and eye icon.The password-input-wrapper pattern properly groups the input and visibility toggle icon, which improves styling flexibility and DOM organization. This is applied consistently to both password and confirm password fields.
json-form-builder/src/components/FileUploadComponent.ts (2)
174-179: File type validation may fail for files with slightly different MIME types.Browsers sometimes report different MIME subtypes (e.g.,
image/jpgvsimage/jpeg). The strictincludescheck may reject valid files.Consider normalizing common MIME type variations, or verify that the configured
acceptedFileTypesin the schema accounts for these variations.
83-86: Regarding static analysis innerHTML warnings for icons and static content.The static analysis flags innerHTML usage at lines 83 and 86. These are false positives since
uploadIconSvgis a controlled constant fromicons.tsand the string literal is static. No user input flows into these assignments.json-form-builder/src/JsonFormBuilder.ts (3)
157-172: Good addition of centralized submit button state management.The
updateSubmitButtonStatefunction properly combines form validation and reCAPTCHA validation to control button state. This follows the single responsibility principle.
940-957:getFormDataFILE handling looks correct.The logic properly distinguishes between photo (single object) and document (array) modes based on
acceptedFileTypes. The filtering on line 951 ensures only valid objects are included.
334-336: Static analysis innerHTML warning for radio label is a partial concern.
getLabelTextreturns a string that may include HTML (the required asterisk span). While not directly user-controlled, usinginnerHTMLhere is intentional for the required indicator. Consider documenting this decision or sanitizing if label content ever becomes user-configurable.json-form-builder/src/utils/utils.ts (6)
264-269:hideAllInfois called on every showInfo but queries entire document.Calling
document.querySelectorAll(".info-detail.active")on every hover/focus may be inefficient if there are many info icons. For a form builder, this is likely acceptable, but consider caching or a more targeted approach if performance becomes an issue.
410-426: Bidirectional map logic is correct and improved.The refactored
buildBidirectionalLanguageMapnow correctly prevents overwriting existing backward mappings, which handles edge cases where multiple keys map to the same value.
633-674: MIME utilities are well-structured and handle common cases.The
mimeToExtension,getAcceptString, andmimeToLabelfunctions provide good coverage for common document and image types. The fallback logic handles unknown MIME types gracefully.Note: Consider adding
image/jpg→jpegnormalization if browser compatibility requires it.
480-527: Radio validation logic in validateForm is comprehensive.The switch statement properly handles different input types including the new radio case. The logic to reconstruct multilingual values for simpleType radios and single-language values for regular radios is correct.
578-592: Good addition of textarea and radio cases in hasFormData.The control type checks are properly extended to include the new field types.
98-99: Static analysis innerHTML warnings for icon SVGs are false positives.The
errorIconSvg,infoIconSvg, and arrow SVG are all controlled constants defined inicons.ts. No user input flows into these assignments, so the XSS warnings are not applicable.Also applies to: 242-243, 250-251, 320-320
gk-XL7
changed the title
…d issuses fixed. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
json-form-builder/src/components/RadioComponent.ts (1)
28-29: Same innerHTML pattern as other components.This follows the same pattern noted in TextareaComponent. The XSS risk depends on whether schema data is trusted.
🧹 Nitpick comments (3)
json-form-builder/src/types.ts (1)
153-156: Consider makingisFormInitializedoptional.
isFormInitializedis declared as a requiredboolean, but this could cause type errors if the property isn't set during initial state construction. Since it represents an initialization flag that may not exist before the form is fully set up, consider making it optional.- isFormInitialized: boolean; + isFormInitialized?: boolean;json-form-builder/src/components/FileUploadComponent.ts (2)
197-202: Temporal Dead Zone issue:fileListContaineraccessed before declaration.
fileListContaineris declared withletat line 227, but the drop handler (lines 151-224) references it at line 197. Due to JavaScript's Temporal Dead Zone (TDZ), accessing aletvariable before its declaration would throw aReferenceError. However, since the drop handler is a callback executed later (not synchronously), this works in practice because the declaration is hoisted and initialized toundefinedby the time the handler runs.For clarity and to avoid confusion, consider moving the declaration before the drop handler:
+ /* ----------------------- MULTIPLE DOC PREVIEW LIST ----------------------- */ + let fileListContainer: HTMLDivElement | null = null; + uploadArea.addEventListener("click", () => !field.disabled && input.click()); /* ----------------------- DRAG & DROP SUPPORT ----------------------- */ uploadArea.addEventListener("dragover", (e) => {Then remove the duplicate declaration at line 227.
Also applies to: 227-227
17-24: FileReader error handling could be improved.The
fileToBase64function rejects on error but doesn't provide context. Consider wrapping the error with more detail for debugging.async function fileToBase64(file: File): Promise<string> { return new Promise((resolve, reject) => { const reader = new FileReader(); reader.onload = () => resolve(reader.result!.toString().split(",")[1]); - reader.onerror = reject; + reader.onerror = () => reject(new Error(`Failed to read file: ${file.name}`)); reader.readAsDataURL(file); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
json-form-builder/README.md(3 hunks)json-form-builder/src/JsonFormBuilder.ts(18 hunks)json-form-builder/src/components/FileUploadComponent.ts(1 hunks)json-form-builder/src/components/RadioComponent.ts(1 hunks)json-form-builder/src/components/TextareaComponent.ts(1 hunks)json-form-builder/src/types.ts(4 hunks)json-form-builder/src/utils/responsive-style.ts(15 hunks)json-form-builder/src/utils/utils.ts(15 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
json-form-builder/src/JsonFormBuilder.ts (3)
json-form-builder/src/components/TextareaComponent.ts (1)
createTextareaField(22-107)json-form-builder/src/components/RadioComponent.ts (1)
createRadioField(18-148)json-form-builder/src/components/FileUploadComponent.ts (1)
createFileUploadField(36-402)
🪛 ast-grep (0.40.0)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 48-48: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 82-82: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 85-85: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">Click to upload</span> or drag & drop
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 121-121: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 248-248: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: fileIconDiv.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 269-269: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: deleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 48-48: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 82-82: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 85-85: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">Click to upload</span> or drag & drop
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 121-121: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 248-248: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: fileIconDiv.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 269-269: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: deleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/utils/utils.ts
[warning] 97-97: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 97-97: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 233-233: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 241-241: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 233-233: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 241-241: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 801-801: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 821-821: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 821-821: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/components/RadioComponent.ts
[warning] 28-28: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 28-28: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/components/TextareaComponent.ts
[warning] 32-32: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 32-32: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (22)
json-form-builder/src/utils/responsive-style.ts (2)
142-144: Previously flagged issue has been addressed.The
:disabledpseudo-class selector is now correctly using a single colon instead of the invalid::disabledpseudo-element syntax.
247-251: Duplicate CSS definition has been consolidated.The
.file-preview-leftclass now appears only once with the intended properties, resolving the previous duplicate definition issue.json-form-builder/src/types.ts (2)
16-19: New control types added correctly.The addition of
"radio","textarea", and"fileupload"control types aligns with the new component implementations introduced in this PR.
33-36: New field configuration options added.The
minAge,maxAge,rows, andmaxFileSizeMBoptional properties appropriately extendFormFieldto support date validation, textarea sizing, and file upload constraints.json-form-builder/src/components/TextareaComponent.ts (3)
55-55: Previously flagged optional chaining issue has been addressed.The code now correctly uses
state.allowedValues?.[field.id]to safely access the property.
67-95: Input validation logic is well-structured.The validation flow correctly handles required field checks, regex validation, error state management, and custom validity. The pattern of clearing errors before validation and updating
state.lastErrorsis consistent with other components.
32-33: Review manual verification of schema data sources for innerHTML safety.Static analysis flags
label.innerHTML = getLabelText(state, field)as a potential XSS vector. The primary concern is whetherfield.labelNameand related label data originate exclusively from trusted, developer-controlled schema configuration or if any code path allows user input to influence these values. Verify the data flow from schema definition through to rendering, and confirm thatgetLabelTextdoes not process untrusted sources.json-form-builder/src/components/RadioComponent.ts (1)
106-118: SIMPLE_TYPE multi-language value mapping looks correct.The logic correctly maps mandatory languages to their corresponding values from the allowed values object, handling both direct language codes and mapped language codes.
json-form-builder/src/components/FileUploadComponent.ts (4)
62-62: Previously flaggedisPhotoUploadlogic has been corrected.The code now correctly uses
.every()to ensure all types are images before treating the field as single-photo mode.
254-260: XSS vulnerability has been addressed.The file name and size are now safely set using
textContentinstead of being interpolated into innerHTML.
177-179: Memory leak addressed: Object URLs are now revoked.The code now properly checks for and revokes existing blob URLs before creating new ones, preventing memory accumulation on repeated uploads.
Also applies to: 341-343
82-83: innerHTML assignments with controlled constants are safe.The static analysis flags these innerHTML assignments, but they use internal SVG constants (
uploadIconSvg,trashIconSvg,fileIconSvg) which are developer-controlled and safe. No user input is involved.Also applies to: 85-86, 121-122, 248-249, 269-270
json-form-builder/README.md (1)
26-26: Import path corrected.The import statement now correctly uses
"@mosip/json-form-builder", aligning with the npm package scope and the link command shown later in the documentation.json-form-builder/src/JsonFormBuilder.ts (5)
157-172: Good addition for UX.The
updateSubmitButtonStatefunction properly manages the submit button's enabled/disabled state based on both form validity and reCAPTCHA status, improving user experience by providing immediate feedback.
332-371: Radio value handling correctly preserves original keys.The implementation properly:
- Stores the original key in
dataset.originalValue(Line 352)- Updates only the visible label text using
textContent(Line 366)- Never overwrites
radio.valuewith translated textThis addresses the previous review concern about data integrity and XSS.
487-487: Good UX improvement with initialization gating.The
isFormInitializedcheck prevents error messages from displaying prematurely during the initial render, ensuring users only see validation errors after they've had a chance to interact with the form.
937-964: File data handling properly structured.The
getFormDatafunction correctly differentiates between:
- Photo uploads (single object with value/docType/format)
- Document uploads (array of objects)
- Default empty values when no file is selected
The filtering ensures only valid objects are included in the form data.
828-848: Live validation enhances user feedback.The
attachLiveValidationListenersfunction appropriately uses:
inputevents for immediate text field validationchangeevents for dropdowns, checkboxes, and radiosCalled once during initialization (Line 859), it provides real-time validation feedback without accumulating duplicate listeners.
json-form-builder/src/utils/utils.ts (4)
227-294: Excellent accessibility improvements.The reworked
createInfoIconfunction now properly supports:
- Keyboard navigation with
tabIndexand focus events- ARIA attributes for screen readers
- Both hover and focus interactions
- Escape key to dismiss
- Single active popup via
hideAllInfoThe keydown listener is now attached to the icon element (Line 285) rather than globally, addressing the previous concern about accumulating listeners.
401-417: Bidirectional mapping correctly implemented.The updated implementation properly creates two-way language mappings while preventing overwrites by checking
if (!(value in twoWayMap))before adding backward mappings. This ensures both forward and reverse lookups work correctly.
471-521: Radio validation handles multilingual values correctly.The radio button handling in
validateFormproperly:
- Distinguishes between
simpleType(multilingual) and regular radio fields- Preserves original keys via
dataset.originalValue- Creates multilingual arrays for
simpleTypewith normalized language codes- Extracts single values for non-simpleType using the first mandatory language
- Provides appropriate fallbacks
The implementation aligns with the radio component logic from
RadioComponent.ts.
658-699: MIME utilities well-structured.The new MIME utility functions provide clean abstractions:
mimeToExtension: Handles both special office formats and generic MIME patterns with appropriate fallbacksgetAcceptString: Generates proper file input accept attributesmimeToLabel: Provides user-friendly labelsThese utilities support the new file upload functionality effectively.
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
json-form-builder/src/JsonFormBuilder.ts (3)
332-371: Radio label refresh correctly preserves values; consider avoidinginnerHTMLon the group label.The new RADIO block:
- Uses
dataset.originalValueto remember the logical key and keepsradio.valueunchanged, fixing the previous issue where values were overwritten with translated labels.- Updates only the visible label text via
labelElement.textContent, which is safer and avoids XSS/static-analysis noise.For the group label,
mainLabel.innerHTML = getLabelText(state, field);introduces a newinnerHTMLsink. If the label text is not intended to contain markup, prefertextContentinstead to reduce XSS exposure and quiet the static analysis warning:- if (mainLabel) { - mainLabel.innerHTML = getLabelText(state, field); - } + if (mainLabel) { + mainLabel.textContent = getLabelText(state, field); + }(If
getLabelTextintentionally returns HTML, consider sanitizing it or documenting that only trusted config can supply markup.)
429-437: Checkbox/Radio required validation is generally sound; minor redundancy informDataupdate.
- The checkbox required check against
input#${field.id}[type="checkbox"]and use ofsetCustomValidityfor the first radio in the group are conventional and should integrate well withform.reportValidity().- For radios,
state.formData[field.id]is set to the checkedradio.valueinside the loop; the subsequent assignment in theelsebranch (state.formData[field.id] = state.formData[field.id] || "") is effectively a no-op and can be safely removed to simplify the logic.This is a small clean-up only; behavior is otherwise correct.
- } else { - // Ensure formData is always up-to-date for radios - state.formData[field.id] = state.formData[field.id] || ""; - } + }Also applies to: 438-464
828-848: Live validation wiring works but may be heavy on large forms.
attachLiveValidationListenerswires everyinput,select,textarea, checkbox, and radio to callrefreshLabels(state)on bothinputandchange, and you enable this right afterrender(state)while then flippingstate.isFormInitialized = true.Functionally this gives immediate validation feedback and ensures the submit button state stays in sync via
refreshLabels→updateSubmitButtonState. On very large schemas, however, runningrefreshLabels(which walks the entirestate.schemaeach time) for every keystroke could become costly.If performance becomes an issue, consider in the future:
- Scoping validation to the field being changed, or
- Debouncing
refreshLabelsoninputevents.For now, this is acceptable given typical form sizes.
Also applies to: 859-861
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/src/JsonFormBuilder.ts(18 hunks)
🧰 Additional context used
🪛 ast-grep (0.40.0)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 801-801: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 821-821: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 821-821: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (7)
json-form-builder/src/JsonFormBuilder.ts (7)
246-261: Textarea placeholder + validation integration looks correct.Unifying the selector to handle both
input#idandtextarea#id, while skipping DATE control types for placeholder updates, and extending the required/regex validation selector to covertextarea[data-field-id]is consistent and should make TEXTAREA fields behave like other textual inputs. The additional gating of error messages onstate.isFormInitializedavoids noisy errors on initial render.Also applies to: 467-472, 487-507
526-549: ExpandedtriggerAllEventsselector and error lookup look reasonable.Including
textarea,input[type='checkbox'], andinput[type='radio']in the selector ensures those controls participate in the syntheticinput/changetriggering, and the broadened error-container resolution (sibling, parent sibling, closest.form-field/.form-field-group, then parent) should make the “touchedOnly” mode more robust across different layouts.No functional issues spotted here.
891-905: New TEXTAREA, RADIO, and FILE control types are wired correctly intocreateFormElement.The switch extension to route TEXTAREA →
createTextareaField, RADIO →createRadioField, and FILE →createFileUploadFieldis straightforward and consistent with the existing pattern for other control types.No issues spotted here.
682-684: Clarify units formaxFileSizeMBto avoid confusion and potential misuse.
maxFileSizeMBis initialized withconfig.maxFileSizeMB || 5242880and the comment says "Default to 5MB given as bytes", but the property name suggests a value in megabytes, while the default is in bytes. Verify all usages in the FileUpload component and elsewhere to ensure the value is treated consistently (no double conversions), and then either:
- Store the value in MB (e.g., default
5) and convert to bytes where needed, or- Rename the field to clarify it is in bytes (e.g.,
maxFileSizeBytes) and update all references accordingly.
157-172: Submit-button enable/disable flow is good but depends onvalidateFormnot callingrefreshLabels.Using
updateSubmitButtonStateto gate the submit button onvalidateForm(state)andvalidateRecaptcha(state)is a solid UX improvement, and the various call sites (after initial render, after label refresh, and after submit) are well-placed.One caveat:
updateSubmitButtonStateis called from withinrefreshLabels, and it in turn callsvalidateForm. IfvalidateFormever callsrefreshLabelsinternally, this would create a recursion loop. Please verify thatvalidateFormis purely computational (does not re-invokerefreshLabels) or refactor so that one of them only consumesstate.lastErrorswithout re-triggering the other.Also, note that this change makes the button permanently disabled whenever
validateRecaptchareturnsfalse. That's likely desired when reCAPTCHA is enabled, but it relies onvalidateRecaptchareturningtruewhen reCAPTCHA is disabled/misconfigured; otherwise, the button would never be enabled. Please double-check that invariant.Also applies to: 514-515, 785-786, 819-825
767-772: Tighten TypeScript typings forformButtonand prefer safer text updates overinnerHTML.In
validateAndSubmit:
const formButton: HTMLButtonElement = form.querySelector('button[type="submit"]')!;plus the subsequentif (!formButton) return;mixes a non-null assertion with a null-check. It's clearer and safer to rely on the runtime check and drop the!, using the generic form ofquerySelector:- const formButton: HTMLButtonElement = form.querySelector('button[type="submit"]')!; - if (!formButton) return; + const formButton = form.querySelector<HTMLButtonElement>('button[type="submit"]'); + if (!formButton) return;
- For button label updates, you can avoid
innerHTMLwhere markup isn't required:- formButton.innerHTML = ""; + formButton.textContent = ""; @@ - formButton.innerHTML = state.submitLabel; + formButton.textContent = state.submitLabel; @@ - formButton.innerHTML = state.submitLabel; + formButton.textContent = state.submitLabel;The spinner is still appended as a DOM node, so using
textContenthere should not break anything and will address the static analysis XSS warnings aroundinnerHTMLon this element.Also applies to: 798-802, 819-825
938-970: Verify FILE field shape expectations and backend contract alignment for this refactoring.The refactored
getFormDataimplements specific shape guarantees:
- FILE fields with
acceptedFileTypesincludingimage/→ single object{ value, docType, format }(defaults to empty object)- Other FILE fields → filtered array (defaults to empty array)
- Non-FILE fields → stored value or
""- recaptchaToken appended when present
Confirm with backend integration that these exact shapes are expected. Verify:
- Backend endpoints consuming this data anticipate the guaranteed array/object structure for FILE fields
- PHOTO vs FILE distinction is consistently enforced (no FILE field misconfigured as both)
- This payload shape change is backward-compatible or documented as a breaking change
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
json-form-builder/src/utils/utils.ts (1)
175-178: Bug:defaultLangfallback incorrectly usescurrentLanguage.The fallback for
defaultLangshould referencestate.defaultLanguage, notstate.currentLanguage. This appears to be a copy-paste error from thecurrentLangblock above.if (!defaultLang) { defaultLang = - state.languageMap[state.currentLanguage] || state.currentLanguage; + state.languageMap[state.defaultLanguage] || state.defaultLanguage; }
♻️ Duplicate comments (1)
json-form-builder/src/utils/utils.ts (1)
484-484:dataset.originalValueis never assigned inRadioComponent.This line reads
input.dataset.originalValueas a fallback, butRadioComponent.tsnever sets this property when creating radio inputs. The fallback toinput.valuewill always be used, making theoriginalValuecheck dead code.Either remove the unused fallback here, or add
radio.dataset.originalValue = valueKeyinRadioComponent.tswhen creating each radio input.
🧹 Nitpick comments (2)
json-form-builder/src/components/RadioComponent.ts (1)
106-134: Duplicated radio value assembly logic withvalidateForm.The logic for assembling radio values (SIMPLE_TYPE multi-language array vs. single-language fallback) is duplicated between this change handler and
validateForminutils.ts(lines 496-519). Consider extracting a shared helper to ensure consistency and reduce maintenance burden.json-form-builder/src/utils/utils.ts (1)
586-589: RedundanttoLowerCase()call.The
lngvariable is already lowercased on line 587, so the secondtoLowerCase()on line 588 is unnecessary.const normalize = (lng: string) => { lng = lng.toLowerCase(); - return (lng.length === 3 ? langMap[lng] : lng).toLowerCase(); + return lng.length === 3 ? langMap[lng] : lng; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
json-form-builder/src/components/RadioComponent.ts(1 hunks)json-form-builder/src/utils/utils.ts(15 hunks)
🧰 Additional context used
🪛 ast-grep (0.40.0)
json-form-builder/src/components/RadioComponent.ts
[warning] 28-28: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 28-28: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/utils/utils.ts
[warning] 97-97: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 97-97: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 233-233: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 241-241: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 233-233: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 241-241: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
Signed-off-by: Gk <76690271+gk-XL7@users.noreply.github.com>
…d issuses fixed. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
…into develop-patch3
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
json-form-builder/src/utils/utils.ts (1)
585-589: Misleading comment and potential undefined return.The comment on line 585 claims "Normalize to 3-letter codes ALWAYS", but the
normalizefunction actually converts TO 2-letter codes:
- If
lngis 3 letters, it returnslangMap[lng](which maps 3→2 letter codes based onbuildBidirectionalLanguageMap)- If
lngis 2 letters, it returnslngas-isAdditionally, if a 3-letter code is not present in
langMap, the function returnsundefined, which could cause issues downstream.Apply this diff to correct the comment and add a safety fallback:
- // Normalize to 3-letter codes ALWAYS + // Normalize to 2-letter codes via langMap const normalize = (lng: string) => { lng = lng.toLowerCase(); - return lng.length === 3 ? langMap[lng] : lng; + return lng.length === 3 ? (langMap[lng] || lng) : lng; };
🧹 Nitpick comments (2)
json-form-builder/src/utils/utils.ts (2)
475-521: Consider adding inline comments to clarify radio handling logic.The radio button handling correctly distinguishes between
simpleType(multilingual) and regular fields, but the logic is fairly complex and would benefit from brief inline comments explaining:
- Why
simpleTypefields produce a multilingual array- Why non-simpleType fields use only the first mandatory language
- The purpose of the fallback chain on lines 516-518
Example:
if (fieldDef.type === "simpleType") { + // simpleType fields store multilingual arrays state.formData[fieldId] = state.mandatoryLanguages.map((lng) => { const mappedLng = state.languageMap[lng] || lng; return { language: mappedLng.length === 3 ? mappedLng : lng, value: optionLabels?.[lng] || optionLabels?.[mappedLng] || "" }; }); } else { + // Non-simpleType fields use a single language value const mandatoryLangs: string[] = state?.mandatoryLanguages || [];
404-415: Consider documenting assumption about input data.The function correctly creates bidirectional mappings, but if the input map contains conflicting forward mappings (e.g.,
{ "en": "eng", "eng": "fra" }), the second forward mapping will overwrite the backward mapping from the first iteration. While this is unlikely with well-formed language maps, it could lead to unexpected behavior.Consider adding a JSDoc note:
/** * Converts a one-way language map into a two-way map. * This allows for bidirectional lookup where both keys and values are language codes. + * @param {Record<string, string>} oneWayMap A map where keys are language codes and values are their corresponding labels. + * @note Assumes the input map has no conflicting forward mappings (e.g., both "en" → "eng" and "eng" → "fra"). * @returns Two-way map where both keys and values are language codes, allowing for bidirectional lookup. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/src/utils/utils.ts(16 hunks)
🧰 Additional context used
🪛 ast-grep (0.40.0)
json-form-builder/src/utils/utils.ts
[warning] 97-97: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 97-97: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 233-233: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 241-241: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 233-233: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 241-241: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (8)
json-form-builder/src/utils/utils.ts (8)
658-699: MIME utilities look good with minor edge case.The new MIME utility functions are well-structured and handle common document types correctly. The fallback logic in
mimeToExtensionappropriately handles standard MIME patterns.Minor observation: Lines 677-678 use string replacement to clean up XML-related suffixes. This works for standard MIME types but could theoretically mangle uncommon types like
"image/xmldata"→"data". However, this is unlikely in practice since standard MIME types follow predictable patterns.If you encounter issues with specific MIME types in the future, consider expanding the
specialMapor using more targeted replacements:.replace(/\+xml$/, "") .replace(/^xml$/, "")
349-349: Good extension of utility functions for textarea support.The type extensions to include
HTMLTextAreaElementare consistent and correctly implemented acrossenableCapsLockCheck,disableField, andemptyInvalidFn. This properly supports the new textarea control type introduced in this PR.Also applies to: 375-375, 651-651
356-357: Type assertions are safe and appropriate.The type assertions to
MouseEventandKeyboardEventare correct since these callbacks are attached toclickandkeyupevents respectively, which always emit those specific event types.
233-287: Excellent refactoring of info icon interactions.The refactored info icon implementation addresses the previous review concerns effectively:
- ✅ Keyboard listener is now attached to
infoSpanitself (line 285) instead of accumulating global listeners- ✅ Added
tabIndex = 0for proper keyboard accessibility- ✅ The
hideAllInfo()helper ensures only one info popup is visible at a time (good UX)- ✅ Proper use of
aria-hiddenfor screen readersThe interaction model (hover + focus + ESC key) is well-designed and accessible.
548-565: Well-structured refactoring with improved documentation.The rewritten
hasFormDatafunction is a significant improvement:
- ✅ Clearer signature accepting
FormStateinstead of multiple parameters- ✅ Comprehensive JSDoc explaining behavior for different control types
- ✅ Proper support for new control types (textarea on line 605, radio on lines 613-617)
- ✅ Robust multilingual validation for simpleType fields
The logic correctly validates that all mandatory languages contain non-empty values for multilingual fields, while appropriately handling single-value fields.
Also applies to: 603-635
161-161: Good addition of optional defaultLang parameter.Adding the optional
defaultLangparameter with a fallback tostate.defaultLanguage(line 177) makeshandleRegexValidationmore flexible while maintaining backward compatibility.Also applies to: 176-178
189-198: Language-aware validator filtering is well-implemented.The validator filtering logic correctly:
- ✅ Includes validators without a
langCode(line 191) so they apply to all languages- ✅ Normalizes the validator's
langCodeto 3-letter format for consistent comparison- ✅ Filters to validators matching the current language when
useLangCodeis trueThis enables language-specific validation patterns, which aligns with the multilingual form capabilities.
522-523: Verify date input handling in form validation.The
case "date"block is empty and only contains abreakstatement, meaning date input values are not explicitly stored instate.formDatawithin this switch statement.Verify whether:
- Date inputs are intended to be handled by the
fieldId && langblock (lines 454-469) if they have multilingual attributes, OR- Date inputs fall through to the
defaultcase (lines 524-528), OR- Date values are extracted elsewhere in the codebase
If none of the above apply, this could result in date values being excluded from form submissions.
zesu22
left a comment
zesu22
left a comment
There was a problem hiding this comment.
please check the comments
…s resolved. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
json-form-builder/src/JsonFormBuilder.ts (2)
429-464: Comprehensive validation for checkbox and radio fields.The validation logic now properly handles:
- Checkbox required validation (lines 429-436)
- Radio group validation with proper
setCustomValidityusage (lines 437-464)- Form data population for selected radio values (line 450)
One minor observation: line 463 sets
state.formData[field.id]to an empty string as a fallback. Consider whetherundefinedor the actual stored value would be more appropriate here.If you want to avoid overwriting existing form data, consider:
} else { - // Ensure formData is always up-to-date for radios - state.formData[field.id] = state.formData[field.id] || ""; + // Form data already set; no need to overwrite }
337-337: Static analysis innerHTML warnings are acceptable here.The static analysis tool flags innerHTML usage, but these are controlled scenarios:
- Line 337:
getLabelTextreturns sanitized label text from the form schema- Line 801: Empty string assignment to clear button content (safe)
- Line 821:
state.submitLabelis a controlled string from configuration (safe)For added security, you could replace line 821 with
textContent:- formButton.innerHTML = state.submitLabel; + formButton.textContent = state.submitLabel;However, if
submitLabelneeds to support HTML markup (e.g., icons), the current approach is acceptable with proper input validation at the configuration level.Also applies to: 801-801, 821-821
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
json-form-builder/README.md(3 hunks)json-form-builder/src/JsonFormBuilder.ts(18 hunks)json-form-builder/src/components/PasswordComponent.ts(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
json-form-builder/src/components/PasswordComponent.ts (4)
json-form-builder/src/utils/icons.ts (2)
eyeIconSvg(27-31)eyeCrossedIconSvg(33-36)json-form-builder/src/utils/constants.ts (1)
ControlType(26-26)json-form-builder/src/utils/utils.ts (5)
emptyInvalidFn(717-717)getMultiLangText(703-703)appendError(704-704)handleRequiredValidation(705-705)handleRegexValidation(706-706)json-form-builder/src/types.ts (1)
Label(5-5)
json-form-builder/src/JsonFormBuilder.ts (8)
json-form-builder/src/types.ts (3)
FormState(130-157)Label(5-5)FormData(114-116)json-form-builder/src/utils/utils.ts (3)
validateForm(716-716)getMultiLangText(703-703)getLabelText(702-702)json-form-builder/src/utils/recaptcha.ts (1)
validateRecaptcha(204-204)json-form-builder/src/utils/constants.ts (1)
ControlType(26-26)json-form-builder/src/components/TextareaComponent.ts (1)
createTextareaField(22-107)json-form-builder/src/components/index.ts (3)
createTextareaField(22-22)createRadioField(23-23)createFileUploadField(24-24)json-form-builder/src/components/RadioComponent.ts (1)
createRadioField(18-148)json-form-builder/src/components/FileUploadComponent.ts (1)
createFileUploadField(36-402)
🪛 ast-grep (0.40.0)
json-form-builder/src/components/PasswordComponent.ts
[warning] 34-34: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: eyeIconSpan.innerHTML = eyeCrossedIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 37-37: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: eyeIconSpan.innerHTML = eyeIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 31-31: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 34-34: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = eyeCrossedIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 37-37: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: eyeIconSpan.innerHTML = eyeIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 801-801: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 821-821: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 821-821: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: formButton.innerHTML = state.submitLabel
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (14)
json-form-builder/src/components/PasswordComponent.ts (4)
29-39: Static analysis XSS warnings are false positives.The static analysis tool flags
innerHTMLusage as an XSS vulnerability. However,eyeIconSvgandeyeCrossedIconSvgare static SVG string constants imported from../utils/icons.ts(lines 26-35), not user-supplied content. UsinginnerHTMLis appropriate here for injecting pre-defined SVG markup.
74-92: Improved DOM structure with wrapper-based approach.The new
password-input-wrapperstructure groups the input and eye icon into a single container, improving maintainability and consistency with other components in this PR (e.g., DateComponent, FileUploadComponent).
107-111: Enhanced validation logic with value presence check.The added
value &&check before regex validation ensures that validation only runs when there's actual content, preventing unnecessary validation on empty optional fields. This is a good improvement.
209-217: Improved password mismatch validation with multilingual support.The refactored mismatch validation consolidates the logic and properly uses multilingual error messages from
state.fallbackErrors?.passwordMismatch. This is cleaner than the previous implementation and aligns with the PR's enhanced i18n support.json-form-builder/README.md (2)
26-206: Well-documented examples reflecting PR enhancements.The updated documentation accurately reflects the new features introduced in this PR:
- New control types (textarea, radio, file upload)
- Enhanced multilingual support with Arabic (ara) examples
- Password mismatch validation
- Improved field configuration examples
The examples are clear, comprehensive, and align with the code changes in JsonFormBuilder.ts and component files.
397-407: Verify npm link package name matches published scope.The npm link commands reference
@mosip/json-form-builder. Ensure this matches the actual published package name in yourpackage.json.json-form-builder/src/JsonFormBuilder.ts (8)
157-172: New submit button state management function.The
updateSubmitButtonStatefunction centralizes the logic for enabling/disabling the submit button based on form validity and reCAPTCHA state. This is a good separation of concerns and improves maintainability.
246-261: Proper placeholder handling for textarea and date fields.The updated logic correctly:
- Includes
textareain the selector alongsideinput- Skips placeholder updates for date fields (which have browser-native date pickers)
- Updates placeholders for other field types based on current language
332-371: Enhanced radio field label translation.The new radio field refresh logic:
- Updates the main radio group label
- Translates individual radio option labels using
allowedValues- Preserves original values using
dataset.originalValue- Uses
textContentinstead ofinnerHTMLfor labels (addresses static analysis concern from past reviews)This is a solid implementation that maintains data integrity while supporting i18n.
487-487: Form initialization gating for error display.The
state.isFormInitializedcheck ensures error messages are only displayed after the form has been fully rendered and initialized. This prevents premature error display on page load, improving the user experience.
828-848: Live validation listeners improve user feedback.The
attachLiveValidationListenersfunction wires up real-time validation by:
- Listening to
inputevents for immediate feedback- Using
changeevents as a fallback for elements like<select>- Triggering
refreshLabelsto update error statesThis provides a responsive user experience with instant validation feedback.
859-860: Proper initialization sequencing.The initialization flow correctly:
- Attaches live validation listeners after rendering
- Sets
isFormInitialized = trueafter all setup is completeThis ensures error display gating (line 487) works as intended.
891-904: Extended form element creation for new field types.The switch statement now handles:
TEXTAREA(line 891-892)RADIO(line 901-902)FILE(line 903-904)These align with the new field creator imports (lines 21-23) and expand the form builder's capabilities as documented in the README.
938-972: Enhanced form data handling for file and photo uploads.The refactored
getFormDatafunction:
- Distinguishes between photo (single object) and document (array) file types based on MIME type (lines 943-960)
- Provides sensible defaults for missing file data
- Includes reCAPTCHA token when present (lines 968-970)
The logic is clear and handles the different file upload scenarios appropriately.
…d issuses fixed. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
json-form-builder/src/JsonFormBuilder.ts (4)
246-261: Live validation +isFormInitializedgating look good; consider future perf refinement
- The
inputOrTextareaselector plus theControlType.DATEguard correctly extends placeholder updates to textareas and avoids interfering with date inputs.- The camera‑denied description lookup using
${errorDescription.dataset.errorCode}_descriptionkeeps PHOTO error text language‑aware in a consistent way.- Gating error rendering on
state.isFormInitializedin theerrorContainerblock prevents error messages from flashing on first render, while therender().render()flow setsisFormInitialized = trueonly after the form and listeners are in place. This sequencing is sound.triggerAllEventsnow considersselectandtextareaand has more robust strategies for locating.error-messagecontainers, which should help keep language/validation in sync for new field types.With
attachLiveValidationListenerscallingrefreshLabels(state)on everyinputandchange, validation becomes “live”, butrefreshLabelswalks the entire schema and runs required/regex checks for every field. This is fine for modest forms, but could become noticeable on very large schemas.If you see perf issues later, an incremental improvement would be to:
- extract a per‑field validation helper that only recalculates
lastErrors[field.id]and its error container; and- have the event listeners call that helper (plus
updateSubmitButtonState) instead of re‑runningrefreshLabelsfor all fields.Current behavior is acceptable; this is mainly a future optimization point.
Also applies to: 408-411, 486-507, 525-548, 827-847, 858-860
332-371: Radio label translation and required handling – double‑checkformDatashapeThe radio refresh block looks much healthier than the previous version:
- Using
radio.dataset.originalValueto preserve a stable key while translating labels viaallowedValues[field.id]avoids the earlier bug whereradio.valuewas replaced with translated text.- Updating the per‑option label with
labelElement.textContent = translatedValueis XSS‑safer thaninnerHTMLand aligns with past review feedback.- The dedicated selector for
.radio-container[data-field-id="..."] > .radio-group-labelensures the group label is updated on language change.In the required‑validation branch you also now do:
if (radio.checked) { isChecked = true; state.formData[field.id] = radio.value; }
createRadioFieldalready populatesstate.formData[field.id]onchange(potentially with translated values or a SIMPLE_TYPE‑specific structure). Overwriting it here risks diverging from the shape thatRadioComponentintends to maintain, especially for multilingual or non‑SIMPLE_TYPE radios.I’d recommend either:
- removing the
state.formData[field.id] = radio.valueassignment here and relying oncreateRadioField’s change handler to ownformData; or- factoring out a shared helper (e.g.
setRadioFormData(state, field, radio)inRadioComponent) and calling that helper from both the component and this validation block so the stored structure is guaranteed to match.Please re‑run or add tests for radio fields (both SIMPLE_TYPE and non‑SIMPLE_TYPE) to confirm
formDataremains in the expected shape after these changes.Also applies to: 438-463, 466-467
681-682: File/fileupload data shaping ingetFormData– confirm size units and empty photo semanticsThe new
getFormDatalogic andFormStateadditions look directionally good:
- FILE controls now always yield a predictable shape:
- image/fileupload fields (based on
acceptedFileTypes?.some(t => t.startsWith("image/"))) return a single object for photos, falling back to{ value: "", docType: field.id, format: "" }if nothing is stored.- document uploads always return an array filtered to object entries.
- Non‑FILE fields are passed through as‑is, with
undefined/nullnormalized to"".- The explicit append of
recaptchaTokenensures it’s not lost despite not being inschema.Two things worth double‑checking:
maxFileSizeMBunits (Line 682)
state.maxFileSizeMBis initialized asconfig.maxFileSizeMB || 5242880, but the property name suggests megabytes while5242880is 5 MB in bytes. IfFileUploadComponentor validators later interpret this value as “MB” and multiply by1024 ** 2, the effective default limit jumps to ~5 TB, which is probably not intended.Consider either:
- treating this as bytes and renaming to
maxFileSizeBytes(and updating docs), or- changing the default to
5and performing MB→bytes conversion where you enforce the limit.Empty photo representation
For image FILE fields, when nothing is selected you now always send an object with an emptyvalue. If the backend expects “no upload” to beundefined/omitted/"", this may require an adjustment (e.g. only emitting the object once a file has been chosen).Please confirm how
maxFileSizeMBis consumed inFileUploadComponent.tsand what the backend currently expects for “no file selected” cases to avoid surprises.Also applies to: 937-965, 966-970
769-769: Avoid hard‑coded submit button id to keep multiple forms safe
submitButton.id = "form-submit-button"makes it easy for external code to locate the button, but it also means:
- multiple
JsonFormBuilderinstances on the same page will generate duplicateidattributes (invalid HTML and brittle DOM queries); and- any consumer logic using
document.getElementById("form-submit-button")will arbitrarily hit the first form only.To keep this flexible and safe for multi‑form pages, consider deriving the id from the container or a caller‑provided suffix, e.g.:
- submitButton.id = "form-submit-button"; + if (state.container.id) { + submitButton.id = `${state.container.id}-submit-button`; + }or documenting that consumers should target the button via a CSS selector within the form container rather than a global id.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/src/JsonFormBuilder.ts(18 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
json-form-builder/src/JsonFormBuilder.ts (7)
json-form-builder/src/types.ts (4)
FormState(130-157)Label(5-5)FormData(114-116)FormField(7-37)json-form-builder/src/utils/utils.ts (3)
validateForm(716-716)getMultiLangText(703-703)getLabelText(702-702)json-form-builder/src/utils/constants.ts (1)
ControlType(26-26)json-form-builder/src/components/TextareaComponent.ts (1)
createTextareaField(22-107)json-form-builder/src/components/index.ts (3)
createTextareaField(22-22)createRadioField(23-23)createFileUploadField(24-24)json-form-builder/src/components/RadioComponent.ts (1)
createRadioField(18-148)json-form-builder/src/components/FileUploadComponent.ts (1)
createFileUploadField(36-402)
🪛 ast-grep (0.40.0)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 800-800: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (1)
json-form-builder/src/JsonFormBuilder.ts (1)
21-23: New TEXTAREA/RADIO/FILE control types are correctly wired into the builderImporting
createTextareaField,createRadioField, andcreateFileUploadFieldand branching onControlType.TEXTAREA,ControlType.RADIO, andControlType.FILEincreateFormElementcleanly extends the existing switch without altering prior behavior. No issues from a wiring/typing perspective.Also applies to: 885-903
…d issuses fixed. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
json-form-builder/src/JsonFormBuilder.ts (1)
791-824:isSubmittingnever resets for customsubmitAction, andinnerHTMLusage is flaggedTwo concerns in
validateAndSubmit:
isSubmittingis never reset when a customsubmitActionis usedif (isValid && isFormValid) { const data = getFormData(state); if (typeof state.submitAction === "function") { state.submitAction(data); // <-- no reset after this } else { state.isSubmitting = false; ... } }When a function is provided:
state.isSubmittingstaystrueindefinitely.- The button stays disabled with the spinner.
- Subsequent submits short‑circuit on
if (state.isSubmitting) return;.Unless
submitActionis expected to completely tear down/rebuild the form or manually mutate internals, this effectively turns the form into a one‑shot submit, even on errors.
- Static analysis warning on
formButton.innerHTML = ""Although you’re not inserting user content here, clearing via
innerHTMLis what the scanner flags. You can avoid this by replacing children instead of touching HTML.A concrete adjustment that handles both synchronous and Promise‑based submit actions and removes
innerHTML:- const formButton: HTMLButtonElement = form.querySelector('button[type="submit"]')!; - if (!formButton) return; - - formButton.disabled = true; - formButton.innerHTML = ""; - formButton.appendChild(createLoadingIcon()); + const formButton = form.querySelector<HTMLButtonElement>('button[type="submit"]'); + if (!formButton) return; + + formButton.disabled = true; + formButton.replaceChildren(createLoadingIcon()); @@ - if (isValid && isFormValid) { - const data = getFormData(state); - if (typeof state.submitAction === "function") { - state.submitAction(data); - } else { - state.isSubmitting = false; - formButton.textContent = state.submitLabel; - updateSubmitButtonState(state); - } - } else { - state.isSubmitting = false; - formButton.textContent = state.submitLabel; - form.reportValidity(); - updateSubmitButtonState(state); - } + if (isValid && isFormValid) { + const data = getFormData(state); + if (typeof state.submitAction === "function") { + const result = state.submitAction(data); + // If submitAction returns a Promise, reset on completion + if (result && typeof (result as Promise<unknown>).finally === "function") { + (result as Promise<unknown>).finally(() => { + state.isSubmitting = false; + formButton.textContent = state.submitLabel; + updateSubmitButtonState(state); + }); + } else { + // Synchronous submitAction + state.isSubmitting = false; + formButton.textContent = state.submitLabel; + updateSubmitButtonState(state); + } + } else { + state.isSubmitting = false; + formButton.textContent = state.submitLabel; + updateSubmitButtonState(state); + } + } else { + state.isSubmitting = false; + formButton.textContent = state.submitLabel; + form.reportValidity(); + updateSubmitButtonState(state); + }This ensures:
isSubmittingis always reset, so the user can re‑submit if needed.- Asynchronous
submitActionimplementations can return a Promise to keep the button disabled until completion.innerHTMLis no longer used, satisfying the static analysis warning.Also applies to: 797-803
♻️ Duplicate comments (1)
json-form-builder/src/JsonFormBuilder.ts (1)
332-371: Radio option translation fix is good; consider avoidinginnerHTMLon main label
- Keeping
radio.valuestable and storing the untranslated key inradio.dataset.originalValuewhile translating only the associated<label>viatextContentis exactly what you want for language‑independent form data and avoids the earlier XSS/static‑analysis complaint.- The remaining
mainLabel.innerHTML = getLabelText(state, field)still relies on raw HTML, which is what the static analysis warning is about. If labels are meant to be plain text, you can tighten this totextContentand append any icons separately.Example:
- if (mainLabel) { - mainLabel.innerHTML = getLabelText(state, field); - } + if (mainLabel) { + mainLabel.textContent = getLabelText(state, field); + }(or, if you need to preserve the info icon like other controls, mirror that pattern with explicit DOM nodes instead of raw HTML).
🧹 Nitpick comments (4)
json-form-builder/src/JsonFormBuilder.ts (4)
429-437: Required validation for CHECKBOX/RADIO and textarea is mostly solid; watch multi-checkbox use‑cases
- CHECKBOX: using
input#${field.id}[type="checkbox"]to enforcerequiredworks well for single boolean checkboxes.- RADIO: checking the group by
name="${field.id}", settingstate.formData[field.id] = radio.valuewhen selected, and usingsetCustomValidityon the first radio gives good UX and integrates with nativereportValidity.- Generic branch now also considers
textarea[data-field-id="${field.id}"], so textareas participate in both required and regex validation, which is correct.One thing to double‑check: if you ever support “multi‑option checkbox groups” under
ControlType.CHECKBOX(multiple checkboxes sharing a field), this logic only inspects a single#idcheckbox and would mark the field as required‑invalid even if some other option is selected. If such groups exist, you may want a parallel branch similar to the RADIO one that inspects all checkboxes bynameor a data attribute.Also applies to: 438-463, 466-467
486-486: Error gating viaisFormInitializedis good; live validation may be heavy on large forms
- Gating error rendering with
state.isFormInitializedprevents validation messages from flashing on initial render, which is a nice UX improvement, especially combined with the finalstate.isFormInitialized = trueafterrender(...)and attaching listeners.attachLiveValidationListenershooksinputandchangeon every"input, select, textarea"and callsrefreshLabels(state)on each event. BecauserefreshLabelsiterates over the full schema and now ends by callingupdateSubmitButtonState→validateForm, this effectively runs whole‑form validation on every keystroke.Functionally this works, but for very large forms it could become expensive. If you run into performance issues, consider:
- Debouncing the
inputhandler for text fields, and/or- Narrowing validation to the specific field whose event fired, updating only its entry in
state.lastErrors, and then usingvalidateForm(state)just for aggregate checks.Also applies to: 827-847, 858-860
681-682:maxFileSizeMBnaming vs units is potentially confusingYou default
maxFileSizeMBto5242880with a comment “Default to 5MB given as bytes”. The value is in bytes, but the property name suggests megabytes:maxFileSizeMB: config.maxFileSizeMB || 5242880; // bytesTo avoid confusion (especially for SDK consumers reading the type names/docs), consider either:
- Renaming the field to
maxFileSizeBytes, or- Expecting
config.maxFileSizeMBin megabytes and internally converting to bytes where needed.
937-964: FILE/PHOTO normalization ingetFormDatalooks good; be explicit about “image = single object” ruleThe new
getFormDatabehavior:
- For
ControlType.FILEwithacceptedFileTypescontaining any"image/"type, returns a singleFileUploadData-like object (or a default empty object) for that field.- For other FILE fields, always returns an array of objects (or
[]), filtering out non-object entries.- For non-FILE controls, preserves whatever was in
state.formData[field.id](or normalizes to empty string).- Adds
recaptchaTokenseparately when present.This provides a clear, predictable shape for submit payloads, which is great for consumers. Just ensure the “any image MIME ⇒ treat as single photo object” convention is documented and matches how
FileUploadComponentand consumers interpret these fields—especially if someone ever configures a mixed list like["image/png", "application/pdf"].Otherwise, the normalization logic looks sound.
Also applies to: 967-970
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/src/JsonFormBuilder.ts(18 hunks)
🧰 Additional context used
🪛 ast-grep (0.40.0)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 800-800: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (5)
json-form-builder/src/JsonFormBuilder.ts (5)
21-23: Wiring of new TEXTAREA/RADIO/FILE controls is consistentThe new imports and
createFormElementswitch cases forControlType.TEXTAREA,ControlType.RADIO, andControlType.FILEare consistent with existing patterns (TEXTBOX/PASSWORD/DATE/etc.) and keep the mapping centralized. No issues spotted here.Also applies to: 890-903
157-172: Submit button state now correctly respectsisSubmittingUsing
updateSubmitButtonStatewith:
shouldBeDisabled = state.isSubmitting || !isFormValid || !isRecaptchaValid, and- calling it from
refreshLabels, initialrender, and after label refreshkeeps the button disabled during in‑flight submissions and reCAPTCHA invalid states, addressing the earlier desync concern between UI and
state.isSubmitting. This looks correct and cohesive.Also applies to: 509-514, 783-785
246-260: Date placeholder exemption and textarea support look correctExtending the selector to
input#${field.id}, textarea#${field.id}and skipping placeholder updates forControlType.DATEavoids clobbering date-specific placeholders while still enabling multilingual placeholders for textareas. This is a clean, low‑risk enhancement.
525-548: Expanded error-container resolution intriggerAllEventslooks safeThe new search order for
.error-message(next sibling → parent’s next sibling → closest.form-field→ closest.form-field-group→ parent query) is more robust for varied DOM structures, and the guards aroundnextElementSibling/parentElementavoid null dereferences. No issues seen with this heuristic.
769-769: Adding a stable submit-button id is usefulAssigning
submitButton.id = "form-submit-button";provides a stable hook for external code, tests, or styling to target the submit button. This is a straightforward, low‑risk improvement.
…d component. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
json-form-builder/src/JsonFormBuilder.ts (2)
157-172: Submit button state helper looks correct; consider validateForm cost if forms grow large
updateSubmitButtonStatenow correctly keeps the submit button disabled whilestate.isSubmittingis true and also respects form + reCAPTCHA validity, which fixes the earlier risk of re‑enabling during an in‑flight submit.If
validateFormis non‑trivial, you might later want to cache or scope validation (per field) instead of recomputing full‑form validity on every call, since this function is now invoked quite frequently viarefreshLabelsand live listeners.
797-803: UsinginnerHTML = ""is safe for XSS here, but the alternative removes static analysis noiseSetting
innerHTMLto an empty string is a safe XSS pattern when not concatenating user input—it simply clears content without parsing any string data. Since the code appends only a locally-constructed SVG, there is no XSS surface.If you want to eliminate static analysis warnings, you could replace it with:
- formButton.innerHTML = ""; + while (formButton.firstChild) { + formButton.removeChild(formButton.firstChild); + }Both approaches are functionally equivalent; the choice is based on tooling preferences rather than security requirements.
Also applies to: 816-824
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/src/JsonFormBuilder.ts(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
json-form-builder/src/JsonFormBuilder.ts (3)
json-form-builder/src/types.ts (2)
FormState(130-157)Label(5-5)json-form-builder/src/components/TextareaComponent.ts (1)
createTextareaField(22-107)json-form-builder/src/components/RadioComponent.ts (1)
createRadioField(18-148)
🪛 ast-grep (0.40.0)
json-form-builder/src/JsonFormBuilder.ts
[warning] 337-337: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 337-337: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 800-800: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (3)
json-form-builder/src/JsonFormBuilder.ts (3)
429-437: Checkbox/textarea integration and error‑gating look good
- Required validation for CHECKBOX now explicitly checks the DOM checkbox state instead of just
formData, which is safer and clearer.- Broadening the generic input selector to
input[data-field-id=...], textarea[data-field-id=...]correctly brings TEXTAREA into existing required/regex validation.- Gating error rendering with
state.isFormInitializedprevents errors from flashing on initial render or before the form is ready, which should improve UX.- Calling
updateSubmitButtonState(state)at the end ofrefreshLabelsties submit‑button enable/disable to current validation state consistently.No changes needed here from my side.
Also applies to: 466-471, 485-487, 509-514
21-23: New TEXTAREA/RADIO wiring is consistent; FILE import is ready for later
- Wiring
ControlType.TEXTAREAtocreateTextareaFieldandControlType.RADIOtocreateRadioFieldmatches the component implementations and integrates them cleanly into the genericcreateFormElementswitch.- Keeping the FILE case commented while still importing
createFileUploadFieldis fine as a WIP, assuming your linter isn’t enforcing no‑unused‑imports. Once FILE support is completed, uncommenting the case should be straightforward.No issues from my side here.
Also applies to: 885-905
521-562: Live validation wiring is effective but may be heavy on large forms
attachLiveValidationListenerswiring allinput, select, textareaelements to callrefreshLabels(state)on everyinputandchangeevent gives immediate validation feedback and keeps the submit button state up to date, which is functionally solid.That said, if
refreshLabelswalks the entire schema and performs multiple DOM queries per field while also callingupdateSubmitButtonState(which re‑runsvalidateForm), this could introduce noticeable jank on very large forms as the user types.If you see performance issues in practice, consider a follow‑up refactor where:
- The per‑field listeners only revalidate the affected field and update its
lastErrors.updateSubmitButtonStateuses thoselastErrorsinstead of recalculating full‑form validity, or is called less aggressively.For small/medium forms this is acceptable as‑is.
…nt and other UI fixes. Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
json-form-builder/src/JsonFormBuilder.ts (1)
519-546: Radio validation updates formData, duplicating RadioComponent logic.Lines 528-533 iterate radios and assign
state.formData[field.id] = radio.valuefor the checked one. This duplicates the data-population logic inRadioComponent.ts(lines 104-137), which already updatesformDataon change.Per past review comments (lines 251-266), this duplication was acknowledged. The logic is consistent between both locations, but having two places that manage the same data increases maintenance risk.
The prior review noted this concern. If you want to consolidate, rely on RadioComponent's change handler and have validation only check (not set) formData.
🧹 Nitpick comments (2)
json-form-builder/src/components/FileUploadComponent.ts (1)
164-165: Potential XSS: User-controlled placeholder text in innerHTML.
state.placeholders?.proofOfDoccomes from config butgetMultiLangTextcould return user-controlled content. Even though it's wrapped in a<span>, usinginnerHTMLhere is risky.Use
textContentinstead:const text = document.createElement("div"); - text.innerHTML = `<span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>`; + const textSpan = document.createElement("span"); + textSpan.className = "upload-text"; + textSpan.style.color = "#1B75D0"; + textSpan.style.fontWeight = "600"; + textSpan.textContent = getMultiLangText(state, state.placeholders?.proofOfDoc) || "Click to Upload"; + text.appendChild(textSpan);json-form-builder/src/utils/utils.ts (1)
477-523: Radio validation updates formData, potentially conflicting with RadioComponent.The radio case (lines 477-523) reads the checked radio's value and builds either a SIMPLE_TYPE array or a single value, then writes it to
state.formData[fieldId]. However,RadioComponent.tsalready updatesformDatain its change handler (lines 81-137 in RadioComponent.ts).This means:
- When a user changes a radio selection, RadioComponent updates formData
- During validation (e.g., in refreshLabels), this code also updates formData
While the logic is consistent between both places, having two code paths that manage the same data increases maintenance burden. Consider whether validation should only check formData (not set it) and rely on component handlers to populate it.
If you want validation to only verify (not populate) formData:
case "radio": { if (!input.checked) break; const fieldId = input.name || input.id; if (!fieldId) break; const fieldDef = state.schema.find(f => f.id === fieldId); - if (!fieldDef) break; - - const originalKey = input.value; - - const allOptions = state.allowedValues?.[fieldId]; - - // Narrow the union type - if (!allOptions || typeof allOptions !== "object") { - state.formData[fieldId] = originalKey; - break; - } - - const optionLabels = (allOptions as Record<string, Label>)[originalKey]; - - if (fieldDef.type === "simpleType") { - // SIMPLE TYPE = multilingual array - state.formData[fieldId] = state.mandatoryLanguages.map((lng) => { - const mappedLng = state.languageMap[lng] || lng; - return { - language: mappedLng.length === 3 ? mappedLng : lng, - value: optionLabels?.[lng] || optionLabels?.[mappedLng] || "" - }; - }); - } else { - const mandatoryLangs: string[] = state?.mandatoryLanguages || []; - const firstMandatory = mandatoryLangs[0]; - - // fallback: use langMap if needed - const mappedMandatory = (state.languageMap && state.languageMap[firstMandatory]) - ? state.languageMap[firstMandatory] - : firstMandatory; - - // assign final value - state.formData[fieldId] = - optionLabels?.[firstMandatory] || - optionLabels?.[mappedMandatory] || - originalKey; - } + // formData is already populated by RadioComponent's change handler + // Just verify it exists for validation purposes break; }This keeps the single source of truth in RadioComponent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
json-form-builder/src/JsonFormBuilder.ts(20 hunks)json-form-builder/src/components/DropdownComponent.ts(4 hunks)json-form-builder/src/components/FileUploadComponent.ts(1 hunks)json-form-builder/src/components/RadioComponent.ts(1 hunks)json-form-builder/src/types.ts(5 hunks)json-form-builder/src/utils/responsive-style.ts(17 hunks)json-form-builder/src/utils/utils.ts(16 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
json-form-builder/src/utils/utils.ts (2)
json-form-builder/src/utils/icons.ts (2)
errorIconSvg(22-25)infoIconSvg(38-42)json-form-builder/src/types.ts (4)
Label(5-5)FormField(7-37)FormState(135-162)SubTypeField(39-42)
json-form-builder/src/components/DropdownComponent.ts (1)
json-form-builder/src/types.ts (2)
FormState(135-162)SubTypeField(39-42)
🪛 ast-grep (0.40.0)
json-form-builder/src/components/RadioComponent.ts
[warning] 28-28: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 28-28: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/JsonFormBuilder.ts
[warning] 343-343: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 343-343: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 431-433: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 439-441: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 454-457: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 477-479: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 483-486: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 431-433: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 439-441: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 454-457: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 477-479: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 483-486: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 882-882: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/utils/utils.ts
[warning] 99-99: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 99-99: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 235-235: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 243-243: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 235-235: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 243-243: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 51-51: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 108-113: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 160-160: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 164-164: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 197-197: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 306-306: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 318-318: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 338-338: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 342-342: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 51-51: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 108-113: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 160-160: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 164-164: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 197-197: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 318-318: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 338-338: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (17)
json-form-builder/src/components/DropdownComponent.ts (1)
1-105: LGTM! SubTypeField integration and sub-component support implemented correctly.The changes properly:
- Integrate SubTypeField typing for subtype-aware dropdowns
- Add
isSubComponentflag to control formData updates (preventing subcomponents from directly updating parent formData)- Source options from
field.subTypewhen available, with appropriate fallback tofield.idThe design allows dropdowns to function both as standalone fields and as nested components within other controls (like FileUpload).
json-form-builder/src/components/FileUploadComponent.ts (2)
51-52: Safe innerHTML usage with controlled helper function.Static analysis flags this, but
getLabelTextreturns sanitized/controlled content (field labels from schema). The*for required fields is the only markup, which is intentional and safe.
306-309: Safe innerHTML usage for cleanup.Setting
innerHTML = ""to clear content is safe and intentional.json-form-builder/src/types.ts (2)
7-42: LGTM! Type definitions properly extended for new controls.The changes correctly:
- Expand
FormField.controlTypeto include new controls (radio, textarea, fileupload)- Add relevant optional properties (minAge, maxAge, rows, maxFileSizeMB)
- Introduce
SubTypeFieldinterface for controls requiring subtype-aware behavior- Properly constrain
SubTypeField.controlTypeto only the controls that use subtypesThe type system properly supports the expanded functionality.
95-95: Good: Renamed property for clarity.
maxFileSizeMBis clearer thanmaxUploadFileSizeabout the unit of measurement. The comment confirms it's in megabytes.json-form-builder/src/components/RadioComponent.ts (1)
18-150: LGTM! Radio component properly implements multilingual and subtype-aware behavior.The implementation correctly:
- Sources options from
field.subTypewhen available, falling back tofield.id- Maps UI language codes to allowedValues language keys using
state.languageMap- Handles both SIMPLE_TYPE (multilingual array) and regular (single value) data models
- Validates required fields and updates error state appropriately
- Preserves data structure consistency with the validation logic in JsonFormBuilder.ts
The language-aware value assembly logic (lines 108-136) properly normalizes language codes and builds the expected data structures.
json-form-builder/src/utils/responsive-style.ts (1)
1-871: LGTM! Comprehensive styling for new form controls with RTL support.The CSS additions properly support:
- New form controls (radio, textarea, file upload)
- Date field enhancements with calendar icon
- Improved error message display
- Disabled submit button styling
- Password field wrapper improvements
- File preview and upload area styling
- RTL adaptations for all new controls
Previous issues with duplicate
.file-preview-leftand invalid::disabledpseudo-element have been addressed.json-form-builder/src/utils/utils.ts (4)
99-114: Safe innerHTML usage with controlled SVG constant.Static analysis flags this, but
errorIconSvgis a controlled constant from./icons.ts, not user input. The use ofinnerHTMLto inject SVG is intentional and safe.
235-289: LGTM! Info icon interaction improved with proper event handling.The refactored approach correctly:
- Attaches event listeners to the icon element (not globally)
- Uses
hideAllInfo()helper to close other open popups before showing new one- Handles hover, focus, and keyboard (Escape) interactions
- Previous issue with accumulating global listeners has been resolved
406-418: LGTM! Bidirectional map now properly handles existing mappings.The refactored logic correctly:
- Initializes an empty map
- Adds forward mappings unconditionally
- Only adds backward mappings when not already present (prevents overwriting)
This prevents collisions when multiple keys map to the same value.
661-706: LGTM! MIME helper functions are well-structured.The new helpers correctly:
- Map common MIME types to readable extensions via specialMap
- Provide generic fallback for standard MIME patterns
- Generate accept strings for file inputs
- Convert MIME types to user-friendly labels
The implementation handles edge cases appropriately.
json-form-builder/src/JsonFormBuilder.ts (6)
162-177: LGTM! Submit button state properly gates on isSubmitting.The
updateSubmitButtonStatefunction correctly disables the button when:
state.isSubmittingis true (prevents re-submission during in-flight requests)- Form validation fails
- reCAPTCHA validation fails
This addresses the previous concern about the button being re-enabled during submission.
338-379: LGTM! Radio label refresh preserves data integrity.The implementation correctly:
- Updates only the visible label text using
textContent(not innerHTML, avoiding XSS)- Stores the original key in
radio.dataset.originalValueonce- Preserves
radio.valueas the original key (not translated text)- Translates option labels for display using language mapping
This ensures form data remains language-independent. Based on past review comments, this approach was confirmed as correct.
422-494: FILE control label refresh implementation looks comprehensive.The logic correctly updates:
- Main upload placeholder text
- File type info text
- All subfields (docType, docRef, proofOfDoc) labels and placeholders
- Dropdown options for docType using subType-aware key resolution
The language-aware translation handling is consistent with other controls.
909-929: LGTM! Live validation listeners improve UX.The
attachLiveValidationListenersfunction correctly:
- Attaches to all interactive form elements (input, select, textarea)
- Calls
refreshLabelson bothinputandchangeevents- Provides immediate feedback as users interact with the form
Combined with
isFormInitializedgating (line 568), this prevents showing errors before the user has interacted with the form.
972-987: FILE control creation commented out (WIP).Lines 985-986 show the FILE case is intentionally disabled with a "tentatively disabled (WIP)" comment. This is appropriate for incomplete work but should be tracked.
Please confirm:
- Is there a tracking issue for completing FILE control integration?
- Should the FILE-related code in
refreshLabels(lines 422-494) also be commented out or guarded until the control is ready?
431-433: Safe innerHTML usage for translated text in FILE control refresh.Static analysis flags these lines, but the content comes from:
getMultiLangText()returning translated strings from configgetLabelText()returning field labels with optional*for required fieldsThese are controlled by the schema/config, not direct user input. The innerHTML usage is intentional to support the required indicator markup.
Also applies to: 439-441, 454-457
Signed-off-by: Gk <76690271+gk-XL7@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
json-form-builder/README.md(3 hunks)
🔇 Additional comments (1)
json-form-builder/README.md (1)
215-215: Document new control types (textarea, radio, file-upload) mentioned in PR summary.The PR summary indicates that textarea, radio, and file-upload controls have been added, but these are not listed in the controlType options table (line 215) or in the Field Types section (lines 308–319).
If these controls are now supported, update the documentation to include them in both the controlType options and the Field Types section.
Also applies to: 308-319
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
…into develop-patch3
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
json-form-builder/README.md (2)
215-215: Update controlType options to include new field types.The
controlTypeoptions list is missing the new field types introduced in this PR:textarea,radio, andfileupload.🔎 Proposed fix
-| `controlType` | string | **Mandatory** | UI control type for rendering. Options: `textbox`, `date`, `dropdown`, `password`, `checkbox`, `phone`, `photo`. | +| `controlType` | string | **Mandatory** | UI control type for rendering. Options: `textbox`, `textarea`, `date`, `dropdown`, `password`, `checkbox`, `radio`, `phone`, `photo`, `fileupload`. |
308-319: Update Field Types section to include new types.The Field Types list should include the new field types: Textarea, Radio, and File Upload.
🔎 Proposed fix
## Field Types The form builder supports the following field types: - Textbox (single language) - Textbox (multiple languages) +- Textarea - Password - Date - Dropdown - Checkbox +- Radio - Phone - Photo +- File Upload
♻️ Duplicate comments (4)
json-form-builder/README.md (1)
325-341: Property naming inconsistency:labelshould belabelName.The validation example uses
label(line 329), but the Schema Properties table at line 221 documents the property aslabelName. This could confuse developers.🔎 Proposed fix
{ id: 'email', controlType: 'textbox', - label: { eng: 'Email', fra: 'E-mail' }, + labelName: { eng: 'Email', fra: 'E-mail' }, required: true, validators: [json-form-builder/src/components/FileUploadComponent.ts (2)
160-161: Fix incorrect default file size limit.The default value
5242880is in bytes (~5MB), but the variable namemaxSizeMBimplies megabytes. Line 161 then multiplies by1024 * 1024, resulting in a ~5TB limit instead of 5MB.🔎 Proposed fix
- const maxSizeMB = field.maxFileSizeMB || 5242880; + const maxSizeMB = field.maxFileSizeMB || 5; const maxBytes = maxSizeMB * 1024 * 1024;
316-319: Memory leak: Object URL created but never revoked.
URL.createObjectURL(file)creates a blob URL that persists until explicitly revoked. For repeated uploads, this accumulates memory. Consider revoking the previous URL before creating a new one.🔎 Proposed fix
if (isPhotoUpload) { + // Revoke previous object URL if exists + if (previewImg.src && previewImg.src.startsWith("blob:")) { + URL.revokeObjectURL(previewImg.src); + } previewImg.src = URL.createObjectURL(file); previewImg.style.display = "block"; photoDeleteBtn.style.display = "block";json-form-builder/src/JsonFormBuilder.ts (1)
528-534: RADIO validation still overwritesformDatashape set by RadioComponent.Line 532 assigns
state.formData[field.id] = radio.valueduring validation, which overwrites the multilingual array structure set byRadioComponentforsimpleTypefields. This validation block should only set validity/errors, not mutate formData.🔎 Proposed fix - remove formData mutation
// Clear any previous custom validity and check selection radioElements.forEach((radio) => { radio.setCustomValidity(""); // reset if (radio.checked) { isChecked = true; - state.formData[field.id] = radio.value; // <-- populate formData } });
🧹 Nitpick comments (4)
json-form-builder/src/components/FileUploadComponent.ts (2)
134-155: Duplicate DOM append:docTypeFieldElappended twice.
docTypeFieldElis first appended togroupBoxat line 85, then wrapped and appended again at lines 135-140. This will move the element (not duplicate it), but the first append at line 85 is redundant and confusing.🔎 Proposed fix - remove the first append
docTypeFieldEl = createDropdownField(state, docTypeField, true); docTypeFieldEl.dataset.i18nLabel = "docType"; docTypeFieldEl.dataset.i18nPlaceholder = "docType"; - groupBox.appendChild(docTypeFieldEl); }
181-182: Consider using textContent for translated text to prevent XSS.While
getMultiLangTextreturns translation strings that are typically controlled, usinginnerHTMLwith translated content could be an XSS vector if translations are ever compromised or contain user-generated content. Static analysis flagged this as a potential vulnerability.🔎 Proposed fix
const text = document.createElement("div"); - text.innerHTML = `<span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>`; + const uploadSpan = document.createElement("span"); + uploadSpan.className = "upload-text"; + uploadSpan.style.cssText = "color:#1B75D0; font-weight:600;"; + uploadSpan.textContent = getMultiLangText(state, state.placeholders?.proofOfDoc); + text.appendChild(uploadSpan);json-form-builder/src/utils/utils.ts (1)
640-658: Control flow issue in fileupload validation.When
formField.requiredis false, the function returnstrueat line 657, bypassing the normalreturn hasFormDataat line 660. This works but the pattern is inconsistent with other cases. More concerning: ifrequiredis true but checks pass, there's nobreakto exit the switch, though this likely falls through correctly due to the structure.🔎 Proposed refactor for consistency
case "fileupload": if (formField.required) { if (!value || typeof value !== "object") { hasFormData = false; + break; } const fileData = value as FileUploadData; if (fileData && (!fileData.value || fileData.value === "")) { hasFormData = false; + break; } if (fileData && (!fileData.docType || fileData.docType === "")) { hasFormData = false; } } - else return true; break;json-form-builder/src/JsonFormBuilder.ts (1)
422-494: Consider usingtextContentfor file upload label translations.Static analysis flagged multiple
innerHTMLassignments in the FILE control type refresh block (lines 432, 440, 455, 478, 484). While these source from configuration translations, usingtextContentwhere possible would be safer.🔎 Example fix for uploadText
// Main upload placeholder const uploadText = fileEl.querySelector(".upload-text"); if (uploadText) { - uploadText.innerHTML = + uploadText.textContent = getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) || "Click to upload"; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
json-form-builder/README.mdjson-form-builder/src/JsonFormBuilder.tsjson-form-builder/src/components/FileUploadComponent.tsjson-form-builder/src/utils/utils.ts
🧰 Additional context used
🪛 ast-grep (0.40.3)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 51-51: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 125-130: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 177-177: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 181-181: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 214-214: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 338-338: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 358-358: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 51-51: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 125-130: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 177-177: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 181-181: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 214-214: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 326-326: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 338-338: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 358-358: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 369-369: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/JsonFormBuilder.ts
[warning] 343-343: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 343-343: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 431-433: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 439-441: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 454-457: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 477-479: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 483-486: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 431-433: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 439-441: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 454-457: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 477-479: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 483-486: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 882-882: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/utils/utils.ts
[warning] 100-100: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 100-100: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = errorIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 236-236: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 244-244: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 236-236: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoSpan.innerHTML = infoIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 244-244: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoDetailArrow.innerHTML = <svg class="fill-[#FFFFFF] stroke-[#BCBCBC]" width="10" height="5" viewBox="0 0 30 10" preserveAspectRatio="none" style="display: block;"><polygon points="0,0 30,0 15,10"></polygon></svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (7)
json-form-builder/src/components/FileUploadComponent.ts (1)
325-380: LGTM: File preview uses safe DOM construction.The
renderFilePreviewfunction correctly usestextContentfor user-controlledfileNameand only usesinnerHTMLfor controlled SVG constants, addressing the previous XSS concerns.json-form-builder/src/utils/utils.ts (3)
265-290: LGTM: Info icon keyboard accessibility and scoped event handlers.The info icon now has proper keyboard accessibility (
tabIndex, focus/blur handlers, Escape key support) and the event listeners are correctly scoped to theinfoSpanelement rather than the global document, addressing the previous memory leak concern.
685-725: LGTM: MIME utility functions are well-implemented.The new
mimeToExtension,getAcceptString, andmimeToLabelutilities provide clean abstractions for file type handling with proper fallbacks.
727-730: LGTM: Type guard for SubTypeField.The
isSubTypeFieldtype guard correctly identifies fields that support subtypes (dropdown, radio, file upload).json-form-builder/src/JsonFormBuilder.ts (3)
162-177: LGTM: Submit button state now correctly considersisSubmitting.The
updateSubmitButtonStatefunction now includesstate.isSubmittingin the disabled calculation, preventing the button from re-enabling while a submission is in progress. This addresses the previous review concern.
338-378: LGTM: Radio label translation now uses safetextContent.The radio label translation correctly preserves
radio.valueand usestextContentinstead ofinnerHTMLfor the translated label text, addressing both the data integrity and XSS concerns from previous reviews.
909-929: LGTM: Live validation listeners properly attached.The
attachLiveValidationListenersfunction correctly wires up input and change events to trigger validation and UI updates in real-time.
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
json-form-builder/src/JsonFormBuilder.ts (1)
519-545: Remove formData assignment in radio validation.Line 532 overwrites
state.formData[field.id]withradio.value, butRadioComponentalready maintains the correct shape (an array forSIMPLE_TYPE, or a localized label/key for others). This assignment breaks the data model and conflicts with the component's change handler.Remove the assignment and let this code handle only validation (error state, custom validity):
🔎 Proposed fix
radioElements.forEach((radio) => { radio.setCustomValidity(""); // reset if (radio.checked) { isChecked = true; - state.formData[field.id] = radio.value; // <-- populate formData } });
🧹 Nitpick comments (1)
json-form-builder/src/components/FileUploadComponent.ts (1)
134-155: DOM restructuring pattern is confusing.Elements are appended to
groupBoxinitially (lines 85, 113, 132), then moved again by wrapping them in new containers (lines 134-155). WhileappendChildcorrectly moves existing elements, this two-step pattern reduces readability.Consider creating the wrappers first and appending wrapped elements directly:
🔎 Suggested refactor
if (docTypeFieldEl) { + const docTypeWrapper = document.createElement("div"); + docTypeWrapper.className = "file-subfield"; + docTypeWrapper.dataset.subId = "docType"; + docTypeWrapper.appendChild(docTypeFieldEl); + groupBox.appendChild(docTypeWrapper); - docTypeFieldEl = createDropdownField(state, docTypeField, true); - docTypeFieldEl.dataset.i18nLabel = "docType"; - docTypeFieldEl.dataset.i18nPlaceholder = "docType"; - groupBox.appendChild(docTypeFieldEl); } // ... similar pattern for refEl and podLabelThis makes the intent clearer by showing the final structure upfront.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
json-form-builder/src/JsonFormBuilder.tsjson-form-builder/src/components/FileUploadComponent.ts
🧰 Additional context used
🪛 ast-grep (0.40.3)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 51-51: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 125-130: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 177-177: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 181-181: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 214-214: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 326-326: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 338-338: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 358-358: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 369-369: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 51-51: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 125-130: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 177-177: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 181-181: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 214-214: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 338-338: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 358-358: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
json-form-builder/src/JsonFormBuilder.ts
[warning] 343-343: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 343-343: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 431-433: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 439-441: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 454-457: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 477-479: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 483-486: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 431-433: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 439-441: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 454-457: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 477-479: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 483-486: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 882-882: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🔇 Additional comments (2)
json-form-builder/src/JsonFormBuilder.ts (2)
162-177: LGTM!The submit button state logic correctly considers
state.isSubmitting, preventing re-enablement during in-flight requests. This addresses the previous concern.
338-379: LGTM!Radio label refresh correctly preserves the original key in
dataset.originalValueand updates only the displayed label text usingtextContent. This addresses previous XSS and data integrity concerns.
zesu22
left a comment
zesu22
left a comment
There was a problem hiding this comment.
please check review comment
Signed-off-by: GurukiranP <talk2gurukiran@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @json-form-builder/src/components/FileUploadComponent.ts:
- Around line 178-179: The code creates a div named text and assigns unsafe HTML
using text.innerHTML with getMultiLangText(state,
state.placeholders?.proofOfDoc), which allows XSS if placeholders are
user-controlled; replace the innerHTML usage with a safe text insertion (e.g.,
set text.textContent or append a document.createTextNode with the output of
getMultiLangText) and apply the desired styling via className or style
properties on the element instead of embedding the value into HTML.
- Around line 51-58: The label assignment is vulnerable because getLabelText
returns concatenated unsanitized user data and is set via label.innerHTML;
replace this by assigning safe text or escaping output: change the
FileUploadComponent code to use label.textContent = getLabelText(state, field)
(or update getMultiLangText/getLabelText to HTML-escape user strings before
returning) and only use innerHTML when the string has been explicitly sanitized;
also ensure createInfoIcon still appends safely (keep
label.appendChild(infoIcon)) so no raw HTML from field.labelName is injected.
In @json-form-builder/src/JsonFormBuilder.ts:
- Around line 976-984: Replace the inline type assertions with the existing type
guard to ensure runtime-safe narrowing: in the switch handling
ControlType.DROPDOWN, ControlType.RADIO and ControlType.FILE, call
isSubTypeField(field) and only then invoke createDropdownField(state, field),
createRadioField(state, field) or createFileUploadField(state, field)
respectively; if the guard fails, fall through or break the case rather than
casting, keeping the rest of the switch behavior unchanged.
🧹 Nitpick comments (4)
json-form-builder/src/components/FileUploadComponent.ts (2)
19-27: Consider defensive handling forreader.result.The non-null assertion
reader.result!assumesonloadalways provides a result. While typically safe,resultcan theoretically benull. A defensive check would be more robust.♻️ Suggested improvement
reader.onload = () => - resolve(reader.result!.toString().split(",")[1]); + resolve((reader.result as string)?.split(",")[1] ?? "");
121-129: Consider using CSS class instead of inline style.Line 123 uses inline style
marginBottom: "4px". For consistency and maintainability, consider using a CSS class.♻️ Suggested improvement
const podLabel = document.createElement("label"); podLabel.dataset.i18nLabel = "proofOfDoc"; - podLabel.style.marginBottom = "4px"; + podLabel.className = "proof-of-doc-label"; // Define in CSS: .proof-of-doc-label { margin-bottom: 4px; }json-form-builder/src/JsonFormBuilder.ts (2)
430-443: Consider usingtextContentfor plain text placeholders.The upload text and info text are plain strings without HTML markup. Using
textContentinstead ofinnerHTMLwould be safer.♻️ Suggested improvement
const uploadText = fileEl.querySelector(".upload-text"); if (uploadText) { - uploadText.innerHTML = + uploadText.textContent = getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) || "Click to upload"; } // Info text const infoText = fileEl.querySelector(".file-info-text"); if (infoText) { - infoText.innerHTML = + infoText.textContent = getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) || - infoText.innerHTML; + infoText.textContent || ""; }
877-878: Non-null assertion before null check is misleading.Line 877 uses
!non-null assertion, but line 878 checks ifformButtonis null. While this works at runtime, the assertion is unnecessary and misleading.♻️ Suggested improvement
- const formButton: HTMLButtonElement = form.querySelector('button[type="submit"]')!; + const formButton = form.querySelector('button[type="submit"]') as HTMLButtonElement | null; if (!formButton) return;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
json-form-builder/src/JsonFormBuilder.tsjson-form-builder/src/components/FileUploadComponent.ts
🧰 Additional context used
🧬 Code graph analysis (1)
json-form-builder/src/JsonFormBuilder.ts (8)
json-form-builder/src/types.ts (4)
Label(5-5)FormState(135-162)FormField(7-37)SubTypeField(39-42)json-form-builder/src/utils/utils.ts (4)
validateForm(747-747)getMultiLangText(734-734)isSubTypeField(751-751)getLabelText(733-733)json-form-builder/src/utils/recaptcha.ts (1)
validateRecaptcha(204-204)json-form-builder/src/components/TextareaComponent.ts (1)
createTextareaField(22-107)json-form-builder/src/components/DropdownComponent.ts (1)
createDropdownField(18-105)json-form-builder/src/components/CheckboxComponent.ts (1)
createCheckboxField(17-74)json-form-builder/src/components/RadioComponent.ts (1)
createRadioField(18-150)json-form-builder/src/components/FileUploadComponent.ts (1)
createFileUploadField(39-389)
🪛 ast-grep (0.40.3)
json-form-builder/src/JsonFormBuilder.ts
[warning] 343-343: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 343-343: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: mainLabel.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 431-433: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 439-441: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 454-457: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 477-479: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 483-486: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 431-433: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: uploadText.innerHTML =
getMultiLangText(state, state.placeholders?.proofOfDoc, false, lang, defaultLang) ||
"Click to upload"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 439-441: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: infoText.innerHTML =
getMultiLangText(state, state.placeholders?.fileTypesInfo, false, lang, defaultLang) ||
infoText.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 454-457: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: labelEl.innerHTML = getLabelText(state, {
required: subId !== "docRef",
labelName: state.labels?.[subId]
} as FormField)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 477-479: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
getMultiLangText(state, state.placeholders?.[subFields[0]], false, langCode, state.defaultLanguage) ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 483-486: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: option.innerHTML =
translations[langCode] ||
translations[state.defaultLanguage] ||
option.innerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 880-880: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: formButton.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
json-form-builder/src/components/FileUploadComponent.ts
[warning] 51-51: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 123-128: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 174-174: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 178-178: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 211-211: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 323-323: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 335-335: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 355-355: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 366-366: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: previewWrapper.innerHTML = ""
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 51-51: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: label.innerHTML = getLabelText(state, field)
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 123-128: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: podLabel.innerHTML = getLabelText(state, {
...field,
required: true,
labelName: state.labels?.proofOfDoc
|| { en: "Proof Of Document" }
})
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 174-174: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: iconWrapper.innerHTML = uploadIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 178-178: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: text.innerHTML = <span class="upload-text" style="color:#1B75D0; font-weight:600;">${getMultiLangText(state, state.placeholders?.proofOfDoc)}</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 211-211: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: photoDeleteBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 335-335: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: icon.innerHTML = fileIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 355-355: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: delBtn.innerHTML = trashIconSvg
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (16)
json-form-builder/src/components/FileUploadComponent.ts (7)
1-17: LGTM - Clean imports structure.The imports are well-organized, bringing in necessary types, utility functions, icons, and component creation functions. The separation of concerns is appropriate.
29-37: LGTM - Helper function is well-implemented.Clean implementation with proper deduplication using
Setand filtering of empty values.
67-95: LGTM - Conditional dropdown creation is well-structured.The document type dropdown is correctly created only when
subTypeexists with correspondingallowedValues. The change listener properly syncs the selected value tostate.formData.
226-243: LGTM - Delete handler properly cleans up state.The photo delete button correctly clears
formData, resets the input, and dispatches a change event to trigger re-validation.
247-273: LGTM - Event handlers are complete and well-structured.The click, drag-and-drop, and input change handlers properly coordinate file processing with appropriate disabled-state checks.
322-377: LGTM - File preview rendering is secure and well-structured.The implementation correctly uses
textContentfor the user-controlledfileName, preventing XSS. SVG icons are from controlled constants, making theirinnerHTMLassignment safe.
379-388: LGTM - Helper functions are clean and straightforward.Simple visibility toggle functions that serve their purpose well.
json-form-builder/src/JsonFormBuilder.ts (9)
1-46: LGTM - Imports are well-organized for new functionality.The new imports for
SubTypeField, component creators, andisSubTypeFieldtype guard support the expanded control type handling.
118-160: LGTM - Clean extension with backward compatibility.The optional
subTypeparameter allows dropdowns to look up options from an alternate key inallowedValueswhile maintaining backward compatibility.
162-177: LGTM - Submit button state management correctly includesisSubmitting.The function properly considers
state.isSubmittingin the disabled calculation, preventing the button from re-enabling during an in-flight submission. This addresses the past review concern.
251-266: LGTM - Extended input handling for textareas with appropriate date field handling.The query now includes textareas, and date fields correctly skip placeholder updates since browsers handle date input formatting natively.
338-379: LGTM - Radio label refresh correctly preserves original value.The implementation correctly stores
originalValuein dataset and only updates the visible label text usingtextContent, preserving form data integrity across language switches. This addresses the past review concern about overwritingradio.value.
566-586: LGTM - Error display correctly gated byisFormInitialized.This prevents validation error messages from appearing on initial form render, improving user experience by only showing errors after user interaction.
761-762: LGTM - Correct initialization of new state properties.The
maxFileSizeMBdefault is correctly set to5(megabytes) instead of the previous incorrect value.isFormInitializedis properly initialized tofalse.
907-927: LGTM - Live validation provides immediate feedback.The listeners for both
inputandchangeevents ensure validation feedback across all editable element types. This is called afterrender()to wire up the newly created elements.
929-940: LGTM - Render lifecycle properly sequences initialization.The public
rendermethod correctly sequences: build form → attach listeners → mark initialized. This ensures validation only shows errors after user interaction.
3 participants
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.