Skip to content

chore: improve LK UI #295

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

Jump to bottom
Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

chore: improve LK UI #295

wants to merge 20 commits into from

Conversation

@thomasyuill-livekit
Copy link
Contributor

@thomasyuill-livekit thomasyuill-livekit commented Oct 22, 2025 •
edited
Loading

@thomasyuill-livekit thomasyuill-livekit self-assigned this Oct 22, 2025
@vercel
Copy link

vercel bot commented Oct 22, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
sandbox-voice-assistant Ready Ready Preview Dec 2, 2025 9:20pm

@thomasyuill-livekit thomasyuill-livekit marked this pull request as draft October 22, 2025 21:27
@thomasyuill-livekit thomasyuill-livekit force-pushed the ty/visualizer branch 2 times, most recently from 1c3ffc3 to 7e5f614 Compare October 24, 2025 16:36
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 25, 2025
View reviewed changes
app/ui/components/page.tsx
.sort((a, b) => a.localeCompare(b))
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}

Check failure

Code scanning / CodeQL

Stored cross-site scripting High

Stored cross-site scripting vulnerability due to
stored value
Loading
.

Copilot Autofix

AI 8 days ago

The best fix is to ensure that only valid, expected component names are used, and that they are sanitized before inclusion in URLs and as visible text. Specifically:

  • Only allow component names that match safe slug/identifier patterns (e.g., only letters, numbers, hyphens, and underscores).
  • Filter out unsafe component names when listing them.
  • Sanitize each component name before creating the URL and rendering the link text.

A robust approach is to update getComponentNames() in lib/components.tsx so it only returns component names that match a safe pattern (e.g., /^[a-zA-Z0-9_-]+$/).
Make the change directly in lib/components.tsx.
No changes are needed in app/ui/components/page.tsx, as the source of truth will now be validated and only return safe values.

Suggested changeset 1
lib/components.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/lib/components.tsx b/lib/components.tsx
--- a/lib/components.tsx
+++ b/lib/components.tsx
@@ -6,5 +6,7 @@
   const componentNames = fs.readdirSync(componentsDir);
   return componentNames
     .filter((file) => file.endsWith('.tsx'))
-    .map((file) => file.replace('.tsx', ''));
+    .map((file) => file.replace('.tsx', ''))
+    // Only allow component names containing [A-Za-z0-9_-]
+    .filter((name) => /^[a-zA-Z0-9_-]+$/.test(name));
 }
EOF
@@ -6,5 +6,7 @@
const componentNames = fs.readdirSync(componentsDir);
return componentNames
.filter((file) => file.endsWith('.tsx'))
.map((file) => file.replace('.tsx', ''));
.map((file) => file.replace('.tsx', ''))
// Only allow component names containing [A-Za-z0-9_-]
.filter((name) => /^[a-zA-Z0-9_-]+$/.test(name));
}
Copilot is powered by AI and may make mistakes. Always verify output.
app/ui/components/page.tsx
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}
key={componentName}

Check failure

Code scanning / CodeQL

Stored cross-site scripting High

Stored cross-site scripting vulnerability due to
stored value
Loading
.

Copilot Autofix

AI 8 days ago

The best way to fix the problem is to ensure that all untrusted inputs rendered in the URL context are safely encoded. Specifically, when constructing the href for <Link>, componentName must be passed through encodeURIComponent() to prevent injection of slashes or special characters that could break the URL structure or enable attacks. Also, as a defense-in-depth, if rendering as HTML or if file names might contain suspicious Unicode, one could further escape text using a utility like escape-html, but in this case, React’s default escaping suffices for element children.

Files to change:

  • app/ui/components/page.tsx:
    Change {/ui/components/${componentName}} to {/ui/components/${encodeURIComponent(componentName)}} in the href prop.
    Import encodeURIComponent is not required since it is a global function.

No code changes necessary in lib/components.tsx because filtering occurs after file reading, but does not ensure file name “safety” -- the only thing needed is output encoding as above.

Suggested changeset 1
app/ui/components/page.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/ui/components/page.tsx b/app/ui/components/page.tsx
--- a/app/ui/components/page.tsx
+++ b/app/ui/components/page.tsx
@@ -18,7 +18,7 @@
           .sort((a, b) => a.localeCompare(b))
           .map((componentName) => (
             <Link
-              href={`/ui/components/${componentName}`}
+              href={`/ui/components/${encodeURIComponent(componentName)}`}
               key={componentName}
               className="font-semibold underline-offset-4 hover:underline focus:underline"
             >
EOF
@@ -18,7 +18,7 @@
.sort((a, b) => a.localeCompare(b))
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}
href={`/ui/components/${encodeURIComponent(componentName)}`}
key={componentName}
className="font-semibold underline-offset-4 hover:underline focus:underline"
>
Copilot is powered by AI and may make mistakes. Always verify output.
components/docs/side-nav.tsx
.sort((a, b) => a.localeCompare(b))
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}

Check failure

Code scanning / CodeQL

Stored cross-site scripting High documentation

Stored cross-site scripting vulnerability due to
stored value
Loading
.

Copilot Autofix

AI 8 days ago

The best fix is to sanitize each value from componentNames before using it in any URL (such as the href prop of a Link). The most robust approach, without changing the data source, is to encode or restrict componentName so it cannot be interpreted as anything but a safe path segment. In this context, the standard method is to apply encodeURIComponent() to componentName when constructing URLs. This both prevents breaking out of the path context and blocks attempts to inject scripts via the href attribute.

Apply this change only within components/docs/side-nav.tsx, ensuring minimal impact and no loss of existing functionality. You only need to wrap componentName with encodeURIComponent in the relevant href construction, and can do this inline without additional dependencies.

Suggested changeset 1
components/docs/side-nav.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/components/docs/side-nav.tsx b/components/docs/side-nav.tsx
--- a/components/docs/side-nav.tsx
+++ b/components/docs/side-nav.tsx
@@ -30,7 +30,7 @@
       .sort((a, b) => a.localeCompare(b))
       .map((componentName) => (
         <Link
-          href={`/ui/components/${componentName}`}
+          href={`/ui/components/${encodeURIComponent(componentName)}`}
           key={componentName}
           className={cn(
             'text-sm font-semibold underline-offset-4 hover:underline focus:underline',
EOF
@@ -30,7 +30,7 @@
.sort((a, b) => a.localeCompare(b))
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}
href={`/ui/components/${encodeURIComponent(componentName)}`}
key={componentName}
className={cn(
'text-sm font-semibold underline-offset-4 hover:underline focus:underline',
Copilot is powered by AI and may make mistakes. Always verify output.
components/docs/side-nav.tsx
.map((componentName) => (
<Link
href={`/ui/components/${componentName}`}
key={componentName}

Check failure

Code scanning / CodeQL

Stored cross-site scripting High documentation

Stored cross-site scripting vulnerability due to
stored value
Loading
.

Copilot Autofix

AI 8 days ago

To fix this issue, all component names (derived directly from file names via fs.readdirSync) should be sanitized to ensure that they do not contain dangerous or malformed characters before being used as React keys, display text, and URL path segments. The best approach is to explicitly restrict componentNames to a safe set of characters — e.g., alphanumeric characters, hyphens, and underscores only. This prevents injection attacks and also guards against weird edge cases (e.g., /, ., &, etc.).

The fix should be applied in the getComponentNames function in lib/components.tsx, where file names are first collected and transformed. We will add a filter that only allows component names that match a safe regular expression, e.g., /^[a-zA-Z0-9-_]+$/. This will ensure downstream consumers of the componentNames array will never see untrusted or dangerous values. Alternatively, for even stronger guarantees (if you need to retain more flexibility), encode the names as needed before using them, but filtering at the source is the safest.

No additional npm dependencies are needed, as JavaScript RegExp and array methods suffice.

Suggested changeset 1
lib/components.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/lib/components.tsx b/lib/components.tsx
--- a/lib/components.tsx
+++ b/lib/components.tsx
@@ -6,5 +6,6 @@
   const componentNames = fs.readdirSync(componentsDir);
   return componentNames
     .filter((file) => file.endsWith('.tsx'))
-    .map((file) => file.replace('.tsx', ''));
+    .map((file) => file.replace('.tsx', ''))
+    .filter((name) => /^[a-zA-Z0-9_-]+$/.test(name));
 }
EOF
@@ -6,5 +6,6 @@
const componentNames = fs.readdirSync(componentsDir);
return componentNames
.filter((file) => file.endsWith('.tsx'))
.map((file) => file.replace('.tsx', ''));
.map((file) => file.replace('.tsx', ''))
.filter((name) => /^[a-zA-Z0-9_-]+$/.test(name));
}
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukasIO lukasIO lukasIO left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@thomasyuill-livekit thomasyuill-livekit

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thomasyuill-livekit @lukasIO