fix(test): guard default artifact run id path

[merlinsantiago982-cmd]

Summary

• add a default artifact directory resolver that treats run-id as one path-safe segment
• reject path-like or NUL-containing run IDs before auth/fetch when --out is omitted
• keep explicit --out behavior unchanged

Testing

• npm test -- src/commands/test.artifact-path.spec.ts
• npm run typecheck
• npm run format:check
• npm run lint
• npm run build

Fixes #45

Summary by CodeRabbit

• Bug Fixes
  • Improved default artifact download location to reliably save results under the expected runs directory for the given run ID.
  • Added stricter run ID validation, preventing unsafe values from being used unless a specific output directory is provided.
• Tests
  • Added regression tests covering default path containment, early rejection of unsafe run IDs, and behavior when an explicit output directory is set.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 178ed046-1e6b-4b4f-a33b-32e4d28d2103

📥 Commits

Reviewing files that changed from the base of the PR and between 0d70e2a and 5e71f31.

📒 Files selected for processing (2)

• src/commands/test.artifact-path.spec.ts
• src/commands/test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/commands/test.artifact-path.spec.ts
• src/commands/test.ts

---

📝 Walkthrough

Walkthrough

Adds resolveDefaultArtifactDir(runId, cwd?) to validate default artifact paths, uses it in runArtifactGet when --out is omitted, and adds regression coverage for safe, unsafe, and explicit-output cases.

Changes

Artifact default output path safety

Layer / File(s)	Summary
resolveDefaultArtifactDir helper and runArtifactGet wiring src/commands/test.ts	Adds exported resolveDefaultArtifactDir validating runId against path traversal and null bytes, returning cwd/.testsprite/runs/<runId>; runArtifactGet now uses it for the implicit output path, with a minor local variable reorder.
Regression tests for path validation src/commands/test.artifact-path.spec.ts	Adds helpers and tests covering valid default resolution, unsafe runId rejection, runArtifactGet failing before fetch for unsafe defaults, and explicit out handling for path-like ids.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped by the runs with a tiny soft thump,
No ../ tricks can make the output jump.
Safe run-ids stay snug in their bunny home nest,
And --out still works for the paths you request.
Hop hop! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately describes the main change: guarding the default artifact run-id path.
Linked Issues check	✅ Passed	The changes satisfy #45 by validating unsafe run IDs, preserving the default .testsprite/runs path, keeping --out behavior, and adding regression tests.
Out of Scope Changes check	✅ Passed	No unrelated functionality appears added; the changes stay focused on artifact path validation and its tests.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/commands/test.artifact-path.spec.ts (1)

31-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the explicit --out regression this issue calls for.

These tests cover the defaulted-path rejection path well, but they do not lock in the counterpart requirement that path-like runId values remain allowed when --out is provided. A small test on the opts.out !== undefined branch would keep that acceptance criterion from regressing.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/test.artifact-path.spec.ts` around lines 31 - 56, The current
test only verifies that unsafe default run IDs are rejected when no output path
is provided; add a regression test around runArtifactGet that explicitly sets
opts.out and confirms a path-like runId is still accepted. Use the existing
runArtifactGet helper in src/commands/test.artifact-path.spec.ts, cover the
opts.out !== undefined branch, and assert the command does not fail validation
for a value like ../../outside when --out is present.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/test.ts`:
- Around line 6740-6750: The run-id validation in test.ts only rejects literal
“.” and “..”, so update the existing check around the runId guard to also catch
Windows-trimmed dot segments before building the default
.testsprite/runs/<runId> path. Normalize or trim the candidate segment in the
same validation path, then reject aliases like “. ” and “.. ” in the
localValidationError('run-id', ...) branch, and add regression coverage in the
test cases that exercise runId handling.

---

Nitpick comments:
In `@src/commands/test.artifact-path.spec.ts`:
- Around line 31-56: The current test only verifies that unsafe default run IDs
are rejected when no output path is provided; add a regression test around
runArtifactGet that explicitly sets opts.out and confirms a path-like runId is
still accepted. Use the existing runArtifactGet helper in
src/commands/test.artifact-path.spec.ts, cover the opts.out !== undefined
branch, and assert the command does not fail validation for a value like
../../outside when --out is present.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 02487068-f1d8-44f0-b619-439cdd7ea259

📥 Commits

Reviewing files that changed from the base of the PR and between c4ebb9c and 0d70e2a.

📒 Files selected for processing (2)

• src/commands/test.artifact-path.spec.ts
• src/commands/test.ts