Skip to content

fix(audit): exclude - placeholder from blocked domains and allow-list recommendations#26381

Merged
pelikhan merged 2 commits intomainfrom
copilot/fix-firewall-analysis-allow-list
Apr 15, 2026
Merged

fix(audit): exclude - placeholder from blocked domains and allow-list recommendations#26381
pelikhan merged 2 commits intomainfrom
copilot/fix-firewall-analysis-allow-list

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

When iptables drops traffic before Squid intercepts it, both domain and destIPPort fields are -. The existing fallback only substituted destIPPort when it had a real value — leaving - to propagate into BlockedDomains and produce nonsensical recommendations like network: allowed: ["-"].

Changes

  • firewall_log.go: When both domain and destIPPort are -, replace with an (unknown) sentinel. The sentinel is tracked in RequestsByDomain (preserving blocked request counts) but excluded from BlockedDomains/AllowedDomains.

  • audit_report_analysis.go: Added filterActionableDomains() as defense-in-depth — strips - and (unknown) from domain lists before building finding descriptions and allow-list recommendation examples. Guards against any code path (e.g. extractFirewallFromAgentLog) that could still surface placeholder values.

// Before: domain stays "-", ends up in BlockedDomains and recommendation YAML
domain := entry.Domain
if domain == "-" && entry.DestIPPort != "-" && entry.DestIPPort != "-:-" {
    domain = entry.DestIPPort
}

// After: unknown-destination traffic uses a sentinel, excluded from domain sets
} else if domain == "-" {
    domain = unknownDomain  // "(unknown)" — tracked in RequestsByDomain only
}

