Skip to content

Refactor duplicated host-service port allowance loops in setup-iptables#3443

Merged
lpcox merged 2 commits into
mainfrom
copilot/duplicate-port-validation-loop
May 20, 2026
Merged

Refactor duplicated host-service port allowance loops in setup-iptables#3443
lpcox merged 2 commits into
mainfrom
copilot/duplicate-port-validation-loop

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

setup-iptables.sh had two near-identical AWF_HOST_SERVICE_PORTS loops for host and network gateway destinations, duplicating security-sensitive port validation logic. This change consolidates that logic into a single helper to keep validation behavior consistent across both gateway paths.

  • What changed

    • Added a shared helper in containers/agent/setup-iptables.sh to:
      • trim each configured service port
      • validate port format/range (1..65535)
      • add the corresponding iptables OUTPUT allow rule for a supplied destination IP
    • Replaced both duplicated loops (host gateway and network gateway) with calls to the helper.
    • Preserved existing gateway eligibility checks (is_valid_ipv4, host/network gateway inequality) and existing warning behavior for invalid ports.
  • Why this refactor matters

    • Removes duplicated validation in firewall rule setup.
    • Ensures future port-validation changes are applied in one place, reducing drift risk in security-critical logic.
  • Example (new shared flow)

allow_service_ports_to_ip "$HSP_HOST_GW_IP" "host gateway"
allow_service_ports_to_ip "$HSP_NET_GW_IP" "network gateway"

Copilot AI changed the title [WIP] Refactor duplicate port-validation loops in setup-iptables.sh Refactor duplicated host-service port allowance loops in setup-iptables May 20, 2026
Copilot finished work on behalf of lpcox May 20, 2026 00:38
Copilot AI requested a review from lpcox May 20, 2026 00:38
@lpcox lpcox marked this pull request as ready for review May 20, 2026 01:50
Copilot AI review requested due to automatic review settings May 20, 2026 01:50
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.

Copilot wasn't able to review any files in this pull request.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 20, 2026

@copilot implement this refactoring

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 20, 2026

@copilot implement this refactoring

Implemented in c0906f5. I added a shared allow_service_ports_to_ip() helper in containers/agent/setup-iptables.sh and replaced the duplicated host/network gateway loops with helper calls while preserving gateway checks and invalid-port warning behavior.

Copilot finished work on behalf of lpcox May 20, 2026 03:43
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.53% 95.60% 📈 +0.07%
Statements 95.37% 95.44% 📈 +0.07%
Functions 96.41% 96.41% ➡️ +0.00%
Branches 87.91% 87.95% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub API: 2 PR entries verified in recent-prs.json
✅ Playwright: GitHub homepage navigated successfully (title contains "GitHub")
✅ File verify: smoke-test-claude-26139876453.txt exists

PASS — All smoke tests completed successfully

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Result
GitHub MCP connectivity ✅ PR listed: "Align log discovery with canonical Squid container constant"
GitHub.com HTTP connectivity ⚠️ Pre-step data unavailable (template vars not resolved)
File write/read ⚠️ Pre-step data unavailable (template vars not resolved)

Overall: PARTIAL — MCP confirmed working; pre-step outputs were not injected into the workflow template.

PR author: @Copilot | Assignees: @lpcox, @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list PRs)
GitHub.com connectivity ✅ (template vars unexpanded; MCP confirms connectivity)
File write/read ⚠️ pre-step vars not expanded
BYOK inference (this response)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

PR author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PASS

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test: FAIL
Merged PRs: Align log discovery with canonical Squid container constant; Refactor api-proxy startup to adapter-only validation/model discovery paths
✅ GitHub merged PR review
❌ safeinputs-gh PR query (tool missing)
✅ Playwright GitHub title
❌ Tavily search (no tools available)
✅ File write/read via bash
❌ Discussion comment (github-discussion-query missing)
✅ npm ci && npm run build
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Gemini Engine Validation

  • Last 2 Merged PRs:
    • Align log discovery with canonical Squid container constant
    • Refactor api-proxy startup to adapter-only validation/model discovery paths
  • GitHub.com Connectivity: ✅
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

Overall Status: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #3443 · ● 8.3M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ Timeout/no response
PostgreSQL pg_isready ❌ No response
PostgreSQL SELECT 1 ❌ Connection timeout

Overall: FAILhost.docker.internal (172.17.0.1) is not reachable from this environment. Service containers may not be running or the host is not accessible from the sandbox.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v22.22.3
Go go1.22.12 go1.22.12

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@lpcox lpcox merged commit 1aef56c into main May 20, 2026
64 of 68 checks passed
@lpcox lpcox deleted the copilot/duplicate-port-validation-loop branch May 20, 2026 04:35
@github-actions github-actions Bot mentioned this pull request May 20, 2026
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.

[Duplicate Code] Duplicate port-validation loop for HSP_HOST_GW_IP and HSP_NET_GW_IP in setup-iptables.sh

3 participants