From c727cd0e04d80a7f1ae6324624f34e411a65f5f7 Mon Sep 17 00:00:00 2001 From: Antonio Zaitoun Date: Mon, 28 Sep 2026 17:19:07 +0300 Subject: [PATCH 1/5] Install skills and plugins from any git host A clone URL ending in .git on a host other than github.com or gitlab.com used to fall through to the raw-URL skill type, which saved the host's HTML page as SKILL.md and reported success. Skills and plugins get a `git` type with `def.url` (the clone URL). It reuses the existing mirror cache, keyed by host and path, so pinning by tag or commit, "latest version tag" and the lockfile work as for GitHub. Credentials come from the user's git credential helper. `capa add` recognizes `https://host/path/repo.git[::path][:version|#sha]`, rules and hooks refuse such URLs until they support them, and a remote skill that returns a web page now fails instead of installing HTML. Co-Authored-By: Claude Opus 5.5 --- .../references/capabilities-schema.md | 13 ++- .../references/commands.md | 2 + src/cli/commands/add-builders.ts | 13 +++ src/cli/commands/add-parse-plugin.ts | 17 ++- src/cli/commands/add-parse-skill.ts | 20 +++- src/cli/commands/add.ts | 7 +- src/cli/commands/install-tasks/context.ts | 2 +- .../__tests__/install-one-skill-git.test.ts | 100 ++++++++++++++++++ src/cli/commands/install-tasks/helpers/git.ts | 54 +++++++++- .../helpers/install-one-skill.ts | 77 +++++++++++++- .../install-tasks/helpers/repo-snapshot.ts | 10 +- .../commands/install-tasks/install-skills.ts | 2 +- .../commands/install-tasks/write-lockfile.ts | 2 +- src/cli/commands/plugin-install.ts | 26 +++-- src/server/resolve-effective-capabilities.ts | 2 +- src/shared/__tests__/git-url.test.ts | 74 +++++++++++++ src/shared/cache/mirror.ts | 3 + src/shared/cache/paths.ts | 3 +- src/shared/capabilities.ts | 7 +- src/shared/executable-surface.ts | 6 +- src/shared/git-url.ts | 66 ++++++++++++ src/shared/lockfile.ts | 4 +- src/shared/plugin-source.ts | 44 ++++++++ src/types/capabilities.ts | 4 +- src/types/lockfile.ts | 13 ++- src/types/plugin.ts | 7 +- 26 files changed, 535 insertions(+), 43 deletions(-) create mode 100644 src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts create mode 100644 src/shared/__tests__/git-url.test.ts create mode 100644 src/shared/git-url.ts diff --git a/skills/capabilities-manager/references/capabilities-schema.md b/skills/capabilities-manager/references/capabilities-schema.md index 7496a4a2..6bb8ea1e 100644 --- a/skills/capabilities-manager/references/capabilities-schema.md +++ b/skills/capabilities-manager/references/capabilities-schema.md @@ -38,7 +38,7 @@ tools: # rules: [ { id, type, content?, url?, path?, def?, providers?, appliesTo?, alwaysApply?, description?, visibility?, scope? } ] -# plugins: [ { id?, type: github|gitlab, def: { repo, subpath?, version?, ref?, description? }, servers?: { : { as?: , expose?: all|except|exactly|none, tools?: [] } } } ] +# plugins: [ { id?, type: github|gitlab|git, def: { repo (github/gitlab) | url (git), subpath?, version?, ref?, description? }, servers?: { : { as?: , expose?: all|except|exactly|none, tools?: [] } } } ] # Plugins unpack into skills + servers + rules + sub-agents + hooks (Claude/Cursor manifests). # `def.repo` mirrors the skill grammar: # - `owner/repo` — manifest at the repo root @@ -48,11 +48,12 @@ tools: # subagents: [ { id, providers?, description?, skills, tools, instructions? } ] ``` -## Skills Section (seven types) +## Skills Section (eight types) - **inline**: Embed SKILL.md content in `def.content`. Use `requires: ['@server_id.tool_id']` for MCP tools, plain ID for command tools. - **github**: `def.repo: owner/repo@skill-name` (search) or `owner/repo::path/to/skill-name` (exact). See "Repo string format" below for details. - **gitlab**: `def.repo: group/subgroup/repo@skill-name` (search) or `group/subgroup/repo::path/to/skill-name` (exact). Subgroups are supported. +- **git**: `def.url` is the clone URL of any git host (ends in `.git`); `def.path` is the skill directory inside the repo (default: the root). Pin with `def.version` (tag/branch) or `def.ref` (commit). Unpinned installs take the newest `vX.Y.Z` tag. Private repos use your git credential helper. - **remote**: `def.url` to a SKILL.md URL. - **local**: `def.path` to a directory containing SKILL.md (read on each install). - **installed**: User installed the skill elsewhere; capa only records it for tool binding. Declare directly in capabilities.yaml (no CLI shortcut) — set `def.description` and `def.requires` as needed. @@ -543,6 +544,14 @@ plugins: def: repo: acme/platform/team/services/devops-skills version: v1.0.1 + + # Any other git host: the clone URL, plus an optional subpath. + - id: deploy-tools + type: git + def: + url: https://git.example.com/team/tools.git + subpath: plugins/deploy + version: v2.0.0 ``` Plugins are resolved during `capa install`. The plugin manifest is fetched from the repository; skill files are copied onto disk for each provider; MCP servers are registered; rules / sub-agents / hooks from the plugin are merged into the install pipeline (`${CLAUDE_PLUGIN_ROOT}` in hook scripts is rewritten to the stable plugin copy under `~/.capa/plugins//`). Plugin tools must be declared explicitly in the top-level `tools` section using `@server-id` references; plugin skills are activated via `type: plugin` skill entries (capa warns when a `type: plugin` skill id does not match any plugin manifest skill). diff --git a/skills/capabilities-manager/references/commands.md b/skills/capabilities-manager/references/commands.md index 4ac03145..46abf14b 100644 --- a/skills/capabilities-manager/references/commands.md +++ b/skills/capabilities-manager/references/commands.md @@ -112,6 +112,7 @@ Add an entry to `capabilities.yaml` (default) or write provider-native files imm - **Pinning** (works with both `@` and `::`): append `:v1.2.3` for a tag/branch or `#abc1234` for a commit SHA — e.g. `capa add owner/repo@skill:v1.2.3`, `capa add gitlab:group/repo::skills/x/y#abc1234`. - **Registry**: `capa add :` — resolve the skill from a configured registry adapter. Example: `capa add skills-sh:vercel-labs/skills/find-skills`. - **Installed (no CLI)**: declare `type: installed` directly in `capabilities.yaml` — capa only records the skill for tool binding, it does not fetch or install anything. Set `def.description` and `def.requires` on the entry. Same pattern for `type: plugin` (skills shipped by a configured plugin). +- **Any git host**: `capa add https://git.example.com/team/skills.git` — a clone URL ending in `.git`, stored as type `git` with `def.url`. Installs the `SKILL.md` at the repo root; add `::path/to/skill` for another directory, and `:v1.2.3` / `#abc1234` to pin. Private repos use your git credential helper (clone once with `git` to sign in). - **Remote URL**: `capa add https://example.com/path/to/SKILL.md` - **Local path**: `capa add ./path/to/skill` — directory must contain `SKILL.md`; stored as type `local` so the file is read on each install @@ -123,6 +124,7 @@ Use `--plugin` to add a plugin entry (skills, MCP, rules, sub-agents, hooks) ins - **GitHub subpath**: `capa add --plugin anthropics/claude-plugins-official::plugins/frontend-design` - **GitLab (nested groups)**: `capa add --plugin gitlab:acme/platform/team/services/devops-skills:v1.0.1` - **URL**: `capa add --plugin https://github.com/anthropics/claude-plugins-official/tree/main/plugins/code-review` +- **Any git host**: `capa add --plugin https://git.example.com/team/plugin.git[::path/to/plugin]` — stored as type `git` with `def.url` (and `def.subpath`) - **Registry**: `capa add claude-plugins:frontend-design` — the registry adapter determines whether the item is a skill or plugin. ### Servers, tools, rules, hooks diff --git a/src/cli/commands/add-builders.ts b/src/cli/commands/add-builders.ts index 834ceb81..3c905461 100644 --- a/src/cli/commands/add-builders.ts +++ b/src/cli/commands/add-builders.ts @@ -6,6 +6,7 @@ import { basename, resolve, relative } from 'path'; import { access } from 'fs/promises'; import { constants } from 'fs'; +import { isGitUrl } from '../../shared/git-url'; import { CANONICAL_HOOK_EVENTS, type Hook, type HookSource } from '../../types/hooks'; import type { Rule } from '../../types/rules'; import type { MCPServer, Tool, SecretValue } from '../../types/capabilities'; @@ -328,6 +329,12 @@ export async function parseRuleSource(source: string): Promise }; } + if (isGitUrl(source)) { + throw new Error( + `Rules from a git clone URL are not supported yet: ${source}\n` + + ` Use a raw URL to the rule file, or a GitHub/GitLab repo.`, + ); + } if (source.startsWith('http://') || source.startsWith('https://')) { return { id: basename(source).replace(/\.md$/i, '') || 'custom-rule', @@ -422,6 +429,12 @@ function parseHookSource(raw: string): HookSource { : absPath; return { type: 'local', path: pathToStore }; } + if (isGitUrl(trimmed)) { + throw new Error( + `Hooks from a git clone URL are not supported yet: ${raw}\n` + + ` Use a raw URL to the hook script, or a GitHub/GitLab repo.`, + ); + } if (trimmed.startsWith('http://') || trimmed.startsWith('https://')) { return { type: 'remote', url: trimmed }; } diff --git a/src/cli/commands/add-parse-plugin.ts b/src/cli/commands/add-parse-plugin.ts index 2b640edd..94809257 100644 --- a/src/cli/commands/add-parse-plugin.ts +++ b/src/cli/commands/add-parse-plugin.ts @@ -2,9 +2,10 @@ import { basename } from 'path'; import type { PluginDefinition } from '../../types/plugin'; import { validatePluginDef } from '../../shared/plugin-source'; import { getAllGitProviders } from '../../shared/git-providers/registry'; +import { parseGitUrlSource } from '../../shared/git-url'; export interface ParsedPluginSource { - type: 'github' | 'gitlab'; + type: 'github' | 'gitlab' | 'git'; def: PluginDefinition; idHint: string; } @@ -63,6 +64,17 @@ export function parsePluginSource(source: string): ParsedPluginSource { return buildPluginSourceFromRepoUrl(gp.id as 'github' | 'gitlab', parsed); } + // A clone URL on any other git host: https://host/path/repo.git[::subpath][:version|#sha] + const gitSource = parseGitUrlSource(source); + if (gitSource) { + const { url, path, version, ref, idHint } = gitSource; + const def: PluginDefinition = { url }; + if (path) def.subpath = path; + if (version) def.version = version; + if (ref) def.ref = ref; + return assertValidPluginSource({ type: 'git', def, idHint }); + } + // GitLab `@name` search: gitlab:group/sub/project@plugin-name[:version|#sha] const gitlabAtMatch = source.match( /^gitlab:([\w.-]+(?:\/[\w.-]+)+)@([\w.-]+)(?::([\w.-]+))?(?:#([a-f0-9]{7,40}))?$/i @@ -138,7 +150,8 @@ export function parsePluginSource(source: string): ParsedPluginSource { ` - Nested groups: gitlab:group/sub/project\n` + ` - Exact subpath: gitlab:group/project::plugins/my-plugin\n` + ` - Recursive @: gitlab:group/project@my-plugin\n` + - ` - URL: https://gitlab.com/group/project/-/tree/main/plugins/my-plugin\n\n` + + ` - URL: https://gitlab.com/group/project/-/tree/main/plugins/my-plugin\n` + + ` Any git host: https://git.example.com/team/plugin.git[::path/to/plugin]\n\n` + `Pinning (any of the above):\n` + ` - Tag: capa add --plugin owner/repo@my-plugin:v1.2.3\n` + ` - Commit: capa add --plugin owner/repo@my-plugin#abc123def\n\n` + diff --git a/src/cli/commands/add-parse-skill.ts b/src/cli/commands/add-parse-skill.ts index e57634aa..abc5f80a 100644 --- a/src/cli/commands/add-parse-skill.ts +++ b/src/cli/commands/add-parse-skill.ts @@ -1,10 +1,11 @@ import { resolve, basename, join, relative } from 'path'; import { access } from 'fs/promises'; import { constants } from 'fs'; +import { parseGitUrlSource } from '../../shared/git-url'; export interface ParsedSkillSource { id: string; - type: 'remote' | 'github' | 'gitlab' | 'local'; + type: 'remote' | 'github' | 'gitlab' | 'git' | 'local'; def: { repo?: string; url?: string; @@ -154,6 +155,22 @@ export async function parseSkillSource(source: string): Promise (p as { id?: string }).id === resolved.itemName || (p.type === newPlugin.type && - p.def.repo === newPlugin.def.repo && + pluginSourceOf(p.def) === pluginSourceOf(newPlugin.def) && (p.def.subpath ?? '') === (newPlugin.def.subpath ?? '')), ); if (existing) { @@ -229,7 +230,7 @@ export async function addCommand( const dup = capabilities.plugins.find(p => p.id === id || (p.type === parsed.type - && p.def.repo === parsed.def.repo + && pluginSourceOf(p.def) === pluginSourceOf(parsed.def) && (p.def.subpath ?? '') === (parsed.def.subpath ?? ''))); if (dup) { console.error(`✗ Plugin "${id}" already exists in capabilities file.`); @@ -246,7 +247,7 @@ export async function addCommand( console.log(`✓ Added plugin "${id}" to ${capabilitiesFile.path}`); console.log(` Type: ${parsed.type}`); - console.log(` Repo: ${parsed.def.repo}`); + console.log(` Repo: ${pluginSourceOf(parsed.def)}`); if (parsed.def.version) console.log(` Version: ${parsed.def.version}`); if (parsed.def.ref) console.log(` Ref: ${parsed.def.ref}`); diff --git a/src/cli/commands/install-tasks/context.ts b/src/cli/commands/install-tasks/context.ts index 3b62897e..74348cd9 100644 --- a/src/cli/commands/install-tasks/context.ts +++ b/src/cli/commands/install-tasks/context.ts @@ -111,5 +111,5 @@ export type GetRepoSnapshotFn = ( platform: CachePlatform, repoPath: string, authFetch: AuthenticatedFetch, - opts?: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean } + opts?: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean; repoUrl?: string } ) => Promise diff --git a/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts new file mode 100644 index 00000000..715294d0 --- /dev/null +++ b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { execFileSync } from 'child_process'; +import { pathToFileURL } from 'url'; +import { CapaDatabase } from '../../../../../db/database'; +import { LockfileBuilder } from '../../../../../shared/lockfile'; +import { installOneSkill } from '../install-one-skill'; +import type { Capabilities, Skill } from '../../../../../types/capabilities'; + +// A skill on "any git host": a real repository cloned over file://, so the +// whole path (clone, tag resolution, snapshot, copy, lock entry) runs for real. +describe('installOneSkill with a git clone URL', () => { + let tempDir: string; + let projectPath: string; + let repoUrl: string; + let db: CapaDatabase; + const prevCacheDir = process.env.CAPA_CACHE_DIR; + + const git = (cwd: string, ...args: string[]) => + execFileSync('git', args, { cwd, encoding: 'utf8' }).trim(); + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'capa-git-skill-')); + process.env.CAPA_CACHE_DIR = join(tempDir, 'cache'); + projectPath = join(tempDir, 'project'); + mkdirSync(projectPath, { recursive: true }); + + const repo = join(tempDir, 'work'); + mkdirSync(join(repo, 'nested', 'lint'), { recursive: true }); + git(repo, 'init', '-q', '-b', 'main'); + git(repo, 'config', 'user.email', 'capa-test@example.com'); + git(repo, 'config', 'user.name', 'capa-test'); + writeFileSync(join(repo, 'SKILL.md'), '---\nname: code-review\ndescription: v1\n---\n\nFirst.\n'); + writeFileSync(join(repo, 'nested', 'lint', 'SKILL.md'), '---\nname: lint\ndescription: lint\n---\n'); + git(repo, 'add', '-A'); + git(repo, 'commit', '-q', '-m', 'first'); + git(repo, 'tag', 'v1.0.0'); + writeFileSync(join(repo, 'SKILL.md'), '---\nname: code-review\ndescription: unreleased\n---\n\nSecond.\n'); + git(repo, 'commit', '-q', '-am', 'second'); + // Served like a real host: a bare repository whose name ends in .git. + const bare = join(tempDir, '@acme', 'code-review.git'); + mkdirSync(join(tempDir, '@acme')); + git(tempDir, 'clone', '-q', '--bare', repo, bare); + repoUrl = pathToFileURL(bare).href; + + db = new CapaDatabase(join(tempDir, 'capa.db')); + db.upsertProject({ id: 'proj', path: projectPath }); + }); + + afterEach(() => { + db.close(); + if (prevCacheDir === undefined) delete process.env.CAPA_CACHE_DIR; + else process.env.CAPA_CACHE_DIR = prevCacheDir; + rmSync(tempDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); + }); + + async function install(skill: Skill, lockBuilder = new LockfileBuilder()) { + const capabilities: Capabilities = { providers: ['cursor'], skills: [skill], servers: [], tools: [] }; + await installOneSkill( + skill, + projectPath, + 'proj', + ['cursor'], + db, + { server: { host: '127.0.0.1', port: 5912 } }, + capabilities, + join(projectPath, 'capabilities.yaml'), + lockBuilder, + false, + new Map(), + ); + return lockBuilder.build(); + } + + it('installs the root SKILL.md at the latest version tag and locks the URL', async () => { + const lock = await install({ id: 'code-review', type: 'git', def: { url: repoUrl } }); + + const installed = readFileSync(join(projectPath, '.cursor', 'skills', 'code-review', 'SKILL.md'), 'utf8'); + expect(installed).toContain('description: v1'); + expect(lock.skills[0]).toMatchObject({ + id: 'code-review', + source: 'git', + url: repoUrl, + resolvedVersion: 'v1.0.0', + }); + }); + + it('installs a nested skill from def.path', async () => { + await install({ id: 'lint', type: 'git', def: { url: repoUrl, path: 'nested/lint' } }); + expect(readFileSync(join(projectPath, '.cursor', 'skills', 'lint', 'SKILL.md'), 'utf8')).toContain('name: lint'); + }); + + it('names the skills it found when SKILL.md is missing', async () => { + await expect( + install({ id: 'x', type: 'git', def: { url: repoUrl, path: 'nested' } }), + ).rejects.toThrow(/nested\/lint/); + }); +}); diff --git a/src/cli/commands/install-tasks/helpers/git.ts b/src/cli/commands/install-tasks/helpers/git.ts index cdd95344..dab252d7 100644 --- a/src/cli/commands/install-tasks/helpers/git.ts +++ b/src/cli/commands/install-tasks/helpers/git.ts @@ -26,17 +26,65 @@ export function gitOAuthHelpText(): string { ); } +function hostOf(url: string): string { + try { + return new URL(url).host || url; + } catch { + return url; + } +} + +// Any git host other than github.com / gitlab.com: capa has no integration to +// connect, so credentials come from the user's own git credential helper. +function explainGenericGitError(error: any, errorMessage: string, repoUrl: string, host: string): Error { + if ( + errorMessage.includes('git: command not found') || + errorMessage.includes("'git' is not recognized") || + errorMessage.includes('git: not found') || + error?.code === 'ENOENT' + ) { + return new Error('Git is not installed — install git and re-run `capa install` (https://git-scm.com/downloads).'); + } + if ( + errorMessage.includes('Authentication failed') || + errorMessage.includes('could not read Username') || + errorMessage.includes('could not read Password') || + errorMessage.includes('terminal prompts disabled') + ) { + return new Error( + `Authentication failed for ${repoUrl} (repository not accessible)\n` + + `If the repository is private, sign in once with \`git clone ${repoUrl}\` so your git credential helper ` + + `remembers the credentials, then re-run \`capa install\`.` + ); + } + if (errorMessage.includes('not found') || errorMessage.includes('does not appear to be a git repository')) { + return new Error(`Repository not found: ${repoUrl}\nCheck the URL, or sign in with git if it is private.`); + } + if (errorMessage.includes('unable to access') || errorMessage.includes('Could not resolve host')) { + return new Error(`Network error: cannot reach ${host} — check the URL and your connection.`); + } + const fatal = + errorMessage.split('\n').find((line: string) => line.includes('fatal:') || line.includes('error:')) || + 'Unknown error'; + return new Error(`Failed to clone ${repoUrl}: ${fatal}`); +} + // Returns short, actionable messages; callers wrap them into the per-skill // error block which already prefixes `Skill "" failed:`. export function explainGitError( error: any, platform: CachePlatform, repoPath: string, - hasAuth: boolean + hasAuth: boolean, + cloneUrl?: string ): Error { const errorMessage: string = error?.stderr || error?.message || ''; - const platformName = getGitProvider(platform)?.displayName ?? platform; - const repoUrl = `https://${platform}.com/${repoPath}`; + const host = cloneUrl ? hostOf(cloneUrl) : `${platform}.com`; + const platformName = getGitProvider(platform)?.displayName ?? host; + const repoUrl = cloneUrl ?? `https://${platform}.com/${repoPath}`; + if (platform === 'git') { + return explainGenericGitError(error, errorMessage, repoUrl, host); + } if ( errorMessage.includes('git: command not found') || diff --git a/src/cli/commands/install-tasks/helpers/install-one-skill.ts b/src/cli/commands/install-tasks/helpers/install-one-skill.ts index 47ab57d7..d48a2314 100644 --- a/src/cli/commands/install-tasks/helpers/install-one-skill.ts +++ b/src/cli/commands/install-tasks/helpers/install-one-skill.ts @@ -1,5 +1,6 @@ import { existsSync, mkdirSync, writeFileSync, rmSync } from 'fs'; -import { resolve, join, dirname, basename } from 'path'; +import { resolve, join, dirname, basename, relative } from 'path'; +import { gitRepoKey } from '../../../../shared/git-url'; import type { Skill, Capabilities } from '../../../../types/capabilities'; import type { CapaDatabase } from '../../../../db/database'; import { createAuthenticatedFetch, AuthenticatedFetch } from '../../../../shared/authenticated-fetch'; @@ -34,8 +35,8 @@ import type { SkillInstallOutcome } from '../context'; function buildInvalidSkillMessage(skill: Skill): string { const lines = [`Invalid skill definition: ${skill.id}`]; - if (!skill.type || !['inline', 'remote', 'github', 'gitlab', 'local', 'installed', 'plugin'].includes(skill.type)) { - lines.push(` Invalid or missing 'type'. Must be one of: 'inline', 'remote', 'github', 'gitlab', 'local', 'installed', 'plugin'`); + if (!skill.type || !['inline', 'remote', 'github', 'gitlab', 'git', 'local', 'installed', 'plugin'].includes(skill.type)) { + lines.push(` Invalid or missing 'type'. Must be one of: 'inline', 'remote', 'github', 'gitlab', 'git', 'local', 'installed', 'plugin'`); lines.push(` Current value: ${skill.type || '(not set)'}`); } else if (skill.type === 'inline') { lines.push(` Type is 'inline' but 'def.content' is missing`); @@ -49,6 +50,8 @@ function buildInvalidSkillMessage(skill: Skill): string { if (skill.def.repo) lines.push(` Current value: '${skill.def.repo}'`); } else if (skill.type === 'remote') { lines.push(` Type is 'remote' but 'def.url' is missing`); + } else if (skill.type === 'git') { + lines.push(` Type is 'git' but 'def.url' (the clone URL) is missing`); } return lines.join('\n'); } @@ -228,6 +231,66 @@ export async function installOneSkill( } catch (error: any) { throw error instanceof Error ? error : new Error(String(error)); } + } else if (skill.type === 'git' && skill.def.url) { + // Any git host: `def.url` is the clone URL, `def.path` the directory + // holding SKILL.md (the repo root when unset). Credentials come from the + // user's git credential helper. + const url = skill.def.url; + let repoPath: string; + try { + repoPath = gitRepoKey(url); + } catch { + throw new Error(`Invalid git clone URL for skill "${skill.id}": ${url}`); + } + const skillPath = (skill.def.path ?? '').replace(/^\/+|\/+$/g, ''); + const version = skill.def.version; + const ref = skill.def.ref; + + const repoKey = `git:${url}${version ? ':' + version : ''}${ref ? '#' + ref : ''}`; + let snapshot = resolvedRepos.get(repoKey); + const lockHit = noCache ? null : lockBuilder.findSkill(skill.id, version ?? null, ref ?? null); + // A pin from another source (the URL changed under the same id) must not be reused. + const previousLock = lockHit?.source === 'git' && lockHit.url === url ? lockHit : null; + if (!snapshot) { + snapshot = await repoSnapshot.getRepoSnapshot('git', repoPath, authFetch, { + version, + ref, + pinnedSha: previousLock?.resolvedRef, + noCache, + repoUrl: url, + }); + resolvedRepos.set(repoKey, snapshot); + } + + lockBuilder.upsertSkill({ + id: skill.id, + source: 'git', + repo: repoPath, + url, + skillName: skillPath, + requestedVersion: version ?? null, + requestedRef: ref ?? null, + resolvedRef: snapshot.resolvedSha, + resolvedVersion: preserveResolvedVersion(snapshot, previousLock), + }); + + const skillDir = skillPath ? assertSafeRepoPath(snapshot.snapshotDir, skillPath) : snapshot.snapshotDir; + const skillMdPath = join(skillDir, 'SKILL.md'); + if (!existsSync(skillMdPath)) { + const available = Array.from(findSkillsInDirectory(snapshot.snapshotDir).values()) + .map((p) => relative(snapshot!.snapshotDir, dirname(p)).replace(/\\/g, '/') || '.') + .sort(); + throw new Error( + `SKILL.md not found at ${skillPath ? `"${skillPath}/SKILL.md"` : 'the repository root'}.\n` + + ` Repository: ${url}\n` + + ` Skills in this repository: ${available.join(', ') || 'none'}\n` + + ` Tip: set def.path to the skill's directory, or add it with "${url}::".` + ); + } + const skillData = readSkillFromDirectory(skillMdPath); + skillSourceDir = skillDir; + skillMarkdown = skillData.markdown; + additionalFiles = skillData.additionalFiles; } else if (skill.type === 'remote' && skill.def.url) { try { const response = await authFetch.fetch(skill.def.url); @@ -247,6 +310,14 @@ export async function installOneSkill( throw new Error(`Failed to fetch: ${response.statusText}`); } skillMarkdown = await response.text(); + // A web page is never a SKILL.md: usually a repository page or a clone URL + // missing its .git suffix. Installing it would hand the agent HTML. + if (/^s*<(!doctype|html)[s>]/i.test(skillMarkdown)) { + throw new Error( + `${skill.def.url} returned a web page, not a SKILL.md. ` + + `For a git repository, use its clone URL ending in .git (type: git).` + ); + } } catch (error: any) { throw new Error(`Failed to fetch skill ${skill.id}: ${error.message || error}`); } diff --git a/src/cli/commands/install-tasks/helpers/repo-snapshot.ts b/src/cli/commands/install-tasks/helpers/repo-snapshot.ts index d7c193ab..b362db11 100644 --- a/src/cli/commands/install-tasks/helpers/repo-snapshot.ts +++ b/src/cli/commands/install-tasks/helpers/repo-snapshot.ts @@ -5,13 +5,16 @@ import { explainGitError } from './git'; // Cache-aware replacement for the legacy `cloneRepository` helper. Returns a // stable on-disk snapshot of the repo at the resolved commit SHA. The // snapshot directory is owned by the cache and must NOT be deleted by callers. +// `repoUrl` is required for the `git` platform (any host) and overrides the +// github.com / gitlab.com URL otherwise. export async function getRepoSnapshot( platform: CachePlatform, repoPath: string, authFetch: AuthenticatedFetch, - opts: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean } = {} + opts: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean; repoUrl?: string } = {} ): Promise { - const hasAuth = authFetch.hasAuth(`https://${platform}.com/${repoPath}`); + const repoUrl = opts.repoUrl ?? `https://${platform}.com/${repoPath}`; + const hasAuth = authFetch.hasAuth(repoUrl); try { return await getOrCreateSnapshot({ platform, @@ -21,8 +24,9 @@ export async function getRepoSnapshot( ref: opts.ref, pinnedSha: opts.pinnedSha, noCache: opts.noCache, + repoUrl: opts.repoUrl, }); } catch (error: any) { - throw explainGitError(error, platform, repoPath, hasAuth); + throw explainGitError(error, platform, repoPath, hasAuth, opts.repoUrl); } } diff --git a/src/cli/commands/install-tasks/install-skills.ts b/src/cli/commands/install-tasks/install-skills.ts index 3268ed30..95dbab75 100644 --- a/src/cli/commands/install-tasks/install-skills.ts +++ b/src/cli/commands/install-tasks/install-skills.ts @@ -13,7 +13,7 @@ export function installSkillsTask(): Task { const providers = materialInstallProviders(ctx); const skills = ctx.capabilitiesToUse.skills ?? []; const needsGit = skills.some( - (skill) => skill.type === 'github' || skill.type === 'gitlab', + (skill) => skill.type === 'github' || skill.type === 'gitlab' || skill.type === 'git', ); if (needsGit) { const gitInstalled = await checkGitInstalled(); diff --git a/src/cli/commands/install-tasks/write-lockfile.ts b/src/cli/commands/install-tasks/write-lockfile.ts index 6a06bf81..20640f40 100644 --- a/src/cli/commands/install-tasks/write-lockfile.ts +++ b/src/cli/commands/install-tasks/write-lockfile.ts @@ -15,7 +15,7 @@ export function writeLockfileTask(): Task { task: async (ctx) => { const skillIdsForLock = new Set( ctx.capabilities.skills - .filter((s) => s.type === 'github' || s.type === 'gitlab') + .filter((s) => s.type === 'github' || s.type === 'gitlab' || s.type === 'git') .map((s) => s.id), ); const pluginIdsForLock = new Set( diff --git a/src/cli/commands/plugin-install.ts b/src/cli/commands/plugin-install.ts index e1168260..59aea852 100644 --- a/src/cli/commands/plugin-install.ts +++ b/src/cli/commands/plugin-install.ts @@ -16,7 +16,7 @@ import type { Rule } from '../../types/rules'; import type { Hook } from '../../types/hooks'; import type { CapaDatabase } from '../../db/database'; import type { AuthenticatedFetch } from '../../shared/authenticated-fetch'; -import { validatePluginDef, getPluginInstallId } from '../../shared/plugin-source'; +import { validatePluginDef, getPluginInstallId, pluginSourceOf } from '../../shared/plugin-source'; import { detectAndParseManifest, discoverPluginEntries, @@ -211,7 +211,7 @@ export type GetRepoSnapshotFn = ( platform: CachePlatform, repoPath: string, authFetch: AuthenticatedFetch, - opts?: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean } + opts?: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean; repoUrl?: string } ) => Promise; /** @@ -358,10 +358,11 @@ export async function resolvePlugins( let allowedCharacters: string | null = null; for (const pluginRef of plugins) { - if (!getGitProvider(pluginRef.type)) continue; - if (!pluginRef.def?.repo) continue; + if (pluginRef.type !== 'git' && !getGitProvider(pluginRef.type)) continue; + const source = pluginSourceOf(pluginRef.def); + if (!source) continue; - const pluginLabel = pluginRef.id ?? pluginRef.def.repo; + const pluginLabel = pluginRef.id ?? source; // Isolate per-plugin failures so one bad entry (marketplace repo root, // missing manifest, clone error, …) cannot wipe expansions from the rest. let pluginInstallId: string | undefined; @@ -385,7 +386,7 @@ export async function resolvePlugins( ); } - const { platform, repoPath, subpath, search, version, ref } = validated; + const { platform, repoPath, subpath, search, version, ref, repoUrl } = validated; let snapshot: GetSnapshotResult; let previousLock: LockPluginEntry | null = null; @@ -420,10 +421,11 @@ export async function resolvePlugins( ref, pinnedSha, noCache, + repoUrl, }); } catch (err: any) { throw new Error( - `Failed to clone plugin ${repoPath}: ${err.message}` + `Failed to clone plugin ${repoUrl ?? repoPath}: ${err.message}` ); } @@ -485,7 +487,9 @@ export async function resolvePlugins( throw new Error( `No plugin manifest found in ${repoPath}${subpath ? `/${subpath}` : ''}.\n` + ` Expected one of: .claude-plugin/plugin.json, .cursor-plugin/plugin.json.` + - (pluginRef.id + (repoUrl + ? `\n Tip: if the plugin is not at the repository root, set def.subpath to its directory.` + : pluginRef.id ? `\n Tip: for monorepos, pin the plugin with "${repoPath}@${pluginRef.id}" or "${repoPath}::${pluginRef.id}".` : '') ); @@ -516,6 +520,7 @@ export async function resolvePlugins( id: pluginInstallId, source: platform, repo: repoPath, + ...(repoUrl && { url: repoUrl }), subpath: resolvedSubpath || null, requestedSearchName: search ?? null, requestedVersion: version ?? null, @@ -530,7 +535,10 @@ export async function resolvePlugins( const refish = ref ?? version ?? 'HEAD'; const gp = getGitProvider(platform); const host = gp?.host ?? `${platform}.com`; - const repository = resolvedSubpath + // Any other git host has no known web URL scheme, so link the clone URL. + const repository = repoUrl + ? repoUrl + : resolvedSubpath ? `https://${host}/${repoPath}/tree/${refish}/${resolvedSubpath}` : `https://${host}/${repoPath}`; const sourcePlugin: SourcePlugin = { diff --git a/src/server/resolve-effective-capabilities.ts b/src/server/resolve-effective-capabilities.ts index 7fdfe5db..64cb1cc1 100644 --- a/src/server/resolve-effective-capabilities.ts +++ b/src/server/resolve-effective-capabilities.ts @@ -29,7 +29,7 @@ function pluginsCacheKey( return plugins .map( (p) => - `${p.id ?? ""}|${p.type}|${p.def?.repo ?? ""}|${p.def?.version ?? ""}|${p.def?.ref ?? ""}`, + `${p.id ?? ""}|${p.type}|${p.def?.repo ?? p.def?.url ?? ""}|${p.def?.version ?? ""}|${p.def?.ref ?? ""}`, ) .join("||"); } diff --git a/src/shared/__tests__/git-url.test.ts b/src/shared/__tests__/git-url.test.ts new file mode 100644 index 00000000..47d27dc8 --- /dev/null +++ b/src/shared/__tests__/git-url.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect } from 'bun:test'; +import { gitRepoKey, isGitUrl, parseGitUrlSource } from '../git-url'; +import { parseSkillSource } from '../../cli/commands/add-parse-skill'; +import { parsePluginSource } from '../../cli/commands/add-parse-plugin'; +import { validatePluginDef } from '../plugin-source'; + +describe('parseGitUrlSource', () => { + it('takes any host, including @ in the path and a port', () => { + expect(parseGitUrlSource('https://git.example.com:8443/@acme/code-review.git')).toEqual({ + url: 'https://git.example.com:8443/@acme/code-review.git', + idHint: 'code-review', + }); + }); + + it('parses ::path, :version and #sha suffixes', () => { + expect(parseGitUrlSource('https://h.test/team/skills.git::skills/lint:v1.2.0')).toEqual({ + url: 'https://h.test/team/skills.git', + path: 'skills/lint', + version: 'v1.2.0', + idHint: 'lint', + }); + expect(parseGitUrlSource('https://h.test/team/skills.git#abc1234')).toMatchObject({ + url: 'https://h.test/team/skills.git', + ref: 'abc1234', + }); + }); + + it('rejects URLs that are not clone URLs', () => { + expect(isGitUrl('https://example.com/SKILL.md')).toBe(false); + expect(isGitUrl('https://example.com/a.github.io/page')).toBe(false); + expect(isGitUrl('owner/repo')).toBe(false); + }); +}); + +describe('gitRepoKey', () => { + it('is host plus path without .git, safe for Windows directory names', () => { + expect(gitRepoKey('https://git.example.com/@acme/code-review.git')).toBe('git.example.com/@acme/code-review'); + expect(gitRepoKey('http://localhost:8080/a/b.git')).toBe('localhost_8080/a/b'); + expect(gitRepoKey('https://h.test/a/../../etc.git')).not.toContain('..'); + }); +}); + +describe('capa add with a git clone URL', () => { + it('makes a git skill, not a raw remote file', async () => { + expect(await parseSkillSource('https://h.test/@acme/code-review.git:v1.0.0')).toEqual({ + id: 'code-review', + type: 'git', + def: { url: 'https://h.test/@acme/code-review.git', version: 'v1.0.0' }, + }); + }); + + it('keeps raw SKILL.md URLs as remote', async () => { + expect((await parseSkillSource('https://h.test/skills/SKILL.md')).type).toBe('remote'); + }); + + it('makes a git plugin that validates', () => { + const parsed = parsePluginSource('https://h.test/@acme/tools.git::plugins/deploy'); + expect(parsed).toEqual({ + type: 'git', + def: { url: 'https://h.test/@acme/tools.git', subpath: 'plugins/deploy' }, + idHint: 'deploy', + }); + expect(validatePluginDef({ type: 'git', def: parsed.def })).toMatchObject({ + platform: 'git', + repoPath: 'h.test/@acme/tools', + subpath: 'plugins/deploy', + repoUrl: 'https://h.test/@acme/tools.git', + }); + }); + + it('still parses GitHub URLs as github', () => { + expect(parsePluginSource('https://github.com/owner/repo.git').type).toBe('github'); + }); +}); diff --git a/src/shared/cache/mirror.ts b/src/shared/cache/mirror.ts index 75434cb1..71348de3 100644 --- a/src/shared/cache/mirror.ts +++ b/src/shared/cache/mirror.ts @@ -22,6 +22,9 @@ function publicHttpsRepoUrl( repoPath: string, ): string { validateRepoPath(repoPath); + if (platform === "git") { + throw new Error(`A clone URL is required for git repository ${repoPath}`); + } return `https://${platform}.com/${repoPath}.git`; } diff --git a/src/shared/cache/paths.ts b/src/shared/cache/paths.ts index f136834e..8a2288dc 100644 --- a/src/shared/cache/paths.ts +++ b/src/shared/cache/paths.ts @@ -2,7 +2,8 @@ import { homedir } from "os"; import { join } from "path"; import { validateRepoPath } from "./validate"; -export type CachePlatform = "github" | "gitlab"; +/** `git` is any other host: its repoPath is `gitRepoKey(url)` and callers pass the clone URL. */ +export type CachePlatform = "github" | "gitlab" | "git"; /** Root cache directory. Override via CAPA_CACHE_DIR for tests. */ export function getCacheDir(): string { diff --git a/src/shared/capabilities.ts b/src/shared/capabilities.ts index ade9d7d4..4b373e1d 100644 --- a/src/shared/capabilities.ts +++ b/src/shared/capabilities.ts @@ -62,6 +62,7 @@ const skillSchema = z "remote", "github", "gitlab", + "git", "local", "installed", "plugin", @@ -216,7 +217,9 @@ const toolSchema = z.discriminatedUnion("type", [ const pluginDefSchema = z .object({ - repo: z.string(), + // github/gitlab use `repo`; git uses `url` (validatePluginDef checks which). + repo: z.string().optional(), + url: z.string().optional(), subpath: z.string().optional(), version: z.string().optional(), ref: z.string().optional(), @@ -227,7 +230,7 @@ const pluginDefSchema = z const pluginSchema = z .object({ id: z.string().optional(), - type: z.enum(["github", "gitlab"]), + type: z.enum(["github", "gitlab", "git"]), def: pluginDefSchema, servers: z .record( diff --git a/src/shared/executable-surface.ts b/src/shared/executable-surface.ts index f9802644..9f4d894f 100644 --- a/src/shared/executable-surface.ts +++ b/src/shared/executable-surface.ts @@ -137,11 +137,13 @@ export function collectExecutableSurface(capabilities: Capabilities): Executable const plugins: SurfacePlugin[] = []; for (const plugin of (capabilities.plugins ?? []) as Plugin[]) { - const id = plugin.id ?? plugin.def.repo; + // git plugins (any host) have a clone URL instead of an owner/repo. + const repo = plugin.def.repo ?? plugin.def.url ?? ''; + const id = plugin.id ?? repo; const entry: SurfacePlugin = { id, type: plugin.type, - repo: plugin.def.repo, + repo, }; if (plugin.def.version) entry.version = plugin.def.version; if (plugin.def.ref) entry.ref = plugin.def.ref; diff --git a/src/shared/git-url.ts b/src/shared/git-url.ts new file mode 100644 index 00000000..7f88d791 --- /dev/null +++ b/src/shared/git-url.ts @@ -0,0 +1,66 @@ +/** + * Generic git sources: any repository reachable by a clone URL, not only + * github.com or gitlab.com. `capa add` recognizes them by the `.git` suffix. + * + * https://git.example.com/team/skills.git + * https://git.example.com/team/skills.git::path/to/skill exact subpath + * https://git.example.com/team/skills.git:v1.2.0 tag or branch + * https://git.example.com/team/skills.git# commit + */ + +import { createHash } from "crypto"; + +const GIT_URL_RE = + /^((?:https?|file):\/\/.+?\.git)\/?(?:::([^:#]+))?(?::([\w.+/-]+))?(?:#([a-f0-9]{7,40}))?$/i; + +export interface GitUrlSource { + /** Clone URL, without the `::path` / `:version` / `#sha` suffixes. */ + url: string; + path?: string; + version?: string; + ref?: string; + /** Last segment of the path, or of the URL without `.git`. */ + idHint: string; +} + +/** Parse a `.git` clone URL with optional suffixes. null when it is not one. */ +export function parseGitUrlSource(source: string): GitUrlSource | null { + const m = source.trim().match(GIT_URL_RE); + if (!m) return null; + const [, url, rawPath, version, ref] = m; + const path = rawPath?.replace(/^\/+|\/+$/g, "") || undefined; + const last = (s: string) => s.split("/").filter(Boolean).pop() ?? ""; + const idHint = path ? last(path) : last(url.replace(/\.git$/i, "")); + return { + url, + ...(path && { path }), + ...(version && { version }), + ...(ref && { ref }), + idHint, + }; +} + +/** True for anything `parseGitUrlSource` accepts. */ +export function isGitUrl(source: string): boolean { + return parseGitUrlSource(source) !== null; +} + +/** + * Cache key for a clone URL: `/` without the `.git` suffix, e.g. + * `git.example.com/team/skills`. Characters Windows forbids in directory names + * (a port's `:`) become `_`. file:// URLs become `file/-`, so an + * absolute local path doesn't blow past Windows path limits in the cache. + */ +export function gitRepoKey(url: string): string { + const u = new URL(url); + const path = decodeURIComponent(u.pathname).replace(/\.git\/?$/i, ""); + if (u.protocol === "file:") { + const name = path.split("/").filter(Boolean).pop() ?? "repo"; + const hash = createHash("sha256").update(u.href).digest("hex").slice(0, 12); + return `file/${name.replace(/[<>:"|?*\\]/g, "_")}-${hash}`; + } + return [u.host, ...path.split("/")] + .filter((s) => s && s !== "." && s !== "..") + .map((s) => s.replace(/[<>:"|?*\\]/g, "_")) + .join("/"); +} diff --git a/src/shared/lockfile.ts b/src/shared/lockfile.ts index 6b516752..a955713e 100644 --- a/src/shared/lockfile.ts +++ b/src/shared/lockfile.ts @@ -55,7 +55,7 @@ export function emptyLockfile(): Lockfile { } function isLockSource(value: unknown): value is LockSource { - return value === "github" || value === "gitlab"; + return value === "github" || value === "gitlab" || value === "git"; } function isLockHookSource(value: unknown): value is LockHookEntry["source"] { @@ -74,6 +74,7 @@ export function isValidSkillLockEntry(x: unknown): x is LockSkillEntry { e.id.length > 0 && isLockSource(e.source) && typeof e.repo === "string" && + (e.source !== "git" || typeof e.url === "string") && typeof e.skillName === "string" && isNullableString(e.requestedVersion) && isNullableString(e.requestedRef) && @@ -107,6 +108,7 @@ export function isValidPluginLockEntry(x: unknown): x is LockPluginEntry { e.id.length > 0 && isLockSource(e.source) && typeof e.repo === "string" && + (e.source !== "git" || typeof e.url === "string") && isNullableString(e.subpath) && isNullableString(e.requestedSearchName) && isNullableString(e.requestedVersion) && diff --git a/src/shared/plugin-source.ts b/src/shared/plugin-source.ts index ac05ce05..40e003ba 100644 --- a/src/shared/plugin-source.ts +++ b/src/shared/plugin-source.ts @@ -7,6 +7,7 @@ import type { Plugin } from "../types/plugin"; import type { CachePlatform } from "./cache"; +import { gitRepoKey } from "./git-url"; const SEGMENT_RE = /^[^/:#]+$/; const FORBIDDEN_SEGMENTS = new Set([".", ".."]); @@ -38,6 +39,8 @@ export interface ValidatedPluginDef { search?: string; version?: string; ref?: string; + /** Clone URL. Set only for the `git` platform (any host). */ + repoUrl?: string; } export interface PluginDefError { @@ -86,6 +89,9 @@ export function validatePluginDef( plugin: Plugin, ): ValidatedPluginDef | PluginDefError { const { type, def } = plugin; + if (type === "git") { + return validateGitPluginDef(def); + } if (type !== "github" && type !== "gitlab") { return { error: `Unsupported plugin type: ${type}` }; } @@ -166,6 +172,44 @@ export function validatePluginDef( }; } +/** + * A `git` plugin: `def.url` is the clone URL of any host, `def.subpath` the + * plugin directory (repo root when unset). The `@name` search and `::` suffix + * are github/gitlab grammar, so a git URL is taken as-is. + */ +function validateGitPluginDef( + def: Plugin["def"], +): ValidatedPluginDef | PluginDefError { + if (!def?.url) { + return { error: "Missing required field: def.url (the git clone URL)" }; + } + let repoPath: string; + try { + repoPath = gitRepoKey(def.url); + } catch { + return { error: `Invalid git clone URL: "${def.url}"` }; + } + const subpath = def.subpath ?? ""; + if (subpath !== "" && !isValidPath(subpath, 1)) { + return { + error: `Invalid subpath: "${subpath}". Must be non-empty segments without "." or "..".`, + }; + } + return { + platform: "git", + repoPath, + subpath, + version: def.version, + ref: def.ref, + repoUrl: def.url, + }; +} + +/** The repo string (github/gitlab) or clone URL (git) a plugin entry points at. */ +export function pluginSourceOf(def: Plugin["def"] | undefined): string | undefined { + return def?.repo ?? def?.url; +} + /** * Generate a stable slugified id from a plugin id or manifest name. * No ref/version suffix — server ids and skill bindings stay stable across version bumps. diff --git a/src/types/capabilities.ts b/src/types/capabilities.ts index 53314c27..6124592a 100644 --- a/src/types/capabilities.ts +++ b/src/types/capabilities.ts @@ -322,7 +322,7 @@ export interface Capabilities { export interface Skill { id: string; - type: 'inline' | 'remote' | 'github' | 'gitlab' | 'local' | 'installed' | 'plugin'; + type: 'inline' | 'remote' | 'github' | 'gitlab' | 'git' | 'local' | 'installed' | 'plugin'; def: SkillDefinition; sourcePlugin?: SourcePlugin; } @@ -336,6 +336,7 @@ export interface SkillDefinition { // Capa validates that the skill id matches a skill exposed by some plugin's manifest // and warns if no match is found. // For remote skills (raw SKILL.md URL) + // For git skills: the clone URL of any git host (e.g. "https://git.example.com/team/skills.git") url?: string; // For GitHub skills (e.g., "vercel-labs/agent-skills@find-skills") // For GitLab skills (e.g., "group/project@skill-name") @@ -344,6 +345,7 @@ export interface SkillDefinition { // For inline skills (SKILL.md content as string) content?: string; // For local skills: path to directory containing SKILL.md (relative to project root or absolute) + // For git skills: directory inside the repo holding SKILL.md (default: the repo root) path?: string; // Version or tag to checkout (e.g., "1.2.1" or "v1.2.1") version?: string; diff --git a/src/types/lockfile.ts b/src/types/lockfile.ts index 9cc97a57..bc9180d8 100644 --- a/src/types/lockfile.ts +++ b/src/types/lockfile.ts @@ -10,18 +10,21 @@ export type LockfileFormat = 'json' | 'yaml'; -export type LockSource = 'github' | 'gitlab'; +/** `git` is any other host, identified by `url`. */ +export type LockSource = 'github' | 'gitlab' | 'git'; /** - * Locked entry for a `github`/`gitlab` skill. + * Locked entry for a `github`/`gitlab`/`git` skill. */ export interface LockSkillEntry { /** Skill id from capabilities file. Matches `Skill.id`. */ id: string; /** Source type. */ source: LockSource; - /** "owner/repo" path. */ + /** "owner/repo" path; for `git`, the host/path cache key. */ repo: string; + /** Clone URL. Only for `git`. */ + url?: string; /** Skill directory name within the repo (the "@skill-name" part of `def.repo`). */ skillName: string; /** Version requested in capabilities (tag/branch). null if not specified. */ @@ -42,8 +45,10 @@ export interface LockPluginEntry { id: string; /** Source type. */ source: LockSource; - /** Multi-segment repo path (GitLab nested groups allowed). */ + /** Multi-segment repo path (GitLab nested groups allowed); for `git`, the host/path cache key. */ repo: string; + /** Clone URL. Only for `git`. */ + url?: string; /** * Path inside the repo where the plugin manifest lives — either pinned via * `def.subpath` or resolved from a `def.search` walk. null when at root. diff --git a/src/types/plugin.ts b/src/types/plugin.ts index b74690b9..b1ddc239 100644 --- a/src/types/plugin.ts +++ b/src/types/plugin.ts @@ -40,7 +40,7 @@ export interface PluginServerConfig { export interface Plugin { /** Stable identifier. Defaults to last segment of subpath or repo. */ id?: string; - type: 'github' | 'gitlab'; + type: 'github' | 'gitlab' | 'git'; def: PluginDefinition; /** Per-server aliasing and tool subset, keyed by the manifest's mcpServers key. */ servers?: Record; @@ -57,8 +57,11 @@ export interface PluginDefinition { * - `owner/repo::sub/path` — exact subpath inside the repo * * GitLab nested groups are supported (`group/subgroup/project[...]`). + * Required for github/gitlab; `git` plugins use `url` instead. */ - repo: string; + repo?: string; + /** Clone URL for `git` plugins (any host). The plugin location goes in `subpath`. */ + url?: string; /** * Optional exact subpath inside the repo when you prefer to keep it out of * the `repo` string. Equivalent to writing `owner/repo::` — capa From c85502a39b9cd5da7aaf9cd06c725b32d5878bc3 Mon Sep 17 00:00:00 2001 From: Antonio Zaitoun Date: Sat, 10 Oct 2026 22:51:13 +0300 Subject: [PATCH 2/5] fix: address review findings for generic git sources - Detect HTML responses with a real check (the regex matched a literal s) so a web page is never installed as SKILL.md. - Key the generic git cache by host/path plus a hash of the whole URL, and refuse a cached mirror whose origin is a different URL. - Drop a previous plugin lock pin when its clone URL changed. - Validate that git plugins have def.url and other plugins def.repo, and let entries without a source reach validatePluginDef. - Count generic git repositories in cache stats and clean. - Serve uninstalled git skill content in the web UI. - Record a git skill's lock pin only after it is installed. - Report missing tags and commits as such, not as a missing repository. - Guard the web UI plugin list and preview against plugins without repo. --- .../__tests__/install-one-skill-git.test.ts | 32 ++++++++++++-- src/cli/commands/install-tasks/helpers/git.ts | 7 +++- .../helpers/install-one-skill.ts | 15 +++++-- src/cli/commands/plugin-install.ts | 8 ++-- src/server/skill-content.ts | 24 +++++++++++ src/shared/__tests__/git-url.test.ts | 38 +++++++++++++++-- src/shared/cache/mirror.ts | 18 ++++++++ src/shared/cache/stats.ts | 42 +++++++++---------- src/shared/capabilities.ts | 12 +++++- src/shared/git-url.ts | 31 ++++++++------ .../components/PluginPreviewDialog.tsx | 16 +++---- .../projects/components/PluginsEditor.tsx | 12 +++--- web-ui/src/types/api.ts | 10 ++++- 13 files changed, 201 insertions(+), 64 deletions(-) diff --git a/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts index 715294d0..0d19a541 100644 --- a/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts +++ b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts @@ -6,7 +6,8 @@ import { execFileSync } from 'child_process'; import { pathToFileURL } from 'url'; import { CapaDatabase } from '../../../../../db/database'; import { LockfileBuilder } from '../../../../../shared/lockfile'; -import { installOneSkill } from '../install-one-skill'; +import { installOneSkill, looksLikeHtml } from '../install-one-skill'; +import { getCacheStats } from '../../../../../shared/cache'; import type { Capabilities, Skill } from '../../../../../types/capabilities'; // A skill on "any git host": a real repository cloned over file://, so the @@ -92,9 +93,34 @@ describe('installOneSkill with a git clone URL', () => { expect(readFileSync(join(projectPath, '.cursor', 'skills', 'lint', 'SKILL.md'), 'utf8')).toContain('name: lint'); }); - it('names the skills it found when SKILL.md is missing', async () => { + it('names the skills it found when SKILL.md is missing, and leaves no lock pin', async () => { + const lockBuilder = new LockfileBuilder(); await expect( - install({ id: 'x', type: 'git', def: { url: repoUrl, path: 'nested' } }), + install({ id: 'x', type: 'git', def: { url: repoUrl, path: 'nested' } }, lockBuilder), ).rejects.toThrow(/nested\/lint/); + expect(lockBuilder.build().skills).toEqual([]); + }); + + it('reports a missing tag as a missing tag, not a missing repository', async () => { + await expect( + install({ id: 'code-review', type: 'git', def: { url: repoUrl, version: 'v9.9.9' } }), + ).rejects.toThrow(/Tag\/branch "v9\.9\.9" not found/); + }); + + it('shows git repositories in cache stats', async () => { + await install({ id: 'code-review', type: 'git', def: { url: repoUrl } }); + const repos = getCacheStats().repos.filter((r) => r.platform === 'git'); + expect(repos).toHaveLength(1); + expect(repos[0].repoPath).toMatch(/^file\/code-review-[0-9a-f]{12}$/); + expect(repos[0].snapshotCount).toBe(1); + }); +}); + +describe('looksLikeHtml', () => { + it('catches ordinary web pages, with or without leading whitespace', () => { + expect(looksLikeHtml('\n')).toBe(true); + expect(looksLikeHtml(' \n')).toBe(true); + expect(looksLikeHtml('---\nname: x\n---\n in a code sample')).toBe(false); + expect(looksLikeHtml('# Skill\n')).toBe(false); }); }); diff --git a/src/cli/commands/install-tasks/helpers/git.ts b/src/cli/commands/install-tasks/helpers/git.ts index dab252d7..31389b98 100644 --- a/src/cli/commands/install-tasks/helpers/git.ts +++ b/src/cli/commands/install-tasks/helpers/git.ts @@ -57,7 +57,12 @@ function explainGenericGitError(error: any, errorMessage: string, repoUrl: strin `remembers the credentials, then re-run \`capa install\`.` ); } - if (errorMessage.includes('not found') || errorMessage.includes('does not appear to be a git repository')) { + // The cache's own ref errors ("Tag/branch "v2" not found", "Commit … not + // found", "Pinned commit …") already name the cause; keep them. + if (/^(Tag\/branch |Commit |Pinned commit )/.test(error?.message ?? '')) { + return error instanceof Error ? error : new Error(errorMessage); + } + if (/repository '.*' not found/.test(errorMessage) || errorMessage.includes('does not appear to be a git repository')) { return new Error(`Repository not found: ${repoUrl}\nCheck the URL, or sign in with git if it is private.`); } if (errorMessage.includes('unable to access') || errorMessage.includes('Could not resolve host')) { diff --git a/src/cli/commands/install-tasks/helpers/install-one-skill.ts b/src/cli/commands/install-tasks/helpers/install-one-skill.ts index d48a2314..68bf65f4 100644 --- a/src/cli/commands/install-tasks/helpers/install-one-skill.ts +++ b/src/cli/commands/install-tasks/helpers/install-one-skill.ts @@ -33,6 +33,12 @@ import * as repoSnapshot from './repo-snapshot'; import { findSkillsInDirectory, readSkillFromDirectory } from './skill-discovery'; import type { SkillInstallOutcome } from '../context'; +/** An HTML document (e.g. a repository web page), which is never a SKILL.md. */ +export function looksLikeHtml(text: string): boolean { + const head = text.trimStart().slice(0, 200).toLowerCase(); + return head.startsWith(' = new Map(); let skillSourceDir: string | null = null; + let pendingLock: LockSkillEntry | null = null; if (skill.type === 'installed') { return 'skipped'; @@ -262,7 +269,8 @@ export async function installOneSkill( resolvedRepos.set(repoKey, snapshot); } - lockBuilder.upsertSkill({ + // Recorded once the skill is on disk, so a failed install leaves no pin behind. + pendingLock = { id: skill.id, source: 'git', repo: repoPath, @@ -272,7 +280,7 @@ export async function installOneSkill( requestedRef: ref ?? null, resolvedRef: snapshot.resolvedSha, resolvedVersion: preserveResolvedVersion(snapshot, previousLock), - }); + }; const skillDir = skillPath ? assertSafeRepoPath(snapshot.snapshotDir, skillPath) : snapshot.snapshotDir; const skillMdPath = join(skillDir, 'SKILL.md'); @@ -312,7 +320,7 @@ export async function installOneSkill( skillMarkdown = await response.text(); // A web page is never a SKILL.md: usually a repository page or a clone URL // missing its .git suffix. Installing it would hand the agent HTML. - if (/^s*<(!doctype|html)[s>]/i.test(skillMarkdown)) { + if (looksLikeHtml(skillMarkdown)) { throw new Error( `${skill.def.url} returned a web page, not a SKILL.md. ` + `For a git repository, use its clone URL ending in .git (type: git).` @@ -431,5 +439,6 @@ export async function installOneSkill( } } + if (pendingLock) lockBuilder.upsertSkill(pendingLock); return 'installed'; } diff --git a/src/cli/commands/plugin-install.ts b/src/cli/commands/plugin-install.ts index 59aea852..e22dbfd3 100644 --- a/src/cli/commands/plugin-install.ts +++ b/src/cli/commands/plugin-install.ts @@ -359,10 +359,8 @@ export async function resolvePlugins( for (const pluginRef of plugins) { if (pluginRef.type !== 'git' && !getGitProvider(pluginRef.type)) continue; - const source = pluginSourceOf(pluginRef.def); - if (!source) continue; - - const pluginLabel = pluginRef.id ?? source; + // An entry without a source still goes on to validatePluginDef, which reports it. + const pluginLabel = pluginRef.id ?? pluginSourceOf(pluginRef.def) ?? `(${pluginRef.type} plugin)`; // Isolate per-plugin failures so one bad entry (marketplace repo root, // missing manifest, clone error, …) cannot wipe expansions from the rest. let pluginInstallId: string | undefined; @@ -415,6 +413,8 @@ export async function resolvePlugins( }, ); } + // A git pin only holds for the clone URL it was resolved from. + if (repoUrl && previousLock && previousLock.url !== repoUrl) previousLock = null; const pinnedSha = previousLock?.resolvedRef; snapshot = await getRepoSnapshot(platform, repoPath, authFetch, { version, diff --git a/src/server/skill-content.ts b/src/server/skill-content.ts index 1233f16c..1dfe3941 100644 --- a/src/server/skill-content.ts +++ b/src/server/skill-content.ts @@ -10,6 +10,7 @@ import { } from "../cli/commands/install-tasks/helpers/skill-discovery"; import type { AuthenticatedFetch } from "../shared/authenticated-fetch"; import { type CachePlatform, getOrCreateSnapshot } from "../shared/cache"; +import { gitRepoKey } from "../shared/git-url"; import { getProjectPluginsDir } from "../shared/plugin-paths"; import { getProvider } from "../shared/providers"; import { assertSafeRepoPath } from "../shared/repo-file"; @@ -415,6 +416,29 @@ export async function fetchUninstalledSkillContent( ]; return tryParse(skillData.markdown, files); } + + if (skill.type === "git" && skill.def?.url) { + const snapshot = await getOrCreateSnapshot({ + platform: "git", + repoPath: gitRepoKey(skill.def.url), + repoUrl: skill.def.url, + authFetch, + version: skill.def.version, + ref: skill.def.ref, + }); + const skillPath = (skill.def.path ?? "").replace(/^\/+|\/+$/g, ""); + const skillDir = skillPath + ? assertSafeRepoPath(snapshot.snapshotDir, skillPath) + : snapshot.snapshotDir; + const skillMdPath = join(skillDir, "SKILL.md"); + if (!existsSync(skillMdPath)) return null; + const skillData = readSkillFromDirectory(skillMdPath); + const files = [ + "SKILL.md", + ...Array.from(skillData.additionalFiles.keys()).sort(), + ]; + return tryParse(skillData.markdown, files); + } } catch { return null; } diff --git a/src/shared/__tests__/git-url.test.ts b/src/shared/__tests__/git-url.test.ts index 47d27dc8..9e55b9a4 100644 --- a/src/shared/__tests__/git-url.test.ts +++ b/src/shared/__tests__/git-url.test.ts @@ -33,11 +33,41 @@ describe('parseGitUrlSource', () => { }); describe('gitRepoKey', () => { - it('is host plus path without .git, safe for Windows directory names', () => { - expect(gitRepoKey('https://git.example.com/@acme/code-review.git')).toBe('git.example.com/@acme/code-review'); - expect(gitRepoKey('http://localhost:8080/a/b.git')).toBe('localhost_8080/a/b'); + it('is host plus path plus a URL hash, safe for Windows directory names', () => { + expect(gitRepoKey('https://git.example.com/@acme/code-review.git')).toMatch( + /^git\.example\.com\/@acme\/code-review-[0-9a-f]{12}$/, + ); + expect(gitRepoKey('http://localhost:8080/a/b.git')).toMatch(/^localhost_8080\/a\/b-[0-9a-f]{12}$/); expect(gitRepoKey('https://h.test/a/../../etc.git')).not.toContain('..'); }); + + it('never shares a key between different URLs', () => { + const keys = new Set( + [ + 'https://h.test/a/b.git', + 'http://h.test/a/b.git', + 'https://h.test:8443/a/b.git', + 'https://h.test/a:b.git', + 'https://h.test/a_b.git', + ].map(gitRepoKey), + ); + expect(keys.size).toBe(5); + }); + + it('ignores credentials in the URL', () => { + expect(gitRepoKey('https://user:secret@h.test/a/b.git')).toBe(gitRepoKey('https://h.test/a/b.git')); + }); +}); + +describe('plugin schema', () => { + it('requires the source field that matches the type', async () => { + const { normalizeCapabilities } = await import('../capabilities'); + expect(() => normalizeCapabilities({ plugins: [{ id: 'p', type: 'git', def: {} }] })).toThrow(/def\.url/); + expect(() => normalizeCapabilities({ plugins: [{ id: 'p', type: 'github', def: {} }] })).toThrow(/def\.repo/); + expect( + normalizeCapabilities({ plugins: [{ id: 'p', type: 'git', def: { url: 'https://h.test/p.git' } }] }).plugins, + ).toHaveLength(1); + }); }); describe('capa add with a git clone URL', () => { @@ -62,7 +92,7 @@ describe('capa add with a git clone URL', () => { }); expect(validatePluginDef({ type: 'git', def: parsed.def })).toMatchObject({ platform: 'git', - repoPath: 'h.test/@acme/tools', + repoPath: gitRepoKey('https://h.test/@acme/tools.git'), subpath: 'plugins/deploy', repoUrl: 'https://h.test/@acme/tools.git', }); diff --git a/src/shared/cache/mirror.ts b/src/shared/cache/mirror.ts index 71348de3..79a5e8e9 100644 --- a/src/shared/cache/mirror.ts +++ b/src/shared/cache/mirror.ts @@ -128,6 +128,21 @@ export function scrubCachedMirrorAuthUrls(): void { walkAndScrubMirrors(gitRoot, 0, new Set()); } +/** + * A generic git cache key is derived from the clone URL, so a mirror whose + * origin is a different URL means the cache was tampered with or keyed by an + * older capa. Refuse it rather than serve another repository's content. + */ +async function assertMirrorOrigin(mirrorDir: string, url: string): Promise { + const { stdout } = await git(["-C", mirrorDir, "remote", "get-url", "origin"]); + if (stdout.trim() !== url) { + throw new Error( + `Cached mirror at ${mirrorDir} is for ${stdout.trim()}, not ${url}. ` + + `Run \`capa cache clean\` and try again.`, + ); + } +} + /** * Ensure a mirror clone exists for the given repo. If it doesn't, perform a * bare clone. If it does, leave it as-is — callers can request a fetch via @@ -146,6 +161,9 @@ export async function ensureMirrorClone( const mirrorDir = getRepoMirrorDir(platform, repoPath); if (existsSync(mirrorDir)) { scrubMirrorConfigFile(mirrorDir); + if (platform === "git" && repoUrl) { + await assertMirrorOrigin(mirrorDir, stripHttpUrlUserinfo(repoUrl)); + } return mirrorDir; } mkdirSync(getRepoCacheDir(platform, repoPath), { recursive: true }); diff --git a/src/shared/cache/stats.ts b/src/shared/cache/stats.ts index 90de834b..b7d0157a 100644 --- a/src/shared/cache/stats.ts +++ b/src/shared/cache/stats.ts @@ -56,26 +56,15 @@ export function getCacheStats(): CacheStats { } const repos: CachedRepoInfo[] = []; - for (const platformEntry of readdirSync(gitDir, { withFileTypes: true })) { - if (!platformEntry.isDirectory()) continue; - if (platformEntry.name !== "github" && platformEntry.name !== "gitlab") - continue; - const platform = platformEntry.name as CachePlatform; - const platformDir = join(gitDir, platformEntry.name); - - for (const ownerEntry of readdirSync(platformDir, { - withFileTypes: true, - })) { - if (!ownerEntry.isDirectory()) continue; - const ownerDir = join(platformDir, ownerEntry.name); - - for (const repoEntry of readdirSync(ownerDir, { withFileTypes: true })) { - if (!repoEntry.isDirectory()) continue; - const repoPath = `${ownerEntry.name}/${repoEntry.name}`; - const repoDir = join(ownerDir, repoEntry.name); - const mirrorDir = join(repoDir, "mirror"); - const snapshotsDir = join(repoDir, "snapshots"); - + const platforms: CachePlatform[] = ["github", "gitlab", "git"]; + for (const platform of platforms) { + // A repo directory is one holding `mirror/` or `snapshots/`. Repo paths + // have any depth: GitLab nested groups, and `/` for git hosts. + const visit = (dir: string, repoPath: string, depth: number): void => { + if (depth > 12) return; + const mirrorDir = join(dir, "mirror"); + const snapshotsDir = join(dir, "snapshots"); + if (existsSync(mirrorDir) || existsSync(snapshotsDir)) { const mirrorBytes = dirSize(mirrorDir); let snapshotCount = 0; let snapshotBytes = 0; @@ -97,8 +86,19 @@ export function getCacheStats(): CacheStats { snapshotBytes, totalBytes: mirrorBytes + snapshotBytes, }); + return; } - } + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + visit( + join(dir, entry.name), + repoPath ? `${repoPath}/${entry.name}` : entry.name, + depth + 1, + ); + } + }; + const platformDir = join(gitDir, platform); + if (existsSync(platformDir)) visit(platformDir, "", 0); } repos.sort((a, b) => b.totalBytes - a.totalBytes); diff --git a/src/shared/capabilities.ts b/src/shared/capabilities.ts index 4b373e1d..2bcdd0f6 100644 --- a/src/shared/capabilities.ts +++ b/src/shared/capabilities.ts @@ -245,7 +245,17 @@ const pluginSchema = z ) .optional(), }) - .passthrough(); + .passthrough() + .superRefine((plugin, ctx) => { + const field = plugin.type === "git" ? "url" : "repo"; + if (!plugin.def[field]) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["def", field], + message: `${plugin.type} plugin${plugin.id ? ` "${plugin.id}"` : ""} requires def.${field}`, + }); + } + }); const optionsSchema = z .object({ diff --git a/src/shared/git-url.ts b/src/shared/git-url.ts index 7f88d791..92630a48 100644 --- a/src/shared/git-url.ts +++ b/src/shared/git-url.ts @@ -46,21 +46,26 @@ export function isGitUrl(source: string): boolean { } /** - * Cache key for a clone URL: `/` without the `.git` suffix, e.g. - * `git.example.com/team/skills`. Characters Windows forbids in directory names - * (a port's `:`) become `_`. file:// URLs become `file/-`, so an - * absolute local path doesn't blow past Windows path limits in the cache. + * Cache key for a clone URL: `/-`, e.g. + * `git.example.com/team/skills-3fa2c1d09b7e`. Host and path keep it readable; + * the hash of the whole URL (scheme, host, port, path; never credentials) + * keeps two different URLs from ever sharing a mirror or a lock pin, even when + * sanitizing makes their paths look alike. Characters Windows forbids in + * directory names become `_`. file:// URLs keep only the repository name, so + * an absolute local path doesn't blow past Windows path limits in the cache. */ export function gitRepoKey(url: string): string { const u = new URL(url); - const path = decodeURIComponent(u.pathname).replace(/\.git\/?$/i, ""); - if (u.protocol === "file:") { - const name = path.split("/").filter(Boolean).pop() ?? "repo"; - const hash = createHash("sha256").update(u.href).digest("hex").slice(0, 12); - return `file/${name.replace(/[<>:"|?*\\]/g, "_")}-${hash}`; - } - return [u.host, ...path.split("/")] + u.username = ""; + u.password = ""; + const hash = createHash("sha256").update(u.href).digest("hex").slice(0, 12); + const safe = (s: string) => s.replace(/[<>:"|?*\\]/g, "_"); + const segments = decodeURIComponent(u.pathname) + .replace(/\.git\/?$/i, "") + .split("/") .filter((s) => s && s !== "." && s !== "..") - .map((s) => s.replace(/[<>:"|?*\\]/g, "_")) - .join("/"); + .map(safe); + const name = `${segments.pop() ?? "repo"}-${hash}`; + if (u.protocol === "file:") return `file/${name}`; + return [safe(u.host), ...segments, name].join("/"); } diff --git a/web-ui/src/features/projects/components/PluginPreviewDialog.tsx b/web-ui/src/features/projects/components/PluginPreviewDialog.tsx index 1b1f6061..0fb39669 100644 --- a/web-ui/src/features/projects/components/PluginPreviewDialog.tsx +++ b/web-ui/src/features/projects/components/PluginPreviewDialog.tsx @@ -3,7 +3,7 @@ import { useQueries } from '@tanstack/react-query'; import * as Dialog from '@radix-ui/react-dialog'; import { X } from 'lucide-react'; import { useTranslation } from 'react-i18next'; -import type { AuthoredPlugin, ResolvedPlugin } from '../../../types/api'; +import { pluginSource, type AuthoredPlugin, type ResolvedPlugin } from '../../../types/api'; import { Spinner } from '../../../components/common/Spinner'; import { renderMarkdown } from '../../../lib/markdown'; import { useRegistries } from '../../registries/hooks'; @@ -23,9 +23,9 @@ function buildLocalPluginPreview( plugin: AuthoredPlugin, resolved: ResolvedPlugin | undefined, ): string { - const title = resolved?.name || plugin.id || plugin.def.repo; + const title = resolved?.name || plugin.id || pluginSource(plugin); const parts: string[] = [`# ${title}`, '']; - if (plugin.def.repo) parts.push(`**Repository:** \`${plugin.def.repo}\``, ''); + if (pluginSource(plugin)) parts.push(`**Repository:** \`${pluginSource(plugin)}\``, ''); if (resolved?.version || plugin.def.version) { parts.push(`**Version:** ${resolved?.version || plugin.def.version}`, ''); } @@ -53,7 +53,9 @@ function buildLocalPluginPreview( parts.push('## Install', ''); parts.push('```'); - parts.push(`capa add ${plugin.def.repo}`); + // A git plugin's subpath lives in def.subpath; `capa add` takes it as `::path`. + const gitSubpath = plugin.type === 'git' && plugin.def.subpath ? `::${plugin.def.subpath}` : ''; + parts.push(`capa add --plugin ${pluginSource(plugin)}${gitSubpath}`); parts.push('```'); parts.push(''); return parts.join('\n'); @@ -103,7 +105,7 @@ export function PluginPreviewDialog({ const matchedRegistry = matchedRegistryIndex >= 0 ? pluginRegistries[matchedRegistryIndex] : undefined; - const title = registryDetail?.title || resolved?.name || plugin?.id || plugin?.def.repo || ''; + const title = registryDetail?.title || resolved?.name || plugin?.id || (plugin ? pluginSource(plugin) : '') || ''; const description = registryDetail?.description; const meta = [ matchedRegistry?.name, @@ -127,8 +129,8 @@ export function PluginPreviewDialog({

