Skip to content

feat(cli): auto-add api-target values to allowlist#1290

Merged
lpcox merged 8 commits intomainfrom
claude/add-allowlist-for-firewall
Mar 13, 2026
Merged

feat(cli): auto-add api-target values to allowlist#1290
lpcox merged 8 commits intomainfrom
claude/add-allowlist-for-firewall

Conversation

@Claude
Copy link
Contributor

@Claude Claude AI commented Mar 13, 2026

Automatically adds API target values (--copilot-api-target, --openai-api-target, --anthropic-api-target) to the firewall allowlist when specified, eliminating the need for users to manually include them in --allow-domains.

Changes

  • Auto-add API targets to allowlist (src/cli.ts): When api-target flags or environment variables are set, they are automatically added to the allowlist before domain validation
  • Integration tests (tests/integration/api-target-allowlist.test.ts): Comprehensive test coverage for CLI flags, environment variables, duplicate prevention, and multiple targets
  • Test infrastructure updates (tests/fixtures/awf-runner.ts): Added support for api-target options in test runner

Motivation

In GitHub Agentic Workflows, users can specify engine.api-target (e.g., api.acme.ghe.com for GHEC deployments). Previously, users had to manually add these values to --allow-domains, which was error-prone and confusing. This change makes the configuration seamless by automatically including api-targets in the allowlist.

Testing

  • ✅ All 255 unit tests pass
  • ✅ Build succeeds
  • ✅ Smoke tested with CLI flags and environment variables
  • ✅ Added 7 new integration tests covering various scenarios

Claude AI and others added 3 commits March 13, 2026 03:42
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
@lpcox
Copy link
Collaborator

lpcox commented Mar 13, 2026

@claude[agent] fix the PR title and summary

@Claude
Copy link
Contributor Author

Claude AI commented Mar 13, 2026

@claude[agent] fix the PR title and summary

Updated PR title to "feat(cli): auto-add api-target values to allowlist" and added a comprehensive description including summary, changes, motivation, and testing details.

@Claude Claude AI changed the title Completing task feat(cli): auto-add api-target values to allowlist Mar 13, 2026
@Claude Claude AI requested a review from lpcox March 13, 2026 04:37
@lpcox lpcox marked this pull request as ready for review March 13, 2026 04:54
@lpcox lpcox requested a review from Mossaka as a code owner March 13, 2026 04:54
Copilot AI review requested due to automatic review settings March 13, 2026 04:54
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 85.64% 85.17% 📉 -0.47%
Statements 85.57% 85.10% 📉 -0.47%
Functions 85.77% 85.77% ➡️ +0.00%
Branches 78.48% 77.64% 📉 -0.84%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 55.7% → 53.9% (-1.79%) 56.1% → 54.4% (-1.78%)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)

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

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

Adds CLI behavior to automatically allowlist API target hostnames (Copilot/OpenAI/Anthropic) when users specify them via flags or environment variables, and introduces integration coverage to prevent regressions in GitHub Agentic Workflows setups.

Changes:

  • Auto-adds --copilot-api-target, --openai-api-target, --anthropic-api-target (or corresponding env vars) into the computed allowedDomains list before validation.
  • Extends the test runner to pass the new api-target flags through to the CLI.
  • Adds integration tests intended to verify allowlisting, env-var handling, and duplicate prevention.

Reviewed changes

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

File Description
src/cli.ts Auto-injects api-target hostnames into allowedDomains and logs when doing so.
tests/fixtures/awf-runner.ts Adds support for passing api-target options into CLI invocations in tests.
tests/integration/api-target-allowlist.test.ts New integration tests for the auto-allowlist behavior (currently brittle due to non-resolvable hostnames).
Comments suppressed due to low confidence (3)

