mirror of
https://github.com/lukaszraczylo/traefik-llmgateway.git
synced 2026-10-17 05:37:43 +00:00
fix(webui): defuse scaffold config traps, tighten inline guards
Micro-fix from re-review: 1. webui/components.json: deleted the font/iconLibrary keys instead of picking a value. Re-review checked shadcn-vue's actual parse schema (node_modules/shadcn-vue/dist/schema/index.js's rawConfigSchema): both fields are z.string().optional(), not an enum - the five-choice enum lives only on the init CLI's --font/--icon-library flags, not on the components.json schema. The previous commit's "schema forbids a fix" rationale was wrong; corrected here. Verified empirically, not just by reading the schema: ran a real `npx shadcn-vue add badge -o -y` against components.json with both keys removed - src/assets/ main.css came back byte-for-byte unchanged (previously this exact class of call re-added a Google Fonts import and the .dark-class toggle). That same run did still re-add @lucide/vue to package.json's dependencies (some components' registry entries list an icon package regardless of iconLibrary) - reverted before committing, and webui/README.md's warning section rewritten to explain the omission (so nobody "helpfully" restores the keys), fix the schema claim, and note the narrower remaining package.json trap. 2. admin.go:27-28: adminCSP's doc comment now names all three set-sites (GET /admin, every GET /admin/assets/*, the three JSON routes) - previously only mentioned the page and JSON routes, dropped after the asset route gained the same header in the prior fix round. 3. Both inline-script guards (webui/generate.mjs's assertNoInlineAssets, admin_test.go's hasInlineScript) tightened from a bare \bsrc\s*= pattern to (^|\s)src\s*= - \b alone matches inside "data-src=" too (the "-" before "src" is a non-word character, so \b sits right before "src"), which would wrongly treat a data-src/nosrc attribute as the real src= that makes a <script> external. New Go test TestHasInlineScript_DataSrcIsNotExternal: <script data-src="x"> is still flagged as inline. No webui/src/ change this round - components.json isn't bundled, and generate.mjs's edit only touches its own build-time guard, not the Go-source-generation path. admin_assets_gen.go NOT regenerated (confirmed via a throwaway `node generate.mjs` + gofmt -w run: output is byte-identical to what's already committed).
This commit is contained in:
@@ -24,9 +24,13 @@ const (
|
|||||||
adminUsageHistoryPath = "/admin/api/usage/history"
|
adminUsageHistoryPath = "/admin/api/usage/history"
|
||||||
)
|
)
|
||||||
|
|
||||||
// adminCSP is the Content-Security-Policy header served with GET /admin
|
// adminCSP is the Content-Security-Policy header served with GET /admin,
|
||||||
// and the three /admin/api/* JSON routes (spec §4, v0.2; extended to the
|
// every GET /admin/assets/{hashedname} response (serveAdminAsset), and
|
||||||
// JSON routes by a folded review item, 2026-08-20 review). It was
|
// the three /admin/api/* JSON routes — every /admin* response this
|
||||||
|
// gateway ever sends (spec §4, v0.2; extended to the JSON routes by a
|
||||||
|
// folded review item, 2026-08-20 review; extended to the asset route by
|
||||||
|
// a later review sweep — see serveAdminAsset's own doc comment for why).
|
||||||
|
// It was
|
||||||
// widened with `script-src 'unsafe-inline'`/`style-src 'unsafe-inline'`
|
// widened with `script-src 'unsafe-inline'`/`style-src 'unsafe-inline'`
|
||||||
// for the original vanilla-JS single-file page; the baked Vue build (Vue
|
// for the original vanilla-JS single-file page; the baked Vue build (Vue
|
||||||
// admin-panel task) needs neither: `vite build` emits index.html with
|
// admin-panel task) needs neither: `vite build` emits index.html with
|
||||||
|
|||||||
+43
-7
@@ -696,10 +696,33 @@ func TestAdminAssets_GeneratedFilePresent(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// adminInlineScriptRe matches every <script ...> opening tag, capturing its
|
// adminInlineScriptRe matches every <script ...> opening tag, capturing its
|
||||||
// attribute text — TestAdminIndexHTML_NoInlineAssets checks each capture for
|
// attribute text — hasInlineScript checks each capture against
|
||||||
// a src= attribute.
|
// adminSrcAttrRe for a real src= attribute.
|
||||||
var adminInlineScriptRe = regexp.MustCompile(`(?i)<script\b([^>]*)>`)
|
var adminInlineScriptRe = regexp.MustCompile(`(?i)<script\b([^>]*)>`)
|
||||||
|
|
||||||
|
// adminSrcAttrRe matches a real src= attribute, anchored to the start of an
|
||||||
|
// attribute (start-of-string or preceding whitespace) rather than a bare
|
||||||
|
// \b word boundary: \b alone matches inside "data-src=" too (the "-" before
|
||||||
|
// "src" is itself a non-word character, so \b sits right before "src"),
|
||||||
|
// which would wrongly treat a data-src/nosrc/whatever-src attribute as the
|
||||||
|
// real src= that makes a <script> external. Mirrors
|
||||||
|
// webui/generate.mjs's assertNoInlineAssets, which guards the identical
|
||||||
|
// invariant at build time.
|
||||||
|
var adminSrcAttrRe = regexp.MustCompile(`(?i)(^|\s)src\s*=`)
|
||||||
|
|
||||||
|
// hasInlineScript returns every <script> opening tag in html with no real
|
||||||
|
// src= attribute — an inline script, forbidden under the admin CSP's
|
||||||
|
// script-src 'self' (no 'unsafe-inline').
|
||||||
|
func hasInlineScript(html string) []string {
|
||||||
|
var offenders []string
|
||||||
|
for _, m := range adminInlineScriptRe.FindAllStringSubmatch(html, -1) {
|
||||||
|
if !adminSrcAttrRe.MatchString(m[1]) {
|
||||||
|
offenders = append(offenders, m[0])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return offenders
|
||||||
|
}
|
||||||
|
|
||||||
// TestAdminIndexHTML_NoInlineAssets asserts, server-side, the exact CSP
|
// TestAdminIndexHTML_NoInlineAssets asserts, server-side, the exact CSP
|
||||||
// invariant generate.mjs's own assertNoInlineAssets checks at build time
|
// invariant generate.mjs's own assertNoInlineAssets checks at build time
|
||||||
// (webui/generate.mjs): the decoded adminIndexHTML — what admin.go's
|
// (webui/generate.mjs): the decoded adminIndexHTML — what admin.go's
|
||||||
@@ -714,17 +737,30 @@ func TestAdminIndexHTML_NoInlineAssets(t *testing.T) {
|
|||||||
t.Parallel()
|
t.Parallel()
|
||||||
html := string(adminIndexHTML)
|
html := string(adminIndexHTML)
|
||||||
|
|
||||||
for _, m := range adminInlineScriptRe.FindAllStringSubmatch(html, -1) {
|
for _, tag := range hasInlineScript(html) {
|
||||||
attrs := m[1]
|
t.Errorf("adminIndexHTML contains an inline <script> (no src= attribute): %q", tag)
|
||||||
if !strings.Contains(strings.ToLower(attrs), "src=") {
|
|
||||||
t.Errorf("adminIndexHTML contains an inline <script> (no src= attribute): %q", m[0])
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if strings.Contains(strings.ToLower(html), "<style") {
|
if strings.Contains(strings.ToLower(html), "<style") {
|
||||||
t.Error("adminIndexHTML contains a <style> tag — every style must be an external <link rel=\"stylesheet\">")
|
t.Error("adminIndexHTML contains a <style> tag — every style must be an external <link rel=\"stylesheet\">")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestHasInlineScript_DataSrcIsNotExternal is the negative case
|
||||||
|
// hasInlineScript's adminSrcAttrRe anchoring exists for: a <script
|
||||||
|
// data-src="..."> tag has no REAL src= attribute — data-src is a
|
||||||
|
// distinct, non-standard attribute name, not the src that makes a
|
||||||
|
// <script> external — and must still be flagged as inline. A naive
|
||||||
|
// strings.Contains(attrs, "src=") or a bare \bsrc\s*=\b regexp would both
|
||||||
|
// wrongly treat this tag as external, since "src=" appears as a literal
|
||||||
|
// substring of "data-src=".
|
||||||
|
func TestHasInlineScript_DataSrcIsNotExternal(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
got := hasInlineScript(`<script data-src="x">console.log(1)</script>`)
|
||||||
|
if len(got) != 1 {
|
||||||
|
t.Fatalf("hasInlineScript = %v, want exactly 1 offender (data-src is not src=)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func truncateForTest(b []byte, n int) string {
|
func truncateForTest(b []byte, n int) string {
|
||||||
if len(b) <= n {
|
if len(b) <= n {
|
||||||
return string(b)
|
return string(b)
|
||||||
|
|||||||
+39
-25
@@ -12,33 +12,47 @@ built, baked-in output, never `webui/` itself.
|
|||||||
|
|
||||||
## Warning: `shadcn-vue add`/`init` regressions
|
## Warning: `shadcn-vue add`/`init` regressions
|
||||||
|
|
||||||
`components.json`'s `iconLibrary` and `font` fields are locked to
|
`components.json` has no `iconLibrary` or `font` key. **Do not add them
|
||||||
`"lucide"` and `"inter"` — **not this project's real choices** (this
|
back.** Their omission is the fix, not an oversight — read on before you
|
||||||
project uses FontAwesome only, per `vue-web-development` house skill, and
|
"helpfully" restore either one.
|
||||||
no external font at all). The shadcn-vue CLI's schema does not accept
|
|
||||||
`"none"`/`"fontawesome"`/`"system"` for either field (checked directly:
|
|
||||||
`node_modules/shadcn-vue/dist/preset-*.js`'s `PRESET_ICON_LIBRARIES` and
|
|
||||||
`PRESET_FONTS` arrays — five fixed choices each, no escape hatch), and
|
|
||||||
JSON supports no comment syntax to warn inline (the repo's own
|
|
||||||
pre-commit JSON validator rejects JSONC comments — see `tsconfig.*.json`
|
|
||||||
history). This warning is the only place the trap could be recorded.
|
|
||||||
|
|
||||||
Both `npx shadcn-vue init` and `npx shadcn-vue add <component>` read
|
Earlier revisions of this file set `"iconLibrary": "lucide"` and
|
||||||
these fields and **silently rewrite `src/assets/main.css`** — hit twice
|
`"font": "inter"`, both wrong for this project (FontAwesome only, per
|
||||||
in this project's own history: `init` first, then a follow-up `add`
|
the `vue-web-development` house skill; no external font at all). Both
|
||||||
call, both re-added a `@import url('https://fonts.googleapis.com/...')`
|
`npx shadcn-vue init` and `npx shadcn-vue add <component>` read those two
|
||||||
line and switched the dark-mode `@custom-variant` back to a `.dark`-class
|
fields and, when set, rewrite `src/assets/main.css` — hit twice in this
|
||||||
toggle. If you ever run either command again:
|
project's own history, `init` then a later `add`, both re-adding a
|
||||||
|
`@import url('https://fonts.googleapis.com/...')` line and switching the
|
||||||
|
dark-mode `@custom-variant` back to a `.dark`-class toggle.
|
||||||
|
|
||||||
1. **Diff `src/assets/main.css` before committing.** Remove any
|
The actual shadcn-vue config schema
|
||||||
re-added `@import url('https://fonts.googleapis...')` line (external
|
(`node_modules/shadcn-vue/dist/schema/index.js`'s `rawConfigSchema`)
|
||||||
font, blocked by the admin CSP's `style-src 'self'` anyway) and
|
declares both fields as `z.string().optional()` — not an enum, contrary
|
||||||
restore the `@custom-variant dark` removal (see the file's own header
|
to an earlier, incorrect claim in this project's own review notes that
|
||||||
comment for why: `prefers-color-scheme`, not a class toggle).
|
the CLI schema forced one of five fixed values. The CLI's `init --font`/
|
||||||
2. **Check the new component for a `lucide-vue-next`/`@lucide/vue`
|
`--icon-library` flags do offer five fixed choices each, but that is a
|
||||||
import** and swap it for the FontAwesome equivalent (see
|
flag-parsing constraint, not a `components.json` schema constraint.
|
||||||
`src/components/ui/select/Select*.vue` for the pattern already
|
Deleting both keys leaves them `undefined` after parse, a valid state:
|
||||||
applied there).
|
verified empirically, running `npx shadcn-vue add badge -o -y` against a
|
||||||
|
`components.json` with both keys removed left `src/assets/main.css`
|
||||||
|
byte-for-byte unchanged.
|
||||||
|
|
||||||
|
That verification also surfaced a narrower, remaining trap: the same run
|
||||||
|
still added `@lucide/vue` back to `package.json`'s dependencies (Badge
|
||||||
|
itself does not use it — some components' registry entries list an icon
|
||||||
|
package regardless of `iconLibrary`). So after any future `shadcn-vue
|
||||||
|
add`/`init` call:
|
||||||
|
|
||||||
|
1. **Check `git diff webui/package.json`** for a re-added
|
||||||
|
`lucide-vue-next`/`@lucide/vue` dependency, and remove it
|
||||||
|
(`npm uninstall`) if nothing in `src/` imports from it.
|
||||||
|
2. **Check the new component's own file** for a
|
||||||
|
`lucide-vue-next`/`@lucide/vue` import and swap it for the
|
||||||
|
FontAwesome equivalent (see `src/components/ui/select/Select*.vue`
|
||||||
|
for the pattern already applied there).
|
||||||
|
3. **Diff `src/assets/main.css` anyway** before committing, in case a
|
||||||
|
future shadcn-vue release starts reading a different config key for
|
||||||
|
the same font-injection step.
|
||||||
|
|
||||||
## Local development
|
## Local development
|
||||||
|
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
{
|
{
|
||||||
"$schema": "https://shadcn-vue.com/schema.json",
|
"$schema": "https://shadcn-vue.com/schema.json",
|
||||||
"style": "reka-nova",
|
"style": "reka-nova",
|
||||||
"font": "inter",
|
|
||||||
"typescript": true,
|
"typescript": true,
|
||||||
"tailwind": {
|
"tailwind": {
|
||||||
"config": "",
|
"config": "",
|
||||||
@@ -10,7 +9,6 @@
|
|||||||
"cssVariables": true,
|
"cssVariables": true,
|
||||||
"prefix": ""
|
"prefix": ""
|
||||||
},
|
},
|
||||||
"iconLibrary": "lucide",
|
|
||||||
"rtl": false,
|
"rtl": false,
|
||||||
"pointer": false,
|
"pointer": false,
|
||||||
"aliases": {
|
"aliases": {
|
||||||
|
|||||||
+7
-1
@@ -80,7 +80,13 @@ function assertNoInlineAssets(html, sourceLabel) {
|
|||||||
const scriptTagRe = /<script\b([^>]*)>/gi
|
const scriptTagRe = /<script\b([^>]*)>/gi
|
||||||
let match
|
let match
|
||||||
while ((match = scriptTagRe.exec(html)) !== null) {
|
while ((match = scriptTagRe.exec(html)) !== null) {
|
||||||
if (!/\bsrc\s*=/i.test(match[1])) {
|
// Anchored to attribute-start (start-of-string or preceding
|
||||||
|
// whitespace), not a bare \b word boundary: \b alone matches inside
|
||||||
|
// "data-src=" too (the "-" before "src" is itself a non-word
|
||||||
|
// character, so \b sits right before "src"), which would wrongly
|
||||||
|
// treat a data-src/nosrc/whatever-src attribute as the real src= that
|
||||||
|
// makes a <script> external.
|
||||||
|
if (!/(^|\s)src\s*=/i.test(match[1])) {
|
||||||
console.error(
|
console.error(
|
||||||
`generate.mjs: ${sourceLabel} contains an inline <script> (no src= attribute): ${match[0]}\n` +
|
`generate.mjs: ${sourceLabel} contains an inline <script> (no src= attribute): ${match[0]}\n` +
|
||||||
`The admin CSP ships script-src 'self' with no 'unsafe-inline' — every script must be external.`,
|
`The admin CSP ships script-src 'self' with no 'unsafe-inline' — every script must be external.`,
|
||||||
|
|||||||
Reference in New Issue
Block a user