Skip to content

Commit 596a23f

Browse files
authored
fix(audit): surface Codex firewall blocks from agent-stdio.log and populate action_minutes (#23889)
1 parent 515c8ef commit 596a23f

6 files changed

Lines changed: 442 additions & 6 deletions

File tree

pkg/cli/audit.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,17 @@ func AuditWorkflowRun(ctx context.Context, runID int64, owner, repo, hostname st
327327
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze firewall logs: %v", err)))
328328
}
329329

330+
// Supplement firewall analysis with blocked domains extracted directly from
331+
// agent-stdio.log (e.g., Codex CLI emits "--allow-domains <domain>" warnings
332+
// when the sandbox firewall denies a network request).
333+
if agentLogFirewall := extractFirewallFromAgentLog(runOutputDir, verbose); agentLogFirewall != nil {
334+
if firewallAnalysis == nil {
335+
firewallAnalysis = agentLogFirewall
336+
} else {
337+
firewallAnalysis.AddMetrics(agentLogFirewall)
338+
}
339+
}
340+
330341
// Analyze firewall policy artifacts if available (policy-manifest.json + audit.jsonl)
331342
policyAnalysis, err := analyzeFirewallPolicy(runOutputDir, verbose)
332343
if err != nil && verbose {

pkg/cli/audit_report.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"bufio"
55
"encoding/json"
6+
"math"
67
"os"
78
"path/filepath"
89
"strconv"
@@ -274,6 +275,15 @@ func buildAuditData(processedRun ProcessedRun, metrics LogMetrics, mcpToolUsage
274275
WarningCount: run.WarningCount,
275276
}
276277

278+
// Populate ActionMinutes from run duration so it is always visible even
279+
// when token/turn metrics are zero (e.g. Codex runs that exit early).
280+
// Use math.Ceil to match the billable-minute rounding used elsewhere.
281+
if run.ActionMinutes > 0 {
282+
metricsData.ActionMinutes = run.ActionMinutes
283+
} else if run.Duration > 0 {
284+
metricsData.ActionMinutes = math.Ceil(run.Duration.Minutes())
285+
}
286+
277287
// Build job data
278288
jobs := sliceutil.Map(processedRun.JobDetails, func(jobDetail JobInfoWithDuration) JobData {
279289
job := JobData{

pkg/cli/audit_report_analysis.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,23 @@ func generateFindings(processedRun ProcessedRun, metrics MetricsData, errors []E
143143

144144
// Firewall findings
145145
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 0 {
146+
blockedDomains := processedRun.FirewallAnalysis.GetBlockedDomains()
147+
var desc string
148+
switch {
149+
case len(blockedDomains) == 1:
150+
desc = "Agent attempted to access blocked domain: " + blockedDomains[0]
151+
case len(blockedDomains) > 1 && len(blockedDomains) <= 3:
152+
desc = "Agent attempted to access blocked domains: " + strings.Join(blockedDomains, ", ")
153+
case len(blockedDomains) > 3:
154+
desc = fmt.Sprintf("Agent attempted to access %d blocked domains, including: %s", len(blockedDomains), strings.Join(blockedDomains[:3], ", "))
155+
default:
156+
desc = fmt.Sprintf("%d network request(s) were blocked by firewall", processedRun.FirewallAnalysis.BlockedRequests)
157+
}
146158
findings = append(findings, Finding{
147159
Category: "network",
148160
Severity: "medium",
149161
Title: "Blocked Network Requests",
150-
Description: fmt.Sprintf("%d network requests were blocked by firewall", processedRun.FirewallAnalysis.BlockedRequests),
162+
Description: desc,
151163
Impact: "Blocked requests may indicate missing network permissions or unexpected behavior",
152164
})
153165
}
@@ -238,13 +250,24 @@ func generateRecommendations(processedRun ProcessedRun, metrics MetricsData, fin
238250
})
239251
}
240252

241-
// Recommendations for firewall blocks
242-
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 10 {
253+
// Recommendations for firewall blocks – trigger on any block so even a single
254+
// domain denial (e.g. Codex CLI reporting one blocked domain) surfaces an action.
255+
if processedRun.FirewallAnalysis != nil && processedRun.FirewallAnalysis.BlockedRequests > 0 {
256+
blockedDomains := processedRun.FirewallAnalysis.GetBlockedDomains()
257+
var example string
258+
if len(blockedDomains) > 0 {
259+
example = fmt.Sprintf(
260+
"Add the blocked domain(s) to your workflow frontmatter:\n\n```yaml\nnetwork:\n allowed:\n - %s\n```",
261+
strings.Join(blockedDomains, "\n - "),
262+
)
263+
} else {
264+
example = "Add allowed domains to network configuration or review firewall rules"
265+
}
243266
recommendations = append(recommendations, Recommendation{
244267
Priority: "medium",
245-
Action: "Review network access configuration",
246-
Reason: "Many blocked requests suggest missing network permissions",
247-
Example: "Add allowed domains to network configuration or review firewall rules",
268+
Action: "Add blocked domains to the workflow network allow-list",
269+
Reason: "Firewall-blocked domains prevent the agent from completing its tasks",
270+
Example: example,
248271
})
249272
}
250273

pkg/cli/audit_report_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,3 +1534,107 @@ func TestExtractPreAgentStepErrors(t *testing.T) {
15341534
assert.Contains(t, files, "agent/Run agent", "Should include subdirectory step error")
15351535
})
15361536
}
1537+
1538+
func TestBuildAuditDataActionMinutes(t *testing.T) {
1539+
// Verify that action_minutes is populated from run.Duration even when
1540+
// token/turn metrics are zero (e.g. Codex runs that exit early).
1541+
// math.Ceil should be applied, and a pre-set run.ActionMinutes should take precedence.
1542+
t.Run("derived from Duration with ceil", func(t *testing.T) {
1543+
processedRun := ProcessedRun{
1544+
Run: WorkflowRun{
1545+
DatabaseID: 42,
1546+
WorkflowName: "Codex Test",
1547+
Status: "completed",
1548+
Conclusion: "failure",
1549+
Duration: 6*time.Minute + 30*time.Second, // 6.5 minutes → ceil = 7
1550+
TokenUsage: 0,
1551+
Turns: 0,
1552+
ErrorCount: 1,
1553+
WarningCount: 0,
1554+
},
1555+
}
1556+
1557+
metrics := workflow.LogMetrics{}
1558+
auditData := buildAuditData(processedRun, metrics, nil)
1559+
1560+
assert.InDelta(t, 7.0, auditData.Metrics.ActionMinutes, 0.01,
1561+
"ActionMinutes should be ceil of duration minutes (6.5m → 7)")
1562+
assert.Equal(t, 0, auditData.Metrics.TokenUsage,
1563+
"TokenUsage should be 0 when no log metrics available")
1564+
assert.Equal(t, 0, auditData.Metrics.Turns,
1565+
"Turns should be 0 when no log metrics available")
1566+
})
1567+
1568+
t.Run("uses pre-set ActionMinutes when available", func(t *testing.T) {
1569+
processedRun := ProcessedRun{
1570+
Run: WorkflowRun{
1571+
DatabaseID: 43,
1572+
WorkflowName: "Pre-set Test",
1573+
Status: "completed",
1574+
Conclusion: "success",
1575+
Duration: 6 * time.Minute,
1576+
ActionMinutes: 8.0, // pre-set by orchestrator, takes precedence
1577+
},
1578+
}
1579+
1580+
metrics := workflow.LogMetrics{}
1581+
auditData := buildAuditData(processedRun, metrics, nil)
1582+
1583+
assert.InDelta(t, 8.0, auditData.Metrics.ActionMinutes, 0.01,
1584+
"Pre-set ActionMinutes should take precedence over Duration")
1585+
})
1586+
}
1587+
1588+
func TestGenerateFindingsFirewallWithBlockedDomains(t *testing.T) {
1589+
// A single blocked domain should produce a finding naming the domain.
1590+
pr := createTestProcessedRun()
1591+
fw := &FirewallAnalysis{
1592+
TotalRequests: 1,
1593+
BlockedRequests: 1,
1594+
AllowedRequests: 0,
1595+
RequestsByDomain: map[string]DomainRequestStats{},
1596+
}
1597+
fw.SetBlockedDomains([]string{"chatgpt.com"})
1598+
pr.FirewallAnalysis = fw
1599+
1600+
findings := generateFindings(pr, MetricsData{}, nil, nil)
1601+
1602+
var networkFinding *Finding
1603+
for i := range findings {
1604+
if findings[i].Category == "network" {
1605+
networkFinding = &findings[i]
1606+
break
1607+
}
1608+
}
1609+
require.NotNil(t, networkFinding, "Should generate a network finding when firewall blocks a domain")
1610+
assert.Contains(t, networkFinding.Description, "chatgpt.com",
1611+
"Finding description should name the blocked domain")
1612+
}
1613+
1614+
func TestGenerateRecommendationsFirewallSingleBlock(t *testing.T) {
1615+
// Even a single blocked request (threshold changed from >10 to >0)
1616+
// should generate a recommendation with the domain name in the example.
1617+
pr := createTestProcessedRun()
1618+
fw := &FirewallAnalysis{
1619+
TotalRequests: 1,
1620+
BlockedRequests: 1,
1621+
AllowedRequests: 0,
1622+
RequestsByDomain: map[string]DomainRequestStats{},
1623+
}
1624+
fw.SetBlockedDomains([]string{"chatgpt.com"})
1625+
pr.FirewallAnalysis = fw
1626+
1627+
findings := []Finding{{Category: "network", Severity: "medium", Title: "Blocked Network Requests"}}
1628+
recs := generateRecommendations(pr, MetricsData{}, findings)
1629+
1630+
var networkRec *Recommendation
1631+
for i := range recs {
1632+
if strings.Contains(recs[i].Action, "network") || strings.Contains(recs[i].Action, "blocked") {
1633+
networkRec = &recs[i]
1634+
break
1635+
}
1636+
}
1637+
require.NotNil(t, networkRec, "Should generate a network recommendation for any firewall block")
1638+
assert.Contains(t, networkRec.Example, "chatgpt.com",
1639+
"Recommendation example should include the blocked domain name")
1640+
}

pkg/cli/firewall_log.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ var firewallLogLog = logger.New("cli:firewall_log")
2020
// Pre-compiled regexes for firewall log parsing (performance optimization)
2121
var (
2222
firewallLogFieldSplitter = regexp.MustCompile(`(?:[^\s"]+|"[^"]*")+`)
23+
24+
// agentLogAllowDomainsPattern matches Codex CLI firewall warning lines that suggest
25+
// adding a blocked domain to the allow-list.
26+
// Captures the full token after --allow-domains (up to whitespace) to support
27+
// hostnames, protocol prefixes, wildcard patterns, and ports.
28+
// Example: "add --allow-domains chatgpt.com to your command"
29+
// Example: "add --allow-domains chatgpt.com,other.com to your command"
30+
// Example: "add --allow-domains https://api.example.com:443 to your command"
31+
agentLogAllowDomainsPattern = regexp.MustCompile(`--allow-domains\s+([^\s]+)`)
2332
)
2433

2534
// Firewall Log Parser
@@ -127,7 +136,44 @@ func (f *FirewallAnalysis) AddMetrics(other LogAnalysis) {
127136
f.AllowedRequests += otherFirewall.AllowedRequests
128137
f.BlockedRequests += otherFirewall.BlockedRequests
129138

139+
// Merge blocked domain lists
140+
if len(otherFirewall.BlockedDomains) > 0 {
141+
domainSet := make(map[string]bool, len(f.BlockedDomains)+len(otherFirewall.BlockedDomains))
142+
for _, d := range f.BlockedDomains {
143+
domainSet[d] = true
144+
}
145+
for _, d := range otherFirewall.BlockedDomains {
146+
domainSet[d] = true
147+
}
148+
merged := make([]string, 0, len(domainSet))
149+
for d := range domainSet {
150+
merged = append(merged, d)
151+
}
152+
sort.Strings(merged)
153+
f.SetBlockedDomains(merged)
154+
}
155+
156+
// Merge allowed domain lists
157+
if len(otherFirewall.AllowedDomains) > 0 {
158+
domainSet := make(map[string]bool, len(f.AllowedDomains)+len(otherFirewall.AllowedDomains))
159+
for _, d := range f.AllowedDomains {
160+
domainSet[d] = true
161+
}
162+
for _, d := range otherFirewall.AllowedDomains {
163+
domainSet[d] = true
164+
}
165+
merged := make([]string, 0, len(domainSet))
166+
for d := range domainSet {
167+
merged = append(merged, d)
168+
}
169+
sort.Strings(merged)
170+
f.SetAllowedDomains(merged)
171+
}
172+
130173
// Merge request stats by domain
174+
if f.RequestsByDomain == nil {
175+
f.RequestsByDomain = make(map[string]DomainRequestStats)
176+
}
131177
for domain, stats := range otherFirewall.RequestsByDomain {
132178
existing := f.RequestsByDomain[domain]
133179
existing.Allowed += stats.Allowed
@@ -399,3 +445,67 @@ func analyzeMultipleFirewallLogs(logsDir string, verbose bool) (*FirewallAnalysi
399445
},
400446
)
401447
}
448+
449+
// extractFirewallFromAgentLog scans agent-stdio.log for firewall-blocked domain warnings.
450+
// This supplements dedicated proxy firewall logs (e.g., Squid access logs) by extracting
451+
// network-block information from the agent's own output when proxy logs are unavailable.
452+
//
453+
// The Codex CLI emits lines containing "--allow-domains <domain>" when a domain is blocked
454+
// by the sandbox firewall. For example:
455+
//
456+
// "[WARN] chatgpt.com is not in the allowed domains. To allow access, add --allow-domains chatgpt.com to your command."
457+
//
458+
// Returns nil when no agent-stdio.log exists or no blocked domains are found.
459+
func extractFirewallFromAgentLog(logsPath string, verbose bool) *FirewallAnalysis {
460+
agentStdioPath := filepath.Clean(filepath.Join(logsPath, "agent-stdio.log"))
461+
content, err := os.ReadFile(agentStdioPath) // #nosec G304 -- path is cleaned via filepath.Clean and logsPath is a trusted run output directory
462+
if err != nil {
463+
// File not present is normal (agent didn't run, or run used a different log path)
464+
firewallLogLog.Printf("No agent-stdio.log found at %s: %v", agentStdioPath, err)
465+
return nil
466+
}
467+
468+
blockedDomainsSet := make(map[string]bool)
469+
for line := range strings.SplitSeq(string(content), "\n") {
470+
if matches := agentLogAllowDomainsPattern.FindStringSubmatch(line); len(matches) > 1 {
471+
// Domains can be comma-separated in the suggestion
472+
for domain := range strings.SplitSeq(matches[1], ",") {
473+
if d := strings.TrimSpace(domain); d != "" {
474+
blockedDomainsSet[d] = true
475+
}
476+
}
477+
}
478+
}
479+
480+
if len(blockedDomainsSet) == 0 {
481+
firewallLogLog.Printf("No blocked domains found in agent-stdio.log at %s", agentStdioPath)
482+
return nil
483+
}
484+
485+
blockedDomains := make([]string, 0, len(blockedDomainsSet))
486+
for d := range blockedDomainsSet {
487+
blockedDomains = append(blockedDomains, d)
488+
}
489+
sort.Strings(blockedDomains)
490+
491+
analysis := &FirewallAnalysis{
492+
TotalRequests: len(blockedDomains),
493+
AllowedRequests: 0,
494+
BlockedRequests: len(blockedDomains),
495+
RequestsByDomain: make(map[string]DomainRequestStats),
496+
}
497+
analysis.SetBlockedDomains(blockedDomains)
498+
for _, d := range blockedDomains {
499+
analysis.RequestsByDomain[d] = DomainRequestStats{Blocked: 1}
500+
}
501+
502+
firewallLogLog.Printf("Extracted %d firewall-blocked domain(s) from agent-stdio.log: %s", len(blockedDomains), strings.Join(blockedDomains, ", "))
503+
if verbose {
504+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf(
505+
"Found %d firewall-blocked domain(s) in agent log: %s",
506+
len(blockedDomains), strings.Join(blockedDomains, ", "),
507+
)))
508+
}
509+
510+
return analysis
511+
}

0 commit comments

Comments
 (0)