Skip to content

fix(security): run the connection test's local CLI probe only for local callers (GHSA-jmq6-8j86-8xqj) - #14995

Merged
diegosouzapw merged 4 commits into
release/v3.8.51from
fix/provider-test-local-runtime-probe
Sep 28, 2026
Merged

diegosouzapw merged 4 commits into
release/v3.8.51from
fix/provider-test-local-runtime-probe

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

⚠️ base-red inherited: #14963

Fixes GHSA-jmq6-8j86-8xqj (reported by @zer0d4y5).

Problem

For cline / qoder connections, testSingleConnection() calls getCliRuntimeStatus(), which spawns sh -c 'command -v -- "$1"' on the host. That is the LOCAL_ONLY capability that Hard Rules #15/#17 keep away from non-local callers. Three remotely reachable routes run it:

  • POST /api/providers/{id}/test (the one in the report);
  • POST /api/providers/test-batch;
  • the background test after POST /api/providers.

None of them is in LOCAL_ONLY_* or SPAWN_CAPABLE_*. On an instance with requireLogin=false, an anonymous remote caller could trigger the spawn and learn whether the CLI is installed. As the reporter says, this is not command injection: the tool id comes from a fixed map and is passed as an argv element.

Fix: gate the probe, not the route

Adding only /test to the local-only lists would leave test-batch and provider creation open. It would also remove the "Test connection" button from tunnel-served dashboards for every provider. Instead:

  • getRequestPeerLocality(request) in src/shared/utils/apiAuth.ts returns loopback | lan | remote. It uses the same trusted signals, in the same order, as isLoopbackRequest(), which now delegates to it with identical verdicts. It fails closed to remote, and client-supplied locality headers are not trusted.
  • The three routes pass allowLocalRuntimeProbe = locality !== "remote", which matches the LOCAL_ONLY semantics (loopback or private LAN, never through a reverse proxy).
  • Remote callers get the upstream connection test without the local CLI diagnosis. The credential-health scheduler, which runs inside the server, keeps the probe (the default is true).

The reporter's other suggestion, a build-time check that derives spawn reachability and fails when a route reaches a spawn sink without being gated, would have caught this class earlier. It is worth doing as its own change; it is not part of this fix.

Validation

  • tests/unit/provider-test-local-runtime-probe.test.ts, 5 cases:
    • a remote caller never calls the probe (the test injects a spy);
    • local and scheduler callers do;
    • the locality verdicts, including a client-supplied locality header being ignored when a stamping server is in front;
    • a source guard on the three call sites.
    • All 5 fail on the old code.
  • Existing suites stay green: provider-401-ambiguous-runtime, verified-connection-activation-11446, docker-bootstrap-loopback-14296, v1-models-keyless-loopback-13354 (20 tests).

…al callers (GHSA-jmq6-8j86-8xqj)

testSingleConnection() calls getCliRuntimeStatus() for cline / qoder
connections, which spawns sh -c 'command -v -- "$1"' on the host — the
LOCAL_ONLY capability of Hard Rules #15/#17. It is reached from three routes
that stay remotely reachable (a tunnel-served dashboard tests connections):
POST /api/providers/{id}/test, POST /api/providers/test-batch and the
background test after POST /api/providers. None is in LOCAL_ONLY or
SPAWN_CAPABLE, so on requireLogin=false an anonymous remote caller could
trigger the spawn and learn whether the CLI is installed.

Gate the probe, not the route: getRequestPeerLocality() (apiAuth, same trusted
signals and order as isLoopbackRequest, which now delegates to it) returns
loopback | lan | remote, and the three routes pass
allowLocalRuntimeProbe = locality !== "remote" — the LOCAL_ONLY semantics.
Remote callers get the upstream test without the local runtime diagnosis; the
credential-health scheduler keeps the probe (default true).

Tests (5) fail on the old code; the ambiguous-runtime, activation, docker
bootstrap loopback and keyless /v1/models suites stay green.

Reported-by: zer0d4y5
…s guard

The guard splits the route source on the getProviderRuntimeStatus() call; the
call now passes the caller-locality options (GHSA-jmq6-8j86-8xqj). The
early-return it pins is unchanged.

Refs GHSA-jmq6-8j86-8xqj
…er-test-local-runtime-probe

# Conflicts:
#	stryker.conf.json
@diegosouzapw
diegosouzapw merged commit 8e3b486 into release/v3.8.51 Sep 28, 2026
16 of 21 checks passed
diegosouzapw pushed a commit that referenced this pull request Sep 28, 2026
…d connections (#14941)

Maintainer rework: real merge of release/v3.8.51, keeping the #14995 local-caller restriction on the CLI runtime probe alongside the operator-disable marker. Trimmed test/route.ts to fit its frozen file-size ceiling with no behavior change. The 3 new tests fail on the base route and pass with this change. Focused suites pass 44/44 (15/15 re-run on the latest tip). typecheck:core, open-sse typecheck, eslint and file-size are clean; the only red, response-sanitizer.test.ts over the file-size cap, is inherited from the tip. Thank you @shipsfromrio!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant