Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions pkg/cli/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,14 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st
// file reads (created items, aw_info, etc.) resolve correctly even if the run
// directory has been moved or copied since the summary was first written.
processedRun.Run.LogsPath = runOutputDir
return renderAuditReport(processedRun, summary.Metrics, summary.MCPToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput)
return renderAuditReport(ctx, processedRun, summary.Metrics, summary.MCPToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput)
}

// Check if we have locally cached artifacts first
hasLocalCache := fileutil.DirExists(runOutputDir) && !fileutil.IsDirEmpty(runOutputDir)

// Try to get run metadata from GitHub API
run, metadataErr := fetchWorkflowRunMetadata(runID, owner, repo, hostname, verbose)
run, metadataErr := fetchWorkflowRunMetadata(ctx, runID, owner, repo, hostname, verbose)
var useLocalCache bool

if metadataErr != nil {
Expand Down Expand Up @@ -259,7 +259,7 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st

// Download artifacts for the run
auditLog.Printf("Downloading artifacts for run %d", runID)
err := downloadRunArtifacts(runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter)
err := downloadRunArtifacts(ctx, runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter)
if err != nil {
// Gracefully handle cases where the run legitimately has no artifacts
if errors.Is(err, ErrNoArtifacts) {
Expand Down Expand Up @@ -483,20 +483,20 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st
}
}

return renderAuditReport(processedRun, metrics, mcpToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput)
return renderAuditReport(ctx, processedRun, metrics, mcpToolUsage, runOutputDir, owner, repo, hostname, verbose, parse, jsonOutput)
}

