diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6beabbf465..4cc7925f18 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ on: jobs: test: runs-on: ubuntu-latest - + steps: - name: Checkout repository uses: actions/checkout@v6 @@ -147,3 +147,28 @@ jobs: npx -y --package $($Package.FullName) firecrawl login --help npx -y --package $($Package.FullName) firecrawl setup --help npx -y --package $($Package.FullName) firecrawl help setup + + - name: Verify secure MCP setup through Windows npx.cmd + if: runner.os == 'Windows' + shell: pwsh + run: | + $Package = Get-ChildItem .\firecrawl-cli-*.tgz | Select-Object -First 1 + $TempHome = Join-Path $env:RUNNER_TEMP "firecrawl-mcp-home" + New-Item -ItemType Directory -Force -Path $TempHome | Out-Null + $env:HOME = $TempHome + $env:USERPROFILE = $TempHome + $env:FIRECRAWL_API_KEY = "fc-windows-smoke-secret" + + npx -y --package $($Package.FullName) firecrawl setup mcp --agent claude-code --global --yes + + $ConfigPath = Join-Path $TempHome ".claude.json" + if (-not (Test-Path $ConfigPath)) { + throw "Claude Code MCP config was not created" + } + $Config = Get-Content $ConfigPath -Raw + if ($Config.Contains("fc-windows-smoke-secret")) { + throw "MCP setup persisted the raw environment-backed API key" + } + if (-not $Config.Contains('${FIRECRAWL_API_KEY}')) { + throw "MCP setup did not preserve Claude Code environment expansion" + } diff --git a/src/__tests__/commands/setup.test.ts b/src/__tests__/commands/setup.test.ts index 6c989be738..128c8d9a82 100644 --- a/src/__tests__/commands/setup.test.ts +++ b/src/__tests__/commands/setup.test.ts @@ -1,6 +1,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { execSync } from 'child_process'; -import { mkdtempSync, readFileSync, rmSync } from 'fs'; +import { execFileSync, execSync } from 'child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from 'fs'; import os from 'os'; import path from 'path'; import { @@ -15,6 +23,7 @@ import { configureWebDefaults } from '../../utils/web-defaults'; import { getApiKey } from '../../utils/config'; vi.mock('child_process', () => ({ + execFileSync: vi.fn(), execSync: vi.fn(), })); @@ -28,16 +37,21 @@ vi.mock('../../utils/config', () => ({ describe('handleSetupCommand', () => { let originalHome: string | undefined; + let originalApiKey: string | undefined; beforeEach(() => { vi.clearAllMocks(); vi.mocked(getApiKey).mockReturnValue('fc-test-key'); originalHome = process.env.HOME; + originalApiKey = process.env.FIRECRAWL_API_KEY; + delete process.env.FIRECRAWL_API_KEY; }); afterEach(() => { if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome; + if (originalApiKey === undefined) delete process.env.FIRECRAWL_API_KEY; + else process.env.FIRECRAWL_API_KEY = originalApiKey; vi.restoreAllMocks(); }); @@ -107,6 +121,8 @@ describe('handleSetupCommand', () => { }); it('installs the default setup bundle with --yes', async () => { + vi.mocked(getApiKey).mockReturnValue(undefined); + await handleSetupCommand(undefined, { yes: true }); expect(execSync).toHaveBeenCalledWith( @@ -117,14 +133,22 @@ describe('handleSetupCommand', () => { 'npx -y skills add firecrawl/skills --full-depth --global --all --yes', expect.objectContaining({ stdio: 'inherit' }) ); - expect(execSync).toHaveBeenCalledWith( - 'npx -y add-mcp "https://mcp.firecrawl.dev/fc-test-key/v2/mcp" --name firecrawl --transport http --global --yes', - expect.objectContaining({ - stdio: 'inherit', - }) + expect(execFileSync).toHaveBeenCalledWith( + 'npx', + [ + '-y', + 'add-mcp@1.14.0', + 'https://mcp.firecrawl.dev/v2/mcp', + '--name', + 'firecrawl', + '--transport', + 'http', + '--global', + '--yes', + ], + expect.objectContaining({ stdio: 'inherit' }) ); }); - it('requires a subcommand for bare setup in non-interactive mode', async () => { const originalIsTty = process.stdin.isTTY; Object.defineProperty(process.stdin, 'isTTY', { @@ -171,32 +195,86 @@ describe('handleSetupCommand', () => { }); }); - it('installs MCP with the hosted Firecrawl URL when credentials exist', async () => { + it('fails closed before spawning when only a stored API key is available', async () => { + await expect( + handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + it('normalizes launch aliases for environment-backed MCP setup', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + await handleSetupCommand('mcp', { - agent: 'claude-code', + agent: 'codex-app', global: true, yes: true, }); - expect(execSync).toHaveBeenCalledWith( - 'npx -y add-mcp "https://mcp.firecrawl.dev/fc-test-key/v2/mcp" --name firecrawl --transport http --global --agent claude-code --yes', - expect.objectContaining({ - stdio: 'inherit', - }) + expect(execFileSync).toHaveBeenCalledWith( + 'codex', + [ + 'mcp', + 'add', + 'firecrawl', + '--url', + 'https://mcp.firecrawl.dev/v2/mcp', + '--bearer-token-env-var', + 'FIRECRAWL_API_KEY', + ], + expect.objectContaining({ stdio: 'inherit' }) ); }); + it.each([ + ['claude-code', 'Bearer ${FIRECRAWL_API_KEY}'], + ['vscode', 'Bearer ${env:FIRECRAWL_API_KEY}'], + ['cursor', 'Bearer ${env:FIRECRAWL_API_KEY}'], + ['opencode', 'Bearer {env:FIRECRAWL_API_KEY}'], + ])( + 'uses the %s environment reference when the API key came from the environment', + async (agent, header) => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + await handleSetupCommand('mcp', { + agent, + global: true, + yes: true, + }); + + const args = vi.mocked(execFileSync).mock.calls[0]?.[1]; + expect(args).toContain(`Authorization: ${header}`); + expect(args?.join(' ')).not.toContain('Bearer fc-test-key'); + } + ); + + it('uses Codex native environment-backed bearer configuration', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; - it('normalizes launch aliases when reinstalling MCP after auth changes', async () => { await handleSetupCommand('mcp', { - agent: 'codex-app', + agent: 'codex', global: true, yes: true, }); - expect(execSync).toHaveBeenCalledWith( - 'npx -y add-mcp "https://mcp.firecrawl.dev/fc-test-key/v2/mcp" --name firecrawl --transport http --global --agent codex --yes', + expect(execFileSync).toHaveBeenCalledWith( + 'codex', + [ + 'mcp', + 'add', + 'firecrawl', + '--url', + 'https://mcp.firecrawl.dev/v2/mcp', + '--bearer-token-env-var', + 'FIRECRAWL_API_KEY', + ], expect.objectContaining({ stdio: 'inherit' }) ); + expect(vi.mocked(execFileSync).mock.calls.flat(2).join(' ')).not.toContain( + 'fc-test-key' + ); }); it('installs MCP with the keyless hosted Firecrawl URL without credentials', async () => { @@ -208,15 +286,65 @@ describe('handleSetupCommand', () => { yes: true, }); - expect(execSync).toHaveBeenCalledWith( - 'npx -y add-mcp "https://mcp.firecrawl.dev/v2/mcp" --name firecrawl --transport http --global --agent claude-code --yes', + expect(execFileSync).toHaveBeenCalledWith( + 'npx', + [ + '-y', + 'add-mcp@1.14.0', + 'https://mcp.firecrawl.dev/v2/mcp', + '--name', + 'firecrawl', + '--transport', + 'http', + '--global', + '--agent', + 'claude-code', + '--yes', + ], expect.objectContaining({ stdio: 'inherit' }) ); }); - it('writes Hermes MCP config with Firecrawl credentials', async () => { + it('rejects a stored key before writing Hermes MCP config', async () => { const home = mkdtempSync(path.join(os.tmpdir(), 'firecrawl-hermes-test-')); process.env.HOME = home; + const configPath = path.join(home, '.hermes', 'config.yaml'); + mkdirSync(path.dirname(configPath), { recursive: true }); + const originalConfig = + 'theme: dark\nmcp_servers:\n existing:\n url: https://example.com/mcp\n'; + writeFileSync(configPath, originalConfig, { mode: 0o600 }); + + try { + await expect( + handleSetupCommand('mcp', { + agent: 'hermes', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + + const config = readFileSync(configPath, 'utf-8'); + expect(config).toBe(originalConfig); + expect(config).toContain('theme: dark'); + expect(config).toContain('existing:'); + expect(config).toContain('mcp_servers:'); + expect(config).not.toContain('firecrawl:'); + expect(config).not.toContain('fc-test-key'); + expect(execFileSync).not.toHaveBeenCalled(); + if (process.platform !== 'win32') { + expect(statSync(configPath).mode & 0o777).toBe(0o600); + } + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + it('keeps an environment-backed key indirect in Hermes config', async () => { + const home = mkdtempSync( + path.join(os.tmpdir(), 'firecrawl-hermes-env-test-') + ); + process.env.HOME = home; + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; try { await installHermesMcp(); @@ -225,30 +353,54 @@ describe('handleSetupCommand', () => { path.join(home, '.hermes', 'config.yaml'), 'utf-8' ); - expect(config).toContain('mcp_servers:'); - expect(config).toContain('firecrawl:'); - expect(config).toContain( - 'url: https://mcp.firecrawl.dev/fc-test-key/v2/mcp' - ); + expect(config).toContain('Authorization: Bearer ${FIRECRAWL_API_KEY}'); + expect(config).not.toContain('Bearer fc-test-key'); } finally { rmSync(home, { recursive: true, force: true }); } }); - it('configures OpenClaw MCP with its native CLI command', async () => { + it('rejects a stored key before invoking the OpenClaw CLI', async () => { + await expect(installOpenClawMcp()).rejects.toThrow( + 'Export FIRECRAWL_API_KEY' + ); + expect(execFileSync).not.toHaveBeenCalled(); + }); + it('uses OpenClaw environment expansion instead of persisting an env-backed key', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + await installOpenClawMcp(); - expect(execSync).toHaveBeenCalledWith( - 'openclaw mcp set firecrawl "{\\"url\\":\\"https://mcp.firecrawl.dev/fc-test-key/v2/mcp\\",\\"transport\\":\\"streamable-http\\"}"', - expect.objectContaining({ - stdio: 'inherit', - }) + const config = vi.mocked(execFileSync).mock.calls[0]?.[1]?.[3] as string; + expect(config).toContain('Bearer ${FIRECRAWL_API_KEY}'); + expect(config).not.toContain('Bearer fc-test-key'); + }); + + it('surfaces a sanitized OpenClaw setup failure', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + vi.mocked(execFileSync).mockImplementationOnce(() => { + throw new Error('spawn failed with Authorization: Bearer fc-test-key'); + }); + + await expect(installOpenClawMcp()).rejects.toThrow( + 'Failed to configure Firecrawl MCP for OpenClaw. Verify that OpenClaw is installed and available on PATH.' ); }); - it('reinstalls MCP for all launch integrations with --agent all', async () => { - const home = mkdtempSync(path.join(os.tmpdir(), 'firecrawl-all-mcp-test-')); + it('rejects stored credentials before configuring any launch integration', async () => { + await expect( + handleSetupCommand('mcp', { + agent: 'all', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + it('uses each client native environment binding with --agent all', async () => { + const home = mkdtempSync(path.join(os.tmpdir(), 'firecrawl-all-env-test-')); process.env.HOME = home; + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; try { await handleSetupCommand('mcp', { @@ -257,22 +409,360 @@ describe('handleSetupCommand', () => { yes: true, }); - expect(execSync).toHaveBeenCalledWith( - 'npx -y add-mcp "https://mcp.firecrawl.dev/fc-test-key/v2/mcp" --name firecrawl --transport http --global --all --yes', - expect.objectContaining({ stdio: 'inherit' }) + const calls = vi.mocked(execFileSync).mock.calls; + const serialized = calls.map((call) => (call[1] as string[]).join(' ')); + expect(serialized).toEqual( + expect.arrayContaining([ + expect.stringContaining('claude-code --yes'), + expect.stringContaining( + 'Authorization: Bearer ${env:FIRECRAWL_API_KEY}' + ), + expect.stringContaining('--bearer-token-env-var FIRECRAWL_API_KEY'), + expect.stringContaining( + 'Authorization: Bearer {env:FIRECRAWL_API_KEY}' + ), + expect.stringContaining('Authorization: Bearer ${FIRECRAWL_API_KEY}'), + ]) ); + expect(calls.flat(2).join(' ')).not.toContain('Bearer fc-test-key'); expect( readFileSync(path.join(home, '.hermes', 'config.yaml'), 'utf-8') - ).toContain('url: https://mcp.firecrawl.dev/fc-test-key/v2/mcp'); - expect(execSync).toHaveBeenCalledWith( - 'openclaw mcp set firecrawl "{\\"url\\":\\"https://mcp.firecrawl.dev/fc-test-key/v2/mcp\\",\\"transport\\":\\"streamable-http\\"}"', - expect.objectContaining({ stdio: 'inherit' }) + ).toContain('Authorization: Bearer ${FIRECRAWL_API_KEY}'); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + it('rejects authenticated --agent all project setup before changing any client', async () => { + const home = mkdtempSync( + path.join(os.tmpdir(), 'firecrawl-all-project-preflight-') + ); + process.env.HOME = home; + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + try { + await expect( + handleSetupCommand('mcp', { + agent: 'all', + project: true, + yes: true, + }) + ).rejects.toThrow( + 'Authenticated --agent all setup does not support --project' ); + + expect(execFileSync).not.toHaveBeenCalled(); + expect(execSync).not.toHaveBeenCalled(); + expect(existsSync(path.join(home, '.hermes', 'config.yaml'))).toBe(false); + } finally { + rmSync(home, { recursive: true, force: true }); + } + }); + + it('keeps keyless --agent all project setup available', async () => { + const home = mkdtempSync( + path.join(os.tmpdir(), 'firecrawl-all-project-keyless-') + ); + process.env.HOME = home; + vi.mocked(getApiKey).mockReturnValue(undefined); + + try { + await handleSetupCommand('mcp', { + agent: 'all', + project: true, + yes: true, + }); + + const addMcpCalls = vi + .mocked(execFileSync) + .mock.calls.filter(([, args]) => + (args as string[])?.includes('add-mcp@1.14.0') + ); + expect(addMcpCalls).toHaveLength(5); + expect(addMcpCalls.flat(2)).not.toContain('--global'); + expect( + readFileSync(path.join(home, '.hermes', 'config.yaml'), 'utf-8') + ).toContain('https://mcp.firecrawl.dev/v2/mcp'); } finally { rmSync(home, { recursive: true, force: true }); } }); + it('requires a client selection for no-agent environment-backed setup', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + await expect( + handleSetupCommand('mcp', { global: true, yes: true }) + ).rejects.toThrow('requires --agent'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + + it('rejects an environment-backed key for an unknown client instead of persisting it', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + await expect( + handleSetupCommand('mcp', { + agent: 'future-client', + global: true, + yes: true, + }) + ).rejects.toThrow('does not have a verified environment-variable syntax'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + + it('rejects a stored key for an unknown client before spawning', async () => { + await expect( + handleSetupCommand('mcp', { + agent: 'future-client', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + it('never includes environment-backed credentials in generated URLs or normal output', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + await handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }); + + const args = vi.mocked(execFileSync).mock.calls[0]?.[1]; + expect(args).toContain('https://mcp.firecrawl.dev/v2/mcp'); + expect(args?.join(' ')).not.toContain('fc-test-key'); + expect(log.mock.calls.flat().join(' ')).not.toContain('fc-test-key'); + }); + it('never places a stored API key in subprocess argv', async () => { + await expect( + handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + it('does not print a stored OpenClaw credential when setup is rejected', async () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + await expect(installOpenClawMcp()).rejects.toThrow( + 'Export FIRECRAWL_API_KEY' + ); + + expect(log.mock.calls.flat().join(' ')).not.toContain('fc-test-key'); + }); + it('rejects stored credentials containing hostile characters without spawning or printing them', async () => { + const hostileKey = 'fc-$(touch /tmp/firecrawl-pwned)`echo bad`"\\n$HOME'; + vi.mocked(getApiKey).mockReturnValue(hostileKey); + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + const error = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + await expect( + handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }) + ).rejects.toThrow('Export FIRECRAWL_API_KEY'); + + expect(execFileSync).not.toHaveBeenCalled(); + expect(execSync).not.toHaveBeenCalled(); + expect(log.mock.calls.flat().join(' ')).not.toContain(hostileKey); + expect(error.mock.calls.flat().join(' ')).not.toContain(hostileKey); + }); + // --- Scope: project and global are mutually exclusive --- + + it('rejects conflicting MCP scope flags', async () => { + await expect( + handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + project: true, + }) + ).rejects.toThrow('Choose either --global or --project'); + expect(execFileSync).not.toHaveBeenCalled(); + }); + + it('keeps project scope for an environment-backed credential', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + await handleSetupCommand('mcp', { + agent: 'cursor', + project: true, + yes: true, + }); + + const args = vi.mocked(execFileSync).mock.calls[0]?.[1] as string[]; + expect(args).toContain('Authorization: Bearer ${env:FIRECRAWL_API_KEY}'); + expect(args).not.toContain('--global'); + expect(args.join(' ')).not.toContain('Bearer fc-test-key'); + }); + + it('does not force global MCP scope in the default bundle when --project is set', async () => { + vi.mocked(getApiKey).mockReturnValue(undefined); + + await handleSetupCommand(undefined, { + agent: 'cursor', + project: true, + yes: true, + }); + + const mcpCall = vi + .mocked(execFileSync) + .mock.calls.find(([command]) => command === 'npx'); + expect(mcpCall?.[1]).not.toContain('--global'); + }); + + it('does not force global when using an environment reference (no raw key in header)', async () => { + // Env-backed cursor uses ${env:FIRECRAWL_API_KEY}, not the literal secret, + // so project scope is safe and must not be silently overridden. + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + await handleSetupCommand('mcp', { + agent: 'cursor', + yes: true, + }); + + const args = vi.mocked(execFileSync).mock.calls[0]?.[1] as string[]; + expect(args.join(' ')).not.toContain('Bearer fc-test-key'); + expect(args).toContain('Authorization: Bearer ${env:FIRECRAWL_API_KEY}'); + expect(args).not.toContain('--global'); + }); + + it('does not force global for the keyless (unauthenticated) setup', async () => { + vi.mocked(getApiKey).mockReturnValue(undefined); + + await handleSetupCommand('mcp', { + agent: 'claude-code', + yes: true, + }); + + const args = vi.mocked(execFileSync).mock.calls[0]?.[1] as string[]; + expect(args.join(' ')).not.toContain('--header'); + expect(args).not.toContain('--global'); + }); + + // --- Windows: launch .cmd/.exe shims correctly (execFileSync cannot) --- + + it('launches the npx.cmd shim via the shell on win32 with cmd-escaped args', async () => { + const root = mkdtempSync(path.join(os.tmpdir(), 'firecrawl-win-')); + const bin = path.join(root, 'Program Files', 'nodejs'); + mkdirSync(bin, { recursive: true }); + writeFileSync(path.join(bin, 'npx.CMD'), '@exit /b 0\r\n'); + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + 'platform' + ); + const originalPath = process.env.PATH; + const originalPathext = process.env.PATHEXT; + const originalComspec = process.env.ComSpec; + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32', + }); + process.env.PATH = bin; + process.env.PATHEXT = '.EXE;.CMD'; + process.env.ComSpec = 'cmd.exe'; + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + try { + await handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }); + + const call = vi.mocked(execFileSync).mock.calls[0]; + const command = call?.[0] as string; + const passthruArgs = call?.[1] as string[]; + const opts = call?.[2] as { windowsVerbatimArguments?: boolean }; + + expect(command).toBe('cmd.exe'); + expect(passthruArgs.slice(0, 3)).toEqual(['/d', '/s', '/c']); + expect(opts?.windowsVerbatimArguments).toBe(true); + expect(passthruArgs[3]).toContain(`^\"${path.join(bin, 'npx.CMD')}^\"`); + expect(passthruArgs[3]).toContain('add-mcp@1.14.0'); + expect(passthruArgs[3]).toContain( + '^"Authorization: Bearer ${FIRECRAWL_API_KEY}^"' + ); + } finally { + if (originalPlatform) + Object.defineProperty(process, 'platform', originalPlatform); + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + if (originalPathext === undefined) delete process.env.PATHEXT; + else process.env.PATHEXT = originalPathext; + if (originalComspec === undefined) delete process.env.ComSpec; + else process.env.ComSpec = originalComspec; + rmSync(root, { recursive: true, force: true }); + } + }); + + it('launches a native Codex executable directly on win32', async () => { + const bin = mkdtempSync(path.join(os.tmpdir(), 'firecrawl-win-bin-')); + const codexExe = path.join(bin, 'codex.EXE'); + writeFileSync(codexExe, ''); + const originalPlatform = Object.getOwnPropertyDescriptor( + process, + 'platform' + ); + const originalPath = process.env.PATH; + const originalPathext = process.env.PATHEXT; + Object.defineProperty(process, 'platform', { + configurable: true, + value: 'win32', + }); + process.env.PATH = bin; + process.env.PATHEXT = '.EXE;.CMD'; + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + + try { + await handleSetupCommand('mcp', { + agent: 'codex', + global: true, + yes: true, + }); + + const call = vi.mocked(execFileSync).mock.calls[0]; + const command = call?.[0] as string; + const args = call?.[1] as string[]; + const opts = call?.[2] as { windowsVerbatimArguments?: boolean }; + expect(command).toBe(codexExe); + expect(args).toContain('--bearer-token-env-var'); + expect(opts?.windowsVerbatimArguments).toBeUndefined(); + } finally { + if (originalPlatform) + Object.defineProperty(process, 'platform', originalPlatform); + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + if (originalPathext === undefined) delete process.env.PATHEXT; + else process.env.PATHEXT = originalPathext; + rmSync(bin, { recursive: true, force: true }); + } + }); + + it('still spawns bare argv with no shell on non-win32', async () => { + process.env.FIRECRAWL_API_KEY = 'fc-test-key'; + // Sanity: the pre-existing POSIX path is unchanged (argv-safe, no shell). + await handleSetupCommand('mcp', { + agent: 'claude-code', + global: true, + yes: true, + }); + + const call = vi.mocked(execFileSync).mock.calls[0]; + expect(call?.[0]).toBe('npx'); + expect( + Array.isArray(call?.[1]) && (call?.[1] as string[]).length + ).toBeGreaterThan(0); + expect((call?.[2] as { shell?: boolean })?.shell).toBeUndefined(); + }); + it('strips inherited npm_* env vars before nested npx calls', async () => { // Reproduces the bug where running this CLI under `npx -y firecrawl-cli@VERSION` // leaks npm_command/npm_lifecycle_event/npm_execpath into nested diff --git a/src/commands/setup.ts b/src/commands/setup.ts index 25eac94507..239121fb13 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -3,8 +3,14 @@ * Installs firecrawl skill files and MCP server into AI coding agents */ -import { execSync } from 'child_process'; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs'; +import { execFileSync, execSync } from 'child_process'; +import { + chmodSync, + existsSync, + mkdirSync, + readFileSync, + writeFileSync, +} from 'fs'; import os from 'os'; import path from 'path'; import readline from 'readline'; @@ -35,6 +41,8 @@ type ResolvedMcpAgent = export interface SetupOptions { global?: boolean; + /** Explicitly install MCP into project scope. */ + project?: boolean; agent?: string; undo?: boolean; /** Skip the interactive harness picker and apply to all agents. */ @@ -48,6 +56,15 @@ export interface SetupOptions { const green = '\x1b[32m'; const dim = '\x1b[2m'; const reset = '\x1b[0m'; +const ADD_MCP_PACKAGE = 'add-mcp@1.14.0'; +const ENV_API_KEY = 'FIRECRAWL_API_KEY'; +const ADD_MCP_LAUNCH_AGENTS = [ + 'claude-code', + 'vscode', + 'codex', + 'opencode', + 'cursor', +] as const; const SKILL_REPO_LABELS: Record = { 'firecrawl/cli': 'Core CLI skills', @@ -59,18 +76,156 @@ function skillRepoLabel(repo: string): string { return SKILL_REPO_LABELS[repo] ?? repo; } -function shellQuote(value: string): string { - return JSON.stringify(value); +const CMD_META_CHARS = /([()%!^"<>&|])/g; + +function rejectCommandControlCharacters(value: string, label: string): void { + if (/[\0\r\n]/.test(value)) { + throw new Error(`${label} contains an unsupported control character.`); + } } -function firecrawlHostedMcpUrl(): string { - const apiKey = getApiKey(); - if (apiKey) { - return `https://mcp.firecrawl.dev/${encodeURIComponent(apiKey)}/v2/mcp`; +/** Quote one argv value for cmd.exe using the same two-layer escaping model as + * established Windows spawn libraries: first the C runtime, then cmd.exe. */ +function escapeCmdArg(arg: string): string { + rejectCommandControlCharacters(arg, 'Command argument'); + const quoted = `"${arg + .replace(/(\\*)"/g, '$1$1\\"') + .replace(/(\\*)$/, '$1$1')}"`; + return quoted.replace(CMD_META_CHARS, '^$1'); +} + +function windowsPathExtensions(env: NodeJS.ProcessEnv): string[] { + const configured = env.PATHEXT ?? '.COM;.EXE;.BAT;.CMD'; + return configured + .split(';') + .map((extension) => extension.trim()) + .filter(Boolean); +} + +/** Resolve the actual Windows launcher instead of assuming every tool is a + * `.cmd` shim. Native `.exe` clients must bypass cmd.exe entirely. */ +function resolveWindowsCommand( + command: string, + env: NodeJS.ProcessEnv +): string { + rejectCommandControlCharacters(command, 'Command'); + const hasPath = /[\\/]/.test(command); + const hasExtension = path.extname(command) !== ''; + const candidates = hasExtension + ? [command] + : windowsPathExtensions(env).map((extension) => `${command}${extension}`); + const pathEntries = hasPath + ? [''] + : (env.PATH ?? env.Path ?? env.path ?? '') + .split(path.delimiter) + .map((entry) => entry.replace(/^"|"$/g, '')) + .filter(Boolean); + + for (const directory of pathEntries) { + for (const candidate of candidates) { + const resolved = directory ? path.join(directory, candidate) : candidate; + if (existsSync(resolved)) return resolved; + } } + + // Let CreateProcess perform its normal resolution for native executables. + // Crucially, do not silently rewrite an unknown command to `.cmd`. + return command; +} + +/** + * Cross-platform, injection-safe replacement for `execFileSync`. + * + * On win32, external tools ship as `.cmd`/`.bat` shims (npx.cmd, npm.cmd, + * codex.cmd, openclaw.cmd). Node's `execFile`/`execFileSync` calls CreateProcess + * directly and CANNOT launch a `.cmd`/`.bat` file — it throws ENOENT/EINVAL. The + * only reliable way is to route through the shell (cmd.exe). To keep the argv + * safety this file relies on (secrets must never be shell-interpreted), we + * escape every argument for cmd.exe ourselves instead of letting the shell + * re-split a joined string. + * + * On every other platform we spawn the binary directly with no shell, exactly as + * `execFileSync` did before. + */ +function runClientCommand( + command: string, + args: string[], + options: Parameters[2] +): void { + rejectCommandControlCharacters(command, 'Command'); + for (const arg of args) + rejectCommandControlCharacters(arg, 'Command argument'); + + if (process.platform !== 'win32') { + execFileSync(command, args, options); + return; + } + + const env = options?.env ?? process.env; + const resolved = resolveWindowsCommand(command, env); + if (!/\.(?:cmd|bat)$/i.test(resolved)) { + execFileSync(resolved, args, options); + return; + } + + const line = [escapeCmdArg(resolved), ...args.map(escapeCmdArg)].join(' '); + const comspec = env.ComSpec ?? env.COMSPEC ?? 'cmd.exe'; + const windowsOptions = { + ...options, + windowsVerbatimArguments: true, + } as Parameters[2]; + execFileSync(comspec, ['/d', '/s', '/c', `"${line}"`], windowsOptions); +} + +function firecrawlHostedMcpUrl(): string { return 'https://mcp.firecrawl.dev/v2/mcp'; } +function isEnvironmentBackedApiKey(apiKey: string | undefined): boolean { + return Boolean(apiKey && process.env[ENV_API_KEY] === apiKey); +} + +function assertSubprocessSafeCredential(apiKey = getApiKey()): void { + if (apiKey && !isEnvironmentBackedApiKey(apiKey)) { + throw new Error( + 'Secure MCP setup cannot pass a stored API key to this client. Export FIRECRAWL_API_KEY and rerun with a supported --agent, or run keyless setup without a credential.' + ); + } +} + +function environmentHeaderForAgent(agent?: string): string | undefined { + switch (agent) { + case 'claude-code': + case 'hermes': + case 'openclaw': + return `Bearer \${${ENV_API_KEY}}`; + case 'cursor': + case 'vscode': + return `Bearer \${env:${ENV_API_KEY}}`; + case 'opencode': + return `Bearer {env:${ENV_API_KEY}}`; + default: + return undefined; + } +} + +function firecrawlMcpHeaders( + agent?: string +): Record | undefined { + const apiKey = getApiKey(); + if (!apiKey) return undefined; + + // Keep this helper safe in isolation. Callers currently reject stored keys + // before reaching it, but a future call site must not turn one into a raw + // Authorization header in argv or a client configuration file. + assertSubprocessSafeCredential(apiKey); + const environmentHeader = environmentHeaderForAgent(agent); + if (environmentHeader) return { Authorization: environmentHeader }; + throw new Error( + 'This MCP client does not have a verified environment-variable syntax. Choose a supported --agent, use --agent all, or configure the client manually so FIRECRAWL_API_KEY is not persisted as a literal.' + ); +} + function resolveMcpAgent(agent: string | undefined): ResolvedMcpAgent { if (!agent) return { kind: 'add-mcp' }; @@ -168,7 +323,10 @@ async function handleSetupBundle(options: SetupOptions): Promise { return; } - const bundleOptions = { ...options, global: options.global ?? true }; + const bundleOptions = { + ...options, + global: options.project ? undefined : (options.global ?? true), + }; for (const integration of integrations) { await handleSetupCommand(integration, bundleOptions); } @@ -365,46 +523,81 @@ export async function installSkillsForAgent( } export async function installMcp(options: SetupOptions): Promise { + if (options.global && options.project) { + throw new Error('Choose either --global or --project, not both.'); + } + + const apiKey = getApiKey(); const resolvedAgent = resolveMcpAgent(options.agent); + if (resolvedAgent.kind === 'all-launchers' && options.project && apiKey) { + throw new Error( + 'Authenticated --agent all setup does not support --project because Codex requires a global environment-backed MCP configuration. Choose one --agent for project setup, use --agent all --global, or run keyless setup.' + ); + } + if (!options.agent && isEnvironmentBackedApiKey(apiKey)) { + throw new Error( + "Environment-backed MCP setup requires --agent so Firecrawl can use that client's native variable syntax. Choose a supported client or use --agent all; the API key will not be written literally." + ); + } if (resolvedAgent.kind === 'hermes') { await installHermesMcp(); return; } + assertSubprocessSafeCredential(apiKey); if (resolvedAgent.kind === 'openclaw') { await installOpenClawMcp(); return; } if (resolvedAgent.kind === 'all-launchers') { - await installAddMcp( - { ...options, yes: true }, - { kind: 'add-mcp', all: true } - ); - await installHermesMcp(); - await installOpenClawMcp(); + await installAllMcpLaunchers(options); return; } await installAddMcp(options, resolvedAgent); } +async function installAllMcpLaunchers(options: SetupOptions): Promise { + for (const agent of ADD_MCP_LAUNCH_AGENTS) { + await installAddMcp({ ...options, yes: true }, { kind: 'add-mcp', agent }); + } + await installHermesMcp(); + await installOpenClawMcp(); +} + async function installAddMcp( options: SetupOptions, resolvedAgent: Extract ): Promise { const mcpUrl = firecrawlHostedMcpUrl(); + const apiKey = getApiKey(); + if ( + resolvedAgent.agent === 'codex' && + !options.project && + apiKey && + isEnvironmentBackedApiKey(apiKey) + ) { + installCodexMcpFromEnvironment(options, mcpUrl); + return; + } + + const headers = firecrawlMcpHeaders(resolvedAgent.agent); + const useGlobal = !options.project && Boolean(options.global); const args = [ - 'npx', '-y', - 'add-mcp', - shellQuote(mcpUrl), + ADD_MCP_PACKAGE, + mcpUrl, '--name', 'firecrawl', '--transport', 'http', ]; - if (options.global) { + if (headers?.Authorization) { + args.push('--header', `Authorization: ${headers.Authorization}`); + } + + if (useGlobal) { args.push('--global'); } @@ -418,13 +611,12 @@ async function installAddMcp( args.push('--yes'); } - const cmd = args.join(' '); if (!options.quiet) { - console.log(`Running: ${cmd}\n`); + console.log('Configuring Firecrawl MCP...\n'); } try { - execSync(cmd, { + runClientCommand('npx', args, { stdio: 'inherit', env: cleanNpmEnv(), }); @@ -437,21 +629,53 @@ async function installAddMcp( console.log(` ${green}✓${reset} Firecrawl MCP configured${target}`); } } catch { - process.exit(1); + throw new Error('Failed to configure Firecrawl MCP.'); } } -function firecrawlMcpConfig(): { +function installCodexMcpFromEnvironment( + options: SetupOptions, + mcpUrl: string +): void { + if (!options.quiet) { + console.log('Configuring Firecrawl MCP...\n'); + } + + try { + runClientCommand( + 'codex', + [ + 'mcp', + 'add', + 'firecrawl', + '--url', + mcpUrl, + '--bearer-token-env-var', + 'FIRECRAWL_API_KEY', + ], + { stdio: 'inherit', env: cleanNpmEnv() } + ); + if (options.quiet) { + console.log(` ${green}✓${reset} Firecrawl MCP configured for codex`); + } + } catch { + throw new Error('Failed to configure Firecrawl MCP for Codex.'); + } +} + +function firecrawlMcpConfig(agent?: string): { url: string; + headers?: Record; transport?: string; } { return { url: firecrawlHostedMcpUrl(), + headers: firecrawlMcpHeaders(agent), }; } export async function installHermesMcp(): Promise { - const config = firecrawlMcpConfig(); + const config = firecrawlMcpConfig('hermes'); const configPath = path.join(os.homedir(), '.hermes', 'config.yaml'); mkdirSync(path.dirname(configPath), { recursive: true }); @@ -468,31 +692,35 @@ export async function installHermesMcp(): Promise { mcpServers.firecrawl = config; root.mcp_servers = mcpServers; - writeFileSync(configPath, stringifyYaml(root), 'utf-8'); + writeFileSync(configPath, stringifyYaml(root), { + encoding: 'utf-8', + mode: 0o600, + }); + if (process.platform !== 'win32') { + chmodSync(configPath, 0o600); + } console.log(`Hermes Agent MCP configured at ${configPath}.`); } export async function installOpenClawMcp(): Promise { const config = { - ...firecrawlMcpConfig(), + ...firecrawlMcpConfig('openclaw'), transport: 'streamable-http', }; - const cmd = [ - 'openclaw', - 'mcp', - 'set', - 'firecrawl', - shellQuote(JSON.stringify(config)), - ].join(' '); - - console.log(`Running: ${cmd}\n`); + console.log('Configuring Firecrawl MCP for OpenClaw...\n'); try { - execSync(cmd, { - stdio: 'inherit', - env: cleanNpmEnv(), - }); + runClientCommand( + 'openclaw', + ['mcp', 'set', 'firecrawl', JSON.stringify(config)], + { + stdio: 'pipe', + env: cleanNpmEnv(), + } + ); } catch { - process.exit(1); + throw new Error( + 'Failed to configure Firecrawl MCP for OpenClaw. Verify that OpenClaw is installed and available on PATH.' + ); } } diff --git a/src/index.ts b/src/index.ts index 5c1ae11f3f..b22c8e1a89 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2188,9 +2188,13 @@ program 'What to set up: "skills", "workflows", "mcp", or "defaults"; omit for an interactive installer' ) .option('-g, --global', 'Install globally (user-level)') + .option( + '--project', + 'For "mcp", install into project scope (stored API keys are never written to project files)' + ) .option( '-a, --agent ', - 'Limit to a specific agent; for "mcp", use "all" to update every launch integration' + 'Limit to a specific agent; required for environment-backed MCP setup, or use "all" to update every launch integration' ) .option( '-y, --yes',