Skip to content

bug: detectFirewallAuditArtifacts silently returns partial results on ReadDir error #29680

@github-actions

Description

@github-actions

Problem

pkg/cli/firewall_policy.go:466–468 uses a bare named return when os.ReadDir fails:

// 4. Legacy separate firewall-audit-logs artifact (backward compat for older runs)
entries, err := os.ReadDir(runDir)
if err != nil {
    return  // returns whatever manifestPath/auditJSONLPath were set so far
}

If os.ReadDir(runDir) fails (e.g., permission denied, path does not exist), the function detectFirewallAuditArtifacts silently returns whatever (manifestPath, auditJSONLPath) values were accumulated from steps 1–3, without surfacing the error.

For security auditing, this means:

  • If the legacy artifact was the only artifact present and steps 1–3 found nothing, the audit silently returns empty results
  • The calling analyzeFirewallPolicy function then calls loadPolicyManifest("") which fails, but the root cause (ReadDir error) is invisible
  • There is no log message or error return to indicate the scan was incomplete

Location

pkg/cli/firewall_policy.go, function detectFirewallAuditArtifacts, line 466–468

Current Code

func detectFirewallAuditArtifacts(runDir string) (manifestPath, auditJSONLPath string) {
    // ... steps 1-3 ...

    // Step 4: legacy artifact
    entries, err := os.ReadDir(runDir)
    if err != nil {
        return  // silent partial return
    }
    // ...
}

Recommended Fix

Change the function signature to return an error, or at minimum log the error:

func detectFirewallAuditArtifacts(runDir string) (manifestPath, auditJSONLPath string, err error) {
    // ... steps 1-3 ...

    entries, readErr := os.ReadDir(runDir)
    if readErr != nil {
        err = fmt.Errorf("detectFirewallAuditArtifacts: reading run dir %s: %w", runDir, readErr)
        return
    }
    // ...
}

Or, if a function signature change is too disruptive, log the error before returning:

entries, err := os.ReadDir(runDir)
if err != nil {
    firewallPolicyLog.Printf("Warning: could not read run directory for legacy artifact scan: %v", err)
    return
}

Impact

  • Severity: Medium (security auditing path)
  • Scenario: Affects gh aw audit when the run directory is inaccessible or partially downloaded
  • Risk: Security audit silently reports incomplete results without indicating why

Validation

  • Add test for detectFirewallAuditArtifacts with a non-readable runDir
  • Verify analyzeFirewallPolicy propagates the error correctly
  • Existing audit tests still pass

Estimated Effort: Small


Generated by Sergo · Run §25243999835

Generated by Sergo - Serena Go Expert · ● 767.1K ·

  • expires on May 9, 2026, 4:56 AM UTC

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions