diff --git a/.gitattributes b/.gitattributes index 468c8ed..c7efe70 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,4 +3,4 @@ # (core.autocrlf on Windows) broke those comparisons. * text=auto eol=lf -*.png binary +*.png binary \ No newline at end of file diff --git a/src/commands/agent.test.ts b/src/commands/agent.test.ts index 24d8ec6..19bdd7f 100644 --- a/src/commands/agent.test.ts +++ b/src/commands/agent.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdtempSync, readFileSync, symlinkSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { describe, expect, it, vi } from 'vitest'; @@ -25,6 +25,21 @@ import { runStatus, } from './agent.js'; +/** Windows requires Developer Mode or elevation to create symlinks. */ +function canCreateSymlinks(): boolean { + const probeRoot = mkdtempSync(path.join(tmpdir(), 'agent-symlink-probe-')); + const target = path.join(probeRoot, 'target.txt'); + writeFileSync(target, 'probe'); + try { + symlinkSync(target, path.join(probeRoot, 'link.txt'), 'file'); + return true; + } catch { + return false; + } +} + +const symlinkCapable = canCreateSymlinks(); + // --------------------------------------------------------------------------- // In-memory AgentFs backed by a Map // --------------------------------------------------------------------------- @@ -1108,75 +1123,81 @@ describe('runInstall — default AgentFs (real disk)', () => { expect(readFileSync(abs, 'utf8')).toBe(content); }); - it('refuses to write through a symlinked parent dir (real disk) — exit 5', async () => { - const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-parent-')); - const outside = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-')); - // `.claude` is a real symlink to a directory outside the project root. - symlinkSync(outside, path.join(tmpRoot, '.claude'), 'dir'); - const { deps } = makeCapture(); - - let thrown: unknown; - try { - await runInstall( - { - profile: 'default', - output: 'text', - debug: false, - dryRun: false, - target: ['claude'], - skills: ['testsprite-verify'], - force: false, - dir: tmpRoot, - }, - { ...deps }, - ); - } catch (err) { - thrown = err; - } - - expect(thrown).toBeInstanceOf(CLIError); - expect((thrown as CLIError).exitCode).toBe(5); - // Nothing was created through the symlink, outside --dir. - expect(existsSync(path.join(outside, 'skills'))).toBe(false); - }); - - it('refuses to overwrite a symlinked target file (real disk) with --force — exit 5', async () => { - const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-target-')); - const outsideDir = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-target-')); - const { path: relPath } = renderForTarget('claude', 'testsprite-verify'); - const abs = path.resolve(tmpRoot, relPath); - const nodeFs = await import('node:fs/promises'); - await nodeFs.mkdir(path.dirname(abs), { recursive: true }); - // SKILL.md is a real symlink to a file outside the project root. - const outsideFile = path.join(outsideDir, 'secret.txt'); - await nodeFs.writeFile(outsideFile, 'SECRET', 'utf8'); - symlinkSync(outsideFile, abs, 'file'); - const { deps } = makeCapture(); + it.skipIf(!symlinkCapable)( + 'refuses to write through a symlinked parent dir (real disk) — exit 5', + async () => { + const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-parent-')); + const outside = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-')); + // `.claude` is a real symlink to a directory outside the project root. + symlinkSync(outside, path.join(tmpRoot, '.claude'), 'dir'); + const { deps } = makeCapture(); + + let thrown: unknown; + try { + await runInstall( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + target: ['claude'], + skills: ['testsprite-verify'], + force: false, + dir: tmpRoot, + }, + { ...deps }, + ); + } catch (err) { + thrown = err; + } - let thrown: unknown; - try { - await runInstall( - { - profile: 'default', - output: 'text', - debug: false, - dryRun: false, - target: ['claude'], - skills: ['testsprite-verify'], - force: true, - dir: tmpRoot, - }, - { ...deps }, - ); - } catch (err) { - thrown = err; - } + expect(thrown).toBeInstanceOf(CLIError); + expect((thrown as CLIError).exitCode).toBe(5); + // Nothing was created through the symlink, outside --dir. + expect(existsSync(path.join(outside, 'skills'))).toBe(false); + }, + ); + + it.skipIf(!symlinkCapable)( + 'refuses to overwrite a symlinked target file (real disk) with --force — exit 5', + async () => { + const tmpRoot = mkdtempSync(path.join(tmpdir(), 'agent-test-symlink-target-')); + const outsideDir = mkdtempSync(path.join(tmpdir(), 'agent-test-outside-target-')); + const { path: relPath } = renderForTarget('claude', 'testsprite-verify'); + const abs = path.resolve(tmpRoot, relPath); + const nodeFs = await import('node:fs/promises'); + await nodeFs.mkdir(path.dirname(abs), { recursive: true }); + // SKILL.md is a real symlink to a file outside the project root. + const outsideFile = path.join(outsideDir, 'secret.txt'); + await nodeFs.writeFile(outsideFile, 'SECRET', 'utf8'); + symlinkSync(outsideFile, abs, 'file'); + const { deps } = makeCapture(); + + let thrown: unknown; + try { + await runInstall( + { + profile: 'default', + output: 'text', + debug: false, + dryRun: false, + target: ['claude'], + skills: ['testsprite-verify'], + force: true, + dir: tmpRoot, + }, + { ...deps }, + ); + } catch (err) { + thrown = err; + } - expect(thrown).toBeInstanceOf(CLIError); - expect((thrown as CLIError).exitCode).toBe(5); - // The outside file was NOT overwritten (nor clobbered via the .bak path). - expect(readFileSync(outsideFile, 'utf8')).toBe('SECRET'); - }); + expect(thrown).toBeInstanceOf(CLIError); + expect((thrown as CLIError).exitCode).toBe(5); + // The outside file was NOT overwritten (nor clobbered via the .bak path). + expect(readFileSync(outsideFile, 'utf8')).toBe('SECRET'); + }, + ); }); // --------------------------------------------------------------------------- diff --git a/src/lib/agent-targets.test.ts b/src/lib/agent-targets.test.ts index 2ac9158..dcaaea5 100644 --- a/src/lib/agent-targets.test.ts +++ b/src/lib/agent-targets.test.ts @@ -49,7 +49,7 @@ function parseFrontmatterDescription(content: string): string | undefined { } } if (inFrontmatter && line.startsWith('description: ')) { - return line.slice('description: '.length); + return line.slice('description: '.length).replace(/\r$/, ''); } } return undefined; diff --git a/src/lib/bundle.test.ts b/src/lib/bundle.test.ts index ff20e8c..d18a96e 100644 --- a/src/lib/bundle.test.ts +++ b/src/lib/bundle.test.ts @@ -598,8 +598,9 @@ describe('resolveBundleDir', () => { }); it('strips a trailing slash', () => { - const out = resolveBundleDir('/tmp/x/'); - expect(out).toBe('/tmp/x'); + const base = resolve(process.cwd(), 'tmp', 'x'); + const trailing = process.platform === 'win32' ? `${base}\\` : `${base}/`; + expect(resolveBundleDir(trailing)).toBe(base); }); }); diff --git a/src/lib/bundle.ts b/src/lib/bundle.ts index a3c0808..f23dfbc 100644 --- a/src/lib/bundle.ts +++ b/src/lib/bundle.ts @@ -313,6 +313,17 @@ export function applyFailedOnly(ctx: CliFailureContext): CliFailureContext { * its `.tmp` child — `writeBundle` mkdir's after the integrity check * passes so a forged response never modifies the operator's filesystem. */ +function stripTrailingSeparators(rawPath: string): string { + if (rawPath.length <= 1) return rawPath; + let end = rawPath.length; + while (end > 1 && (rawPath[end - 1] === '/' || rawPath[end - 1] === '\\')) { + // Preserve Windows drive roots (e.g. `C:\`). + if (end === 3 && rawPath[1] === ':' && /[A-Za-z]/.test(rawPath[0]!)) break; + end--; + } + return rawPath.slice(0, end); +} + export function resolveBundleDir(rawPath: string): string { if (typeof rawPath !== 'string' || rawPath.length === 0) { throw ApiError.fromEnvelope({ @@ -325,7 +336,7 @@ export function resolveBundleDir(rawPath: string): string { }, }); } - const trimmed = rawPath.endsWith('/') ? rawPath.slice(0, -1) : rawPath; + const trimmed = stripTrailingSeparators(rawPath); return isAbsolute(trimmed) ? trimmed : resolve(process.cwd(), trimmed); } diff --git a/src/lib/skill-nudge.test.ts b/src/lib/skill-nudge.test.ts index 2b26c0e..dbbdb0c 100644 --- a/src/lib/skill-nudge.test.ts +++ b/src/lib/skill-nudge.test.ts @@ -1,3 +1,4 @@ +import path from 'node:path'; import { describe, expect, it } from 'vitest'; import { MANAGED_SECTION_BEGIN, MANAGED_SECTION_END, TARGETS } from './agent-targets.js'; import type { OutputMode } from './output.js'; @@ -13,73 +14,84 @@ import { // isVerifySkillInstalled // --------------------------------------------------------------------------- -// The implementation joins paths with the native separator; normalize so the -// fakes below match on Windows (backslashes) as well as POSIX. -const toPosix = (p: string) => p.replaceAll('\\', '/'); +/** True when `observed` resolves under `root` (boundary-safe, cross-platform). */ +function isUnderRoot(root: string, observed: string): boolean { + const rel = path.relative(path.resolve(root), path.resolve(observed)); + return !path.isAbsolute(rel) && !rel.startsWith('..'); +} describe('isVerifySkillInstalled', () => { + const projDir = path.resolve('proj'); + it('true when the claude own-file SKILL.md exists', () => { - const existsSync = (p: string) => - toPosix(p).endsWith('.claude/skills/testsprite-verify/SKILL.md'); - expect(isVerifySkillInstalled('/proj', { existsSync })).toBe(true); + const landing = path.join(projDir, TARGETS.claude.path); + const existsSync = (p: string) => p === landing; + expect(isVerifySkillInstalled(projDir, { existsSync })).toBe(true); }); it('true for the cursor .mdc landing file', () => { - const existsSync = (p: string) => toPosix(p).endsWith('.cursor/rules/testsprite-verify.mdc'); - expect(isVerifySkillInstalled('/proj', { existsSync })).toBe(true); + const landing = path.join(projDir, TARGETS.cursor.path); + const existsSync = (p: string) => p === landing; + expect(isVerifySkillInstalled(projDir, { existsSync })).toBe(true); }); it('true for the cline landing file', () => { - const existsSync = (p: string) => toPosix(p).endsWith('.clinerules/testsprite-verify.md'); - expect(isVerifySkillInstalled('/proj', { existsSync })).toBe(true); + const landing = path.join(projDir, TARGETS.cline.path); + const existsSync = (p: string) => p === landing; + expect(isVerifySkillInstalled(projDir, { existsSync })).toBe(true); }); it('true for the antigravity landing file', () => { - const existsSync = (p: string) => - toPosix(p).endsWith('.agents/skills/testsprite-verify/SKILL.md'); - expect(isVerifySkillInstalled('/proj', { existsSync })).toBe(true); + const landing = path.join(projDir, TARGETS.antigravity.path); + const existsSync = (p: string) => p === landing; + expect(isVerifySkillInstalled(projDir, { existsSync })).toBe(true); }); it('true when AGENTS.md exists AND carries our BEGIN sentinel', () => { - const existsSync = (p: string) => p.endsWith('AGENTS.md'); + const landing = path.join(projDir, TARGETS.codex.path); + const existsSync = (p: string) => p === landing; const readFileSync = () => `# project\n${MANAGED_SECTION_BEGIN}\n...skill...\n${MANAGED_SECTION_END}\n`; - expect(isVerifySkillInstalled('/proj', { existsSync, readFileSync })).toBe(true); + expect(isVerifySkillInstalled(projDir, { existsSync, readFileSync })).toBe(true); }); it('false when AGENTS.md has only the BEGIN sentinel without a complete managed section', () => { - const existsSync = (p: string) => p.endsWith('AGENTS.md'); + const landing = path.join(projDir, TARGETS.codex.path); + const existsSync = (p: string) => p === landing; const readFileSync = () => `# project\n${MANAGED_SECTION_BEGIN}\n...partial skill...\n`; - expect(isVerifySkillInstalled('/proj', { existsSync, readFileSync })).toBe(false); + expect(isVerifySkillInstalled(projDir, { existsSync, readFileSync })).toBe(false); }); it('false when only a bare AGENTS.md (no sentinel) exists', () => { - const existsSync = (p: string) => p.endsWith('AGENTS.md'); + const landing = path.join(projDir, TARGETS.codex.path); + const existsSync = (p: string) => p === landing; const readFileSync = () => '# my project\nNothing TestSprite here.\n'; - expect(isVerifySkillInstalled('/proj', { existsSync, readFileSync })).toBe(false); + expect(isVerifySkillInstalled(projDir, { existsSync, readFileSync })).toBe(false); }); it('false when an unreadable AGENTS.md is the only candidate', () => { - const existsSync = (p: string) => p.endsWith('AGENTS.md'); + const landing = path.join(projDir, TARGETS.codex.path); + const existsSync = (p: string) => p === landing; const readFileSync = () => { throw new Error('EACCES'); }; - expect(isVerifySkillInstalled('/proj', { existsSync, readFileSync })).toBe(false); + expect(isVerifySkillInstalled(projDir, { existsSync, readFileSync })).toBe(false); }); it('false when nothing is present', () => { - expect(isVerifySkillInstalled('/proj', { existsSync: () => false })).toBe(false); + expect(isVerifySkillInstalled(projDir, { existsSync: () => false })).toBe(false); }); it('checks paths under the supplied dir', () => { + const root = path.resolve('some', 'proj'); const seen: string[] = []; - isVerifySkillInstalled('/some/proj', { + isVerifySkillInstalled(root, { existsSync: (p: string) => { seen.push(p); return false; }, }); - expect(seen.every(p => toPosix(p).startsWith('/some/proj'))).toBe(true); + expect(seen.every(p => isUnderRoot(root, p))).toBe(true); // One probe per target landing path. expect(seen).toHaveLength(Object.keys(TARGETS).length); }); @@ -194,9 +206,10 @@ describe('maybeEmitSkillNudge', () => { }); it('passes the cwd through to the presence check', () => { + const cwd = path.resolve('work', 'here'); const probed: string[] = []; const { ctx } = makeCtx({ - cwd: '/work/here', + cwd, existsSync: (p: string) => { probed.push(p); return false; @@ -204,6 +217,6 @@ describe('maybeEmitSkillNudge', () => { }); maybeEmitSkillNudge(ctx); expect(probed.length).toBeGreaterThan(0); - expect(probed.every(p => toPosix(p).startsWith('/work/here'))).toBe(true); + expect(probed.every(p => isUnderRoot(cwd, p))).toBe(true); }); }); diff --git a/test/cli.subprocess.test.ts b/test/cli.subprocess.test.ts index 959f212..c2020ab 100644 --- a/test/cli.subprocess.test.ts +++ b/test/cli.subprocess.test.ts @@ -7,8 +7,9 @@ * and runs `auth whoami` against the mock." */ -import { execFileSync, spawn } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { existsSync, mkdtempSync, rmSync, statSync } from 'node:fs'; +import { execNpm } from './helpers/execNpm.js'; import type { IncomingMessage, Server, ServerResponse } from 'node:http'; import { createServer } from 'node:http'; import { tmpdir } from 'node:os'; @@ -37,7 +38,7 @@ beforeAll(async () => { // existsSync skip we used to do here let `dist` rot under // refactors and gave false-green on `project list` once // already. - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); + execNpm(['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); server = createServer((req: IncomingMessage, res: ServerResponse) => { const url = req.url ?? '/'; if (url.startsWith('/api/cli/v1/projects/')) { @@ -358,16 +359,18 @@ interface SpawnResult { stderr: string; } +function isolatedHomeEnv(): Record { + // Windows `os.homedir()` reads USERPROFILE, not HOME. + return { HOME: tmpHome, USERPROFILE: tmpHome }; +} + function runCli(args: string[], envOverrides: Record = {}): Promise { return new Promise((resolveResult, rejectResult) => { const child = spawn('node', [BIN_PATH, ...args], { cwd: REPO_ROOT, env: { ...process.env, - // os.homedir() reads HOME on POSIX but USERPROFILE on Windows — - // set both so the child never sees the real ~/.testsprite. - HOME: tmpHome, - USERPROFILE: tmpHome, + ...isolatedHomeEnv(), TESTSPRITE_API_KEY: undefined, TESTSPRITE_API_URL: undefined, ...envOverrides, diff --git a/test/help.snapshot.test.ts b/test/help.snapshot.test.ts index ee89b3b..5cd504d 100644 --- a/test/help.snapshot.test.ts +++ b/test/help.snapshot.test.ts @@ -10,6 +10,7 @@ */ import { execFileSync } from 'node:child_process'; +import { execNpm } from './helpers/execNpm.js'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { beforeAll, describe, expect, it } from 'vitest'; @@ -47,7 +48,7 @@ const cases: Array<[string, string[]]> = [ describe('--help snapshots', () => { beforeAll(() => { - execFileSync('npm', ['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); + execNpm(['run', 'build'], { cwd: REPO_ROOT, stdio: 'pipe' }); }); for (const [name, args] of cases) { diff --git a/test/helpers/execNpm.ts b/test/helpers/execNpm.ts new file mode 100644 index 0000000..7a56f84 --- /dev/null +++ b/test/helpers/execNpm.ts @@ -0,0 +1,12 @@ +import { execFileSync } from 'node:child_process'; + +/** Cross-platform `npm` invocation (Windows needs `shell: true` for `.cmd` shims). */ +export function execNpm( + args: string[], + options: { cwd: string; stdio?: 'pipe' | 'inherit' | 'ignore' }, +): Buffer | string { + return execFileSync('npm', args, { + ...options, + shell: process.platform === 'win32', + }); +}