feat: rename --security-mode to --legacy-security, deprecate --enable-api-proxy#6209
Conversation
…-api-proxy Replace the --security-mode strict|compat enum with a simple --legacy-security boolean flag. Strict security is the default and requires no flag. Legacy mode must be explicitly opted into via --legacy-security. Key changes: - Add --legacy-security boolean flag (new primary interface) - Keep --security-mode hidden for backward compatibility (emits deprecation warning) - --enable-api-proxy: emit deprecation warning (API proxy is always enabled) - --no-enable-api-proxy: hard error (API proxy cannot be disabled) - API proxy is always forced on in both strict and legacy modes - All warning messages updated to reference --legacy-security The API proxy is now unconditionally enabled for credential isolation — no real auth tokens (GITHUB_TOKEN, OPENAI_API_KEY, ANTHROPIC_API_KEY, COPILOT_GITHUB_TOKEN) are ever exposed to the agent container. Closes #6207 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.86% | 📉 -0.07% |
| Statements | 98.88% | 98.82% | 📉 -0.06% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.04% | 📉 -0.11% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/build-config.ts |
95.7% → 84.8% (-10.81%) | 95.7% → 84.8% (-10.81%) |
src/commands/validators/infrastructure-validator.ts |
100.0% → 96.6% (-3.39%) | 100.0% → 96.6% (-3.39%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
1 similar comment
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.86% | 📉 -0.07% |
| Statements | 98.88% | 98.82% | 📉 -0.06% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.04% | 📉 -0.11% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/build-config.ts |
95.7% → 84.8% (-10.81%) | 95.7% → 84.8% (-10.81%) |
src/commands/validators/infrastructure-validator.ts |
100.0% → 96.6% (-3.39%) | 100.0% → 96.6% (-3.39%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
Replaces security modes with --legacy-security and makes API-proxy credential isolation mandatory.
Changes:
- Adds the legacy-security flag and deprecated CLI compatibility paths.
- Forces API proxy activation across security configurations.
- Updates configuration types, mapping, validation tests, and integration fixtures.
Show a summary per file
| File | Description |
|---|---|
tests/fixtures/awf-runner.ts |
Uses the new legacy flag. |
src/types/security-options.ts |
Replaces the security-mode type. |
src/config-mapper.ts |
Maps the new config property. |
src/config-file.ts |
Updates the config interface. |
src/commands/validators/security-mode.ts |
Enforces security defaults and deprecations. |
src/commands/validators/security-mode.test.ts |
Tests updated enforcement behavior. |
src/commands/validators/config-assembly.test-utils.ts |
Updates test defaults and mocks. |
src/commands/validators/config-assembly-api-proxy.test.ts |
Reflects mandatory API proxy behavior. |
src/commands/validate-options.test.ts |
Updates validation fixtures. |
src/commands/build-config.ts |
Resolves new and deprecated options. |
src/cli-options.ts |
Registers the new and compatibility flags. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 5
- Review effort level: Medium
| }; | ||
| security?: { | ||
| securityMode?: 'strict' | 'compat'; | ||
| legacySecurity?: boolean; |
| function handleApiProxyDeprecation(config: WrapperConfig): void { | ||
| if (config.enableApiProxy === false) { |
| if (config.enableApiProxy === false) { | ||
| throw new Error( | ||
| '--no-enable-api-proxy is not allowed. The API proxy is always enabled for credential isolation.\n' + | ||
| 'Remove the --no-enable-api-proxy flag from your command.', | ||
| ); |
| .option( | ||
| '--enable-api-proxy', | ||
| 'Enable API proxy sidecar for secure credential injection.\n' + | ||
| ' Supports OpenAI (Codex) and Anthropic (Claude) APIs.' | ||
| '[DEPRECATED] The API proxy is always enabled. This flag is ignored.' |
| // Handle deprecated --security-mode flag | ||
| const securityMode = options.securityMode as string | undefined; | ||
| if (securityMode === 'compat') { |
Tests the deprecated --security-mode compat/strict mapping and the new --legacy-security boolean flag to restore coverage above baseline. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.93% | ➡️ +0.00% |
| Statements | 98.88% | 98.89% | 📈 +0.01% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.10% | 📉 -0.05% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/validators/infrastructure-validator.ts |
100.0% → 96.6% (-3.39%) | 100.0% → 96.6% (-3.39%) |
src/commands/build-config.ts |
95.7% → 97.0% (+1.31%) | 95.7% → 97.0% (+1.31%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
- Update awf-config-schema.json: add legacySecurity field, keep securityMode as deprecated for backward compat - Stop mapping apiProxy.enabled from config file to avoid false CLI deprecation warnings (API proxy is unconditionally enabled) - Replace throw with logger.error + process.exit(1) for proper error reporting through the CLI validation path - Hide --enable-api-proxy and --no-enable-api-proxy from --help output - Fix resolveLegacySecurity priority: --legacy-security now wins over deprecated --security-mode when both are specified - Add config-mapper tests for securityMode → legacySecurity mapping - Move API Proxy help section header to first visible option Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.93% | ➡️ +0.00% |
| Statements | 98.88% | 98.89% | 📈 +0.01% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.08% | 📉 -0.07% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/validators/infrastructure-validator.ts |
100.0% → 96.6% (-3.39%) | 100.0% → 96.6% (-3.39%) |
src/commands/build-config.ts |
95.7% → 97.1% (+1.49%) | 95.7% → 97.1% (+1.49%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
…tion path Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Claude passed |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 Tests failed; no label added |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
❌ Contribution Check failed. Please review the logs for details. |
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.96% | 📈 +0.03% |
| Statements | 98.88% | 98.92% | 📈 +0.04% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.11% | 📉 -0.04% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/build-config.ts |
95.7% → 97.1% (+1.49%) | 95.7% → 97.1% (+1.49%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results
Overall: FAIL — pre-computed step outputs were not injected into prompt. PR author: @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot BYOK (Direct) Mode
Status: PASS (core BYOK path working) Running in direct BYOK mode via api-proxy → api.githubcopilot.com Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot PAT Auth
Overall: PARTIAL — MCP reachable; pre-computed outputs not expanded. Auth mode: PAT (COPILOT_GITHUB_TOKEN) | Author: @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Claude Engine ValidationOverall result: ✅ PASS
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
Smoke test results:\n- feat: add --security-mode strict|compat with strict as default ✅\n- feat: rename --security-mode to --legacy-security, deprecate --enable-api-proxy ✅\n- GitHub title check ✅\n- Discussion lookup/comment ✅\n- File write/read ✅\n- Build ✅\nOverall status: PASS Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
🔬 Smoke Test: API Proxy OpenTelemetry Tracing
Overall: All pre-step validations passed. OTEL tracing integration is functional. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Services Connectivity
Overall: FAIL — service containers are not reachable from this runner environment ( Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Chroot Version Comparison
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
Smoke Test: Copilot BYOK (Direct) Mode — Azure OpenAI (Foundry, api-key)
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS CC: @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔥 Smoke Test: Docker Sbx — PASS
PR: feat: rename --security-mode to --legacy-security, deprecate --enable-api-proxy Overall: PASS
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
|
test from agent Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.93% | 98.96% | 📈 +0.03% |
| Statements | 98.88% | 98.92% | 📈 +0.04% |
| Functions | 99.34% | 99.34% | ➡️ +0.00% |
| Branches | 95.15% | 95.14% | 📉 -0.01% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/commands/build-config.ts |
95.7% → 97.1% (+1.49%) | 95.7% → 97.1% (+1.49%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Cover the AWF_DEBUG_TOKENS env var branch in build-config to push overall branch coverage above the base threshold. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dcc99fc-3d0e-40c5-8b75-bc43d9bf5dee
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Summary
Replaces the
--security-mode strict|compatenum with a simple--legacy-securityboolean flag and makes the API proxy unconditionally enabled.Changes
New flag:
--legacy-securityDeprecated:
--security-mode--help)--security-mode compat→ maps to--legacy-security, emits deprecation warning--security-mode strict→ emits deprecation warning (strict is the default)API proxy always enabled
--enable-api-proxy: emits deprecation warning, continues normally--no-enable-api-proxy: hard error — API proxy cannot be disabledConfig file schema
security.securityMode→security.legacySecurity(boolean)Motivation
Closes #6207
Related
--legacy-securityin lock files)--security-mode strict|compatimplementation