feat: auto-copy input files to staging directory in upload_artifact handler

[Copilot]

Summary

When the agent provides a path to a file or directory that is not already in the upload-artifact staging directory (/tmp/gh-aw/safeoutputs/upload-artifacts/), the upload_artifact handler now automatically copies it from its original location before proceeding with the upload.

Previously, the agent was required to manually copy files into the staging directory before calling the upload_artifact tool. Files referenced by absolute paths were rejected, and files not found in the staging directory would fail with an error. This enhancement removes that friction.

Changes

actions/setup/js/upload_artifact.cjs

• Added copySingleFileToStaging() — copies a single file to the staging directory with symlink rejection
• Added copyDirectoryToStaging() — recursively copies a directory, skipping symlinks with warnings
• Added autoCopyToStaging() — orchestrates auto-copy for both absolute and relative paths, searching GITHUB_WORKSPACE and cwd as fallback roots
• Modified resolveFiles() to invoke auto-copy when:
  • An absolute path is provided (copies using basename as the relative path)
  • A relative path is not found in the staging directory (searches workspace/cwd and copies preserving relative structure)
• Updated JSDoc header to document the new behavior

actions/setup/js/safe_outputs_tools.json and pkg/workflow/js/safe_outputs_tools.json

• Updated upload_artifact tool description to mention auto-copy support
• Updated path field description to note that absolute paths and workspace-relative paths are now accepted

actions/setup/js/upload_artifact.test.cjs

• Added 6 new tests for auto-copy behavior:
  • Auto-copies a file from an absolute path
  • Auto-copies a directory from an absolute path
  • Auto-copies a relative path from GITHUB_WORKSPACE
  • Fails for an absolute path that does not exist
  • Prefers files already in the staging directory (no overwrite)
  • Rejects symlinks during auto-copy
• Updated existing "fails when absolute path is provided" test → now "fails when absolute path does not exist"

Security

• Symlinks are still rejected at every level (single file copy, directory copy, auto-copy detection)
• Path traversal validation still applied after auto-copy
• Size limits, allowed-paths policy, and all other validation still enforced
• No changes to the upload/artifact REST API interaction

[Copilot]

Pull request overview

This PR enhances the upload_artifact safe-output handler to automatically copy requested files/directories into the staging directory when they’re referenced from outside of it (supporting absolute paths and workspace/cwd-relative paths), then proceeds with the upload using the staged copy.

Changes:

• Added auto-copy logic to upload_artifact handler (single-file + recursive directory copy), including symlink handling.
• Updated resolveFiles() to trigger auto-copy when a requested path isn’t already present in staging (and for absolute paths).
• Updated tool descriptions and added tests covering the new auto-copy behavior.

Show a summary per file

File	Description
actions/setup/js/upload_artifact.cjs	Implements auto-copy-to-staging behavior and integrates it into file resolution.
actions/setup/js/upload_artifact.test.cjs	Adds tests for absolute/workspace auto-copy, symlink rejection, and staging precedence.
actions/setup/js/safe_outputs_tools.json	Updates upload_artifact tool docs to reflect auto-copy and absolute/workspace paths.
pkg/workflow/js/safe_outputs_tools.json	Mirrors the upload_artifact tool documentation updates.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (2)

actions/setup/js/upload_artifact.cjs:205

