diff --git a/pkg/cli/logs_download.go b/pkg/cli/logs_download.go index 715ab2f66b6..6a46080d6aa 100644 --- a/pkg/cli/logs_download.go +++ b/pkg/cli/logs_download.go @@ -11,11 +11,9 @@ package cli import ( - "archive/zip" "context" "errors" "fmt" - "io" "os" "path/filepath" "strconv" @@ -67,272 +65,6 @@ func shouldDownloadWorkflowRunLogs(artifactFilter []string) bool { return false } -// flattenSingleFileArtifacts checks artifact directories and flattens any that contain a single file -// This handles the case where gh CLI creates a directory for each artifact, even if it's just one file -func flattenSingleFileArtifacts(outputDir string, verbose bool) error { - logsDownloadLog.Printf("Flattening single-file artifacts in: %s", outputDir) - entries, err := os.ReadDir(outputDir) - if err != nil { - return fmt.Errorf("failed to read output directory: %w", err) - } - - for _, entry := range entries { - if !entry.IsDir() || entry.Name() == downloadedArtifactsMarkerDir { - continue - } - - artifactDir := filepath.Join(outputDir, entry.Name()) - - // Read contents of artifact directory - artifactEntries, err := os.ReadDir(artifactDir) - if err != nil { - logsDownloadLog.Printf("Failed to read artifact directory %s: %v", artifactDir, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to read artifact directory %s: %v", artifactDir, err))) - } - continue - } - - logsDownloadLog.Printf("Artifact directory %s contains %d entries", entry.Name(), len(artifactEntries)) - - // Apply unfold rule: Check if directory contains exactly one entry and it's a file - if len(artifactEntries) != 1 { - if verbose && len(artifactEntries) > 1 { - // Log what's in multi-file artifacts for debugging - var fileNames []string - for _, e := range artifactEntries { - fileNames = append(fileNames, e.Name()) - } - logsDownloadLog.Printf("Artifact directory %s has %d files, not flattening: %v", entry.Name(), len(artifactEntries), fileNames) - } - continue - } - - singleEntry := artifactEntries[0] - if singleEntry.IsDir() { - logsDownloadLog.Printf("Artifact directory %s contains a subdirectory, not flattening", entry.Name()) - continue - } - - // Unfold: Move the single file to parent directory and remove the artifact folder - sourcePath := filepath.Join(artifactDir, singleEntry.Name()) - destPath := filepath.Join(outputDir, singleEntry.Name()) - - logsDownloadLog.Printf("Flattening: %s → %s", sourcePath, destPath) - - // Move the file to root (parent directory) - if err := os.Rename(sourcePath, destPath); err != nil { - logsDownloadLog.Printf("Failed to move file %s to %s: %v", sourcePath, destPath, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to move file %s to %s: %v", sourcePath, destPath, err))) - } - continue - } - - // Delete the now-empty artifact folder - if err := os.Remove(artifactDir); err != nil { - logsDownloadLog.Printf("Failed to remove empty directory %s: %v", artifactDir, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove empty directory %s: %v", artifactDir, err))) - } - continue - } - - logsDownloadLog.Printf("Successfully flattened: %s/%s → %s", entry.Name(), singleEntry.Name(), singleEntry.Name()) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Unfolded single-file artifact: %s → %s", filepath.Join(entry.Name(), singleEntry.Name()), singleEntry.Name()))) - } - } - - return nil -} - -// findArtifactDir looks for an artifact directory by its base name (suffix) in outputDir. -// It handles three cases: -// 1. Exact match: "agent" → outputDir/agent -// 2. Legacy name: for "agent", also checks "agent-artifacts" -// 3. Prefixed name (workflow_call): "*-agent" → outputDir/-agent -// -// Returns the first matching directory path, or empty string if none found. -func findArtifactDir(outputDir, baseName string, legacyName string) string { - // First, try exact match - exactPath := filepath.Join(outputDir, baseName) - if fileutil.DirExists(exactPath) { - return exactPath - } - - // Try legacy name if provided - if legacyName != "" { - legacyPath := filepath.Join(outputDir, legacyName) - if fileutil.DirExists(legacyPath) { - return legacyPath - } - } - - // Scan for prefixed names (workflow_call context): any directory ending with "-{baseName}" - entries, err := os.ReadDir(outputDir) - if err != nil { - return "" - } - suffix := "-" + baseName - for _, entry := range entries { - if entry.IsDir() && strings.HasSuffix(entry.Name(), suffix) { - return filepath.Join(outputDir, entry.Name()) - } - } - - return "" -} - -// flattenArtifactTree moves all files from sourceDir into outputDir, preserving relative paths, -// then removes artifactDir (which may equal sourceDir, or be a parent of it in the old-structure -// case). label is used in log and user-facing messages. -// Cleanup failures are non-fatal: they are logged (and optionally printed) but do not return an error. -func flattenArtifactTree(sourceDir, artifactDir, outputDir, label string, verbose bool) error { - walkErr := filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // Skip the source directory itself - if path == sourceDir { - return nil - } - - // Calculate relative path from source - relPath, err := filepath.Rel(sourceDir, path) - if err != nil { - return fmt.Errorf("failed to get relative path for %s: %w", path, err) - } - - destPath := filepath.Join(outputDir, relPath) - - if info.IsDir() { - // Create directory in destination with world-readable permissions (0755) - if err := os.MkdirAll(destPath, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create directory %s: %w", destPath, err) - } - logsDownloadLog.Printf("Created directory: %s", destPath) - } else { - // Ensure parent directory exists with world-readable permissions (0755) - if err := os.MkdirAll(filepath.Dir(destPath), constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create parent directory for %s: %w", destPath, err) - } - - if err := os.Rename(path, destPath); err != nil { - return fmt.Errorf("failed to move file %s to %s: %w", path, destPath, err) - } - logsDownloadLog.Printf("Moved file: %s → %s", path, destPath) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Flattened: %s → %s", relPath, relPath))) - } - } - - return nil - }) - - if walkErr != nil { - return fmt.Errorf("failed to flatten %s: %w", label, walkErr) - } - - // Remove the now-empty artifact directory structure. - // Don't fail the entire operation if cleanup fails. - if err := os.RemoveAll(artifactDir); err != nil { - logsDownloadLog.Printf("Failed to remove %s directory %s: %v", label, artifactDir, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove %s directory: %v", label, err))) - } - } else { - logsDownloadLog.Printf("Removed %s directory: %s", label, artifactDir) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Flattened %s and removed nested structure", label))) - } - } - - return nil -} - -// flattenUnifiedArtifact flattens the unified agent artifact directory structure. -// The artifact is uploaded with all paths under /tmp/gh-aw/, so the action strips the -// common prefix and files land directly inside the artifact directory (new structure). -// For backward compatibility, it also handles the old structure where the full -// tmp/gh-aw/ path was preserved inside the artifact directory. -// New artifact name: "agent" (preferred) -// Legacy artifact name: "agent-artifacts" (backward compat for older workflow runs) -// In workflow_call context, the artifact may be prefixed: "-agent" -func flattenUnifiedArtifact(outputDir string, verbose bool) error { - agentArtifactsDir := findArtifactDir(outputDir, "agent", "agent-artifacts") - if agentArtifactsDir == "" { - // No unified artifact, nothing to flatten - return nil - } - - logsDownloadLog.Printf("Flattening unified agent artifact directory: %s", agentArtifactsDir) - - // Determine the source path: old structure preserves the tmp/gh-aw/ prefix inside the artifact - sourceDir := agentArtifactsDir - tmpGhAwPath := filepath.Join(agentArtifactsDir, "tmp", "gh-aw") - if fileutil.DirExists(tmpGhAwPath) { - logsDownloadLog.Printf("Found old artifact structure with tmp/gh-aw prefix") - sourceDir = tmpGhAwPath - } else { - logsDownloadLog.Printf("Found new artifact structure without tmp/gh-aw prefix") - } - - return flattenArtifactTree(sourceDir, agentArtifactsDir, outputDir, "unified agent artifact", verbose) -} - -// flattenActivationArtifact flattens the activation artifact directory structure. -// The activation artifact contains aw_info.json and aw-prompts/prompt.txt. -// This function moves those files to the root output directory and removes the nested structure. -// In workflow_call context, the artifact may be prefixed: "-activation" -func flattenActivationArtifact(outputDir string, verbose bool) error { - activationDir := findArtifactDir(outputDir, "activation", "") - if activationDir == "" { - // No activation artifact, nothing to flatten - return nil - } - - logsDownloadLog.Printf("Flattening activation artifact directory: %s", activationDir) - - return flattenArtifactTree(activationDir, activationDir, outputDir, "activation artifact", verbose) -} - -// flattenAgentOutputsArtifact flattens the agent_outputs artifact directory structure. -// The agent_outputs artifact contains session logs with detailed token usage data -// that are critical for accurate token count parsing. -func flattenAgentOutputsArtifact(outputDir string, verbose bool) error { - agentOutputsDir := filepath.Join(outputDir, "agent_outputs") - - // Check if agent_outputs directory exists - if _, err := os.Stat(agentOutputsDir); os.IsNotExist(err) { - // No agent_outputs artifact, nothing to flatten - logsDownloadLog.Print("No agent_outputs artifact found (session logs may be missing)") - return nil - } - - logsDownloadLog.Printf("Flattening agent_outputs directory: %s", agentOutputsDir) - - return flattenArtifactTree(agentOutputsDir, agentOutputsDir, outputDir, "agent_outputs artifact", verbose) -} - -// flattenSafeOutputsItemsArtifact flattens the safe-outputs-items artifact directory -// structure. The safe-outputs-items artifact contains safe-output-items.jsonl and -// temporary-id-map.json. After flattening, these files land at the run directory root -// where extractCreatedItemsFromManifest and loadResolvedTemporaryIDTargets expect them. -// The artifact may be prefixed in workflow_call context: "-safe-outputs-items". -func flattenSafeOutputsItemsArtifact(outputDir string, verbose bool) error { - safeOutputsItemsDir := findArtifactDir(outputDir, constants.SafeOutputItemsArtifactName, "") - if safeOutputsItemsDir == "" { - // No safe-outputs-items artifact, nothing to flatten - return nil - } - - logsDownloadLog.Printf("Flattening safe-outputs-items artifact directory: %s", safeOutputsItemsDir) - - return flattenArtifactTree(safeOutputsItemsDir, safeOutputsItemsDir, outputDir, "safe-outputs-items artifact", verbose) -} - // downloadWorkflowRunLogs downloads and unzips workflow run logs using GitHub API func downloadWorkflowRunLogs(ctx context.Context, runID int64, outputDir string, verbose bool, owner, repo, hostname string) error { logsDownloadLog.Printf("Downloading workflow run logs: run_id=%d, output_dir=%s, owner=%s, repo=%s", runID, outputDir, owner, repo) @@ -401,302 +133,6 @@ func downloadWorkflowRunLogs(ctx context.Context, runID int64, outputDir string, return nil } -// unzipFile extracts a zip file to a destination directory -func unzipFile(zipPath, destDir string, verbose bool) error { - // Open the zip file - r, err := zip.OpenReader(zipPath) - if err != nil { - return fmt.Errorf("failed to open zip file: %w", err) - } - defer r.Close() - - // Extract each file in the zip - for _, f := range r.File { - if err := extractZipFile(f, destDir, verbose); err != nil { - return err - } - } - - return nil -} - -// extractZipFile extracts a single file from a zip archive -func extractZipFile(f *zip.File, destDir string, verbose bool) (extractErr error) { - // #nosec G305 -- Path traversal is prevented by filepath.Clean and prefix check below - // Validate file name doesn't contain path traversal attempts - cleanName := filepath.Clean(f.Name) - if strings.Contains(cleanName, "..") { - return fmt.Errorf("invalid file path in zip (contains ..): %s", f.Name) - } - - // Construct the full path for the file - filePath := filepath.Join(destDir, cleanName) - - // Prevent zip slip vulnerability - ensure extracted path is within destDir - cleanDest := filepath.Clean(destDir) - if !strings.HasPrefix(filepath.Clean(filePath), cleanDest+string(os.PathSeparator)) && filepath.Clean(filePath) != cleanDest { - return fmt.Errorf("invalid file path in zip (outside destination): %s", f.Name) - } - - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Extracting: "+cleanName)) - } - - // Create directory if it's a directory entry - if f.FileInfo().IsDir() { - return os.MkdirAll(filePath, constants.DirPermPublic) - } - - // Decompression bomb protection - limit individual file size to 1GB - // #nosec G110 -- Decompression bomb is mitigated by size check below - const maxFileSize = 1 * 1024 * 1024 * 1024 // 1GB - if f.UncompressedSize64 > maxFileSize { - return fmt.Errorf("file too large in zip (>1GB): %s (%d bytes)", f.Name, f.UncompressedSize64) - } - - // Create parent directory if needed - if err := os.MkdirAll(filepath.Dir(filePath), constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create directory: %w", err) - } - - // Open the file in the zip - srcFile, err := f.Open() - if err != nil { - return fmt.Errorf("failed to open file in zip: %w", err) - } - defer srcFile.Close() - - // Create the destination file - destFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode()) - if err != nil { - return fmt.Errorf("failed to create destination file: %w", err) - } - defer func() { - // Handle errors from closing the writable file to prevent data loss - // Data written to a file may be cached in memory and only flushed when the file is closed. - // If Close() fails and the error is ignored, data loss can occur silently. - if err := destFile.Close(); extractErr == nil && err != nil { - extractErr = fmt.Errorf("failed to close destination file: %w", err) - } - }() - - // Copy the content with size limit enforcement - // Use LimitReader to prevent reading more than declared size - limitedReader := io.LimitReader(srcFile, int64(maxFileSize)) - written, err := io.Copy(destFile, limitedReader) - if err != nil { - extractErr = fmt.Errorf("failed to extract file: %w", err) - return extractErr - } - - // Verify we didn't exceed the size limit - if uint64(written) > maxFileSize { - extractErr = fmt.Errorf("file extraction exceeded size limit: %s", f.Name) - return extractErr - } - - return nil -} - -// listArtifacts creates a list of all artifact files in the output directory -func listArtifacts(outputDir string) ([]string, error) { - var artifacts []string - - walkErr := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // Skip directories and the summary file itself - if info.IsDir() || filepath.Base(path) == runSummaryFileName { - return nil - } - - // Get relative path from outputDir - relPath, err := filepath.Rel(outputDir, path) - if err != nil { - return err - } - - artifacts = append(artifacts, relPath) - return nil - }) - - if walkErr != nil { - return nil, walkErr - } - - return artifacts, nil -} - -// isNonZipArtifactError reports whether the output from gh run download indicates -// that the failure was caused by one or more non-zip artifacts (e.g. .dockerbuild files). -// Such artifacts cannot be extracted as zip archives and should be skipped rather than -// failing the entire download. -func isNonZipArtifactError(output []byte) bool { - s := string(output) - return strings.Contains(s, "zip: not a valid zip file") -} - -// isCaseCollisionArtifactError reports whether gh run download failed because -// a zip extraction attempted to write a file that already exists. This can -// happen on case-insensitive filesystems (e.g. macOS) when an artifact -// contains files whose names differ only by case. -func isCaseCollisionArtifactError(output []byte) bool { - s := string(output) - return strings.Contains(s, "error extracting zip archive") && strings.Contains(s, "file exists") -} - -// isDockerBuildArtifact reports whether an artifact name represents a .dockerbuild artifact. -// These are not zip archives and cannot be extracted by gh run download. -func isDockerBuildArtifact(name string) bool { - return strings.HasSuffix(name, ".dockerbuild") -} - -// listRunArtifactNames returns the names of all artifacts for the given workflow run -// by querying the GitHub Actions API. Returns an error if the API call fails. -func listRunArtifactNames(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) ([]string, error) { - var endpoint string - if owner != "" && repo != "" { - endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d/artifacts", owner, repo, runID) - } else { - endpoint = fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d/artifacts", runID) - } - - args := []string{"api", "--paginate", endpoint, "--jq", ".artifacts[].name"} - if hostname != "" && hostname != "github.com" { - args = append(args, "--hostname", hostname) - } - - logsDownloadLog.Printf("Listing artifacts for run %d: gh %s", runID, strings.Join(args, " ")) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Listing artifacts: gh "+strings.Join(args, " "))) - } - - cmd := workflow.ExecGHContext(ctx, args...) - output, err := cmd.Output() - if err != nil { - return nil, fmt.Errorf("failed to list artifacts for run %d: %w", runID, err) - } - - var names []string - for line := range strings.SplitSeq(strings.TrimSpace(string(output)), "\n") { - name := strings.TrimSpace(line) - if name != "" { - names = append(names, name) - } - } - return names, nil -} - -// downloadArtifactsByName downloads a list of artifacts individually by name. -// This is used when some artifacts (e.g. .dockerbuild) need to be skipped and -// only a subset of the run's artifacts should be downloaded. -func downloadArtifactsByName(ctx context.Context, opts downloadArtifactsOptions, names []string) error { - var repoFlag string - shouldLogProgress := IsRunningInCI() || opts.verbose - if opts.owner != "" && opts.repo != "" { - if opts.hostname != "" && opts.hostname != "github.com" { - repoFlag = opts.hostname + "/" + opts.owner + "/" + opts.repo - } else { - repoFlag = opts.owner + "/" + opts.repo - } - } - - for _, name := range names { - args := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", opts.outputDir} - if repoFlag != "" { - args = append(args, "-R", repoFlag) - } - - logsDownloadLog.Printf("Downloading artifact %q individually: gh %s", name, strings.Join(args, " ")) - if shouldLogProgress { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Downloading artifact: "+name)) - } - - cmd := workflow.ExecGHContext(ctx, args...) - cmdOutput, cmdErr := cmd.CombinedOutput() - if cmdErr != nil { - logsDownloadLog.Printf("Failed to download artifact %q: %v (%s)", name, cmdErr, string(cmdOutput)) - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download artifact %q: %v", name, cmdErr))) - } - // Non-fatal: continue downloading other artifacts - } else { - logsDownloadLog.Printf("Downloaded artifact %q", name) - if err := markArtifactDownloaded(opts.outputDir, name); err != nil { - return err - } - } - } - - return nil -} - -// criticalArtifactNames lists the artifact names that are essential for audit analysis. -// When a bulk download fails partially (e.g., due to non-zip artifacts), these artifacts -// are retried individually so that flattening and audit extraction have data to work with. -var criticalArtifactNames = []string{"activation", "agent"} - -// retryCriticalArtifacts downloads critical artifacts individually when the bulk download -// was only partially successful. gh run download aborts on the first non-zip artifact, -// which may prevent valid artifacts from being downloaded. -// artifactFilter limits which critical artifacts are retried; nil means retry all. -func retryCriticalArtifacts(ctx context.Context, opts downloadArtifactsOptions) { - // Build the repo flag once for reuse across retries - var repoFlag string - if opts.owner != "" && opts.repo != "" { - if opts.hostname != "" && opts.hostname != "github.com" { - repoFlag = opts.hostname + "/" + opts.owner + "/" + opts.repo - } else { - repoFlag = opts.owner + "/" + opts.repo - } - } - - for _, name := range criticalArtifactNames { - // Skip artifacts not included in the active filter. - if !artifactMatchesFilter(name, opts.artifactFilter) { - logsDownloadLog.Printf("Skipping critical artifact %q (not in artifact filter)", name) - continue - } - artifactDir := filepath.Join(opts.outputDir, name) - if fileutil.DirExists(artifactDir) { - logsDownloadLog.Printf("Critical artifact %q already present, skipping retry", name) - continue - } - - retryArgs := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", opts.outputDir} - if repoFlag != "" { - retryArgs = append(retryArgs, "-R", repoFlag) - } - - logsDownloadLog.Printf("Retrying individual download for artifact %q: gh %s", name, strings.Join(retryArgs, " ")) - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Retrying download for missing artifact: "+name)) - } - - retryCmd := workflow.ExecGHContext(ctx, retryArgs...) - retryOutput, retryErr := retryCmd.CombinedOutput() - if retryErr != nil { - logsDownloadLog.Printf("Failed to download artifact %q individually: %v (%s)", name, retryErr, string(retryOutput)) - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not download artifact %q: %v", name, retryErr))) - } - } else { - logsDownloadLog.Printf("Successfully downloaded artifact %q individually", name) - // Marker write failures are non-fatal in the retry path: retryCriticalArtifacts - // is a best-effort recovery after a partial bulk download, so a missing marker - // only causes a redundant re-download on the next run (not data loss). - if err := markArtifactDownloaded(opts.outputDir, name); err != nil { - logsDownloadLog.Printf("Failed to mark artifact %q as downloaded: %v", name, err) - } - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Downloaded missing artifact: "+name)) - } - } - } -} - // downloadRunArtifacts downloads artifacts for a specific workflow run. // artifactFilter is a list of artifact base names to download; nil means download all. func downloadRunArtifacts(ctx context.Context, opts downloadArtifactsOptions) error { @@ -1001,34 +437,8 @@ func downloadRunArtifacts(ctx context.Context, opts downloadArtifactsOptions) er spinner.StopWithMessage(fmt.Sprintf("✓ Downloaded artifacts for run %d", opts.runID)) } - // Flatten single-file artifacts - if err := flattenSingleFileArtifacts(opts.outputDir, opts.verbose); err != nil { - return fmt.Errorf("failed to flatten artifacts: %w", err) - } - - // Flatten activation artifact directory structure (contains aw_info.json and prompt.txt) - if err := flattenActivationArtifact(opts.outputDir, opts.verbose); err != nil { - return fmt.Errorf("failed to flatten activation artifact: %w", err) - } - - ensureUsageAwInfoFallback(ctx, opts) - - // Flatten unified agent directory structure - if err := flattenUnifiedArtifact(opts.outputDir, opts.verbose); err != nil { - return fmt.Errorf("failed to flatten unified artifact: %w", err) - } - - // Flatten agent_outputs artifact if present - if err := flattenAgentOutputsArtifact(opts.outputDir, opts.verbose); err != nil { - return fmt.Errorf("failed to flatten agent_outputs artifact: %w", err) - } - - // Flatten safe-outputs-items artifact if present. - // This artifact contains safe-output-items.jsonl and temporary-id-map.json. - // Flattening moves them to the run root so extractCreatedItemsFromManifest - // and loadResolvedTemporaryIDTargets can find them at their expected paths. - if err := flattenSafeOutputsItemsArtifact(opts.outputDir, opts.verbose); err != nil { - return fmt.Errorf("failed to flatten safe-outputs-items artifact: %w", err) + if err := flattenDownloadedArtifacts(ctx, opts); err != nil { + return err } // Download and unzip workflow run logs unless caller requested usage-only mode. @@ -1043,46 +453,7 @@ func downloadRunArtifacts(ctx context.Context, opts downloadArtifactsOptions) er } if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Downloaded artifacts for run %d to %s", opts.runID, opts.outputDir))) - // Enumerate created files (shallow + summary) for immediate visibility - var fileCount int - var firstFiles []string - var walkFailed bool - if walkErr := filepath.Walk(opts.outputDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - logsDownloadLog.Printf("walk error at %s: %v", path, err) - walkFailed = true - return nil - } - if info.IsDir() { - return nil - } - fileCount++ - if len(firstFiles) < 12 { // capture a reasonable preview - rel, relErr := filepath.Rel(opts.outputDir, path) - if relErr == nil { - firstFiles = append(firstFiles, rel) - } - } - return nil - }); walkErr != nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("filesystem error enumerating artifacts in %s: %v", opts.outputDir, walkErr))) - } - if fileCount == 0 { - if walkFailed { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Download completed but artifact files could not be enumerated (filesystem error)")) - } else { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Download completed but no artifact files were created (empty run)")) - } - } else { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Artifact file count: %d", fileCount))) - for _, f := range firstFiles { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(" • "+f)) - } - if fileCount > len(firstFiles) { - fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf(" … %d more files omitted", fileCount-len(firstFiles)))) - } - } + logVerboseDownloadSummary(opts) } return nil diff --git a/pkg/cli/logs_download_artifacts.go b/pkg/cli/logs_download_artifacts.go new file mode 100644 index 00000000000..ff1aa6e5eca --- /dev/null +++ b/pkg/cli/logs_download_artifacts.go @@ -0,0 +1,262 @@ +// This file provides command-line interface functionality for gh-aw. +// This file (logs_download_artifacts.go) contains functions for discovering, +// filtering, and downloading individual workflow run artifacts by name via +// the GitHub CLI. + +package cli + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/fileutil" + "github.com/github/gh-aw/pkg/workflow" +) + +// buildRepoFlag returns the "-R" flag value for gh commands given the owner, +// repo, and optional hostname. Returns an empty string when owner or repo is +// unset (the gh CLI will infer the repository from git context in that case). +func buildRepoFlag(owner, repo, hostname string) string { + if owner == "" || repo == "" { + return "" + } + if hostname != "" && hostname != "github.com" { + return hostname + "/" + owner + "/" + repo + } + return owner + "/" + repo +} + +// listArtifacts creates a list of all artifact files in the output directory +func listArtifacts(outputDir string) ([]string, error) { + var artifacts []string + + walkErr := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Skip directories and the summary file itself + if info.IsDir() || filepath.Base(path) == runSummaryFileName { + return nil + } + + // Get relative path from outputDir + relPath, err := filepath.Rel(outputDir, path) + if err != nil { + return err + } + + artifacts = append(artifacts, relPath) + return nil + }) + + if walkErr != nil { + return nil, walkErr + } + + return artifacts, nil +} + +// isNonZipArtifactError reports whether the output from gh run download indicates +// that the failure was caused by one or more non-zip artifacts (e.g. .dockerbuild files). +// Such artifacts cannot be extracted as zip archives and should be skipped rather than +// failing the entire download. +func isNonZipArtifactError(output []byte) bool { + s := string(output) + return strings.Contains(s, "zip: not a valid zip file") +} + +// isCaseCollisionArtifactError reports whether gh run download failed because +// a zip extraction attempted to write a file that already exists. This can +// happen on case-insensitive filesystems (e.g. macOS) when an artifact +// contains files whose names differ only by case. +func isCaseCollisionArtifactError(output []byte) bool { + s := string(output) + return strings.Contains(s, "error extracting zip archive") && strings.Contains(s, "file exists") +} + +// isDockerBuildArtifact reports whether an artifact name represents a .dockerbuild artifact. +// These are not zip archives and cannot be extracted by gh run download. +func isDockerBuildArtifact(name string) bool { + return strings.HasSuffix(name, ".dockerbuild") +} + +// listRunArtifactNames returns the names of all artifacts for the given workflow run +// by querying the GitHub Actions API. Returns an error if the API call fails. +func listRunArtifactNames(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) ([]string, error) { + var endpoint string + if owner != "" && repo != "" { + endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d/artifacts", owner, repo, runID) + } else { + endpoint = fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d/artifacts", runID) + } + + args := []string{"api", "--paginate", endpoint, "--jq", ".artifacts[].name"} + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) + } + + logsDownloadLog.Printf("Listing artifacts for run %d: gh %s", runID, strings.Join(args, " ")) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Listing artifacts: gh "+strings.Join(args, " "))) + } + + cmd := workflow.ExecGHContext(ctx, args...) + output, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to list artifacts for run %d: %w", runID, err) + } + + var names []string + for line := range strings.SplitSeq(strings.TrimSpace(string(output)), "\n") { + name := strings.TrimSpace(line) + if name != "" { + names = append(names, name) + } + } + return names, nil +} + +// downloadArtifactsByName downloads a list of artifacts individually by name. +// This is used when some artifacts (e.g. .dockerbuild) need to be skipped and +// only a subset of the run's artifacts should be downloaded. +func downloadArtifactsByName(ctx context.Context, opts downloadArtifactsOptions, names []string) error { + repoFlag := buildRepoFlag(opts.owner, opts.repo, opts.hostname) + shouldLogProgress := IsRunningInCI() || opts.verbose + + for _, name := range names { + args := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", opts.outputDir} + if repoFlag != "" { + args = append(args, "-R", repoFlag) + } + + logsDownloadLog.Printf("Downloading artifact %q individually: gh %s", name, strings.Join(args, " ")) + if shouldLogProgress { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Downloading artifact: "+name)) + } + + cmd := workflow.ExecGHContext(ctx, args...) + cmdOutput, cmdErr := cmd.CombinedOutput() + if cmdErr != nil { + logsDownloadLog.Printf("Failed to download artifact %q: %v (%s)", name, cmdErr, string(cmdOutput)) + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download artifact %q: %v", name, cmdErr))) + } + // Non-fatal: continue downloading other artifacts + } else { + logsDownloadLog.Printf("Downloaded artifact %q", name) + if err := markArtifactDownloaded(opts.outputDir, name); err != nil { + return err + } + } + } + + return nil +} + +// criticalArtifactNames lists the artifact names that are essential for audit analysis. +// When a bulk download fails partially (e.g., due to non-zip artifacts), these artifacts +// are retried individually so that flattening and audit extraction have data to work with. +var criticalArtifactNames = []string{"activation", "agent"} + +// retryCriticalArtifacts downloads critical artifacts individually when the bulk download +// was only partially successful. gh run download aborts on the first non-zip artifact, +// which may prevent valid artifacts from being downloaded. +// artifactFilter limits which critical artifacts are retried; nil means retry all. +func retryCriticalArtifacts(ctx context.Context, opts downloadArtifactsOptions) { + // Build the repo flag once for reuse across retries + repoFlag := buildRepoFlag(opts.owner, opts.repo, opts.hostname) + + for _, name := range criticalArtifactNames { + // Skip artifacts not included in the active filter. + if !artifactMatchesFilter(name, opts.artifactFilter) { + logsDownloadLog.Printf("Skipping critical artifact %q (not in artifact filter)", name) + continue + } + artifactDir := filepath.Join(opts.outputDir, name) + if fileutil.DirExists(artifactDir) { + logsDownloadLog.Printf("Critical artifact %q already present, skipping retry", name) + continue + } + + retryArgs := []string{"run", "download", strconv.FormatInt(opts.runID, 10), "--name", name, "--dir", opts.outputDir} + if repoFlag != "" { + retryArgs = append(retryArgs, "-R", repoFlag) + } + + logsDownloadLog.Printf("Retrying individual download for artifact %q: gh %s", name, strings.Join(retryArgs, " ")) + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Retrying download for missing artifact: "+name)) + } + + retryCmd := workflow.ExecGHContext(ctx, retryArgs...) + retryOutput, retryErr := retryCmd.CombinedOutput() + if retryErr != nil { + logsDownloadLog.Printf("Failed to download artifact %q individually: %v (%s)", name, retryErr, string(retryOutput)) + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not download artifact %q: %v", name, retryErr))) + } + } else { + logsDownloadLog.Printf("Successfully downloaded artifact %q individually", name) + // Marker write failures are non-fatal in the retry path: retryCriticalArtifacts + // is a best-effort recovery after a partial bulk download, so a missing marker + // only causes a redundant re-download on the next run (not data loss). + if err := markArtifactDownloaded(opts.outputDir, name); err != nil { + logsDownloadLog.Printf("Failed to mark artifact %q as downloaded: %v", name, err) + } + if opts.verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Downloaded missing artifact: "+name)) + } + } + } +} + +// logVerboseDownloadSummary prints a success message and a shallow enumeration of the +// files created under opts.outputDir. It is only invoked when verbose mode is enabled. +func logVerboseDownloadSummary(opts downloadArtifactsOptions) { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Downloaded artifacts for run %d to %s", opts.runID, opts.outputDir))) + // Enumerate created files (shallow + summary) for immediate visibility + var fileCount int + var firstFiles []string + var walkFailed bool + if walkErr := filepath.Walk(opts.outputDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + logsDownloadLog.Printf("walk error at %s: %v", path, err) + walkFailed = true + return nil + } + if info.IsDir() { + return nil + } + fileCount++ + if len(firstFiles) < 12 { // capture a reasonable preview + rel, relErr := filepath.Rel(opts.outputDir, path) + if relErr == nil { + firstFiles = append(firstFiles, rel) + } + } + return nil + }); walkErr != nil { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("filesystem error enumerating artifacts in %s: %v", opts.outputDir, walkErr))) + } + if fileCount == 0 { + if walkFailed { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Download completed but artifact files could not be enumerated (filesystem error)")) + } else { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Download completed but no artifact files were created (empty run)")) + } + } else { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Artifact file count: %d", fileCount))) + for _, f := range firstFiles { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(" • "+f)) + } + if fileCount > len(firstFiles) { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf(" … %d more files omitted", fileCount-len(firstFiles)))) + } + } +} diff --git a/pkg/cli/logs_download_flatten.go b/pkg/cli/logs_download_flatten.go new file mode 100644 index 00000000000..f84d0cf7206 --- /dev/null +++ b/pkg/cli/logs_download_flatten.go @@ -0,0 +1,321 @@ +// This file provides command-line interface functionality for gh-aw. +// This file (logs_download_flatten.go) contains functions for flattening +// downloaded artifact directories into the run's output directory, undoing +// the per-artifact directory nesting created by `gh run download`. + +package cli + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/fileutil" +) + +// flattenSingleFileArtifacts checks artifact directories and flattens any that contain a single file +// This handles the case where gh CLI creates a directory for each artifact, even if it's just one file +func flattenSingleFileArtifacts(outputDir string, verbose bool) error { + logsDownloadLog.Printf("Flattening single-file artifacts in: %s", outputDir) + entries, err := os.ReadDir(outputDir) + if err != nil { + return fmt.Errorf("failed to read output directory: %w", err) + } + + for _, entry := range entries { + if !entry.IsDir() || entry.Name() == downloadedArtifactsMarkerDir { + continue + } + + artifactDir := filepath.Join(outputDir, entry.Name()) + + // Read contents of artifact directory + artifactEntries, err := os.ReadDir(artifactDir) + if err != nil { + logsDownloadLog.Printf("Failed to read artifact directory %s: %v", artifactDir, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to read artifact directory %s: %v", artifactDir, err))) + } + continue + } + + logsDownloadLog.Printf("Artifact directory %s contains %d entries", entry.Name(), len(artifactEntries)) + + // Apply unfold rule: Check if directory contains exactly one entry and it's a file + if len(artifactEntries) != 1 { + if verbose && len(artifactEntries) > 1 { + // Log what's in multi-file artifacts for debugging + var fileNames []string + for _, e := range artifactEntries { + fileNames = append(fileNames, e.Name()) + } + logsDownloadLog.Printf("Artifact directory %s has %d files, not flattening: %v", entry.Name(), len(artifactEntries), fileNames) + } + continue + } + + singleEntry := artifactEntries[0] + if singleEntry.IsDir() { + logsDownloadLog.Printf("Artifact directory %s contains a subdirectory, not flattening", entry.Name()) + continue + } + + // Unfold: Move the single file to parent directory and remove the artifact folder + sourcePath := filepath.Join(artifactDir, singleEntry.Name()) + destPath := filepath.Join(outputDir, singleEntry.Name()) + + logsDownloadLog.Printf("Flattening: %s → %s", sourcePath, destPath) + + // Move the file to root (parent directory) + if err := os.Rename(sourcePath, destPath); err != nil { + logsDownloadLog.Printf("Failed to move file %s to %s: %v", sourcePath, destPath, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to move file %s to %s: %v", sourcePath, destPath, err))) + } + continue + } + + // Delete the now-empty artifact folder + if err := os.Remove(artifactDir); err != nil { + logsDownloadLog.Printf("Failed to remove empty directory %s: %v", artifactDir, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove empty directory %s: %v", artifactDir, err))) + } + continue + } + + logsDownloadLog.Printf("Successfully flattened: %s/%s → %s", entry.Name(), singleEntry.Name(), singleEntry.Name()) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Unfolded single-file artifact: %s → %s", filepath.Join(entry.Name(), singleEntry.Name()), singleEntry.Name()))) + } + } + + return nil +} + +// findArtifactDir looks for an artifact directory by its base name (suffix) in outputDir. +// It handles three cases: +// 1. Exact match: "agent" → outputDir/agent +// 2. Legacy name: for "agent", also checks "agent-artifacts" +// 3. Prefixed name (workflow_call): "*-agent" → outputDir/-agent +// +// Returns the first matching directory path, or empty string if none found. +func findArtifactDir(outputDir, baseName string, legacyName string) string { + // First, try exact match + exactPath := filepath.Join(outputDir, baseName) + if fileutil.DirExists(exactPath) { + return exactPath + } + + // Try legacy name if provided + if legacyName != "" { + legacyPath := filepath.Join(outputDir, legacyName) + if fileutil.DirExists(legacyPath) { + return legacyPath + } + } + + // Scan for prefixed names (workflow_call context): any directory ending with "-{baseName}" + entries, err := os.ReadDir(outputDir) + if err != nil { + return "" + } + suffix := "-" + baseName + for _, entry := range entries { + if entry.IsDir() && strings.HasSuffix(entry.Name(), suffix) { + return filepath.Join(outputDir, entry.Name()) + } + } + + return "" +} + +// flattenArtifactTree moves all files from sourceDir into outputDir, preserving relative paths, +// then removes artifactDir (which may equal sourceDir, or be a parent of it in the old-structure +// case). label is used in log and user-facing messages. +// Cleanup failures are non-fatal: they are logged (and optionally printed) but do not return an error. +func flattenArtifactTree(sourceDir, artifactDir, outputDir, label string, verbose bool) error { + walkErr := filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Skip the source directory itself + if path == sourceDir { + return nil + } + + // Calculate relative path from source + relPath, err := filepath.Rel(sourceDir, path) + if err != nil { + return fmt.Errorf("failed to get relative path for %s: %w", path, err) + } + + destPath := filepath.Join(outputDir, relPath) + + if info.IsDir() { + // Create directory in destination with world-readable permissions (0755) + if err := os.MkdirAll(destPath, constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create directory %s: %w", destPath, err) + } + logsDownloadLog.Printf("Created directory: %s", destPath) + } else { + // Ensure parent directory exists with world-readable permissions (0755) + if err := os.MkdirAll(filepath.Dir(destPath), constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create parent directory for %s: %w", destPath, err) + } + + if err := os.Rename(path, destPath); err != nil { + return fmt.Errorf("failed to move file %s to %s: %w", path, destPath, err) + } + logsDownloadLog.Printf("Moved file: %s → %s", path, destPath) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Flattened: %s → %s", relPath, relPath))) + } + } + + return nil + }) + + if walkErr != nil { + return fmt.Errorf("failed to flatten %s: %w", label, walkErr) + } + + // Remove the now-empty artifact directory structure. + // Don't fail the entire operation if cleanup fails. + if err := os.RemoveAll(artifactDir); err != nil { + logsDownloadLog.Printf("Failed to remove %s directory %s: %v", label, artifactDir, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove %s directory: %v", label, err))) + } + } else { + logsDownloadLog.Printf("Removed %s directory: %s", label, artifactDir) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Flattened %s and removed nested structure", label))) + } + } + + return nil +} + +// flattenUnifiedArtifact flattens the unified agent artifact directory structure. +// The artifact is uploaded with all paths under /tmp/gh-aw/, so the action strips the +// common prefix and files land directly inside the artifact directory (new structure). +// For backward compatibility, it also handles the old structure where the full +// tmp/gh-aw/ path was preserved inside the artifact directory. +// New artifact name: "agent" (preferred) +// Legacy artifact name: "agent-artifacts" (backward compat for older workflow runs) +// In workflow_call context, the artifact may be prefixed: "-agent" +func flattenUnifiedArtifact(outputDir string, verbose bool) error { + agentArtifactsDir := findArtifactDir(outputDir, "agent", "agent-artifacts") + if agentArtifactsDir == "" { + // No unified artifact, nothing to flatten + return nil + } + + logsDownloadLog.Printf("Flattening unified agent artifact directory: %s", agentArtifactsDir) + + // Determine the source path: old structure preserves the tmp/gh-aw/ prefix inside the artifact + sourceDir := agentArtifactsDir + tmpGhAwPath := filepath.Join(agentArtifactsDir, "tmp", "gh-aw") + if fileutil.DirExists(tmpGhAwPath) { + logsDownloadLog.Printf("Found old artifact structure with tmp/gh-aw prefix") + sourceDir = tmpGhAwPath + } else { + logsDownloadLog.Printf("Found new artifact structure without tmp/gh-aw prefix") + } + + return flattenArtifactTree(sourceDir, agentArtifactsDir, outputDir, "unified agent artifact", verbose) +} + +// flattenActivationArtifact flattens the activation artifact directory structure. +// The activation artifact contains aw_info.json and aw-prompts/prompt.txt. +// This function moves those files to the root output directory and removes the nested structure. +// In workflow_call context, the artifact may be prefixed: "-activation" +func flattenActivationArtifact(outputDir string, verbose bool) error { + activationDir := findArtifactDir(outputDir, "activation", "") + if activationDir == "" { + // No activation artifact, nothing to flatten + return nil + } + + logsDownloadLog.Printf("Flattening activation artifact directory: %s", activationDir) + + return flattenArtifactTree(activationDir, activationDir, outputDir, "activation artifact", verbose) +} + +// flattenAgentOutputsArtifact flattens the agent_outputs artifact directory structure. +// The agent_outputs artifact contains session logs with detailed token usage data +// that are critical for accurate token count parsing. +func flattenAgentOutputsArtifact(outputDir string, verbose bool) error { + agentOutputsDir := filepath.Join(outputDir, "agent_outputs") + + // Check if agent_outputs directory exists + if _, err := os.Stat(agentOutputsDir); os.IsNotExist(err) { + // No agent_outputs artifact, nothing to flatten + logsDownloadLog.Print("No agent_outputs artifact found (session logs may be missing)") + return nil + } + + logsDownloadLog.Printf("Flattening agent_outputs directory: %s", agentOutputsDir) + + return flattenArtifactTree(agentOutputsDir, agentOutputsDir, outputDir, "agent_outputs artifact", verbose) +} + +// flattenSafeOutputsItemsArtifact flattens the safe-outputs-items artifact directory +// structure. The safe-outputs-items artifact contains safe-output-items.jsonl and +// temporary-id-map.json. After flattening, these files land at the run directory root +// where extractCreatedItemsFromManifest and loadResolvedTemporaryIDTargets expect them. +// The artifact may be prefixed in workflow_call context: "-safe-outputs-items". +func flattenSafeOutputsItemsArtifact(outputDir string, verbose bool) error { + safeOutputsItemsDir := findArtifactDir(outputDir, constants.SafeOutputItemsArtifactName, "") + if safeOutputsItemsDir == "" { + // No safe-outputs-items artifact, nothing to flatten + return nil + } + + logsDownloadLog.Printf("Flattening safe-outputs-items artifact directory: %s", safeOutputsItemsDir) + + return flattenArtifactTree(safeOutputsItemsDir, safeOutputsItemsDir, outputDir, "safe-outputs-items artifact", verbose) +} + +// flattenDownloadedArtifacts normalizes the directory structure of all known artifact +// types after a successful download, moving files up out of per-artifact subdirectories +// so downstream audit/parsing code finds them at their expected paths. +func flattenDownloadedArtifacts(ctx context.Context, opts downloadArtifactsOptions) error { + // Flatten single-file artifacts + if err := flattenSingleFileArtifacts(opts.outputDir, opts.verbose); err != nil { + return fmt.Errorf("failed to flatten artifacts: %w", err) + } + + // Flatten activation artifact directory structure (contains aw_info.json and prompt.txt) + if err := flattenActivationArtifact(opts.outputDir, opts.verbose); err != nil { + return fmt.Errorf("failed to flatten activation artifact: %w", err) + } + + ensureUsageAwInfoFallback(ctx, opts) + + // Flatten unified agent directory structure + if err := flattenUnifiedArtifact(opts.outputDir, opts.verbose); err != nil { + return fmt.Errorf("failed to flatten unified artifact: %w", err) + } + + // Flatten agent_outputs artifact if present + if err := flattenAgentOutputsArtifact(opts.outputDir, opts.verbose); err != nil { + return fmt.Errorf("failed to flatten agent_outputs artifact: %w", err) + } + + // Flatten safe-outputs-items artifact if present. + // This artifact contains safe-output-items.jsonl and temporary-id-map.json. + // Flattening moves them to the run root so extractCreatedItemsFromManifest + // and loadResolvedTemporaryIDTargets can find them at their expected paths. + if err := flattenSafeOutputsItemsArtifact(opts.outputDir, opts.verbose); err != nil { + return fmt.Errorf("failed to flatten safe-outputs-items artifact: %w", err) + } + + return nil +} diff --git a/pkg/cli/logs_download_zip.go b/pkg/cli/logs_download_zip.go new file mode 100644 index 00000000000..f9fb3750bbd --- /dev/null +++ b/pkg/cli/logs_download_zip.go @@ -0,0 +1,118 @@ +// This file provides command-line interface functionality for gh-aw. +// This file (logs_download_zip.go) contains functions for extracting zip +// archives downloaded from GitHub Actions, including protections against +// path traversal (zip slip) and decompression bombs. + +package cli + +import ( + "archive/zip" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" +) + +// unzipFile extracts a zip file to a destination directory +func unzipFile(zipPath, destDir string, verbose bool) error { + // Open the zip file + r, err := zip.OpenReader(zipPath) + if err != nil { + return fmt.Errorf("failed to open zip file: %w", err) + } + defer r.Close() + + // Extract each file in the zip + for _, f := range r.File { + if err := extractZipFile(f, destDir, verbose); err != nil { + return err + } + } + + return nil +} + +// extractZipFile extracts a single file from a zip archive +func extractZipFile(f *zip.File, destDir string, verbose bool) (extractErr error) { + // #nosec G305 -- Path traversal is prevented by filepath.Clean and prefix check below + // Validate file name doesn't contain path traversal attempts + cleanName := filepath.Clean(f.Name) + if strings.Contains(cleanName, "..") { + return fmt.Errorf("invalid file path in zip (contains ..): %s", f.Name) + } + + // Construct the full path for the file + filePath := filepath.Join(destDir, cleanName) + + // Prevent zip slip vulnerability - ensure extracted path is within destDir + cleanDest := filepath.Clean(destDir) + if !strings.HasPrefix(filepath.Clean(filePath), cleanDest+string(os.PathSeparator)) && filepath.Clean(filePath) != cleanDest { + return fmt.Errorf("invalid file path in zip (outside destination): %s", f.Name) + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Extracting: "+cleanName)) + } + + // Create directory if it's a directory entry + if f.FileInfo().IsDir() { + return os.MkdirAll(filePath, constants.DirPermPublic) + } + + // Decompression bomb protection - limit individual file size to 1GB + // #nosec G110 -- Decompression bomb is mitigated by size check below + const maxFileSize = 1 * 1024 * 1024 * 1024 // 1GB + if f.UncompressedSize64 > maxFileSize { + return fmt.Errorf("file too large in zip (>1GB): %s (%d bytes)", f.Name, f.UncompressedSize64) + } + + // Create parent directory if needed + if err := os.MkdirAll(filepath.Dir(filePath), constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + // Open the file in the zip + srcFile, err := f.Open() + if err != nil { + return fmt.Errorf("failed to open file in zip: %w", err) + } + defer srcFile.Close() + + // Create the destination file + destFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode()) + if err != nil { + return fmt.Errorf("failed to create destination file: %w", err) + } + defer func() { + // Handle errors from closing the writable file to prevent data loss + // Data written to a file may be cached in memory and only flushed when the file is closed. + // If Close() fails and the error is ignored, data loss can occur silently. + if err := destFile.Close(); extractErr == nil && err != nil { + extractErr = fmt.Errorf("failed to close destination file: %w", err) + } + }() + + // Copy the content with size limit enforcement. + // Limit to maxFileSize+1 bytes: if exactly maxFileSize+1 bytes can be read + // the archive is over the limit and must be rejected. + limitedReader := io.LimitReader(srcFile, int64(maxFileSize)+1) + written, err := io.Copy(destFile, limitedReader) + if err != nil { + extractErr = fmt.Errorf("failed to extract file: %w", err) + return extractErr + } + + // Verify we didn't exceed the size limit. + // written == maxFileSize+1 means the reader was not exhausted, i.e. the + // actual content is larger than maxFileSize. + if written > int64(maxFileSize) { + extractErr = fmt.Errorf("file extraction exceeded size limit: %s", f.Name) + return extractErr + } + + return nil +}