tests/integration/api-target-allowlist.test.ts:33

  • These tests use hostnames like api.acme.ghe.com that are unlikely to resolve/reliably accept TCP 443 in CI. Since toAllowDomain only passes when Squid logs a successful TCP_TUNNEL/200, this can make the test fail due to DNS/connectivity rather than allowlist behavior. Use a stable resolvable host (e.g., example.com) for the curl target and api-target value.
  test('should automatically add copilot-api-target to allowlist', async () => {
    const result = await runner.runWithSudo(
      'curl -s https://api.acme.ghe.com',
      {
        allowDomains: ['github.com'], // Note: NOT including api.acme.ghe.com
        copilotApiTarget: 'api.acme.ghe.com',
        enableApiProxy: true,
        buildLocal: true,

tests/integration/api-target-allowlist.test.ts:55

  • This test uses a .internal hostname (custom.openai-router.internal) which typically won’t resolve in CI, so Squid may never log a TCP_TUNNEL/200 even if the domain is allowlisted. Prefer a stable public domain (e.g., example.com) so the test outcome reflects the allowlist behavior.
  test('should automatically add openai-api-target to allowlist', async () => {
    const result = await runner.runWithSudo(
      'curl -s https://custom.openai-router.internal',
      {
        allowDomains: ['github.com'], // Note: NOT including custom.openai-router.internal
        openaiApiTarget: 'custom.openai-router.internal',
        enableApiProxy: true,
        buildLocal: true,
        logLevel: 'debug',
        timeout: 60000,

tests/integration/api-target-allowlist.test.ts:75

  • Same issue here: custom.anthropic-router.internal is unlikely to resolve/reliably accept connections in CI, which makes this allowlist integration test brittle. Use a resolvable host for both the curl URL and anthropicApiTarget so Squid can record a TCP_TUNNEL/200 when allowlisted.
  test('should automatically add anthropic-api-target to allowlist', async () => {
    const result = await runner.runWithSudo(
      'curl -s https://custom.anthropic-router.internal',
      {
        allowDomains: ['github.com'], // Note: NOT including custom.anthropic-router.internal
        anthropicApiTarget: 'custom.anthropic-router.internal',
        enableApiProxy: true,
        buildLocal: true,
        logLevel: 'debug',
        timeout: 60000,

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

You can also share your feedback on Copilot code review. Take the survey.

@github-actions

This comment has been minimized.

@lpcox lpcox enabled auto-merge (squash) March 13, 2026 13:59
lpcox and others added 3 commits March 13, 2026 08:01
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 85.64% 85.17% 📉 -0.47%
Statements 85.57% 85.10% 📉 -0.47%
Functions 85.77% 85.77% ➡️ +0.00%
Branches 78.48% 77.64% 📉 -0.84%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 55.7% → 53.9% (-1.79%) 56.1% → 54.4% (-1.78%)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)

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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 85.64% 85.17% 📉 -0.47%
Statements 85.57% 85.10% 📉 -0.47%
Functions 85.77% 85.77% ➡️ +0.00%
Branches 78.48% 77.64% 📉 -0.84%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 55.7% → 53.9% (-1.79%) 56.1% → 54.4% (-1.78%)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)

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

@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 85.64% 84.96% 📉 -0.68%
Statements 85.57% 84.87% 📉 -0.70%
Functions 85.77% 85.40% 📉 -0.37%
Branches 78.48% 77.30% 📉 -1.18%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 55.7% → 53.3% (-2.39%) 56.1% → 53.7% (-2.48%)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)

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

@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 85.64% 84.86% 📉 -0.78%
Statements 85.57% 84.76% 📉 -0.81%
Functions 85.77% 85.40% 📉 -0.37%
Branches 78.48% 77.19% 📉 -1.29%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 55.7% → 53.0% (-2.69%) 56.1% → 53.4% (-2.77%)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)

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

@github-actions
Copy link
Contributor

Overall: FAIL
feat: add DLP scanning to block credential exfiltration in URLs
feat(docker): switch seccomp profile to deny-by-default
GitHub MCP merged PR review: ✅
safeinputs-gh PR list: ✅
Playwright github.com title: ✅
Tavily web search: ❌ (tool unavailable)
Build + file + bash + discussion: ✅

🔮 The oracle has spoken through Smoke Codex for issue #1290

@github-actions
Copy link
Contributor

Smoke Test Results

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1290

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.1 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Result: ❌ Not all versions match — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1290

@github-actions
Copy link
Contributor

Smoke Test Results

  • GitHub MCP: Last 2 merged PRs: #1288: feat: add DLP scanning... | #1286: feat(docker): switch seccomp profile... (by @Mossaka)
  • Playwright: github.com title contains "GitHub"
  • File Write: /tmp/gh-aw/agent/smoke-test-copilot-23056869172.txt created
  • Bash: File verified via cat

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1290

@github-actions
Copy link
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 passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit 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 #1290 ·

…1294)

* Initial plan

* fix: extract api-target logic into testable helper, add unit tests

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 85.64% 85.88% 📈 +0.24%
Statements 85.57% 85.81% 📈 +0.24%
Functions 85.77% 85.95% 📈 +0.18%
Branches 78.48% 78.92% 📈 +0.44%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 87.4% → 87.9% (+0.50%) 86.8% → 87.2% (+0.48%)
src/cli.ts 55.7% → 57.7% (+2.03%) 56.1% → 58.2% (+2.06%)

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

@github-actions
Copy link
Contributor

Smoke Test Results

Test Result
GitHub MCP: #1294 fix: extract api-target logic into testable helper, add unit tests
GitHub MCP: #1270 fix(docker): simplify to localhost+Squid-only iptables
Playwright: github.com title contains "GitHub"
File write + bash verify

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1290

@github-actions
Copy link
Contributor

Smoke Test Results — PASS

GitHub MCP: Last 2 merged PRs: #1294 "fix: extract api-target logic into testable helper, add unit tests" (@Copilot), #1288 "feat: add DLP scanning to block credential exfiltration in URLs" (@Mossaka)
Playwright: GitHub page title contains "GitHub"
File Write: /tmp/gh-aw/agent/smoke-test-copilot-23060732601.txt created and verified
Bash: cat confirmed file contents

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1290

@github-actions
Copy link
Contributor

  1. GitHub MCP merged PRs: ✅ fix: extract api-target logic into testable helper, add unit tests | fix(docker): simplify to localhost+Squid-only iptables
  2. safeinputs-gh pr list: ✅
  3. Playwright title contains GitHub: ✅
  4. Tavily web search: ❌
  5. File writing: ✅
  6. Bash cat readback: ✅
  7. Discussion comment: ✅
  8. Build AWF (npm ci && npm run build): ✅
  9. Overall status: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1290

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot environments. Go versions match.

Tested by Smoke Chroot for issue #1290

@lpcox lpcox merged commit 8cd7451 into main Mar 13, 2026
58 checks passed
@lpcox lpcox deleted the claude/add-allowlist-for-firewall branch March 13, 2026 17:13
@github-actions
Copy link
Contributor

Smoke Test Results — PASS

💥 [THE END] — Illustrated by Smoke Claude

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.

4 participants