# Patch Analysis — CVE-2026-34594 (Coolify Destination Network command injection)

## Fix under analysis

- **PR:** #9228 — `fix: add validation and escaping for Docker network names`
- **Merge commit (beta.471):** `9e96a20a49f32e46d7a31e66e1aa6338b029f284`
- **Branch commit:** `3d1b9f53a0aec74468be75675bcaaaed0fd41d46`
- **Released in:** `4.0.0-beta.471` (release tag commit `914d7e0b50505bc1fd56c34974fca09ad354e92a`)
- **Vulnerable baseline:** `4.0.0-beta.470` (commit `575b0766d12bad2a78febff72ab59c017772bcf7`)

## What the fix changes

The fix applies a three-layer defense to the Docker **network** field of a
Destination, but **only on the `Destination\New\Docker` and `Destination\Show`
Livewire components**:

1. **Regex validation on the Livewire `$network` property**
   - `app/Livewire/Destination/New/Docker.php`:
     `#[Validate(['required','string','max:255','regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])]`
   - `app/Livewire/Destination/Show.php`: same rule on `$network`.
   - Rejects any network containing shell metacharacters
     (`; | $ \` & > < space ' " # / \ newline …`) at the Livewire validation
     layer with `The network field format is invalid.`

2. **Model mutator (defense in depth)**
   - `app/Models/StandaloneDocker.php` and `app/Models/SwarmDocker.php` gain
     `setNetworkAttribute(string $value)` which throws
     `InvalidArgumentException` unless the value matches
     `ValidationPatterns::DOCKER_NETWORK_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'`.
   - On beta.471 both models also declare an explicit
     `$fillable = ['server_id','name','network']`, so mass-assignment via
     `::create(['network' => …])` routes through `setAttribute()` and the
     mutator fires.

3. **`escapeshellarg()` on every shell consumer of the network field**
   - `StandaloneDocker::created` boot event (the primary sink): the network is
     now `escapeshellarg()`-ed before being interpolated into
     `docker network inspect/create`.
   - `Destination\Show::delete()`, `ApplicationDeploymentJob` (6 sites),
     `DatabaseBackupJob`, `StartService`, `Init` command, and
     `bootstrap/helpers/proxy.php` (`connectProxyToNetworks`,
     `ensureProxyNetworksExist`) all wrap the network with `escapeshellarg()`.
   - Added unit tests: `tests/Unit/DockerNetworkInjectionTest.php` and
     `tests/Unit/ValidationPatternsTest.php`.

## Root cause being addressed

The `network` attribute of a Docker Destination is attacker-controlled input
that is interpolated **unsanitized** into shell commands executed on the
destination server over SSH
(`instant_remote_process()` → `SshMultiplexingHelper::generateSshCommand()` →
`ssh root@<host> 'bash -se' << delim`). The network becomes the body of a
here-doc fed to `bash -se`, so metacharacters (`;`, `#`, `$()`, backticks) are
interpreted by the remote bash.

## Trust boundary / threat model (per `SECURITY.md`)

Coolify's `SECURITY.md` states 4.x is actively supported and invites private
reports to `security@coollabs.io`. There is **no** statement excluding
authenticated command injection from scope. The Destination Network Management
surface is an authenticated web UI (`/server/{uuid}/destinations`) where an
authenticated user with destination-management permissions crosses from
"browser input" to "command executed as root on a managed server over SSH" — a
clear trust-boundary crossing that the maintainers explicitly fixed. The
variant search below stays within this in-scope authenticated→root boundary.

## Assumptions the fix makes

1. **The only user-controlled entry point for the network field is the
   `Destination\New\Docker` and `Destination\Show` Livewire components**, where
   `$network` is a `#[Validate]`-annotated public property. The fix assumes any
   network value must pass through one of these two validated properties.
2. **The model mutator is a sufficient backstop** for any other code path that
   constructs a `StandaloneDocker`/`SwarmDocker` with a network value, because
   `::create()` / mass assignment invokes `setNetworkAttribute`.
3. **`escapeshellarg()` is correctly applied** to every shell consumer, so even
   a value that slipped past validation cannot break out of the here-doc token.

## Code paths / inputs the fix does NOT cover (gaps)

### Gap 1 (primary): `App\Livewire\Server\Destinations::add($name)` — NO regex validation added

`app/Livewire/Server/Destinations.php` is a **different Livewire component**
(`server.destinations`, route `/server/{uuid}/destinations`) from
`Destination\New\Docker`. Its `add($name)` method takes the network name as a
**method argument** (not a `#[Validate]`-annotated property):

```php
public function add($name)
{
    if ($this->server->isSwarm()) {
        ... SwarmDocker::create([ 'name' => $this->server->name.'-'.$name,
                                  'network' => $this->name, // pre-existing bug: should be $name
                                  'server_id' => $this->server->id ]);
    } else {
        $this->authorize('create', StandaloneDocker::class);
        $found = $this->server->standaloneDockers()->where('network', $name)->first();
        if ($found) { $this->dispatch('error', 'Network already added to this server.'); return; }
        StandaloneDocker::create([ 'name' => $this->server->name.'-'.$name,
                                   'network' => $name,            // <-- attacker-controlled arg
                                   'server_id' => $this->server->id ]);
    }
    $this->createNetworkAndAttachToProxy();
}
```