• Auto-copying arbitrary absolute paths allows the caller to exfiltrate any readable file on the runner (e.g. /etc/*, service credentials) even when the workflow has no allowed-paths policy configured. Consider restricting auto-copy sources to trusted roots (e.g. only within GITHUB_WORKSPACE and/or process.cwd()), rejecting absolute paths outside those roots, and preserving a workspace-relative destination path (instead of basename) so allowed-paths patterns remain meaningful and collisions are avoided.

function autoCopyToStaging(reqPath) {
  if (path.isAbsolute(reqPath)) {
    if (!fs.existsSync(reqPath)) {
      return { copied: false, relPath: "", error: `absolute path does not exist: ${reqPath}` };
    }
    const stat = fs.lstatSync(reqPath);
    if (stat.isSymbolicLink()) {
      return { copied: false, relPath: "", error: `symlinks are not allowed: ${reqPath}` };
    }
    // Derive a relative destination path from the basename (or relative to filesystem root for nested paths).
    const relPath = path.basename(reqPath);
    if (stat.isDirectory()) {
      const result = copyDirectoryToStaging(reqPath, relPath);
      if (result.error) return { copied: false, relPath: "", error: result.error };
      core.info(`Auto-copied directory ${reqPath} to staging (${result.copiedCount} file(s))`);
    } else {
      const result = copySingleFileToStaging(reqPath, relPath);
      if (result.error) return { copied: false, relPath: "", error: result.error };
      core.info(`Auto-copied file ${reqPath} to staging as ${relPath}`);
    }
    return { copied: true, relPath, error: null };
  }

actions/setup/js/upload_artifact.cjs:195

• The comment about deriving the destination path says "or relative to filesystem root for nested paths", but the implementation always uses path.basename(reqPath). Either adjust the comment or update the logic to match the described behavior.

      return { copied: false, relPath: "", error: `symlinks are not allowed: ${reqPath}` };
    }
    // Derive a relative destination path from the basename (or relative to filesystem root for nested paths).
    const relPath = path.basename(reqPath);
    if (stat.isDirectory()) {

• Files reviewed: 4/4 changed files
• Comments generated: 1

[Copilot]

copySingleFileToStaging() unconditionally overwrites an existing destination file in the staging directory. This can clobber pre-staged files (and makes basename-based absolute-path copies particularly risky due to collisions). Consider failing or skipping when destPath already exists, or generating a deterministic non-colliding destination path.

This issue also appears in the following locations of the same file:

• line 184
• line 191

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 87/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	7
✅ Design tests (behavioral contracts)	7 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	4 (57%)
Duplicate test clusters	0
Test inflation detected	No (test:prod ratio = 0.63:1)
🚨 Coding-guideline violations	None

---

Test Classification Details

All 7 test cases

Test	File	Classification	Notes
"fails when absolute path does not exist"	upload_artifact.test.cjs:164	✅ Design	Modified from old "must be relative" behavior; correctly reflects new contract
"auto-copies a file from an absolute path"	upload_artifact.test.cjs:370	✅ Design	Verifies real file-system side effect (file exists in staging), checks success result
"auto-copies a directory from an absolute path"	upload_artifact.test.cjs:385	✅ Design	Verifies recursive copy via real fs checks on nested paths
"auto-copies a relative path from GITHUB_WORKSPACE"	upload_artifact.test.cjs:400	✅ Design	Verifies GITHUB_WORKSPACE env-var resolution against real filesystem
"fails for an absolute path that does not exist"	upload_artifact.test.cjs:413	✅ Design	Error path — confirms correct failure message when source is missing
"still prefers files already in the staging directory"	upload_artifact.test.cjs:420	✅ Design	Edge case — reads file content to confirm staging version was not overwritten; high value
"rejects symlinks during auto-copy from absolute path"	upload_artifact.test.cjs:432	✅ Design	Security edge case — enforces symlink rejection contract

---

Highlights

Strengths:

• All tests use real filesystem I/O (fs.writeFileSync, fs.existsSync, fs.readFileSync) — no mocking of internal business logic. The only mocks are mockCore and mockArtifactClient, which are legitimate stubs for the external GitHub Actions runtime.
• "still prefers files already in the staging directory" is particularly high-value: it reads back file content to prove the correct version was retained, not just that success was reported.
• "rejects symlinks during auto-copy" enforces a security invariant that would be easy to forget.
• beforeEach/afterEach lifecycle hooks properly clean up the real temp directory, making tests hermetic.

Minor observation (not flagged):

• Three tests (auto-copies a file, auto-copies a directory, auto-copies a relative path) cover happy-path only. They each have strong behavioral assertions (file existence checks, results[0].success), but none of them include a negative assertion beyond not.toHaveBeenCalled() on setFailed. The overall suite is still well-balanced given the three dedicated error/edge-case tests.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 7 tests (vitest)

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). The new test suite directly verifies observable filesystem side-effects, error contracts, and a security invariant — all of which would catch real behavioral regressions if deleted.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 531K · ◷

[github-actions]

✅ Test Quality Sentinel: 87/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 7 new/modified tests verify observable behavioral contracts using real filesystem I/O, including error paths and a security invariant (symlink rejection).

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Addressed the reviewer's comment about copySingleFileToStaging() unconditionally overwriting pre-staged files. Added an existence check to skip copying when the destination already exists in the staging directory, plus a dedicated test for the absolute-path overwrite case. See commit 71f98f3.