Security: Unrestricted External CDN Script Loading#363
Conversation
The root layout injects a script tag directly from `https://unpkg.com/external-svg-loader@1.6.8/svg-loader.min.js`. If the CDN is compromised or the package is tampered with, arbitrary malicious JavaScript will be executed in the context of the user's browser (Supply Chain Attack / Cross-Site Scripting). Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
WalkthroughThe root layout’s SVG loader script source changed from an external CDN URL to the local ChangesSVG loader configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@frontends/ui/src/app/layout.tsx`:
- Around line 53-55: Ensure the SVG loader referenced by the layout script is
available at the configured /svg-loader.min.js path by vendoring the asset under
the UI public directory, or replace the local source with the existing CDN
source. Preserve the SVG loader initialization in the layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 38641378-270f-4a72-832d-6698e09f6448
📒 Files selected for processing (1)
frontends/ui/src/app/layout.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/app/layout.tsx
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/app/layout.tsx
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/app/layout.tsx
| {/* SVG icon loader - inlines <svg data-src="..."> elements */} | ||
| <script | ||
| src="https://unpkg.com/external-svg-loader@1.6.8/svg-loader.min.js" | ||
| src="/svg-loader.min.js" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -H -t f 'svg-loader\.min\.js$' frontends/ui
cd frontends/ui
npm run lint
npm run type-check
npm run test:ci
npm run buildRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n-- layout.tsx excerpt --\n'
sed -n '1,120p' frontends/ui/src/app/layout.tsx
printf '\n-- candidate loader files --\n'
fd -a -H -t f 'svg-loader.*\.js$' frontends/ui || true
printf '\n-- public assets matching svg-loader --\n'
fd -a -H -t f 'svg-loader' frontends/ui/public frontends/ui/src frontends/ui || true
printf '\n-- references to svg-loader.min.js --\n'
rg -n --hidden --glob '!node_modules' 'svg-loader\.min\.js|svg-loader' frontends/ui
printf '\n-- next config and public root clues --\n'
git ls-files frontends/ui | rg '^(frontends/ui/(public/|next\.config|src/app/layout\.tsx|package\.json))'Repository: NVIDIA-AI-Blueprints/aiq
Length of output: 3378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' frontends/ui/src/app/layout.tsx
fd -a -H -t f 'svg-loader.*\.js$' frontends/ui || true
rg -n --hidden --glob '!node_modules' 'svg-loader\.min\.js|svg-loader' frontends/ui || true
git ls-files frontends/ui | rg '^(frontends/ui/(public/|next\.config|src/app/layout\.tsx|package\.json))' || trueRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 3126
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '-- files named svg-loader.min.js --'
fd -a -H -t f 'svg-loader\.min\.js$' .
printf '\n%s\n' '-- references to svg-loader.min.js --'
rg -n --hidden --glob '!node_modules' 'svg-loader\.min\.js|svg-loader' frontends/ui || true
printf '\n%s\n' '-- layout.tsx lines 45-65 --'
sed -n '45,65p' frontends/ui/src/app/layout.tsxRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 1662
Ship the SVG loader with the UI The app now points at /svg-loader.min.js, but there’s no vendored file under frontends/ui/public, so the script will 404 and svg[data-src] icons won’t inline. Either commit the asset there or keep the CDN source.
🤖 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 `@frontends/ui/src/app/layout.tsx` around lines 53 - 55, Ensure the SVG loader
referenced by the layout script is available at the configured
/svg-loader.min.js path by vendoring the asset under the UI public directory, or
replace the local source with the existing CDN source. Preserve the SVG loader
initialization in the layout.
Sources: Coding guidelines, Path instructions
Summary
Security: Unrestricted External CDN Script Loading
Problem
Severity:
Medium| File:frontends/ui/src/app/layout.tsx:L48The root layout injects a script tag directly from
https://unpkg.com/external-svg-loader@1.6.8/svg-loader.min.js. If the CDN is compromised or the package is tampered with, arbitrary malicious JavaScript will be executed in the context of the user's browser (Supply Chain Attack / Cross-Site Scripting).Solution
Use Subresource Integrity (SRI) hashes for the external script tag or vendor the script locally to eliminate the external dependency.
Changes
frontends/ui/src/app/layout.tsx(modified)Summary by CodeRabbit