From e3b3c67020901c9baf76a9ab6e0cd623e69b2bfe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:53:31 +0000 Subject: [PATCH 1/4] fix: extend detectFirewallAuditArtifacts to find audit files in non-flattened agent artifact Previously detectFirewallAuditArtifacts only looked in: - sandbox/firewall/audit/ (post-flattening) - firewall-audit*/ directories (legacy separate artifact) It did not check inside the non-flattened unified agent artifact directory (agent/, agent-artifacts/, or *-agent/ for workflow_call), so the files would not be found when audit is run on a directory populated via `gh run download` without going through flattenUnifiedArtifact. Adds lookup for: - agent/sandbox/firewall/audit/ (new structure, v4+ prefix stripping) - agent/tmp/gh-aw/sandbox/firewall/audit/ (old structure, full path preserved) - agent-artifacts/... (legacy artifact name) - -agent/... (workflow_call prefixed names) Also adds 4 new test cases for these additional lookup paths. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2303d4dd-1c37-4812-aa7c-d740e1a19b96 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../docs/reference/frontmatter-full.md | 25 ++++- pkg/cli/firewall_policy.go | 91 +++++++++++-------- pkg/cli/firewall_policy_test.go | 68 ++++++++++++++ 3 files changed, 144 insertions(+), 40 deletions(-) diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 55ac3b4a676..b4815a6805e 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -5558,12 +5558,24 @@ safe-outputs: # when safe-outputs are configured) threat-detection: true - # Option 2: Threat detection configuration object + # Option 2: GitHub Actions expression that resolves to a boolean at runtime, + # enabling or disabling threat detection based on workflow inputs (e.g. '${{ + # inputs.enable-threat-detection }}') + threat-detection: "example-value" + + # Option 3: Threat detection configuration object threat-detection: - # Whether threat detection is enabled + # Whether threat detection is enabled. Accepts a boolean literal or a GitHub + # Actions expression (e.g. '${{ inputs.enable-threat-detection }}'). # (optional) + # This field supports multiple formats (oneOf): + + # Option 1: boolean enabled: true + # Option 2: GitHub Actions expression that resolves to a boolean at runtime + enabled: "example-value" + # Additional custom prompt instructions to append to threat detection analysis # (optional) prompt: "example-value" @@ -5594,10 +5606,17 @@ safe-outputs: # When true (default), detection failures produce warnings and allow safe outputs # to proceed with a caution notice and 'needs-review' label. When false, detection - # failures block safe outputs entirely. + # failures block safe outputs entirely. Accepts a boolean literal or a GitHub + # Actions expression. # (optional) + # This field supports multiple formats (oneOf): + + # Option 1: boolean continue-on-error: true + # Option 2: GitHub Actions expression that resolves to a boolean at runtime + continue-on-error: "example-value" + # Custom safe-output jobs that can be executed based on agentic workflow output. # Job names containing dashes will be automatically normalized to underscores # (e.g., 'send-notification' becomes 'send_notification'). diff --git a/pkg/cli/firewall_policy.go b/pkg/cli/firewall_policy.go index 74477c4ecce..d92a9615d60 100644 --- a/pkg/cli/firewall_policy.go +++ b/pkg/cli/firewall_policy.go @@ -405,52 +405,69 @@ func enrichWithPolicyRules(entries []AuditLogEntry, manifest *PolicyManifest) *P // detectFirewallAuditArtifacts looks for policy-manifest.json and audit.jsonl in the run directory. // Returns the paths if found, or empty strings if not present. +// +// Search order: +// 1. sandbox/firewall/audit/ — primary path after flattenUnifiedArtifact strips the /tmp/gh-aw/ prefix +// 2. agent/sandbox/firewall/audit/ — non-flattened unified agent artifact (new structure) +// 3. agent/tmp/gh-aw/sandbox/firewall/audit/ — non-flattened unified agent artifact (old structure) +// 4. firewall-audit*/ — legacy separate firewall-audit-logs artifact (backward compat) func detectFirewallAuditArtifacts(runDir string) (manifestPath, auditJSONLPath string) { firewallPolicyLog.Printf("Detecting firewall audit artifacts in: %s", runDir) - // Check for artifacts in sandbox/firewall/audit/ (primary path after artifact download) - auditDir := filepath.Join(runDir, "sandbox", "firewall", "audit") - manifestCandidate := filepath.Join(auditDir, "policy-manifest.json") - auditCandidate := filepath.Join(auditDir, "audit.jsonl") - - if _, err := os.Stat(manifestCandidate); err == nil { - manifestPath = manifestCandidate - firewallPolicyLog.Printf("Found policy manifest: %s", manifestPath) + // checkDir probes dir for policy-manifest.json and audit.jsonl, populating the + // return variables for any files not yet found. Returns true when both are found. + checkDir := func(dir, label string) bool { + if manifestPath == "" { + candidate := filepath.Join(dir, "policy-manifest.json") + if _, err := os.Stat(candidate); err == nil { + manifestPath = candidate + firewallPolicyLog.Printf("Found policy manifest in %s: %s", label, manifestPath) + } + } + if auditJSONLPath == "" { + candidate := filepath.Join(dir, "audit.jsonl") + if _, err := os.Stat(candidate); err == nil { + auditJSONLPath = candidate + firewallPolicyLog.Printf("Found audit JSONL in %s: %s", label, auditJSONLPath) + } + } + return manifestPath != "" && auditJSONLPath != "" } - if _, err := os.Stat(auditCandidate); err == nil { - auditJSONLPath = auditCandidate - firewallPolicyLog.Printf("Found audit JSONL: %s", auditJSONLPath) + // 1. Primary path: sandbox/firewall/audit/ after flattenUnifiedArtifact + if checkDir(filepath.Join(runDir, "sandbox", "firewall", "audit"), "sandbox/firewall/audit") { + return } - // Also check for legacy firewall-audit-logs directory (backward compat for older runs) - if manifestPath == "" || auditJSONLPath == "" { - entries, err := os.ReadDir(runDir) - if err != nil { + // 2 & 3. Non-flattened unified agent artifact (before flattenUnifiedArtifact is called, + // e.g., when the audit command is run on a directory populated via `gh run download`). + // Handles "agent", "agent-artifacts", and workflow_call prefixed names (e.g. "hash-agent"). + if agentDir := findArtifactDir(runDir, "agent", "agent-artifacts"); agentDir != "" { + // New artifact structure (actions/upload-artifact v4+, /tmp/gh-aw/ prefix stripped): + // agent/sandbox/firewall/audit/ + if !checkDir(filepath.Join(agentDir, "sandbox", "firewall", "audit"), "agent/sandbox/firewall/audit") { + // Old artifact structure (/tmp/gh-aw/ prefix preserved inside the artifact): + // agent/tmp/gh-aw/sandbox/firewall/audit/ + checkDir(filepath.Join(agentDir, "tmp", "gh-aw", "sandbox", "firewall", "audit"), "agent/tmp/gh-aw/sandbox/firewall/audit") + } + if manifestPath != "" && auditJSONLPath != "" { return } - for _, entry := range entries { - if !entry.IsDir() { - continue - } - name := entry.Name() - if strings.HasPrefix(name, "firewall-audit") { - dir := filepath.Join(runDir, name) - if manifestPath == "" { - candidate := filepath.Join(dir, "policy-manifest.json") - if _, err := os.Stat(candidate); err == nil { - manifestPath = candidate - firewallPolicyLog.Printf("Found policy manifest in %s: %s", name, manifestPath) - } - } - if auditJSONLPath == "" { - candidate := filepath.Join(dir, "audit.jsonl") - if _, err := os.Stat(candidate); err == nil { - auditJSONLPath = candidate - firewallPolicyLog.Printf("Found audit JSONL in %s: %s", name, auditJSONLPath) - } - } - } + } + + // 4. Legacy separate firewall-audit-logs artifact (backward compat for older runs that + // uploaded the audit directory as a standalone artifact named firewall-audit-logs). + entries, err := os.ReadDir(runDir) + if err != nil { + return + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + name := entry.Name() + if strings.HasPrefix(name, "firewall-audit") { + checkDir(filepath.Join(runDir, name), name) } } diff --git a/pkg/cli/firewall_policy_test.go b/pkg/cli/firewall_policy_test.go index d321e365753..07e8788bb95 100644 --- a/pkg/cli/firewall_policy_test.go +++ b/pkg/cli/firewall_policy_test.go @@ -566,6 +566,74 @@ func TestDetectFirewallAuditArtifacts(t *testing.T) { assert.Equal(t, auditPath, foundAudit, "Should find audit JSONL") }) + t.Run("agent artifact new structure (not yet flattened)", func(t *testing.T) { + // Simulates a directory populated by `gh run download` before flattenUnifiedArtifact + // is called. actions/upload-artifact v4+ strips the /tmp/gh-aw/ common prefix, so + // files land at agent/sandbox/firewall/audit/ inside the downloaded artifact dir. + dir := t.TempDir() + auditDir := filepath.Join(dir, "agent", "sandbox", "firewall", "audit") + require.NoError(t, os.MkdirAll(auditDir, 0755)) + + manifestPath := filepath.Join(auditDir, "policy-manifest.json") + auditPath := filepath.Join(auditDir, "audit.jsonl") + require.NoError(t, os.WriteFile(manifestPath, []byte("{}"), 0644)) + require.NoError(t, os.WriteFile(auditPath, []byte(""), 0644)) + + foundManifest, foundAudit := detectFirewallAuditArtifacts(dir) + assert.Equal(t, manifestPath, foundManifest, "Should find policy manifest in agent/sandbox/firewall/audit") + assert.Equal(t, auditPath, foundAudit, "Should find audit JSONL in agent/sandbox/firewall/audit") + }) + + t.Run("agent artifact old structure with tmp/gh-aw prefix (not yet flattened)", func(t *testing.T) { + // Simulates older artifact structure where the full /tmp/gh-aw/ path was preserved + // inside the agent artifact directory before the v4+ prefix-stripping behavior. + dir := t.TempDir() + auditDir := filepath.Join(dir, "agent", "tmp", "gh-aw", "sandbox", "firewall", "audit") + require.NoError(t, os.MkdirAll(auditDir, 0755)) + + manifestPath := filepath.Join(auditDir, "policy-manifest.json") + auditPath := filepath.Join(auditDir, "audit.jsonl") + require.NoError(t, os.WriteFile(manifestPath, []byte("{}"), 0644)) + require.NoError(t, os.WriteFile(auditPath, []byte(""), 0644)) + + foundManifest, foundAudit := detectFirewallAuditArtifacts(dir) + assert.Equal(t, manifestPath, foundManifest, "Should find policy manifest in agent/tmp/gh-aw/sandbox/firewall/audit") + assert.Equal(t, auditPath, foundAudit, "Should find audit JSONL in agent/tmp/gh-aw/sandbox/firewall/audit") + }) + + t.Run("agent-artifacts legacy artifact name (not yet flattened)", func(t *testing.T) { + // Simulates the legacy "agent-artifacts" artifact name used before the rename to "agent". + dir := t.TempDir() + auditDir := filepath.Join(dir, "agent-artifacts", "sandbox", "firewall", "audit") + require.NoError(t, os.MkdirAll(auditDir, 0755)) + + manifestPath := filepath.Join(auditDir, "policy-manifest.json") + auditPath := filepath.Join(auditDir, "audit.jsonl") + require.NoError(t, os.WriteFile(manifestPath, []byte("{}"), 0644)) + require.NoError(t, os.WriteFile(auditPath, []byte(""), 0644)) + + foundManifest, foundAudit := detectFirewallAuditArtifacts(dir) + assert.Equal(t, manifestPath, foundManifest, "Should find policy manifest in agent-artifacts/sandbox/firewall/audit") + assert.Equal(t, auditPath, foundAudit, "Should find audit JSONL in agent-artifacts/sandbox/firewall/audit") + }) + + t.Run("workflow_call prefixed agent artifact (not yet flattened)", func(t *testing.T) { + // Simulates the workflow_call artifact naming where a hash prefix is added: + // e.g., "abc123-agent" instead of "agent". + dir := t.TempDir() + auditDir := filepath.Join(dir, "abc123-agent", "sandbox", "firewall", "audit") + require.NoError(t, os.MkdirAll(auditDir, 0755)) + + manifestPath := filepath.Join(auditDir, "policy-manifest.json") + auditPath := filepath.Join(auditDir, "audit.jsonl") + require.NoError(t, os.WriteFile(manifestPath, []byte("{}"), 0644)) + require.NoError(t, os.WriteFile(auditPath, []byte(""), 0644)) + + foundManifest, foundAudit := detectFirewallAuditArtifacts(dir) + assert.Equal(t, manifestPath, foundManifest, "Should find policy manifest in prefixed-agent/sandbox/firewall/audit") + assert.Equal(t, auditPath, foundAudit, "Should find audit JSONL in prefixed-agent/sandbox/firewall/audit") + }) + t.Run("firewall-audit-logs directory", func(t *testing.T) { dir := t.TempDir() auditDir := filepath.Join(dir, "firewall-audit-logs") From 4d219d1ea824372d9c05e4a57fff2dd7027f7ac8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:54:27 +0000 Subject: [PATCH 2/4] revert: remove unrelated frontmatter-full.md documentation change Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2303d4dd-1c37-4812-aa7c-d740e1a19b96 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../docs/reference/frontmatter-full.md | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index b4815a6805e..55ac3b4a676 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -5558,24 +5558,12 @@ safe-outputs: # when safe-outputs are configured) threat-detection: true - # Option 2: GitHub Actions expression that resolves to a boolean at runtime, - # enabling or disabling threat detection based on workflow inputs (e.g. '${{ - # inputs.enable-threat-detection }}') - threat-detection: "example-value" - - # Option 3: Threat detection configuration object + # Option 2: Threat detection configuration object threat-detection: - # Whether threat detection is enabled. Accepts a boolean literal or a GitHub - # Actions expression (e.g. '${{ inputs.enable-threat-detection }}'). + # Whether threat detection is enabled # (optional) - # This field supports multiple formats (oneOf): - - # Option 1: boolean enabled: true - # Option 2: GitHub Actions expression that resolves to a boolean at runtime - enabled: "example-value" - # Additional custom prompt instructions to append to threat detection analysis # (optional) prompt: "example-value" @@ -5606,17 +5594,10 @@ safe-outputs: # When true (default), detection failures produce warnings and allow safe outputs # to proceed with a caution notice and 'needs-review' label. When false, detection - # failures block safe outputs entirely. Accepts a boolean literal or a GitHub - # Actions expression. + # failures block safe outputs entirely. # (optional) - # This field supports multiple formats (oneOf): - - # Option 1: boolean continue-on-error: true - # Option 2: GitHub Actions expression that resolves to a boolean at runtime - continue-on-error: "example-value" - # Custom safe-output jobs that can be executed based on agentic workflow output. # Job names containing dashes will be automatically normalized to underscores # (e.g., 'send-notification' becomes 'send_notification'). From e7dba465b25e1ca44e615ef35439ea445125d567 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:07:05 +0000 Subject: [PATCH 3/4] docs: add draft ADR-29360 for multi-path artifact discovery strategy Co-Authored-By: Claude Sonnet 4.6 --- ...h-artifact-discovery-for-firewall-audit.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/adr/29360-multi-path-artifact-discovery-for-firewall-audit.md diff --git a/docs/adr/29360-multi-path-artifact-discovery-for-firewall-audit.md b/docs/adr/29360-multi-path-artifact-discovery-for-firewall-audit.md new file mode 100644 index 00000000000..0e54732d02e --- /dev/null +++ b/docs/adr/29360-multi-path-artifact-discovery-for-firewall-audit.md @@ -0,0 +1,73 @@ +# ADR-29360: Multi-Path Artifact Discovery Strategy for Firewall Audit Command + +**Date**: 2026-04-30 +**Status**: Draft +**Deciders**: pelikhan + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `gh aw audit` command relies on `detectFirewallAuditArtifacts` to locate `policy-manifest.json` and `audit.jsonl` before it can display firewall audit results. Artifact directories may arrive in several layouts depending on how they were obtained: some users run `gh run download` directly without passing the directory through `flattenUnifiedArtifact`, and the internal path structure of the downloaded artifact varies further by the version of `actions/upload-artifact` used (v4+ strips the `/tmp/gh-aw/` common prefix; older releases preserve it). On top of that, the artifact container itself may be named `agent`, `agent-artifacts`, or `-agent` depending on when the workflow was created. The original implementation only covered the post-`flattenUnifiedArtifact` path and the legacy `firewall-audit-logs` directory, leaving the non-flattened cases completely unsupported. + +### Decision + +We will extend `detectFirewallAuditArtifacts` to probe four candidate locations in a fixed priority order: (1) the post-flatten primary path, (2) the non-flattened new artifact structure, (3) the non-flattened old artifact structure, and (4) the legacy standalone `firewall-audit*` directory. The function stops as soon as both files are found and uses a shared `checkDir` closure to eliminate the repetitive stat-and-assign pattern across all search steps. This makes `gh aw audit` work correctly regardless of whether `flattenUnifiedArtifact` has been called and regardless of the `actions/upload-artifact` version in use. + +### Alternatives Considered + +#### Alternative 1: Require `flattenUnifiedArtifact` Before Audit + +Reject non-flattened directories and document that `gh aw audit` requires a pre-processed directory. This would keep the function simple but breaks the manual download workflow — users who run `gh run download` and point the command at the result directory would get a silent failure (no audit output) rather than a useful error. That silent failure is worse than added complexity. + +#### Alternative 2: Normalise on Download + +Run the path-normalisation step (`flattenUnifiedArtifact`) automatically inside `gh aw audit` before probing for files. This would work but couples the audit command to the artifact download pipeline, is harder to test in isolation, and could mutate the caller's directory unexpectedly. + +### Consequences + +#### Positive +- `gh aw audit` works for both the automated pipeline path and the manual `gh run download` path without requiring any user-side workaround. +- Fully backward-compatible: all existing search paths (post-flatten primary and legacy `firewall-audit*`) are preserved at the same priority they had before. +- The `checkDir` closure removes duplicated stat+assign logic, making each search step a single readable call. + +#### Negative +- `detectFirewallAuditArtifacts` now encodes knowledge of four distinct directory layouts; adding a fifth layout in the future requires understanding this ordering contract. +- The function implicitly depends on the naming conventions of `findArtifactDir` (suffixes `agent`, `agent-artifacts`, and `*-agent`); a rename of those artifacts without a corresponding update here would silently break step 2 and 3. + +#### Neutral +- Four new table-driven sub-tests are added to `TestDetectFirewallAuditArtifacts` to cover each newly supported layout; the test file grows by ~68 lines. +- The search order is documented in a block comment on the function signature, establishing a reference point for future readers. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Artifact Discovery Search Order + +1. Implementations **MUST** probe candidate directories in the following fixed priority order and stop as soon as both `policy-manifest.json` and `audit.jsonl` are found: + 1. `/sandbox/firewall/audit/` — post-`flattenUnifiedArtifact` primary path + 2. `/sandbox/firewall/audit/` — non-flattened artifact, `actions/upload-artifact` v4+ (prefix stripped) + 3. `/tmp/gh-aw/sandbox/firewall/audit/` — non-flattened artifact, older upload action (prefix preserved) + 4. `/firewall-audit*/` — legacy standalone audit artifact +2. Implementations **MUST** resolve `` by searching `` for a subdirectory whose name is `agent`, `agent-artifacts`, or matches the pattern `*-agent` (workflow_call hash prefix). +3. Implementations **MUST NOT** mutate or reorder the caller-supplied `runDir` as a side-effect of artifact discovery. +4. Implementations **SHOULD** log the path of each file found and the search label where it was located to aid future debugging. + +### Shared Discovery Helper + +1. Implementations **MUST** use a single shared helper (closure or function) that checks one directory for both `policy-manifest.json` and `audit.jsonl`, so that the check logic is not duplicated across search steps. +2. The shared helper **MUST** only populate a result slot (manifest path or audit path) if that slot has not already been filled by an earlier search step. +3. The shared helper **MUST** return a boolean indicating whether both files have been found, so callers can short-circuit early. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25181134127) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 7c6b5c646159342292ca540daac86f5701dc26ee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:37:02 +0000 Subject: [PATCH 4/4] fix: validate agentDir is a directory and use dynamic label in checkDir logs - Guard against findArtifactDir returning a file path: check os.Stat IsDir() before probing for audit files inside the agent artifact directory - Derive log labels from filepath.Base(agentDir) so debug output reflects the actual on-disk name (agent-artifacts, abc123-agent, etc.) instead of the hardcoded "agent/..." string - Add test: file named "agent" does not panic or falsely match Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5534ec16-9018-4e51-8a41-c13438eecaf6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/firewall_policy.go | 24 +++++++++++++++--------- pkg/cli/firewall_policy_test.go | 11 +++++++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/cli/firewall_policy.go b/pkg/cli/firewall_policy.go index d92a9615d60..36a48667b22 100644 --- a/pkg/cli/firewall_policy.go +++ b/pkg/cli/firewall_policy.go @@ -443,15 +443,21 @@ func detectFirewallAuditArtifacts(runDir string) (manifestPath, auditJSONLPath s // e.g., when the audit command is run on a directory populated via `gh run download`). // Handles "agent", "agent-artifacts", and workflow_call prefixed names (e.g. "hash-agent"). if agentDir := findArtifactDir(runDir, "agent", "agent-artifacts"); agentDir != "" { - // New artifact structure (actions/upload-artifact v4+, /tmp/gh-aw/ prefix stripped): - // agent/sandbox/firewall/audit/ - if !checkDir(filepath.Join(agentDir, "sandbox", "firewall", "audit"), "agent/sandbox/firewall/audit") { - // Old artifact structure (/tmp/gh-aw/ prefix preserved inside the artifact): - // agent/tmp/gh-aw/sandbox/firewall/audit/ - checkDir(filepath.Join(agentDir, "tmp", "gh-aw", "sandbox", "firewall", "audit"), "agent/tmp/gh-aw/sandbox/firewall/audit") - } - if manifestPath != "" && auditJSONLPath != "" { - return + // Guard: findArtifactDir checks existence but not type; skip if it resolved to a file. + if info, err := os.Stat(agentDir); err != nil || !info.IsDir() { + firewallPolicyLog.Printf("Skipping agent artifact path (not a directory): %s", agentDir) + } else { + agentBase := filepath.Base(agentDir) + // New artifact structure (actions/upload-artifact v4+, /tmp/gh-aw/ prefix stripped): + // /sandbox/firewall/audit/ + if !checkDir(filepath.Join(agentDir, "sandbox", "firewall", "audit"), agentBase+"/sandbox/firewall/audit") { + // Old artifact structure (/tmp/gh-aw/ prefix preserved inside the artifact): + // /tmp/gh-aw/sandbox/firewall/audit/ + checkDir(filepath.Join(agentDir, "tmp", "gh-aw", "sandbox", "firewall", "audit"), agentBase+"/tmp/gh-aw/sandbox/firewall/audit") + } + if manifestPath != "" && auditJSONLPath != "" { + return + } } } diff --git a/pkg/cli/firewall_policy_test.go b/pkg/cli/firewall_policy_test.go index 07e8788bb95..347288dda37 100644 --- a/pkg/cli/firewall_policy_test.go +++ b/pkg/cli/firewall_policy_test.go @@ -655,6 +655,17 @@ func TestDetectFirewallAuditArtifacts(t *testing.T) { assert.Empty(t, foundManifest, "Should not find manifest") assert.Empty(t, foundAudit, "Should not find audit JSONL") }) + + t.Run("file named 'agent' does not panic or falsely match", func(t *testing.T) { + // If a plain file happens to be named "agent" in the run directory (e.g., a flattened + // single-file artifact from a different upload), the lookup must skip it gracefully. + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "agent"), []byte("not a directory"), 0644)) + + foundManifest, foundAudit := detectFirewallAuditArtifacts(dir) + assert.Empty(t, foundManifest, "Should not find manifest when 'agent' is a file") + assert.Empty(t, foundAudit, "Should not find audit JSONL when 'agent' is a file") + }) } func TestAnalyzeFirewallPolicy(t *testing.T) {