Skip to content

fix(test): prevent path-like run IDs escaping artifact default output dir #45

Description

@Lexiie

Summary

testsprite test artifact get <run-id> uses <run-id> directly as the final path segment for its default output directory:

./.testsprite/runs/<run-id>/

Because the value is not validated as a path-safe segment, a path-like runId such as ../../outside can make the default bundle directory resolve outside the documented .testsprite/runs/ root when --out is omitted.

This is a local filesystem safety issue in the CLI default path construction. It is separate from explicit --out handling: users can already choose any custom output location with --out; the default path should remain constrained to the CLI-owned artifact tree.

Why this matters

The command is agent-facing and is often copied from CLI output after a failed run:

testsprite test artifact get <run-id>

A caller reasonably expects the default destination to be under:

./.testsprite/runs/<run-id>/

But if <run-id> contains path separators or traversal segments, Node path joining treats it as path structure rather than an opaque identifier. That can cause the bundle writer to create or overwrite files outside the documented artifact root.

Even if production run IDs are normally generated by TestSprite, the CLI should fail closed here because:

  • runId is a positional user input, not a trusted local path.
  • artifact get writes multiple files and directories.
  • --out already exists for intentional custom paths.
  • The documented default path implies containment under .testsprite/runs/.

Reproduction

From a clean checkout, call runArtifactGet without --out and with a path-like run id:

await runArtifactGet(
  {
    profile: 'default',
    output: 'json',
    debug: false,
    runId: '../../outside',
    failedOnly: false,
  },
  { fetchImpl, stdout: () => {} },
);

Current behavior: the default directory is constructed from the raw runId, equivalent to:

join(process.cwd(), '.testsprite', 'runs', '../../outside')

which normalizes outside ./.testsprite/runs/.

A CLI-level reproduction is the same shape:

testsprite test artifact get ../../outside --dry-run --output json

The dry-run path is especially useful as a contract check because it reports the default out value without network or disk writes.

Expected behavior

When --out is omitted, <run-id> should be treated as an opaque identifier and must be a single path-safe segment. Values containing path separators, traversal segments, or NUL bytes should fail before auth/network/disk work with a typed validation error:

{
  "error": {
    "code": "VALIDATION_ERROR",
    "details": {
      "field": "run-id"
    }
  }
}

If a caller intentionally wants a custom directory, they should use the existing explicit path mechanism:

testsprite test artifact get <run-id> --out ./some/custom/dir

Suggested fix

Add a small helper for the default artifact directory path:

function resolveDefaultArtifactDir(runId: string, cwd = process.cwd()): string {
  requireNonEmpty('run-id', runId);
  if (runId === '.' || runId === '..' || runId.includes('/') || runId.includes('\\')) {
    throw localValidationError(
      'run-id',
      'must be a single path-safe segment for the default output directory; pass --out <dir> to choose a custom path',
    );
  }
  if (runId.includes('\0')) {
    throw localValidationError('run-id', 'must not contain NUL bytes');
  }
  return join(cwd, '.testsprite', 'runs', runId);
}

Then use it in runArtifactGet only for the implicit default path:

const resolvedDir =
  opts.out !== undefined
    ? resolveBundleDir(opts.out)
    : resolveDefaultArtifactDir(runId);

It is also worth resolving/validating the output path before constructing the HTTP client so invalid local input fails without requiring credentials or making a request.

Acceptance criteria

  • test artifact get ../../outside without --out exits with VALIDATION_ERROR before auth/fetch.
  • Normal run IDs still use the documented default directory: ./.testsprite/runs/<run-id>/.
  • Explicit --out <dir> behavior remains unchanged.
  • Add regression coverage for both unsafe path-like run IDs and normal run IDs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions