fix(api): classify /api/acp/agents as loopback-only (#7948) (#7966)

* fix(api): classify /api/acp/agents as loopback-only (#7948)

/api/acp/agents is spawn-capable (POST registers a client-chosen `binary`
via src/lib/acp/registry.ts; GET / POST {action:"refresh"} runs
detectInstalledAgents() -> execFileSync(probe.command, probe.args, { shell })
transitively) but was never added to LOCAL_ONLY_API_PREFIXES in
src/server/authz/routeGuard.ts, unlike every sibling spawn-capable route
(Hard Rules #15/#17). A leaked JWT via tunnel could reach the version-probe
execFileSync path.

Adds the prefix to the loopback gate plus a direct regression test
(tests/unit/route-guard-acp-agents-local-only.test.ts) and an assertion in
tests/unit/check-route-guard-membership.test.ts documenting why the existing
source-scan subcheck cannot catch this class of gap (the spawn call is
transitive via registry.ts, not in the route file itself).

* chore(quality): register route-guard-acp-agents-local-only test in stryker tap.testFiles

changelog.d/fixes/7948-acp-agents-loopback-gate.md

@@ -0,0 +1 @@
+- fix(api): classify `/api/acp/agents` as loopback-only to close an RCE-via-tunnel gap in the ACP custom-agent registry (#7948)

src/server/authz/routeGuard.ts

@@ -52,6 +52,7 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray<string> = [
   "/api/oauth/cursor/auto-import", // spawns `execFile("which", ["cursor"])` to verify a local Cursor install before importing creds — RCE-via-tunnel surface (Hard Rules #15 + #17, found by 6A.8 route-guard gate). Specific path only: the rest of /api/oauth/ (browser redirect/callback flows) must stay remote-reachable.
   "/api/skills/collect/", // Skill Collector CLI detection: GET .../detect probes getCliRuntimeStatus() per CLI_TOOL_IDS entry, which spawns a child process to check each tool — RCE-via-tunnel surface (Hard Rules #15 + #17, PR #6294 review).
   "/api/discovery/", // Discovery tool (opt-in provider scanner): the scan route makes outbound probes to provider endpoints (SSRF-adjacent) and the whole surface is an admin research tool — strict-loopback only, no manage-scope bypass (NOT in LOCAL_ONLY_MANAGE_SCOPE_BYPASS_PREFIXES). See _tasks/features-v3.8.42/gaps/DISCOVERY_TOOL_DESIGN.md.
+  "/api/acp/agents", // ACP custom-agent registry: POST registers a client-chosen `binary`; GET / POST {action:"refresh"} runs detectInstalledAgents() -> execFileSync(probe.command, probe.args, { shell }) transitively (src/lib/acp/registry.ts) — RCE-via-tunnel surface (Hard Rules #15 + #17, #7948)
 ];
 
 /**

stryker.conf.json

@@ -39,7 +39,9 @@
   "incremental": true,
   "incrementalFile": "reports/mutation/stryker-incremental.json",
   "testRunner": "tap",
-  "plugins": ["@stryker-mutator/tap-runner"],
+  "plugins": [
+    "@stryker-mutator/tap-runner"
+  ],
   "tap": {
     "testFiles": [
       "tests/unit/account-fallback-anthropic-quota.test.ts",
@@ -103,20 +105,17 @@
       "tests/unit/circuit-breaker-failure-kind.test.ts",
       "tests/unit/circuit-breaker-registry-cap.test.ts",
       "tests/unit/circuit-breaker-stream-controller-4602.test.ts",
-      "tests/unit/cliproxyapi-dedicated-credential-7645.test.ts",
-      "tests/unit/combo-least-used-account.test.ts",
-      "tests/unit/combo/recovery-hint.test.ts",
-      "tests/unit/ddg-circuit-breaker-null-content-6999-7000.test.ts",
       "tests/unit/claude-code-parity.test.ts",
       "tests/unit/claude-effort-suffix-strip.test.ts",
       "tests/unit/claude-oauth-provider.test.ts",
       "tests/unit/claude-passthrough-stream-boolean.test.ts",
       "tests/unit/claude-passthrough-thinking-2454.test.ts",
       "tests/unit/cli-simulate.test.ts",
-      "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts",
       "tests/unit/cline-response-envelope.test.ts",
-      "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts",
       "tests/unit/clinepass-provider.test.ts",
+      "tests/unit/cliproxyapi-dedicated-credential-7645.test.ts",
+      "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts",
+      "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts",
       "tests/unit/cliproxyapi-model-mapping-dispatch.test.ts",
       "tests/unit/codex-failover.test.ts",
       "tests/unit/codex-quota-selection-hydration.test.ts",
@@ -135,6 +134,7 @@
       "tests/unit/combo-health-dashboard.test.ts",
       "tests/unit/combo-health-route.test.ts",
       "tests/unit/combo-hedging.test.ts",
+      "tests/unit/combo-least-used-account.test.ts",
       "tests/unit/combo-lockout-quota-reset-6863.test.ts",
       "tests/unit/combo-max-depth-config.test.ts",
       "tests/unit/combo-model-lockout-honors-reset-1308.test.ts",
@@ -168,6 +168,7 @@
       "tests/unit/combo/auto-status-penalty-4540.test.ts",
       "tests/unit/combo/combo-exhausted-skip.test.ts",
       "tests/unit/combo/effective-max-concurrency.test.ts",
+      "tests/unit/combo/recovery-hint.test.ts",
       "tests/unit/complexity-aware-scoring-wiring.test.ts",
       "tests/unit/context-pinning-tool-calls.test.ts",
       "tests/unit/cooldown-epoch-string-3954.test.ts",
@@ -175,6 +176,7 @@
       "tests/unit/correctness/sanitizers.property.test.ts",
       "tests/unit/custom-model-target-format.test.ts",
       "tests/unit/db-reset-module-state.test.ts",
+      "tests/unit/ddg-circuit-breaker-null-content-6999-7000.test.ts",
       "tests/unit/domain-persistence.test.ts",
       "tests/unit/edgetts-provider.test.ts",
       "tests/unit/embeddings-auth.test.ts",
@@ -239,25 +241,26 @@
       "tests/unit/responses-handler.test.ts",
       "tests/unit/rotation-config-omniroute.test.ts",
       "tests/unit/route-explainability.test.ts",
+      "tests/unit/route-guard-acp-agents-local-only.test.ts",
       "tests/unit/route-guard-forge-jcode-settings-local-only.test.ts",
       "tests/unit/route-guard-grok-build-settings-local-only.test.ts",
-      "tests/unit/route-guard-qwen-settings-local-only.test.ts",
       "tests/unit/route-guard-middleware-local-only.test.ts",
       "tests/unit/route-guard-plugins-local-only.test.ts",
       "tests/unit/route-guard-private-lan.test.ts",
       "tests/unit/route-guard-provider-login-local-only.test.ts",
+      "tests/unit/route-guard-qwen-settings-local-only.test.ts",
       "tests/unit/router-strategies.test.ts",
       "tests/unit/rule12-error-sanitization-sweep.test.ts",
       "tests/unit/serial/combo-health-autopilot.test.ts",
       "tests/unit/serial/combo-quota-share-cooldown-wait-timing.test.ts",
       "tests/unit/serial/combo-strategy-fallbacks-half-open-timing.test.ts",
       "tests/unit/serial/provider-health-autopilot.test.ts",
-      "tests/unit/service-provider-plugin-registry.test.ts",
-      "tests/unit/session-affinity-generic-7274.test.ts",
       "tests/unit/service-combo-metrics.test.ts",
+      "tests/unit/service-provider-plugin-registry.test.ts",
       "tests/unit/services-branch-hardening.test.ts",
       "tests/unit/services/bifrost-route-guard.test.ts",
       "tests/unit/services/combo-metrics-memory.test.ts",
+      "tests/unit/session-affinity-generic-7274.test.ts",
       "tests/unit/settings/authz-bypass.test.ts",
       "tests/unit/skip-provider-breaker-consumer-2743.test.ts",
       "tests/unit/sse-auth.test.ts",
@@ -385,7 +388,11 @@
     ".worktrees",
     ".stryker-tmp"
   ],
-  "reporters": ["progress", "html", "json"],
+  "reporters": [
+    "progress",
+    "html",
+    "json"
+  ],
   "htmlReporter": {
     "fileName": "reports/mutation/mutation.html"
   },

tests/unit/check-route-guard-membership.test.ts

@@ -145,6 +145,14 @@ test("6A.8 P1 RESOLVED: spawn-capable system/db-backups routes are classified lo
   assert.equal(isLocalOnlyPath("/api/db-backups/exportAll"), true);
 });
 
+test("#7948: /api/acp/agents (transitive execFileSync via registry) is classified local-only", () => {
+  // /api/acp/agents is spawn-capable only transitively (route.ts imports
+  // src/lib/acp/registry.ts, which calls execFileSync) — the source-scan
+  // subcheck above only greps the route file itself, so it cannot catch this
+  // class of gap. This assertion is the direct regression guard for #7948.
+  assert.equal(isLocalOnlyPath("/api/acp/agents"), true);
+});
+
 test("6A.8: spawn-capable routes in SPAWN_CAPABLE_ROUTE_ROOTS are still all classified local-only", async () => {
   // The original subcheck (SPAWN_CAPABLE_ROUTE_ROOTS) must still pass.
   // This test is a regression guard — the new source-scan does not break the old check.

tests/unit/route-guard-acp-agents-local-only.test.ts

@@ -0,0 +1,31 @@
+/**
+ * Security regression (#7948): /api/acp/agents must be classified as LOCAL_ONLY
+ * so loopback enforcement runs unconditionally before any auth check.
+ *
+ * POST registers a CustomAgentDef with a client-chosen `binary`
+ * (src/lib/acp/registry.ts:40-48); `resolveVersionProbe()` only checks the
+ * tokenized `versionCommand`'s command matches the supplied `binary` — no
+ * known-safe allowlist. A later GET (or POST {action:"refresh"}) runs
+ * `detectInstalledAgents()` -> `detectAgent()` -> `execFileSync(probe.command,
+ * probe.args, { shell: shouldUseShellForVersionProbe(probe.command) })`
+ * (registry.ts:320-325), with `shell:true` on Windows for `.cmd`/`.bat`/
+ * extensionless commands — the same spawn-capable class already gated for
+ * every sibling route (Hard Rules #15 + #17). Classifying it LOCAL_ONLY closes
+ * the remote-RCE vector: a leaked JWT over a Cloudflared/Ngrok tunnel cannot
+ * trigger process spawning. See docs/security/ROUTE_GUARD_TIERS.md.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import { isLocalOnlyPath } from "../../src/server/authz/routeGuard.ts";
+
+test("#7948: /api/acp/agents is LOCAL_ONLY (spawns via registry execFileSync)", () => {
+  assert.equal(isLocalOnlyPath("/api/acp/agents"), true);
+});
+
+test("#7948: /api/acp/agents with trailing slash is LOCAL_ONLY", () => {
+  assert.equal(isLocalOnlyPath("/api/acp/agents/"), true);
+});
+
+test("#7948: nested acp agent paths stay LOCAL_ONLY under the same prefix", () => {
+  assert.equal(isLocalOnlyPath("/api/acp/agents/sub"), true);
+});