Skip to content

feat: max-permission-denied guard to halt runs on repeated auth failures#4206

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/max-permission-denied
Open

feat: max-permission-denied guard to halt runs on repeated auth failures#4206
Copilot wants to merge 3 commits into
mainfrom
copilot/max-permission-denied

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 2, 2026

Permission misconfigurations (bad/missing API keys) waste tokens by letting agents spin through repeated 401/403 errors. This adds a configurable circuit-breaker that stops the run once a threshold of upstream permission denials is hit.

Changes

New guard: guards/max-permission-denied-guard.js

  • Tracks 401/403 upstream response count against AWF_MAX_PERMISSION_DENIED
  • Same pattern as max-runs-guard: config cache, state object, block/reflect/reset API

Upstream integration

  • upstream-response.js — calls applyPermissionDenied() whenever logUpstreamAuthError fires (i.e., on any 401 or 403 from upstream)
  • proxy-request.js — checks guard state before forwarding; blocks with HTTP 403 + structured JSON when limit is exceeded; exports getPermissionDeniedReflectState / resetPermissionDeniedGuardForTests
  • websocket-proxy.js — same check for WebSocket upgrade requests
  • management.js + server.js — exposes permission_denied state under /reflect

Host-side config wiring

  • src/types/rate-limit-options.ts — added maxPermissionDenied?: number to RateLimitOptions
  • src/config-file.ts — added field to AwfFileConfig.apiProxy; mapped in mapAwfFileConfigToCliOptions
  • src/commands/validators/log-and-limits.ts — reads and validates the field (must be a positive integer)
  • src/commands/validators/config-assembly.ts + src/commands/build-config.ts — passes it through to WrapperConfig
  • src/services/api-proxy-service.ts — sets AWF_MAX_PERMISSION_DENIED env var on the api-proxy container when configured via config file

Schema and documentation

  • docs/awf-config.schema.json + src/awf-config-schema.jsonmaxPermissionDenied field with full description
  • docs/api-proxy-sidecar.md — added "Max-permission-denied limit" section covering configuration, enforcement, introspection, and error detection

Error response

{
  "error": {
    "type": "permission_denied_limit_exceeded",
    "message": "Permission denied limit exceeded (3 / 3). The run has been stopped due to repeated permission errors — check that all API keys and tokens are correctly configured.",
    "denied_count": 3,
    "max_permission_denied": 3
  }
}

Config

{ "apiProxy": { "maxPermissionDenied": 3 } }

Configured via the AWF config file (stdin config). When unset, the guard is disabled (no-op).

Copilot AI linked an issue Jun 2, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix permission misconfiguration to stop token waste feat: max-permission-denied guard to halt runs on repeated auth failures Jun 2, 2026
Copilot finished work on behalf of lpcox June 2, 2026 14:49
Copilot AI requested a review from lpcox June 2, 2026 14:49
@lpcox lpcox marked this pull request as ready for review June 2, 2026 15:07
Copilot AI review requested due to automatic review settings June 2, 2026 15:07
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.35% 96.47% 📈 +0.12%
Statements 96.21% 96.32% 📈 +0.11%
Functions 98.27% 98.27% ➡️ +0.00%
Branches 90.42% 90.57% 📈 +0.15%
📁 Per-file Coverage Changes (3 files)
File Lines (Before → After) Statements (Before → After)
src/commands/validators/log-and-limits.ts 91.5% → 92.3% (+0.82%) 91.5% → 92.3% (+0.82%)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)
src/container-lifecycle.ts 93.9% → 95.7% (+1.84%) 94.2% → 95.9% (+1.75%)

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

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

Adds a new “max permission denied” circuit-breaker guard to the API proxy to prevent runs from repeatedly burning tokens/time when upstream credentials are missing or misconfigured (repeated 401/403 responses).

Changes:

  • Introduces max-permission-denied-guard that tracks upstream 401/403 counts via AWF_MAX_PERMISSION_DENIED, with block/reflect/reset helpers.
  • Integrates guard enforcement into HTTP requests (proxy-request.js) and WebSocket upgrades (websocket-proxy.js), returning a structured JSON error when blocked.
  • Exposes guard state under /reflect and adds unit/integration tests covering the new behavior.
Show a summary per file
File Description
containers/api-proxy/guards/max-permission-denied-guard.js Implements the new permission-denied counter + block/reflect helpers.
containers/api-proxy/guards/max-permission-denied-guard.test.js Unit tests for config parsing, counting, reflect state, and error payload.
containers/api-proxy/upstream-response.js Increments the permission-denied counter on upstream 401/403 logging path.
containers/api-proxy/proxy-request.js Blocks requests before forwarding when the permission-denied limit is exceeded; exports reflect/reset helpers.
containers/api-proxy/websocket-proxy.js Applies the same guard pre-check for WebSocket upgrade requests.
containers/api-proxy/management.js Adds permission_denied state into the /reflect response payload.
containers/api-proxy/server.js Wires permission-denied reflect state into management handler deps.
containers/api-proxy/server.token-guards.test.js Integration-style tests verifying HTTP blocking on repeated 401/403 and no-op when unset.