{description}

)} {meta &&

{meta}

} - {!description && plugin?.def.repo && ( -

{plugin.def.repo}

+ {!description && plugin && pluginSource(plugin) && ( +

{pluginSource(plugin)}

)} diff --git a/web-ui/src/features/projects/components/PluginsEditor.tsx b/web-ui/src/features/projects/components/PluginsEditor.tsx index 61c32b17..e8ede18c 100644 --- a/web-ui/src/features/projects/components/PluginsEditor.tsx +++ b/web-ui/src/features/projects/components/PluginsEditor.tsx @@ -1,7 +1,7 @@ import { useState, type ReactNode } from 'react'; import { Trash2 } from 'lucide-react'; import { useTranslation } from 'react-i18next'; -import type { AuthoredPlugin, ResolvedPlugin } from '../../../types/api'; +import { pluginSource, type AuthoredPlugin, type ResolvedPlugin } from '../../../types/api'; import { SourceBadge } from '../../../components/common/ServerBadge'; import { ReorderableList } from '../../../components/common/ReorderableList'; import { useDeleteCapability, useReorderCapability } from '../hooks'; @@ -43,9 +43,9 @@ function PluginRow({ className="min-w-0 flex-1 p-3 text-left transition-colors hover:bg-hover-bg cursor-pointer" >
- +
-
{plugin.def.repo}
+
{pluginSource(plugin)}
{resolvedInfo?.version && (
{resolvedInfo.version}
)} @@ -83,13 +83,13 @@ export function PluginsEditor({ authored, resolved, projectId }: PluginsEditorPr ? resolved.find( (r) => r.name === previewPlugin.id || - r.repository?.includes(previewPlugin.def.repo.split('@')[0]), + r.repository?.includes(pluginSource(previewPlugin).split('@')[0]), ) : undefined; function resolveInfo(plugin: AuthoredPlugin) { return resolved.find( - (r) => r.name === plugin.id || r.repository?.includes(plugin.def.repo.split('@')[0]), + (r) => r.name === plugin.id || r.repository?.includes(pluginSource(plugin).split('@')[0]), ); } @@ -125,7 +125,7 @@ export function PluginsEditor({ authored, resolved, projectId }: PluginsEditorPr )} {withoutId.map((plugin) => ( setPreviewPlugin(plugin)} diff --git a/web-ui/src/types/api.ts b/web-ui/src/types/api.ts index 63cc91cb..90f2c5f0 100644 --- a/web-ui/src/types/api.ts +++ b/web-ui/src/types/api.ts @@ -191,13 +191,21 @@ export interface AuthoredPlugin { id: string | null; type: string; def: { - repo: string; + /** github / gitlab repo string. */ + repo?: string; + /** Clone URL of a `git` plugin (any host). */ + url?: string; version?: string; ref?: string; subpath?: string; }; } +/** What a plugin entry points at: its repo string, or clone URL for `git` plugins. */ +export function pluginSource(plugin: AuthoredPlugin): string { + return plugin.def.repo ?? plugin.def.url ?? ''; +} + export interface RequiredCommand { cli: string; description: string | null; From fac2232b0c187a6f7fe6ea1a56ef4817f73f74d3 Mon Sep 17 00:00:00 2001 From: Antonio Zaitoun Date: Sat, 10 Oct 2026 22:53:09 +0300 Subject: [PATCH 3/5] fix: only clone git sources over real transports new URL accepts git helper schemes such as ext::, which git can run. Git blocks ext by default, but capabilities files can come from elsewhere, so allow only https, http, ssh and file, and separate the clone URL from options with --. Also accept ssh:// URLs in capa add. --- src/shared/__tests__/git-url.test.ts | 14 ++++++++++++++ src/shared/cache/mirror.ts | 2 +- src/shared/git-url.ts | 11 ++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/shared/__tests__/git-url.test.ts b/src/shared/__tests__/git-url.test.ts index 9e55b9a4..fe8ee97b 100644 --- a/src/shared/__tests__/git-url.test.ts +++ b/src/shared/__tests__/git-url.test.ts @@ -57,6 +57,20 @@ describe('gitRepoKey', () => { it('ignores credentials in the URL', () => { expect(gitRepoKey('https://user:secret@h.test/a/b.git')).toBe(gitRepoKey('https://h.test/a/b.git')); }); + + it('accepts ssh clone URLs', () => { + expect(gitRepoKey('ssh://git@h.test/a/b.git')).toMatch(/^h\.test\/a\/b-[0-9a-f]{12}$/); + expect(parseGitUrlSource('ssh://git@h.test/a/b.git:v1.0.0')).toMatchObject({ + url: 'ssh://git@h.test/a/b.git', + version: 'v1.0.0', + }); + }); + + it('rejects git helper transports that run commands', () => { + expect(() => gitRepoKey('ext::sh -c touch% /tmp/pwned')).toThrow(/Unsupported git URL scheme/); + expect(() => gitRepoKey('fd::17/foo.git')).toThrow(/Unsupported git URL scheme/); + expect(() => gitRepoKey('--upload-pack=touch /tmp/pwned')).toThrow(); + }); }); describe('plugin schema', () => { diff --git a/src/shared/cache/mirror.ts b/src/shared/cache/mirror.ts index 79a5e8e9..eebb809f 100644 --- a/src/shared/cache/mirror.ts +++ b/src/shared/cache/mirror.ts @@ -186,7 +186,7 @@ export async function ensureMirrorClone( // // Requires git >= 2.19. Servers without partial-clone support degrade // gracefully to a full clone (git warns and ignores the filter). - await git(["clone", "--mirror", "--filter=blob:none", url, mirrorDir], { + await git(["clone", "--mirror", "--filter=blob:none", "--", url, mirrorDir], { env, }); await git(["-C", mirrorDir, "remote", "set-url", "origin", url]); diff --git a/src/shared/git-url.ts b/src/shared/git-url.ts index 92630a48..0d8d775d 100644 --- a/src/shared/git-url.ts +++ b/src/shared/git-url.ts @@ -6,12 +6,13 @@ * https://git.example.com/team/skills.git::path/to/skill exact subpath * https://git.example.com/team/skills.git:v1.2.0 tag or branch * https://git.example.com/team/skills.git# commit + * ssh://git@git.example.com/team/skills.git over SSH */ import { createHash } from "crypto"; const GIT_URL_RE = - /^((?:https?|file):\/\/.+?\.git)\/?(?:::([^:#]+))?(?::([\w.+/-]+))?(?:#([a-f0-9]{7,40}))?$/i; + /^((?:https?|ssh|file):\/\/.+?\.git)\/?(?:::([^:#]+))?(?::([\w.+/-]+))?(?:#([a-f0-9]{7,40}))?$/i; export interface GitUrlSource { /** Clone URL, without the `::path` / `:version` / `#sha` suffixes. */ @@ -45,6 +46,8 @@ export function isGitUrl(source: string): boolean { return parseGitUrlSource(source) !== null; } +const CLONE_PROTOCOLS = new Set(["https:", "http:", "ssh:", "file:"]); + /** * Cache key for a clone URL: `/-`, e.g. * `git.example.com/team/skills-3fa2c1d09b7e`. Host and path keep it readable; @@ -56,6 +59,12 @@ export function isGitUrl(source: string): boolean { */ export function gitRepoKey(url: string): string { const u = new URL(url); + // Only real transports: `new URL` also accepts helper schemes such as + // `ext::`, which git would run (it blocks `ext` by default, but + // capabilities files can come from elsewhere, so don't rely on that). + if (!CLONE_PROTOCOLS.has(u.protocol)) { + throw new Error(`Unsupported git URL scheme "${u.protocol}" in ${url}`); + } u.username = ""; u.password = ""; const hash = createHash("sha256").update(u.href).digest("hex").slice(0, 12); From 7e2b5573eb2f60ae90a634f254b603c97d4f816c Mon Sep 17 00:00:00 2001 From: Antonio Zaitoun Date: Sat, 10 Oct 2026 22:53:21 +0300 Subject: [PATCH 4/5] docs: note supported git URL schemes --- skills/capabilities-manager/references/capabilities-schema.md | 2 +- skills/capabilities-manager/references/commands.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/capabilities-manager/references/capabilities-schema.md b/skills/capabilities-manager/references/capabilities-schema.md index 6bb8ea1e..d0b195d3 100644 --- a/skills/capabilities-manager/references/capabilities-schema.md +++ b/skills/capabilities-manager/references/capabilities-schema.md @@ -53,7 +53,7 @@ tools: - **inline**: Embed SKILL.md content in `def.content`. Use `requires: ['@server_id.tool_id']` for MCP tools, plain ID for command tools. - **github**: `def.repo: owner/repo@skill-name` (search) or `owner/repo::path/to/skill-name` (exact). See "Repo string format" below for details. - **gitlab**: `def.repo: group/subgroup/repo@skill-name` (search) or `group/subgroup/repo::path/to/skill-name` (exact). Subgroups are supported. -- **git**: `def.url` is the clone URL of any git host (ends in `.git`); `def.path` is the skill directory inside the repo (default: the root). Pin with `def.version` (tag/branch) or `def.ref` (commit). Unpinned installs take the newest `vX.Y.Z` tag. Private repos use your git credential helper. +- **git**: `def.url` is the clone URL of any git host (ends in `.git`; `https`, `http`, `ssh` or `file`); `def.path` is the skill directory inside the repo (default: the root). Pin with `def.version` (tag/branch) or `def.ref` (commit). Unpinned installs take the newest `vX.Y.Z` tag. Private repos use your git credential helper. - **remote**: `def.url` to a SKILL.md URL. - **local**: `def.path` to a directory containing SKILL.md (read on each install). - **installed**: User installed the skill elsewhere; capa only records it for tool binding. Declare directly in capabilities.yaml (no CLI shortcut) — set `def.description` and `def.requires` as needed. diff --git a/skills/capabilities-manager/references/commands.md b/skills/capabilities-manager/references/commands.md index 46abf14b..809697d0 100644 --- a/skills/capabilities-manager/references/commands.md +++ b/skills/capabilities-manager/references/commands.md @@ -112,7 +112,7 @@ Add an entry to `capabilities.yaml` (default) or write provider-native files imm - **Pinning** (works with both `@` and `::`): append `:v1.2.3` for a tag/branch or `#abc1234` for a commit SHA — e.g. `capa add owner/repo@skill:v1.2.3`, `capa add gitlab:group/repo::skills/x/y#abc1234`. - **Registry**: `capa add :` — resolve the skill from a configured registry adapter. Example: `capa add skills-sh:vercel-labs/skills/find-skills`. - **Installed (no CLI)**: declare `type: installed` directly in `capabilities.yaml` — capa only records the skill for tool binding, it does not fetch or install anything. Set `def.description` and `def.requires` on the entry. Same pattern for `type: plugin` (skills shipped by a configured plugin). -- **Any git host**: `capa add https://git.example.com/team/skills.git` — a clone URL ending in `.git`, stored as type `git` with `def.url`. Installs the `SKILL.md` at the repo root; add `::path/to/skill` for another directory, and `:v1.2.3` / `#abc1234` to pin. Private repos use your git credential helper (clone once with `git` to sign in). +- **Any git host**: `capa add https://git.example.com/team/skills.git` — a clone URL ending in `.git`, stored as type `git` with `def.url`. Installs the `SKILL.md` at the repo root; add `::path/to/skill` for another directory, and `:v1.2.3` / `#abc1234` to pin. `https://`, `http://`, `ssh://` and `file://` URLs work. Private repos use your git credential helper or SSH key (clone once with `git` to sign in). - **Remote URL**: `capa add https://example.com/path/to/SKILL.md` - **Local path**: `capa add ./path/to/skill` — directory must contain `SKILL.md`; stored as type `local` so the file is read on each install From 6685e40f20806dbbba93382f279e873892450cae Mon Sep 17 00:00:00 2001 From: Antonio Zaitoun Date: Sat, 10 Oct 2026 23:30:48 +0300 Subject: [PATCH 5/5] fix: harden git sources against SSRF and token leaks - Server skill preview only clones public https git URLs and sends no git-host token, matching the remote skill rule. - Server plugin expansion does the same for git plugins, except that a snapshot capa install already pinned is used offline. - Stored tokens are only attached to https URLs (http only on loopback), and git clone URLs accept plain http only for localhost. - Credentials embedded in a git URL are stripped before the lockfile, errors and printed output. - The server preview refuses a SKILL.md that is a symlink or resolves outside the repo snapshot. --- src/cli/commands/add.ts | 5 +- .../__tests__/install-one-skill-git.test.ts | 25 ++- .../helpers/install-one-skill.ts | 11 +- .../install-tasks/helpers/repo-snapshot.ts | 4 +- .../__tests__/skill-content-git.test.ts | 160 ++++++++++++++++++ src/server/resolve-effective-capabilities.ts | 42 ++++- src/server/skill-content.ts | 59 +++++-- .../__tests__/authenticated-fetch.test.ts | 17 ++ src/shared/__tests__/cache-git-auth.test.ts | 19 +++ src/shared/__tests__/git-url.test.ts | 21 ++- src/shared/authenticated-fetch.ts | 4 + src/shared/cache/mirror.ts | 39 ++--- src/shared/cache/snapshot.ts | 3 +- src/shared/executable-surface.ts | 3 +- src/shared/git-url.ts | 41 +++++ src/shared/plugin-source.ts | 14 +- 16 files changed, 406 insertions(+), 61 deletions(-) create mode 100644 src/server/__tests__/skill-content-git.test.ts diff --git a/src/cli/commands/add.ts b/src/cli/commands/add.ts index bd684e3c..0e2a2f96 100644 --- a/src/cli/commands/add.ts +++ b/src/cli/commands/add.ts @@ -17,6 +17,7 @@ import { tryResolveRegistryItem } from './resolve-registry-source'; import { parseSkillSource, type ParsedSkillSource } from './add-parse-skill'; import { parsePluginSource, type ParsedPluginSource } from './add-parse-plugin'; import { pluginSourceOf } from '../../shared/plugin-source'; +import { stripHttpUrlUserinfo } from '../../shared/git-url'; export { parseSkillSource, type ParsedSkillSource } from './add-parse-skill'; export { parsePluginSource, type ParsedPluginSource } from './add-parse-plugin'; @@ -247,7 +248,7 @@ export async function addCommand( console.log(`✓ Added plugin "${id}" to ${capabilitiesFile.path}`); console.log(` Type: ${parsed.type}`); - console.log(` Repo: ${pluginSourceOf(parsed.def)}`); + console.log(` Repo: ${stripHttpUrlUserinfo(pluginSourceOf(parsed.def) ?? '')}`); if (parsed.def.version) console.log(` Version: ${parsed.def.version}`); if (parsed.def.ref) console.log(` Ref: ${parsed.def.ref}`); @@ -289,7 +290,7 @@ export async function addCommand( if (skillDef.def.repo) { console.log(` Repo: ${skillDef.def.repo}`); } else if (skillDef.def.url) { - console.log(` URL: ${skillDef.def.url}`); + console.log(` URL: ${stripHttpUrlUserinfo(skillDef.def.url)}`); } else if (skillDef.def.path) { console.log(` Path: ${skillDef.def.path}`); } diff --git a/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts index 0d19a541..20173aa4 100644 --- a/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts +++ b/src/cli/commands/install-tasks/helpers/__tests__/install-one-skill-git.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { describe, it, expect, beforeEach, afterEach, spyOn } from 'bun:test'; import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; import { join } from 'path'; import { tmpdir } from 'os'; @@ -8,6 +8,7 @@ import { CapaDatabase } from '../../../../../db/database'; import { LockfileBuilder } from '../../../../../shared/lockfile'; import { installOneSkill, looksLikeHtml } from '../install-one-skill'; import { getCacheStats } from '../../../../../shared/cache'; +import * as repoSnapshot from '../repo-snapshot'; import type { Capabilities, Skill } from '../../../../../types/capabilities'; // A skill on "any git host": a real repository cloned over file://, so the @@ -88,6 +89,28 @@ describe('installOneSkill with a git clone URL', () => { }); }); + it('keeps credentials in the URL out of the lock entry and the clone', async () => { + const real = repoSnapshot.getRepoSnapshot; + const seen: (string | undefined)[] = []; + // No real host in tests: record the URL capa clones, then serve the local repo. + const spy = spyOn(repoSnapshot, 'getRepoSnapshot').mockImplementation((platform, repoPath, auth, opts = {}) => { + seen.push(opts.repoUrl); + return real(platform, repoPath, auth, { ...opts, repoUrl }); + }); + try { + const lock = await install({ + id: 'code-review', + type: 'git', + def: { url: 'https://user:s3cret@git.example.com/acme/code-review.git' }, + }); + expect(lock.skills[0].url).toBe('https://git.example.com/acme/code-review.git'); + expect(JSON.stringify(lock)).not.toContain('s3cret'); + expect(seen).toEqual(['https://git.example.com/acme/code-review.git']); + } finally { + spy.mockRestore(); + } + }); + it('installs a nested skill from def.path', async () => { await install({ id: 'lint', type: 'git', def: { url: repoUrl, path: 'nested/lint' } }); expect(readFileSync(join(projectPath, '.cursor', 'skills', 'lint', 'SKILL.md'), 'utf8')).toContain('name: lint'); diff --git a/src/cli/commands/install-tasks/helpers/install-one-skill.ts b/src/cli/commands/install-tasks/helpers/install-one-skill.ts index 68bf65f4..8ed01d33 100644 --- a/src/cli/commands/install-tasks/helpers/install-one-skill.ts +++ b/src/cli/commands/install-tasks/helpers/install-one-skill.ts @@ -1,6 +1,6 @@ import { existsSync, mkdirSync, writeFileSync, rmSync } from 'fs'; import { resolve, join, dirname, basename, relative } from 'path'; -import { gitRepoKey } from '../../../../shared/git-url'; +import { gitRepoKey, stripHttpUrlUserinfo } from '../../../../shared/git-url'; import type { Skill, Capabilities } from '../../../../types/capabilities'; import type { CapaDatabase } from '../../../../db/database'; import { createAuthenticatedFetch, AuthenticatedFetch } from '../../../../shared/authenticated-fetch'; @@ -242,12 +242,15 @@ export async function installOneSkill( // Any git host: `def.url` is the clone URL, `def.path` the directory // holding SKILL.md (the repo root when unset). Credentials come from the // user's git credential helper. - const url = skill.def.url; + // Credentials in the URL are never used for the clone; keep them out of + // the lockfile and messages too. + const url = stripHttpUrlUserinfo(skill.def.url); let repoPath: string; try { repoPath = gitRepoKey(url); - } catch { - throw new Error(`Invalid git clone URL for skill "${skill.id}": ${url}`); + } catch (err) { + const why = err instanceof TypeError ? '' : ` (${(err as Error).message})`; + throw new Error(`Invalid git clone URL for skill "${skill.id}": ${url}${why}`); } const skillPath = (skill.def.path ?? '').replace(/^\/+|\/+$/g, ''); const version = skill.def.version; diff --git a/src/cli/commands/install-tasks/helpers/repo-snapshot.ts b/src/cli/commands/install-tasks/helpers/repo-snapshot.ts index b362db11..5fcd2c95 100644 --- a/src/cli/commands/install-tasks/helpers/repo-snapshot.ts +++ b/src/cli/commands/install-tasks/helpers/repo-snapshot.ts @@ -10,11 +10,11 @@ import { explainGitError } from './git'; export async function getRepoSnapshot( platform: CachePlatform, repoPath: string, - authFetch: AuthenticatedFetch, + authFetch: AuthenticatedFetch | undefined, opts: { version?: string; ref?: string; pinnedSha?: string; noCache?: boolean; repoUrl?: string } = {} ): Promise { const repoUrl = opts.repoUrl ?? `https://${platform}.com/${repoPath}`; - const hasAuth = authFetch.hasAuth(repoUrl); + const hasAuth = authFetch?.hasAuth(repoUrl) ?? false; try { return await getOrCreateSnapshot({ platform, diff --git a/src/server/__tests__/skill-content-git.test.ts b/src/server/__tests__/skill-content-git.test.ts new file mode 100644 index 00000000..39ca9335 --- /dev/null +++ b/src/server/__tests__/skill-content-git.test.ts @@ -0,0 +1,160 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { execFileSync } from "child_process"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + rmSync, + symlinkSync, + writeFileSync, +} from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { pathToFileURL } from "url"; +import type { AuthenticatedFetch } from "../../shared/authenticated-fetch"; +import { getSnapshotDir } from "../../shared/cache"; +import { gitRepoKey } from "../../shared/git-url"; +import { serverRepoSnapshot } from "../resolve-effective-capabilities"; +import { + fetchUninstalledSkillContent, + previewSnapshotSkill, +} from "../skill-content"; + +const SKILL = "---\nname: s\ndescription: d\n---\n\nBody.\n"; + +// Hands out a token for every URL, so a test fails if one ever gets used. +const leakyAuth = { + hasAuth: () => true, + getTokenForUrl: () => "gho_SHOULD_NOT_BE_SENT", + fetch: () => Promise.reject(new Error("no network in tests")), +} as unknown as AuthenticatedFetch; + +let dir: string; +const prevCacheDir = process.env.CAPA_CACHE_DIR; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "capa-skill-content-git-")); + process.env.CAPA_CACHE_DIR = join(dir, "cache"); +}); + +afterEach(() => { + if (prevCacheDir === undefined) delete process.env.CAPA_CACHE_DIR; + else process.env.CAPA_CACHE_DIR = prevCacheDir; + rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); +}); + +function bareRepoWithSkill(): string { + const git = (cwd: string, ...args: string[]) => + execFileSync("git", args, { cwd, encoding: "utf8" }); + const work = join(dir, "work"); + mkdirSync(work); + git(work, "init", "-q", "-b", "main"); + git(work, "config", "user.email", "capa-test@example.com"); + git(work, "config", "user.name", "capa-test"); + writeFileSync(join(work, "SKILL.md"), SKILL); + git(work, "add", "-A"); + git(work, "commit", "-q", "-m", "init"); + const bare = join(dir, "skill.git"); + git(dir, "clone", "-q", "--bare", work, bare); + return pathToFileURL(bare).href; +} + +function cacheIsEmpty(): boolean { + const cache = join(dir, "cache"); + return !existsSync(cache) || readdirSync(cache).length === 0; +} + +describe("server preview of an uninstalled git skill", () => { + it("does not clone a file:// URL from the project file", async () => { + const url = bareRepoWithSkill(); + const result = await fetchUninstalledSkillContent( + { id: "s", type: "git", def: { url } }, + leakyAuth, + ); + expect(result).toBeNull(); + expect(cacheIsEmpty()).toBe(true); + }); + + it("refuses loopback, private, ssh and plain http URLs before any git call", async () => { + for (const url of [ + "http://localhost:8080/a/s.git", + "https://127.0.0.1/a/s.git", + "https://10.0.0.5/a/s.git", + "ssh://git@h.test/a/s.git", + "https://user:pw@h.test/a/s.git", + ]) { + const result = await fetchUninstalledSkillContent( + { id: "s", type: "git", def: { url } }, + leakyAuth, + ); + expect(result).toBeNull(); + } + expect(cacheIsEmpty()).toBe(true); + }); +}); + +describe("serverRepoSnapshot for git plugins", () => { + it("refuses to clone a non-public URL", async () => { + const url = bareRepoWithSkill(); + await expect( + serverRepoSnapshot("git", gitRepoKey(url), leakyAuth, { repoUrl: url }), + ).rejects.toThrow(/Only https URLs are allowed/); + expect(cacheIsEmpty()).toBe(true); + }); + + it("uses a snapshot capa install already pinned, offline, whatever the URL", async () => { + const url = "ssh://git@h.test/a/plugin.git"; + const sha = "a".repeat(40); + const snapshotDir = getSnapshotDir("git", gitRepoKey(url), sha); + mkdirSync(snapshotDir, { recursive: true }); + const result = await serverRepoSnapshot("git", gitRepoKey(url), leakyAuth, { + repoUrl: url, + pinnedSha: sha, + }); + expect(result.snapshotDir).toBe(snapshotDir); + }); +}); + +describe("previewSnapshotSkill", () => { + function snapshot(): string { + const root = join(dir, "snap"); + mkdirSync(join(root, "skill"), { recursive: true }); + return root; + } + + function outsideSkillDir(): string { + const outside = join(dir, "outside"); + mkdirSync(outside); + writeFileSync(join(outside, "SKILL.md"), "---\nname: secret\n---\n"); + return outside; + } + + it("reads a regular SKILL.md", () => { + const root = snapshot(); + writeFileSync(join(root, "skill", "SKILL.md"), SKILL); + expect(previewSnapshotSkill(root, join(root, "skill", "SKILL.md"))?.content).toBe(SKILL); + }); + + it("refuses a skill directory linked outside the snapshot", () => { + const root = snapshot(); + const outside = outsideSkillDir(); + // Junctions need no privileges on Windows; elsewhere the type is ignored. + symlinkSync(outside, join(root, "linked"), "junction"); + expect(previewSnapshotSkill(root, join(root, "linked", "SKILL.md"))).toBeNull(); + }); + + it("refuses a SKILL.md that is a symlink", () => { + const root = snapshot(); + const outside = outsideSkillDir(); + const link = join(root, "skill", "SKILL.md"); + try { + symlinkSync(join(outside, "SKILL.md"), link, "file"); + } catch { + // File symlinks need Developer Mode or admin on Windows. + console.warn("skipping: cannot create file symlinks here"); + return; + } + expect(previewSnapshotSkill(root, link)).toBeNull(); + }); +}); diff --git a/src/server/resolve-effective-capabilities.ts b/src/server/resolve-effective-capabilities.ts index 64cb1cc1..3a7a219b 100644 --- a/src/server/resolve-effective-capabilities.ts +++ b/src/server/resolve-effective-capabilities.ts @@ -1,8 +1,17 @@ -import { rmSync, statSync } from "fs"; +import { existsSync, rmSync, statSync } from "fs"; import { getRepoSnapshot } from "../cli/commands/install-tasks/helpers/repo-snapshot"; import { resolvePlugins } from "../cli/commands/plugin-install"; import type { CapaDatabase } from "../db/database"; -import { createAuthenticatedFetch } from "../shared/authenticated-fetch"; +import { + type AuthenticatedFetch, + createAuthenticatedFetch, +} from "../shared/authenticated-fetch"; +import { + type CachePlatform, + type GetSnapshotResult, + getSnapshotDir, +} from "../shared/cache"; +import { assertPublicHttpsUrl } from "../shared/safe-remote-url"; import { LockfileBuilder, loadLockfile } from "../shared/lockfile"; import { logger } from "../shared/logger"; import { validateProvider } from "../shared/providers/resolve"; @@ -34,6 +43,32 @@ function pluginsCacheKey( .join("||"); } +/** + * Snapshot fetcher for server-side plugin expansion. A `git` plugin's clone + * URL comes straight from the project file, so (like the remote skill preview) + * the server only clones public https URLs and sends no git-host token. A + * plugin `capa install` already pinned and cached is used offline, whatever + * its URL, so private and ssh plugins keep working after an install. + */ +export async function serverRepoSnapshot( + platform: CachePlatform, + repoPath: string, + auth: AuthenticatedFetch, + opts: Parameters[3] = {}, +): Promise { + if (platform !== "git") { + return getRepoSnapshot(platform, repoPath, auth, opts); + } + const pinned = opts.pinnedSha; + const cached = + !!pinned && + !opts.noCache && + /^[a-f0-9]{40}$/i.test(pinned) && + existsSync(getSnapshotDir(platform, repoPath, pinned)); + if (!cached) await assertPublicHttpsUrl(opts.repoUrl ?? ""); + return getRepoSnapshot(platform, repoPath, undefined, opts); +} + /** * Resolve providers for server-side plugin expansion without interactive prompts. * Returns null when none are configured (caller should skip resolvePlugins). @@ -95,8 +130,7 @@ export async function resolveEffectiveCapabilities( projectId, authFetch, db, - (platform, repoPath, auth, opts) => - getRepoSnapshot(platform, repoPath, auth, opts), + serverRepoSnapshot, capabilitiesFilePath, lockBuilder, { diff --git a/src/server/skill-content.ts b/src/server/skill-content.ts index 1dfe3941..b1f71801 100644 --- a/src/server/skill-content.ts +++ b/src/server/skill-content.ts @@ -2,7 +2,14 @@ * Resolve and read a skill's SKILL.md for a project. * Used by the project-detail API for frontmatter descriptions and the skill content endpoint. */ -import { existsSync, readdirSync, readFileSync, statSync } from "fs"; +import { + existsSync, + lstatSync, + readdirSync, + readFileSync, + realpathSync, + statSync, +} from "fs"; import { isAbsolute, join, relative } from "path"; import { findSkillsInDirectory, @@ -16,7 +23,10 @@ import { getProvider } from "../shared/providers"; import { assertSafeRepoPath } from "../shared/repo-file"; import { parseRepoString } from "../shared/repo-string"; import { isSafeCapabilityId } from "../shared/safe-id"; -import { fetchPublicHttpsText } from "../shared/safe-remote-url"; +import { + assertPublicHttpsUrl, + fetchPublicHttpsText, +} from "../shared/safe-remote-url"; import { parseSkillMd, type SkillMetadata } from "../shared/skill-md"; import type { Capabilities, Skill } from "../types/capabilities"; import type { ResolvedPluginInfo } from "../types/plugin"; @@ -358,6 +368,31 @@ export async function resolveSkillContentById( return null; } +/** + * Parse a SKILL.md out of a cached repo snapshot. Refuses a SKILL.md that is a + * symlink or resolves outside the snapshot, so a cloned repo can't make the + * server read (and return) a file elsewhere on disk. + */ +export function previewSnapshotSkill( + snapshotDir: string, + skillMdPath: string, +): SkillContentResult | null { + const rel = relative(realpathSync(snapshotDir), realpathSync(skillMdPath)); + if ( + lstatSync(skillMdPath).isSymbolicLink() || + isAbsolute(rel) || + rel.split(/[\\/]/)[0] === ".." + ) { + return null; + } + const skillData = readSkillFromDirectory(skillMdPath); + const files = [ + "SKILL.md", + ...Array.from(skillData.additionalFiles.keys()).sort(), + ]; + return tryParse(skillData.markdown, files); +} + /** * Fetch SKILL.md for an uninstalled github/gitlab/remote skill using the * shared git cache (or HTTP for remote URLs). Returns null on failure. @@ -409,20 +444,19 @@ export async function fetchUninstalledSkillContent( if (!skillMdPath) return null; } - const skillData = readSkillFromDirectory(skillMdPath); - const files = [ - "SKILL.md", - ...Array.from(skillData.additionalFiles.keys()).sort(), - ]; - return tryParse(skillData.markdown, files); + return previewSnapshotSkill(snapshot.snapshotDir, skillMdPath); } if (skill.type === "git" && skill.def?.url) { + // Same rule as `remote`: an uninstalled skill's URL comes from the + // project file, so the server only clones public https hosts and + // sends no git-host token (SSRF / credential leak). Other URLs are + // previewable once `capa install` has put the skill on disk. + await assertPublicHttpsUrl(skill.def.url); const snapshot = await getOrCreateSnapshot({ platform: "git", repoPath: gitRepoKey(skill.def.url), repoUrl: skill.def.url, - authFetch, version: skill.def.version, ref: skill.def.ref, }); @@ -432,12 +466,7 @@ export async function fetchUninstalledSkillContent( : snapshot.snapshotDir; const skillMdPath = join(skillDir, "SKILL.md"); if (!existsSync(skillMdPath)) return null; - const skillData = readSkillFromDirectory(skillMdPath); - const files = [ - "SKILL.md", - ...Array.from(skillData.additionalFiles.keys()).sort(), - ]; - return tryParse(skillData.markdown, files); + return previewSnapshotSkill(snapshot.snapshotDir, skillMdPath); } } catch { return null; diff --git a/src/shared/__tests__/authenticated-fetch.test.ts b/src/shared/__tests__/authenticated-fetch.test.ts index f1481da3..7fec3ff2 100644 --- a/src/shared/__tests__/authenticated-fetch.test.ts +++ b/src/shared/__tests__/authenticated-fetch.test.ts @@ -103,6 +103,23 @@ describe('AuthenticatedFetch', () => { expect(headers.get('Authorization')).toBe('token gho_test_token'); }); + it('never hands a token to a plain http URL on a known host', async () => { + const authFetch = new AuthenticatedFetch(makeDb(makeIntegration())); + const httpUrl = 'http://github.com/owner/repo.git'; + + expect(authFetch.hasAuth(httpUrl)).toBe(false); + expect(authFetch.getTokenForUrl(httpUrl)).toBeNull(); + await authFetch.fetch(httpUrl); + expect((fetchCalls[0]!.init?.headers as Headers).get('Authorization')).toBeNull(); + expect(authFetch.getTokenForUrl('https://github.com/owner/repo.git')).toBe('gho_test_token'); + }); + + it('still allows http for a self-managed host on loopback', () => { + const integration = makeIntegration({ platform: 'gitlab-self-managed', host: 'localhost' }); + const authFetch = new AuthenticatedFetch(makeDb(integration)); + expect(authFetch.getTokenForUrl('http://localhost:8929/group/repo.git')).toBe('gho_test_token'); + }); + it('returns a 200 response unchanged', async () => { const db = makeDb(makeIntegration()); const authFetch = new AuthenticatedFetch(db); diff --git a/src/shared/__tests__/cache-git-auth.test.ts b/src/shared/__tests__/cache-git-auth.test.ts index ea7dff02..0cf59125 100644 --- a/src/shared/__tests__/cache-git-auth.test.ts +++ b/src/shared/__tests__/cache-git-auth.test.ts @@ -162,6 +162,25 @@ describe('cache git auth (F10)', () => { } }); + it('does not inject the token for a plain http URL on the token host', async () => { + const calls: { args: string[]; env?: NodeJS.ProcessEnv }[] = []; + const execFileSpy = spyOn(childProcess, 'execFile').mockImplementation( + ((_cmd: string, args: string[], opts: object, cb: (err: null, stdout: string, stderr: string) => void) => { + calls.push({ args, env: (opts as { env?: NodeJS.ProcessEnv }).env }); + cb(null, '', ''); + }) as typeof childProcess.execFile, + ); + + try { + await ensureMirrorClone('github', 'owner/repo', authFetch, 'http://github.com/owner/repo.git'); + const cloneCall = calls.find((c) => c.args.includes('clone')); + expect(cloneCall).toBeDefined(); + expect(envCarriesOutOfBandAuth(cloneCall!.env)).toBe(false); + } finally { + execFileSpy.mockRestore(); + } + }); + it('does not inject auth env for public clones', async () => { const calls: { args: string[]; env?: NodeJS.ProcessEnv }[] = []; const execFileSpy = spyOn(childProcess, 'execFile').mockImplementation( diff --git a/src/shared/__tests__/git-url.test.ts b/src/shared/__tests__/git-url.test.ts index fe8ee97b..fd7494f2 100644 --- a/src/shared/__tests__/git-url.test.ts +++ b/src/shared/__tests__/git-url.test.ts @@ -44,8 +44,8 @@ describe('gitRepoKey', () => { it('never shares a key between different URLs', () => { const keys = new Set( [ - 'https://h.test/a/b.git', - 'http://h.test/a/b.git', + 'https://localhost/a/b.git', + 'http://localhost/a/b.git', 'https://h.test:8443/a/b.git', 'https://h.test/a:b.git', 'https://h.test/a_b.git', @@ -66,6 +66,14 @@ describe('gitRepoKey', () => { }); }); + it('allows plain http only to this machine', () => { + expect(() => gitRepoKey('http://git.example.com/a/b.git')).toThrow(/only allowed for localhost/); + expect(() => gitRepoKey('http://user:s3cret@github.com/a/b.git')).toThrow(/^(?!.*s3cret)/); + for (const host of ['localhost', '127.0.0.1', '[::1]']) { + expect(() => gitRepoKey(`http://${host}:8080/a/b.git`)).not.toThrow(); + } + }); + it('rejects git helper transports that run commands', () => { expect(() => gitRepoKey('ext::sh -c touch% /tmp/pwned')).toThrow(/Unsupported git URL scheme/); expect(() => gitRepoKey('fd::17/foo.git')).toThrow(/Unsupported git URL scheme/); @@ -112,6 +120,15 @@ describe('capa add with a git clone URL', () => { }); }); + it('keeps credentials in the URL out of the validated (locked, printed) plugin URL', () => { + expect( + validatePluginDef({ type: 'git', def: { url: 'https://user:s3cret@h.test/acme/tools.git' } }), + ).toMatchObject({ repoUrl: 'https://h.test/acme/tools.git' }); + expect(validatePluginDef({ type: 'git', def: { url: 'http://h.test/acme/tools.git' } })).toMatchObject({ + error: expect.stringMatching(/only allowed for localhost/), + }); + }); + it('still parses GitHub URLs as github', () => { expect(parsePluginSource('https://github.com/owner/repo.git').type).toBe('github'); }); diff --git a/src/shared/authenticated-fetch.ts b/src/shared/authenticated-fetch.ts index 2a2c3775..ac1785e6 100644 --- a/src/shared/authenticated-fetch.ts +++ b/src/shared/authenticated-fetch.ts @@ -9,6 +9,7 @@ import type { CapaDatabase } from "../db/database"; import type { GitIntegration } from "../types/database"; import type { GitPlatform } from "../types/git-integration"; import { getGitProvider, getGitProviderByHost } from "./git-providers/registry"; +import { isHttpsOrLoopback } from "./git-url"; import { isPermanentRefreshFailure } from "./oauth-refresh"; const CLOUD_OAUTH_ENDPOINT = "https://capa.infragate.ai/auth"; @@ -43,6 +44,9 @@ export class AuthenticatedFetch { ): { platform: GitPlatform; host?: string } | null { try { const urlObj = new URL(url); + // Tokens only travel over https (or http to this machine): a plain + // http URL on a known host would send them in cleartext. + if (!isHttpsOrLoopback(urlObj)) return null; const host = urlObj.hostname; const provider = getGitProviderByHost(host); diff --git a/src/shared/cache/mirror.ts b/src/shared/cache/mirror.ts index eebb809f..0951c350 100644 --- a/src/shared/cache/mirror.ts +++ b/src/shared/cache/mirror.ts @@ -8,6 +8,11 @@ import { } from "fs"; import { join } from "path"; import type { AuthenticatedFetch } from "../authenticated-fetch"; +import { + isHttpsOrLoopback, + stripHttpUrlUserinfo, + stripUserinfoFromHttpUrl, +} from "../git-url"; import { git, gitHttpCredentialEnv } from "./git-cli"; import { type CachePlatform, @@ -28,33 +33,17 @@ function publicHttpsRepoUrl( return `https://${platform}.com/${repoPath}.git`; } -/** Strip userinfo from an http(s) URL. Returns null when there is none or the value is not an http(s) URL. */ -function stripUserinfoFromHttpUrl(raw: string): string | null { - const trimmed = raw.trim().replace(/^["']|["']$/g, ""); - try { - const u = new URL(trimmed); - if (u.protocol !== "http:" && u.protocol !== "https:") return null; - if (!u.username && !u.password) return null; - u.username = ""; - u.password = ""; - let out = u.toString(); - if (!trimmed.endsWith("/") && out.endsWith("/")) { - out = out.slice(0, -1); - } - return out; - } catch { - return null; - } -} - -function stripHttpUrlUserinfo(url: string): string { - return stripUserinfoFromHttpUrl(url) ?? url; -} - function credentialEnvFor( url: string, - authFetch: AuthenticatedFetch, + authFetch: AuthenticatedFetch | undefined, ): Record | undefined { + if (!authFetch) return undefined; + // Never hand a stored token to a cleartext (non-loopback http) remote. + try { + if (!isHttpsOrLoopback(new URL(url))) return undefined; + } catch { + return undefined; + } if (!authFetch.hasAuth(url)) return undefined; const token = authFetch.getTokenForUrl(url); if (!token) return undefined; @@ -153,7 +142,7 @@ async function assertMirrorOrigin(mirrorDir: string, url: string): Promise export async function ensureMirrorClone( platform: CachePlatform, repoPath: string, - authFetch: AuthenticatedFetch, + authFetch: AuthenticatedFetch | undefined, repoUrl?: string, ): Promise { validateRepoPath(repoPath); diff --git a/src/shared/cache/snapshot.ts b/src/shared/cache/snapshot.ts index 1da6c8dc..84b138a9 100644 --- a/src/shared/cache/snapshot.ts +++ b/src/shared/cache/snapshot.ts @@ -91,7 +91,8 @@ export async function materializeSnapshot( export interface GetSnapshotOptions extends ResolveOptions { platform: CachePlatform; repoPath: string; - authFetch: AuthenticatedFetch; + /** Omit to clone with no capa-stored git-host token. */ + authFetch?: AuthenticatedFetch; /** When true, ignore any existing snapshot/mirror state and re-resolve from network. */ noCache?: boolean; /** Override the clone URL instead of deriving it from platform/repoPath (useful for tests). */ diff --git a/src/shared/executable-surface.ts b/src/shared/executable-surface.ts index 9f4d894f..6ad73660 100644 --- a/src/shared/executable-surface.ts +++ b/src/shared/executable-surface.ts @@ -5,6 +5,7 @@ import type { Capabilities, SecretValue } from '../types/capabilities'; import type { Hook } from '../types/hooks'; import type { Plugin } from '../types/plugin'; import { getCapaDir } from './config'; +import { stripHttpUrlUserinfo } from './git-url'; import { secretValueMapForFingerprint } from './secret-value'; export interface SurfaceServer { @@ -138,7 +139,7 @@ export function collectExecutableSurface(capabilities: Capabilities): Executable const plugins: SurfacePlugin[] = []; for (const plugin of (capabilities.plugins ?? []) as Plugin[]) { // git plugins (any host) have a clone URL instead of an owner/repo. - const repo = plugin.def.repo ?? plugin.def.url ?? ''; + const repo = plugin.def.repo ?? stripHttpUrlUserinfo(plugin.def.url ?? ''); const id = plugin.id ?? repo; const entry: SurfacePlugin = { id, diff --git a/src/shared/git-url.ts b/src/shared/git-url.ts index 0d8d775d..fdf2cce2 100644 --- a/src/shared/git-url.ts +++ b/src/shared/git-url.ts @@ -47,6 +47,42 @@ export function isGitUrl(source: string): boolean { } const CLONE_PROTOCOLS = new Set(["https:", "http:", "ssh:", "file:"]); +const LOOPBACK_HOSTS = new Set(["localhost", "127.0.0.1", "[::1]", "::1"]); + +/** + * https, or plain http to this machine only. Anything else would send a + * token (or the clone itself) in cleartext over the network. + */ +export function isHttpsOrLoopback(url: URL): boolean { + return ( + url.protocol === "https:" || + (url.protocol === "http:" && LOOPBACK_HOSTS.has(url.hostname)) + ); +} + +/** Strip userinfo from an http(s) URL. Returns null when there is none or the value is not an http(s) URL. */ +export function stripUserinfoFromHttpUrl(raw: string): string | null { + const trimmed = raw.trim().replace(/^["']|["']$/g, ""); + try { + const u = new URL(trimmed); + if (u.protocol !== "http:" && u.protocol !== "https:") return null; + if (!u.username && !u.password) return null; + u.username = ""; + u.password = ""; + let out = u.toString(); + if (!trimmed.endsWith("/") && out.endsWith("/")) { + out = out.slice(0, -1); + } + return out; + } catch { + return null; + } +} + +/** `url` without `user:token@`, for anything stored (lockfile) or printed. */ +export function stripHttpUrlUserinfo(url: string): string { + return stripUserinfoFromHttpUrl(url) ?? url; +} /** * Cache key for a clone URL: `/-`, e.g. @@ -65,6 +101,11 @@ export function gitRepoKey(url: string): string { if (!CLONE_PROTOCOLS.has(u.protocol)) { throw new Error(`Unsupported git URL scheme "${u.protocol}" in ${url}`); } + if (!isHttpsOrLoopback(u) && u.protocol === "http:") { + throw new Error( + `Plain http git URLs are only allowed for localhost, use https: ${stripHttpUrlUserinfo(url)}`, + ); + } u.username = ""; u.password = ""; const hash = createHash("sha256").update(u.href).digest("hex").slice(0, 12); diff --git a/src/shared/plugin-source.ts b/src/shared/plugin-source.ts index 40e003ba..292574ad 100644 --- a/src/shared/plugin-source.ts +++ b/src/shared/plugin-source.ts @@ -7,7 +7,7 @@ import type { Plugin } from "../types/plugin"; import type { CachePlatform } from "./cache"; -import { gitRepoKey } from "./git-url"; +import { gitRepoKey, stripHttpUrlUserinfo } from "./git-url"; const SEGMENT_RE = /^[^/:#]+$/; const FORBIDDEN_SEGMENTS = new Set([".", ".."]); @@ -186,8 +186,12 @@ function validateGitPluginDef( let repoPath: string; try { repoPath = gitRepoKey(def.url); - } catch { - return { error: `Invalid git clone URL: "${def.url}"` }; + } catch (err) { + // gitRepoKey explains scheme problems; `new URL` failures are TypeErrors. + const why = err instanceof TypeError ? "" : ` (${(err as Error).message})`; + return { + error: `Invalid git clone URL: "${stripHttpUrlUserinfo(def.url)}"${why}`, + }; } const subpath = def.subpath ?? ""; if (subpath !== "" && !isValidPath(subpath, 1)) { @@ -201,7 +205,9 @@ function validateGitPluginDef( subpath, version: def.version, ref: def.ref, - repoUrl: def.url, + // Credentials in the URL are never used for the clone; keep them out + // of the lockfile and messages too. + repoUrl: stripHttpUrlUserinfo(def.url), }; }