From 7ed25de12028ba24975c0ac6e43dc77eafc75746 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Tue, 30 Jun 2026 10:29:49 -0700 Subject: [PATCH] fix(ui): allow any git host on the skills add form (LIT-4053) (#31652) * fix(ui): allow any git host on the skills add form (LIT-4053) The skills add form only accepted GitHub URLs: its URL parser bailed on any host that did not start with github.com, so GitLab, Bitbucket, and self-hosted repos (and any repo subfolder on them) were rejected before a request was ever sent. The backend already accepts arbitrary git hosts via its url and git-subdir sources, with no host allowlist, so this was a client-side restriction only. Generalize the parser into an exported, host-agnostic parseSkillSource: GitHub URLs keep their github / git-subdir shorthand, every other host is treated as a raw repo url, and an optional Subfolder path field turns any repo into a git-subdir source (url + path). When a pasted GitHub tree/blob URL already encodes a subfolder, the field is cleared and disabled so a contradictory source can never be submitted. The parser is hardened to match the backend contract: query strings and fragments are stripped, the host match is case-insensitive and drops a leading www., the extracted and field-entered subfolder paths are both validated against the same regex the server uses, a real file-extension allowlist (not "any dot") decides whether a trailing blob segment is a file, a branch-only tree URL falls back to the repo, non-GitHub URLs require at least an org/repo, and the suggested skill name is kebab-cased so it satisfies the name field's own rule. The git-subdir source is now handled in the display helpers (getSourceDisplayText, getSourceLink, formatInstallCommand), which previously showed it as "Unknown source" with no link. The submit path is fully typed (RegisterPluginRequest plus an AddPluginFormValues interface), removing the two prior any usages; as a result an author with an email but no name is dropped rather than sent, since the backend requires the author name. No backend changes. Tests cover the full host/subfolder matrix at the parser level plus form-submit assertions on the exact source payload. * refactor(ui): sync skill register types to the generated OpenAPI schema, surface backend errors Replace the hand-maintained, already-drifted API types for the skills add flow with the generated ones from schema.d.ts: PluginAuthor now aliases components["schemas"]["PluginAuthor"], the registration payload is a new SkillRegisterRequest (the generated RegisterPluginRequest envelope with source narrowed to our PluginSource union, since the backend types source as a loose string map, and version kept optional since the backend defaults it), and the dead, mismatched RegisterPluginResponse is deleted. registerClaudeCodePlugin's inline payload type (which was missing the git-subdir path field entirely) is replaced with SkillRegisterRequest, so the networking layer and the form can no longer drift from the backend. Error handling: the add-skill form swallowed the real failure and always showed "Failed to register skill". registerClaudeCodePlugin already derives the backend message and throws it, so the form now surfaces it ("Failed to register skill: "), and the networking helper falls back to the raw body / status when the error response is not JSON instead of throwing a JSON parse error. A regression test asserts the backend message reaches the user. * fix(ui): reject credentialed git URLs on the skills form A repo URL with embedded user-info (user:token@host) passed the raw-host parser and was stored verbatim as the skill source, which is served on the unauthenticated /public/skill_hub and marketplace.json feeds, leaking the credentials. Reject any host segment containing '@'. * fix(ui): validate skill repo URLs through one WHATWG URL gate Replace the ad-hoc string parsing (stripScheme / splitHost / manual scheme, @, ?# checks) with a single parseRepoUrl gate built on the URL parser, so every malformed/unsafe class is handled in one place and the URL stored on the public skill feeds is always canonical. It enforces https (rejecting http/ssh/git/file/javascript/data and protocol-relative //host), rejects embedded credentials (user:token@host, including userinfo-confusion like github.com@evil.com), rejects IP-literal hosts (loopback/private/metadata and obfuscated/IPv6 forms), and rebuilds the stored url from origin+pathname so query strings, fragments, and trailing slashes can never be published. The GitHub org/repo shorthand is now charset-validated like the other paths, so junk can't reach the stored repo. Closes both Veria findings (credentialed and http sources) plus the adversarial-review follow-ups, with regression tests for each class. --- ui/litellm-dashboard/eslint-metrics.json | 4 +- ui/litellm-dashboard/eslint-suppressions.json | 5 - .../add_plugin_form.test.tsx | 190 ++++++++++++- .../claude_code_plugins/add_plugin_form.tsx | 219 ++++++++------- .../claude_code_plugins/helpers.test.ts | 260 +++++++++++++++++- .../components/claude_code_plugins/helpers.ts | 191 ++++++++++++- .../components/claude_code_plugins/types.ts | 48 +--- .../src/components/networking.tsx | 24 +- 8 files changed, 750 insertions(+), 191 deletions(-) diff --git a/ui/litellm-dashboard/eslint-metrics.json b/ui/litellm-dashboard/eslint-metrics.json index deef1136b43..09ad247391b 100644 --- a/ui/litellm-dashboard/eslint-metrics.json +++ b/ui/litellm-dashboard/eslint-metrics.json @@ -1,5 +1,5 @@ { - "@typescript-eslint/no-explicit-any": 2016, - "complexity": 127, + "@typescript-eslint/no-explicit-any": 2014, + "complexity": 126, "max-depth": 61 } diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index e3ae304c41c..7a3c8f4a42c 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -873,11 +873,6 @@ "count": 1 } }, - "src/components/claude_code_plugins/helpers.test.ts": { - "unused-imports/no-unused-imports": { - "count": 1 - } - }, "src/components/claude_code_plugins/plugin_table.tsx": { "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.test.tsx b/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.test.tsx index 9aa6ec7a969..0453cd8a695 100644 --- a/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.test.tsx +++ b/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.test.tsx @@ -3,11 +3,20 @@ import { act, fireEvent, screen, waitFor } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { renderWithProviders } from "../../../tests/test-utils"; import AddPluginForm from "./add_plugin_form"; +import { registerClaudeCodePlugin } from "../networking"; +import MessageManager from "@/components/molecules/message_manager"; vi.mock("../networking", () => ({ registerClaudeCodePlugin: vi.fn().mockResolvedValue({ status: "success" }), })); +vi.mock("@/components/molecules/message_manager", () => ({ + default: { error: vi.fn(), success: vi.fn() }, +})); + +const mockRegister = vi.mocked(registerClaudeCodePlugin); +const mockMessageError = vi.mocked(MessageManager.error); + const DEFAULT_PROPS = { visible: true, onClose: vi.fn(), @@ -15,22 +24,27 @@ const DEFAULT_PROPS = { onSuccess: vi.fn(), }; +const URL_PLACEHOLDER = "https://github.com/org/repo or https://gitlab.com/org/repo"; +const SUBPATH_PLACEHOLDER = "plugins/my-skill"; + describe("AddPluginForm", () => { beforeEach(() => { vi.clearAllMocks(); }); - it("renders with GitHub URL input", () => { + it("renders the host-agnostic repository URL input and subfolder field", () => { renderWithProviders(); - expect(screen.getByText("GitHub URL")).toBeInTheDocument(); - expect(screen.getByPlaceholderText("https://github.com/org/repo/tree/main/my-skill")).toBeInTheDocument(); + expect(screen.getByText("Repository URL")).toBeInTheDocument(); + expect(screen.getByPlaceholderText(URL_PLACEHOLDER)).toBeInTheDocument(); + expect(screen.getByText("Subfolder path (Optional)")).toBeInTheDocument(); + expect(screen.getByPlaceholderText(SUBPATH_PLACEHOLDER)).toBeInTheDocument(); }); it("shows GitHub repo preview for a plain repo URL", async () => { renderWithProviders(); - const urlInput = screen.getByPlaceholderText("https://github.com/org/repo/tree/main/my-skill"); + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); await act(async () => { fireEvent.change(urlInput, { @@ -43,10 +57,10 @@ describe("AddPluginForm", () => { }); }); - it("shows git-subdir preview for a tree URL", async () => { + it("shows git-subdir preview for a tree URL and disables the subfolder field", async () => { renderWithProviders(); - const urlInput = screen.getByPlaceholderText("https://github.com/org/repo/tree/main/my-skill"); + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); await act(async () => { fireEvent.change(urlInput, { @@ -59,12 +73,51 @@ describe("AddPluginForm", () => { await waitFor(() => { expect(screen.getByText(/GitHub subdir/)).toBeInTheDocument(); }); + expect(screen.getByPlaceholderText(SUBPATH_PLACEHOLDER)).toBeDisabled(); + }); + + it("shows a raw url preview for a non-github host", async () => { + renderWithProviders(); + + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); + + await act(async () => { + fireEvent.change(urlInput, { + target: { value: "https://gitlab.com/group/repo" }, + }); + }); + + await waitFor(() => { + expect(screen.getByText(/Git repo/)).toBeInTheDocument(); + }); + expect(screen.getByPlaceholderText(SUBPATH_PLACEHOLDER)).not.toBeDisabled(); + }); + + it("combines a repo URL with a subfolder into a git-subdir preview", async () => { + renderWithProviders(); + + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); + await act(async () => { + fireEvent.change(urlInput, { + target: { value: "https://gitlab.com/group/repo" }, + }); + }); + + const subPathInput = screen.getByPlaceholderText(SUBPATH_PLACEHOLDER); + await act(async () => { + fireEvent.change(subPathInput, { target: { value: "plugins/x" } }); + }); + + await waitFor(() => { + expect(screen.getByText(/Git subdir/)).toBeInTheDocument(); + expect(screen.getByText(/plugins\/x/)).toBeInTheDocument(); + }); }); it("auto-fills skill name from repo URL", async () => { renderWithProviders(); - const urlInput = screen.getByPlaceholderText("https://github.com/org/repo/tree/main/my-skill"); + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); await act(async () => { fireEvent.change(urlInput, { @@ -84,7 +137,7 @@ describe("AddPluginForm", () => { const nameInput = screen.getByPlaceholderText("my-skill") as HTMLInputElement; fireEvent.change(nameInput, { target: { value: "existing-name" } }); - const urlInput = screen.getByPlaceholderText("https://github.com/org/repo/tree/main/my-skill"); + const urlInput = screen.getByPlaceholderText(URL_PLACEHOLDER); await act(async () => { fireEvent.change(urlInput, { @@ -96,4 +149,125 @@ describe("AddPluginForm", () => { expect(nameInput.value).toBe("existing-name"); }); }); + + const typeUrl = async (value: string) => { + await act(async () => { + fireEvent.change(screen.getByPlaceholderText(URL_PLACEHOLDER), { target: { value } }); + }); + }; + + const typeSubPath = async (value: string) => { + await act(async () => { + fireEvent.change(screen.getByPlaceholderText(SUBPATH_PLACEHOLDER), { target: { value } }); + }); + }; + + const submit = async () => { + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Add Skill" })); + }); + }; + + it("submits a github repo source", async () => { + renderWithProviders(); + + await typeUrl("https://github.com/anthropics/claude-code"); + await submit(); + + await waitFor(() => { + expect(mockRegister).toHaveBeenCalledWith( + "sk-test", + expect.objectContaining({ source: { source: "github", repo: "anthropics/claude-code" } }), + ); + }); + }); + + it("submits a github subdir source from a tree URL", async () => { + renderWithProviders(); + + await typeUrl("https://github.com/anthropics/claude-code/tree/main/plugins/my-skill"); + await submit(); + + await waitFor(() => { + expect(mockRegister).toHaveBeenCalledWith( + "sk-test", + expect.objectContaining({ + source: { source: "git-subdir", url: "https://github.com/anthropics/claude-code", path: "plugins/my-skill" }, + }), + ); + }); + }); + + it("submits a raw url source for a gitlab repo", async () => { + renderWithProviders(); + + await typeUrl("https://gitlab.com/group/repo"); + await submit(); + + await waitFor(() => { + expect(mockRegister).toHaveBeenCalledWith( + "sk-test", + expect.objectContaining({ source: { source: "url", url: "https://gitlab.com/group/repo" } }), + ); + }); + }); + + it("submits a git-subdir source from a gitlab repo plus subfolder field", async () => { + renderWithProviders(); + + await typeUrl("https://gitlab.com/group/repo"); + await typeSubPath("plugins/x"); + await submit(); + + await waitFor(() => { + expect(mockRegister).toHaveBeenCalledWith( + "sk-test", + expect.objectContaining({ + source: { source: "git-subdir", url: "https://gitlab.com/group/repo", path: "plugins/x" }, + }), + ); + }); + }); + + it("clears the subfolder field and uses the URL path once a tree URL is entered", async () => { + renderWithProviders(); + + await typeSubPath("plugins/x"); + const subPathInput = screen.getByPlaceholderText(SUBPATH_PLACEHOLDER) as HTMLInputElement; + expect(subPathInput.value).toBe("plugins/x"); + + await typeUrl("https://github.com/anthropics/claude-code/tree/main/plugins/from-url"); + + await waitFor(() => { + expect(subPathInput.value).toBe(""); + expect(subPathInput).toBeDisabled(); + }); + + await submit(); + + await waitFor(() => { + expect(mockRegister).toHaveBeenCalledWith( + "sk-test", + expect.objectContaining({ + source: { + source: "git-subdir", + url: "https://github.com/anthropics/claude-code", + path: "plugins/from-url", + }, + }), + ); + }); + }); + + it("surfaces the backend error message when registration fails", async () => { + mockRegister.mockRejectedValueOnce(new Error("Plugin 'claude-code' already exists")); + renderWithProviders(); + + await typeUrl("https://github.com/anthropics/claude-code"); + await submit(); + + await waitFor(() => { + expect(mockMessageError).toHaveBeenCalledWith(expect.stringContaining("Plugin 'claude-code' already exists")); + }); + }); }); diff --git a/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.tsx b/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.tsx index f90261ae3a3..a1587bc5189 100644 --- a/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.tsx +++ b/ui/litellm-dashboard/src/components/claude_code_plugins/add_plugin_form.tsx @@ -3,7 +3,17 @@ import { Modal, Form, Input, Select } from "antd"; import MessageManager from "@/components/molecules/message_manager"; import { Button } from "@tremor/react"; import { registerClaudeCodePlugin } from "../networking"; -import { validatePluginName, isValidSemanticVersion, isValidEmail, isValidUrl, parseKeywords } from "./helpers"; +import { + validatePluginName, + isValidSemanticVersion, + isValidEmail, + isValidUrl, + parseKeywords, + parseSkillSource, + isValidSubPath, + SkillSourcePreview, +} from "./helpers"; +import { PluginAuthor, PluginSource, SkillRegisterRequest } from "./types"; const { TextArea } = Input; const { Option } = Select; @@ -15,6 +25,46 @@ interface AddPluginFormProps { onSuccess: () => void; } +interface AddPluginFormValues { + name: string; + skillUrl?: string; + subPath?: string; + version?: string; + description?: string; + authorName?: string; + authorEmail?: string; + homepage?: string; + category?: string; + keywords?: string; + domain?: string; + namespace?: string; +} + +const buildAuthor = (values: AddPluginFormValues): PluginAuthor | undefined => { + const name = values.authorName?.trim(); + const email = values.authorEmail?.trim(); + if (!name) { + return undefined; + } + return email ? { name, email } : { name }; +}; + +const buildRegisterRequest = (values: AddPluginFormValues, source: PluginSource): SkillRegisterRequest => { + const author = buildAuthor(values); + return { + name: values.name.trim(), + source, + ...(values.version ? { version: values.version.trim() } : {}), + ...(values.description ? { description: values.description.trim() } : {}), + ...(author ? { author } : {}), + ...(values.homepage ? { homepage: values.homepage.trim() } : {}), + ...(values.category ? { category: values.category } : {}), + ...(values.keywords ? { keywords: parseKeywords(values.keywords) } : {}), + ...(values.domain ? { domain: values.domain.trim() } : {}), + ...(values.namespace ? { namespace: values.namespace.trim() } : {}), + }; +}; + const PREDEFINED_CATEGORIES = [ "Development", "Productivity", @@ -26,106 +76,41 @@ const PREDEFINED_CATEGORIES = [ "Documentation", ]; -interface ParsedSource { - source: "github" | "url" | "git-subdir"; - repo?: string; - url?: string; - path?: string; -} - -interface ParsePreview { - parsed: ParsedSource; - label: string; - suggestedName: string; -} - -function parseGitHubUrl(raw: string): ParsePreview | null { - // Strip protocol and trailing slashes/spaces - let s = raw - .trim() - .replace(/^https?:\/\//, "") - .replace(/\/+$/, ""); - - if (!s.startsWith("github.com/")) return null; - - // Remove "github.com/" - const rest = s.slice("github.com/".length); - const parts = rest.split("/"); - - if (parts.length < 2) return null; - - const org = parts[0]; - const repo = parts[1]; - const repoBase = repo.replace(/\.git$/, ""); - - // github.com/org/repo (exactly 2 parts, or ends with .git) - if (parts.length === 2 || (parts.length === 2 && repoBase)) { - return { - parsed: { source: "github", repo: `${org}/${repoBase}` }, - label: `GitHub repo — ${org}/${repoBase}`, - suggestedName: repoBase, - }; - } - - // github.com/org/repo/tree/branch/folder or /blob/branch/folder/FILE.md - if (parts.length >= 5 && (parts[2] === "tree" || parts[2] === "blob")) { - // parts[3] = branch, parts[4..] = path segments - const pathParts = parts.slice(4); - // If last segment looks like a file (has extension), drop it - const lastPart = pathParts[pathParts.length - 1]; - if (lastPart && lastPart.includes(".")) { - pathParts.pop(); - } - if (pathParts.length === 0) { - // Path resolved to repo root — treat as plain github source - return { - parsed: { source: "github", repo: `${org}/${repoBase}` }, - label: `GitHub repo — ${org}/${repoBase}`, - suggestedName: repoBase, - }; - } - const subPath = pathParts.join("/"); - const suggestedName = pathParts[pathParts.length - 1]; - return { - parsed: { - source: "git-subdir", - url: `https://github.com/${org}/${repoBase}`, - path: subPath, - }, - label: `GitHub subdir — ${org}/${repoBase} @ ${subPath}`, - suggestedName, - }; - } - - return null; -} - const AddPluginForm: React.FC = ({ visible, onClose, accessToken, onSuccess }) => { const [form] = Form.useForm(); const [isSubmitting, setIsSubmitting] = useState(false); - const [urlPreview, setUrlPreview] = useState(null); + const [urlPreview, setUrlPreview] = useState(null); + const [urlEncodesSubdir, setUrlEncodesSubdir] = useState(false); - const handleUrlChange = (e: React.ChangeEvent) => { - const val = e.target.value; - const preview = parseGitHubUrl(val); + const recomputePreview = (skillUrl: string, subPath: string) => { + const encodesSubdir = parseSkillSource(skillUrl)?.parsed.source === "git-subdir"; + setUrlEncodesSubdir(encodesSubdir); + if (encodesSubdir && form.getFieldValue("subPath")) { + form.setFieldsValue({ subPath: "" }); + } + const preview = parseSkillSource(skillUrl, encodesSubdir ? undefined : subPath); setUrlPreview(preview); - if (preview) { - // Auto-fill name only if it's currently empty - const currentName = form.getFieldValue("name"); - if (!currentName) { - form.setFieldsValue({ name: preview.suggestedName }); - } + if (preview && !form.getFieldValue("name")) { + form.setFieldsValue({ name: preview.suggestedName }); } }; - const handleSubmit = async (values: any) => { + const handleUrlChange = (e: React.ChangeEvent) => { + recomputePreview(e.target.value, form.getFieldValue("subPath") ?? ""); + }; + + const handleSubPathChange = (e: React.ChangeEvent) => { + recomputePreview(form.getFieldValue("skillUrl") ?? "", e.target.value); + }; + + const handleSubmit = async (values: AddPluginFormValues) => { if (!accessToken) { MessageManager.error("No access token available"); return; } if (!urlPreview) { - MessageManager.error("Please enter a valid GitHub URL"); + MessageManager.error("Please enter a valid repository URL"); return; } @@ -151,33 +136,17 @@ const AddPluginForm: React.FC = ({ visible, onClose, accessT setIsSubmitting(true); try { - const pluginData: any = { - name: values.name.trim(), - source: urlPreview.parsed, - }; - - if (values.version) pluginData.version = values.version.trim(); - if (values.description) pluginData.description = values.description.trim(); - if (values.authorName || values.authorEmail) { - pluginData.author = {}; - if (values.authorName) pluginData.author.name = values.authorName.trim(); - if (values.authorEmail) pluginData.author.email = values.authorEmail.trim(); - } - if (values.homepage) pluginData.homepage = values.homepage.trim(); - if (values.category) pluginData.category = values.category; - if (values.keywords) pluginData.keywords = parseKeywords(values.keywords); - if (values.domain) pluginData.domain = values.domain.trim(); - if (values.namespace) pluginData.namespace = values.namespace.trim(); - - await registerClaudeCodePlugin(accessToken, pluginData); + await registerClaudeCodePlugin(accessToken, buildRegisterRequest(values, urlPreview.parsed)); MessageManager.success("Skill registered successfully"); form.resetFields(); setUrlPreview(null); + setUrlEncodesSubdir(false); onSuccess(); onClose(); } catch (error) { console.error("Error registering skill:", error); - MessageManager.error("Failed to register skill"); + const reason = error instanceof Error && error.message ? error.message : "Failed to register skill"; + MessageManager.error(`Failed to register skill: ${reason}`); } finally { setIsSubmitting(false); } @@ -186,6 +155,7 @@ const AddPluginForm: React.FC = ({ visible, onClose, accessT const handleCancel = () => { form.resetFields(); setUrlPreview(null); + setUrlEncodesSubdir(false); onClose(); }; @@ -194,18 +164,45 @@ const AddPluginForm: React.FC = ({ visible, onClose, accessT
{/* Smart URL Input */} + {/* Optional subfolder for monorepos */} + + !value || isValidSubPath(value) + ? Promise.resolve() + : Promise.reject( + new Error( + "Subfolder must be a relative path like plugins/my-skill (letters, numbers, dots, hyphens, underscores)", + ), + ), + }, + ]} + tooltip="Path within the repository where the skill lives (e.g., plugins/my-skill). Leave empty if the skill is at the repo root." + extra={urlEncodesSubdir ? "The URL already points to a subfolder, so this field is disabled" : undefined} + > + + + {/* Parsed preview */} {urlPreview && (
diff --git a/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.test.ts b/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.test.ts index b3930d15718..4c84db2a97d 100644 --- a/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.test.ts +++ b/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.test.ts @@ -15,23 +15,34 @@ import { isValidUrl, parseKeywords, formatKeywords, + parseSkillSource, + isValidSubPath, } from "./helpers"; import { MarketplacePluginEntry, PluginSource } from "./types"; describe("formatInstallCommand", () => { it("formats github source with repo", () => { - const plugin = { name: "my-plugin", source: { source: "github" as const, repo: "org/repo" } }; - expect(formatInstallCommand(plugin)).toBe("/plugin marketplace add org/repo"); + const source: PluginSource = { source: "github", repo: "org/repo" }; + expect(formatInstallCommand({ name: "my-plugin", source })).toBe("/plugin marketplace add org/repo"); }); it("formats url source", () => { - const plugin = { name: "my-plugin", source: { source: "url" as const, url: "https://example.com/plugin" } }; - expect(formatInstallCommand(plugin)).toBe("/plugin marketplace add https://example.com/plugin"); + const source: PluginSource = { source: "url", url: "https://example.com/plugin" }; + expect(formatInstallCommand({ name: "my-plugin", source })).toBe( + "/plugin marketplace add https://example.com/plugin", + ); + }); + + it("formats git-subdir source using its url", () => { + const source: PluginSource = { source: "git-subdir", url: "https://github.com/org/repo", path: "plugins/x" }; + expect(formatInstallCommand({ name: "my-plugin", source })).toBe( + "/plugin marketplace add https://github.com/org/repo", + ); }); it("falls back to plugin name when no repo or url", () => { - const plugin = { name: "my-plugin", source: { source: "github" as const } }; - expect(formatInstallCommand(plugin)).toBe("/plugin marketplace add my-plugin"); + const source: PluginSource = { source: "github" }; + expect(formatInstallCommand({ name: "my-plugin", source })).toBe("/plugin marketplace add my-plugin"); }); }); @@ -91,6 +102,18 @@ describe("getSourceDisplayText", () => { expect(getSourceDisplayText({ source: "url", url: "https://example.com" })).toBe("https://example.com"); }); + it("shows git-subdir as url @ path for a github subdir", () => { + expect(getSourceDisplayText({ source: "git-subdir", url: "https://github.com/org/repo", path: "plugins/x" })).toBe( + "https://github.com/org/repo @ plugins/x", + ); + }); + + it("shows git-subdir as url @ path for a gitlab subdir", () => { + expect(getSourceDisplayText({ source: "git-subdir", url: "https://gitlab.com/org/repo", path: "sub/dir" })).toBe( + "https://gitlab.com/org/repo @ sub/dir", + ); + }); + it("returns unknown for missing data", () => { expect(getSourceDisplayText({ source: "github" })).toBe("Unknown source"); }); @@ -105,6 +128,18 @@ describe("getSourceLink", () => { expect(getSourceLink({ source: "url", url: "https://example.com" })).toBe("https://example.com"); }); + it("returns the repo url for a github git-subdir source", () => { + expect(getSourceLink({ source: "git-subdir", url: "https://github.com/org/repo", path: "plugins/x" })).toBe( + "https://github.com/org/repo", + ); + }); + + it("returns the repo url for a gitlab git-subdir source", () => { + expect(getSourceLink({ source: "git-subdir", url: "https://gitlab.com/org/repo", path: "sub/dir" })).toBe( + "https://gitlab.com/org/repo", + ); + }); + it("returns null when no repo or url", () => { expect(getSourceLink({ source: "github" })).toBeNull(); }); @@ -323,3 +358,216 @@ describe("formatKeywords", () => { expect(formatKeywords(undefined)).toBe(""); }); }); + +describe("parseSkillSource", () => { + it("parses a plain github repo", () => { + expect(parseSkillSource("github.com/org/repo")?.parsed).toEqual({ source: "github", repo: "org/repo" }); + }); + + it("strips a .git suffix from the github repo shorthand", () => { + expect(parseSkillSource("https://github.com/org/repo.git")?.parsed).toEqual({ + source: "github", + repo: "org/repo", + }); + }); + + it("parses a github tree URL into a git-subdir", () => { + expect(parseSkillSource("github.com/org/repo/tree/main/plugins/x")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "plugins/x", + }); + }); + + it("drops a trailing file segment from a github blob URL", () => { + expect(parseSkillSource("github.com/org/repo/blob/main/x/SKILL.md")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "x", + }); + }); + + it("combines a github repo with an explicit subfolder", () => { + expect(parseSkillSource("github.com/org/repo", "plugins/x")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "plugins/x", + }); + }); + + it("treats a gitlab repo as a raw url source", () => { + expect(parseSkillSource("gitlab.com/org/repo")?.parsed).toEqual({ + source: "url", + url: "https://gitlab.com/org/repo", + }); + }); + + it("keeps the .git suffix on raw urls", () => { + expect(parseSkillSource("https://gitlab.com/org/repo.git")?.parsed).toEqual({ + source: "url", + url: "https://gitlab.com/org/repo.git", + }); + }); + + it("combines a gitlab repo with an explicit subfolder", () => { + expect(parseSkillSource("gitlab.com/org/repo", "plugins/x")?.parsed).toEqual({ + source: "git-subdir", + url: "https://gitlab.com/org/repo", + path: "plugins/x", + }); + }); + + it("combines a self-hosted host with an explicit subfolder", () => { + expect(parseSkillSource("https://git.acme.com/team/repo", "sub/dir")?.parsed).toEqual({ + source: "git-subdir", + url: "https://git.acme.com/team/repo", + path: "sub/dir", + }); + }); + + it("lets a github URL-encoded subdir win over an also-provided subfolder", () => { + expect(parseSkillSource("github.com/org/repo/tree/main/plugins/x", "ignored/path")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "plugins/x", + }); + }); + + it("rejects traversal, absolute, and double-slash subfolders", () => { + expect(parseSkillSource("gitlab.com/org/repo", "../etc")).toBeNull(); + expect(parseSkillSource("gitlab.com/org/repo", "/abs")).toBeNull(); + expect(parseSkillSource("gitlab.com/org/repo", "a//b")).toBeNull(); + }); + + it("returns null for empty and garbage input", () => { + expect(parseSkillSource("")).toBeNull(); + expect(parseSkillSource(" ")).toBeNull(); + expect(parseSkillSource("not a url")).toBeNull(); + }); + + it("suggests a kebab-friendly name from the last path segment", () => { + expect(parseSkillSource("github.com/org/my-awesome-skill")?.suggestedName).toBe("my-awesome-skill"); + expect(parseSkillSource("github.com/org/repo/tree/main/plugins/cool-skill")?.suggestedName).toBe("cool-skill"); + expect(parseSkillSource("gitlab.com/org/repo", "plugins/x")?.suggestedName).toBe("x"); + }); + + it("rejects a bad explicit subfolder for a github repo", () => { + expect(parseSkillSource("github.com/org/repo", "../etc")).toBeNull(); + expect(parseSkillSource("github.com/org/repo", "/abs")).toBeNull(); + expect(parseSkillSource("github.com/org/repo", "a//b")).toBeNull(); + }); + + it("treats a blob URL pointing at a root file as the plain repo", () => { + expect(parseSkillSource("github.com/org/repo/blob/main/SKILL.md")?.parsed).toEqual({ + source: "github", + repo: "org/repo", + }); + }); + + it("strips query strings and fragments before parsing", () => { + expect(parseSkillSource("github.com/org/repo?tab=readme")?.parsed).toEqual({ source: "github", repo: "org/repo" }); + expect(parseSkillSource("github.com/org/repo#section")?.parsed).toEqual({ source: "github", repo: "org/repo" }); + }); + + it("rejects a tree URL whose folder has a space or percent-encoded segment", () => { + expect(parseSkillSource("github.com/org/repo/tree/main/a b")).toBeNull(); + expect(parseSkillSource("github.com/org/repo/tree/main/a%20b")).toBeNull(); + }); + + it("routes uppercase and www github hosts through the github shorthand", () => { + expect(parseSkillSource("GitHub.com/org/repo/tree/main/x")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "x", + }); + expect(parseSkillSource("www.github.com/org/repo")?.parsed).toEqual({ source: "github", repo: "org/repo" }); + }); + + it("keeps a dotted folder name as the subdir path", () => { + expect(parseSkillSource("github.com/org/repo/blob/main/my.skill")?.parsed).toEqual({ + source: "git-subdir", + url: "https://github.com/org/repo", + path: "my.skill", + }); + }); + + it("falls back to the repo for a tree URL with a branch but no folder", () => { + expect(parseSkillSource("github.com/org/repo/tree/main")?.parsed).toEqual({ source: "github", repo: "org/repo" }); + }); + + it("kebab-cases the suggested name from a mixed-case repo", () => { + expect(parseSkillSource("github.com/Org/My_Repo")?.suggestedName).toBe("my-repo"); + }); + + it("rejects a bare host or single-segment raw git url", () => { + expect(parseSkillSource("gitlab.com")).toBeNull(); + expect(parseSkillSource("gitlab.com/org")).toBeNull(); + }); +}); + +// Skill sources are served on the unauthenticated public feeds and cloned by clients, so the +// parser must never publish an insecure, credentialed, internal, or malformed clone URL. +describe("parseSkillSource — security boundary", () => { + it("rejects non-https schemes", () => { + for (const url of [ + "http://gitlab.com/org/repo", + "HTTP://gitlab.com/org/repo", + "ssh://gitlab.com/org/repo", + "git://gitlab.com/org/repo", + "ftp://gitlab.com/org/repo", + "file:///etc/passwd", + "javascript:alert(1)", + "data:text/plain,hi", + "//gitlab.com/org/repo", + ]) { + expect(parseSkillSource(url)).toBeNull(); + } + }); + + it("rejects URLs with embedded credentials", () => { + expect(parseSkillSource("https://user:token@gitlab.com/org/repo")).toBeNull(); + expect(parseSkillSource("https://user@gitlab.com/org/repo")).toBeNull(); + // userinfo confusion: the real host is evil.com, not github.com + expect(parseSkillSource("https://github.com@evil.com/org/repo")).toBeNull(); + }); + + it("rejects IP-literal hosts (loopback, private, metadata, obfuscated, IPv6)", () => { + for (const url of [ + "https://127.0.0.1/org/repo", + "https://10.0.0.5/org/repo", + "https://169.254.169.254/org/repo", + "https://2130706433/org/repo", + "https://[::ffff:127.0.0.1]/org/repo", + ]) { + expect(parseSkillSource(url)).toBeNull(); + } + }); + + it("does not grant GitHub shorthand to a look-alike host", () => { + expect(parseSkillSource("https://github.com.evil.com/org/repo")?.parsed).toEqual({ + source: "url", + url: "https://github.com.evil.com/org/repo", + }); + }); + + it("rejects GitHub org/repo segments with illegal characters", () => { + expect(parseSkillSource("github.com/o@x/repo")).toBeNull(); + expect(parseSkillSource("github.com/org/..%2f..%2fx")).toBeNull(); + }); +}); + +describe("isValidSubPath", () => { + it("accepts relative segment paths", () => { + expect(isValidSubPath("plugins/x")).toBe(true); + expect(isValidSubPath("sub/dir")).toBe(true); + expect(isValidSubPath("a.b-c_d")).toBe(true); + expect(isValidSubPath("plugins/x/")).toBe(true); + }); + + it("rejects empty, traversal, absolute, and double-slash paths", () => { + expect(isValidSubPath("")).toBe(false); + expect(isValidSubPath("../etc")).toBe(false); + expect(isValidSubPath("/abs")).toBe(false); + expect(isValidSubPath("a//b")).toBe(false); + }); +}); diff --git a/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.ts b/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.ts index d696a78b4cc..cab3c5cba3c 100644 --- a/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.ts +++ b/ui/litellm-dashboard/src/components/claude_code_plugins/helpers.ts @@ -4,15 +4,189 @@ import { PluginSource, MarketplacePluginEntry } from "./types"; +export interface SkillSourcePreview { + parsed: PluginSource; + label: string; + suggestedName: string; +} + +export const SUBDIR_PATH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._-]*(\/[a-zA-Z0-9][a-zA-Z0-9._-]*)*$/; + +export const normalizeSubPath = (subPath: string): string => subPath.trim().replace(/\/+$/, ""); + +export const isValidSubPath = (subPath: string): boolean => { + const normalized = normalizeSubPath(subPath); + return normalized !== "" && SUBDIR_PATH_REGEX.test(normalized); +}; + +const GITHUB_HOST = "github.com"; + +const SKILL_FILE_EXTENSION_REGEX = /\.(md|markdown|txt|json|ya?ml|toml)$/i; + +// WHATWG normalizes obfuscated IPv4 (e.g. 2130706433, 0x7f.0.0.1) to dotted-decimal, so this +// catches every IPv4 form; bracketed IPv6 is rejected separately. +const IPV4_HOST_REGEX = /^\d{1,3}(\.\d{1,3}){3}$/; + +const GITHUB_ORG_REGEX = /^[A-Za-z0-9-]+$/; +const GITHUB_REPO_REGEX = /^[A-Za-z0-9._-]+$/; + +const buildRepoUrl = (url: URL): string => `${url.protocol}//${url.host}${url.pathname.replace(/\/+$/, "")}`; + +const pathSegments = (url: URL): string[] => url.pathname.split("/").filter((seg) => seg !== ""); + +/** + * Validate and normalize a repository URL into a parsed URL, or null. Enforces https (rejects + * http/ssh/git/etc.), rejects embedded credentials, and requires a dotted host, so the public + * skill feeds never serve an insecure or credentialed clone URL. Everything downstream parses + * this normalized object rather than the raw string. + */ +const parseRepoUrl = (raw: string): URL | null => { + const trimmed = raw.trim(); + if (trimmed === "" || trimmed.startsWith("//")) { + return null; + } + const withScheme = /^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) ? trimmed : `https://${trimmed}`; + let url: URL; + try { + url = new URL(withScheme); + } catch { + return null; + } + if ( + url.protocol !== "https:" || + url.username !== "" || + url.password !== "" || + !url.hostname.includes(".") || + url.hostname.startsWith("[") || + IPV4_HOST_REGEX.test(url.hostname) + ) { + return null; + } + return url; +}; + +const lastSegment = (path: string): string => { + const segments = path.split("/").filter((seg) => seg !== ""); + return segments[segments.length - 1] ?? ""; +}; + +const toKebabCase = (value: string): string => + value + .toLowerCase() + .replace(/[^a-z0-9-]+/g, "-") + .replace(/-+/g, "-") + .replace(/^-+|-+$/g, ""); + +const parseGitHubSource = (url: URL, subPath?: string): SkillSourcePreview | null => { + const parts = pathSegments(url); + if (parts.length < 2) { + return null; + } + + const org = parts[0]; + const repoBase = parts[1].replace(/\.git$/, ""); + if (!GITHUB_ORG_REGEX.test(org) || !GITHUB_REPO_REGEX.test(repoBase)) { + return null; + } + const repoFull = `${org}/${repoBase}`; + const repoUrl = `https://github.com/${repoFull}`; + const repoPreview: SkillSourcePreview = { + parsed: { source: "github", repo: repoFull }, + label: `GitHub repo — ${repoFull}`, + suggestedName: toKebabCase(repoBase), + }; + + const isTreeOrBlob = parts.length >= 4 && (parts[2] === "tree" || parts[2] === "blob"); + if (isTreeOrBlob) { + const pathParts = parts.slice(4); + const last = lastSegment(pathParts.join("/")); + const effective = SKILL_FILE_EXTENSION_REGEX.test(last) ? pathParts.slice(0, -1) : pathParts; + if (effective.length === 0) { + return repoPreview; + } + const path = normalizeSubPath(effective.join("/")); + if (!SUBDIR_PATH_REGEX.test(path)) { + return null; + } + return { + parsed: { source: "git-subdir", url: repoUrl, path }, + label: `GitHub subdir — ${repoFull} @ ${path}`, + suggestedName: toKebabCase(lastSegment(path)), + }; + } + + if (parts.length !== 2) { + return null; + } + + const normalized = normalizeSubPath(subPath ?? ""); + if (normalized !== "") { + if (!SUBDIR_PATH_REGEX.test(normalized)) { + return null; + } + return { + parsed: { source: "git-subdir", url: repoUrl, path: normalized }, + label: `GitHub subdir — ${repoFull} @ ${normalized}`, + suggestedName: toKebabCase(lastSegment(normalized)), + }; + } + + return repoPreview; +}; + +const parseRawGitSource = (url: URL, subPath?: string): SkillSourcePreview | null => { + if (pathSegments(url).length < 2) { + return null; + } + + const repoUrl = buildRepoUrl(url); + + const normalized = normalizeSubPath(subPath ?? ""); + if (normalized !== "") { + if (!SUBDIR_PATH_REGEX.test(normalized)) { + return null; + } + return { + parsed: { source: "git-subdir", url: repoUrl, path: normalized }, + label: `Git subdir — ${repoUrl} @ ${normalized}`, + suggestedName: toKebabCase(lastSegment(normalized)), + }; + } + + return { + parsed: { source: "url", url: repoUrl }, + label: `Git repo — ${repoUrl}`, + suggestedName: toKebabCase(lastSegment(url.pathname).replace(/\.git$/, "")), + }; +}; + +/** + * Parse any git-accessible repository URL into a registerable skill source. + * GitHub URLs keep their `github`/`git-subdir` shorthand; every other host is + * treated as a raw repo URL, with an optional subfolder turning it into git-subdir. + */ +export const parseSkillSource = (rawUrl: string, subPath?: string): SkillSourcePreview | null => { + const url = parseRepoUrl(rawUrl); + if (!url) { + return null; + } + if (url.hostname.replace(/^www\./, "") === GITHUB_HOST) { + return parseGitHubSource(url, subPath); + } + return parseRawGitSource(url, subPath); +}; + /** * Generate install command for Claude Code CLI * Format: /plugin marketplace add org/repo OR /plugin marketplace add url */ export const formatInstallCommand = (plugin: { name: string; source: PluginSource }): string => { - if (plugin.source.source === "github" && plugin.source.repo) { - return `/plugin marketplace add ${plugin.source.repo}`; - } else if (plugin.source.source === "url" && plugin.source.url) { - return `/plugin marketplace add ${plugin.source.url}`; + const { source } = plugin; + if (source.source === "github" && source.repo) { + return `/plugin marketplace add ${source.repo}`; + } + if ((source.source === "url" || source.source === "git-subdir") && source.url) { + return `/plugin marketplace add ${source.url}`; } // Fallback to plugin name return `/plugin marketplace add ${plugin.name}`; @@ -55,7 +229,11 @@ export const validatePluginName = (name: string): boolean => { export const getSourceDisplayText = (source: PluginSource): string => { if (source.source === "github" && source.repo) { return `GitHub: ${source.repo}`; - } else if (source.source === "url" && source.url) { + } + if (source.source === "git-subdir" && source.url && source.path) { + return `${source.url} @ ${source.path}`; + } + if (source.source === "url" && source.url) { return source.url; } return "Unknown source"; @@ -67,7 +245,8 @@ export const getSourceDisplayText = (source: PluginSource): string => { export const getSourceLink = (source: PluginSource): string | null => { if (source.source === "github" && source.repo) { return `https://github.com/${source.repo}`; - } else if (source.source === "url" && source.url) { + } + if ((source.source === "url" || source.source === "git-subdir") && source.url) { return source.url; } return null; diff --git a/ui/litellm-dashboard/src/components/claude_code_plugins/types.ts b/ui/litellm-dashboard/src/components/claude_code_plugins/types.ts index fcb1146685d..d16c880749b 100644 --- a/ui/litellm-dashboard/src/components/claude_code_plugins/types.ts +++ b/ui/litellm-dashboard/src/components/claude_code_plugins/types.ts @@ -1,8 +1,12 @@ /** * TypeScript types for Claude Code Marketplace - * Matches backend API types from /litellm/types/proxy/claude_code_endpoints.py + * API request/response shapes are synced from the generated OpenAPI types in @/lib/http/schema. */ +import type { components } from "@/lib/http/schema"; + +// Kept hand-written: the backend types `source` as Dict[str, str], so the generated type is a +// loose string map; this discriminant union is what the parser and display helpers rely on. export interface PluginSource { source: "github" | "url" | "git-subdir"; repo?: string; // Format: "org/repo" for GitHub @@ -10,10 +14,7 @@ export interface PluginSource { path?: string; // Subdirectory path for git-subdir } -export interface PluginAuthor { - name: string; - email?: string; -} +export type PluginAuthor = components["schemas"]["PluginAuthor"]; export interface Plugin { id: string; @@ -56,24 +57,12 @@ export interface ListPluginsResponse { count: number; } -export interface RegisterPluginRequest { - name: string; +// Request envelope synced from the OpenAPI spec, with `source` narrowed to our PluginSource +// union and `version` kept optional (the backend supplies its default). +export type SkillRegisterRequest = Omit & { source: PluginSource; version?: string; - description?: string; - author?: PluginAuthor; - homepage?: string; - keywords?: string[]; - category?: string; - domain?: string; - namespace?: string; -} - -export interface RegisterPluginResponse { - plugin: Plugin; - action: "created" | "updated"; - message: string; -} +}; // Public marketplace types export interface MarketplacePluginEntry { @@ -104,20 +93,3 @@ export interface CategoryTab { label: string; count: number; } - -export interface PluginFormData { - name: string; - sourceType: "github" | "url" | "git-subdir"; - repo: string; - url: string; - path: string; - version: string; - description: string; - authorName: string; - authorEmail: string; - homepage: string; - category: string; - keywords: string; // Comma-separated string, will be split into array - domain: string; - namespace: string; -} diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index f5bae832e64..0bbf4d2a6e8 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -27,6 +27,7 @@ import { TagNewRequest, TagUpdateRequest, TagListResponse, TagInfoResponse } fro import { Team } from "./key_team_helpers/key_list"; import { UserInfo } from "./view_users/types"; import { EmailEventSettingsResponse, EmailEventSettingsUpdateRequest } from "./email_events/types"; +import type { SkillRegisterRequest } from "./claude_code_plugins/types"; import { jsonFields } from "./common_components/check_openapi_schema"; import NotificationsManager from "./molecules/notifications_manager"; import type { MCPUserEnvVarsStatus } from "./mcp_tools/types"; @@ -7402,19 +7403,7 @@ export const getClaudeCodePluginDetails = async (accessToken: string, pluginName * @param accessToken - Admin access token * @param pluginData - Plugin registration data */ -export const registerClaudeCodePlugin = async ( - accessToken: string, - pluginData: { - name: string; - source: { source: string; repo?: string; url?: string }; - version?: string; - description?: string; - author?: { name: string; email?: string }; - homepage?: string; - keywords?: string[]; - category?: string; - }, -) => { +export const registerClaudeCodePlugin = async (accessToken: string, pluginData: SkillRegisterRequest) => { try { const proxyBaseUrl = getProxyBaseUrl(); const url = proxyBaseUrl ? `${proxyBaseUrl}/claude-code/plugins` : `/claude-code/plugins`; @@ -7429,8 +7418,13 @@ export const registerClaudeCodePlugin = async ( }); if (!response.ok) { - const errorData = await response.text(); - const errorMessage = deriveErrorMessage(JSON.parse(errorData)); + const errorBody = await response.text(); + let errorMessage: string; + try { + errorMessage = deriveErrorMessage(JSON.parse(errorBody)); + } catch { + errorMessage = errorBody || `Request failed with status ${response.status}`; + } handleError(errorMessage); throw new Error(errorMessage); }