From 7e5b2e7a00f4b2a68d9b80097c4aadf3d01a55d2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 21:08:54 +0000 Subject: [PATCH 1/2] Initial plan From 96b79a1e252fec67b98054ef608eb8e9ee6ff7cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 21:20:54 +0000 Subject: [PATCH 2/2] Remove parseRunURL() wrapper and RunURLInfo struct - Removed unnecessary parseRunURL() wrapper function - Removed RunURLInfo struct definition - Updated all functions to use parser.ParseRunURL() directly with individual return values - Updated AuditWorkflowRun() signature to accept individual parameters - Updated fetchWorkflowRunMetadata() signature to accept individual parameters - Updated test cases to remove RunURLInfo struct usage - All tests passing Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- .github/workflows/release.lock.yml | 6 +- pkg/cli/audit.go | 74 ++++++--------- pkg/cli/audit_test.go | 140 ++++++++++++++--------------- 3 files changed, 94 insertions(+), 126 deletions(-) diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index 184dd51541d..e459c176fb4 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -5968,19 +5968,19 @@ jobs: - name: Download Go modules run: go mod download - name: Generate SBOM (SPDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 with: artifact-name: sbom.spdx.json format: spdx-json output-file: sbom.spdx.json - name: Generate SBOM (CycloneDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 with: artifact-name: sbom.cdx.json format: cyclonedx-json output-file: sbom.cdx.json - name: Upload SBOM artifacts - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 with: name: sbom-artifacts path: | diff --git a/pkg/cli/audit.go b/pkg/cli/audit.go index e1e0cff7e25..e70c991965c 100644 --- a/pkg/cli/audit.go +++ b/pkg/cli/audit.go @@ -57,7 +57,7 @@ Examples: runIDOrURL := args[0] // Parse run information from input (either numeric ID or URL) - runInfo, err := parseRunURL(runIDOrURL) + runID, owner, repo, hostname, err := parser.ParseRunURL(runIDOrURL) if err != nil { return err } @@ -67,7 +67,7 @@ Examples: jsonOutput, _ := cmd.Flags().GetBool("json") parse, _ := cmd.Flags().GetBool("parse") - return AuditWorkflowRun(runInfo, outputDir, verbose, parse, jsonOutput) + return AuditWorkflowRun(runID, owner, repo, hostname, outputDir, verbose, parse, jsonOutput) }, } @@ -82,35 +82,13 @@ Examples: return auditCmd } -// RunURLInfo contains the parsed information from a workflow run URL -type RunURLInfo struct { - RunID int64 - Owner string - Repo string - Hostname string -} - // extractRunID extracts the run ID from either a numeric string or a GitHub Actions URL func extractRunID(input string) (int64, error) { - info, err := parseRunURL(input) + runID, _, _, _, err := parser.ParseRunURL(input) if err != nil { return 0, err } - return info.RunID, nil -} - -// parseRunURL parses a run ID or URL and extracts all relevant information -func parseRunURL(input string) (RunURLInfo, error) { - runID, owner, repo, hostname, err := parser.ParseRunURL(input) - if err != nil { - return RunURLInfo{}, err - } - return RunURLInfo{ - RunID: runID, - Owner: owner, - Repo: repo, - Hostname: hostname, - }, nil + return runID, nil } // isPermissionError checks if an error is related to permissions/authentication @@ -127,21 +105,21 @@ func isPermissionError(err error) bool { } // AuditWorkflowRun audits a single workflow run and generates a report -func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse bool, jsonOutput bool) error { - auditLog.Printf("Starting audit for workflow run: runID=%d, owner=%s, repo=%s", runInfo.RunID, runInfo.Owner, runInfo.Repo) +func AuditWorkflowRun(runID int64, owner, repo, hostname string, outputDir string, verbose bool, parse bool, jsonOutput bool) error { + auditLog.Printf("Starting audit for workflow run: runID=%d, owner=%s, repo=%s", runID, owner, repo) if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Auditing workflow run %d...", runInfo.RunID))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Auditing workflow run %d...", runID))) } - runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", runInfo.RunID)) + runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", runID)) auditLog.Printf("Using output directory: %s", runOutputDir) // 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(runInfo, verbose) + run, metadataErr := fetchWorkflowRunMetadata(runID, owner, repo, hostname, verbose) var useLocalCache bool if metadataErr != nil { @@ -158,7 +136,7 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse " - run_id: %d\n"+ " - output_directory: %s\n\n"+ "2. After downloading, run this audit command again to analyze the cached artifacts.\n\n"+ - "Original error: %v", runInfo.RunID, runOutputDir, metadataErr) + "Original error: %v", runID, runOutputDir, metadataErr) } } else { return fmt.Errorf("failed to fetch run metadata: %w", metadataErr) @@ -171,12 +149,12 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse } // Download artifacts for the run - auditLog.Printf("Downloading artifacts for run %d", runInfo.RunID) - err := downloadRunArtifacts(runInfo.RunID, runOutputDir, verbose) + auditLog.Printf("Downloading artifacts for run %d", runID) + err := downloadRunArtifacts(runID, runOutputDir, verbose) if err != nil { // Gracefully handle cases where the run legitimately has no artifacts if errors.Is(err, ErrNoArtifacts) { - auditLog.Printf("No artifacts found for run %d", runInfo.RunID) + auditLog.Printf("No artifacts found for run %d", runID) if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage("No artifacts attached to this run. Proceeding with metadata-only audit.")) } @@ -191,7 +169,7 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse " - run_id: %d\n"+ " - output_directory: %s\n\n"+ "2. After downloading, run this audit command again to analyze the cached artifacts.\n\n"+ - "Original error: %v", runInfo.RunID, runOutputDir, err) + "Original error: %v", runID, runOutputDir, err) } } else { return fmt.Errorf("failed to download artifacts: %w", err) @@ -202,8 +180,8 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse // If using local cache without metadata, create a minimal run structure if useLocalCache && run.DatabaseID == 0 { run = WorkflowRun{ - DatabaseID: runInfo.RunID, - WorkflowName: fmt.Sprintf("Workflow Run %d", runInfo.RunID), + DatabaseID: runID, + WorkflowName: fmt.Sprintf("Workflow Run %d", runID), Status: "unknown", LogsPath: runOutputDir, } @@ -319,13 +297,13 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse if engine := extractEngineFromAwInfo(awInfoPath, verbose); engine != nil { // reuse existing helper in same package if err := parseAgentLog(runOutputDir, engine, verbose); err != nil { if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse agent log for run %d: %v", runInfo.RunID, err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse agent log for run %d: %v", runID, err))) } } else { // Always show success message for parsing, not just in verbose mode logMdPath := filepath.Join(runOutputDir, "log.md") if _, err := os.Stat(logMdPath); err == nil { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("✓ Parsed log for run %d → %s", runInfo.RunID, logMdPath))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("✓ Parsed log for run %d → %s", runID, logMdPath))) } } } else if verbose { @@ -335,13 +313,13 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse // Also parse firewall logs if they exist if err := parseFirewallLogs(runOutputDir, verbose); err != nil { if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse firewall logs for run %d: %v", runInfo.RunID, err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse firewall logs for run %d: %v", runID, err))) } } else { // Show success message if firewall.md was created firewallMdPath := filepath.Join(runOutputDir, "firewall.md") if _, err := os.Stat(firewallMdPath); err == nil { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("✓ Parsed firewall logs for run %d → %s", runInfo.RunID, firewallMdPath))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("✓ Parsed firewall logs for run %d → %s", runID, firewallMdPath))) } } } @@ -379,22 +357,22 @@ func AuditWorkflowRun(runInfo RunURLInfo, outputDir string, verbose bool, parse } // fetchWorkflowRunMetadata fetches metadata for a single workflow run -func fetchWorkflowRunMetadata(runInfo RunURLInfo, verbose bool) (WorkflowRun, error) { +func fetchWorkflowRunMetadata(runID int64, owner, repo, hostname string, verbose bool) (WorkflowRun, error) { // Build the API endpoint var endpoint string - if runInfo.Owner != "" && runInfo.Repo != "" { + if owner != "" && repo != "" { // Use explicit owner/repo from the URL - endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d", runInfo.Owner, runInfo.Repo, runInfo.RunID) + endpoint = fmt.Sprintf("repos/%s/%s/actions/runs/%d", owner, repo, runID) } else { // Fall back to {owner}/{repo} placeholders for context-based resolution - endpoint = fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d", runInfo.RunID) + endpoint = fmt.Sprintf("repos/{owner}/{repo}/actions/runs/%d", runID) } args := []string{"api"} // Add hostname flag if specified (for GitHub Enterprise) - if runInfo.Hostname != "" && runInfo.Hostname != "github.com" { - args = append(args, "--hostname", runInfo.Hostname) + if hostname != "" && hostname != "github.com" { + args = append(args, "--hostname", hostname) } args = append(args, diff --git a/pkg/cli/audit_test.go b/pkg/cli/audit_test.go index 49740f1df4e..1cca9a7c0d5 100644 --- a/pkg/cli/audit_test.go +++ b/pkg/cli/audit_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/githubnext/gh-aw/pkg/parser" "github.com/githubnext/gh-aw/pkg/testutil" "github.com/githubnext/gh-aw/pkg/workflow" ) @@ -105,94 +106,83 @@ func TestExtractRunID(t *testing.T) { func TestParseRunURL(t *testing.T) { tests := []struct { - name string - input string - expectedInfo RunURLInfo - shouldErr bool + name string + input string + expectedRunID int64 + expectedOwner string + expectedRepo string + expectedHostname string + shouldErr bool }{ { - name: "Numeric run ID", - input: "1234567890", - expectedInfo: RunURLInfo{ - RunID: 1234567890, - Owner: "", - Repo: "", - Hostname: "", - }, - shouldErr: false, + name: "Numeric run ID", + input: "1234567890", + expectedRunID: 1234567890, + expectedOwner: "", + expectedRepo: "", + expectedHostname: "", + shouldErr: false, }, { - name: "Run URL with /actions/runs/", - input: "https://github.com/owner/repo/actions/runs/12345678", - expectedInfo: RunURLInfo{ - RunID: 12345678, - Owner: "owner", - Repo: "repo", - Hostname: "github.com", - }, - shouldErr: false, + name: "Run URL with /actions/runs/", + input: "https://github.com/owner/repo/actions/runs/12345678", + expectedRunID: 12345678, + expectedOwner: "owner", + expectedRepo: "repo", + expectedHostname: "github.com", + shouldErr: false, }, { - name: "Job URL", - input: "https://github.com/owner/repo/actions/runs/12345678/job/98765432", - expectedInfo: RunURLInfo{ - RunID: 12345678, - Owner: "owner", - Repo: "repo", - Hostname: "github.com", - }, - shouldErr: false, + name: "Job URL", + input: "https://github.com/owner/repo/actions/runs/12345678/job/98765432", + expectedRunID: 12345678, + expectedOwner: "owner", + expectedRepo: "repo", + expectedHostname: "github.com", + shouldErr: false, }, { - name: "Workflow run URL without /actions/", - input: "https://github.com/owner/repo/runs/12345678", - expectedInfo: RunURLInfo{ - RunID: 12345678, - Owner: "owner", - Repo: "repo", - Hostname: "github.com", - }, - shouldErr: false, + name: "Workflow run URL without /actions/", + input: "https://github.com/owner/repo/runs/12345678", + expectedRunID: 12345678, + expectedOwner: "owner", + expectedRepo: "repo", + expectedHostname: "github.com", + shouldErr: false, }, { - name: "GitHub Enterprise URL", - input: "https://github.example.com/owner/repo/actions/runs/12345678", - expectedInfo: RunURLInfo{ - RunID: 12345678, - Owner: "owner", - Repo: "repo", - Hostname: "github.example.com", - }, - shouldErr: false, + name: "GitHub Enterprise URL", + input: "https://github.example.com/owner/repo/actions/runs/12345678", + expectedRunID: 12345678, + expectedOwner: "owner", + expectedRepo: "repo", + expectedHostname: "github.example.com", + shouldErr: false, }, { - name: "GitHub Enterprise URL without /actions/", - input: "https://ghe.company.com/myorg/myrepo/runs/99999", - expectedInfo: RunURLInfo{ - RunID: 99999, - Owner: "myorg", - Repo: "myrepo", - Hostname: "ghe.company.com", - }, - shouldErr: false, + name: "GitHub Enterprise URL without /actions/", + input: "https://ghe.company.com/myorg/myrepo/runs/99999", + expectedRunID: 99999, + expectedOwner: "myorg", + expectedRepo: "myrepo", + expectedHostname: "ghe.company.com", + shouldErr: false, }, { - name: "Invalid URL format", - input: "https://github.com/owner/repo/actions", - expectedInfo: RunURLInfo{}, - shouldErr: true, + name: "Invalid URL format", + input: "https://github.com/owner/repo/actions", + shouldErr: true, }, { - name: "Invalid string", - input: "not-a-number", - expectedInfo: RunURLInfo{}, - shouldErr: true, + name: "Invalid string", + input: "not-a-number", + shouldErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := parseRunURL(tt.input) + runID, owner, repo, hostname, err := parser.ParseRunURL(tt.input) if tt.shouldErr { if err == nil { @@ -202,17 +192,17 @@ func TestParseRunURL(t *testing.T) { if err != nil { t.Errorf("Unexpected error: %v", err) } - if result.RunID != tt.expectedInfo.RunID { - t.Errorf("Expected run ID %d, got %d", tt.expectedInfo.RunID, result.RunID) + if runID != tt.expectedRunID { + t.Errorf("Expected run ID %d, got %d", tt.expectedRunID, runID) } - if result.Owner != tt.expectedInfo.Owner { - t.Errorf("Expected owner '%s', got '%s'", tt.expectedInfo.Owner, result.Owner) + if owner != tt.expectedOwner { + t.Errorf("Expected owner '%s', got '%s'", tt.expectedOwner, owner) } - if result.Repo != tt.expectedInfo.Repo { - t.Errorf("Expected repo '%s', got '%s'", tt.expectedInfo.Repo, result.Repo) + if repo != tt.expectedRepo { + t.Errorf("Expected repo '%s', got '%s'", tt.expectedRepo, repo) } - if result.Hostname != tt.expectedInfo.Hostname { - t.Errorf("Expected hostname '%s', got '%s'", tt.expectedInfo.Hostname, result.Hostname) + if hostname != tt.expectedHostname { + t.Errorf("Expected hostname '%s', got '%s'", tt.expectedHostname, hostname) } } })