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
11 changes: 11 additions & 0 deletions pkg/cli/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,17 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze firewall logs: %v", err)))
}

// Supplement firewall analysis with blocked domains extracted directly from
// agent-stdio.log (e.g., Codex CLI emits "--allow-domains <domain>" warnings
// when the sandbox firewall denies a network request).
if agentLogFirewall := extractFirewallFromAgentLog(runOutputDir, verbose); agentLogFirewall != nil {
if firewallAnalysis == nil {
firewallAnalysis = agentLogFirewall
} else {
firewallAnalysis.AddMetrics(agentLogFirewall)
}
}

// Analyze firewall policy artifacts if available (policy-manifest.json + audit.jsonl)
policyAnalysis, err := analyzeFirewallPolicy(runOutputDir, verbose)
if err != nil && verbose {
Expand Down
10 changes: 10 additions & 0 deletions pkg/cli/audit_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"bufio"
"encoding/json"
"math"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -274,6 +275,15 @@ func buildAuditData(processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage
WarningCount: run.WarningCount,
}

// Populate ActionMinutes from run duration so it is always visible even
// when token/turn metrics are zero (e.g. Codex runs that exit early).
// Use math.Ceil to match the billable-minute rounding used elsewhere.
if run.ActionMinutes > 0 {
metricsData.ActionMinutes = run.ActionMinutes
} else if run.Duration > 0 {
metricsData.ActionMinutes = math.Ceil(run.Duration.Minutes())
}
Comment on lines +278 to +285
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActionMinutes is described and used elsewhere as billable Actions minutes (rounded up), but here it’s set to run.Duration.Minutes() which can be fractional and inconsistent with math.Ceil(run.Duration.Minutes()) used in other code paths. Consider setting metricsData.ActionMinutes from run.ActionMinutes when available, or computing it as ceil(duration minutes) to match existing semantics.

Copilot uses AI. Check for mistakes.

// Build job data
jobs := sliceutil.Map(processedRun.JobDetails, func(jobDetail JobInfoWithDuration) JobData {
job := JobData{
Expand Down
35 changes: 29 additions & 6 deletions pkg/cli/audit_report_analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,23 @@ func generateFindings(processedRun ProcessedRun, metrics MetricsData, errors []E

// Firewall findings
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 0 {
blockedDomains := processedRun.FirewallAnalysis.GetBlockedDomains()
var desc string
switch {
case len(blockedDomains) == 1:
desc = "Agent attempted to access blocked domain: " + blockedDomains[0]
case len(blockedDomains) > 1 && len(blockedDomains) <= 3:
desc = "Agent attempted to access blocked domains: " + strings.Join(blockedDomains, ", ")
case len(blockedDomains) > 3:
desc = fmt.Sprintf("Agent attempted to access %d blocked domains, including: %s", len(blockedDomains), strings.Join(blockedDomains[:3], ", "))
default:
desc = fmt.Sprintf("%d network request(s) were blocked by firewall", processedRun.FirewallAnalysis.BlockedRequests)
}
findings = append(findings, Finding{
Category: "network",
Severity: "medium",
Title: "Blocked Network Requests",
Description: fmt.Sprintf("%d network requests were blocked by firewall", processedRun.FirewallAnalysis.BlockedRequests),
Description: desc,
Impact: "Blocked requests may indicate missing network permissions or unexpected behavior",
})
}
Expand Down Expand Up @@ -238,13 +250,24 @@ func generateRecommendations(processedRun ProcessedRun, metrics MetricsData, fin
})
}

// Recommendations for firewall blocks
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 10 {
// Recommendations for firewall blocks – trigger on any block so even a single
// domain denial (e.g. Codex CLI reporting one blocked domain) surfaces an action.
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 0 {
blockedDomains := processedRun.FirewallAnalysis.GetBlockedDomains()
var example string
if len(blockedDomains) > 0 {
example = fmt.Sprintf(
"Add the blocked domain(s) to your workflow frontmatter:\n\n```yaml\nnetwork:\n allowed:\n - %s\n```",
strings.Join(blockedDomains, "\n - "),
)
} else {
example = "Add allowed domains to network configuration or review firewall rules"
}
recommendations = append(recommendations, Recommendation{
Priority: "medium",
Action: "Review network access configuration",
Reason: "Many blocked requests suggest missing network permissions",
Example: "Add allowed domains to network configuration or review firewall rules",
Action: "Add blocked domains to the workflow network allow-list",
Reason: "Firewall-blocked domains prevent the agent from completing its tasks",
Example: example,
})
}

Expand Down
104 changes: 104 additions & 0 deletions pkg/cli/audit_report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1534,3 +1534,107 @@ func TestExtractPreAgentStepErrors(t *testing.T) {
assert.Contains(t, files, "agent/Run agent", "Should include subdirectory step error")
})
}

func TestBuildAuditDataActionMinutes(t *testing.T) {
// Verify that action_minutes is populated from run.Duration even when
// token/turn metrics are zero (e.g. Codex runs that exit early).
// math.Ceil should be applied, and a pre-set run.ActionMinutes should take precedence.
t.Run("derived from Duration with ceil", func(t *testing.T) {
processedRun := ProcessedRun{
Run: WorkflowRun{
DatabaseID: 42,
WorkflowName: "Codex Test",
Status: "completed",
Conclusion: "failure",
Duration: 6*time.Minute + 30*time.Second, // 6.5 minutes → ceil = 7
TokenUsage: 0,
Turns: 0,
ErrorCount: 1,
WarningCount: 0,
},
}

metrics := workflow.LogMetrics{}
auditData := buildAuditData(processedRun, metrics, nil)

assert.InDelta(t, 7.0, auditData.Metrics.ActionMinutes, 0.01,
"ActionMinutes should be ceil of duration minutes (6.5m → 7)")
assert.Equal(t, 0, auditData.Metrics.TokenUsage,
"TokenUsage should be 0 when no log metrics available")
assert.Equal(t, 0, auditData.Metrics.Turns,
"Turns should be 0 when no log metrics available")
})

t.Run("uses pre-set ActionMinutes when available", func(t *testing.T) {
processedRun := ProcessedRun{
Run: WorkflowRun{
DatabaseID: 43,
WorkflowName: "Pre-set Test",
Status: "completed",
Conclusion: "success",
Duration: 6 * time.Minute,
ActionMinutes: 8.0, // pre-set by orchestrator, takes precedence
},
}

metrics := workflow.LogMetrics{}
auditData := buildAuditData(processedRun, metrics, nil)

assert.InDelta(t, 8.0, auditData.Metrics.ActionMinutes, 0.01,
"Pre-set ActionMinutes should take precedence over Duration")
})
}

func TestGenerateFindingsFirewallWithBlockedDomains(t *testing.T) {
// A single blocked domain should produce a finding naming the domain.
pr := createTestProcessedRun()
fw := &FirewallAnalysis{
TotalRequests: 1,
BlockedRequests: 1,
AllowedRequests: 0,
RequestsByDomain: map[string]DomainRequestStats{},
}
fw.SetBlockedDomains([]string{"chatgpt.com"})
pr.FirewallAnalysis = fw

findings := generateFindings(pr, MetricsData{}, nil, nil)

var networkFinding *Finding
for i := range findings {
if findings[i].Category == "network" {
networkFinding = &findings[i]
break
}
}
require.NotNil(t, networkFinding, "Should generate a network finding when firewall blocks a domain")
assert.Contains(t, networkFinding.Description, "chatgpt.com",
"Finding description should name the blocked domain")
}

func TestGenerateRecommendationsFirewallSingleBlock(t *testing.T) {
// Even a single blocked request (threshold changed from >10 to >0)
// should generate a recommendation with the domain name in the example.
pr := createTestProcessedRun()
fw := &FirewallAnalysis{
TotalRequests: 1,
BlockedRequests: 1,
AllowedRequests: 0,
RequestsByDomain: map[string]DomainRequestStats{},
}
fw.SetBlockedDomains([]string{"chatgpt.com"})
pr.FirewallAnalysis = fw

findings := []Finding{{Category: "network", Severity: "medium", Title: "Blocked Network Requests"}}
recs := generateRecommendations(pr, MetricsData{}, findings)

var networkRec *Recommendation
for i := range recs {
if strings.Contains(recs[i].Action, "network") || strings.Contains(recs[i].Action, "blocked") {
networkRec = &recs[i]
break
}
}
require.NotNil(t, networkRec, "Should generate a network recommendation for any firewall block")
assert.Contains(t, networkRec.Example, "chatgpt.com",
"Recommendation example should include the blocked domain name")
}
110 changes: 110 additions & 0 deletions pkg/cli/firewall_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ var firewallLogLog = logger.New("cli:firewall_log")
// Pre-compiled regexes for firewall log parsing (performance optimization)
var (
firewallLogFieldSplitter = regexp.MustCompile(`(?:[^\s"]+|"[^"]*")+`)

// agentLogAllowDomainsPattern matches Codex CLI firewall warning lines that suggest
// adding a blocked domain to the allow-list.
// Captures the full token after --allow-domains (up to whitespace) to support
// hostnames, protocol prefixes, wildcard patterns, and ports.
// Example: "add --allow-domains chatgpt.com to your command"
// Example: "add --allow-domains chatgpt.com,other.com to your command"
// Example: "add --allow-domains https://api.example.com:443 to your command"
agentLogAllowDomainsPattern = regexp.MustCompile(`--allow-domains\s+([^\s]+)`)
)

// Firewall Log Parser
Expand Down Expand Up @@ -127,7 +136,44 @@ func (f *FirewallAnalysis) AddMetrics(other LogAnalysis) {
f.AllowedRequests += otherFirewall.AllowedRequests
f.BlockedRequests += otherFirewall.BlockedRequests

// Merge blocked domain lists
if len(otherFirewall.BlockedDomains) > 0 {
domainSet := make(map[string]bool, len(f.BlockedDomains)+len(otherFirewall.BlockedDomains))
for _, d := range f.BlockedDomains {
domainSet[d] = true
}
for _, d := range otherFirewall.BlockedDomains {
domainSet[d] = true
}
merged := make([]string, 0, len(domainSet))
for d := range domainSet {
merged = append(merged, d)
}
sort.Strings(merged)
f.SetBlockedDomains(merged)
}

// Merge allowed domain lists
if len(otherFirewall.AllowedDomains) > 0 {
domainSet := make(map[string]bool, len(f.AllowedDomains)+len(otherFirewall.AllowedDomains))
for _, d := range f.AllowedDomains {
domainSet[d] = true
}
for _, d := range otherFirewall.AllowedDomains {
domainSet[d] = true
}
merged := make([]string, 0, len(domainSet))
for d := range domainSet {
merged = append(merged, d)
}
sort.Strings(merged)
f.SetAllowedDomains(merged)
}

// Merge request stats by domain
if f.RequestsByDomain == nil {
f.RequestsByDomain = make(map[string]DomainRequestStats)
}
for domain, stats := range otherFirewall.RequestsByDomain {
existing := f.RequestsByDomain[domain]
existing.Allowed += stats.Allowed
Expand Down Expand Up @@ -399,3 +445,67 @@ func analyzeMultipleFirewallLogs(logsDir string, verbose bool) (*FirewallAnalysi
},
)
}

// extractFirewallFromAgentLog scans agent-stdio.log for firewall-blocked domain warnings.
// This supplements dedicated proxy firewall logs (e.g., Squid access logs) by extracting
// network-block information from the agent's own output when proxy logs are unavailable.
//
// The Codex CLI emits lines containing "--allow-domains <domain>" when a domain is blocked
// by the sandbox firewall. For example:
//
// "[WARN] chatgpt.com is not in the allowed domains. To allow access, add --allow-domains chatgpt.com to your command."
//
// Returns nil when no agent-stdio.log exists or no blocked domains are found.
func extractFirewallFromAgentLog(logsPath string, verbose bool) *FirewallAnalysis {
agentStdioPath := filepath.Clean(filepath.Join(logsPath, "agent-stdio.log"))
content, err := os.ReadFile(agentStdioPath) // #nosec G304 -- path is cleaned via filepath.Clean and logsPath is a trusted run output directory
if err != nil {
// File not present is normal (agent didn't run, or run used a different log path)
firewallLogLog.Printf("No agent-stdio.log found at %s: %v", agentStdioPath, err)
return nil
}

blockedDomainsSet := make(map[string]bool)
for line := range strings.SplitSeq(string(content), "\n") {
if matches := agentLogAllowDomainsPattern.FindStringSubmatch(line); len(matches) > 1 {
// Domains can be comma-separated in the suggestion
for domain := range strings.SplitSeq(matches[1], ",") {
if d := strings.TrimSpace(domain); d != "" {
blockedDomainsSet[d] = true
}
}
}
}

if len(blockedDomainsSet) == 0 {
firewallLogLog.Printf("No blocked domains found in agent-stdio.log at %s", agentStdioPath)
return nil
}

blockedDomains := make([]string, 0, len(blockedDomainsSet))
for d := range blockedDomainsSet {
blockedDomains = append(blockedDomains, d)
}
sort.Strings(blockedDomains)

analysis := &FirewallAnalysis{
TotalRequests: len(blockedDomains),
AllowedRequests: 0,
BlockedRequests: len(blockedDomains),
RequestsByDomain: make(map[string]DomainRequestStats),
}
analysis.SetBlockedDomains(blockedDomains)
for _, d := range blockedDomains {
analysis.RequestsByDomain[d] = DomainRequestStats{Blocked: 1}
}

firewallLogLog.Printf("Extracted %d firewall-blocked domain(s) from agent-stdio.log: %s", len(blockedDomains), strings.Join(blockedDomains, ", "))
if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf(
"Found %d firewall-blocked domain(s) in agent log: %s",
len(blockedDomains), strings.Join(blockedDomains, ", "),
)))
}

return analysis
}
Loading