Skip to content

Fix firewall SSL-bump field extraction in frontmatter parser#13920

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-ssl-bump-field-extraction
Open

Fix firewall SSL-bump field extraction in frontmatter parser#13920
Copilot wants to merge 3 commits intomainfrom
copilot/fix-ssl-bump-field-extraction

Conversation

Copy link
Contributor

Copilot AI commented Feb 5, 2026

Two firewall configuration fields (ssl-bump, allow-urls) were defined in the schema and struct but not extracted from workflow frontmatter YAML, preventing users from enabling HTTPS content inspection.

Changes

Modified pkg/workflow/frontmatter_extraction_security.go:

  • Added extraction for ssl-bump (boolean) - enables HTTPS content inspection
  • Added extraction for allow-urls (string array) - URL patterns to allow when ssl-bump enabled

Added tests:

  • 8 unit tests in frontmatter_extraction_security_test.go covering field extraction, defaults, and error handling
  • Integration test in firewall_args_integration_test.go verifying compiled AWF commands include the flags

Result

Users can now configure SSL bump for HTTPS inspection:

network:
  firewall:
    ssl-bump: true
    allow-urls:
      - "https://github.com/githubnext/*"
      - "https://api.github.com/repos/*"

Generates:

awf --ssl-bump --allow-urls 'https://github.com/githubnext/*,https://api.github.com/repos/*' ...
Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Fix firewall SSL-bump field extraction in frontmatter parser</issue_title>
<issue_description>### Description

The firewall configuration fields ssl-bump, allow-urls, and cleanup-script are defined in the schema and have corresponding struct fields, but are not extracted from workflow frontmatter YAML. This prevents users from configuring HTTPS inspection for the network firewall.

Problem

Current State:

  • Schema defines these fields (pkg/parser/schemas/main_workflow_schema.json)
  • FirewallConfig struct has fields (pkg/workflow/firewall.go:12-22)
  • Extraction code is missing these fields (pkg/workflow/frontmatter_extraction_security.go:98-132)
  • Downstream code tries to use them (pkg/workflow/firewall.go:195-218)

Impact:

  • Users cannot enable SSL bump for HTTPS content inspection
  • URL filtering through allow-urls is non-functional
  • Fields remain at zero values (false, nil, empty string)

Suggested Changes

Add extraction logic to extractFirewallConfig() in pkg/workflow/frontmatter_extraction_security.go after line 127:

// Extract ssl-bump if present  
if sslBump, hasSslBump := firewallObj["ssl-bump"]; hasSslBump {
    if sslBumpBool, ok := sslBump.(bool); ok {
        config.SSLBump = sslBumpBool
    }
}

// Extract allow-urls if present
if allowUrls, hasAllowUrls := firewallObj["allow-urls"]; hasAllowUrls {
    if urlsSlice, ok := allowUrls.([]any); ok {
        for _, url := range urlsSlice {
            if urlStr, ok := url.(string); ok {
                config.AllowURLs = append(config.AllowURLs, urlStr)
            }
        }
    }
}

// Extract cleanup-script if present (deprecated but still in struct)
if cleanupScript, hasCleanup := firewallObj["cleanup-script"]; hasCleanup {
    if scriptStr, ok := cleanupScript.(string); ok {
        config.CleanupScript = scriptStr
    }
}

Files Affected

  • pkg/workflow/frontmatter_extraction_security.go (lines 98-132) - Add extraction logic

Success Criteria

  • ssl-bump boolean field extracted from YAML
  • allow-urls string array field extracted from YAML
  • cleanup-script string field extracted from YAML (maintain backwards compatibility)
  • Test workflow with firewall SSL bump compiles successfully
  • Generated AWF command includes SSL bump arguments when configured
  • All existing firewall tests pass
  • Add test case for firewall field extraction

Source

Extracted from Schema Consistency Analysis discussion github/gh-aw#13862

Critical Issue #1: Firewall Configuration Fields Not Extracted from YAML

Priority

High - Blocks users from using documented firewall features for HTTPS inspection

AI generated by Discussion Task Miner - Code Quality Improvement Agent

  • expires on Feb 6, 2026, 1:25 PM UTC

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix firewall SSL-bump field extraction in frontmatter parser Fix firewall SSL-bump field extraction in frontmatter parser Feb 5, 2026
Copilot AI requested a review from pelikhan February 5, 2026 13:56
@pelikhan
Copy link
Contributor

pelikhan commented Feb 5, 2026

@copilot remove cleanup-script support

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

@copilot remove cleanup-script support

Removed cleanup-script extraction logic and all related tests in commit 6d99e4e.

@Mossaka
Copy link
Collaborator

Mossaka commented Feb 5, 2026

lgtm

@Mossaka Mossaka marked this pull request as ready for review February 5, 2026 17:02
Copilot AI review requested due to automatic review settings February 5, 2026 17:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a bug where firewall configuration fields ssl-bump and allow-urls were defined in the schema and struct but not extracted from workflow frontmatter YAML, preventing users from enabling HTTPS content inspection for network firewalls.

Changes:

  • Added extraction logic for ssl-bump (boolean) and allow-urls (string array) fields in the frontmatter parser
  • Added 8 comprehensive unit tests covering field extraction, defaults, and error handling
  • Added integration test verifying the compiled AWF commands include the SSL bump flags

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/workflow/frontmatter_extraction_security.go Added extraction logic for ssl-bump and allow-urls fields, following the same pattern as existing fields (args, version, log-level)
pkg/workflow/frontmatter_extraction_security_test.go Added comprehensive unit tests covering various scenarios including field extraction, defaults, type safety, and error handling
pkg/workflow/firewall_args_integration_test.go Added end-to-end integration test verifying that workflows with ssl-bump configuration compile correctly and include the expected flags in the generated AWF command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

🔍 PR Triage Results

Category: bug | Risk: medium | Priority: 73/100

Scores Breakdown

  • Impact: 40/50 - Fixes critical configuration fields (ssl-bump, allow-urls) not being extracted from YAML, blocking users from using documented firewall HTTPS inspection features. High impact.
  • Urgency: 23/30 - Fixes issue [Code Quality] Fix firewall SSL-bump field extraction in frontmatter parser #13908. Blocks documented feature usage. Age: 4.8 hours. High priority issue from Schema Consistency Analysis discussion.
  • Quality: 10/20 - Excellent description with code examples. 8 unit tests + integration test. Not draft but CI pending. Fixes known issue.

📋 Recommended Action: fast_track

Critical bug fix for documented feature. Well-tested, fixes known issue. Fast-track once CI passes despite medium risk (security-related changes).


Triaged by PR Triage Agent on 2026-02-05T18:24:58Z

AI generated by PR Triage Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Fix firewall SSL-bump field extraction in frontmatter parser

3 participants