From b3b12e65f3a8600277ca70e7231db3a71485a6b7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 16:19:15 +0000 Subject: [PATCH 1/2] feat: auto-copy input files to staging directory in upload_artifact handler When the agent provides a path to a file or directory that is not already in the staging directory (/tmp/gh-aw/safeoutputs/upload-artifacts/), the handler now automatically copies it from its original location before proceeding with the upload. Supports absolute paths, GITHUB_WORKSPACE- relative paths, and cwd-relative paths. Symlinks are still rejected. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4682b046-7ef3-4d00-a4de-18dfab17cfd7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/safe_outputs_tools.json | 4 +- actions/setup/js/upload_artifact.cjs | 177 ++++++++++++++++++++-- actions/setup/js/upload_artifact.test.cjs | 105 ++++++++++++- pkg/workflow/js/safe_outputs_tools.json | 4 +- 4 files changed, 269 insertions(+), 21 deletions(-) diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index ce0cbcd65e1..23cca70a837 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -851,13 +851,13 @@ }, { "name": "upload_artifact", - "description": "Upload files as a run-scoped GitHub Actions artifact. The model must first copy files to /tmp/gh-aw/safeoutputs/upload-artifacts/ then request upload using this tool. Returns a temporary artifact ID (aw_*) that can be resolved to a download URL by an authorised step. Retention and archive settings are fixed by workflow configuration. Exactly one of path or filters must be present.", + "description": "Upload files as a run-scoped GitHub Actions artifact. Files can be pre-staged in /tmp/gh-aw/safeoutputs/upload-artifacts/ or referenced by their original path — files not already in the staging directory are automatically copied there before upload. Absolute paths and paths relative to the workspace are supported. Returns a temporary artifact ID (aw_*) that can be resolved to a download URL by an authorised step. Retention and archive settings are fixed by workflow configuration. Exactly one of path or filters must be present.", "inputSchema": { "type": "object", "properties": { "path": { "type": "string", - "description": "Path to the file or directory to upload, relative to /tmp/gh-aw/safeoutputs/upload-artifacts/ (e.g., \"report.json\" or \"dist/\"). Required unless filters is provided." + "description": "Path to the file or directory to upload. Can be relative to /tmp/gh-aw/safeoutputs/upload-artifacts/, an absolute path, or a path relative to the workspace. Files not already in the staging directory are automatically copied there. Required unless filters is provided." }, "filters": { "type": "object", diff --git a/actions/setup/js/upload_artifact.cjs b/actions/setup/js/upload_artifact.cjs index eb4d34eacb4..b7e70f3cd37 100644 --- a/actions/setup/js/upload_artifact.cjs +++ b/actions/setup/js/upload_artifact.cjs @@ -6,8 +6,11 @@ * * Validates artifact upload requests emitted by the model via the upload_artifact safe output * tool, then uploads the approved files directly via the @actions/artifact REST API client. - * The model must have already copied the files it wants to upload to - * /tmp/gh-aw/safeoutputs/upload-artifacts/ before calling the tool. + * + * Files can be pre-staged in /tmp/gh-aw/safeoutputs/upload-artifacts/ or referenced by their + * original path. When a requested path is not found in the staging directory the handler + * automatically copies the file (or directory) from its original location — supporting + * absolute paths, workspace-relative paths, and cwd-relative paths. * * This handler follows the per-message handler pattern used by the safe_outputs handler loop. * main(config) returns a per-message handler function that: @@ -111,9 +114,135 @@ function listFilesRecursive(dir, baseDir) { return files; } +/** + * Copy a single file to the staging directory, preserving the relative path structure. + * Rejects symlinks and creates intermediate directories as needed. + * + * @param {string} sourcePath - Absolute path to the source file + * @param {string} destRelPath - Relative path within the staging directory + * @returns {{ error: string|null }} + */ +function copySingleFileToStaging(sourcePath, destRelPath) { + const destPath = path.join(STAGING_DIR, destRelPath); + const stat = fs.lstatSync(sourcePath); + if (stat.isSymbolicLink()) { + return { error: `symlinks are not allowed: ${sourcePath}` }; + } + if (!stat.isFile()) { + return { error: `not a regular file: ${sourcePath}` }; + } + fs.mkdirSync(path.dirname(destPath), { recursive: true }); + fs.copyFileSync(sourcePath, destPath); + return { error: null }; +} + +/** + * Recursively copy a directory into the staging directory. + * Skips symlinks and logs warnings for them. + * + * @param {string} sourceDir - Absolute path to the source directory + * @param {string} destRelDir - Relative directory path within the staging directory + * @returns {{ copiedCount: number, error: string|null }} + */ +function copyDirectoryToStaging(sourceDir, destRelDir) { + let copiedCount = 0; + const entries = fs.readdirSync(sourceDir, { withFileTypes: true }); + for (const entry of entries) { + const srcFull = path.join(sourceDir, entry.name); + const destRel = path.join(destRelDir, entry.name); + const stat = fs.lstatSync(srcFull); + if (stat.isSymbolicLink()) { + core.warning(`Skipping symlink during auto-copy: ${srcFull}`); + continue; + } + if (entry.isDirectory()) { + const sub = copyDirectoryToStaging(srcFull, destRel); + if (sub.error) return sub; + copiedCount += sub.copiedCount; + } else if (entry.isFile()) { + const result = copySingleFileToStaging(srcFull, destRel); + if (result.error) return { copiedCount, error: result.error }; + copiedCount++; + } + } + return { copiedCount, error: null }; +} + +/** + * Attempt to locate the requested path outside the staging directory and copy it in. + * + * Search order for absolute paths: + * 1. Use the absolute path directly. + * + * Search order for relative paths: + * 1. GITHUB_WORKSPACE environment variable (GitHub Actions workspace). + * 2. Current working directory (process.cwd()). + * + * @param {string} reqPath - The path from the request (absolute or relative) + * @returns {{ copied: boolean, relPath: string, error: string|null }} + */ +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 }; + } + + // Relative path: search in GITHUB_WORKSPACE, then cwd. + const searchRoots = []; + if (process.env.GITHUB_WORKSPACE) { + searchRoots.push(process.env.GITHUB_WORKSPACE); + } + const cwd = process.cwd(); + if (!searchRoots.includes(cwd)) { + searchRoots.push(cwd); + } + + for (const root of searchRoots) { + const candidate = path.resolve(root, reqPath); + if (!fs.existsSync(candidate)) continue; + const stat = fs.lstatSync(candidate); + if (stat.isSymbolicLink()) { + return { copied: false, relPath: "", error: `symlinks are not allowed: ${candidate}` }; + } + if (stat.isDirectory()) { + const result = copyDirectoryToStaging(candidate, reqPath); + if (result.error) return { copied: false, relPath: "", error: result.error }; + core.info(`Auto-copied directory ${candidate} to staging as ${reqPath} (${result.copiedCount} file(s))`); + } else { + const result = copySingleFileToStaging(candidate, reqPath); + if (result.error) return { copied: false, relPath: "", error: result.error }; + core.info(`Auto-copied file ${candidate} to staging as ${reqPath}`); + } + return { copied: true, relPath: reqPath, error: null }; + } + + return { copied: false, relPath: "", error: null }; +} + /** * Resolve the list of files to upload for a single request. - * Applies: staging root → allowed-paths → request include/exclude → dedup + sort. + * Applies: staging root → auto-copy → allowed-paths → request include/exclude → dedup + sort. + * + * If a path-based request refers to a file that is not in the staging directory but exists + * elsewhere (absolute path, workspace, or cwd), the file is automatically copied into the + * staging directory before resolution continues. * * @param {Record} request - Parsed upload_artifact record * @param {string[]} allowedPaths - Policy allowed-paths patterns @@ -131,30 +260,50 @@ function resolveFiles(request, allowedPaths, defaultInclude, defaultExclude) { let candidateRelPaths; if ("path" in request) { - const reqPath = String(request.path); - // Reject absolute paths + let reqPath = String(request.path); + + // For absolute paths, attempt auto-copy to staging. if (path.isAbsolute(reqPath)) { - return { files: [], error: `path must be relative (staging-dir-relative), got absolute path: ${reqPath}` }; + const copyResult = autoCopyToStaging(reqPath); + if (copyResult.error) { + return { files: [], error: copyResult.error }; + } + if (!copyResult.copied) { + return { files: [], error: `path must be relative (staging-dir-relative), got absolute path: ${reqPath}` }; + } + // Switch to the relative path inside the staging directory. + reqPath = copyResult.relPath; } + // Reject traversal const resolved = path.resolve(STAGING_DIR, reqPath); if (!isWithinRoot(resolved, STAGING_DIR)) { return { files: [], error: `path must not traverse outside staging directory: ${reqPath}` }; } + + // If the path does not exist in staging, try auto-copy from workspace/cwd. if (!fs.existsSync(resolved)) { - const available = listFilesRecursive(STAGING_DIR, STAGING_DIR); - const hint = - available.length > 0 - ? ` Available files: [${available.slice(0, 20).join(", ")}]${available.length > 20 ? ` … and ${available.length - 20} more` : ""}` - : " The staging directory is empty — did you forget to copy files to " + STAGING_DIR + "?"; - return { files: [], error: `path does not exist in staging directory: ${reqPath}.${hint}` }; + const copyResult = autoCopyToStaging(reqPath); + if (copyResult.error) { + return { files: [], error: copyResult.error }; + } + if (!copyResult.copied) { + const available = listFilesRecursive(STAGING_DIR, STAGING_DIR); + const hint = + available.length > 0 + ? ` Available files: [${available.slice(0, 20).join(", ")}]${available.length > 20 ? ` … and ${available.length - 20} more` : ""}` + : " The staging directory is empty — did you forget to copy files to " + STAGING_DIR + "?"; + return { files: [], error: `path does not exist in staging directory: ${reqPath}.${hint}` }; + } + reqPath = copyResult.relPath; } - const stat = fs.lstatSync(resolved); + + const stat = fs.lstatSync(path.resolve(STAGING_DIR, reqPath)); if (stat.isSymbolicLink()) { return { files: [], error: `symlinks are not allowed: ${reqPath}` }; } if (stat.isDirectory()) { - candidateRelPaths = listFilesRecursive(resolved, STAGING_DIR); + candidateRelPaths = listFilesRecursive(path.resolve(STAGING_DIR, reqPath), STAGING_DIR); } else { candidateRelPaths = [reqPath]; } diff --git a/actions/setup/js/upload_artifact.test.cjs b/actions/setup/js/upload_artifact.test.cjs index 73f0b529e6f..793797fab30 100644 --- a/actions/setup/js/upload_artifact.test.cjs +++ b/actions/setup/js/upload_artifact.test.cjs @@ -164,9 +164,9 @@ describe("upload_artifact.cjs", () => { expect(mockArtifactClient.uploadArtifact).not.toHaveBeenCalled(); }); - it("fails when absolute path is provided", async () => { - await runHandler(buildConfig(), [{ type: "upload_artifact", path: "/etc/passwd" }]); - expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("must be relative")); + it("fails when absolute path does not exist", async () => { + await runHandler(buildConfig(), [{ type: "upload_artifact", path: "/nonexistent/path/file.json" }]); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("absolute path does not exist")); expect(mockArtifactClient.uploadArtifact).not.toHaveBeenCalled(); }); @@ -334,4 +334,103 @@ describe("upload_artifact.cjs", () => { expect(keys[0]).toMatch(/^aw_[A-Za-z0-9]{8}$/); }); }); + + describe("auto-copy from outside staging directory", () => { + const WORKSPACE_DIR = "/tmp/gh-aw-test-workspace"; + + beforeEach(() => { + if (fs.existsSync(WORKSPACE_DIR)) { + fs.rmSync(WORKSPACE_DIR, { recursive: true }); + } + fs.mkdirSync(WORKSPACE_DIR, { recursive: true }); + }); + + afterEach(() => { + if (fs.existsSync(WORKSPACE_DIR)) { + fs.rmSync(WORKSPACE_DIR, { recursive: true }); + } + }); + + /** + * Write a file into the test workspace directory. + * @param {string} relPath + * @param {string} content + */ + function writeWorkspace(relPath, content = "workspace content") { + const fullPath = path.join(WORKSPACE_DIR, relPath); + fs.mkdirSync(path.dirname(fullPath), { recursive: true }); + fs.writeFileSync(fullPath, content); + } + + it("auto-copies a file from an absolute path", async () => { + const absFile = path.join(WORKSPACE_DIR, "report.json"); + writeWorkspace("report.json", '{"ok":true}'); + + const results = await runHandler(buildConfig(), [{ type: "upload_artifact", path: absFile }]); + + expect(results[0].success).toBe(true); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(mockArtifactClient.uploadArtifact).toHaveBeenCalledOnce(); + // The file should have been copied into the staging directory. + expect(fs.existsSync(path.join(STAGING_DIR, "report.json"))).toBe(true); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Auto-copied file")); + }); + + it("auto-copies a directory from an absolute path", async () => { + writeWorkspace("output/a.txt", "aaa"); + writeWorkspace("output/sub/b.txt", "bbb"); + const absDir = path.join(WORKSPACE_DIR, "output"); + + const results = await runHandler(buildConfig(), [{ type: "upload_artifact", path: absDir }]); + + expect(results[0].success).toBe(true); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(STAGING_DIR, "output/a.txt"))).toBe(true); + expect(fs.existsSync(path.join(STAGING_DIR, "output/sub/b.txt"))).toBe(true); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Auto-copied directory")); + }); + + it("auto-copies a relative path from GITHUB_WORKSPACE", async () => { + process.env.GITHUB_WORKSPACE = WORKSPACE_DIR; + writeWorkspace("data/results.csv", "col1,col2"); + + const results = await runHandler(buildConfig(), [{ type: "upload_artifact", path: "data/results.csv" }]); + + expect(results[0].success).toBe(true); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(STAGING_DIR, "data/results.csv"))).toBe(true); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Auto-copied file")); + }); + + it("fails for an absolute path that does not exist", async () => { + const absFile = path.join(WORKSPACE_DIR, "missing.json"); + + await runHandler(buildConfig(), [{ type: "upload_artifact", path: absFile }]); + + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("absolute path does not exist")); + }); + + it("still prefers files already in the staging directory", async () => { + process.env.GITHUB_WORKSPACE = WORKSPACE_DIR; + writeStaging("report.json", "staged version"); + writeWorkspace("report.json", "workspace version"); + + const results = await runHandler(buildConfig(), [{ type: "upload_artifact", path: "report.json" }]); + + expect(results[0].success).toBe(true); + // Verify the staged version was used (not overwritten by the workspace version). + const content = fs.readFileSync(path.join(STAGING_DIR, "report.json"), "utf8"); + expect(content).toBe("staged version"); + }); + + it("rejects symlinks during auto-copy from absolute path", async () => { + writeWorkspace("real.txt", "real content"); + const linkPath = path.join(WORKSPACE_DIR, "link.txt"); + fs.symlinkSync(path.join(WORKSPACE_DIR, "real.txt"), linkPath); + + await runHandler(buildConfig(), [{ type: "upload_artifact", path: linkPath }]); + + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("symlinks are not allowed")); + }); + }); }); diff --git a/pkg/workflow/js/safe_outputs_tools.json b/pkg/workflow/js/safe_outputs_tools.json index a4efcda4ead..9adf16ec2c2 100644 --- a/pkg/workflow/js/safe_outputs_tools.json +++ b/pkg/workflow/js/safe_outputs_tools.json @@ -1586,13 +1586,13 @@ }, { "name": "upload_artifact", - "description": "Upload files as a run-scoped GitHub Actions artifact. The model must first copy files to /tmp/gh-aw/safeoutputs/upload-artifacts/ then request upload using this tool. Returns a temporary artifact ID (aw_*) that can be resolved to a download URL by an authorised step. Retention and archive settings are fixed by workflow configuration. Exactly one of path or filters must be present.", + "description": "Upload files as a run-scoped GitHub Actions artifact. Files can be pre-staged in /tmp/gh-aw/safeoutputs/upload-artifacts/ or referenced by their original path — files not already in the staging directory are automatically copied there before upload. Absolute paths and paths relative to the workspace are supported. Returns a temporary artifact ID (aw_*) that can be resolved to a download URL by an authorised step. Retention and archive settings are fixed by workflow configuration. Exactly one of path or filters must be present.", "inputSchema": { "type": "object", "properties": { "path": { "type": "string", - "description": "Path to the file or directory to upload, relative to /tmp/gh-aw/safeoutputs/upload-artifacts/ (e.g., \"report.json\" or \"dist/\"). Required unless filters is provided." + "description": "Path to the file or directory to upload. Can be relative to /tmp/gh-aw/safeoutputs/upload-artifacts/, an absolute path, or a path relative to the workspace. Files not already in the staging directory are automatically copied there. Required unless filters is provided." }, "filters": { "type": "object", From 71f98f3ec2f3217a594fcc4c8247621ddfebcfff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 16:39:48 +0000 Subject: [PATCH 2/2] fix: prevent copySingleFileToStaging from overwriting pre-staged files Add an existence check to skip copying when the destination file already exists in the staging directory. This prevents accidental overwrites of pre-staged files, particularly with basename-based absolute-path copies where collisions are more likely. Adds a dedicated test for this case. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b2907f12-f664-4fcb-ab8e-3693b58dd893 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/upload_artifact.cjs | 5 +++++ actions/setup/js/upload_artifact.test.cjs | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/actions/setup/js/upload_artifact.cjs b/actions/setup/js/upload_artifact.cjs index b7e70f3cd37..4b94d114d8e 100644 --- a/actions/setup/js/upload_artifact.cjs +++ b/actions/setup/js/upload_artifact.cjs @@ -124,6 +124,11 @@ function listFilesRecursive(dir, baseDir) { */ function copySingleFileToStaging(sourcePath, destRelPath) { const destPath = path.join(STAGING_DIR, destRelPath); + // Never overwrite a file that is already staged — the pre-staged version takes precedence. + if (fs.existsSync(destPath)) { + core.info(`Skipping auto-copy for ${destRelPath}: already exists in staging directory`); + return { error: null }; + } const stat = fs.lstatSync(sourcePath); if (stat.isSymbolicLink()) { return { error: `symlinks are not allowed: ${sourcePath}` }; diff --git a/actions/setup/js/upload_artifact.test.cjs b/actions/setup/js/upload_artifact.test.cjs index 793797fab30..4920550342d 100644 --- a/actions/setup/js/upload_artifact.test.cjs +++ b/actions/setup/js/upload_artifact.test.cjs @@ -423,6 +423,20 @@ describe("upload_artifact.cjs", () => { expect(content).toBe("staged version"); }); + it("does not overwrite pre-staged file when auto-copying from absolute path", async () => { + writeStaging("data.json", "original staged"); + writeWorkspace("data.json", "workspace version"); + const absFile = path.join(WORKSPACE_DIR, "data.json"); + + const results = await runHandler(buildConfig(), [{ type: "upload_artifact", path: absFile }]); + + expect(results[0].success).toBe(true); + // The pre-staged file must not be overwritten. + const content = fs.readFileSync(path.join(STAGING_DIR, "data.json"), "utf8"); + expect(content).toBe("original staged"); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("already exists in staging")); + }); + it("rejects symlinks during auto-copy from absolute path", async () => { writeWorkspace("real.txt", "real content"); const linkPath = path.join(WORKSPACE_DIR, "link.txt");