Tests

  • Updated TestParseFirewallLogIptablesDropped to expect (unknown) in RequestsByDomain and assert - is absent from BlockedDomains.
  • Added TestParseFirewallLogUnknownAllowedDomain for the allowed-traffic edge case.
  • Added TestGenerateRecommendationsFiltersDashPlaceholder and TestGenerateRecommendationsFiltersUnknownSentinel to cover the defense-in-depth filtering.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw om/santhosh-tekurev-parse ache/go/1.25.8/x--show-toplevel git rev-�� --show-toplevel ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile /usr/bin/git 1829-32114/test-git ho52/RILG8Ja3npvrev-parse e/git-upload-pac--show-toplevel git (http block)
  • https://api.github.com/orgs/test-owner/actions/secrets
    • Triggering command: /usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD wasm.s go (http block)
  • https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --get remote.origin.url /usr/bin/git -json GO111MODULE 64/pkg/tool/linu--show-toplevel /usr/bin/git remo�� -v 64/pkg/tool/linuTest User /usr/bin/git 695979929/.githugit rotocol/go-sdk@vrev-parse 64/pkg/tool/linu--show-toplevel git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha k/gh-aw/gh-aw/.github/workflows/agentic-observabv1.0.0 config /usr/bin/git remote.origin.urgit GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha 46/001/test-frontmatter-with-env-template-expressions.md qrk06zUvD ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linutest@example.com env rtcfg GO111MODULE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE fips140/sha3 GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel 64/pkg/tool/linuremote1 /usr/bin/git _.a 6D-KwQuTc stants.test git rev-�� --show-toplevel stants.test /usr/bin/git 46/001/test-compgit R30X4Bcts ache/go/1.25.8/x--show-toplevel git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linuother /usr/bin/git ithub/workflows/git s /opt/hostedtoolc--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/compile otOrdering4280334210/001/go/1.25.0/x64/bin/go /tmp/go-build397git -trimpath 4399542/b438/sty--show-toplevel git (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v8
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha --show-toplevel ache/go/1.25.8/x64/pkg/tool/linuremote /usr/bin/git rtcfg GO111MODULE ache/go/1.25.8/x/tmp/gh-aw/aw-master.patch git rev-�� --show-toplevel ache/go/1.25.8/x^remote\..*\.gh-resolved$ /usr/bin/git 1829-32114/test-git GO111MODULE ptables git (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v9
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v9 --jq .object.sha go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build186149962/b242/importcfg -pack /home/REDACTED/go/pkg/mod/golang.org/x/text@v0.36.0/internal/tag/tag.go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v9 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v9 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha add other /usr/bin/git 18/001 GO111MODULE x_amd64/compile git rev-�� --show-toplevel x_amd64/compile /usr/bin/git -json GO111MODULE 64/pkg/tool/linu--show-toplevel git (http block)
  • https://api.github.com/repos/actions/setup-node/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha remove upstream /usr/bin/git 18/001 GO111MODULE x_amd64/compile git rev-�� --show-toplevel x_amd64/compile /usr/bin/git -json GO111MODULE 64/pkg/tool/linu--show-toplevel git (http block)
  • https://api.github.com/repos/actions/upload-artifact/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v4 --jq .object.sha -aw/git/ref/tags/v3.0.0 -test.v=true /usr/bin/gh -test.timeout=10git -test.run=^Test -test.short=true--show-toplevel gh api /repos/test-owner/test-repo/actions/secrets --jq /usr/bin/git -json GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/actions/upload-artifact/git/ref/tags/v7
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v7 --jq .object.sha 1/test2.md rg/x/text@v0.36.0/internal/number/common.go ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE sysrand 149962/b015/syma--show-toplevel ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile env /ref/tags/v9 REzZ/UVSmm-gThuyfG0BeREzZ ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE t/internal/catmsrev-parse GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v7 --jq .object.sha 149962/b193/_pkg_.a TJ4J/EoB_P8I8HxwDW6KATJ4J 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env g_.a hW2J/wOV8HATkBOuFSSLThW2J 64/pkg/tool/linux_amd64/compile GOINSECURE eutil GOMODCACHE 64/pkg/tool/linux_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v7 --jq .object.sha 1829-32114/test-2498791570 _zAe/m6K4S-499xrKjIdi_zAe (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v0.1.2
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v0.1.2 --jq .object.sha add origin /usr/bin/git rity2521087949/0git GO111MODULE 64/bin/go git rev-�� --show-toplevel go 4399542/b451/vet.cfg -json GO111MODULE 64/pkg/tool/linu--show-toplevel git (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.0.0 --jq .object.sha --show-toplevel 6nfC8zN5c6kASggpb24G/6nfC8zN5c6kASggpb24G /usr/bin/git -goversion go1.25.8 -c=4 git rev-�� ons-test857177418 -pack /usr/bin/git l GO111MODULE 64/bin/go git (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.2.3
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.2.3 --jq .object.sha 4399542/b438/_pkg_.a HceIUipbAWMY9sJvruy0/HceIUipbAWMY9sJvruy0 4399542/b438=> -goversion go1.25.8 -c=4 git rev-�� o0IW/KSEM5q0eEd8ODRUBo0IW -pack /usr/bin/git 68619392/001' 68619392/001' 64/bin/go git (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/1/artifacts --jq .artifacts[].name _56Gjvce9 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a KmEF_rn9z ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE pproxy GOMODCACHE ache/go/1.25.8/xorigin (http block)
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 0/message/catalo-nolocalimports 64/pkg/tool/linu-importcfg GOINSECURE tants 64/src/internal/--show-toplevel 64/pkg/tool/linu/home/REDACTED/work/gh-aw/gh-aw/scripts/lint_error_messages_test.go env _.a GO111MODULE ache/go/1.25.8/x64/pkg/tool/linux_amd64/asm GOINSECURE chema/v6/kind GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/asm (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12345/artifacts --jq .artifacts[].name GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a m0O72i2Jk x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env 413209772 LZuHOSZyr /opt/hostedtoolcache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12346/artifacts --jq .artifacts[].name GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a h00yucQ7c /opt/hostedtoolcache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD bis 64/pkg/tool/linuremote.origin.url (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/2/artifacts --jq .artifacts[].name emplate/v3@v3.0.2/compile.go 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a LamLkoYmy ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE pguts GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 0/internal/internal.go 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD bis 64/pkg/tool/linutest@example.com env _.a J9_2Hh5RJ 64/pkg/tool/linux_amd64/compile GOINSECURE t GOMODCACHE 64/pkg/tool/linux_amd64/compile (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/3/artifacts --jq .artifacts[].name _3ywvdE5S 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env 2498791570 dq87ptaK6 64/pkg/tool/linux_amd64/compile GOINSECURE go-sdk/jsonrpc GOMODCACHE 64/pkg/tool/linuTest User (http block)
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 CY7t-lTSd 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env _.a Y_7BzNNuM x_amd64/link GOINSECURE (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/4/artifacts --jq .artifacts[].name rotocol/go-sdk@v1.5.0/jsonrpc/jsonrpc.go 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env 2498791570 go x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 0/message/catalog/catalog.go 64/pkg/tool/linux_amd64/compile GOINSECURE ic_wasm.o 64/src/internal/--show-toplevel 64/pkg/tool/linux_amd64/compile env _.a YfB4YDUdE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/5/artifacts --jq .artifacts[].name GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD bis 64/pkg/tool/linux_amd64/compile env 2498791570 GO111MODULE ache/go/1.25.8/x64/pkg/tool/linu-test.short=true GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linu-test.v=true (http block)
    • Triggering command: /usr/bin/gh gh run download 5 --dir test-logs/run-5 0/internal/format/format.go 64/pkg/tool/linux_amd64/compile GOINSECURE are_wasm.o 64/src/internal/--git-dir 64/pkg/tool/linux_amd64/compile env til.go til_test.go ache/go/1.25.8/x64/pkg/tool/linux_amd64/asm GOINSECURE g GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linu/tmp/file-tracker-test3606697617/test2.lock.yml (http block)
  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path -c=4 -nolocalimports -importcfg /tmp/go-build3974399542/b411/importcfg -pack /home/REDACTED/work/gh-aw/gh-aw/pkg/fileutil/fileutil.go /home/REDACTED/work/gh-aw/gh-aw/pkg/fileutil/tar.go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 GOMOD GOMODCACHE 64/pkg/tool/linuupstream env /workflows GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD bis 64/pkg/tool/linutest@example.com (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v0.47.4
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v0.47.4 --jq .object.sha --show-toplevel 64/pkg/tool/linux_amd64/vet /usr/bin/git rtcfg BytXhgNOP x_amd64/vet git rev-�� 210/001/go/1.25.0/x64"; export PATH="$(find "/tmp/TestGetNpmBinPathSetup_GorootOrdering428033421git x_amd64/vet /usr/bin/git sRemoteWithRealG/bin/sh sRemoteWithRealG-c .cfg git (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha _.a nLaxVxxol aw.test GOINSECURE GOMOD GOMODCACHE aw.test 9743�� rtcfg 1T9iaPhBz x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.2.3
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v2.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm env 619392/001 619392/002/work x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json o x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v3.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v3.0.0 --jq .object.sha -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
  • https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha rity2201258363/001 GO111MODULE ntdrain.test GOINSECURE GOMOD bis ntdrain.test 9743�� -json Tbt35DxwQ ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x-buildtags (http block)
  • https://api.github.com/repos/nonexistent/repo/actions/runs/12345
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/contents/file.md
    • Triggering command: /tmp/go-build3974399542/b397/cli.test /tmp/go-build3974399542/b397/cli.test -test.testlogfile=/tmp/go-build3974399542/b397/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true -nolocalimports -importcfg /tmp/go-build186149962/b211/importcfg -pack env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/test-owner/test-repo/actions/secrets
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)

If you need me to access, download, or install something from one of these locations, you can either:

…ommendations

- In firewall_log.go: replace '-' with '(unknown)' sentinel when both
  domain and destIPPort are placeholders; exclude sentinel from
  BlockedDomains/AllowedDomains sets so it never appears in allow-list
  recommendations
- In audit_report_analysis.go: add filterActionableDomains() helper that
  strips '-' and '(unknown)' before building findings descriptions and
  network allow-list recommendation examples (defense-in-depth)
- Update TestParseFirewallLogIptablesDropped to expect '(unknown)' in
  RequestsByDomain and verify '-' is absent from BlockedDomains
- Add TestParseFirewallLogUnknownAllowedDomain for the allowed-traffic edge case
- Add TestGenerateRecommendationsFiltersDashPlaceholder and
  TestGenerateRecommendationsFiltersUnknownSentinel in audit_report_test.go

Fixes #<issue>"

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/dd8a9c21-b6de-4bcd-bf2f-910a20093667

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix blocked domain recommendation in firewall analysis fix(audit): exclude - placeholder from blocked domains and allow-list recommendations Apr 15, 2026
Copilot AI requested a review from pelikhan April 15, 2026 06:33
@github-actions github-actions bot mentioned this pull request Apr 15, 2026
@github-actions github-actions bot added the lgtm label Apr 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — great work on this firewall log fix! The two-layer defense (sentinel replacement in firewall_log.go + filterActionableDomains in audit_report_analysis.go) is a solid approach to make sure - placeholders never surface in allow-list recommendations, regardless of ingestion path. The test coverage across all four new/updated test cases looks comprehensive. This PR looks ready for maintainer review! 🚀

Generated by Contribution Check · ● 1.9M ·

@pelikhan pelikhan marked this pull request as ready for review April 15, 2026 12:45
Copilot AI review requested due to automatic review settings April 15, 2026 12:45
@pelikhan pelikhan merged commit 6fd2154 into main Apr 15, 2026
57 checks passed
@pelikhan pelikhan deleted the copilot/fix-firewall-analysis-allow-list branch April 15, 2026 12:45
Copy link
Copy Markdown
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

Improves audit firewall-domain handling so placeholder values (-) don’t propagate into blocked/allowed domain lists or allow-list recommendation examples when iptables drops traffic before Squid can identify the destination.

Changes:

  • Introduced an (unknown) sentinel for truly-unknown destinations and excluded it from BlockedDomains/AllowedDomains while still tracking counts in RequestsByDomain.
  • Added defense-in-depth filtering (filterActionableDomains) so recommendations/findings never include - or (unknown) domains.
  • Expanded/updated tests to cover the iptables-drop edge cases and recommendation filtering.
Show a summary per file
File Description
pkg/cli/firewall_log.go Adds unknownDomain sentinel and prevents placeholder/sentinel from entering blocked/allowed domain sets.
pkg/cli/audit_report_analysis.go Filters non-actionable domains out of findings/recommendations to prevent nonsensical allow-list examples.
pkg/cli/firewall_log_test.go Updates expectations for iptables-dropped traffic and adds an allowed-traffic edge-case test.
pkg/cli/audit_report_test.go Adds tests ensuring recommendations filter out - and (unknown) values.

Copilot's findings

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

Excellent test quality

Metric Value
New/modified tests analyzed 4
✅ Design tests (behavioral contracts) 4 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 4 (100%)
Duplicate test clusters 0
Test inflation detected ⚠️ Yes (both test files exceed 2:1 lines-added ratio)
🚨 Coding-guideline violations 0

Test Classification Details

View all 4 test classifications
Test File Classification Notes
TestGenerateRecommendationsFiltersDashPlaceholder pkg/cli/audit_report_test.go:1626 ✅ Design Verifies - is excluded from recommendation allow-list output
TestGenerateRecommendationsFiltersUnknownSentinel pkg/cli/audit_report_test.go:1658 ✅ Design Verifies unknownDomain sentinel is excluded from recommendation allow-list output
TestParseFirewallLogIptablesDropped (modified assertions) pkg/cli/firewall_log_test.go:503 ✅ Design Verifies sentinel replacement and that - / unknownDomain are excluded from BlockedDomains
TestParseFirewallLogUnknownAllowedDomain pkg/cli/firewall_log_test.go:539 ✅ Design Verifies allowed-request sentinel exclusion from AllowedDomains using real file I/O

Test Inflation Note

Both test files have a additions ratio above 2:1 relative to their production counterparts:

Test file Lines added Production file Lines added Ratio
audit_report_test.go +60 audit_report_analysis.go +16 3.75 : 1
firewall_log_test.go +55 firewall_log.go +15 3.67 : 1

This is not a quality concern in this specific case — the production changes introduce a small sentinel constant and a one-line filter call, while the tests cover multiple behaviorally distinct edge cases (the - placeholder, the unknownDomain sentinel, and the allowed-request edge case). The verbosity is justified and the tests are high-value.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 4 tests — all unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All four tests verify observable behavioral contracts — none use mock libraries, all carry descriptive assertion messages, and all test meaningful edge cases (placeholder domain exclusion from allow-list recommendations and domain sentinel handling during log parsing).


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24455236413

🧪 Test quality analysis by Test Quality Sentinel · ● 541.6K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 4 new/modified tests are behavioral contract tests with edge-case coverage, no mock libraries, correct build tags, and descriptive assertion messages.

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Design Decision Gate — ADR Required (Post-Merge)

This PR makes significant changes to core business logic in pkg/ (146 new lines) but does not have a linked Architecture Decision Record (ADR).

The PR branch has already been merged and deleted, so the draft ADR could not be pushed automatically. Please add it to main in a follow-up commit.

AI has analyzed the PR diff and generated a draft ADR based on the design decisions in the code:

📄 Draft filename: docs/adr/26381-unknown-domain-sentinel-for-firewall-analysis.md

📋 View Draft ADR Content
# ADR-26381: Unknown-Domain Sentinel for Firewall Log Analysis

**Date**: 2026-04-15
**Status**: Draft
**Deciders**: pelikhan, Copilot

---

## Part 1 — Narrative (Human-Friendly)

### Context

The firewall proxy (Squid) logs every request with a `domain` field and a `destIPPort` field. When the kernel's iptables ruleset drops a packet before Squid can inspect it, Squid records both fields as the literal string `"-"`. The firewall log parser had a partial fallback — it substituted `destIPPort` for `domain` when `destIPPort` was a real value — but when both fields were `"-"`, the raw placeholder propagated unchanged into `BlockedDomains` and `AllowedDomains`. This caused downstream recommendation output to emit malformed YAML like `network: allowed: ["-"]`, which is meaningless and potentially confusing to users configuring firewall allow-lists.

### Decision

We will replace every entry where both `domain` and `destIPPort` are `"-"` with a named sentinel constant `unknownDomain = "(unknown)"` at parse time. The sentinel is tracked in `RequestsByDomain` so that blocked/allowed request counts remain accurate, but it is **excluded** from the `BlockedDomains` and `AllowedDomains` sets. As a defense-in-depth measure, we also introduce `filterActionableDomains()`, which strips both `"-"` and `unknownDomain` from any domain list before it is used to build finding descriptions or allow-list recommendation examples.

### Alternatives Considered

#### Alternative 1: Filter only at recommendation generation
Strip `"-"` at the point where recommendations are formatted, without changing `parseFirewallLog`. This fixes the symptom for the known code path but leaves `"-"` in `BlockedDomains`/`AllowedDomains` for any consumer. It provides only a single defense layer and does not communicate intent — every consumer must independently rediscover and guard against the `"-"` placeholder.

#### Alternative 2: Discard iptables-dropped entries entirely
Skip entries where both `domain` and `destIPPort` are `"-"` and do not count them toward `BlockedRequests`. This eliminates placeholder pollution but loses the fact that traffic was dropped — a security-relevant signal. Operators would see lower-than-real blocked counts, which could mask policy issues.

#### Alternative 3: Attempt reverse-DNS or connection-table lookup
When both fields are `"-"`, resolve a real hostname from `/proc/net/tcp` or DNS. Complex, introduces system-call dependencies inside a log parser, and not guaranteed to succeed. Not justified for a bug fix.

### Consequences

#### Positive
- Allow-list recommendation YAML no longer contains `"-"` as a domain entry.
- The `"(unknown)"` sentinel communicates intent explicitly for future readers.
- `filterActionableDomains()` provides defense-in-depth against other ingestion paths.
- `RequestsByDomain` retains a count of truly-unknown traffic, preserving observability.

#### Negative
- Two defense layers (parse-time + recommendation-time) can cause confusion about where the "real" fix lives.
- The `unknownDomain` constant couples `firewall_log.go` and `audit_report_analysis.go`.
- Defense-in-depth test setup requires manually injecting `"-"` into `BlockedDomains`, which can diverge from production behavior.

#### Neutral
- The `"(unknown)"` string will appear in structured output that serializes `RequestsByDomain`. Consumers parsing that field must handle it.

---

## 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]((www.rfceditor.org/redacted)

### Firewall Log Parsing

1. When a log entry's `domain` field equals `"-"` and a non-placeholder `destIPPort` is available, implementations **MUST** substitute `destIPPort` for `domain` before further processing.
2. When a log entry's `domain` field equals `"-"` and `destIPPort` is also `"-"` or `"-:-"`, implementations **MUST** replace `domain` with the `unknownDomain` sentinel value (`"(unknown)"`).
3. Implementations **MUST NOT** use the raw `"-"` string as a domain key in `RequestsByDomain`, `BlockedDomains`, or `AllowedDomains`.
4. The `unknownDomain` sentinel **MUST** be tracked in `RequestsByDomain` so that blocked and allowed request counts remain accurate.
5. The `unknownDomain` sentinel **MUST NOT** be added to `BlockedDomains` or `AllowedDomains`.

### Domain List Filtering

1. Implementations **MUST** filter domain lists through `filterActionableDomains()` (or equivalent logic) before using them to build finding descriptions or allow-list recommendation examples.
2. `filterActionableDomains()` **MUST** remove all entries equal to `"-"` or equal to the `unknownDomain` sentinel from the returned list.
3. Implementations **MUST NOT** include `"-"` or the `unknownDomain` sentinel as entries in user-visible allow-list YAML or recommendation output.
4. When a filtered domain list is empty (all entries were placeholders), implementations **SHOULD** still generate a firewall recommendation if `BlockedRequests > 0`, omitting only the domain-specific example text.

### Sentinel Constant

1. The sentinel constant **MUST** be defined as the single canonical string `"(unknown)"` (with parentheses) and **MUST** be referenced by name (`unknownDomain`) rather than as an inline string literal.
2. Implementations **MUST NOT** introduce additional sentinel or placeholder strings for the same concept without superseding this ADR.

### Conformance

An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24455236468) workflow.*

What to do next

  1. Copy the draft ADR above into docs/adr/26381-unknown-domain-sentinel-for-firewall-analysis.md on main
  2. Review and complete — add any context the AI couldn't infer, and change status from Draft to Accepted once agreed
  3. Reference the ADR in this PR by adding to the PR body:

    ADR: ADR-26381: Unknown-Domain Sentinel for Firewall Log Analysis

Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors will thank you when they encounter unknownDomain or filterActionableDomains() and wonder why two defense layers exist.


📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

  • Context — What is the problem? What forces are at play?
  • Decision — What did you decide? Why?
  • Alternatives Considered — What else could have been done?
  • Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 26381-unknown-domain-sentinel-for-firewall-analysis.md for PR #26381).

References: §24455236468

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · ● 184.8K ·

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[cli-tools-test] Firewall analysis shows - as blocked domain with incorrect allow-list recommendation

3 participants