Skip to content

Commit d72290d

Browse files
authored
fix(test): guard default artifact run id path (#172)
* fix(test): validate artifact run id default path * fix(test): reject windows dot artifact run ids * fix(test): reject windows dot-suffix artifact run ids * style(test): format artifact run id guard --------- Co-authored-by: Lexiie <28455136+Lexiie@users.noreply.github.com>
1 parent 17650be commit d72290d

2 files changed

Lines changed: 70 additions & 4 deletions

File tree

src/commands/test.artifact.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
assertOutDirParentExists,
2222
createTestArtifactCommand,
2323
createTestCommand,
24+
resolveDefaultArtifactDir,
2425
runArtifactGet,
2526
runFailureGet,
2627
} from './test.js';
@@ -322,6 +323,53 @@ describe('runArtifactGet', () => {
322323
}
323324
});
324325

326+
it('rejects path-like runId before auth or fetch when default --out is used', async () => {
327+
const fetchImpl = vi.fn<typeof globalThis.fetch>();
328+
329+
await expect(
330+
runArtifactGet(
331+
{
332+
profile: 'default',
333+
output: 'json',
334+
debug: false,
335+
runId: '../../outside',
336+
failedOnly: false,
337+
},
338+
{ fetchImpl, stdout: () => {} },
339+
),
340+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
341+
342+
expect(fetchImpl).not.toHaveBeenCalled();
343+
});
344+
345+
it.each([
346+
'.',
347+
'..',
348+
'. ',
349+
'.. ',
350+
'...',
351+
'.. .',
352+
'. .',
353+
'../outside',
354+
'..\\outside',
355+
'nested/run',
356+
'nested\\run',
357+
'bad\0id',
358+
])('rejects unsafe default artifact runId segment %j', runId => {
359+
expect(() => resolveDefaultArtifactDir(runId, '/repo')).toThrowError(
360+
expect.objectContaining({
361+
code: 'VALIDATION_ERROR',
362+
details: expect.objectContaining({ field: 'run-id' }),
363+
}),
364+
);
365+
});
366+
367+
it('keeps the documented default directory for path-safe runIds', () => {
368+
expect(resolveDefaultArtifactDir(SAMPLE_RUN_ID, '/repo')).toBe(
369+
join('/repo', '.testsprite', 'runs', SAMPLE_RUN_ID),
370+
);
371+
});
372+
325373
// ---- --failed-only passed through to writeBundle ----
326374

327375
it('passes --failed-only through to writeBundle (steps filtered to failed ± 1)', async () => {

src/commands/test.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7169,6 +7169,25 @@ export interface ArtifactGetResult {
71697169
bundle?: WriteBundleResult;
71707170
}
71717171

7172+
export function resolveDefaultArtifactDir(runId: string, cwd: string = process.cwd()): string {
7173+
requireNonEmpty('run-id', runId);
7174+
const windowsNormalizedSegment = runId.replace(/[ .]+$/u, '');
7175+
if (
7176+
windowsNormalizedSegment === '' ||
7177+
windowsNormalizedSegment === '.' ||
7178+
windowsNormalizedSegment === '..' ||
7179+
runId.includes('/') ||
7180+
runId.includes('\\') ||
7181+
runId.includes('\0')
7182+
) {
7183+
throw localValidationError(
7184+
'run-id',
7185+
'must be a single path-safe segment for the default output directory; pass --out <dir> to choose a custom path',
7186+
);
7187+
}
7188+
return join(cwd, '.testsprite', 'runs', runId);
7189+
}
7190+
71727191
/**
71737192
* Validate that the parent directory of `resolvedDir` exists and is a
71747193
* directory. Surfaces `VALIDATION_ERROR` (exit 5) — matches the convention
@@ -7218,14 +7237,11 @@ export async function runArtifactGet(
72187237
deps: TestDeps = {},
72197238
): Promise<ArtifactGetResult> {
72207239
const out = makeOutput(opts.output, deps);
7221-
const client = makeClient(opts, deps);
72227240
const { runId } = opts;
72237241

72247242
// Resolve output dir: explicit --out or the default .testsprite/runs/<runId>/
72257243
const resolvedDir =
7226-
opts.out !== undefined
7227-
? resolveBundleDir(opts.out)
7228-
: join(process.cwd(), '.testsprite', 'runs', runId);
7244+
opts.out !== undefined ? resolveBundleDir(opts.out) : resolveDefaultArtifactDir(runId);
72297245

72307246
// --dry-run: no network, no disk write.
72317247
// The client (makeClient) is already wired with createDryRunFetch() when
@@ -7271,6 +7287,8 @@ export async function runArtifactGet(
72717287
await assertOutDirParentExists(resolvedDir);
72727288
}
72737289

7290+
const client = makeClient(opts, deps);
7291+
72747292
// Fetch the run-scoped failure bundle.
72757293
const { body: context, requestId: fetchRequestId } = await client.getWithMeta<CliFailureContext>(
72767294
`/runs/${encodeURIComponent(runId)}/failure`,

0 commit comments

Comments
 (0)