// renderAuditReport builds and renders the audit report from a fully-populated processedRun.
// It is called both when serving from a cached run summary and after a fresh processing pass,
// ensuring that the two paths produce identical output.
func renderAuditReport(processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage *MCPToolUsageData, runOutputDir string, owner, repo, hostname string, verbose bool, parse bool, jsonOutput bool) error {
func renderAuditReport(ctx context.Context, processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage *MCPToolUsageData, runOutputDir string, owner, repo, hostname string, verbose bool, parse bool, jsonOutput bool) error {
runID := processedRun.Run.DatabaseID

currentCreatedItems := extractCreatedItemsFromManifest(runOutputDir)
processedRun.Run.SafeItemsCount = len(currentCreatedItems)

currentSnapshot := buildAuditComparisonSnapshot(processedRun, currentCreatedItems)
comparison := buildAuditComparisonForRun(processedRun, currentSnapshot, runOutputDir, owner, repo, hostname, verbose)
comparison := buildAuditComparisonForRun(ctx, processedRun, currentSnapshot, runOutputDir, owner, repo, hostname, verbose)

// Build structured audit data
auditData := buildAuditData(processedRun, metrics, mcpToolUsage)
Expand Down Expand Up @@ -752,7 +752,7 @@ func findFirstFailingStep(jobLog string) (int, string) {
}

// fetchWorkflowRunMetadata fetches metadata for a single workflow run
func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) {
func fetchWorkflowRunMetadata(ctx context.Context, runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) {
// Build the API endpoint
var endpoint string
if owner != "" && repo != "" {
Expand Down Expand Up @@ -780,7 +780,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Executing: gh "+strings.Join(args, " ")))
}

output, err := workflow.RunGHCombined("Fetching run metadata...", args...)
output, err := workflow.RunGHCombinedContext(ctx, "Fetching run metadata...", args...)
if err != nil {
if verbose {
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(string(output)))
Expand All @@ -807,7 +807,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose
// that were cancelled or failed before any jobs started), resolve the actual workflow
// display name so that audit output is consistent with 'gh aw logs'.
if strings.HasPrefix(run.WorkflowName, ".github/") {
if displayName := resolveWorkflowDisplayName(run.WorkflowPath, owner, repo, hostname); displayName != "" {
if displayName := resolveWorkflowDisplayName(ctx, run.WorkflowPath, owner, repo, hostname); displayName != "" {
auditLog.Printf("Resolved workflow display name: %q -> %q", run.WorkflowName, displayName)
run.WorkflowName = displayName
}
Expand All @@ -821,7 +821,7 @@ func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose
// relative to the git repository root so that it works from any working directory inside
// the repo); if that fails it falls back to a GitHub API call. An empty string is
// returned on any error so that callers can gracefully keep the original value.
func resolveWorkflowDisplayName(workflowPath, owner, repo, hostname string) string {
func resolveWorkflowDisplayName(ctx context.Context, workflowPath, owner, repo, hostname string) string {
// Try local file first. workflowPath is a repo-relative path like
// ".github/workflows/foo.lock.yml", so we resolve it against the git root to
// produce a correct absolute path regardless of the current working directory.
Expand Down Expand Up @@ -849,7 +849,7 @@ func resolveWorkflowDisplayName(workflowPath, owner, repo, hostname string) stri
}
args = append(args, endpoint, "--jq", ".name")

out, err := workflow.RunGHCombined("Fetching workflow name...", args...)
out, err := workflow.RunGHCombinedContext(ctx, "Fetching workflow name...", args...)
if err != nil {
auditLog.Printf("Failed to fetch workflow display name for %q: %v", workflowPath, err)
return ""
Expand Down
13 changes: 7 additions & 6 deletions pkg/cli/audit_comparison.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"context"
"encoding/json"
"fmt"
"net/url"
Expand Down Expand Up @@ -457,7 +458,7 @@ func collectMCPFailureServers(failures []MCPFailureReport) []string {
return servers
}

func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostname string, verbose bool) ([]WorkflowRun, error) {
func findPreviousSuccessfulWorkflowRuns(ctx context.Context, current WorkflowRun, owner, repo, hostname string, verbose bool) ([]WorkflowRun, error) {
_ = verbose
workflowID := filepath.Base(current.WorkflowPath)
if workflowID == "." || workflowID == "" {
Comment thread
dsyme marked this conversation as resolved.
Expand All @@ -480,7 +481,7 @@ func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostna
}
args = append(args, endpoint, "--jq", jq)

output, err := workflow.RunGHCombined("Fetching previous successful workflow run...", args...)
output, err := workflow.RunGHCombinedContext(ctx, "Fetching previous successful workflow run...", args...)
if err != nil {
return nil, fmt.Errorf("failed to fetch previous successful workflow run: %w", err)
}
Expand All @@ -497,7 +498,7 @@ func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostna

for index := range runs {
if strings.HasPrefix(runs[index].WorkflowName, ".github/") {
if displayName := resolveWorkflowDisplayName(runs[index].WorkflowPath, owner, repo, hostname); displayName != "" {
if displayName := resolveWorkflowDisplayName(ctx, runs[index].WorkflowPath, owner, repo, hostname); displayName != "" {
runs[index].WorkflowName = displayName
}
}
Expand All @@ -506,8 +507,8 @@ func findPreviousSuccessfulWorkflowRuns(current WorkflowRun, owner, repo, hostna
return runs, nil
}

func buildAuditComparisonForRun(currentRun ProcessedRun, currentSnapshot auditComparisonSnapshot, outputDir string, owner, repo, hostname string, verbose bool) *AuditComparisonData {
baselineRuns, err := findPreviousSuccessfulWorkflowRuns(currentRun.Run, owner, repo, hostname, verbose)
func buildAuditComparisonForRun(ctx context.Context, currentRun ProcessedRun, currentSnapshot auditComparisonSnapshot, outputDir string, owner, repo, hostname string, verbose bool) *AuditComparisonData {
baselineRuns, err := findPreviousSuccessfulWorkflowRuns(ctx, currentRun.Run, owner, repo, hostname, verbose)
if err != nil {
auditLog.Printf("Skipping audit comparison: failed to find baseline: %v", err)
return &AuditComparisonData{BaselineFound: false}
Expand All @@ -520,7 +521,7 @@ func buildAuditComparisonForRun(currentRun ProcessedRun, currentSnapshot auditCo
for _, baselineRun := range baselineRuns {
baselineOutputDir := filepath.Join(outputDir, fmt.Sprintf("baseline-%d", baselineRun.DatabaseID))
if _, err := os.Stat(baselineOutputDir); err != nil {
if downloadErr := downloadRunArtifacts(baselineRun.DatabaseID, baselineOutputDir, verbose, owner, repo, hostname, nil); downloadErr != nil {
if downloadErr := downloadRunArtifacts(ctx, baselineRun.DatabaseID, baselineOutputDir, verbose, owner, repo, hostname, nil); downloadErr != nil {
auditLog.Printf("Skipping candidate baseline for run %d: failed to download baseline artifacts: %v", baselineRun.DatabaseID, downloadErr)
continue
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/audit_diff.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"context"
"errors"
"fmt"
"math"
Expand Down Expand Up @@ -681,7 +682,7 @@ func formatCountChange(count1, count2 int) string {
// metrics); otherwise it downloads artifacts and analyzes firewall logs, returning a partial
// summary with only FirewallAnalysis populated.
// artifactFilter restricts which artifacts are downloaded; nil means download all.
func loadRunSummaryForDiff(runID int64, outputDir string, owner, repo, hostname string, verbose bool, artifactFilter []string) (*RunSummary, error) {
func loadRunSummaryForDiff(ctx context.Context, runID int64, outputDir string, owner, repo, hostname string, verbose bool, artifactFilter []string) (*RunSummary, error) {
runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", runID))
if absDir, err := filepath.Abs(runOutputDir); err == nil {
runOutputDir = absDir
Expand All @@ -694,7 +695,7 @@ func loadRunSummaryForDiff(runID int64, outputDir string, owner, repo, hostname
}

// Download artifacts if needed
if err := downloadRunArtifacts(runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter); err != nil {
if err := downloadRunArtifacts(ctx, runID, runOutputDir, verbose, owner, repo, hostname, artifactFilter); err != nil {
if !errors.Is(err, ErrNoArtifacts) {
return nil, fmt.Errorf("failed to download artifacts for run %d: %w", runID, err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/audit_diff_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func RunAuditDiff(ctx context.Context, baseRunID int64, compareRunIDs []int64, o

// Load base run summary once (shared across all comparisons)
fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Loading data for base run %d...", baseRunID)))
baseSummary, err := loadRunSummaryForDiff(baseRunID, outputDir, owner, repo, hostname, verbose, artifactFilter)
baseSummary, err := loadRunSummaryForDiff(ctx, baseRunID, outputDir, owner, repo, hostname, verbose, artifactFilter)
if err != nil {
return fmt.Errorf("failed to load data for base run %d: %w", baseRunID, err)
}
Expand All @@ -153,7 +153,7 @@ func RunAuditDiff(ctx context.Context, baseRunID int64, compareRunIDs []int64, o
}

fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Loading data for run %d...", compareRunID)))
compareSummary, err := loadRunSummaryForDiff(compareRunID, outputDir, owner, repo, hostname, verbose, artifactFilter)
compareSummary, err := loadRunSummaryForDiff(ctx, compareRunID, outputDir, owner, repo, hostname, verbose, artifactFilter)
if err != nil {
return fmt.Errorf("failed to load data for run %d: %w", compareRunID, err)
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"context"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -423,7 +424,7 @@ func TestAuditCachingBehavior(t *testing.T) {
// Verify that downloadRunArtifacts skips download when valid summary exists
// This is tested by checking that the function returns without error
// and doesn't attempt to call `gh run download`
err := downloadRunArtifacts(run.DatabaseID, runOutputDir, false, "", "", "", nil)
err := downloadRunArtifacts(context.Background(), run.DatabaseID, runOutputDir, false, "", "", "", nil)
if err != nil {
t.Errorf("downloadRunArtifacts should skip download when valid summary exists, but got error: %v", err)
}
Expand Down Expand Up @@ -579,7 +580,7 @@ func TestRenderAuditReportUsesProvidedMetrics(t *testing.T) {
// renderAuditReport should complete without error even without GitHub API access.
// No GitHub calls are made because WorkflowPath is empty, causing findPreviousSuccessfulWorkflowRuns
// to return early with an error before any network requests are issued.
err := renderAuditReport(processedRun, metrics, nil, runOutputDir, "", "", "", false, false, false)
err := renderAuditReport(context.Background(), processedRun, metrics, nil, runOutputDir, "", "", "", false, false, false)
if err != nil {
t.Errorf("renderAuditReport returned unexpected error: %v", err)
}
Expand Down
33 changes: 17 additions & 16 deletions pkg/cli/download_workflow.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"context"
"encoding/base64"
"fmt"
"os"
Expand All @@ -18,7 +19,7 @@ import (
var downloadLog = logger.New("cli:download_workflow")

// downloadWorkflowContentViaGit downloads a workflow file using git archive
func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte, error) {
func downloadWorkflowContentViaGit(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) {
if verbose {
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s via git", repo, path, ref)))
}
Expand All @@ -31,12 +32,12 @@ func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte

// git archive command: git archive --remote=<repo> <ref> <path>
// #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the
// developer; exec.Command with separate args (not shell execution) prevents shell injection.
cmd := exec.Command("git", "archive", "--remote="+repoURL, ref, path)
// developer; exec.CommandContext with separate args (not shell execution) prevents shell injection.
cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, path)
archiveOutput, err := cmd.Output()
if err != nil {
// If git archive fails, try with git clone + read file as a fallback
return downloadWorkflowContentViaGitClone(repo, path, ref, verbose)
return downloadWorkflowContentViaGitClone(ctx, repo, path, ref, verbose)
}

// Extract the file from the tar archive using Go's archive/tar (cross-platform)
Expand All @@ -53,7 +54,7 @@ func downloadWorkflowContentViaGit(repo, path, ref string, verbose bool) ([]byte
}

// downloadWorkflowContentViaGitClone downloads a workflow file by shallow cloning with sparse checkout
func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([]byte, error) {
func downloadWorkflowContentViaGitClone(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) {
if verbose {
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s via git clone", repo, path, ref)))
}
Expand All @@ -71,19 +72,19 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([
repoURL := fmt.Sprintf("%s/%s.git", githubHost, repo)

// Initialize git repository
initCmd := exec.Command("git", "-C", tmpDir, "init")
initCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "init")
if output, err := initCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to initialize git repository: %w\nOutput: %s", err, string(output))
}

// Add remote
remoteCmd := exec.Command("git", "-C", tmpDir, "remote", "add", "origin", repoURL)
remoteCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "remote", "add", "origin", repoURL)
if output, err := remoteCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to add remote: %w\nOutput: %s", err, string(output))
}

// Enable sparse-checkout
sparseCmd := exec.Command("git", "-C", tmpDir, "config", "core.sparseCheckout", "true")
sparseCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "config", "core.sparseCheckout", "true")
if output, err := sparseCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to enable sparse checkout: %w\nOutput: %s", err, string(output))
}
Expand All @@ -109,29 +110,29 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([
// Note: sparse-checkout with SHA refs may not reduce bandwidth as much as with branch refs,
// because the server needs to send enough history to reach the specific commit.
// However, it still limits the working directory to only the requested file.
fetchCmd := exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref)
fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref)
if _, err := fetchCmd.CombinedOutput(); err != nil {
// If fetching specific SHA fails, try fetching all branches with depth 1
fetchCmd = exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin")
fetchCmd = exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin")
if output, err := fetchCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, string(output))
}
}

// Checkout the specific commit
checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref)
checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", ref)
if output, err := checkoutCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output))
}
} else {
// For branch/tag refs, fetch the specific ref
fetchCmd := exec.Command("git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref)
fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref)
if output, err := fetchCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to fetch ref %s: %w\nOutput: %s", ref, err, string(output))
}

// Checkout FETCH_HEAD
checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", "FETCH_HEAD")
checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "FETCH_HEAD")
if output, err := checkoutCmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to checkout FETCH_HEAD: %w\nOutput: %s", err, string(output))
}
Expand All @@ -155,21 +156,21 @@ func downloadWorkflowContentViaGitClone(repo, path, ref string, verbose bool) ([
}

// downloadWorkflowContent downloads the content of a workflow file from GitHub
func downloadWorkflowContent(repo, path, ref string, verbose bool) ([]byte, error) {
func downloadWorkflowContent(ctx context.Context, repo, path, ref string, verbose bool) ([]byte, error) {
downloadLog.Printf("Downloading workflow content: %s/%s@%s", repo, path, ref)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Fetching %s/%s@%s", repo, path, ref)))
}

// Use gh CLI to download the file
output, err := workflow.RunGHCombined("Downloading workflow...", "api", fmt.Sprintf("/repos/%s/contents/%s?ref=%s", repo, path, ref), "--jq", ".content")
output, err := workflow.RunGHCombinedContext(ctx, "Downloading workflow...", "api", fmt.Sprintf("/repos/%s/contents/%s?ref=%s", repo, path, ref), "--jq", ".content")
if err != nil {
// Check if this is an authentication error
outputStr := string(output)
if gitutil.IsAuthError(outputStr) || gitutil.IsAuthError(err.Error()) {
downloadLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s@%s", repo, path, ref)
// Try fallback using git commands
content, gitErr := downloadWorkflowContentViaGit(repo, path, ref, verbose)
content, gitErr := downloadWorkflowContentViaGit(ctx, repo, path, ref, verbose)
if gitErr != nil {
return nil, fmt.Errorf("failed to fetch file content via GitHub API and git: API error: %w, Git error: %w", err, gitErr)
}
Expand Down
Loading
Loading