Skip to content

fix: code quality and safety improvements #913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saurabhhhcodes wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: code quality and safety improvements #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/apiUsageDashboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ function escapeHtml(value: string): string {
}

export async function renderApiUsageDashboard(container: HTMLElement): Promise<void> {
container.innerHTML = '<p class="usage-loading">Loading API usage stats…</p>';
container.textContent = '<p class="usage-loading">Loading API usage stats…</p>';

try {
const stats = await getUsageStats();
container.innerHTML = buildDashboardHTML(stats);
container.textContent = buildDashboardHTML(stats);
attachEventListeners(container);
} catch (err) {
console.error("[LateMeet] Failed to load API usage dashboard:", err);
container.innerHTML = '<p class="usage-error">Failed to load API usage data.</p>';
container.textContent = '<p class="usage-error">Failed to load API usage data.</p>';
Comment on lines +18 to +26

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files:\n'
git ls-files | rg '(^|/)apiUsageDashboard\.ts$|(^|/)domHelpers\.ts$' || true

printf '\napiUsageDashboard outline:\n'
ast-grep outline src/apiUsageDashboard.ts || true

printf '\ndomHelpers outline:\n'
ast-grep outline src/utils/domHelpers.ts || true

printf '\nRelevant snippets:\n'
sed -n '1,120p' src/apiUsageDashboard.ts
printf '\n---\n'
sed -n '1,120p' src/utils/domHelpers.ts

printf '\nSearch for helper/dashboard functions:\n'
rg -n "buildDashboardHTML|getUsageStats|escapeHtml|attachEventListeners|usage-loading|usage-error" src || true

Repository: shouri123/Late-Meet

Length of output: 14426

🏁 Script executed:

# Read-only behavioral probe: show exactly what text node content each current assignment produces,
# without executing repository code or modifying files.
node - <<'JS'
const samples = [
  ['loading', '<p class="usage-loading">Loading API usage stats…</p>'],
  ['error', '<p class="usage-error">Failed to load API usage data.</p>'],
  ['example usage', '<div class="usage-dashboard"><div class="usage-card"><span>Tokens</span><strong>42</strong></div></div>'],
  ['example dynamic with angle brace', '<span>Alice <script>alert(1)</script> Smith</span>'],
];

for (const [name, html] of samples) {
  const doc = { body: document.createElement('div') };
  const container = doc.body;

  // Simulate current .textContent assignment.
  container.textContent = html;

  console.log(`${name}:`);
  console.log('textContent produces a single text node containing:', JSON.stringify(container.textContent));
  console.log('outerHTML:', container.outerHTML);
  console.log('firstChild tagName:', container.firstChild?.tagName ?? '<text>');
  console.log('has usage-card class:', container.classList && container.classList.contains('usage-card'));
  console.log('has script element:', container.querySelector?.('script') != null);
  console.log();
}
JS

Repository: shouri123/Late-Meet

Length of output: 793

🌐 Web query:

MDN Node textContent string markup rendered as text vs innerHTML

💡 Result:

The difference between using Node.textContent and Element.innerHTML primarily lies in how the browser handles markup, security, and performance [1][2]. Node.textContent - Rendering: When you set textContent, the browser treats the input string as raw text [1][2]. Any HTML markup included in the string (e.g., hello) will not be rendered as HTML; instead, it will appear literally as text on the page [2][3]. - Security: It is secure against cross-site scripting (XSS) attacks because it does not parse or execute the provided string as HTML [1][4]. - Performance: It is generally faster than innerHTML because the browser does not need to invoke the HTML parser [1][4]. Element.innerHTML - Rendering: When you set innerHTML, the browser parses the provided string as HTML markup [2]. Any valid HTML tags within the string will be rendered according to their standard behaviors [2][3]. - Security: It is a common vector for XSS attacks if the content being inserted comes from an untrusted source, as malicious scripts could be injected and executed [2][4]. - Performance: It is slower than textContent because it must invoke the HTML parser to construct the DOM tree from the provided string [1][4]. In summary, MDN advises using textContent whenever you intend to display plain text, as it is more semantic, performant, and secure [1][4]. You should only use innerHTML when you specifically need to insert and render HTML markup, and even then, you should ensure the content is sanitized or trusted to prevent security vulnerabilities [2][5].

Citations:

Do not assign HTML markup to textContent.

textContent treats the string as raw text, so the dashboard’s loading, error, and generated markup are rendered literally instead of as elements. Render the loading/error <p> elements explicitly, and insert buildDashboardHTML(stats) via innerHTML or DOM construction with escaped dynamic values. The src/utils/domHelpers.ts example also has this issue; create the <span> in the DOM or use innerHTML with escapeHtml().

📍 Affects 2 files
  • src/apiUsageDashboard.ts#L18-L26 (this comment)
  • src/utils/domHelpers.ts#L29-L29
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/apiUsageDashboard.ts` around lines 18 - 26, Update
src/apiUsageDashboard.ts lines 18-26 to render the loading and error paragraphs
as DOM elements and insert buildDashboardHTML(stats) through innerHTML or
equivalent DOM construction, ensuring dynamic values are escaped; update
src/utils/domHelpers.ts line 29 likewise by creating the span through DOM APIs
or using innerHTML with escapeHtml().

}
}

Expand Down
2 changes: 1 addition & 1 deletion src/utils/domHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const HTML_ESCAPE_MAP: Record<string, string> = {
* their corresponding named HTML entities.
*
* @example
* element.innerHTML = `<span>${escapeHtml(user.displayName)}</span>`;
* element.textContent = `<span>${escapeHtml(user.displayName)}</span>`;

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file outline =="
ast-grep outline src/utils/domHelpers.ts || true

echo "== relevant file contents =="
cat -n src/utils/domHelpers.ts | sed -n '1,160p'

echo "== references to domHelpers/escapeHtml =="
rg -n "escapeHtml|domHelpers|textContent|innerHTML" src -S || true

echo "== local script behavior probe for TextContent/InnerHTML semantics in browser would require runtime; instead use Node browserless unavailable by requirement. We'll inspect only source/config."

Repository: shouri123/Late-Meet

Length of output: 25216

🌐 Web query:

MDN textContent property HTML parsing does not parse tags

💡 Result:

The textContent property does not parse HTML tags. When you read the textContent of a node, it returns the concatenated text content of that node and all its descendants, explicitly excluding any HTML tags [1][2]. When you set the textContent of a node, the provided string is treated as raw text rather than HTML markup; it does not invoke the HTML parser [1][3][4]. Consequently, any HTML tags included in the string will be rendered as literal text on the page rather than being interpreted by the browser [4]. This behavior makes textContent a secure way to insert user-provided content into the DOM, as it prevents cross-site scripting (XSS) attacks that could otherwise occur if the input were parsed as HTML [1][3]. In contrast, the innerHTML property does invoke the HTML parser, meaning it treats the input string as markup and converts it into DOM elements [3][4].

Citations:

Correct the escapeHtml example.

textContent does not parse HTML. The example assigns literal <span>...</span> text instead of creating a span. Set the raw user.displayName as textContent, or assign escapeHtml(user.displayName) through innerHTML if the example is meant to demonstrate escaped HTML injection.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/domHelpers.ts` at line 29, Correct the usage example in the
escapeHtml documentation comment: use raw user.displayName with textContent, or
use escapeHtml(user.displayName) with innerHTML when demonstrating escaped HTML.
Do not combine HTML markup with textContent, since it remains literal text.

* // Input: 'Alice <script>alert(1)</script>'
* // Output: 'Alice &lt;script&gt;alert(1)&lt;/script&gt;'
*/
Expand Down