Copilot's findings

Tip

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

  • Files reviewed: 8/8 changed files
  • Comments generated: 1

Comment on lines +15 to +17
function getPermDeniedConfig() {
const rawMax = process.env.AWF_MAX_PERMISSION_DENIED;
if (permDeniedConfigCache.rawMax === rawMax) {
@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 github-actions Bot mentioned this pull request Jun 2, 2026
@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 Jun 2, 2026

@copilot address review feedback and make sure that any new configuration params are thoroughly described in teh specs and schema. non-security sensitive config params should be passed through stdin config and security sensitive through environment variables

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 2, 2026

@copilot address review feedback and make sure that any new configuration params are thoroughly described in teh specs and schema. non-security sensitive config params should be passed through stdin ...

Done in commit feat: wire maxPermissionDenied through stdin config, schema, and docs.

maxPermissionDenied is now a proper stdin config param (non-security-sensitive, as requested) wired through the full TypeScript host-side stack:

  • src/types/rate-limit-options.ts — added maxPermissionDenied?: number to RateLimitOptions (part of WrapperConfig)
  • src/config-file.ts — added field to AwfFileConfig.apiProxy interface and mapped it in mapAwfFileConfigToCliOptions
  • src/commands/validators/log-and-limits.ts — reads and validates it (must be a positive integer)
  • src/commands/validators/config-assembly.ts + src/commands/build-config.ts — passes it through to WrapperConfig
  • src/services/api-proxy-service.ts — sets AWF_MAX_PERMISSION_DENIED env var in the api-proxy container when configured

Schema and docs:

  • docs/awf-config.schema.json + src/awf-config-schema.json — added maxPermissionDenied with full description
  • docs/api-proxy-sidecar.md — added a "Max-permission-denied limit" section (matching the style of the existing "Max-runs limit" section) covering configuration, enforcement, introspection, and error detection

Tests updated across config-file-mapping, api-proxy-service-rate-limit, build-config, config-assembly, and validate-options.

Copilot finished work on behalf of lpcox June 2, 2026 16:51
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Documentation Preview

Documentation build failed for this PR. View logs.

Built from commit a3a72fa

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test: Claude Engine ✅

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list PRs) ✅ PR #4207 "refactor(api-proxy): extract duplicate collectLogOutput() into shared test helper"
GitHub.com connectivity ⚠️ Pre-step template vars not expanded (${{ }} not resolved)
File write/read ⚠️ Pre-step template vars not expanded
BYOK inference (this response)

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

Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PARTIAL — BYOK inference ✅, pre-step data unavailable (template vars not resolved in workflow)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Merged PRs reviewed:\n- refactor(api-proxy): extract duplicate collectLogOutput() into shared test helper\n- [Test Coverage] container-lifecycle.ts branch coverage\n- GitHub access: ✅\n- Playwright title check: ✅\n- Temp file write/read: ✅\n- Discussion comment: ✅\n- Build: ✅\n- Overall status: PASS

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

github-actions Bot commented Jun 2, 2026

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP connectivity ⚠️ (pre-step data unavailable)
File write/read ⚠️ (pre-step data unavailable)

PR: feat: max-permission-denied guard to halt runs on repeated auth failures
Author: @Copilot | Assignees: @lpcox, @Copilot

⚠️ Workflow template variables were not expanded — pre-step data unavailable. MCP test passed. Overall: PARTIAL

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
S1: Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled + internal helpers
S2: Test Suite 33/33 tests passed in otel.test.js (span creation, token attrs, OTLP export, file fallback, graceful degradation)
S3: Env Var Forwarding api-proxy-service.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
S4: Token Tracker Integration onUsage callback present in token-tracker-http.js (line 237) as OTEL hook point
S5: OTEL Diagnostics FileSpanExporter provides local JSONL fallback when OTLP endpoint not configured; no errors during no-config run

All 5 scenarios passed. OTEL tracing integration is working correctly.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Chroot Smoke Test Results

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

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

🏗️ 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 #4206 · sonnet46 1.2M ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test Results

Check Result
Redis PING ❌ timeout / no response
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ connection failed

Overall: FAIL

host.docker.internal is unreachable from this runner environment — service containers appear to be unavailable or not started.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Smoke Test: Gemini Engine Validation

  • Review last 2 merged PRs: ❌ (mcpscripts not found)
  • GitHub Connectivity: ❌ (TLS error 35)
  • File Writing: ✅
  • Bash Tool: ✅

Overall status: FAIL

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

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.

max-permission-denied

3 participants