Refactor ministry + organization into organization unit with tree structure#173
Refactor ministry + organization into organization unit with tree structure#173robbertbos wants to merge 8 commits intomainfrom
Conversation
…ame of organization to organizationunit
| li.innerHTML = ` | ||
| <span class="org-filter-modal__selected-name">${name}</span> | ||
| <button type="button" | ||
| class="org-filter-modal__selected-remove" | ||
| onclick="removeOrgFromModalSelection(${id})" | ||
| aria-label="Verwijder ${name}"> | ||
| <c-icon icon="kruis" color="grijs-600" size="sm"></c-icon> | ||
| </button> | ||
| `; |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
In general, the fix is to avoid interpreting potentially tainted strings as HTML. Instead of using innerHTML with template literals that directly interpolate name and type/id, construct the DOM using safe APIs: set textContent for textual content and set attributes via setAttribute or direct property assignment. If you truly need to insert HTML fragments, sanitize the input first with a robust HTML sanitizer, but in this case everything we build can be pure DOM.
Concretely for this file:
- Keep using
innerHTMLonly for constant, hardcoded HTML that contains no user data (like the “Geen opdrachtgevers geselecteerd” message). - For the selected items, replace:
li.innerHTML = `
<span class="org-filter-modal__selected-name">${name}</span>
<button ... onclick="removeOrgTypeFromModalSelection('${type}')" aria-label="Verwijder ${name}">
<c-icon ...></c-icon>
</button>
`;and the similar block for orgs, with explicit DOM construction:
- Create
spanelements, set theirclassNameandtextContenttoname. - Create
buttonelements, settype,className,aria-label, and attach aclickevent listener instead of using anonclickstring. - Create the
<c-icon>element and append it to the button. - Append the span and button to the
li.
This keeps all untrusted data (name, type, id) out of innerHTML and out of string-based event handlers, preventing HTML interpretation and XSS. The changes are all within wies/core/static/js/placement_filter_sidebar.js in the block that updates selectedList around lines 1077–1105, and they require no new imports or external libraries.
| li.innerHTML = ` | ||
| <span class="org-filter-modal__selected-name">${name}</span> | ||
| <button type="button" | ||
| class="org-filter-modal__selected-remove" | ||
| onclick="removeOrgFromModalSelection(${id})" | ||
| aria-label="Verwijder ${name}"> | ||
| <c-icon icon="kruis" color="grijs-600" size="sm"></c-icon> | ||
| </button> | ||
| `; |
Check failure
Code scanning / CodeQL
DOM text reinterpreted as HTML High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
In general, the fix is to avoid inserting untrusted text into the DOM via innerHTML. Instead, create elements and text nodes with document.createElement and textContent/setAttribute, so user-controlled strings are treated strictly as text, not parsed as HTML. For event handlers, use addEventListener instead of inline onclick attributes, so you don’t need to interpolate IDs into HTML strings.
Concretely for this file:
- Keep the overall structure (UL with LI items, buttons, icon component).
- Replace
li.innerHTML = \... ${name} ... ${type/id} ...`` blocks with:- Create the
<span>and settextContent = name. - Create the
<button>, set class, type,aria-labelviasetAttribute. - Attach the appropriate click handler with
addEventListener("click", ...)instead ofonclickHTML. - Create the
<c-icon>element (custom element) and append it to the button.
- Create the
- Ensure we do not rely on
innerHTMLfor the dynamic parts. We can leave the static “empty” message string (Geen opdrachtgevers geselecteerd) as a fixed string assigned toinnerHTMLsince it contains no user data.
All changes are confined to the snippet shown in wies/core/static/js/placement_filter_sidebar.js, around lines 1075–1107. No new external dependencies are needed; we only use standard DOM APIs that are already in use elsewhere in the file.
No description provided.