From 0d70e2af60e041fe942428e36c35919cb10e924b Mon Sep 17 00:00:00 2001 From: merlinsantiago982-cmd Date: Tue, 30 Jun 2026 10:35:15 +0800 Subject: [PATCH 1/2] guard default artifact run id path --- src/commands/test.artifact-path.spec.ts | 56 +++++++++++++++++++++++++ src/commands/test.ts | 24 +++++++++-- 2 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 src/commands/test.artifact-path.spec.ts diff --git a/src/commands/test.artifact-path.spec.ts b/src/commands/test.artifact-path.spec.ts new file mode 100644 index 0000000..f75b512 --- /dev/null +++ b/src/commands/test.artifact-path.spec.ts @@ -0,0 +1,56 @@ +/** + * Regression coverage for default artifact output path containment. + */ + +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { resolveDefaultArtifactDir, runArtifactGet } from './test.js'; + +describe('resolveDefaultArtifactDir', () => { + it('keeps a normal run id under .testsprite/runs', () => { + const cwd = join(tmpdir(), 'artifact-path'); + expect(resolveDefaultArtifactDir('run_abc123', cwd)).toBe( + join(cwd, '.testsprite', 'runs', 'run_abc123'), + ); + }); + + it.each(['.', '..', '../outside', '..\\outside', 'nested/run', 'nested\\run', 'bad\0id'])( + 'rejects path-like run id %j', + runId => { + expect(() => resolveDefaultArtifactDir(runId, '/tmp/work')).toThrowError( + expect.objectContaining({ + code: 'VALIDATION_ERROR', + details: expect.objectContaining({ field: 'run-id' }), + }), + ); + }, + ); +}); + +describe('runArtifactGet default output path validation', () => { + it('rejects unsafe default run ids before auth or fetch work', async () => { + let fetchCalls = 0; + const fetchImpl = (async () => { + fetchCalls++; + return new Response('{}', { status: 200 }); + }) as typeof globalThis.fetch; + + await expect( + runArtifactGet( + { + profile: 'default', + output: 'json', + debug: false, + runId: '../../outside', + failedOnly: false, + }, + { fetchImpl, stdout: () => {} }, + ), + ).rejects.toMatchObject({ + code: 'VALIDATION_ERROR', + details: expect.objectContaining({ field: 'run-id' }), + }); + expect(fetchCalls).toBe(0); + }); +}); diff --git a/src/commands/test.ts b/src/commands/test.ts index a86b929..bf3db18 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -6735,6 +6735,23 @@ export async function assertOutDirParentExists(resolvedDir: string): Promise to choose a custom path', + ); + } + return join(cwd, '.testsprite', 'runs', runId); +} + /** * `test artifact get ` — run-scoped failure-bundle download. * @@ -6752,14 +6769,11 @@ export async function runArtifactGet( deps: TestDeps = {}, ): Promise { const out = makeOutput(opts.output, deps); - const client = makeClient(opts, deps); const { runId } = opts; // Resolve output dir: explicit --out or the default .testsprite/runs// const resolvedDir = - opts.out !== undefined - ? resolveBundleDir(opts.out) - : join(process.cwd(), '.testsprite', 'runs', runId); + opts.out !== undefined ? resolveBundleDir(opts.out) : resolveDefaultArtifactDir(runId); // --dry-run: no network, no disk write. // The client (makeClient) is already wired with createDryRunFetch() when @@ -6799,6 +6813,8 @@ export async function runArtifactGet( return { context: cannedCtx }; } + const client = makeClient(opts, deps); + // Parent-dir validation for explicit --out only. The default path // (.testsprite/runs//) is always under cwd — mkdir will create it. if (opts.out !== undefined) { From 5e71f316cd2ec138a26c4e314b6822ea58711b3a Mon Sep 17 00:00:00 2001 From: merlinsantiago982-cmd Date: Tue, 30 Jun 2026 10:57:23 +0800 Subject: [PATCH 2/2] address artifact path review feedback --- src/commands/test.artifact-path.spec.ts | 109 +++++++++++++++++++++--- src/commands/test.ts | 5 +- 2 files changed, 100 insertions(+), 14 deletions(-) diff --git a/src/commands/test.artifact-path.spec.ts b/src/commands/test.artifact-path.spec.ts index f75b512..be465b7 100644 --- a/src/commands/test.artifact-path.spec.ts +++ b/src/commands/test.artifact-path.spec.ts @@ -3,9 +3,60 @@ */ import { tmpdir } from 'node:os'; +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; -import { resolveDefaultArtifactDir, runArtifactGet } from './test.js'; +import { type CliFailureContext, resolveDefaultArtifactDir, runArtifactGet } from './test.js'; + +function makeCreds(): string { + const dir = mkdtempSync(join(tmpdir(), 'artifact-path-creds-')); + const credentialsPath = join(dir, 'credentials'); + writeFileSync( + credentialsPath, + '[default]\napi_url = http://localhost:14400\napi_key = sk-test\n', + { + mode: 0o600, + }, + ); + return credentialsPath; +} + +function makeContext(runId: string): CliFailureContext { + return { + snapshotId: 'snap_path_test', + testId: 'test_path', + projectId: 'project_path', + result: { + testId: 'test_path', + status: 'failed', + startedAt: null, + finishedAt: '2026-06-30T02:40:00.000Z', + videoUrl: null, + failureAnalysisUrl: null, + snapshotId: 'snap_path_test', + runIdIfAvailable: runId, + codeVersion: 'v1', + targetUrl: 'https://example.com', + failedStepIndex: null, + failureKind: 'assertion', + summary: { passed: 0, failed: 1, skipped: 0 }, + }, + steps: [], + code: { + testId: 'test_path', + language: 'typescript', + framework: 'playwright', + code: 'test("path", () => {});', + codeVersion: 'v1', + etag: null, + }, + failure: { + rootCauseHypothesis: 'path test', + recommendedFixTarget: { kind: 'unknown', reference: null, rationale: null }, + evidence: [], + }, + }; +} describe('resolveDefaultArtifactDir', () => { it('keeps a normal run id under .testsprite/runs', () => { @@ -15,17 +66,24 @@ describe('resolveDefaultArtifactDir', () => { ); }); - it.each(['.', '..', '../outside', '..\\outside', 'nested/run', 'nested\\run', 'bad\0id'])( - 'rejects path-like run id %j', - runId => { - expect(() => resolveDefaultArtifactDir(runId, '/tmp/work')).toThrowError( - expect.objectContaining({ - code: 'VALIDATION_ERROR', - details: expect.objectContaining({ field: 'run-id' }), - }), - ); - }, - ); + it.each([ + '.', + '..', + '. ', + '.. ', + '../outside', + '..\\outside', + 'nested/run', + 'nested\\run', + 'bad\0id', + ])('rejects path-like run id %j', runId => { + expect(() => resolveDefaultArtifactDir(runId, '/tmp/work')).toThrowError( + expect.objectContaining({ + code: 'VALIDATION_ERROR', + details: expect.objectContaining({ field: 'run-id' }), + }), + ); + }); }); describe('runArtifactGet default output path validation', () => { @@ -53,4 +111,31 @@ describe('runArtifactGet default output path validation', () => { }); expect(fetchCalls).toBe(0); }); + + it('allows path-like run ids when an explicit --out directory is provided', async () => { + const runId = '../../outside'; + const tmp = mkdtempSync(join(tmpdir(), 'artifact-path-out-')); + const outDir = join(tmp, 'bundle'); + mkdirSync(tmp, { recursive: true }); + + const fetchImpl = (async () => + new Response(JSON.stringify(makeContext(runId)), { + status: 200, + headers: { 'content-type': 'application/json', 'x-request-id': 'req_path_test' }, + })) as typeof globalThis.fetch; + + const result = await runArtifactGet( + { + profile: 'default', + output: 'json', + debug: false, + runId, + out: outDir, + failedOnly: false, + }, + { credentialsPath: makeCreds(), fetchImpl, stdout: () => {} }, + ); + + expect(result.bundle?.dir).toBe(outDir); + }); }); diff --git a/src/commands/test.ts b/src/commands/test.ts index bf3db18..2078ac7 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -6737,9 +6737,10 @@ export async function assertOutDirParentExists(resolvedDir: string): Promise