diff --git a/packages/docs/public/llms.txt b/packages/docs/public/llms.txt index aa87a9ec..f837ea85 100644 --- a/packages/docs/public/llms.txt +++ b/packages/docs/public/llms.txt @@ -192,7 +192,7 @@ Skills define what Warden analyzes and when. | `ignorePaths` | Files to exclude (glob patterns) | | `failOn` | Minimum severity to fail: `critical`, `high`, `medium`, `low`, `info`, `off` | | `reportOn` | Minimum severity to report | -| `remote` | GitHub repository for remote skills: `owner/repo` or `owner/repo@sha` | +| `remote` | GitHub repository for remote skills: `owner/repo` or `owner/repo@sha`. Hosted private remotes use `github-token` auth on `github.com`. | | `model` | Model override | | `maxTurns` | Max agentic turns per hunk | @@ -473,7 +473,7 @@ jobs: | Input | Default | Description | |-------|---------|-------------| -| `github-token` | `GITHUB_TOKEN` | GitHub token for posting comments | +| `github-token` | `GITHUB_TOKEN` | GitHub token for posting comments and private remote-skill fetch auth in hosted runs | | `anthropic-api-key` | - | Anthropic API key (falls back to `WARDEN_ANTHROPIC_API_KEY`) | | `config-path` | `warden.toml` | Path to config file | | `fail-on` | - | Minimum severity to fail the check | diff --git a/packages/docs/src/pages/config.astro b/packages/docs/src/pages/config.astro index 7a52016e..e73c889d 100644 --- a/packages/docs/src/pages/config.astro +++ b/packages/docs/src/pages/config.astro @@ -41,7 +41,10 @@ actions = ["opened", "synchronize"]`}
reportOn
Minimum severity to report
remote
-
GitHub repository for remote skills: owner/repo or owner/repo@sha
+
+ GitHub repository for remote skills: owner/repo or owner/repo@sha. + In hosted GitHub Actions runs, private remotes use github-token auth on github.com. +
model
Model override (optional)
maxTurns
@@ -426,7 +429,7 @@ jobs:
github-token
-
GitHub token for posting comments. Default: GITHUB_TOKEN
+
GitHub token for posting comments and private remote-skill fetch auth in hosted runs. Default: GITHUB_TOKEN
anthropic-api-key
Anthropic API key (falls back to WARDEN_ANTHROPIC_API_KEY)
config-path
@@ -444,4 +447,13 @@ jobs:
parallel
Maximum concurrent trigger executions. Default: 5
+ +

Private Remote Troubleshooting

+ + diff --git a/specs/private-remote-auth-rollout.md b/specs/private-remote-auth-rollout.md new file mode 100644 index 00000000..0455a0cf --- /dev/null +++ b/specs/private-remote-auth-rollout.md @@ -0,0 +1,43 @@ +# Private Remote Auth Rollout Notes + +## What Changed +- Hosted GitHub Action runs now pass `github-token` into remote skill resolution. +- Remote fetches can authenticate private `github.com` remotes using per-command git auth env injection. +- For authenticated GitHub fetches, runtime transport uses HTTPS even if config used SSH remote syntax. + +## Known Limitations +- Authenticated private-remote fetch support is scoped to `github.com`. +- GitHub token must have repository read access and, for GitHub Apps, installation access to the remote skill repo. +- CLI token fallback (`WARDEN_GITHUB_TOKEN`) is not part of this MVP. + +## Operator Runbook +1. Confirm token source: +- Action: `github-token` input (defaults to `GITHUB_TOKEN`) +- Ensure token is non-empty and has read access to the remote repo. + +2. Confirm repository access model: +- For GitHub App tokens: app must be installed on both code repo and remote skill repo. +- For PATs: token owner must have read access to remote skill repo. + +3. Confirm remote host: +- MVP auth path supports `github.com` remotes. +- Non-`github.com` remotes follow existing unauthenticated behavior. + +4. Failure interpretation: +- `Failed to authenticate when cloning owner/repo` indicates token access/scope/installation issue. +- `Failed to clone ... via HTTPS` in unauthenticated flow indicates missing credentials. + +## Verification Summary +Executed: +- `corepack pnpm lint` ✅ +- `corepack pnpm build` ✅ +- Targeted auth and action tests ✅ + - `corepack pnpm test src/skills/remote-auth.test.ts src/skills/remote.test.ts src/action/triggers/executor.test.ts src/action/workflow/schedule.test.ts` +- Secret-safety sweep ✅ + - `rg -n "ghp_|github_pat_|Authorization:\s*Bearer|x-access-token" src packages/docs agent-docs -S` + +Full suite status: +- `corepack pnpm test` currently fails due pre-existing unrelated tests: + - `src/action/inputs.test.ts` (`setupAuthEnv` OAuth env expectation) + - `src/cli/output/tty.test.ts` (`FORCE_COLOR` expectation) + - `src/action/fix-evaluation/judge.test.ts` (live judge fallback assertions) diff --git a/src/action/triggers/executor.test.ts b/src/action/triggers/executor.test.ts index b262f9f4..1318e5e4 100644 --- a/src/action/triggers/executor.test.ts +++ b/src/action/triggers/executor.test.ts @@ -31,6 +31,7 @@ vi.mock('../../output/renderer.js', () => ({ import { runSkillTask } from '../../cli/output/tasks.js'; import { createSkillCheck, updateSkillCheck, failSkillCheck } from '../../output/github-checks.js'; import { renderSkillReport } from '../../output/renderer.js'; +import { resolveSkillAsync } from '../../skills/loader.js'; describe('executeTrigger', () => { // Suppress console output during tests @@ -75,6 +76,7 @@ describe('executeTrigger', () => { context: mockContext, config: mockConfig, anthropicApiKey: 'test-key', + githubToken: 'gh-token', claudePath: '/test/claude', globalMaxFindings: 10, }; @@ -102,7 +104,12 @@ describe('executeTrigger', () => { vi.mocked(updateSkillCheck).mockResolvedValue(undefined); vi.mocked(renderSkillReport).mockReturnValue(mockRenderResult); - const result = await executeTrigger(mockTrigger, mockDeps); + const triggerWithRemote: ResolvedTrigger = { + ...mockTrigger, + remote: 'owner/repo', + }; + + const result = await executeTrigger(triggerWithRemote, mockDeps); expect(result.triggerName).toBe('test-trigger'); expect(result.report).toBe(mockReport); @@ -122,6 +129,14 @@ describe('executeTrigger', () => { minConfidence: 'medium', failCheck: undefined, }); + + const taskOptions = vi.mocked(runSkillTask).mock.calls[0]?.[0]; + expect(taskOptions).toBeDefined(); + await taskOptions?.resolveSkill(); + expect(resolveSkillAsync).toHaveBeenCalledWith('test-skill', '/test/path', { + remote: 'owner/repo', + githubToken: 'gh-token', + }); }); it('executes a trigger successfully with no findings', async () => { diff --git a/src/action/triggers/executor.ts b/src/action/triggers/executor.ts index 42c77e46..90a19761 100644 --- a/src/action/triggers/executor.ts +++ b/src/action/triggers/executor.ts @@ -44,6 +44,7 @@ export interface TriggerExecutorDeps { context: EventContext; config: WardenConfig; anthropicApiKey: string; + githubToken?: string; claudePath: string; /** Global fail-on from action inputs (trigger-specific takes precedence) */ globalFailOn?: SeverityThreshold; @@ -131,6 +132,7 @@ export async function executeTrigger( failOn, resolveSkill: () => resolveSkillAsync(trigger.skill, context.repoPath, { remote: trigger.remote, + githubToken: deps.githubToken, }), context: filterContextByPaths(context, trigger.filters), runnerOptions: { diff --git a/src/action/workflow/pr-workflow.ts b/src/action/workflow/pr-workflow.ts index df68ec79..0eb906ed 100644 --- a/src/action/workflow/pr-workflow.ts +++ b/src/action/workflow/pr-workflow.ts @@ -255,6 +255,7 @@ async function executeAllTriggers( context, config, anthropicApiKey: inputs.anthropicApiKey, + githubToken: inputs.githubToken, claudePath, globalFailOn: inputs.failOn, globalReportOn: inputs.reportOn, @@ -403,7 +404,7 @@ async function evaluateFixesAndResolveStale( logAction(`Resolved ${resolvedCount} comments via fix evaluation`); } // Track only actually resolved comments for allResolved check - resolvedIds.forEach((id) => commentsResolvedByFixEval.add(id)); + for (const id of resolvedIds) commentsResolvedByFixEval.add(id); } // Post replies for failed fixes and track them so stale pass doesn't override @@ -467,7 +468,7 @@ async function evaluateFixesAndResolveStale( emitStaleResolutionMetric(count, skill); } } - resolvedIds.forEach((id) => commentsResolvedByStale.add(id)); + for (const id of resolvedIds) commentsResolvedByStale.add(id); } } catch (error) { Sentry.captureException(error, { tags: { operation: 'resolve_stale_comments' } }); diff --git a/src/action/workflow/schedule.test.ts b/src/action/workflow/schedule.test.ts index c697212f..7e80c6bc 100644 --- a/src/action/workflow/schedule.test.ts +++ b/src/action/workflow/schedule.test.ts @@ -277,6 +277,10 @@ describe('runScheduleWorkflow', () => { await runScheduleWorkflow(mockOctokit, createDefaultInputs(), SCHEDULE_FIXTURES); + expect(mockResolveSkillAsync).toHaveBeenCalledWith('test-skill', SCHEDULE_FIXTURES, { + remote: undefined, + githubToken: 'test-github-token', + }); expect(mockRunSkill).toHaveBeenCalledTimes(1); expect(mockCreateOrUpdateIssue).toHaveBeenCalledWith( mockOctokit, diff --git a/src/action/workflow/schedule.ts b/src/action/workflow/schedule.ts index 361b0ee7..49b1c0ea 100644 --- a/src/action/workflow/schedule.ts +++ b/src/action/workflow/schedule.ts @@ -113,6 +113,7 @@ export async function runScheduleWorkflow( // Run skill const skill = await resolveSkillAsync(resolved.skill, repoPath, { remote: resolved.remote, + githubToken: inputs.githubToken, }); const claudePath = await findClaudeCodeExecutable(); const report = await runSkill(skill, context, { diff --git a/src/skills/auth-options.ts b/src/skills/auth-options.ts new file mode 100644 index 00000000..dcda4488 --- /dev/null +++ b/src/skills/auth-options.ts @@ -0,0 +1,15 @@ +/** + * Optional authentication options for remote skill/agent fetches. + */ +export interface RemoteAuthOptions { + /** + * GitHub token for authenticating private remote skill/agent fetches. + * + * Accepts: PAT (classic/fine-grained), GitHub App token, or GITHUB_TOKEN. + * Must have repository read access to the remote skill repo. + * For GitHub Apps, the app must be installed on the remote repo. + * + * Whitespace-only values are treated as unset (useful for CI env vars). + */ + githubToken?: string; +} diff --git a/src/skills/index.ts b/src/skills/index.ts index 5fea4746..e6557be2 100644 --- a/src/skills/index.ts +++ b/src/skills/index.ts @@ -22,6 +22,8 @@ export type { ResolveSkillOptions, } from './loader.js'; +export type { RemoteAuthOptions } from './auth-options.js'; + export { parseRemoteRef, formatRemoteRef, diff --git a/src/skills/loader.test.ts b/src/skills/loader.test.ts index fe454a8a..9eac0d2f 100644 --- a/src/skills/loader.test.ts +++ b/src/skills/loader.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterAll } from 'vitest'; +import { describe, it, expect, beforeEach, afterAll, vi } from 'vitest'; import { join } from 'node:path'; import { homedir } from 'node:os'; import { writeFileSync, unlinkSync, mkdtempSync, mkdirSync, rmSync } from 'node:fs'; @@ -18,6 +18,13 @@ import { AGENT_MARKER_FILE, } from './loader.js'; +vi.mock('./remote.js', () => ({ + resolveRemoteSkill: vi.fn(), + resolveRemoteAgent: vi.fn(), +})); + +import { resolveRemoteSkill, resolveRemoteAgent } from './remote.js'; + describe('loadSkillFromFile', () => { it('rejects unsupported file types', async () => { await expect(loadSkillFromFile('/path/to/skill.json')).rejects.toThrow(SkillLoaderError); @@ -41,6 +48,46 @@ describe('resolveSkillAsync', () => { await expect(resolveSkillAsync('nonexistent-skill')).rejects.toThrow(SkillLoaderError); await expect(resolveSkillAsync('nonexistent-skill')).rejects.toThrow('Skill not found'); }); + + it('forwards githubToken and offline to remote skill resolution', async () => { + vi.mocked(resolveRemoteSkill).mockResolvedValue({ + name: 'remote-skill', + description: 'from remote', + prompt: 'prompt', + }); + + await resolveSkillAsync('remote-skill', '/tmp/repo', { + remote: 'owner/repo', + offline: true, + githubToken: 'test-token', + }); + + expect(resolveRemoteSkill).toHaveBeenCalledWith('owner/repo', 'remote-skill', { + offline: true, + githubToken: 'test-token', + }); + }); +}); + +describe('resolveAgentAsync', () => { + it('forwards githubToken and offline to remote agent resolution', async () => { + vi.mocked(resolveRemoteAgent).mockResolvedValue({ + name: 'remote-agent', + description: 'from remote', + prompt: 'prompt', + }); + + await resolveAgentAsync('remote-agent', '/tmp/repo', { + remote: 'owner/repo', + offline: true, + githubToken: 'test-token', + }); + + expect(resolveRemoteAgent).toHaveBeenCalledWith('owner/repo', 'remote-agent', { + offline: true, + githubToken: 'test-token', + }); + }); }); describe('skills caching', () => { diff --git a/src/skills/loader.ts b/src/skills/loader.ts index 2bd342f1..8658b231 100644 --- a/src/skills/loader.ts +++ b/src/skills/loader.ts @@ -4,6 +4,7 @@ import { existsSync } from 'node:fs'; import { homedir } from 'node:os'; import type { SkillDefinition, ToolName } from '../config/schema.js'; import { ToolNameSchema } from '../config/schema.js'; +import type { RemoteAuthOptions } from './auth-options.js'; export class SkillLoaderError extends Error { constructor(message: string, options?: { cause?: unknown }) { @@ -123,7 +124,7 @@ function parseMarkdownFrontmatter(content: string): { frontmatter: Record { - const { remote, offline } = options ?? {}; + const { remote, offline, githubToken } = options ?? {}; // 1. Remote repository resolution takes priority when specified if (remote) { // Dynamic import to avoid circular dependencies const { resolveRemoteSkill, resolveRemoteAgent } = await import('./remote.js'); const resolver = config.kind === 'skill' ? resolveRemoteSkill : resolveRemoteAgent; - return resolver(remote, nameOrPath, { offline }); + return resolver(remote, nameOrPath, { offline, githubToken }); } // 2. Direct path resolution diff --git a/src/skills/remote-auth.test.ts b/src/skills/remote-auth.test.ts new file mode 100644 index 00000000..1091ae68 --- /dev/null +++ b/src/skills/remote-auth.test.ts @@ -0,0 +1,157 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { join } from 'node:path'; +import { mkdirSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; + +vi.mock('../utils/exec.js', () => ({ + execGitNonInteractive: vi.fn(), +})); + +import { execGitNonInteractive } from '../utils/exec.js'; +import { fetchRemote, getRemotePath, saveState } from './remote.js'; +import { SkillLoaderError } from './loader.js'; + +describe('fetchRemote auth behavior', () => { + const originalStateDir = process.env['WARDEN_STATE_DIR']; + let stateDir: string; + + beforeEach(() => { + stateDir = join(tmpdir(), `warden-remote-auth-${Date.now()}-${Math.random().toString(36).slice(2)}`); + process.env['WARDEN_STATE_DIR'] = stateDir; + mkdirSync(stateDir, { recursive: true }); + + vi.mocked(execGitNonInteractive).mockImplementation((args: string[]) => { + if (args[0] === 'clone') { + const targetDir = args[args.length - 1]; + if (typeof targetDir === 'string') { + mkdirSync(targetDir, { recursive: true }); + } + return ''; + } + if (args[0] === 'rev-parse') return 'deadbeef'; + return ''; + }); + }); + + afterEach(() => { + if (originalStateDir === undefined) { + delete process.env['WARDEN_STATE_DIR']; + } else { + process.env['WARDEN_STATE_DIR'] = originalStateDir; + } + rmSync(stateDir, { recursive: true, force: true }); + vi.clearAllMocks(); + }); + + it('uses runtime HTTPS clone URL and auth env for GitHub SSH remotes when token is set', async () => { + await fetchRemote('git@github.com:owner/repo.git', { githubToken: 'test-token' }); + + const cloneCall = vi.mocked(execGitNonInteractive).mock.calls.find((call) => call[0][0] === 'clone'); + expect(cloneCall).toBeDefined(); + expect(cloneCall?.[0]).toEqual(['clone', '--depth=1', '--', 'https://github.com/owner/repo.git', expect.any(String)]); + + const cloneOptions = cloneCall?.[1]; + expect(cloneOptions?.env?.['GIT_CONFIG_COUNT']).toBe('1'); + expect(cloneOptions?.env?.['GIT_CONFIG_KEY_0']).toBe('http.https://github.com/.extraheader'); + expect(cloneOptions?.env?.['GIT_CONFIG_VALUE_0']).toContain('AUTHORIZATION: basic '); + expect(cloneCall?.[0].join(' ')).not.toContain('test-token'); + }); + + it('treats whitespace-only tokens as unset', async () => { + await fetchRemote('git@github.com:owner/repo.git', { githubToken: ' ' }); + + const cloneCall = vi.mocked(execGitNonInteractive).mock.calls.find((call) => call[0][0] === 'clone'); + expect(cloneCall).toBeDefined(); + expect(cloneCall?.[0]).toEqual(['clone', '--depth=1', '--', 'git@github.com:owner/repo.git', expect.any(String)]); + + const cloneOptions = cloneCall?.[1]; + expect(cloneOptions?.env?.['GIT_CONFIG_COUNT']).toBeUndefined(); + expect(cloneOptions?.env?.['GIT_CONFIG_KEY_0']).toBeUndefined(); + expect(cloneOptions?.env?.['GIT_CONFIG_VALUE_0']).toBeUndefined(); + }); + + it('does not apply github auth env for persisted non-github clone URLs', async () => { + const remotePath = getRemotePath('owner/repo'); + mkdirSync(remotePath, { recursive: true }); + saveState({ + remotes: { + 'owner/repo': { + sha: 'abc123', + fetchedAt: new Date().toISOString(), + cloneUrl: 'https://example.com/owner/repo.git', + }, + }, + }); + + await fetchRemote('owner/repo', { githubToken: 'test-token', force: true }); + + const fetchCall = vi.mocked(execGitNonInteractive).mock.calls.find((call) => call[0][0] === 'fetch'); + expect(fetchCall).toBeDefined(); + expect(fetchCall?.[0]).toEqual(['fetch', '--depth=1', '--', 'https://example.com/owner/repo.git']); + const fetchOptions = fetchCall?.[1]; + expect(fetchOptions?.env?.['GIT_CONFIG_COUNT']).toBeUndefined(); + }); + + it('sanitizes token from auth-failure error messages', async () => { + vi.mocked(execGitNonInteractive).mockImplementation((args: string[]) => { + if (args[0] === 'clone') { + throw new Error('authentication failed for test-token'); + } + return ''; + }); + + await expect(fetchRemote('owner/repo', { githubToken: 'test-token' })) + .rejects.toThrow('Failed to authenticate when cloning owner/repo'); + await expect(fetchRemote('owner/repo', { githubToken: 'test-token' })) + .rejects.not.toThrow('test-token'); + }); + + it('preserves original auth failure as cause for tokenized fetches', async () => { + vi.mocked(execGitNonInteractive).mockImplementation((args: string[]) => { + if (args[0] === 'clone') { + throw new Error('fatal: authentication failed'); + } + return ''; + }); + + try { + await fetchRemote('owner/repo', { githubToken: 'test-token' }); + throw new Error('expected fetchRemote to throw'); + } catch (error) { + expect(error).toBeInstanceOf(SkillLoaderError); + expect((error as Error).cause).toBeInstanceOf(Error); + expect(((error as Error).cause as Error).message).toContain('authentication failed'); + } + }); + + it('preserves original HTTPS prompt failure as cause for unauthenticated shorthand refs', async () => { + vi.mocked(execGitNonInteractive).mockImplementation((args: string[]) => { + if (args[0] === 'clone') { + throw new Error('fatal: could not read Username for \'https://github.com\': terminal prompts disabled'); + } + return ''; + }); + + try { + await fetchRemote('owner/repo'); + throw new Error('expected fetchRemote to throw'); + } catch (error) { + expect(error).toBeInstanceOf(SkillLoaderError); + expect((error as Error).cause).toBeInstanceOf(Error); + expect(((error as Error).cause as Error).message).toContain('terminal prompts disabled'); + } + }); + + it('keeps per-command auth env isolated across concurrent fetches', async () => { + await Promise.all([ + fetchRemote('owner/repo-a', { githubToken: 'token-a' }), + fetchRemote('owner/repo-b', { githubToken: 'token-b' }), + ]); + + const cloneCalls = vi.mocked(execGitNonInteractive).mock.calls.filter((call) => call[0][0] === 'clone'); + expect(cloneCalls).toHaveLength(2); + const headers = cloneCalls.map((call) => call[1]?.env?.['GIT_CONFIG_VALUE_0']); + expect(headers[0]).not.toBe(headers[1]); + expect(headers.every((h) => typeof h === 'string' && h.startsWith('AUTHORIZATION: basic '))).toBe(true); + }); +}); diff --git a/src/skills/remote.test.ts b/src/skills/remote.test.ts index 883d5f7b..fa0ae07c 100644 --- a/src/skills/remote.test.ts +++ b/src/skills/remote.test.ts @@ -667,6 +667,23 @@ describe('discoverRemoteSkills', () => { expect(skills[0]?.name).toBe('good-skill'); expect(skills[0]?.pluginName).toBe('legit'); }); + + it('ignores plugins with absolute source paths', async () => { + const remotePath = getRemotePath('test/repo'); + createFileTree(remotePath, { + '.claude-plugin/marketplace.json': marketplaceJson([ + { name: 'absolute', source: '/tmp/absolute-plugin' }, + { name: 'legit', source: './plugins/legit' }, + ]), + 'plugins/legit/skills/good-skill/SKILL.md': skillMd('good-skill', 'Legit skill'), + }); + + const skills = await discoverRemoteSkills('test/repo'); + + expect(skills.length).toBe(1); + expect(skills[0]?.name).toBe('good-skill'); + expect(skills[0]?.pluginName).toBe('legit'); + }); }); }); diff --git a/src/skills/remote.ts b/src/skills/remote.ts index 5ac73568..acd5b8f0 100644 --- a/src/skills/remote.ts +++ b/src/skills/remote.ts @@ -1,9 +1,10 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync, renameSync, readdirSync, statSync } from 'node:fs'; import { homedir } from 'node:os'; -import { dirname, join, resolve } from 'node:path'; +import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { z } from 'zod'; import { execGitNonInteractive } from '../utils/exec.js'; import { loadSkillFromMarkdown, SkillLoaderError, AGENT_MARKER_FILE } from './loader.js'; +import type { RemoteAuthOptions } from './auth-options.js'; import type { SkillDefinition } from '../config/schema.js'; /** Default TTL for unpinned remote skills: 24 hours */ @@ -257,8 +258,8 @@ export function saveState(state: RemoteState): void { export function getCacheTtlSeconds(): number { const envTtl = process.env['WARDEN_SKILL_CACHE_TTL']; if (envTtl) { - const parsed = parseInt(envTtl, 10); - if (!isNaN(parsed) && parsed > 0) { + const parsed = Number.parseInt(envTtl, 10); + if (!Number.isNaN(parsed) && parsed > 0) { return parsed; } } @@ -289,7 +290,7 @@ export function shouldRefresh(ref: string, state: RemoteState): boolean { return now - fetchedAt > ttl; } -export interface FetchRemoteOptions { +export interface FetchRemoteOptions extends RemoteAuthOptions { /** Force refresh even if cache is valid */ force?: boolean; /** Skip network operations - only use cache */ @@ -298,14 +299,68 @@ export interface FetchRemoteOptions { onProgress?: (message: string) => void; } +/** + * Normalize token input from env/action inputs. + * Empty-string values in CI should behave as "unset". + */ +function normalizeToken(token?: string): string | undefined { + const normalized = token?.trim(); + return normalized ? normalized : undefined; +} + +/** + * Build one-shot git auth environment for GitHub HTTPS requests. + * Uses a transient extraheader, avoiding tokenized URLs or persisted git config. + */ +function buildGitHubAuthEnv(token: string): Record { + const basic = Buffer.from(`x-access-token:${token}`).toString('base64'); + return { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.https://github.com/.extraheader', + GIT_CONFIG_VALUE_0: `AUTHORIZATION: basic ${basic}`, + }; +} + +/** + * Check whether this remote points to github.com. + */ +function isGitHubRemote(parsed: ParsedRemoteRef, cloneUrl?: string): boolean { + // owner/repo shorthand is GitHub unless an explicit non-GitHub URL is persisted in state. + const effectiveUrl = cloneUrl ?? parsed.cloneUrl; + if (!effectiveUrl) return true; + return normalizeGitHubUrl(effectiveUrl) !== null; +} + +/** + * Runtime clone URL for authenticated GitHub fetches. + * Deliberately separate from stored cloneUrl/state. + */ +function getGitHubHttpsUrl(parsed: ParsedRemoteRef): string { + return `https://github.com/${parsed.owner}/${parsed.repo}.git`; +} + +function isGitAuthFailure(message: string): boolean { + const lower = message.toLowerCase(); + return ( + lower.includes('authentication failed') || + lower.includes('could not read username') || + lower.includes('terminal prompts disabled') || + lower.includes('repository not found') || + lower.includes('http basic: access denied') || + lower.includes('permission denied') || + lower.includes('access denied') || + lower.includes('403') + ); +} + /** * Execute a git command and return stdout. * Uses non-interactive mode to prevent SSH passphrase prompts. * Throws SkillLoaderError on failure. */ -function execGit(args: string[], options?: { cwd?: string }): string { +function execGit(args: string[], options?: { cwd?: string; env?: Record }): string { try { - return execGitNonInteractive(args, { cwd: options?.cwd }); + return execGitNonInteractive(args, { cwd: options?.cwd, env: options?.env }); } catch (error) { const message = error instanceof Error ? error.message : String(error); throw new SkillLoaderError(`Git command failed: git ${args.join(' ')}: ${message}`, { cause: error }); @@ -351,8 +406,15 @@ export async function fetchRemote(ref: string, options: FetchRemoteOptions = {}) return stateEntry.sha; } - // Use the original clone URL if provided, fall back to stored URL from state, then HTTPS - const repoUrl = parsed.cloneUrl ?? stateEntry?.cloneUrl ?? `https://github.com/${parsed.owner}/${parsed.repo}.git`; + const sourceCloneUrl = parsed.cloneUrl ?? stateEntry?.cloneUrl; + const token = normalizeToken(options.githubToken); + const useGitHubAuth = !!token && isGitHubRemote(parsed, sourceCloneUrl); + const gitAuthEnv = token && useGitHubAuth ? buildGitHubAuthEnv(token) : undefined; + + // Use runtime HTTPS for authenticated GitHub fetches. Otherwise preserve existing URL semantics. + const repoUrl = useGitHubAuth + ? getGitHubHttpsUrl(parsed) + : (sourceCloneUrl ?? `https://github.com/${parsed.owner}/${parsed.repo}.git`); // Clone or update if (!isCached) { @@ -369,28 +431,35 @@ export async function fetchRemote(ref: string, options: FetchRemoteOptions = {}) const { sha } = parsed; if (sha) { // For pinned refs, shallow clone then checkout the specific SHA - execGit(['clone', '--depth=1', '--', repoUrl, remotePath]); + execGit(['clone', '--depth=1', '--', repoUrl, remotePath], { env: gitAuthEnv }); try { // Try to fetch the pinned SHA directly - execGit(['fetch', '--depth=1', 'origin', '--', sha], { cwd: remotePath }); + execGit(['fetch', '--depth=1', 'origin', '--', sha], { cwd: remotePath, env: gitAuthEnv }); execGit(['checkout', sha], { cwd: remotePath }); } catch { // If SHA not found in shallow history, do a full fetch and retry - execGit(['fetch', '--unshallow'], { cwd: remotePath }); + execGit(['fetch', '--unshallow'], { cwd: remotePath, env: gitAuthEnv }); execGit(['checkout', sha], { cwd: remotePath }); } } else { // For unpinned refs, shallow clone of default branch - execGit(['clone', '--depth=1', '--', repoUrl, remotePath]); + execGit(['clone', '--depth=1', '--', repoUrl, remotePath], { env: gitAuthEnv }); } } catch (error) { const message = error instanceof Error ? error.message : String(error); - // Detect HTTPS auth failures and suggest SSH URL - if (!parsed.cloneUrl && (message.includes('terminal prompts disabled') || message.includes('could not read Username'))) { + if (token && isGitAuthFailure(message)) { + throw new SkillLoaderError( + `Failed to authenticate when cloning ${stateKey}. ` + + `Ensure the provided GitHub token has read access to ${parsed.owner}/${parsed.repo}.`, + { cause: error } + ); + } + // Unauthenticated shorthand HTTPS failure: provide explicit auth guidance. + if (!token && !parsed.cloneUrl && (message.includes('terminal prompts disabled') || message.includes('could not read Username'))) { throw new SkillLoaderError( - `Failed to clone ${stateKey} via HTTPS. ` + - `For private repos, use the SSH URL: warden add --remote git@github.com:${parsed.owner}/${parsed.repo}.git` + `Failed to clone ${stateKey} via HTTPS. For private repos, provide a GitHub token (GITHUB_TOKEN or WARDEN_GITHUB_TOKEN) or use the SSH URL: warden add --remote git@github.com:${parsed.owner}/${parsed.repo}.git`, + { cause: error } ); } throw error; @@ -400,9 +469,14 @@ export async function fetchRemote(ref: string, options: FetchRemoteOptions = {}) onProgress?.(`Updating ${ref}...`); if (!isPinned) { - // For unpinned refs, pull latest - execGit(['fetch', '--depth=1', 'origin'], { cwd: remotePath }); - execGit(['reset', '--hard', 'origin/HEAD'], { cwd: remotePath }); + // For unpinned refs, pull latest from the explicit remote URL so cached SSH remotes + // keep their transport semantics across refreshes. + if (useGitHubAuth) { + execGit(['fetch', '--depth=1', '--', repoUrl], { cwd: remotePath, env: gitAuthEnv }); + } else { + execGit(['fetch', '--depth=1', '--', repoUrl], { cwd: remotePath }); + } + execGit(['reset', '--hard', 'FETCH_HEAD'], { cwd: remotePath }); } // Pinned refs don't need updates - SHA is immutable } @@ -537,7 +611,8 @@ async function discoverMarketplaceSkills( // Security: Ensure plugin source doesn't escape the repo directory via path traversal const resolvedSkillsPath = resolve(skillsPath); const resolvedRemotePath = resolve(remotePath); - if (!resolvedSkillsPath.startsWith(resolvedRemotePath + '/')) { + const relativePath = relative(resolvedRemotePath, resolvedSkillsPath); + if (relativePath === '..' || relativePath.startsWith('..') || isAbsolute(relativePath)) { continue; // Silently skip — attacker-controlled marketplace.json, don't leak info }