PR #9228 did **not** touch this file. The intended UI flow is the "Scan for
Destinations" → "Add `<discovered network>`" button
(`wire:click="add('{{ data_get($network, 'Name') }}')"`), where `$name` is a
real Docker network name returned by `docker network ls`. **But Livewire method
arguments are not constrained to rendered values** — an authenticated user can
POST `/livewire/update` directly and call `add` with an arbitrary string,
including shell metacharacters.

- **On beta.470 (vulnerable):** `StandaloneDocker::create(['network' =>
  "x;id > /tmp/marker #"])` succeeds (no validation, no mutator, no escaping),
  the `StandaloneDocker::created` boot event fires `instant_remote_process(["docker
  network inspect x;id > /tmp/marker # ..."], $server)`, and `id` executes **as
  root** on the managed server over the SSH here-doc. **Same sink, same root
  cause, different entry point** — a confirmed alternate-trigger variant.
- **On beta.471 (fixed):** `StandaloneDocker::create([...])` invokes
  `setNetworkAttribute($name)`, which throws `InvalidArgumentException` for an
  invalid name. The exception propagates out of `add()` (no try/catch) to
  Livewire; the `created` event never runs and no command executes. **Blocked
  by the model mutator (defense in depth)** — but the fix did **not** add a
  clean validation message to this component, so the attacker gets an unhandled
  500/exception instead of a friendly validation error.

➡️ This is an **alternate-trigger variant, not a bypass**: it reproduces on the
vulnerable build and is blocked on the fixed build. However, it exposes an
**incomplete surface** in the fix — the regex-validation layer was only applied
to two of the three components that create destinations.

### Gap 2: `Destination\Show::syncData(true)` server-IP field

`Destination\Show::submit()` also sets
`$this->destination->server->ip = $this->serverIp`, where `$serverIp` is only
`#[Validate(['string','required'])]` (no IP/hostname regex). The server IP is
the SSH target host. Inspection of `SshMultiplexingHelper::escapedUserAtHost()`
shows the IP/user are wrapped in `escapeshellarg()` for the SSH here-doc sink,
so the IP does **not** reach the here-doc body unsanitized via the main
`instant_remote_process` path. This is therefore **not** a clean variant of the
network-field bug for the primary sink. (One unrelated unescaped usage exists in
`CleanupStaleMultiplexedConnections` for an `ssh -O check` ControlPath string,
out of this ticket's scope.)

### Gap 3: pre-existing bug in `add()` swarm branch

The swarm branch of `add()` uses `'network' => $this->name` (undefined property
→ null) instead of `$name`. On beta.471 the mutator's `string` type hint turns
this null into a `TypeError`; on beta.470 it produces an empty network. This is
a separate correctness bug, not an injection variant, and is out of scope for
the network-field fix — but a complete fix should correct it to `$name` and
validate it.

## Behavior before vs after the fix (network field)

| Path | beta.470 | beta.471 |
|------|----------|----------|
| `destination.new.docker.submit()` (validated property) | injection → root cmd exec | regex validation rejects (`The network field format is invalid.`) |
| `destination.show.submit()` (update) | injection → root cmd exec | regex validation + mutator reject |
| `destination.show.delete()` | injection via `docker network rm` | `escapeshellarg()` blocks |
| `server.destinations.add($name)` (method arg) | **injection → root cmd exec** (variant) | mutator throws `InvalidArgumentException` (blocked, but no clean validation msg) |
| `ApplicationDeploymentJob` / `DatabaseBackupJob` / `StartService` / `Init` / `proxy.php` | injection via stored network | `escapeshellarg()` + mutator on create blocks stored bad networks |

## Is the fix complete?

**Mostly, but the surface coverage is incomplete.** The fix's *primary*
mitigation (regex validation on the Livewire `$network` property) was applied
to only `Destination\New\Docker` and `Destination\Show`. A third component,
`Server\Destinations::add($name)`, accepts the network as a method argument
with **no** regex validation; it is saved from being a bypass **only** by the
model mutator (defense in depth). The mutator also produces an unhandled
`InvalidArgumentException` (500) rather than a user-facing validation error.

**Recommendation for a complete fix (for the Coding agent):**
1. Add `ValidationPatterns::dockerNetworkRules()` validation to
   `Server\Destinations::add($name)` (validate `$name` before `::create`), with
   a friendly error dispatch, mirroring `Destination\New\Docker`.
2. Fix the swarm branch of `add()` to use `$name` instead of `$this->name`.
3. Keep the model mutator and `escapeshellarg()` as defense in depth (already
   done).
4. Consider validating the network argument type/charset at the Livewire method
   boundary generally, since method-argument inputs bypass `#[Validate]`
   property rules.