Skip to content

refactor: use path.relative for traversal guard, wrap env mutation in try/finally#9

Merged
danielmeppiel merged 1 commit intomainfrom
fix/clearprimitives-review-feedback
Mar 10, 2026
Merged

refactor: use path.relative for traversal guard, wrap env mutation in try/finally#9
danielmeppiel merged 1 commit intomainfrom
fix/clearprimitives-review-feedback

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Addresses review feedback from #8:

  1. path.relative traversal guard — Replaces startsWith(resolved + path.sep) with path.relative() + isAbsolute() check. The previous approach breaks when resolved is the filesystem root (/), producing a // prefix that rejects valid subpaths. path.relative handles all edge cases correctly.

  2. try/finally for env mutation — The cross-workspace test mutates process.env.GITHUB_WORKSPACE. If an assertion or clearPrimitives throws before the restore block, the env var stays polluted for subsequent tests. Wrapping in try/finally guarantees cleanup.

… try/finally

- Replace startsWith prefix check with path.relative — handles edge
  cases like root directories where resolved + path.sep produces
  incorrect prefixes.
- Wrap GITHUB_WORKSPACE mutation in test with try/finally to guarantee
  restoration even when assertions fail.
Copilot AI review requested due to automatic review settings March 10, 2026 21:50
@danielmeppiel danielmeppiel merged commit a2b2dd5 into main Mar 10, 2026
13 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines the clearPrimitives safety logic and strengthens test isolation for the GitHub Action runner utilities, addressing edge cases and ensuring test environment cleanup.

Changes:

  • Replace the traversal guard from startsWith(resolved + path.sep) to path.relative() + path.isAbsolute() to correctly handle filesystem-root working directories.
  • Wrap process.env.GITHUB_WORKSPACE mutation in the cross-workspace test with try/finally to guarantee restoration even on failure.
  • Update the compiled dist/ output to reflect the TypeScript changes.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
src/runner.ts Uses path.relative()-based traversal guard to correctly validate subpaths within the resolved working directory, including root edge cases.
src/__tests__/runner.test.ts Ensures GITHUB_WORKSPACE is always restored via try/finally, preventing test pollution.
dist/index.js Regenerates bundled output to match updated source logic (required by CI).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants