refine embedding page

[iceljc]

PR Type

Enhancement

---

Description

• Extract embedding logic into reusable component

• Reduce code duplication across embedding pages

• Add support for custom HTML styling and token authentication

• Refactor reporting page to use new EmbeddingPage component

---

Diagram Walkthrough

flowchart LR
  A["Embedding Logic<br/>in Page Component"] -->|"Extract & Generalize"| B["EmbeddingPage<br/>Reusable Component"]
  C["Reporting Page<br/>with Duplicated Code"] -->|"Refactor"| D["Reporting Page<br/>using EmbeddingPage"]
  B -->|"Used by"| D
  E["EmbeddingInfoModel<br/>Type Definition"] -->|"Extend with<br/>htmlStyle & appendTokenName"| F["Enhanced Type<br/>Support"]

Loading

File Walkthrough

Relevant files

Enhancement

EmbeddingPage.svelte New reusable embedding page component                                        src/lib/common/embedding/EmbeddingPage.svelte New reusable component for embedding external content Handles script injection and dynamic HTML element creation Supports token authentication via URL parameters Manages menu subscription and page slug tracking +101/-0  +page.svelte Refactor to use EmbeddingPage component                                    src/routes/page/agent/reporting/[reportType]/+page.svelte Remove 78 lines of duplicated embedding logic Replace with single EmbeddingPage component import Simplify page structure using component abstraction Add addOn parameter to HeadTitle component +7/-85    pluginTypes.js Extend EmbeddingInfoModel type definition                                src/lib/helpers/types/pluginTypes.js Add htmlStyle property for custom element styling Add appendTokenName property for token authentication Fix JSDoc formatting alignment +3/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Script injection Description: The component dynamically injects and executes a remote script via document.createElement("script") using data.scriptSrc, which can enable XSS/supply-chain compromise if embeddingInfo.scriptSrc is not strictly trusted/allowlisted (e.g., a malicious plugin/menu entry could load attacker-controlled JavaScript). EmbeddingPage.svelte [57-67] Referred Code if (data.scriptSrc) { const script = document.createElement("script"); script.id = data.source; script.src = data.scriptSrc; script.async = true; if (data.scriptType) { script.type = data.scriptType; } document.head.appendChild(script); }
	Token leakage in URL Description: The code appends user?.token to the embedded url as a query parameter (appendTokenName), which risks token leakage via browser history, logs, referrers, shared URLs, and third-party iframe destinations; additionally, the embedded URL is set directly on a created element (elem.src = url) without origin validation, increasing exposure if data.url is attacker-influenced. EmbeddingPage.svelte [77-86] Referred Code let url = data.url; if (data.appendTokenName) { const user = getUserStore(); url += `${url.includes('?') ? '&' : '?'}${data.appendTokenName}=${user?.token}`; } // @ts-ignore elem.src = url; div.appendChild(elem); }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: No failure handling: The embedding logic performs DOM/script injection and tokenized URL construction but only returns early on missing data without handling load failures or reporting actionable errors. Referred Code function embed(data) { if (!data) return; if (data.scriptSrc) { const script = document.createElement("script"); script.id = data.source; script.src = data.scriptSrc; script.async = true; if (data.scriptType) { script.type = data.scriptType; } document.head.appendChild(script); } const div = document.querySelector(`#${htmlTagId}`); if (!data.url || !div) return; if (data.htmlTag) { const elem = document.createElement(data.htmlTag); elem.id = data.source; ... (clipped 12 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Token in URL: The code appends user?.token into the embedded url query string and injects external scriptSrc/htmlTag without validation/allowlisting, increasing risk of token leakage and injection. Referred Code if (data.scriptSrc) { const script = document.createElement("script"); script.id = data.source; script.src = data.scriptSrc; script.async = true; if (data.scriptType) { script.type = data.scriptType; } document.head.appendChild(script); } const div = document.querySelector(`#${htmlTagId}`); if (!data.url || !div) return; if (data.htmlTag) { const elem = document.createElement(data.htmlTag); elem.id = data.source; elem.style = data.htmlStyle || 'width: 100%; height: 100%; justify-content: center;'; let url = data.url; ... (clipped 8 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logging: The new embedding flow injects external scripts/URLs and can append an auth token to requests without any audit logging of these security-relevant actions. Referred Code onMount(async () => { menuUnsubscribe = globalMenuStore.subscribe((/** @type {import('$pluginTypes').PluginMenuDefModel[]} */ menu) => { const url = getPathUrl(); let found = menu.find(x => x.link === url); label = found?.label || null; if (!found?.embeddingInfo) { const subFound = menu.find(x => !!x.subMenu?.find(y => y.link === url)); found = subFound?.subMenu?.find(x => x.link === url); label = found?.label || null; } embed(found?.embeddingInfo || null); }); }); onDestroy(() => { menuUnsubscribe?.(); contentSubscribe?.(); }); /** @param {import('$pluginTypes').EmbeddingInfoModel?} data */ ... (clipped 33 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Avoid full page reloads for navigation The EmbeddingPage component should use Svelte's reactivity to update content when the URL changes, instead of forcing a full page reload with location.reload(). This will improve performance and user experience. Examples: src/lib/common/embedding/EmbeddingPage.svelte [26-31] const contentSubscribe = slug.subscribe(value => { if (curSlug && curSlug !== value) { location.reload(); } curSlug = value; }); Solution Walkthrough: Before: <script> // ... let curSlug = ''; const slug = derived(page, $page => $page.params[slugName]); const contentSubscribe = slug.subscribe(value => { if (curSlug && curSlug !== value) { location.reload(); // Forces a full page reload on navigation } curSlug = value; }); onMount(async () => { // ... logic to find and embed content on initial load }); </script> After: <script> import { browser } from '$app/environment'; // ... const slug = derived(page, $page => $page.params[slugName]); // Reactive statement to find embedding info when slug or menu changes $: embeddingInfo = findEmbeddingInfoForUrl($slug, $globalMenuStore, $page.url.pathname); // Reactive statement to re-embed content when it changes $: if (browser && embeddingInfo) { const container = document.querySelector(`#${htmlTagId}`); if (container) { cleanupAndEmbed(embeddingInfo, container); } } function cleanupAndEmbed(data, container) { // 1. Remove old script and elements from the DOM // 2. Embed new content based on `data` } </script> Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical design flaw (location.reload()) in the new EmbeddingPage component that negates the benefits of a single-page application, causing poor performance and user experience.	High
Possible issue	Correct inline style application Use elem.style.cssText instead of elem.style to correctly assign an inline style string to an element, as direct assignment does not work. src/lib/common/embedding/EmbeddingPage.svelte [75] -elem.style = data.htmlStyle || 'width: 100%; height: 100%; justify-content: center;'; +elem.style.cssText = data.htmlStyle || 'width: 100%; height: 100%; justify-content: center;'; Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion fixes a definite bug where styles would not be applied because elem.style was being assigned a string directly, which is incorrect. Using elem.style.cssText makes the styling functional as intended.	High
	Avoid appending an undefined token Add a check to ensure user?.token exists before appending it to the URL to avoid creating malformed URLs with an "undefined" value. src/lib/common/embedding/EmbeddingPage.svelte [78-81] if (data.appendTokenName) { const user = getUserStore(); - url += `${url.includes('?') ? '&' : '?'}${data.appendTokenName}=${user?.token}`; + if (user?.token) { + url += `${url.includes('?') ? '&' : '?'}${data.appendTokenName}=${user.token}`; + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: This suggestion correctly identifies that appending user?.token can result in a URL with ...=undefined, which is poor practice and could cause issues. Adding a check improves the robustness of the URL generation.	Low
General	Prevent duplicate script injection Prevent duplicate script injection by checking if a script with the same id already exists in the document before appending it. src/lib/common/embedding/EmbeddingPage.svelte [66] -document.head.appendChild(script); +if (!document.getElementById(data.source)) { + document.head.appendChild(script); +} Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies that the embed function can be called multiple times, leading to duplicate script injection. Adding a check for an existing script ID prevents this bug and improves component robustness.	Medium
Security	Use getElementById for safer lookup Replace document.querySelector with document.getElementById for improved performance and to prevent potential selector injection vulnerabilities. src/lib/common/embedding/EmbeddingPage.svelte [69] -const div = document.querySelector(`#${htmlTagId}`); +const div = document.getElementById(htmlTagId); Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion to use getElementById is a good practice for performance and safety, although the risk of selector injection is low in this context as htmlTagId is a component prop with a default value.	Low
More