diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index d9be8c8..5befdf2 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -41,7 +41,7 @@ usage as of the Portwing version in the row above: |---|---| | `app/configs/portwing.yaml` | Base preset: container lifecycle, image pull/inspect/remove, `/events`, narrow network/volume/distribution/service reads. No exec, no compose-stack network/volume creation, no build. | | `app/configs/portwing-with-exec.yaml` | `portwing.yaml` plus the exec/attach paths Portwing's interactive terminal feature needs. | -| `app/configs/portwing-with-compose.yaml` | `portwing.yaml` plus `POST /networks/create`, `DELETE /networks/*`, `POST /networks/*/disconnect`, `POST /volumes/create`, `DELETE /volumes/*` — what compose-stack deploys through Portwing need. Still denies `/build`; BuildKit fallback needs `/session` + `/grpc`, which no preset here models yet. | +| `app/configs/portwing-with-compose.yaml` | `portwing.yaml` plus `POST /networks/create`, `POST /networks/*/connect`, `DELETE /networks/*`, `POST /networks/*/disconnect`, `POST /volumes/create`, `DELETE /volumes/*` — what compose-stack deploys through Portwing need. Still denies `/build`; BuildKit fallback needs `/session` + `/grpc`, which no preset here models yet. | Portwing's `examples/sockguard.yaml` is a manually-synced copy of sockguard's `app/configs/portwing.yaml` (the no-exec, no-compose base preset) — update diff --git a/README.md b/README.md index 4b1381e..a10e1d2 100644 --- a/README.md +++ b/README.md @@ -611,13 +611,16 @@ live container state through this endpoint using their standard tool-call flow. | Tool | Description | |------|-------------| | `list_containers` | All containers — id, names, image, state, status, labels | -| `inspect_container(id)` | State, image, env-var count (values never exposed), mounts, networks, restart policy | +| `inspect_container(id)` | State, image, env-var count (values never exposed via this MCP tool), mounts, networks, restart policy | | `container_logs(id, tail)` | Last N lines of stdout/stderr (max 500) | | `host_metrics` | CPU, memory, disk, network, uptime snapshot | | `container_stats(id)` | One-shot CPU/memory/network stats for a container | **Credential hygiene:** `inspect_container` returns only the *count* of environment variables — -values are never transmitted, preventing accidental secret leakage. +values are never transmitted through this MCP tool, preventing accidental secret leakage. This +scoping applies only to `inspect_container`: the sibling `GET /api/containers` REST endpoint (and +the container inventory synced over the edge WebSocket) returns full container env var values by +design — see [Security Model](docs/security-model.md) for details. #### Add to Claude Desktop (claude_desktop_config.json) diff --git a/SPEC.md b/SPEC.md index c92893b..568f552 100644 --- a/SPEC.md +++ b/SPEC.md @@ -142,6 +142,8 @@ The Drydock `/api/portwing/ws` endpoint requires the Ed25519 fields (`pubKeyId`, | `dd:trigger_response` | Agent -> Server | Trigger result | | `dd:container_log_request` | Server -> Agent | Request container logs | | `dd:container_log_response` | Agent -> Server | Container log data | +| `dd:container_delete_request` | Server -> Agent | Request container removal | +| `dd:container_delete_response` | Agent -> Server | Removal result (`success`/`error`) | ## 4. Standard Mode HTTP API diff --git a/docs/content/docs/authentication.mdx b/docs/content/docs/authentication.mdx index 91f8e92..b22f4a7 100644 --- a/docs/content/docs/authentication.mdx +++ b/docs/content/docs/authentication.mdx @@ -87,7 +87,7 @@ docker run -d --name portwing \ ``` -`TOKEN_HASH` is the recommended choice when you need shared-secret token auth. The plaintext token exists only on the caller side; the agent stores and compares only the Argon2id hash. +`TOKEN_HASH` is the recommended *token-based* option — among the three shared-secret mechanisms above, it's the one to reach for, since the plaintext token exists only on the caller side and the agent stores and compares only the Argon2id hash. [Ed25519 per-client keys](#4-ed25519-per-client-keys) below remain the recommended mechanism overall for production, since they avoid a shared secret entirely; use `TOKEN_HASH` when a caller can't do per-request signing. --- @@ -186,7 +186,7 @@ The hello carries `pubKeyId`, `timestamp`, `nonce`, and `signature` fields. The See [Connection Modes](/connection-modes) for a full description of edge mode. -Edge mode requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely. +Edge mode requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry). diff --git a/docs/content/docs/drydock-integration.mdx b/docs/content/docs/drydock-integration.mdx index ab9593a..8e01473 100644 --- a/docs/content/docs/drydock-integration.mdx +++ b/docs/content/docs/drydock-integration.mdx @@ -91,7 +91,7 @@ Sent on every new SSE connection. Drydock reads `version`, `os`, `arch`, `cpus`, `memoryGb` is read from `/proc/meminfo` (the binary stays cgo-free) and rounded to one decimal GiB; non-Linux hosts report `0`, which Drydock accepts. `pollInterval` is the agent's `DD_POLL_INTERVAL` rendered as a Go duration string — the field is informational and displayed as-is. Portwing 0.5.x and earlier sent `memoryGb: 0` and omitted `logLevel`/`pollInterval`. -`pollInterval` actually takes three different string shapes today depending on the source — a Go duration (`"5m0s"`, Standard Mode `dd:ack`), a cron expression (Drydock's own legacy Node.js agent), and a bare integer string (`"300"`, Edge Mode's `welcome` frame — see below). Treat it as an opaque, display-only string in all three cases; do not parse it. +`pollInterval` takes several different shapes depending on the source: a Go duration string (`"5m0s"`, Standard Mode `dd:ack`), a cron expression (Drydock's own legacy Node.js agent), and a bare integer string (`"300"`) on Drydock's REST `AgentInfo` surface. Edge Mode's `welcome` frame is different again — it sends `pollInterval` as a JSON *number* (e.g. `300`), not a string (see below). Treat it as an opaque, display-only value in all cases; do not parse it. ### `dd:container-added` / `dd:container-updated` @@ -208,7 +208,7 @@ The sequence below is the production handshake, implemented on the agent side in -Requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely. +Requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry). Handshake sequence: diff --git a/docs/content/docs/security-model.mdx b/docs/content/docs/security-model.mdx index 3d8fc6a..8cee338 100644 --- a/docs/content/docs/security-model.mdx +++ b/docs/content/docs/security-model.mdx @@ -319,6 +319,14 @@ Portwing is a privileged agent: an authenticated caller has full Docker API acce --- +## Container env vars are not redacted on `/api/containers` + +`GET /api/containers` and the container inventory synced to Drydock over the edge WebSocket (`dd:container_sync` / `dd:container_added` / `_updated`) include each container's environment variables as plaintext key/value pairs (`RuntimeDetails.Env` in `internal/adapter/containers.go`). This is deliberate: Portwing does not redact or filter env values on this surface, and leaves that responsibility to Drydock (or another downstream consumer) if redaction before display or storage is required. + +This is distinct from the MCP `inspect_container` tool, which reports only an env-var *count* and never the values — see the [README](https://github.com/codeswhat/portwing#mcp--ai-assistant-integration). Any authenticated caller of `/api/containers`, and any system that receives the edge sync, sees full env var values. This is most relevant to standalone or no-Sockguard deployments, where the caller/consumer trust boundary is wider than "operator only." + +--- + ## Related pages - [Authentication](/authentication) — token, Argon2id hash, and Ed25519 key setup diff --git a/docs/drydock-integration.md b/docs/drydock-integration.md index f01dcb7..66f980e 100644 --- a/docs/drydock-integration.md +++ b/docs/drydock-integration.md @@ -89,7 +89,7 @@ The edge-mode hello is Ed25519-signed (`pubKeyId`/`timestamp`/`nonce`/`signature > **Drydock version:** the `/api/portwing/ws` controller endpoint and the `portwing/1.0` protocol string require a Drydock build that ships them. Drydock 1.5 is the first controller release with this endpoint, so edge mode needs Drydock 1.5+; older controllers do not expose it. > -> **Requires `DD_EXPERIMENTAL_PORTWING=true`:** the Drydock controller must be started with this flag set — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely. +> **Requires `DD_EXPERIMENTAL_PORTWING=true`:** the Drydock controller must be started with this flag set — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry). --- @@ -148,7 +148,7 @@ Portwing sends: Note: `memoryGb` is read from `/proc/meminfo` (no cgo) and rounded to one decimal GiB; non-Linux hosts report 0, which Drydock accepts. `pollInterval` is the agent's `DD_POLL_INTERVAL` as a Go duration string (Drydock's own agent sends a cron expression here — the field is informational, displayed as-is). Portwing 0.5.x and earlier sent `memoryGb: 0` and omitted `logLevel`/`pollInterval`. -`pollInterval` actually appears in three different string shapes depending on the source, and none of them should be parsed as anything but an opaque, display-only string: a Go duration (`"5m0s"`, from Portwing's Standard-mode `dd:ack`), a cron expression (from Drydock's own legacy Node.js agent), and a bare integer string (`"300"`, from Drydock's Edge Mode `welcome` frame — see the Edge Mode section below). +`pollInterval` appears in several shapes depending on the source, and none of them should be parsed as anything but opaque, display-only values: a Go duration string (`"5m0s"`, from Portwing's Standard-mode `dd:ack`), a cron expression (from Drydock's own legacy Node.js agent), and a bare integer string (`"300"`) on Drydock's REST `AgentInfo` surface. Drydock's Edge Mode `welcome` frame is different again — it sends `pollInterval` as a JSON *number* (e.g. `300`), not a string (see the Edge Mode section below). ### `dd:container-added` / `dd:container-updated` diff --git a/docs/security-model.md b/docs/security-model.md index 9a4ec3c..9f68d5f 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -253,3 +253,19 @@ access. The controls above prevent *unauthenticated or improperly authorized* access and certain classes of *injection* attacks against the Compose path. They do not limit what an authenticated caller can do with the Docker API. Operators should treat the Portwing token as a root-equivalent credential. + +### Container Env Vars Are Not Redacted on `/api/containers` + +`GET /api/containers` and the container inventory synced to Drydock over the +edge WebSocket (`dd:container_sync` / `dd:container_added` / `_updated`) +include each container's environment variables as plaintext key/value pairs +(`RuntimeDetails.Env` in `internal/adapter/containers.go`). This is a +deliberate design decision — Portwing does not redact or filter env values on +this surface; that responsibility belongs to Drydock (or another downstream +consumer) if redaction before display or storage is required. This is +distinct from the MCP `inspect_container` tool, which reports only an env-var +*count* and never the values (see the [README](../README.md#mcp--ai-assistant-integration)). +Any authenticated caller of `/api/containers`, and any system that receives +the edge sync, sees full env var values — this is most relevant to +standalone or no-Sockguard deployments where the caller/consumer trust +boundary is wider than "operator only." diff --git a/internal/edge/client.go b/internal/edge/client.go index 37cb209..95a7fb5 100644 --- a/internal/edge/client.go +++ b/internal/edge/client.go @@ -277,7 +277,9 @@ func (c *Client) connect(ctx context.Context) (bool, error) { } if env.Type == protocol.TypeError { var errMsg protocol.ErrorMessage - if err := json.Unmarshal(env.Data, &errMsg); err == nil { + if err := json.Unmarshal(env.Data, &errMsg); err != nil { + slog.Warn("controller rejected hello with an unparseable error payload", "error", err, "raw", string(env.Data)) + } else { slog.Warn("controller rejected hello", "code", errMsg.Code, "message", errMsg.Message) } closeWebSocket(conn, "controller rejected hello") diff --git a/internal/edge/wire_contract_test.go b/internal/edge/wire_contract_test.go index 082933b..3b2183a 100644 --- a/internal/edge/wire_contract_test.go +++ b/internal/edge/wire_contract_test.go @@ -267,7 +267,14 @@ func TestConnectFatal404(t *testing.T) { // rather than the generic "expected welcome, got ..." string, and logs both // fields. func TestConnectHelloRejectedByController(t *testing.T) { - t.Parallel() + // Deliberately NOT t.Parallel(): this test mutates the process-global + // slog default (slog.SetDefault) to capture log output, which races + // with any other test in this package that logs concurrently via the + // package-level slog.* helpers. Go only runs parallel subtests + // concurrently with each other; keeping this test sequential guarantees + // no other test observes or contends on the global logger while it's + // swapped out. Don't "fix" this by re-adding t.Parallel() and widening + // a sleep — that masks the race instead of removing it. srv := newControllerServer(t, func(ctrl *websocket.Conn) { readAndAckHello(t, ctrl) diff --git a/internal/protocol/messages.go b/internal/protocol/messages.go index dd713cb..1e5df4a 100644 --- a/internal/protocol/messages.go +++ b/internal/protocol/messages.go @@ -1,6 +1,10 @@ package protocol -import "encoding/json" +import ( + "encoding/json" + "fmt" + "strconv" +) // Core message types. const ( @@ -72,6 +76,45 @@ type WelcomeMessage struct { Config map[string]string `json:"config,omitempty"` } +// UnmarshalJSON tolerates pollInterval arriving as either a JSON number (the +// shape Drydock's Edge Mode welcome frame actually sends) or a numeric +// string (a shape seen elsewhere in the ecosystem, e.g. Drydock's REST +// AgentInfo surface). Without this, a numeric-string welcome frame would +// fail to unmarshal entirely, silently dropping the serverCompatLevel +// mismatch warning along with the poll interval. +func (w *WelcomeMessage) UnmarshalJSON(data []byte) error { + type alias WelcomeMessage + aux := struct { + PollInterval json.RawMessage `json:"pollInterval"` + *alias + }{ + alias: (*alias)(w), + } + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + if len(aux.PollInterval) == 0 { + return nil + } + + var n int + if err := json.Unmarshal(aux.PollInterval, &n); err == nil { + w.PollInterval = n + return nil + } + + var s string + if err := json.Unmarshal(aux.PollInterval, &s); err != nil { + return fmt.Errorf("pollInterval: neither a number nor a string: %w", err) + } + n, err := strconv.Atoi(s) + if err != nil { + return fmt.Errorf("pollInterval: %q is not a valid integer: %w", s, err) + } + w.PollInterval = n + return nil +} + type RequestMessage struct { RequestID string `json:"requestId"` Method string `json:"method"` diff --git a/internal/protocol/welcome_test.go b/internal/protocol/welcome_test.go new file mode 100644 index 0000000..723d0b8 --- /dev/null +++ b/internal/protocol/welcome_test.go @@ -0,0 +1,119 @@ +package protocol + +import ( + "encoding/json" + "testing" +) + +// TestWelcomeMessagePollIntervalShapes verifies WelcomeMessage.PollInterval +// decodes correctly whether the wire sends it as a JSON number (the actual +// shape of Drydock's Edge Mode welcome frame) or as a numeric string (a +// shape documented elsewhere in the ecosystem, e.g. Drydock's REST +// AgentInfo surface). Before UnmarshalJSON was added, a numeric-string +// pollInterval failed the whole WelcomeMessage decode. +func TestWelcomeMessagePollIntervalShapes(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + json string + want int + }{ + { + name: "numeric pollInterval (real welcome frame shape)", + json: `{"pollInterval":300}`, + want: 300, + }, + { + name: "numeric-string pollInterval (compat shape)", + json: `{"pollInterval":"300"}`, + want: 300, + }, + { + name: "numeric pollInterval with config", + json: `{"pollInterval":60,"config":{"serverCompatLevel":"1.5.0"}}`, + want: 60, + }, + { + name: "numeric-string pollInterval with config", + json: `{"pollInterval":"60","config":{"serverCompatLevel":"1.5.0"}}`, + want: 60, + }, + { + name: "missing pollInterval", + json: `{"config":{"serverCompatLevel":"1.5.0"}}`, + want: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var w WelcomeMessage + if err := json.Unmarshal([]byte(tt.json), &w); err != nil { + t.Fatalf("Unmarshal(%q) returned error: %v", tt.json, err) + } + if w.PollInterval != tt.want { + t.Errorf("PollInterval = %d, want %d", w.PollInterval, tt.want) + } + }) + } +} + +// TestWelcomeMessagePollIntervalInvalid verifies malformed pollInterval +// shapes fail decoding cleanly (an error, not a panic or silent zero). +func TestWelcomeMessagePollIntervalInvalid(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + json string + }{ + {name: "non-numeric string", json: `{"pollInterval":"not-a-number"}`}, + {name: "boolean", json: `{"pollInterval":true}`}, + {name: "object", json: `{"pollInterval":{}}`}, + {name: "array", json: `{"pollInterval":[]}`}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var w WelcomeMessage + if err := json.Unmarshal([]byte(tt.json), &w); err == nil { + t.Errorf("Unmarshal(%q) = nil error, want an error", tt.json) + } + }) + } +} + +// TestWelcomeMessageRoundTrip verifies a WelcomeMessage encoded by this +// package (always a JSON number) decodes back to the same value, so the +// custom UnmarshalJSON doesn't break the common/agent-facing path. +func TestWelcomeMessageRoundTrip(t *testing.T) { + t.Parallel() + + want := WelcomeMessage{ + PollInterval: 300, + Config: map[string]string{"serverCompatLevel": "1.5.0"}, + } + + data, err := json.Marshal(want) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + + var got WelcomeMessage + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if got.PollInterval != want.PollInterval { + t.Errorf("PollInterval = %d, want %d", got.PollInterval, want.PollInterval) + } + if got.Config["serverCompatLevel"] != want.Config["serverCompatLevel"] { + t.Errorf("Config[serverCompatLevel] = %q, want %q", got.Config["serverCompatLevel"], want.Config["serverCompatLevel"]) + } +}