From fe47bb28eadb9815e714a9c0a2be64770646c8db Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 18:59:32 -0400 Subject: [PATCH 01/30] fix: proxy and expose the MCP Apps sandbox port --- internal/resources/common.go | 11 ++ internal/resources/configmap.go | 56 ++++++++- internal/resources/mcp_apps_test.go | 170 +++++++++++++++++++++++++++ internal/resources/networkpolicy.go | 6 + internal/resources/resources_test.go | 44 +++---- internal/resources/service.go | 10 ++ internal/resources/statefulset.go | 16 ++- 7 files changed, 287 insertions(+), 26 deletions(-) create mode 100644 internal/resources/mcp_apps_test.go diff --git a/internal/resources/common.go b/internal/resources/common.go index 119f36d6..d8778b14 100644 --- a/internal/resources/common.go +++ b/internal/resources/common.go @@ -52,6 +52,17 @@ const ( // canvas traffic. The Service targets this port instead of CanvasPort. CanvasProxyPort = 18794 + // McpAppsSandboxPort is the loopback port the operator assigns to the + // OpenClaw MCP Apps sandbox listener (mcp.apps.sandboxPort). OpenClaw's own + // default is "gateway port plus one" (18790), which is GatewayProxyPort and + // therefore already taken by the gateway proxy sidecar (#615). + McpAppsSandboxPort = 18795 + + // McpAppsSandboxProxyPort is the port the nginx reverse proxy listens on + // for MCP Apps sandbox traffic. The Service targets this port instead of + // McpAppsSandboxPort because the sandbox listener binds to loopback only. + McpAppsSandboxProxyPort = 18796 + // DefaultGatewayProxyImageRepository is the default gateway proxy image repository. DefaultGatewayProxyImageRepository = "docker.io/library/nginx" // DefaultGatewayProxyImageTag is the default gateway proxy image tag. diff --git a/internal/resources/configmap.go b/internal/resources/configmap.go index 15d21de0..eb12890d 100644 --- a/internal/resources/configmap.go +++ b/internal/resources/configmap.go @@ -55,7 +55,7 @@ func BuildConfigMapFromBytes(instance *openclawv1alpha1.OpenClawInstance, baseCo configBytes = []byte("{}") } - // Enrichment pipeline: gateway mode -> OTel metrics -> gateway auth -> tailscale -> browser -> gateway bind -> trusted proxies -> control UI origins -> skill packs + // Enrichment pipeline: gateway mode -> OTel metrics -> gateway auth -> tailscale -> browser -> gateway bind -> MCP Apps sandbox port -> trusted proxies -> control UI origins -> skill packs if enriched, err := enrichConfigWithGatewayMode(configBytes); err == nil { configBytes = enriched } @@ -83,6 +83,9 @@ func BuildConfigMapFromBytes(instance *openclawv1alpha1.OpenClawInstance, baseCo if enriched, err := enrichConfigWithGatewayBind(configBytes, instance); err == nil { configBytes = enriched } + if enriched, err := enrichConfigWithMcpAppsSandboxPort(configBytes, instance); err == nil { + configBytes = enriched + } if enriched, err := enrichConfigWithTrustedProxies(configBytes); err == nil { configBytes = enriched } @@ -506,6 +509,51 @@ func enrichConfigWithGatewayBind(configJSON []byte, instance *openclawv1alpha1.O return json.Marshal(config) } +// enrichConfigWithMcpAppsSandboxPort pins mcp.apps.sandboxPort to +// McpAppsSandboxPort when the user enabled MCP Apps without choosing a port. +// +// OpenClaw starts the sandbox listener on "gateway port plus one" by default. +// With the gateway proxy sidecar enabled that port (18790) is GatewayProxyPort, +// which nginx already holds on 0.0.0.0, so the listener fails with +// "Address already in use" (#615). Pinning the port also gives the proxy +// sidecar and the Service a fixed loopback target to forward to. +// +// The key is only written when mcp.apps.enabled is true so the config of +// OpenClaw versions that predate MCP Apps is never touched. If the user has +// already set mcp.apps.sandboxPort, or the gateway proxy is disabled (nothing +// occupies gateway+1), the config is returned unchanged (user override wins). +func enrichConfigWithMcpAppsSandboxPort(configJSON []byte, instance *openclawv1alpha1.OpenClawInstance) ([]byte, error) { + if !IsGatewayProxyEnabled(instance) { + return configJSON, nil + } + + var config map[string]interface{} + if err := json.Unmarshal(configJSON, &config); err != nil { + return configJSON, nil // not a JSON object, return unchanged + } + + mcp, _ := config["mcp"].(map[string]interface{}) + if mcp == nil { + return configJSON, nil + } + apps, _ := mcp["apps"].(map[string]interface{}) + if apps == nil { + return configJSON, nil + } + if enabled, _ := apps["enabled"].(bool); !enabled { + return configJSON, nil + } + if _, ok := apps["sandboxPort"]; ok { + return configJSON, nil + } + + apps["sandboxPort"] = McpAppsSandboxPort + mcp["apps"] = apps + config["mcp"] = mcp + + return json.Marshal(config) +} + // HasGatewayBindConflict returns true when the gateway proxy is disabled but // the user has manually set gateway.bind to loopback in their config JSON. // This combination makes the pod unreachable because nothing is listening on @@ -724,6 +772,10 @@ stream { listen 0.0.0.0:%d; proxy_pass 127.0.0.1:%d; } + server { + listen 0.0.0.0:%d; + proxy_pass 127.0.0.1:%d; + } } -`, GatewayProxyPort, GatewayPort, CanvasProxyPort, CanvasPort) +`, GatewayProxyPort, GatewayPort, CanvasProxyPort, CanvasPort, McpAppsSandboxProxyPort, McpAppsSandboxPort) } diff --git a/internal/resources/mcp_apps_test.go b/internal/resources/mcp_apps_test.go new file mode 100644 index 00000000..efacc436 --- /dev/null +++ b/internal/resources/mcp_apps_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2026 Paperclip Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "k8s.io/apimachinery/pkg/runtime" + + openclawv1alpha1 "github.com/paperclipinc/openclaw-operator/api/v1alpha1" +) + +// These tests cover #615: the MCP Apps sandbox listener must not collide with +// the gateway proxy port and must be reachable through the proxy sidecar, the +// Service and the NetworkPolicy. + +func TestMcpAppsSandboxPorts_DoNotCollide(t *testing.T) { + seen := map[int]string{} + for name, port := range map[string]int{ + "GatewayPort": GatewayPort, + "GatewayProxyPort": GatewayProxyPort, + "CanvasPort": CanvasPort, + "CanvasProxyPort": CanvasProxyPort, + "McpAppsSandboxPort": McpAppsSandboxPort, + "McpAppsSandboxProxyPort": McpAppsSandboxProxyPort, + } { + if other, ok := seen[port]; ok { + t.Errorf("%s and %s both use port %d", name, other, port) + } + seen[port] = name + } + // OpenClaw's default sandbox port is "gateway port plus one". That is the + // gateway proxy port, so the operator must never leave the default in place. + if McpAppsSandboxPort == GatewayPort+1 { + t.Errorf("McpAppsSandboxPort must not be gateway+1 (%d): nginx listens there", GatewayPort+1) + } +} + +func mcpAppsFromConfig(t *testing.T, instance *openclawv1alpha1.OpenClawInstance) map[string]interface{} { + t.Helper() + cm := BuildConfigMap(instance, "", nil) + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(cm.Data["openclaw.json"]), &parsed); err != nil { + t.Fatalf("failed to parse config JSON: %v", err) + } + mcp, _ := parsed["mcp"].(map[string]interface{}) + if mcp == nil { + return nil + } + apps, _ := mcp["apps"].(map[string]interface{}) + return apps +} + +func TestBuildConfigMap_McpAppsSandboxPort(t *testing.T) { + for _, tt := range []struct { + name string + raw string + proxyDisabled bool + wantApps bool + wantPort interface{} // nil means the key must be absent + }{ + {name: "no mcp section leaves config untouched", raw: `{}`}, + {name: "mcp without apps leaves config untouched", raw: `{"mcp":{"servers":{}}}`}, + {name: "apps disabled is not modified", raw: `{"mcp":{"apps":{"enabled":false}}}`, wantApps: true}, + {name: "apps enabled gets the operator port", raw: `{"mcp":{"apps":{"enabled":true}}}`, wantApps: true, wantPort: float64(McpAppsSandboxPort)}, + {name: "user port wins", raw: `{"mcp":{"apps":{"enabled":true,"sandboxPort":19001}}}`, wantApps: true, wantPort: float64(19001)}, + {name: "proxy disabled keeps the OpenClaw default", raw: `{"mcp":{"apps":{"enabled":true}}}`, proxyDisabled: true, wantApps: true}, + } { + t.Run(tt.name, func(t *testing.T) { + instance := newTestInstance("mcp-apps") + instance.Spec.Config.Raw = &openclawv1alpha1.RawConfig{ + RawExtension: runtime.RawExtension{Raw: []byte(tt.raw)}, + } + if tt.proxyDisabled { + instance.Spec.Gateway.Enabled = Ptr(false) + } + + apps := mcpAppsFromConfig(t, instance) + if (apps != nil) != tt.wantApps { + t.Fatalf("mcp.apps present = %v, want %v", apps != nil, tt.wantApps) + } + if apps == nil { + return + } + got, ok := apps["sandboxPort"] + if tt.wantPort == nil { + if ok { + t.Errorf("mcp.apps.sandboxPort = %v, want it absent", got) + } + return + } + if got != tt.wantPort { + t.Errorf("mcp.apps.sandboxPort = %v, want %v", got, tt.wantPort) + } + }) + } +} + +func TestNginxStreamConfig_ProxiesMcpAppsSandbox(t *testing.T) { + conf := nginxStreamConfig() + listen := fmt.Sprintf("listen 0.0.0.0:%d;", McpAppsSandboxProxyPort) + pass := fmt.Sprintf("proxy_pass 127.0.0.1:%d;", McpAppsSandboxPort) + i := strings.Index(conf, listen) + if i < 0 { + t.Fatalf("nginx config does not listen on the sandbox proxy port:\n%s", conf) + } + // The proxy_pass must belong to the same server block as the listen line. + block := conf[i:] + if end := strings.Index(block, "}"); end >= 0 { + block = block[:end] + } + if !strings.Contains(block, pass) { + t.Errorf("sandbox proxy server block does not forward to %d:\n%s", McpAppsSandboxPort, block) + } +} + +func TestBuildStatefulSet_McpAppsContainerPorts(t *testing.T) { + instance := newTestInstance("mcp-apps-sts") + sts := BuildStatefulSet(instance, "", nil, nil, nil) + + var main, proxy bool + for _, c := range sts.Spec.Template.Spec.Containers { + switch c.Name { + case "openclaw": + main = true + assertContainerPort(t, c.Ports, "mcp-apps", McpAppsSandboxPort) + case "gateway-proxy": + proxy = true + assertContainerPort(t, c.Ports, "mcp-apps-proxy", McpAppsSandboxProxyPort) + } + } + if !main || !proxy { + t.Fatalf("expected openclaw and gateway-proxy containers, got main=%v proxy=%v", main, proxy) + } +} + +func TestBuildService_McpAppsPort(t *testing.T) { + instance := newTestInstance("mcp-apps-svc") + svc := BuildService(instance) + assertServicePortWithTarget(t, svc.Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), int32(McpAppsSandboxProxyPort)) + + instance.Spec.Gateway.Enabled = Ptr(false) + svc = BuildService(instance) + assertServicePortWithTarget(t, svc.Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), int32(McpAppsSandboxPort)) +} + +func TestBuildNetworkPolicy_McpAppsIngressPort(t *testing.T) { + instance := newTestInstance("mcp-apps-np") + assertNPPort(t, networkPolicyIngressPorts(instance), McpAppsSandboxProxyPort) + + instance.Spec.Gateway.Enabled = Ptr(false) + assertNPPort(t, networkPolicyIngressPorts(instance), McpAppsSandboxPort) +} diff --git a/internal/resources/networkpolicy.go b/internal/resources/networkpolicy.go index e2f7157f..1554e352 100644 --- a/internal/resources/networkpolicy.go +++ b/internal/resources/networkpolicy.go @@ -84,9 +84,11 @@ func networkPolicyIngressPorts(instance *openclawv1alpha1.OpenClawInstance) []ne // otherwise use the direct gateway/canvas ports. gwPort := int32(GatewayProxyPort) canvasPort := int32(CanvasProxyPort) + mcpAppsPort := int32(McpAppsSandboxProxyPort) if !IsGatewayProxyEnabled(instance) { gwPort = int32(GatewayPort) canvasPort = int32(CanvasPort) + mcpAppsPort = int32(McpAppsSandboxPort) } ports := []networkingv1.NetworkPolicyPort{ @@ -98,6 +100,10 @@ func networkPolicyIngressPorts(instance *openclawv1alpha1.OpenClawInstance) []ne Protocol: Ptr(corev1.ProtocolTCP), Port: Ptr(intstr.FromInt32(canvasPort)), }, + { + Protocol: Ptr(corev1.ProtocolTCP), + Port: Ptr(intstr.FromInt32(mcpAppsPort)), + }, } if instance.Spec.WebTerminal.Enabled { diff --git a/internal/resources/resources_test.go b/internal/resources/resources_test.go index 72553af6..f10ccf34 100644 --- a/internal/resources/resources_test.go +++ b/internal/resources/resources_test.go @@ -356,8 +356,8 @@ func TestBuildStatefulSet_Defaults(t *testing.T) { } // Ports (gateway, canvas - metrics port is on the OTel collector sidecar) - if len(main.Ports) != 2 { - t.Fatalf("expected 2 ports, got %d", len(main.Ports)) + if len(main.Ports) != 3 { + t.Fatalf("expected 3 ports, got %d", len(main.Ports)) } assertContainerPort(t, main.Ports, "gateway", GatewayPort) assertContainerPort(t, main.Ports, "canvas", CanvasPort) @@ -1453,8 +1453,8 @@ func TestBuildService_Default(t *testing.T) { } // Ports - should have gateway, canvas, and metrics (metrics enabled by default) - if len(svc.Spec.Ports) != 3 { - t.Fatalf("expected 3 ports, got %d", len(svc.Spec.Ports)) + if len(svc.Spec.Ports) != 4 { + t.Fatalf("expected 4 ports, got %d", len(svc.Spec.Ports)) } assertServicePortWithTarget(t, svc.Spec.Ports, "gateway", int32(GatewayPort), int32(GatewayProxyPort)) @@ -1468,8 +1468,8 @@ func TestBuildService_WithChromium(t *testing.T) { svc := BuildService(instance) - if len(svc.Spec.Ports) != 4 { - t.Fatalf("expected 4 ports with chromium, got %d", len(svc.Spec.Ports)) + if len(svc.Spec.Ports) != 5 { + t.Fatalf("expected 5 ports with chromium, got %d", len(svc.Spec.Ports)) } assertServicePortWithTarget(t, svc.Spec.Ports, "gateway", int32(GatewayPort), int32(GatewayProxyPort)) @@ -1803,8 +1803,8 @@ func TestBuildNetworkPolicy_Default(t *testing.T) { // Ingress ports - gateway proxy and canvas proxy. Metrics live in their own // rule so they can be restricted independently of application traffic (#578). - if len(firstIngress.Ports) != 2 { - t.Fatalf("expected 2 ingress ports, got %d", len(firstIngress.Ports)) + if len(firstIngress.Ports) != 3 { + t.Fatalf("expected 3 ingress ports, got %d", len(firstIngress.Ports)) } assertNPPort(t, firstIngress.Ports, GatewayProxyPort) assertNPPort(t, firstIngress.Ports, CanvasProxyPort) @@ -1866,8 +1866,8 @@ func TestBuildNetworkPolicy_SameNamespaceIngressDisabled(t *testing.T) { if got := peer.NamespaceSelector.MatchLabels["kubernetes.io/metadata.name"]; got != "ingress-system" { t.Errorf("ingress namespace selector = %q, want ingress-system", got) } - if len(np.Spec.Ingress[0].Ports) != 2 { - t.Fatalf("explicit ingress rule has %d ports, want 2", len(np.Spec.Ingress[0].Ports)) + if len(np.Spec.Ingress[0].Ports) != 3 { + t.Fatalf("explicit ingress rule has %d ports, want 3", len(np.Spec.Ingress[0].Ports)) } assertNPPort(t, np.Spec.Ingress[0].Ports, GatewayProxyPort) assertNPPort(t, np.Spec.Ingress[0].Ports, CanvasProxyPort) @@ -11095,8 +11095,8 @@ func TestBuildService_MetricsPortDisabled(t *testing.T) { t.Error("service should not include metrics port when metrics is disabled") } } - if len(svc.Spec.Ports) != 2 { - t.Errorf("expected 2 ports (gateway, canvas) when metrics disabled, got %d", len(svc.Spec.Ports)) + if len(svc.Spec.Ports) != 3 { + t.Errorf("expected 3 ports (gateway, canvas, mcp-apps) when metrics disabled, got %d", len(svc.Spec.Ports)) } } @@ -11149,8 +11149,8 @@ func TestBuildStatefulSet_MetricsPortDisabled(t *testing.T) { } main := sts.Spec.Template.Spec.Containers[0] - if len(main.Ports) != 2 { - t.Errorf("expected 2 ports (gateway, canvas) when metrics disabled, got %d", len(main.Ports)) + if len(main.Ports) != 3 { + t.Errorf("expected 3 ports (gateway, canvas, mcp-apps) when metrics disabled, got %d", len(main.Ports)) } } @@ -11533,8 +11533,8 @@ func TestBuildService_WithWebTerminal(t *testing.T) { svc := BuildService(instance) - if len(svc.Spec.Ports) != 4 { - t.Fatalf("expected 4 ports with web terminal (gateway, canvas, web-terminal, metrics), got %d", len(svc.Spec.Ports)) + if len(svc.Spec.Ports) != 5 { + t.Fatalf("expected 5 ports with web terminal (gateway, canvas, mcp-apps, web-terminal, metrics), got %d", len(svc.Spec.Ports)) } // gateway and canvas use proxy targetPorts; web-terminal and metrics are direct @@ -11575,8 +11575,8 @@ func TestBuildNetworkPolicy_WebTerminalIngressPort(t *testing.T) { } ports := np.Spec.Ingress[0].Ports - if len(ports) != 3 { - t.Fatalf("expected 3 ingress ports with web terminal, got %d", len(ports)) + if len(ports) != 4 { + t.Fatalf("expected 4 ingress ports with web terminal, got %d", len(ports)) } // Verify web-terminal port is present @@ -11604,8 +11604,8 @@ func TestBuildNetworkPolicy_ChromiumIngressAndEgress(t *testing.T) { } ports := np.Spec.Ingress[0].Ports - if len(ports) != 3 { - t.Fatalf("expected 3 ingress ports with chromium (gateway, canvas, chromium), got %d", len(ports)) + if len(ports) != 4 { + t.Fatalf("expected 4 ingress ports with chromium (gateway, canvas, mcp-apps, chromium), got %d", len(ports)) } foundChromiumIngress := false @@ -11690,8 +11690,8 @@ func TestBuildStatefulSet_HasGatewayProxyContainer(t *testing.T) { } // Ports - if len(proxy.Ports) != 2 { - t.Fatalf("expected 2 proxy ports, got %d", len(proxy.Ports)) + if len(proxy.Ports) != 3 { + t.Fatalf("expected 3 proxy ports, got %d", len(proxy.Ports)) } assertContainerPort(t, proxy.Ports, "gw-proxy", GatewayProxyPort) assertContainerPort(t, proxy.Ports, "canvas-proxy", CanvasProxyPort) diff --git a/internal/resources/service.go b/internal/resources/service.go index 72940a2f..0166c3e5 100644 --- a/internal/resources/service.go +++ b/internal/resources/service.go @@ -85,9 +85,11 @@ func buildServicePorts(instance *openclawv1alpha1.OpenClawInstance) []corev1.Ser // When disabled, target the gateway and canvas ports directly. gwTarget := int32(GatewayProxyPort) canvasTarget := int32(CanvasProxyPort) + mcpAppsTarget := int32(McpAppsSandboxProxyPort) if !IsGatewayProxyEnabled(instance) { gwTarget = int32(GatewayPort) canvasTarget = int32(CanvasPort) + mcpAppsTarget = int32(McpAppsSandboxPort) } ports := []corev1.ServicePort{ @@ -103,6 +105,14 @@ func buildServicePorts(instance *openclawv1alpha1.OpenClawInstance) []corev1.Ser TargetPort: intstr.FromInt32(canvasTarget), Protocol: corev1.ProtocolTCP, }, + // MCP Apps sandbox listener (#615). Only reachable when + // mcp.apps.enabled is set in the OpenClaw config. + { + Name: "mcp-apps", + Port: int32(McpAppsSandboxPort), + TargetPort: intstr.FromInt32(mcpAppsTarget), + Protocol: corev1.ProtocolTCP, + }, } if instance.Spec.Chromium.Enabled { diff --git a/internal/resources/statefulset.go b/internal/resources/statefulset.go index 3a77f57f..9950407b 100644 --- a/internal/resources/statefulset.go +++ b/internal/resources/statefulset.go @@ -286,8 +286,10 @@ func buildContainers(instance *openclawv1alpha1.OpenClawInstance, gatewayTokenSe } // buildMainContainerPorts returns the container ports for the main container. -// Always includes gateway and canvas. The metrics port is on the OTel -// Collector sidecar, not the main container. +// Always includes gateway, canvas and the MCP Apps sandbox. The sandbox +// listener only exists when mcp.apps.enabled is set in the OpenClaw config; +// declaring the port is informational and harmless otherwise. The metrics +// port is on the OTel Collector sidecar, not the main container. func buildMainContainerPorts(instance *openclawv1alpha1.OpenClawInstance) []corev1.ContainerPort { _ = instance // signature kept for consistency return []corev1.ContainerPort{ @@ -301,6 +303,11 @@ func buildMainContainerPorts(instance *openclawv1alpha1.OpenClawInstance) []core ContainerPort: CanvasPort, Protocol: corev1.ProtocolTCP, }, + { + Name: "mcp-apps", + ContainerPort: McpAppsSandboxPort, + Protocol: corev1.ProtocolTCP, + }, } } @@ -1877,6 +1884,11 @@ func buildGatewayProxyContainer(instance *openclawv1alpha1.OpenClawInstance) cor ContainerPort: CanvasProxyPort, Protocol: corev1.ProtocolTCP, }, + { + Name: "mcp-apps-proxy", + ContainerPort: McpAppsSandboxProxyPort, + Protocol: corev1.ProtocolTCP, + }, }, VolumeMounts: []corev1.VolumeMount{ { From e74a988f87d928249c11d2c22af044a0c97f0469 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 19:01:18 -0400 Subject: [PATCH 02/30] docs: document the MCP Apps sandbox port --- README.md | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3449f9c5..65df33e7 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Every request is validated against the instance's allowlist policy. Protected co | | Tailscale (opt) + custom sidecars | | | +------------------------------------------------------------+ | | | -| Service (default: 18789, 18793 or custom) -> Ingress (opt) | +| Service (default: 18789, 18793, 18795 or custom) -> Ingress | +-----------------------------------------------------------------+ ``` @@ -1070,7 +1070,7 @@ The operator builds a single `PathPrefix: /` rule routing to the instance Servic ### Custom service ports -By default the operator creates a Service with the gateway (18789) and canvas (18793) ports. To expose custom ports instead (e.g., for a non-default application), set `spec.networking.service.ports`: +By default the operator creates a Service with the gateway (18789), canvas (18793) and MCP Apps sandbox (18795) ports. To expose custom ports instead (e.g., for a non-default application), set `spec.networking.service.ports`: ```yaml spec: @@ -1085,6 +1085,26 @@ spec: When `ports` is set, it fully replaces the default ports -- including the Chromium port if the sidecar is enabled. To keep the defaults alongside custom ports, include them explicitly. If `targetPort` is omitted it defaults to `port`. See the [API reference](docs/api-reference.md#specnetworkingservice) for all fields. +### MCP Apps sandbox + +OpenClaw's [MCP Apps](https://docs.openclaw.ai/cli/mcp/apps) and script-enabled dashboard widgets are served from a separate sandbox listener. Upstream defaults that listener to the gateway port plus one (18790) on loopback, which inside an operator-managed pod is the port the gateway proxy sidecar already listens on. The operator therefore handles the sandbox like the gateway and canvas ports: + +- When `mcp.apps.enabled` is `true` in your OpenClaw config and `mcp.apps.sandboxPort` is not set, the operator sets `mcp.apps.sandboxPort` to `18795`. +- The gateway proxy sidecar forwards pod port `18796` to the loopback listener on `18795`. +- The Service exposes port `18795` (named `mcp-apps`), and the NetworkPolicy allows ingress to it under the same rules as the gateway and canvas ports. + +```yaml +spec: + config: + raw: + mcp: + apps: + enabled: true + sandboxOrigin: "https://apps.openclaw.example.com" +``` + +OpenClaw requires the sandbox origin to differ from the Control UI origin, so give it a dedicated hostname and route only that hostname to Service port `18795` with your own Ingress or HTTPRoute. If you set `mcp.apps.sandboxPort` yourself, your value wins; use `18795` unless you also expose your port with a custom sidecar, because the proxy only forwards to `18795`. With `spec.networking.service.ports` set, add the `mcp-apps` port (target port `18796`) to your list explicitly. + ### CA bundle injection Inject a custom CA certificate bundle for environments with TLS-intercepting proxies or private CAs: From 6b27b4c359ceea98cec4594b73177752375c528a Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 19:01:41 -0400 Subject: [PATCH 03/30] docs: list the MCP Apps sandbox port in the network policy overview --- docs/architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture.md b/docs/architecture.md index b33f3545..c4fbb83f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -166,7 +166,7 @@ The Chromium sidecar (if enabled) runs as UID `1001` with `readOnlyRootFilesyste When `security.networkPolicy.enabled` is `true` (the default), the operator creates a NetworkPolicy that implements a default-deny posture with selective allowlisting: **Ingress rules:** -- Allow traffic from the same namespace on ports 18789 (gateway) and 18793 (canvas) by default. Set `security.networkPolicy.allowSameNamespaceIngress: false` when explicit ingress rules own the application boundary. +- Allow traffic from the same namespace on ports 18789 (gateway), 18793 (canvas) and 18795 (MCP Apps sandbox) by default. Set `security.networkPolicy.allowSameNamespaceIngress: false` when explicit ingress rules own the application boundary. - Allow traffic from explicitly listed namespaces (`allowedIngressNamespaces`). - Allow traffic from explicitly listed CIDRs (`allowedIngressCIDRs`). From 70630eed766b866289e4fb5ed13e063f48348cb0 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 19:02:47 -0400 Subject: [PATCH 04/30] test: add MCP Apps sandbox e2e test --- test/e2e/e2e_mcp_apps_test.go | 132 ++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 test/e2e/e2e_mcp_apps_test.go diff --git a/test/e2e/e2e_mcp_apps_test.go b/test/e2e/e2e_mcp_apps_test.go new file mode 100644 index 00000000..63c85327 --- /dev/null +++ b/test/e2e/e2e_mcp_apps_test.go @@ -0,0 +1,132 @@ +/* +Copyright 2026 Paperclip Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "encoding/json" + "fmt" + "os" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + + openclawv1alpha1 "github.com/paperclipinc/openclaw-operator/api/v1alpha1" + "github.com/paperclipinc/openclaw-operator/internal/resources" +) + +// This suite verifies the fix for #615: OpenClaw's MCP Apps sandbox listener +// defaults to "gateway port plus one", which is the port the gateway proxy +// sidecar already listens on, and it binds to loopback only. The operator pins +// the sandbox to its own port and forwards it through the proxy, the Service +// and the NetworkPolicy like the gateway and canvas ports. +var _ = Describe("MCP Apps sandbox port (#615)", func() { + const ( + timeout = time.Second * 60 + interval = time.Second * 1 + ) + + var namespace string + + BeforeEach(func() { + if os.Getenv("E2E_SKIP_RESOURCE_VALIDATION") == "true" { + Skip("Skipping resource validation in minimal mode") + } + namespace = "test-mcp-apps-" + time.Now().Format("20060102150405") + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + }) + + AfterEach(func() { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + _ = k8sClient.Delete(ctx, ns) + }) + + It("Should pin the sandbox port and expose it through the proxy and Service", func() { + instance := &openclawv1alpha1.OpenClawInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mcp-apps", + Namespace: namespace, + Annotations: map[string]string{ + "openclaw.rocks/skip-backup": "true", + }, + }, + Spec: openclawv1alpha1.OpenClawInstanceSpec{ + Image: openclawv1alpha1.ImageSpec{ + Repository: "ghcr.io/openclaw/openclaw", + Tag: "latest", + }, + Config: openclawv1alpha1.ConfigSpec{ + Raw: &openclawv1alpha1.RawConfig{ + RawExtension: runtime.RawExtension{ + Raw: []byte(`{"mcp":{"apps":{"enabled":true}}}`), + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, instance)).Should(Succeed()) + + cm := &corev1.ConfigMap{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: resources.ConfigMapName(instance), + Namespace: namespace, + }, cm) + }, timeout, interval).Should(Succeed()) + + var parsed map[string]interface{} + Expect(json.Unmarshal([]byte(cm.Data["openclaw.json"]), &parsed)).To(Succeed()) + mcp, ok := parsed["mcp"].(map[string]interface{}) + Expect(ok).To(BeTrue(), "rendered config should keep the mcp section") + apps, ok := mcp["apps"].(map[string]interface{}) + Expect(ok).To(BeTrue(), "rendered config should keep mcp.apps") + Expect(apps["sandboxPort"]).To(BeEquivalentTo(resources.McpAppsSandboxPort), + "the sandbox must not stay on gateway+1, which the gateway proxy occupies") + + nginxConf := cm.Data[resources.NginxConfigKey] + Expect(nginxConf).To(ContainSubstring(fmt.Sprintf("listen 0.0.0.0:%d;", resources.McpAppsSandboxProxyPort))) + Expect(nginxConf).To(ContainSubstring(fmt.Sprintf("proxy_pass 127.0.0.1:%d;", resources.McpAppsSandboxPort))) + Expect(strings.Count(nginxConf, "listen 0.0.0.0:")).To(Equal(3), + "gateway, canvas and MCP Apps sandbox should each have one proxy listener") + + service := &corev1.Service{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: resources.ServiceName(instance), + Namespace: namespace, + }, service) + }, timeout, interval).Should(Succeed()) + + var found bool + for _, p := range service.Spec.Ports { + if p.Name != "mcp-apps" { + continue + } + found = true + Expect(p.Port).To(BeEquivalentTo(resources.McpAppsSandboxPort)) + Expect(p.TargetPort.IntValue()).To(Equal(resources.McpAppsSandboxProxyPort), + "the Service must target the proxy port because the sandbox binds to loopback") + } + Expect(found).To(BeTrue(), "Service should expose the mcp-apps port") + }) +}) From 9a3dca46faa926fe2bc195d161a039ff70fb47b2 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:05:01 -0400 Subject: [PATCH 05/30] fix: skip init-data-owner under restricted PSS --- .../data_owner_pod_security_test.go | 125 ++++++++++++++++++ .../controller/openclawinstance_controller.go | 39 ++++++ internal/controller/suite_test.go | 1 + 3 files changed, 165 insertions(+) create mode 100644 internal/controller/data_owner_pod_security_test.go diff --git a/internal/controller/data_owner_pod_security_test.go b/internal/controller/data_owner_pod_security_test.go new file mode 100644 index 00000000..32fdcb65 --- /dev/null +++ b/internal/controller/data_owner_pod_security_test.go @@ -0,0 +1,125 @@ +/* +Copyright 2026 Paperclip Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + + openclawv1alpha1 "github.com/paperclipinc/openclaw-operator/api/v1alpha1" + "github.com/paperclipinc/openclaw-operator/internal/resources" +) + +// The init-data-owner init container (#607) runs as root with CAP_CHOWN. A +// namespace enforcing the "restricted" Pod Security Standard rejects such a +// pod at admission, so with storage.fixOwnership unset the operator must leave +// it out there, while an explicit value always wins. +var _ = Describe("init-data-owner and Pod Security Standards", func() { + const ( + timeout = time.Second * 30 + interval = time.Millisecond * 250 + ) + + var counter int + + newNamespace := func(enforce string) string { + counter++ + name := fmt.Sprintf("data-owner-pss-%d-%d", time.Now().UnixNano(), counter) + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}} + if enforce != "" { + ns.Labels = map[string]string{resources.PodSecurityEnforceLabel: enforce} + } + Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + return name + } + + // initContainerNames creates an instance and returns the init container + // names of the StatefulSet the operator renders for it. + initContainerNames := func(namespace string, fixOwnership *bool) []string { + instance := &openclawv1alpha1.OpenClawInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "data-owner", Namespace: namespace}, + Spec: openclawv1alpha1.OpenClawInstanceSpec{ + Storage: openclawv1alpha1.StorageSpec{FixOwnership: fixOwnership}, + }, + } + instance.Spec.Config.Raw = &openclawv1alpha1.RawConfig{ + RawExtension: runtime.RawExtension{Raw: []byte(`{}`)}, + } + Expect(k8sClient.Create(ctx, instance)).To(Succeed()) + + sts := &appsv1.StatefulSet{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: resources.StatefulSetName(instance), + Namespace: namespace, + }, sts) + }, timeout, interval).Should(Succeed()) + + names := make([]string, 0, len(sts.Spec.Template.Spec.InitContainers)) + for _, c := range sts.Spec.Template.Spec.InitContainers { + names = append(names, c.Name) + } + return names + } + + It("runs init-data-owner by default in a namespace without Pod Security enforcement", func() { + names := initContainerNames(newNamespace(""), nil) + Expect(names).To(ContainElement(resources.DataOwnerInitContainerName)) + Expect(names[0]).To(Equal(resources.DataOwnerInitContainerName)) + }) + + It("runs init-data-owner by default in a namespace enforcing baseline", func() { + // baseline permits root containers and the CHOWN capability. + Expect(initContainerNames(newNamespace("baseline"), nil)). + To(ContainElement(resources.DataOwnerInitContainerName)) + }) + + It("leaves init-data-owner out by default in a namespace enforcing restricted", func() { + Expect(initContainerNames(newNamespace("restricted"), nil)). + NotTo(ContainElement(resources.DataOwnerInitContainerName)) + }) + + It("honors an explicit fixOwnership=true in a namespace enforcing restricted", func() { + Expect(initContainerNames(newNamespace("restricted"), resources.Ptr(true))). + To(ContainElement(resources.DataOwnerInitContainerName)) + }) + + It("honors an explicit fixOwnership=false in an unrestricted namespace", func() { + Expect(initContainerNames(newNamespace(""), resources.Ptr(false))). + NotTo(ContainElement(resources.DataOwnerInitContainerName)) + }) + + It("does not write the resolved default back to the stored spec", func() { + namespace := newNamespace("restricted") + Expect(initContainerNames(namespace, nil)). + NotTo(ContainElement(resources.DataOwnerInitContainerName)) + + stored := &openclawv1alpha1.OpenClawInstance{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "data-owner", Namespace: namespace}, stored)).To(Succeed()) + Expect(stored.Spec.Storage.FixOwnership).To(BeNil(), + "the namespace-derived default must stay in memory so relabeling the namespace takes effect") + }) +}) diff --git a/internal/controller/openclawinstance_controller.go b/internal/controller/openclawinstance_controller.go index f5b87118..f2867cce 100644 --- a/internal/controller/openclawinstance_controller.go +++ b/internal/controller/openclawinstance_controller.go @@ -86,6 +86,11 @@ type OpenClawInstanceReconciler struct { Scheme *runtime.Scheme Recorder record.EventRecorder OperatorNamespace string + // APIReader reads straight from the API server, bypassing the cache. It is + // used for the instance's Namespace, which the operator only has "get" + // permission for (a cached read would need a cluster-wide list/watch). + // Optional: when nil, namespace-dependent defaults are skipped. + APIReader client.Reader VersionResolver *registry.Resolver SkillPackResolver *skillpacks.Resolver } @@ -111,6 +116,7 @@ type OpenClawInstanceReconciler struct { // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;delete // +kubebuilder:rbac:groups=batch,resources=cronjobs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=prometheusrules,verbs=get;list;watch;create;update;patch;delete @@ -383,6 +389,35 @@ func (r *OpenClawInstanceReconciler) applyClusterDefaults(ctx context.Context, i return nil } +// applyNamespaceDefaults resolves spec fields whose default depends on the +// instance's namespace. It only mutates the in-memory instance used for +// rendering; the stored spec is never written back. +// +// storage.fixOwnership: the init-data-owner init container runs as root with +// CAP_CHOWN. A namespace that enforces the "restricted" Pod Security Standard +// rejects such a pod at admission, which would take a running instance down on +// its next rollout. When the user left the field unset, skip the init +// container there. An explicit true or false is always honored. +// +// Failing to read the namespace (for example RBAC that predates the +// namespaces "get" rule) is not fatal: the default stays on, as documented. +func (r *OpenClawInstanceReconciler) applyNamespaceDefaults(ctx context.Context, instance *openclawv1alpha1.OpenClawInstance) { + if instance.Spec.Storage.FixOwnership != nil || r.APIReader == nil { + return + } + logger := log.FromContext(ctx) + + ns := &corev1.Namespace{} + if err := r.APIReader.Get(ctx, types.NamespacedName{Name: instance.Namespace}, ns); err != nil { + logger.V(1).Info("could not read namespace to resolve storage.fixOwnership; keeping it enabled", "error", err.Error()) + return + } + if resources.NamespaceEnforcesRestrictedPodSecurity(ns.Labels) { + instance.Spec.Storage.FixOwnership = resources.Ptr(false) + logger.V(1).Info("namespace enforces the restricted Pod Security Standard; skipping init-data-owner") + } +} + // reconcileResources reconciles all managed resources func (r *OpenClawInstanceReconciler) reconcileResources(ctx context.Context, instance *openclawv1alpha1.OpenClawInstance) error { logger := log.FromContext(ctx) @@ -395,6 +430,10 @@ func (r *OpenClawInstanceReconciler) reconcileResources(ctx context.Context, ins return fmt.Errorf("failed to apply cluster defaults: %w", err) } + // Resolve defaults that depend on the instance's namespace. Like cluster + // defaults, this only touches the in-memory copy. + r.applyNamespaceDefaults(ctx, instance) + // 1. Reconcile RBAC (ServiceAccount, Role, RoleBinding) if err := r.reconcileRBAC(ctx, instance); err != nil { return fmt.Errorf("failed to reconcile RBAC: %w", err) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 2ee11c8e..be2aef04 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -87,6 +87,7 @@ var _ = BeforeSuite(func() { Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor("openclawinstance-controller"), OperatorNamespace: "default", + APIReader: mgr.GetAPIReader(), }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) From a44d36a5415eb431fc9085a8186d58883eea5878 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:05:46 -0400 Subject: [PATCH 06/30] fix: add restricted PSS namespace helper --- internal/resources/common.go | 15 ++++++++++++++- internal/resources/data_owner_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/resources/common.go b/internal/resources/common.go index 2b3268bb..4733d5ff 100644 --- a/internal/resources/common.go +++ b/internal/resources/common.go @@ -387,11 +387,24 @@ func IsPersistenceEnabled(instance *openclawv1alpha1.OpenClawInstance) bool { } // IsDataOwnershipFixEnabled returns whether the init-data-owner init container -// runs. Defaults to true; see StorageSpec.FixOwnership. +// runs. Defaults to true; see StorageSpec.FixOwnership. The controller resolves +// an unset value to false for namespaces that enforce the "restricted" Pod +// Security Standard before the builders run. func IsDataOwnershipFixEnabled(instance *openclawv1alpha1.OpenClawInstance) bool { return instance.Spec.Storage.FixOwnership == nil || *instance.Spec.Storage.FixOwnership } +// PodSecurityEnforceLabel is the namespace label Pod Security Admission reads +// to pick the Pod Security Standard it enforces. +const PodSecurityEnforceLabel = "pod-security.kubernetes.io/enforce" + +// NamespaceEnforcesRestrictedPodSecurity reports whether a namespace with the +// given labels rejects pods that violate the "restricted" Pod Security +// Standard. Only the enforce mode matters: audit and warn never block a pod. +func NamespaceEnforcesRestrictedPodSecurity(namespaceLabels map[string]string) bool { + return namespaceLabels[PodSecurityEnforceLabel] == "restricted" +} + // ChromiumPVCName returns the name of the Chromium browser profile PVC func ChromiumPVCName(instance *openclawv1alpha1.OpenClawInstance) string { return instance.Name + "-chromium-data" diff --git a/internal/resources/data_owner_test.go b/internal/resources/data_owner_test.go index aabde33c..abcf91d0 100644 --- a/internal/resources/data_owner_test.go +++ b/internal/resources/data_owner_test.go @@ -184,3 +184,30 @@ func TestIsDataOwnershipFixEnabled(t *testing.T) { t.Error("false should be disabled") } } + +func TestNamespaceEnforcesRestrictedPodSecurity(t *testing.T) { + for _, tt := range []struct { + name string + labels map[string]string + want bool + }{ + {name: "no labels", labels: nil, want: false}, + {name: "enforce restricted", labels: map[string]string{PodSecurityEnforceLabel: "restricted"}, want: true}, + {name: "enforce baseline", labels: map[string]string{PodSecurityEnforceLabel: "baseline"}, want: false}, + {name: "enforce privileged", labels: map[string]string{PodSecurityEnforceLabel: "privileged"}, want: false}, + { + name: "warn and audit restricted do not block pods", + labels: map[string]string{ + "pod-security.kubernetes.io/warn": "restricted", + "pod-security.kubernetes.io/audit": "restricted", + }, + want: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + if got := NamespaceEnforcesRestrictedPodSecurity(tt.labels); got != tt.want { + t.Errorf("NamespaceEnforcesRestrictedPodSecurity(%v) = %v, want %v", tt.labels, got, tt.want) + } + }) + } +} From 0881cd12bf5a8e99d8f6b6479d37abace9dde793 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:06:07 -0400 Subject: [PATCH 07/30] fix: make storage.fixOwnership tri-state --- api/v1alpha1/openclawinstance_types.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/openclawinstance_types.go b/api/v1alpha1/openclawinstance_types.go index 1f5c2c8e..823564ed 100644 --- a/api/v1alpha1/openclawinstance_types.go +++ b/api/v1alpha1/openclawinstance_types.go @@ -722,10 +722,13 @@ type StorageSpec struct { // modes on that path when it writes config and fails with // "EPERM: operation not permitted, fchmod" if the directory is not owned by // the pod UID. The init container runs as root with only CAP_CHOWN and - // exits immediately when ownership is already correct. Set to false on - // clusters that forbid root init containers (e.g. the "restricted" Pod - // Security Standard) and fix ownership out of band instead. - // +kubebuilder:default=true + // exits immediately when ownership is already correct. + // + // When unset, the init container runs unless the instance's namespace + // enforces the "restricted" Pod Security Standard + // (pod-security.kubernetes.io/enforce=restricted), which would reject a + // root init container at admission. In such namespaces fix ownership out + // of band instead. Set to true to always run it, or false to never run it. // +optional FixOwnership *bool `json:"fixOwnership,omitempty"` } From c09fc615b7cabd92c25f60765d0cb91d066b415a Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:06:28 -0400 Subject: [PATCH 08/30] chore: sync OLM bundle CRD and RBAC --- ...penclaw-operator.v0.9.0.clusterserviceversion.yaml | 6 ++++++ .../manifests/openclaw.rocks_openclawinstances.yaml | 11 +++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/bundle/manifests/openclaw-operator.v0.9.0.clusterserviceversion.yaml b/bundle/manifests/openclaw-operator.v0.9.0.clusterserviceversion.yaml index 5cf3d066..c6773559 100644 --- a/bundle/manifests/openclaw-operator.v0.9.0.clusterserviceversion.yaml +++ b/bundle/manifests/openclaw-operator.v0.9.0.clusterserviceversion.yaml @@ -473,6 +473,12 @@ spec: - pods verbs: - list + - apiGroups: + - "" + resources: + - namespaces + verbs: + - get - apiGroups: - apps resources: diff --git a/bundle/manifests/openclaw.rocks_openclawinstances.yaml b/bundle/manifests/openclaw.rocks_openclawinstances.yaml index 75b59c9a..136ff379 100644 --- a/bundle/manifests/openclaw.rocks_openclawinstances.yaml +++ b/bundle/manifests/openclaw.rocks_openclawinstances.yaml @@ -9700,7 +9700,6 @@ spec: description: Storage specifies persistent storage configuration properties: fixOwnership: - default: true description: |- FixOwnership runs the operator-managed init-data-owner init container, which chowns the root of the data volume to the pod's runAsUser:runAsGroup @@ -9710,9 +9709,13 @@ spec: modes on that path when it writes config and fails with "EPERM: operation not permitted, fchmod" if the directory is not owned by the pod UID. The init container runs as root with only CAP_CHOWN and - exits immediately when ownership is already correct. Set to false on - clusters that forbid root init containers (e.g. the "restricted" Pod - Security Standard) and fix ownership out of band instead. + exits immediately when ownership is already correct. + + When unset, the init container runs unless the instance's namespace + enforces the "restricted" Pod Security Standard + (pod-security.kubernetes.io/enforce=restricted), which would reject a + root init container at admission. In such namespaces fix ownership out + of band instead. Set to true to always run it, or false to never run it. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim From 4a04d4455bf9992cf1d5c494fa6154c1f1f565ff Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:06:48 -0400 Subject: [PATCH 09/30] chore: grant namespaces get in chart RBAC --- charts/openclaw-operator/templates/_helpers.tpl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/charts/openclaw-operator/templates/_helpers.tpl b/charts/openclaw-operator/templates/_helpers.tpl index 3f07b29e..dc92a770 100644 --- a/charts/openclaw-operator/templates/_helpers.tpl +++ b/charts/openclaw-operator/templates/_helpers.tpl @@ -88,6 +88,10 @@ kubebuilder-generated config/rbac/role.yaml. - apiGroups: [""] resources: ["pods"] verbs: ["get", "list", "watch"] +# Read the instance namespace's Pod Security labels (storage.fixOwnership default) +- apiGroups: [""] + resources: ["namespaces"] + verbs: ["get"] # Apps API - apiGroups: ["apps"] resources: ["statefulsets"] From 78c3762f9b4bea4aaf4ec39ccd5e2bd38bb9e935 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:07:08 -0400 Subject: [PATCH 10/30] chore: sync Helm chart CRD --- .../crds/openclaw.rocks_openclawinstances.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml index 744be5c7..e0d96484 100644 --- a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml +++ b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml @@ -9706,7 +9706,6 @@ spec: description: Storage specifies persistent storage configuration properties: fixOwnership: - default: true description: |- FixOwnership runs the operator-managed init-data-owner init container, which chowns the root of the data volume to the pod's runAsUser:runAsGroup @@ -9716,9 +9715,13 @@ spec: modes on that path when it writes config and fails with "EPERM: operation not permitted, fchmod" if the directory is not owned by the pod UID. The init container runs as root with only CAP_CHOWN and - exits immediately when ownership is already correct. Set to false on - clusters that forbid root init containers (e.g. the "restricted" Pod - Security Standard) and fix ownership out of band instead. + exits immediately when ownership is already correct. + + When unset, the init container runs unless the instance's namespace + enforces the "restricted" Pod Security Standard + (pod-security.kubernetes.io/enforce=restricted), which would reject a + root init container at admission. In such namespaces fix ownership out + of band instead. Set to true to always run it, or false to never run it. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim From c1b0995757a753ff20f1bc9a07119c278e81cbdd Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:07:28 -0400 Subject: [PATCH 11/30] fix: wire the uncached API reader --- cmd/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/main.go b/cmd/main.go index 93730549..13dd614b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -167,6 +167,7 @@ func main() { Scheme: mgr.GetScheme(), Recorder: mgr.GetEventRecorderFor("openclawinstance-controller"), OperatorNamespace: operatorNamespace, + APIReader: mgr.GetAPIReader(), VersionResolver: versionResolver, SkillPackResolver: skillPackResolver, }).SetupWithManager(mgr); err != nil { From e6b0b04705528d9a9eca8d2a652a29e814f19589 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:07:51 -0400 Subject: [PATCH 12/30] chore: regenerate CRD --- .../crd/bases/openclaw.rocks_openclawinstances.yaml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/config/crd/bases/openclaw.rocks_openclawinstances.yaml b/config/crd/bases/openclaw.rocks_openclawinstances.yaml index 75b59c9a..136ff379 100644 --- a/config/crd/bases/openclaw.rocks_openclawinstances.yaml +++ b/config/crd/bases/openclaw.rocks_openclawinstances.yaml @@ -9700,7 +9700,6 @@ spec: description: Storage specifies persistent storage configuration properties: fixOwnership: - default: true description: |- FixOwnership runs the operator-managed init-data-owner init container, which chowns the root of the data volume to the pod's runAsUser:runAsGroup @@ -9710,9 +9709,13 @@ spec: modes on that path when it writes config and fails with "EPERM: operation not permitted, fchmod" if the directory is not owned by the pod UID. The init container runs as root with only CAP_CHOWN and - exits immediately when ownership is already correct. Set to false on - clusters that forbid root init containers (e.g. the "restricted" Pod - Security Standard) and fix ownership out of band instead. + exits immediately when ownership is already correct. + + When unset, the init container runs unless the instance's namespace + enforces the "restricted" Pod Security Standard + (pod-security.kubernetes.io/enforce=restricted), which would reject a + root init container at admission. In such namespaces fix ownership out + of band instead. Set to true to always run it, or false to never run it. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim From 3f3434394e9d0fe70267da279a4ff4a51cb67dcd Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:08:33 -0400 Subject: [PATCH 13/30] chore: regenerate manager role --- config/rbac/role.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c1b57baa..f71eea73 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -26,6 +26,12 @@ rules: verbs: - create - patch +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get - apiGroups: - "" resources: From e7d9f6e362fa3f6d7d3c7eee81e85e945c580cbd Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:08:52 -0400 Subject: [PATCH 14/30] docs: regenerate API reference --- docs/api-reference.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/api-reference.md b/docs/api-reference.md index 14e50847..d3206c33 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1350,6 +1350,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `persistence` _[PersistenceSpec](#persistencespec)_ | Persistence configures the PersistentVolumeClaim | | Optional: \{\}
| +| `fixOwnership` _boolean_ | FixOwnership runs the operator-managed init-data-owner init container,
which chowns the root of the data volume to the pod's runAsUser:runAsGroup
when it is owned by someone else (typically root). Kubernetes only applies
fsGroup to a volume root, so on most PVCs the directory mounted at
~/.openclaw stays owned by root. OpenClaw >= 2026.9 tightens directory
modes on that path when it writes config and fails with
"EPERM: operation not permitted, fchmod" if the directory is not owned by
the pod UID. The init container runs as root with only CAP_CHOWN and
exits immediately when ownership is already correct.
When unset, the init container runs unless the instance's namespace
enforces the "restricted" Pod Security Standard
(pod-security.kubernetes.io/enforce=restricted), which would reject a
root init container at admission. In such namespaces fix ownership out
of band instead. Set to true to always run it, or false to never run it. | | Optional: \{\}
| #### TailscaleImageSpec From f4df94fce77f377c3c952cd44ec9b53e4f6e1ac6 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 20:09:11 -0400 Subject: [PATCH 15/30] docs: describe fixOwnership under restricted PSS --- README.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2063a074..1306f0ab 100644 --- a/README.md +++ b/README.md @@ -933,7 +933,15 @@ Kubernetes applies `fsGroup` to the root of a volume but never changes its owner The operator therefore runs an `init-data-owner` init container first on every pod start. It runs as root with every capability dropped except `CHOWN`, changes the owner of the volume root to the pod's `runAsUser:runAsGroup` when it differs, and exits without touching anything when ownership is already correct. Children of the volume root are created by the pod UID and are never modified. -If your cluster forbids root init containers (for example the `restricted` Pod Security Standard), disable it and fix ownership out of band once per PVC: +Namespaces that enforce the `restricted` Pod Security Standard (`pod-security.kubernetes.io/enforce: restricted`) reject root init containers at admission. The operator detects that label and leaves `init-data-owner` out there automatically, so upgrading the operator never blocks pods in a restricted namespace. In those namespaces fix ownership out of band once per PVC (for example with a one-off Job in a namespace that permits it, or through your storage provisioner) before moving to OpenClaw 2026.9 or later. + +`spec.storage.fixOwnership` overrides the automatic behavior: + +| Value | Behavior | +|-------|----------| +| unset (default) | Run `init-data-owner`, except in namespaces that enforce the `restricted` Pod Security Standard | +| `true` | Always run it (use when the namespace is exempted from Pod Security Admission some other way) | +| `false` | Never run it (use when another admission policy forbids root init containers) | ```yaml spec: From ec4e9f3588b4e03194d1c76d5f3950e89e9b3170 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 22:39:33 -0400 Subject: [PATCH 16/30] test(e2e): dump pod diagnostics on readiness --- test/e2e/diagnostics_test.go | 94 +++++++++++++++++++++++++++++++ test/e2e/e2e_chromium_cdp_test.go | 10 ++++ 2 files changed, 104 insertions(+) create mode 100644 test/e2e/diagnostics_test.go diff --git a/test/e2e/diagnostics_test.go b/test/e2e/diagnostics_test.go new file mode 100644 index 00000000..06376d5e --- /dev/null +++ b/test/e2e/diagnostics_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2026 Paperclip Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "os/exec" + "strings" + + . "github.com/onsi/ginkgo/v2" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" +) + +// diagnosticLogTailLines bounds how much of each container log is written to +// the spec output when a pod fails to become ready. +const diagnosticLogTailLines = "150" + +// dumpPodDiagnostics writes the state a human needs to explain why a pod never +// became ready: per-container status (including the last termination), the +// namespace events, and the tail of the current and previous logs of every +// container that is not ready. +// +// It deliberately avoids `kubectl describe pod` and never prints the pod spec: +// specs in this suite carry API keys as plain env values. +// +// Call it before the test namespace is deleted; everything it reads goes away +// with the namespace. +func dumpPodDiagnostics(namespace, podName string) { + GinkgoWriter.Printf("\n===== diagnostics for pod %s/%s =====\n", namespace, podName) + + pod := &corev1.Pod{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: podName, Namespace: namespace}, pod); err != nil { + GinkgoWriter.Printf("could not get pod: %v\n", err) + return + } + GinkgoWriter.Printf("phase=%s reason=%q message=%q\n", pod.Status.Phase, pod.Status.Reason, pod.Status.Message) + for _, c := range pod.Status.Conditions { + GinkgoWriter.Printf("condition %s=%s reason=%q message=%q\n", c.Type, c.Status, c.Reason, c.Message) + } + + var notReady []string + report := func(kind string, statuses []corev1.ContainerStatus) { + for _, cs := range statuses { + GinkgoWriter.Printf("%s %s: ready=%t restarts=%d", kind, cs.Name, cs.Ready, cs.RestartCount) + switch { + case cs.State.Waiting != nil: + GinkgoWriter.Printf(" waiting reason=%q message=%q", cs.State.Waiting.Reason, cs.State.Waiting.Message) + case cs.State.Running != nil: + GinkgoWriter.Printf(" running since=%s", cs.State.Running.StartedAt.UTC().Format("15:04:05")) + case cs.State.Terminated != nil: + GinkgoWriter.Printf(" terminated exit=%d reason=%q", cs.State.Terminated.ExitCode, cs.State.Terminated.Reason) + } + if t := cs.LastTerminationState.Terminated; t != nil { + GinkgoWriter.Printf(" | last termination exit=%d reason=%q signal=%d", t.ExitCode, t.Reason, t.Signal) + } + GinkgoWriter.Println() + if !cs.Ready && cs.State.Terminated == nil { + notReady = append(notReady, cs.Name) + } + } + } + report("init container", pod.Status.InitContainerStatuses) + report("container", pod.Status.ContainerStatuses) + + kubectl := func(title string, args ...string) { + out, err := exec.Command("kubectl", args...).CombinedOutput() + GinkgoWriter.Printf("----- %s -----\n%s\n", title, strings.TrimSpace(string(out))) + if err != nil { + GinkgoWriter.Printf("(kubectl %s: %v)\n", strings.Join(args, " "), err) + } + } + + kubectl("events", "get", "events", "-n", namespace, "--sort-by=.lastTimestamp", + "-o", "custom-columns=TIME:.lastTimestamp,TYPE:.type,REASON:.reason,OBJECT:.involvedObject.name,MESSAGE:.message") + for _, name := range notReady { + kubectl("logs "+name, "logs", podName, "-n", namespace, "-c", name, "--tail="+diagnosticLogTailLines) + kubectl("previous logs "+name, "logs", podName, "-n", namespace, "-c", name, "--previous", "--tail="+diagnosticLogTailLines) + } + GinkgoWriter.Printf("===== end diagnostics for pod %s/%s =====\n\n", namespace, podName) +} diff --git a/test/e2e/e2e_chromium_cdp_test.go b/test/e2e/e2e_chromium_cdp_test.go index 9380afc5..6e318b75 100644 --- a/test/e2e/e2e_chromium_cdp_test.go +++ b/test/e2e/e2e_chromium_cdp_test.go @@ -761,6 +761,15 @@ var _ = Describe("Chromium Full Integration Tests", Ordered, func() { }, 120*time.Second, 3*time.Second).ShouldNot(BeEmpty()) By("Waiting for all containers to be ready") + // A failed assertion unwinds through this deferred call, so the pod's + // status, events and logs are captured before AfterAll deletes the + // namespace. Without it a timeout here only says "not ready". + allReady := false + defer func() { + if !allReady { + dumpPodDiagnostics(namespace, podName) + } + }() Eventually(func() bool { pod := &corev1.Pod{} err := k8sClient.Get(ctx, types.NamespacedName{ @@ -790,6 +799,7 @@ var _ = Describe("Chromium Full Integration Tests", Ordered, func() { } return true }, 10*time.Minute, 5*time.Second).Should(BeTrue()) + allReady = true By("Finding a free local port for port-forward") listener, err := net.Listen("tcp", ":0") From 7580f6252c230b8c54d7b9c1c44f9d94635e7367 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sat, 3 Oct 2026 23:13:12 -0400 Subject: [PATCH 17/30] test(e2e): iterate statuses by index --- test/e2e/diagnostics_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/e2e/diagnostics_test.go b/test/e2e/diagnostics_test.go index 06376d5e..b66a38a6 100644 --- a/test/e2e/diagnostics_test.go +++ b/test/e2e/diagnostics_test.go @@ -48,13 +48,15 @@ func dumpPodDiagnostics(namespace, podName string) { return } GinkgoWriter.Printf("phase=%s reason=%q message=%q\n", pod.Status.Phase, pod.Status.Reason, pod.Status.Message) - for _, c := range pod.Status.Conditions { + for i := range pod.Status.Conditions { + c := &pod.Status.Conditions[i] GinkgoWriter.Printf("condition %s=%s reason=%q message=%q\n", c.Type, c.Status, c.Reason, c.Message) } var notReady []string report := func(kind string, statuses []corev1.ContainerStatus) { - for _, cs := range statuses { + for i := range statuses { + cs := &statuses[i] GinkgoWriter.Printf("%s %s: ready=%t restarts=%d", kind, cs.Name, cs.Ready, cs.RestartCount) switch { case cs.State.Waiting != nil: From eae70d056fdafb8574bb82c281b7b24b750ef9a5 Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sun, 11 Oct 2026 12:51:33 -0400 Subject: [PATCH 18/30] fix: requeue instead of guessing when the namespace cannot be read With spec.storage.fixOwnership unset, the controller reads the instance's Namespace to decide whether init-data-owner is admissible. When that read failed (for example RBAC without the namespaces "get" rule), it kept the init container enabled, which could flip a working StatefulSet in a restricted namespace to a Pod template admission rejects. The read failure now aborts reconciliation before any managed resource is touched, surfaces through the ReconcileFailed event and Ready condition, and is retried. An explicit fixOwnership still bypasses the lookup. Adds envtest coverage: a restricted-compatible instance whose namespace read then fails keeps its StatefulSet unchanged, and explicit values skip the lookup. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 + api/v1alpha1/openclawinstance_types.go | 2 + .../openclaw.rocks_openclawinstances.yaml | 2 + .../openclaw.rocks_openclawinstances.yaml | 2 + .../openclaw.rocks_openclawinstances.yaml | 2 + docs/api-reference.md | 2 +- .../data_owner_pod_security_test.go | 66 +++++++++++++++++++ .../controller/openclawinstance_controller.go | 25 ++++--- 8 files changed, 94 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 1306f0ab..079e9045 100644 --- a/README.md +++ b/README.md @@ -935,6 +935,8 @@ The operator therefore runs an `init-data-owner` init container first on every p Namespaces that enforce the `restricted` Pod Security Standard (`pod-security.kubernetes.io/enforce: restricted`) reject root init containers at admission. The operator detects that label and leaves `init-data-owner` out there automatically, so upgrading the operator never blocks pods in a restricted namespace. In those namespaces fix ownership out of band once per PVC (for example with a one-off Job in a namespace that permits it, or through your storage provisioner) before moving to OpenClaw 2026.9 or later. +To detect the label the operator needs `get` on `namespaces` (granted by the Helm chart, the OLM bundle, and `config/rbac`). If it cannot read the instance's Namespace, it does not guess: reconciliation fails with a `ReconcileFailed` event naming the missing permission and is retried, without changing the existing StatefulSet. Setting `spec.storage.fixOwnership` explicitly skips the lookup. + `spec.storage.fixOwnership` overrides the automatic behavior: | Value | Behavior | diff --git a/api/v1alpha1/openclawinstance_types.go b/api/v1alpha1/openclawinstance_types.go index 823564ed..59396d15 100644 --- a/api/v1alpha1/openclawinstance_types.go +++ b/api/v1alpha1/openclawinstance_types.go @@ -729,6 +729,8 @@ type StorageSpec struct { // (pod-security.kubernetes.io/enforce=restricted), which would reject a // root init container at admission. In such namespaces fix ownership out // of band instead. Set to true to always run it, or false to never run it. + // Resolving the unset default requires the operator to read the Namespace; + // if that read fails, reconciliation is retried without changing workloads. // +optional FixOwnership *bool `json:"fixOwnership,omitempty"` } diff --git a/bundle/manifests/openclaw.rocks_openclawinstances.yaml b/bundle/manifests/openclaw.rocks_openclawinstances.yaml index 136ff379..6ed5df79 100644 --- a/bundle/manifests/openclaw.rocks_openclawinstances.yaml +++ b/bundle/manifests/openclaw.rocks_openclawinstances.yaml @@ -9716,6 +9716,8 @@ spec: (pod-security.kubernetes.io/enforce=restricted), which would reject a root init container at admission. In such namespaces fix ownership out of band instead. Set to true to always run it, or false to never run it. + Resolving the unset default requires the operator to read the Namespace; + if that read fails, reconciliation is retried without changing workloads. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim diff --git a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml index e0d96484..acf8b003 100644 --- a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml +++ b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml @@ -9722,6 +9722,8 @@ spec: (pod-security.kubernetes.io/enforce=restricted), which would reject a root init container at admission. In such namespaces fix ownership out of band instead. Set to true to always run it, or false to never run it. + Resolving the unset default requires the operator to read the Namespace; + if that read fails, reconciliation is retried without changing workloads. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim diff --git a/config/crd/bases/openclaw.rocks_openclawinstances.yaml b/config/crd/bases/openclaw.rocks_openclawinstances.yaml index 136ff379..6ed5df79 100644 --- a/config/crd/bases/openclaw.rocks_openclawinstances.yaml +++ b/config/crd/bases/openclaw.rocks_openclawinstances.yaml @@ -9716,6 +9716,8 @@ spec: (pod-security.kubernetes.io/enforce=restricted), which would reject a root init container at admission. In such namespaces fix ownership out of band instead. Set to true to always run it, or false to never run it. + Resolving the unset default requires the operator to read the Namespace; + if that read fails, reconciliation is retried without changing workloads. type: boolean persistence: description: Persistence configures the PersistentVolumeClaim diff --git a/docs/api-reference.md b/docs/api-reference.md index d3206c33..7e2e0665 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1350,7 +1350,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `persistence` _[PersistenceSpec](#persistencespec)_ | Persistence configures the PersistentVolumeClaim | | Optional: \{\}
| -| `fixOwnership` _boolean_ | FixOwnership runs the operator-managed init-data-owner init container,
which chowns the root of the data volume to the pod's runAsUser:runAsGroup
when it is owned by someone else (typically root). Kubernetes only applies
fsGroup to a volume root, so on most PVCs the directory mounted at
~/.openclaw stays owned by root. OpenClaw >= 2026.9 tightens directory
modes on that path when it writes config and fails with
"EPERM: operation not permitted, fchmod" if the directory is not owned by
the pod UID. The init container runs as root with only CAP_CHOWN and
exits immediately when ownership is already correct.
When unset, the init container runs unless the instance's namespace
enforces the "restricted" Pod Security Standard
(pod-security.kubernetes.io/enforce=restricted), which would reject a
root init container at admission. In such namespaces fix ownership out
of band instead. Set to true to always run it, or false to never run it. | | Optional: \{\}
| +| `fixOwnership` _boolean_ | FixOwnership runs the operator-managed init-data-owner init container,
which chowns the root of the data volume to the pod's runAsUser:runAsGroup
when it is owned by someone else (typically root). Kubernetes only applies
fsGroup to a volume root, so on most PVCs the directory mounted at
~/.openclaw stays owned by root. OpenClaw >= 2026.9 tightens directory
modes on that path when it writes config and fails with
"EPERM: operation not permitted, fchmod" if the directory is not owned by
the pod UID. The init container runs as root with only CAP_CHOWN and
exits immediately when ownership is already correct.
When unset, the init container runs unless the instance's namespace
enforces the "restricted" Pod Security Standard
(pod-security.kubernetes.io/enforce=restricted), which would reject a
root init container at admission. In such namespaces fix ownership out
of band instead. Set to true to always run it, or false to never run it.
Resolving the unset default requires the operator to read the Namespace;
if that read fails, reconciliation is retried without changing workloads. | | Optional: \{\}
| #### TailscaleImageSpec diff --git a/internal/controller/data_owner_pod_security_test.go b/internal/controller/data_owner_pod_security_test.go index 32fdcb65..53b0f731 100644 --- a/internal/controller/data_owner_pod_security_test.go +++ b/internal/controller/data_owner_pod_security_test.go @@ -17,6 +17,8 @@ limitations under the License. package controller import ( + "context" + "errors" "fmt" "time" @@ -28,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" openclawv1alpha1 "github.com/paperclipinc/openclaw-operator/api/v1alpha1" "github.com/paperclipinc/openclaw-operator/internal/resources" @@ -122,4 +125,67 @@ var _ = Describe("init-data-owner and Pod Security Standards", func() { Expect(stored.Spec.Storage.FixOwnership).To(BeNil(), "the namespace-derived default must stay in memory so relabeling the namespace takes effect") }) + + It("fails reconciliation without touching the StatefulSet when the namespace cannot be read", func() { + namespace := newNamespace("restricted") + Expect(initContainerNames(namespace, nil)). + NotTo(ContainElement(resources.DataOwnerInitContainerName)) + + stsKey := types.NamespacedName{Name: "data-owner", Namespace: namespace} + before := &appsv1.StatefulSet{} + Expect(k8sClient.Get(ctx, stsKey, before)).To(Succeed()) + + instance := &openclawv1alpha1.OpenClawInstance{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "data-owner", Namespace: namespace}, instance)).To(Succeed()) + + // A reconciler whose Namespace read fails, as with RBAC that predates + // the namespaces "get" rule, while workload writes would still succeed. + r := &OpenClawInstanceReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + OperatorNamespace: "default", + APIReader: failingNamespaceReader{Reader: k8sClient}, + } + err := r.reconcileResources(ctx, instance) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.storage.fixOwnership")) + Expect(errors.Is(err, errNamespaceForbidden)).To(BeTrue()) + + after := &appsv1.StatefulSet{} + Expect(k8sClient.Get(ctx, stsKey, after)).To(Succeed()) + Expect(after.ResourceVersion).To(Equal(before.ResourceVersion)) + names := make([]string, 0, len(after.Spec.Template.Spec.InitContainers)) + for _, c := range after.Spec.Template.Spec.InitContainers { + names = append(names, c.Name) + } + Expect(names).NotTo(ContainElement(resources.DataOwnerInitContainerName)) + }) + + It("skips the namespace lookup when fixOwnership is set explicitly", func() { + r := &OpenClawInstanceReconciler{APIReader: failingNamespaceReader{Reader: k8sClient}} + for _, v := range []bool{true, false} { + instance := &openclawv1alpha1.OpenClawInstance{ + ObjectMeta: metav1.ObjectMeta{Name: "explicit", Namespace: "unreadable"}, + Spec: openclawv1alpha1.OpenClawInstanceSpec{ + Storage: openclawv1alpha1.StorageSpec{FixOwnership: resources.Ptr(v)}, + }, + } + Expect(r.applyNamespaceDefaults(ctx, instance)).To(Succeed()) + Expect(*instance.Spec.Storage.FixOwnership).To(Equal(v)) + } + }) }) + +var errNamespaceForbidden = errors.New("namespaces is forbidden") + +// failingNamespaceReader fails every Namespace read and delegates the rest. +type failingNamespaceReader struct { + client.Reader +} + +func (f failingNamespaceReader) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.Namespace); ok { + return errNamespaceForbidden + } + return f.Reader.Get(ctx, key, obj, opts...) +} diff --git a/internal/controller/openclawinstance_controller.go b/internal/controller/openclawinstance_controller.go index f2867cce..4cff34f0 100644 --- a/internal/controller/openclawinstance_controller.go +++ b/internal/controller/openclawinstance_controller.go @@ -399,23 +399,29 @@ func (r *OpenClawInstanceReconciler) applyClusterDefaults(ctx context.Context, i // its next rollout. When the user left the field unset, skip the init // container there. An explicit true or false is always honored. // -// Failing to read the namespace (for example RBAC that predates the -// namespaces "get" rule) is not fatal: the default stays on, as documented. -func (r *OpenClawInstanceReconciler) applyNamespaceDefaults(ctx context.Context, instance *openclawv1alpha1.OpenClawInstance) { +// If the namespace cannot be read (for example RBAC that predates the +// namespaces "get" rule), the default cannot be resolved and an error is +// returned. Guessing either way could flip a working StatefulSet to a Pod +// template that admission rejects, so reconciliation stops before any managed +// resource is touched and is retried. An explicit fixOwnership skips the +// lookup entirely. +func (r *OpenClawInstanceReconciler) applyNamespaceDefaults(ctx context.Context, instance *openclawv1alpha1.OpenClawInstance) error { if instance.Spec.Storage.FixOwnership != nil || r.APIReader == nil { - return + return nil } logger := log.FromContext(ctx) ns := &corev1.Namespace{} if err := r.APIReader.Get(ctx, types.NamespacedName{Name: instance.Namespace}, ns); err != nil { - logger.V(1).Info("could not read namespace to resolve storage.fixOwnership; keeping it enabled", "error", err.Error()) - return + return fmt.Errorf("cannot resolve the default for spec.storage.fixOwnership: reading Namespace %q failed "+ + "(the operator needs \"get\" on namespaces; set spec.storage.fixOwnership explicitly to skip this lookup): %w", + instance.Namespace, err) } if resources.NamespaceEnforcesRestrictedPodSecurity(ns.Labels) { instance.Spec.Storage.FixOwnership = resources.Ptr(false) logger.V(1).Info("namespace enforces the restricted Pod Security Standard; skipping init-data-owner") } + return nil } // reconcileResources reconciles all managed resources @@ -431,8 +437,11 @@ func (r *OpenClawInstanceReconciler) reconcileResources(ctx context.Context, ins } // Resolve defaults that depend on the instance's namespace. Like cluster - // defaults, this only touches the in-memory copy. - r.applyNamespaceDefaults(ctx, instance) + // defaults, this only touches the in-memory copy. It must succeed before + // any managed resource is reconciled. + if err := r.applyNamespaceDefaults(ctx, instance); err != nil { + return err + } // 1. Reconcile RBAC (ServiceAccount, Role, RoleBinding) if err := r.reconcileRBAC(ctx, instance); err != nil { From 2b514fa09cb3e4f2ff68e656023c2662ec3c73fb Mon Sep 17 00:00:00 2001 From: Jannes Stubbemann Date: Sun, 4 Oct 2026 01:09:44 -0400 Subject: [PATCH 19/30] fix: give the main container a sticky /tmp --- internal/resources/resources_test.go | 31 ++--- internal/resources/statefulset.go | 67 +++++++++++ internal/resources/tmp_dir_test.go | 173 +++++++++++++++++++++++++++ 3 files changed, 257 insertions(+), 14 deletions(-) create mode 100644 internal/resources/tmp_dir_test.go diff --git a/internal/resources/resources_test.go b/internal/resources/resources_test.go index ed0242a8..609de344 100644 --- a/internal/resources/resources_test.go +++ b/internal/resources/resources_test.go @@ -879,8 +879,8 @@ func TestBuildStatefulSet_ConfigVolume_RawConfig(t *testing.T) { // Init container should copy config from ConfigMap to data volume initContainers := sts.Spec.Template.Spec.InitContainers - if len(initContainers) != 5 { - t.Fatalf("expected 5 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps), got %d", len(initContainers)) + if len(initContainers) != 6 { + t.Fatalf("expected 6 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps + init-tmp-dir), got %d", len(initContainers)) } initC := initContainers[1] if initC.Name != "init-config" { @@ -924,8 +924,8 @@ func TestBuildStatefulSet_ConfigVolume_ConfigMapRef(t *testing.T) { // The controller reads the external CM and writes enriched content into the // operator-managed CM under "openclaw.json", so the init container always uses that key. initContainers := sts.Spec.Template.Spec.InitContainers - if len(initContainers) != 5 { - t.Fatalf("expected 5 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps), got %d", len(initContainers)) + if len(initContainers) != 6 { + t.Fatalf("expected 6 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps + init-tmp-dir), got %d", len(initContainers)) } initC := initContainers[1] assertVolumeMount(t, initC.VolumeMounts, "data", "/data") @@ -959,8 +959,8 @@ func TestBuildStatefulSet_ConfigMapRef_DefaultKey(t *testing.T) { // Init container should use "openclaw.json" (operator-managed key) initContainers := sts.Spec.Template.Spec.InitContainers - if len(initContainers) != 5 { - t.Fatalf("expected 5 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps), got %d", len(initContainers)) + if len(initContainers) != 6 { + t.Fatalf("expected 6 init containers (init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps + init-tmp-dir), got %d", len(initContainers)) } expectedPrefix := "cp /config/'openclaw.json' /data/openclaw.json" if !strings.HasPrefix(initContainers[1].Command[2], expectedPrefix) { @@ -983,9 +983,9 @@ func TestBuildStatefulSet_VanillaDeployment_HasInitContainer(t *testing.T) { sts := BuildStatefulSet(instance, "", nil, nil, nil) - // Vanilla deployments get init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps - if len(sts.Spec.Template.Spec.InitContainers) != 5 { - t.Fatalf("expected 5 init containers for vanilla deployment, got %d", len(sts.Spec.Template.Spec.InitContainers)) + // Vanilla deployments get init-data-owner + init-config + init-uv + init-pip + init-plugin-runtime-deps + init-tmp-dir + if len(sts.Spec.Template.Spec.InitContainers) != 6 { + t.Fatalf("expected 6 init containers for vanilla deployment, got %d", len(sts.Spec.Template.Spec.InitContainers)) } if sts.Spec.Template.Spec.InitContainers[1].Name != "init-config" { t.Errorf("init container name = %q, want %q", sts.Spec.Template.Spec.InitContainers[1].Name, "init-config") @@ -8029,8 +8029,8 @@ func TestBuildStatefulSet_CustomInitContainers_AfterOperatorManaged(t *testing.T sts := BuildStatefulSet(instance, "", nil, nil, nil) initContainers := sts.Spec.Template.Spec.InitContainers - if len(initContainers) != 7 { - t.Fatalf("expected 7 init containers, got %d", len(initContainers)) + if len(initContainers) != 8 { + t.Fatalf("expected 8 init containers, got %d", len(initContainers)) } if initContainers[0].Name != "init-data-owner" { t.Errorf("initContainers[0] = %q, want init-data-owner", initContainers[0].Name) @@ -8050,8 +8050,11 @@ func TestBuildStatefulSet_CustomInitContainers_AfterOperatorManaged(t *testing.T if initContainers[5].Name != "init-skills" { t.Errorf("initContainers[5] = %q, want init-skills", initContainers[5].Name) } - if initContainers[6].Name != "user-init" { - t.Errorf("initContainers[6] = %q, want user-init", initContainers[6].Name) + if initContainers[6].Name != "init-tmp-dir" { + t.Errorf("initContainers[6] = %q, want init-tmp-dir", initContainers[6].Name) + } + if initContainers[7].Name != "user-init" { + t.Errorf("initContainers[7] = %q, want user-init", initContainers[7].Name) } } @@ -8422,7 +8425,7 @@ func TestBuildStatefulSet_RuntimeDeps_InitContainerOrder(t *testing.T) { sts := BuildStatefulSet(instance, "", nil, nil, nil) initContainers := sts.Spec.Template.Spec.InitContainers - expected := []string{"init-data-owner", "init-config", "init-uv", "init-pip", "init-plugin-runtime-deps", "init-pnpm", "init-python", "init-skills", "user-init"} + expected := []string{"init-data-owner", "init-config", "init-uv", "init-pip", "init-plugin-runtime-deps", "init-pnpm", "init-python", "init-skills", "init-tmp-dir", "user-init"} if len(initContainers) != len(expected) { t.Fatalf("expected %d init containers, got %d: %v", len(expected), len(initContainers), func() []string { diff --git a/internal/resources/statefulset.go b/internal/resources/statefulset.go index 85c79c93..217bda94 100644 --- a/internal/resources/statefulset.go +++ b/internal/resources/statefulset.go @@ -337,9 +337,12 @@ func buildMainContainer(instance *openclawv1alpha1.OpenClawInstance, gatewayToke MountPath: "/home/openclaw/.config", SubPath: ".config", }, + // Mounted through a subdirectory that init-tmp-dir creates with the + // sticky bit. See buildTmpDirInitContainer. { Name: "tmp", MountPath: "/tmp", + SubPath: MainTmpSubPath, }, }, } @@ -671,6 +674,10 @@ func buildInitContainers(instance *openclawv1alpha1.OpenClawInstance, externalWo } } + // Sticky /tmp for the main container. Always present: the main container + // mounts the subdirectory this creates. + initContainers = append(initContainers, buildTmpDirInitContainer(instance)) + // Ollama model-pulling init container (only if enabled and models are specified) if instance.Spec.Ollama.Enabled && len(instance.Spec.Ollama.Models) > 0 { initContainers = append(initContainers, buildOllamaModelPullInitContainer(instance)) @@ -1544,6 +1551,66 @@ pnpm --version` } } +// TmpDirInitContainerName is the name of the init container that prepares the +// main container's /tmp. +const TmpDirInitContainerName = "init-tmp-dir" + +// MainTmpSubPath is the subdirectory of the "tmp" emptyDir that the main +// container mounts at /tmp. +const MainTmpSubPath = "tmp" + +// buildTmpDirInitContainer creates the init container that prepares a /tmp +// with the sticky bit for the main container. +// +// The main container has a read-only root filesystem, so /tmp is an emptyDir. +// With fsGroup set, the kubelet makes the root of an emptyDir group-owned by +// fsGroup with mode 2777: world-writable, setgid, no sticky bit. OpenClaw +// 2026.9.6 and later refuse to create a temporary workspace below such a +// directory ("temp workspace ancestor is group/world writable without sticky +// protection"). The startup doctor needs that workspace whenever it has to +// install a configured plugin, so the gateway exits and the pod crash-loops. +// #616 fixed the same check for init-plugins, which can use the image's own +// /tmp because its root filesystem is writable. +// +// Nothing in the pod spec can set the mode of a volume root, and the pod UID +// does not own it, so it cannot chmod it either. A directory the pod UID +// creates inside the volume is its own, though. This container creates one +// with mode 1777 and the main container mounts it via subPath, which hides the +// 2777 volume root from the main container's path ancestry. +// +// It runs as the pod UID with no capabilities, so it is admitted under the +// "restricted" Pod Security Standard. +func buildTmpDirInitContainer(instance *openclawv1alpha1.OpenClawInstance) corev1.Container { + // chmod after mkdir: the setgid volume root and the umask both influence + // the mode mkdir produces, and a restart of the pod sandbox finds the + // directory already present. + script := fmt.Sprintf("set -e\nmkdir -p /tmp-volume/%[1]s\nchmod 1777 /tmp-volume/%[1]s", MainTmpSubPath) + + return corev1.Container{ + Name: TmpDirInitContainerName, + Image: ApplyRegistryOverride("docker.io/library/busybox:1.37", instance.Spec.Registry), + Command: []string{"sh", "-c", script}, + ImagePullPolicy: corev1.PullIfNotPresent, + TerminationMessagePath: corev1.TerminationMessagePathDefault, + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + Resources: corev1.ResourceRequirements{}, + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: Ptr(false), + ReadOnlyRootFilesystem: Ptr(true), + RunAsNonRoot: Ptr(true), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + SeccompProfile: &corev1.SeccompProfile{ + Type: corev1.SeccompProfileTypeRuntimeDefault, + }, + }, + VolumeMounts: []corev1.VolumeMount{ + {Name: "tmp", MountPath: "/tmp-volume"}, + }, + } +} + // DataOwnerInitContainerName is the name of the init container that fixes // ownership of the data volume root. const DataOwnerInitContainerName = "init-data-owner" diff --git a/internal/resources/tmp_dir_test.go b/internal/resources/tmp_dir_test.go new file mode 100644 index 00000000..ad1ef752 --- /dev/null +++ b/internal/resources/tmp_dir_test.go @@ -0,0 +1,173 @@ +/* +Copyright 2026 Paperclip Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" +) + +// OpenClaw >= 2026.9.6 refuses to create a temporary workspace below a +// directory that is group/world writable without the sticky bit. The root of +// an emptyDir is exactly that (mode 2777) once fsGroup is applied, so the main +// container must get its /tmp from a subdirectory that init-tmp-dir creates +// with mode 1777. + +func findInitContainer(t *testing.T, containers []corev1.Container, name string) (int, *corev1.Container) { + t.Helper() + for i := range containers { + if containers[i].Name == name { + return i, &containers[i] + } + } + t.Fatalf("init container %q not found", name) + return -1, nil +} + +func TestBuildStatefulSet_MainTmpIsMountedThroughStickySubPath(t *testing.T) { + instance := newTestInstance("sticky-tmp") + sts := BuildStatefulSet(instance, "", nil, nil, nil) + + var main *corev1.Container + for i := range sts.Spec.Template.Spec.Containers { + if sts.Spec.Template.Spec.Containers[i].Name == "openclaw" { + main = &sts.Spec.Template.Spec.Containers[i] + } + } + if main == nil { + t.Fatal("openclaw container not found") + } + + var tmpMounts int + for _, m := range main.VolumeMounts { + if m.MountPath != "/tmp" { + continue + } + tmpMounts++ + if m.Name != "tmp" { + t.Errorf("/tmp volume = %q, want tmp", m.Name) + } + if m.SubPath != MainTmpSubPath { + t.Errorf("/tmp subPath = %q, want %q: mounting the emptyDir root exposes its 2777 mode", m.SubPath, MainTmpSubPath) + } + } + if tmpMounts != 1 { + t.Fatalf("main container has %d mounts at /tmp, want 1", tmpMounts) + } + + tmpVol := findVolume(sts.Spec.Template.Spec.Volumes, "tmp") + if tmpVol == nil || tmpVol.EmptyDir == nil { + t.Fatal("tmp volume should be an emptyDir") + } +} + +func TestBuildStatefulSet_TmpDirInitContainer(t *testing.T) { + instance := newTestInstance("sticky-tmp-init") + sts := BuildStatefulSet(instance, "", nil, nil, nil) + _, c := findInitContainer(t, sts.Spec.Template.Spec.InitContainers, TmpDirInitContainerName) + + if len(c.Command) != 3 { + t.Fatalf("command = %v, want sh -c