diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 02a736b0..b8637f3a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -151,6 +151,8 @@ jobs: with: go-version: ${{ env.GO_VERSION }} + - uses: azure/setup-helm@v4 + - uses: actions/setup-node@v4 with: node-version: '22' diff --git a/.github/workflows/release-please.yaml b/.github/workflows/release-please.yaml index 29119f74..0f6d65dc 100644 --- a/.github/workflows/release-please.yaml +++ b/.github/workflows/release-please.yaml @@ -10,15 +10,35 @@ permissions: jobs: release-please: runs-on: ubuntu-latest + # Only needed by the GITHUB_TOKEN fallback below; the App token and the + # PAT carry their own permissions. + permissions: + contents: write + pull-requests: write + actions: write + env: + # Credential precedence: GitHub App token > RELEASE_PLEASE_TOKEN PAT > + # GITHUB_TOKEN. Events created with GITHUB_TOKEN do not trigger other + # workflows, so with the fallback the release PR gets no CI run and the + # tag does not start release.yaml on its own (dispatched explicitly below). + HAS_APP: ${{ vars.RELEASE_BOT_APP_ID != '' }} + HAS_PAT: ${{ secrets.RELEASE_PLEASE_TOKEN != '' }} steps: - # Mint a short-lived token (~1h) from the paperclip-release-bot GitHub App. - # Avoids long-lived PATs that the paperclipinc org policy now blocks. + # Mint a short-lived token (~1h) from the release-bot GitHub App. + # Skipped when the App is not configured for this repository (e.g. right + # after a repository transfer, see #625) instead of failing the run. - uses: actions/create-github-app-token@v1 id: app-token + if: vars.RELEASE_BOT_APP_ID != '' with: app-id: ${{ vars.RELEASE_BOT_APP_ID }} private-key: ${{ secrets.RELEASE_BOT_PRIVATE_KEY }} + - name: Warn about fallback credentials + if: env.HAS_APP != 'true' && env.HAS_PAT != 'true' + run: | + echo "::warning::Neither vars.RELEASE_BOT_APP_ID nor secrets.RELEASE_PLEASE_TOKEN is set; using GITHUB_TOKEN. The release PR will not get CI runs automatically (push an empty commit to it or close/reopen it), and release.yaml is dispatched explicitly after tagging." + - uses: googleapis/release-please-action@v4 id: release with: @@ -26,7 +46,7 @@ jobs: manifest-file: .release-please-manifest.json # App token makes the tag push appear as a user action, # which triggers downstream release.yaml. GITHUB_TOKEN cannot. - token: ${{ steps.app-token.outputs.token }} + token: ${{ steps.app-token.outputs.token || secrets.RELEASE_PLEASE_TOKEN || github.token }} # skip-github-release is set in release-please-config.json so that # release-please does NOT create a GitHub release (which would @@ -36,7 +56,7 @@ jobs: # tag manually below, and flip stale labels in a separate step. - name: Create release tag env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} + GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.RELEASE_PLEASE_TOKEN || github.token }} run: | # Detect if the current push merged a release-please PR. # Check both the commit message (squash merge) and the merged @@ -67,7 +87,13 @@ jobs: gh api "repos/${{ github.repository }}/git/refs" \ -f ref="refs/tags/${TAG}" \ -f sha="${{ github.sha }}" - echo "Tag ${TAG} created - release workflow will trigger" + echo "Tag ${TAG} created" + if [[ "${HAS_APP}" != "true" && "${HAS_PAT}" != "true" ]]; then + # A tag pushed with GITHUB_TOKEN does not fire release.yaml's + # push trigger, but a workflow_dispatch from GITHUB_TOKEN does. + echo "Dispatching release.yaml for ${TAG}" + gh workflow run release.yaml --repo "${{ github.repository }}" --ref "${TAG}" + fi fi else echo "Not a release commit, skipping tag creation" @@ -81,7 +107,7 @@ jobs: # label gets fixed even if the tag step above was skipped or failed. - name: Fix stale autorelease labels env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} + GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.RELEASE_PLEASE_TOKEN || github.token }} run: | for STATE in merged closed; do STALE_PRS=$(gh pr list \ diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 56737fe5..1c4fe38f 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -4,26 +4,44 @@ on: push: tags: - 'v*' + # Release Please dispatches this with --ref when it had to fall back to + # GITHUB_TOKEN, whose tag pushes do not fire the push trigger above. Run it + # manually the same way: gh workflow run release.yaml --ref vX.Y.Z + workflow_dispatch: env: REGISTRY: ghcr.io + # Single source of truth for the GHCR namespace the image and the chart are + # published to. It must match the chart's default image.repository (guarded + # by hack/check-chart-image-repository.sh), the bundle CSV and + # operatorhub.yaml, so it is pinned rather than derived from the repository + # owner (which changed when the repository moved to stubbi, see #625). + IMAGE_NAMESPACE: ${{ vars.IMAGE_NAMESPACE || 'paperclipinc' }} jobs: release: name: Release runs-on: ubuntu-latest + if: startsWith(github.ref, 'refs/tags/v') permissions: contents: write packages: write id-token: write # For signing + actions: write # GITHUB_TOKEN fallback dispatches operatorhub.yaml + env: + HAS_APP: ${{ vars.RELEASE_BOT_APP_ID != '' }} + HAS_PAT: ${{ secrets.RELEASE_PLEASE_TOKEN != '' }} steps: # Mint a short-lived App installation token to use for actions that # need to trigger downstream workflows (release.yaml's events from # GITHUB_TOKEN don't fan-out). Replaces the long-lived # RELEASE_PLEASE_TOKEN PAT that was previously stored as a secret. + # Skipped when the App is not configured (see #625); the publish step + # then falls back to RELEASE_PLEASE_TOKEN or GITHUB_TOKEN. - uses: actions/create-github-app-token@v1 id: app-token + if: vars.RELEASE_BOT_APP_ID != '' with: app-id: ${{ vars.RELEASE_BOT_APP_ID }} private-key: ${{ secrets.RELEASE_BOT_PRIVATE_KEY }} @@ -45,12 +63,25 @@ jobs: uses: docker/login-action@v3 with: registry: ${{ env.REGISTRY }} - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} + # GITHUB_TOKEN can only push to packages this repository owns or + # has been granted access to. To publish into a namespace owned by + # another account (ghcr.io/paperclipinc after the transfer), set + # GHCR_PUSH_TOKEN to a PAT with write:packages from a member of that + # namespace and GHCR_PUSH_USER to its login. + username: ${{ vars.GHCR_PUSH_USER || github.actor }} + password: ${{ secrets.GHCR_PUSH_TOKEN || secrets.GITHUB_TOKEN }} - name: Install Cosign uses: sigstore/cosign-installer@v3 + - name: Check image namespace matches the chart default + run: | + CHART_REPO=$(yq '.image.repository' charts/openclaw-operator/values.yaml) + if [ "${CHART_REPO}" != "ghcr.io/${IMAGE_NAMESPACE}/openclaw-operator" ]; then + echo "::error::IMAGE_NAMESPACE=${IMAGE_NAMESPACE} but the chart defaults to ${CHART_REPO}; publishing would ship a chart pointing at an image that was never pushed (#536)." + exit 1 + fi + - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 with: @@ -58,12 +89,11 @@ jobs: args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # IMAGE_NAMESPACE (workflow env) is read by .goreleaser.yaml. - name: Docker image name id: docker-image - run: | - OWNER="${GITHUB_REPOSITORY_OWNER,,}" - echo "name=${{ env.REGISTRY }}/${OWNER}/openclaw-operator" >> $GITHUB_OUTPUT + run: echo "name=${{ env.REGISTRY }}/${IMAGE_NAMESPACE}/openclaw-operator" >> "$GITHUB_OUTPUT" - name: Sign container images run: | @@ -107,11 +137,17 @@ jobs: - name: Publish release env: - # PAT required: events from GITHUB_TOKEN don't trigger other - # workflows. Using PAT ensures the release:published event - # fires the OperatorHub submission workflow. - GH_TOKEN: ${{ steps.app-token.outputs.token }} - run: gh release edit "${{ github.ref_name }}" --draft=false + # Events from GITHUB_TOKEN don't trigger other workflows. The App + # token (or PAT) makes release:published fire the OperatorHub + # submission workflow; with the GITHUB_TOKEN fallback it is + # dispatched explicitly instead. + GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.RELEASE_PLEASE_TOKEN || secrets.GITHUB_TOKEN }} + run: | + gh release edit "${{ github.ref_name }}" --draft=false + if [[ "${HAS_APP}" != "true" && "${HAS_PAT}" != "true" ]]; then + echo "::warning::Published with GITHUB_TOKEN; dispatching operatorhub.yaml explicitly." + gh workflow run operatorhub.yaml --repo "${{ github.repository }}" -f tag="${{ github.ref_name }}" + fi helm-release: name: Helm Chart Release @@ -126,10 +162,6 @@ jobs: - name: Checkout uses: actions/checkout@v4 - - name: Lowercase owner name - id: owner - run: echo "name=${GITHUB_REPOSITORY_OWNER,,}" >> $GITHUB_OUTPUT - - name: Install Helm uses: azure/setup-helm@v4 @@ -138,19 +170,24 @@ jobs: - name: Extract version id: version - run: echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT + run: echo "version=${GITHUB_REF#refs/tags/v}" >> "$GITHUB_OUTPUT" - name: Login to GHCR uses: docker/login-action@v3 with: registry: ghcr.io - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} + # GITHUB_TOKEN can only push to packages this repository owns or + # has been granted access to. To publish into a namespace owned by + # another account (ghcr.io/paperclipinc after the transfer), set + # GHCR_PUSH_TOKEN to a PAT with write:packages from a member of that + # namespace and GHCR_PUSH_USER to its login. + username: ${{ vars.GHCR_PUSH_USER || github.actor }} + password: ${{ secrets.GHCR_PUSH_TOKEN || secrets.GITHUB_TOKEN }} - name: Package, push, and sign Helm chart run: | VERSION="${{ steps.version.outputs.version }}" - OWNER="${{ steps.owner.outputs.name }}" + OWNER="${IMAGE_NAMESPACE}" helm package charts/openclaw-operator --version "${VERSION}" --app-version "${VERSION}" PUSH_OUT=$(helm push "openclaw-operator-${VERSION}.tgz" "oci://ghcr.io/${OWNER}/charts" 2>&1) echo "${PUSH_OUT}" diff --git a/.github/workflows/verify-signing.yaml b/.github/workflows/verify-signing.yaml index d3b4d2ca..7712eaa4 100644 --- a/.github/workflows/verify-signing.yaml +++ b/.github/workflows/verify-signing.yaml @@ -26,6 +26,7 @@ jobs: id: verify env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + IMAGE_NAMESPACE: ${{ vars.IMAGE_NAMESPACE || 'paperclipinc' }} run: | set +e make verify-signing diff --git a/.goreleaser.yaml b/.goreleaser.yaml index f3e83939..af759730 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -25,7 +25,8 @@ dockers_v2: ids: - manager images: - - "ghcr.io/paperclipinc/openclaw-operator" + # Set by release.yaml (vars.IMAGE_NAMESPACE, default paperclipinc). + - 'ghcr.io/{{ envOrDefault "IMAGE_NAMESPACE" "paperclipinc" }}/openclaw-operator' tags: - "{{ .Tag }}" - "{{ .Major }}.{{ .Minor }}" @@ -70,5 +71,5 @@ release: draft: true replace_existing_artifacts: true github: - owner: paperclipinc + owner: stubbi name: openclaw-operator diff --git a/CLAUDE.md b/CLAUDE.md index 8e053ba2..8dfd532a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,8 +210,13 @@ Automated via release-please + GoReleaser: - `.release-please-manifest.json` — tracks current version - `.goreleaser.yaml` — `release.draft: true` (immutable releases require draft→publish flow) -**Secrets required:** -- `RELEASE_PLEASE_TOKEN` — classic PAT with `repo` scope; used for tag creation, release publishing, and OperatorHub cross-repo PRs +**Credentials (repository settings):** +- `vars.RELEASE_BOT_APP_ID` + `secrets.RELEASE_BOT_PRIVATE_KEY` — GitHub App used by release-please and for publishing the release, so the tag push and `release: published` trigger `release.yaml` and `operatorhub.yaml`. Preferred. +- `secrets.RELEASE_PLEASE_TOKEN` — PAT fallback when the App is not configured. +- Neither set: both workflows fall back to `GITHUB_TOKEN` and dispatch `release.yaml` / `operatorhub.yaml` explicitly. The release PR then gets no automatic CI run (push an empty commit to it). +- `vars.IMAGE_NAMESPACE` (default `paperclipinc`) — GHCR namespace for the image and the chart. Must match the chart's default `image.repository` (`release.yaml` fails early otherwise). +- `secrets.GHCR_PUSH_TOKEN` + `vars.GHCR_PUSH_USER` — needed when `IMAGE_NAMESPACE` is owned by another account than the repository (the repository moved to `stubbi`; the images still live under `paperclipinc`). +- `secrets.OPERATORHUB_FORK_TOKEN` — PAT for the OperatorHub cross-repo PRs. **Manual trigger:** `gh workflow run "OperatorHub Submission" -f tag=vX.Y.Z` diff --git a/Makefile b/Makefile index 6e99f38b..36794321 100644 --- a/Makefile +++ b/Makefile @@ -133,17 +133,22 @@ conformance-gitops: ## Run the GitOps coexistence conformance category. conformance-failure: ## Run the failure-modes conformance category. cd test/conformance && go test -v -timeout 20m -ginkgo.v -ginkgo.focus="failure modes" ./... +# Releases signed before the repository moved from paperclipinc to stubbi carry +# the old workflow identity, later ones the new one; accept both. +SIGNING_IDENTITY_REGEXP ?= ^https://github.com/(paperclipinc|stubbi)/openclaw-operator/.github/workflows/.* +IMAGE_NAMESPACE ?= paperclipinc + .PHONY: verify-signing verify-signing: ## Verify the latest published release is Cosign-signed and SBOM-attested. - @VERSION=$$(gh release view --repo paperclipinc/openclaw-operator --json tagName --jq .tagName); \ - IMAGE="ghcr.io/paperclipinc/openclaw-operator:$${VERSION}"; \ + @VERSION=$$(gh release view --repo stubbi/openclaw-operator --json tagName --jq .tagName); \ + IMAGE="ghcr.io/$(IMAGE_NAMESPACE)/openclaw-operator:$${VERSION}"; \ echo "Verifying $${IMAGE}..."; \ cosign verify "$${IMAGE}" \ - --certificate-identity-regexp 'https://github.com/paperclipinc/openclaw-operator/.github/workflows/.*' \ + --certificate-identity-regexp '$(SIGNING_IDENTITY_REGEXP)' \ --certificate-oidc-issuer https://token.actions.githubusercontent.com >/dev/null || { echo "::error::signature verification failed for $${IMAGE}"; exit 1; }; \ echo "Verifying SBOM attestation..."; \ cosign verify-attestation "$${IMAGE}" --type spdxjson \ - --certificate-identity-regexp 'https://github.com/paperclipinc/openclaw-operator/.github/workflows/.*' \ + --certificate-identity-regexp '$(SIGNING_IDENTITY_REGEXP)' \ --certificate-oidc-issuer https://token.actions.githubusercontent.com >/dev/null || { echo "::error::SBOM attestation verification failed for $${IMAGE}"; exit 1; }; \ echo "OK: $${IMAGE} is signed and SBOM-attested." diff --git a/README.md b/README.md index 75d0185d..3ab546bd 100644 --- a/README.md +++ b/README.md @@ -1005,7 +1005,17 @@ 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 gateway's effective UID and the pod's `runAsGroup` when it differs, and exits without touching anything when ownership is already correct. The effective UID uses `security.containerSecurityContext.runAsUser` when set, otherwise the pod UID. Operator-managed init containers that write to the data volume use the same UID override, so they can access the directory after the gateway tightens its permissions. The ownership helper does not modify existing children; changing the runtime UID of an existing installation may require an out-of-band migration of those files. User-defined init containers retain their configured identities. -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. + +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 | +|-------|----------| +| 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: @@ -1156,7 +1166,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) and canvas (18793) ports, plus the MCP Apps sandbox (18795) port when MCP Apps is enabled (see below). To expose custom ports instead (e.g., for a non-default application), set `spec.networking.service.ports`: ```yaml spec: @@ -1171,6 +1181,27 @@ 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`. A port you set yourself wins. +- With the gateway proxy enabled (the default), the proxy sidecar forwards pod port `18796` to the loopback sandbox listener. With `spec.gateway.enabled: false` the Service targets the sandbox listener port directly. +- 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. +- These ports are only added when MCP Apps is enabled in `spec.config.raw`, so instances that do not use MCP Apps keep their pod template, Service and NetworkPolicy unchanged. The operator cannot see the effective config when it comes from `spec.config.configMapRef` or when `mergeMode: merge` lets runtime changes on the PVC enable MCP Apps, so in those cases the sandbox ports are always added. + +```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, the proxy, the Service and the NetworkPolicy follow it. One exception: with `spec.config.configMapRef` and the gateway proxy disabled, the operator cannot read your port and the Service targets `18795`, so keep the default there. With `spec.networking.service.ports` set, add the `mcp-apps` port (target port `18796`, or your sandbox port when the proxy is disabled) to your list explicitly. + ### CA bundle injection Inject a custom CA certificate bundle for environments with TLS-intercepting proxies or private CAs: @@ -1306,7 +1337,7 @@ The operator follows a **secure-by-default** philosophy. Every instance ships wi ### Defaults - **Non-root execution**: containers run as UID 1000; root (UID 0) is blocked by the validating webhook (exception: Ollama sidecar requires root per the official image) -- **Read-only root filesystem**: enabled by default for the main container and the Chromium sidecar; the PVC at `~/.openclaw/` provides writable home, and a `/tmp` emptyDir handles temp files +- **Read-only root filesystem**: enabled by default for the main container and the Chromium sidecar; the PVC at `~/.openclaw/` provides writable home, and a `/tmp` emptyDir handles temp files. The `init-tmp-dir` init container gives that `/tmp` the sticky bit (mode `1777`), which OpenClaw 2026.9.6 and later require before they create temporary workspaces; the root of an emptyDir is `2777` under `fsGroup` and cannot be changed from the pod spec - **All capabilities dropped**: no ambient Linux capabilities - **Seccomp RuntimeDefault**: syscall filtering enabled - **Default-deny NetworkPolicy**: DNS (53) and HTTPS (443) egress allowed by default; set `security.networkPolicy.allowHTTPS: false` and use `additionalEgress` to restrict HTTPS destinations (see [Network Isolation](docs/architecture.md#network-isolation)). Ingress is limited to the same namespace. Metrics ingress is a separate rule that can be restricted independently -- see [Restricting who can scrape metrics](#restricting-who-can-scrape-metrics) diff --git a/api/v1alpha1/openclawinstance_types.go b/api/v1alpha1/openclawinstance_types.go index 05f8f90e..d44cff9b 100644 --- a/api/v1alpha1/openclawinstance_types.go +++ b/api/v1alpha1/openclawinstance_types.go @@ -746,10 +746,15 @@ 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 gateway 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. + // 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-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 20f738f5..aea6106f 100644 --- a/bundle/manifests/openclaw.rocks_openclawinstances.yaml +++ b/bundle/manifests/openclaw.rocks_openclawinstances.yaml @@ -9790,7 +9790,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 gateway's effective UID @@ -9801,9 +9800,15 @@ 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 gateway 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. + 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/_helpers.tpl b/charts/openclaw-operator/templates/_helpers.tpl index 3f07b29e..941a0d8a 100644 --- a/charts/openclaw-operator/templates/_helpers.tpl +++ b/charts/openclaw-operator/templates/_helpers.tpl @@ -159,6 +159,13 @@ hack/check-helm-rbac-sync.sh parses this define alongside managerRules so the union is asserted to be a superset of config/rbac/role.yaml. */}} {{- define "openclaw-operator.clusterScopedRules" -}} +# Read the instance namespace's Pod Security labels (storage.fixOwnership default) +- apiGroups: [""] + resources: ["namespaces"] + verbs: ["get"] +{{- if .Values.watchNamespaces }} + resourceNames: {{ .Values.watchNamespaces | toJson }} +{{- end }} # OpenClawClusterDefaults singleton (#457) - apiGroups: ["openclaw.rocks"] resources: ["openclawclusterdefaults"] diff --git a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml index 0200636b..d1ad8d43 100644 --- a/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml +++ b/charts/openclaw-operator/templates/crds/openclaw.rocks_openclawinstances.yaml @@ -9796,7 +9796,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 gateway's effective UID @@ -9807,9 +9806,15 @@ 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 gateway 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. + 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/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 { diff --git a/config/crd/bases/openclaw.rocks_openclawinstances.yaml b/config/crd/bases/openclaw.rocks_openclawinstances.yaml index 20f738f5..aea6106f 100644 --- a/config/crd/bases/openclaw.rocks_openclawinstances.yaml +++ b/config/crd/bases/openclaw.rocks_openclawinstances.yaml @@ -9790,7 +9790,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 gateway's effective UID @@ -9801,9 +9800,15 @@ 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 gateway 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. + 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/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: diff --git a/docs/api-reference.md b/docs/api-reference.md index ef351700..ec4bee61 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1371,7 +1371,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 gateway's effective UID
(container runAsUser override, otherwise pod runAsUser) and pod 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 gateway 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. | true | Optional: \{\}
| +| `fixOwnership` _boolean_ | FixOwnership runs the operator-managed init-data-owner init container,
which chowns the root of the data volume to the gateway's effective UID
(container runAsUser override, otherwise pod runAsUser) and pod 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 gateway 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/docs/architecture.md b/docs/architecture.md index 7ad4beaf..412fab1e 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) and 18793 (canvas) by default, plus the MCP Apps sandbox port when MCP Apps is enabled. 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`). diff --git a/hack/check-helm-rbac-sync.sh b/hack/check-helm-rbac-sync.sh index 92e84b2d..8b691e42 100755 --- a/hack/check-helm-rbac-sync.sh +++ b/hack/check-helm-rbac-sync.sh @@ -72,7 +72,8 @@ parse_generated() { parse_helm() { awk ' /\{\{-? *define .openclaw-operator\.(managerRules|clusterScopedRules). *-?\}\}/ { in_rules = 1; next } - in_rules && /\{\{-? *end *-?\}\}/ { in_rules = 0; next } + in_rules && /\{\{-? *end *-?\}\}/ { if (depth > 0) { depth--; next }; in_rules = 0; next } + in_rules && /\{\{-? *if / { depth++; next } !in_rules { next } /^\s*#/ { next } 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..53b0f731 --- /dev/null +++ b/internal/controller/data_owner_pod_security_test.go @@ -0,0 +1,191 @@ +/* +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 ( + "context" + "errors" + "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" + "sigs.k8s.io/controller-runtime/pkg/client" + + 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") + }) + + 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 f5b87118..4cff34f0 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,41 @@ 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. +// +// 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 nil + } + logger := log.FromContext(ctx) + + ns := &corev1.Namespace{} + if err := r.APIReader.Get(ctx, types.NamespacedName{Name: instance.Namespace}, ns); err != nil { + 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 func (r *OpenClawInstanceReconciler) reconcileResources(ctx context.Context, instance *openclawv1alpha1.OpenClawInstance) error { logger := log.FromContext(ctx) @@ -395,6 +436,13 @@ 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. 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 { 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()) diff --git a/internal/resources/common.go b/internal/resources/common.go index 2b3268bb..b0c83721 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. @@ -387,11 +398,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/configmap.go b/internal/resources/configmap.go index 15d21de0..bfe0e979 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 } @@ -111,7 +114,11 @@ func BuildConfigMapFromBytes(instance *openclawv1alpha1.OpenClawInstance, baseCo // Only include nginx config when the gateway proxy is enabled if IsGatewayProxyEnabled(instance) { - data[NginxConfigKey] = nginxStreamConfig() + var mcpAppsUpstream int32 + if sandbox := ResolveMcpAppsSandbox(instance); sandbox.Exposed { + mcpAppsUpstream = mcpAppsListenPortFromConfig(configBytes, sandbox.ListenPort) + } + data[NginxConfigKey] = nginxStreamConfig(mcpAppsUpstream) } // Mesh provider ConfigMap keys, e.g. the Tailscale serve config the sidecar @@ -506,6 +513,47 @@ 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). The port is pinned with the proxy disabled +// too, so the Service always has a known target port. +// +// 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 the config is returned unchanged (user +// override wins). +func enrichConfigWithMcpAppsSandboxPort(configJSON []byte, _ *openclawv1alpha1.OpenClawInstance) ([]byte, error) { + 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 @@ -703,10 +751,20 @@ func enrichConfigWithSkillPacks(configJSON []byte, skillEntries map[string]inter return json.Marshal(config) } -// nginxStreamConfig returns the static nginx stream configuration for the -// gateway reverse proxy sidecar. It proxies external traffic on dedicated -// ports to the gateway and canvas processes listening on loopback. -func nginxStreamConfig() string { +// nginxStreamConfig returns the nginx stream configuration for the gateway +// reverse proxy sidecar. It proxies external traffic on dedicated ports to the +// gateway and canvas processes listening on loopback. When mcpAppsUpstream is +// non-zero it also forwards McpAppsSandboxProxyPort to the MCP Apps sandbox +// listener on that loopback port (#615). +func nginxStreamConfig(mcpAppsUpstream int32) string { + mcpAppsServer := "" + if mcpAppsUpstream > 0 { + mcpAppsServer = fmt.Sprintf(` + server { + listen 0.0.0.0:%d; + proxy_pass 127.0.0.1:%d; + }`, McpAppsSandboxProxyPort, mcpAppsUpstream) + } return fmt.Sprintf(`worker_processes 1; pid /tmp/nginx.pid; error_log /dev/stderr warn; @@ -723,7 +781,7 @@ stream { server { listen 0.0.0.0:%d; proxy_pass 127.0.0.1:%d; - } + }%s } -`, GatewayProxyPort, GatewayPort, CanvasProxyPort, CanvasPort) +`, GatewayProxyPort, GatewayPort, CanvasProxyPort, CanvasPort, mcpAppsServer) } diff --git a/internal/resources/data_owner_test.go b/internal/resources/data_owner_test.go index 583b8292..d42e7799 100644 --- a/internal/resources/data_owner_test.go +++ b/internal/resources/data_owner_test.go @@ -251,3 +251,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) + } + }) + } +} diff --git a/internal/resources/mcp_apps.go b/internal/resources/mcp_apps.go new file mode 100644 index 00000000..84898f33 --- /dev/null +++ b/internal/resources/mcp_apps.go @@ -0,0 +1,102 @@ +/* +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" + + openclawv1alpha1 "github.com/paperclipinc/openclaw-operator/api/v1alpha1" +) + +// McpAppsSandbox describes how the operator exposes the OpenClaw MCP Apps +// sandbox listener (#615). +type McpAppsSandbox struct { + // Exposed is true when the operator renders the sandbox ports (container + // ports, nginx listener, Service port and NetworkPolicy ingress). + Exposed bool + // ListenPort is the port the OpenClaw sandbox listener binds to: the + // user's mcp.apps.sandboxPort when set, otherwise McpAppsSandboxPort, + // which config enrichment pins. + ListenPort int32 +} + +// ResolveMcpAppsSandbox decides whether the MCP Apps sandbox is exposed and +// which port its listener uses. +// +// The operator can only see the inline config (spec.config.raw). When the +// config comes from spec.config.configMapRef, or mergeMode=merge lets runtime +// changes on the PVC enable MCP Apps, the operator cannot tell whether MCP +// Apps is on, so it keeps exposing the sandbox ports. Otherwise the ports are +// only rendered when the inline config sets mcp.apps.enabled=true, so +// instances that do not use MCP Apps see no pod template, Service or +// NetworkPolicy change. +func ResolveMcpAppsSandbox(instance *openclawv1alpha1.OpenClawInstance) McpAppsSandbox { + enabled, port := mcpAppsFromRawConfig(instance) + result := McpAppsSandbox{Exposed: enabled, ListenPort: McpAppsSandboxPort} + if port > 0 { + result.ListenPort = port + } + if instance.Spec.Config.ConfigMapRef != nil || instance.Spec.Config.MergeMode == ConfigMergeModeMerge { + result.Exposed = true + } + return result +} + +// mcpAppsFromRawConfig reports mcp.apps.enabled and mcp.apps.sandboxPort +// (0 when unset or not a valid port) from the inline config. +func mcpAppsFromRawConfig(instance *openclawv1alpha1.OpenClawInstance) (enabled bool, port int32) { + if instance.Spec.Config.Raw == nil || len(instance.Spec.Config.Raw.Raw) == 0 { + return false, 0 + } + var config map[string]interface{} + if err := json.Unmarshal(instance.Spec.Config.Raw.Raw, &config); err != nil { + return false, 0 + } + return mcpAppsSettings(config) +} + +// mcpAppsSettings extracts mcp.apps.enabled and mcp.apps.sandboxPort from a +// parsed OpenClaw config. +func mcpAppsSettings(config map[string]interface{}) (enabled bool, port int32) { + mcp, _ := config["mcp"].(map[string]interface{}) + if mcp == nil { + return false, 0 + } + apps, _ := mcp["apps"].(map[string]interface{}) + if apps == nil { + return false, 0 + } + enabled, _ = apps["enabled"].(bool) + if p, ok := apps["sandboxPort"].(float64); ok && p >= 1 && p <= 65535 && p == float64(int32(p)) { + port = int32(p) + } + return enabled, port +} + +// mcpAppsListenPortFromConfig returns the sandbox listener port from the +// final (enriched) config bytes, falling back to fallback. Used for the nginx +// upstream so that a port set in a configMapRef config is honored too. +func mcpAppsListenPortFromConfig(configJSON []byte, fallback int32) int32 { + var config map[string]interface{} + if err := json.Unmarshal(configJSON, &config); err != nil { + return fallback + } + if _, port := mcpAppsSettings(config); port > 0 { + return port + } + return fallback +} diff --git a/internal/resources/mcp_apps_test.go b/internal/resources/mcp_apps_test.go new file mode 100644 index 00000000..b5ae03af --- /dev/null +++ b/internal/resources/mcp_apps_test.go @@ -0,0 +1,317 @@ +/* +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" + + corev1 "k8s.io/api/core/v1" + "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 still pins the operator port", raw: `{"mcp":{"apps":{"enabled":true}}}`, proxyDisabled: true, wantApps: true, wantPort: float64(McpAppsSandboxPort)}, + {name: "proxy disabled keeps the user port", raw: `{"mcp":{"apps":{"enabled":true,"sandboxPort":19001}}}`, proxyDisabled: true, wantApps: true, wantPort: float64(19001)}, + } { + 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 withRawConfig(instance *openclawv1alpha1.OpenClawInstance, raw string) *openclawv1alpha1.OpenClawInstance { + instance.Spec.Config.Raw = &openclawv1alpha1.RawConfig{ + RawExtension: runtime.RawExtension{Raw: []byte(raw)}, + } + return instance +} + +const ( + mcpAppsEnabledRaw = `{"mcp":{"apps":{"enabled":true}}}` + mcpAppsUserPortRaw = `{"mcp":{"apps":{"enabled":true,"sandboxPort":19001}}}` +) + +// nginxSandboxBlock returns the server block that listens on the sandbox proxy +// port, or "" when there is none. +func nginxSandboxBlock(conf string) string { + listen := fmt.Sprintf("listen 0.0.0.0:%d;", McpAppsSandboxProxyPort) + i := strings.Index(conf, listen) + if i < 0 { + return "" + } + block := conf[i:] + if end := strings.Index(block, "}"); end >= 0 { + block = block[:end] + } + return block +} + +func TestResolveMcpAppsSandbox(t *testing.T) { + for _, tt := range []struct { + name string + raw string + configMap bool + merge bool + wantExposed bool + wantPort int32 + }{ + {name: "no config", wantPort: McpAppsSandboxPort}, + {name: "apps not configured", raw: `{"gateway":{}}`, wantPort: McpAppsSandboxPort}, + {name: "apps disabled", raw: `{"mcp":{"apps":{"enabled":false}}}`, wantPort: McpAppsSandboxPort}, + {name: "apps enabled", raw: mcpAppsEnabledRaw, wantExposed: true, wantPort: McpAppsSandboxPort}, + {name: "apps enabled with user port", raw: mcpAppsUserPortRaw, wantExposed: true, wantPort: 19001}, + {name: "invalid user port falls back", raw: `{"mcp":{"apps":{"enabled":true,"sandboxPort":70000}}}`, wantExposed: true, wantPort: McpAppsSandboxPort}, + {name: "configMapRef cannot be inspected", configMap: true, wantExposed: true, wantPort: McpAppsSandboxPort}, + {name: "mergeMode=merge may enable apps at runtime", raw: `{}`, merge: true, wantExposed: true, wantPort: McpAppsSandboxPort}, + } { + t.Run(tt.name, func(t *testing.T) { + instance := newTestInstance("mcp-apps-resolve") + if tt.raw != "" { + withRawConfig(instance, tt.raw) + } + if tt.configMap { + instance.Spec.Config.ConfigMapRef = &openclawv1alpha1.ConfigMapKeySelector{Name: "external"} + } + if tt.merge { + instance.Spec.Config.MergeMode = ConfigMergeModeMerge + } + got := ResolveMcpAppsSandbox(instance) + if got.Exposed != tt.wantExposed || got.ListenPort != tt.wantPort { + t.Errorf("ResolveMcpAppsSandbox() = %+v, want exposed=%v port=%d", got, tt.wantExposed, tt.wantPort) + } + }) + } +} + +func TestNginxStreamConfig_McpAppsSandbox(t *testing.T) { + if block := nginxSandboxBlock(nginxStreamConfig(0)); block != "" { + t.Errorf("sandbox server block rendered without an upstream:\n%s", block) + } + + block := nginxSandboxBlock(nginxStreamConfig(McpAppsSandboxPort)) + if block == "" { + t.Fatal("nginx config does not listen on the sandbox proxy port") + } + if pass := fmt.Sprintf("proxy_pass 127.0.0.1:%d;", McpAppsSandboxPort); !strings.Contains(block, pass) { + t.Errorf("sandbox proxy server block does not forward to %d:\n%s", McpAppsSandboxPort, block) + } +} + +func TestBuildConfigMap_NginxMcpAppsUpstream(t *testing.T) { + for _, tt := range []struct { + name string + raw string + wantUpstream int32 // 0 means no sandbox server block + }{ + {name: "apps not enabled", raw: `{}`}, + {name: "apps enabled", raw: mcpAppsEnabledRaw, wantUpstream: McpAppsSandboxPort}, + {name: "user port", raw: mcpAppsUserPortRaw, wantUpstream: 19001}, + } { + t.Run(tt.name, func(t *testing.T) { + instance := withRawConfig(newTestInstance("mcp-apps-nginx"), tt.raw) + block := nginxSandboxBlock(BuildConfigMap(instance, "", nil).Data[NginxConfigKey]) + if tt.wantUpstream == 0 { + if block != "" { + t.Errorf("unexpected sandbox server block:\n%s", block) + } + return + } + if pass := fmt.Sprintf("proxy_pass 127.0.0.1:%d;", tt.wantUpstream); !strings.Contains(block, pass) { + t.Errorf("sandbox server block does not forward to %d:\n%s", tt.wantUpstream, block) + } + }) + } + + // A port set in an external (configMapRef) config is honored by nginx. + instance := newTestInstance("mcp-apps-nginx-cmref") + instance.Spec.Config.ConfigMapRef = &openclawv1alpha1.ConfigMapKeySelector{Name: "external"} + cm := BuildConfigMapFromBytes(instance, []byte(mcpAppsUserPortRaw), "", nil) + if pass := "proxy_pass 127.0.0.1:19001;"; !strings.Contains(nginxSandboxBlock(cm.Data[NginxConfigKey]), pass) { + t.Errorf("configMapRef sandbox port not used as nginx upstream:\n%s", cm.Data[NginxConfigKey]) + } +} + +func containerByName(t *testing.T, instance *openclawv1alpha1.OpenClawInstance, name string) *corev1.Container { + t.Helper() + sts := BuildStatefulSet(instance, "", nil, nil, nil) + for i := range sts.Spec.Template.Spec.Containers { + if sts.Spec.Template.Spec.Containers[i].Name == name { + return &sts.Spec.Template.Spec.Containers[i] + } + } + return nil +} + +func hasContainerPortNamed(ports []corev1.ContainerPort, name string) bool { + for _, p := range ports { + if p.Name == name { + return true + } + } + return false +} + +func TestBuildStatefulSet_McpAppsContainerPorts(t *testing.T) { + instance := withRawConfig(newTestInstance("mcp-apps-sts"), mcpAppsEnabledRaw) + main := containerByName(t, instance, "openclaw") + proxy := containerByName(t, instance, "gateway-proxy") + if main == nil || proxy == nil { + t.Fatalf("expected openclaw and gateway-proxy containers") + } + assertContainerPort(t, main.Ports, "mcp-apps", McpAppsSandboxPort) + assertContainerPort(t, proxy.Ports, "mcp-apps-proxy", McpAppsSandboxProxyPort) + + // User port is declared on the main container. + withRawConfig(instance, mcpAppsUserPortRaw) + assertContainerPort(t, containerByName(t, instance, "openclaw").Ports, "mcp-apps", 19001) +} + +func TestBuildStatefulSet_McpAppsDisabledAddsNoPorts(t *testing.T) { + instance := newTestInstance("mcp-apps-sts-off") + if hasContainerPortNamed(containerByName(t, instance, "openclaw").Ports, "mcp-apps") { + t.Error("main container declares mcp-apps without MCP Apps enabled") + } + if hasContainerPortNamed(containerByName(t, instance, "gateway-proxy").Ports, "mcp-apps-proxy") { + t.Error("proxy container declares mcp-apps-proxy without MCP Apps enabled") + } +} + +func TestBuildService_McpAppsPort(t *testing.T) { + instance := withRawConfig(newTestInstance("mcp-apps-svc"), mcpAppsEnabledRaw) + assertServicePortWithTarget(t, BuildService(instance).Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), int32(McpAppsSandboxProxyPort)) + + // Proxy disabled: target the listener directly (review on #617). + instance.Spec.Gateway.Enabled = Ptr(false) + assertServicePortWithTarget(t, BuildService(instance).Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), int32(McpAppsSandboxPort)) + + // Proxy disabled with a user port: target the user port. + withRawConfig(instance, mcpAppsUserPortRaw) + assertServicePortWithTarget(t, BuildService(instance).Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), 19001) + + // Proxy enabled with a user port: still through nginx. + instance.Spec.Gateway.Enabled = nil + assertServicePortWithTarget(t, BuildService(instance).Spec.Ports, "mcp-apps", int32(McpAppsSandboxPort), int32(McpAppsSandboxProxyPort)) +} + +func TestBuildService_McpAppsDisabledAddsNoPort(t *testing.T) { + for _, p := range BuildService(newTestInstance("mcp-apps-svc-off")).Spec.Ports { + if p.Name == "mcp-apps" { + t.Fatalf("Service has an mcp-apps port without MCP Apps enabled: %+v", p) + } + } +} + +func TestBuildNetworkPolicy_McpAppsIngressPort(t *testing.T) { + instance := withRawConfig(newTestInstance("mcp-apps-np"), mcpAppsEnabledRaw) + assertNPPort(t, networkPolicyIngressPorts(instance), McpAppsSandboxProxyPort) + + instance.Spec.Gateway.Enabled = Ptr(false) + assertNPPort(t, networkPolicyIngressPorts(instance), McpAppsSandboxPort) + + withRawConfig(instance, mcpAppsUserPortRaw) + assertNPPort(t, networkPolicyIngressPorts(instance), 19001) +} + +func TestBuildNetworkPolicy_McpAppsDisabledAddsNoPort(t *testing.T) { + for _, p := range networkPolicyIngressPorts(newTestInstance("mcp-apps-np-off")) { + if p.Port != nil && (p.Port.IntValue() == McpAppsSandboxProxyPort || p.Port.IntValue() == McpAppsSandboxPort) { + t.Fatalf("NetworkPolicy allows the sandbox port without MCP Apps enabled: %+v", p) + } + } +} diff --git a/internal/resources/networkpolicy.go b/internal/resources/networkpolicy.go index 6ae55897..4fe310c1 100644 --- a/internal/resources/networkpolicy.go +++ b/internal/resources/networkpolicy.go @@ -100,6 +100,18 @@ func networkPolicyIngressPorts(instance *openclawv1alpha1.OpenClawInstance) []ne }, } + // MCP Apps sandbox (#615): same target port as the Service uses. + if sandbox := ResolveMcpAppsSandbox(instance); sandbox.Exposed { + mcpAppsPort := int32(McpAppsSandboxProxyPort) + if !IsGatewayProxyEnabled(instance) { + mcpAppsPort = sandbox.ListenPort + } + ports = append(ports, networkingv1.NetworkPolicyPort{ + Protocol: Ptr(corev1.ProtocolTCP), + Port: Ptr(intstr.FromInt32(mcpAppsPort)), + }) + } + if instance.Spec.WebTerminal.Enabled { ports = append(ports, networkingv1.NetworkPolicyPort{ Protocol: Ptr(corev1.ProtocolTCP), 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/service.go b/internal/resources/service.go index 72940a2f..77ed97ee 100644 --- a/internal/resources/service.go +++ b/internal/resources/service.go @@ -105,6 +105,21 @@ func buildServicePorts(instance *openclawv1alpha1.OpenClawInstance) []corev1.Ser }, } + // MCP Apps sandbox listener (#615). Through the proxy sidecar when it is + // enabled, otherwise straight to the listener's effective port. + if sandbox := ResolveMcpAppsSandbox(instance); sandbox.Exposed { + mcpAppsTarget := int32(McpAppsSandboxProxyPort) + if !IsGatewayProxyEnabled(instance) { + mcpAppsTarget = sandbox.ListenPort + } + ports = append(ports, corev1.ServicePort{ + Name: "mcp-apps", + Port: int32(McpAppsSandboxPort), + TargetPort: intstr.FromInt32(mcpAppsTarget), + Protocol: corev1.ProtocolTCP, + }) + } + if instance.Spec.Chromium.Enabled { ports = append(ports, corev1.ServicePort{ Name: "chromium", diff --git a/internal/resources/statefulset.go b/internal/resources/statefulset.go index 0d3c71f1..7a69834f 100644 --- a/internal/resources/statefulset.go +++ b/internal/resources/statefulset.go @@ -286,11 +286,11 @@ 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 +// Always includes gateway and canvas, plus the MCP Apps sandbox when it is +// exposed (see ResolveMcpAppsSandbox). 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{ + ports := []corev1.ContainerPort{ { Name: "gateway", ContainerPort: GatewayPort, @@ -302,6 +302,14 @@ func buildMainContainerPorts(instance *openclawv1alpha1.OpenClawInstance) []core Protocol: corev1.ProtocolTCP, }, } + if sandbox := ResolveMcpAppsSandbox(instance); sandbox.Exposed { + ports = append(ports, corev1.ContainerPort{ + Name: "mcp-apps", + ContainerPort: sandbox.ListenPort, + Protocol: corev1.ProtocolTCP, + }) + } + return ports } // buildMainContainer creates the main OpenClaw container @@ -337,9 +345,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, }, }, } @@ -674,6 +685,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)) @@ -1565,6 +1580,72 @@ 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) + + // Run as the gateway's effective UID so the directory belongs to the user + // that writes to it (and so it matches the other operator init containers; + // see #619). Falls back to the pod UID when no container UID is set. + mainSC := buildContainerSecurityContext(instance) + + 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), + RunAsUser: mainSC.RunAsUser, + 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" @@ -1639,6 +1720,35 @@ echo "data volume root ownership changed: $have -> $want"`, uid, gid) } } +// uvInitScript is the init-uv shell script. See buildUvInitContainer. +const uvInitScript = `set -e +mkdir -p /data/.local/bin /data/.cache /data/.config /data/skills +f=/data/.local/bin/uv +if [ -L "$f" ]; then + link=$(readlink "$f") + # Map gateway mount points back to /data. Relative targets resolve + # against the link's directory, which is the same in both containers. + case "$link" in + /home/openclaw/.local/*) target="/data/.local/${link#/home/openclaw/.local/}" ;; + /home/openclaw/.openclaw/*) target="/data/${link#/home/openclaw/.openclaw/}" ;; + /*) target="$link" ;; + *) target="/data/.local/bin/$link" ;; + esac + if [ -e "$target" ] || [ -L "$target" ]; then + echo "uv already installed (symlink to $link)" + exit 0 + fi + echo "replacing dangling uv symlink (target $link does not exist)" + rm -f "$f" +elif [ -x "$f" ]; then + echo "uv already installed" + exit 0 +fi +rm -f "$f.tmp" +cp /usr/local/bin/uv "$f.tmp" +mv -f "$f.tmp" "$f" +echo "uv $("$f" --version) installed"` + // buildUvInitContainer creates an init container that copies the uv binary from // the uv image into the PVC-backed ~/.local/bin/. This gives agents immediate // access to "uv pip install" for Python packages and "uv tool install" for CLI @@ -1652,17 +1762,21 @@ echo "data volume root ownership changed: $have -> $want"`, uid, gid) // ownership is not applied (e.g. Rancher local-path-provisioner on Talos). // Every downstream container that mounts these paths via SubPath inherits the // correct ownership from the pre-created directory. See #448. +// +// ~/.local/bin/uv may be a symlink that a user or agent created from the +// gateway container, for example to pin a newer uv under ~/.openclaw. Absolute +// targets use gateway paths, which do not exist here, so a link that is valid +// for the gateway would look dangling to this container. The script maps the +// two gateway mount points back to /data before deciding: a link whose target +// exists is kept, a link whose target is gone is replaced. The binary is +// copied to a temporary file and renamed over the destination, so the script +// never writes through a symlink and a dangling link cannot fail the pod +// start. See #626. func buildUvInitContainer(instance *openclawv1alpha1.OpenClawInstance) corev1.Container { - script := `set -e -mkdir -p /data/.local/bin /data/.cache /data/.config /data/skills -if [ -x /data/.local/bin/uv ]; then echo "uv already installed"; exit 0; fi -cp /usr/local/bin/uv /data/.local/bin/uv -echo "uv $(/data/.local/bin/uv --version) installed"` - return corev1.Container{ Name: "init-uv", Image: GetUVImage(instance), - Command: []string{"sh", "-c", script}, + Command: []string{"sh", "-c", uvInitScript}, ImagePullPolicy: corev1.PullIfNotPresent, TerminationMessagePath: corev1.TerminationMessagePathDefault, TerminationMessagePolicy: corev1.TerminationMessageReadFile, @@ -1987,22 +2101,31 @@ func buildTailscaleBinInitContainer(instance *openclawv1alpha1.OpenClawInstance) // buildGatewayProxyContainer creates the nginx reverse proxy sidecar that // exposes the loopback-bound gateway and canvas ports for external access. func buildGatewayProxyContainer(instance *openclawv1alpha1.OpenClawInstance) corev1.Container { + ports := []corev1.ContainerPort{ + { + Name: "gw-proxy", + ContainerPort: GatewayProxyPort, + Protocol: corev1.ProtocolTCP, + }, + { + Name: "canvas-proxy", + ContainerPort: CanvasProxyPort, + Protocol: corev1.ProtocolTCP, + }, + } + // MCP Apps sandbox (#615), only when exposed. + if ResolveMcpAppsSandbox(instance).Exposed { + ports = append(ports, corev1.ContainerPort{ + Name: "mcp-apps-proxy", + ContainerPort: McpAppsSandboxProxyPort, + Protocol: corev1.ProtocolTCP, + }) + } return corev1.Container{ Name: "gateway-proxy", Image: GetGatewayProxyImage(instance), ImagePullPolicy: corev1.PullIfNotPresent, - Ports: []corev1.ContainerPort{ - { - Name: "gw-proxy", - ContainerPort: GatewayProxyPort, - Protocol: corev1.ProtocolTCP, - }, - { - Name: "canvas-proxy", - ContainerPort: CanvasProxyPort, - Protocol: corev1.ProtocolTCP, - }, - }, + Ports: ports, VolumeMounts: []corev1.VolumeMount{ { Name: "config", 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