Skip to content

fix: give the main container a sticky /tmp - #623

Open
stubbi wants to merge 6 commits into
mainfrom
fix/sticky-main-tmp
Open

stubbi wants to merge 6 commits into
mainfrom
fix/sticky-main-tmp

Conversation

@stubbi

@stubbi stubbi commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Problem

The required E2E check is red on main and on every same-repo PR: the Chromium Full Integration Tests BeforeAll times out because the openclaw container never becomes ready. The diagnostics from #620 show why. The container exits with code 1 and goes into CrashLoopBackOff, and its log says:

Doctor could not complete maintenance. Check the reported service state and resolve the failure.
temp workspace ancestor is group/world writable without sticky protection

The main container has a read-only root filesystem, so its /tmp is an emptyDir. With fsGroup set, the kubelet gives the root of an emptyDir mode 2777: world-writable, setgid, no sticky bit. OpenClaw 2026.9.6 and later refuse to create a temporary workspace below such a directory. The startup doctor needs that workspace whenever it has to install a configured plugin.

This is the same check #616 fixed for init-plugins. That container could fall back to the image's own /tmp because its root filesystem is writable; the main container cannot.

Only the Full Integration suite hits it because it is the only one that passes OPENROUTER_API_KEY, which makes OpenClaw install its Perplexity plugin at startup. Fork PRs skip that suite (no secret), and the smoke test has no key, so both stay green. Real instances are affected whenever the doctor has to install a plugin at startup.

Change

  • New init container init-tmp-dir creates a subdirectory in the tmp emptyDir and sets it to mode 1777.
  • The main container mounts that subdirectory at /tmp via subPath, which hides the 2777 volume root from its path ancestry.
  • The init container runs as the pod UID with all capabilities dropped and a read-only root filesystem, so it is admitted under the restricted Pod Security Standard. It is always rendered, including when storage.fixOwnership is false.
  • init-tmp-dir is added to the reserved init container names in the webhook.

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 the root itself. A directory it creates inside the volume is its own.

Behavior change to note

Every instance gets one more init container, so pods roll on operator upgrade.

Testing

  • make test passes locally (unit + envtest). New unit tests in internal/resources/tmp_dir_test.go, new e2e spec test/e2e/e2e_tmp_dir_test.go.
  • Reproduced outside the cluster with ghcr.io/openclaw/openclaw:latest (2026.9.8) under Docker, using the operator's rendered config, volume layout, read-only root and dropped capabilities:
    • /tmp as a 2777 root-owned directory with OPENROUTER_API_KEY set: exits 1 with the message above.
    • Same, without the key: boots normally, which matches the smoke test passing.
    • The generated init script run as 1000:1000 against a 2777 root:1000 volume produces a 1777 directory owned by 1000, and is idempotent. With that directory mounted at /tmp and the key set, the gateway reaches readyz 200.
  • Not run on a real cluster before this PR; this PR's own E2E run is the first. I could not run golangci-lint locally (it runs out of memory in my environment), so Lint is covered by CI only.

Follow-ups

🤖 Generated with Claude Code

https://claude.ai/code/session_011HrjPjDpPgTdCFicDWQpvH

@jtatum

jtatum commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

One P2 edge case: init-tmp-dir should use the main container's effective UID, including security.containerSecurityContext.runAsUser.

Currently the helper inherits the pod UID. With pod UID 1000 and gateway UID 2000, it creates the directory owned by 1000. The sticky bit fixes the permissions check, but OpenClaw also requires a temporary-workspace directory to be owned by root or the current user.

I reproduced this in isolated Docker containers using OpenClaw 2026.9.6's actual @openclaw/fs-safe admitTempWorkspaceRoot implementation, running as UID 2000 with a read-only root filesystem and all capabilities dropped:

/tmp owner Mode Result
1000 1777 Rejected: temp workspace directory has an untrusted owner
2000 1777 Accepted

Suggested fix: set the helper's RunAsUser from buildContainerSecurityContext(instance).RunAsUser, retaining pod inheritance when that override is unset. Add a regression test with different pod and gateway UIDs; the current tests require the helper's RunAsUser to be unset. #619 won't cover this helper because its UID propagation targets containers that mount the data volume.

Otherwise the approach looks sound and complements the separate installer scratch-directory work in #622. Resource and webhook tests passed locally. The failing API Docs Sync check is inherited from the base branch: the generated reference is missing storage.fixOwnership; running make api-docs and committing the result should resolve it.

@stubbi
stubbi force-pushed the fix/sticky-main-tmp branch from cf1c871 to 3bf8c00 Compare October 11, 2026 16:54
stubbi and others added 2 commits October 11, 2026 13:22
#619 made every operator init container that writes pod storage use the
gateway's effective UID. Do the same for init-tmp-dir so the sticky /tmp
directory belongs to the user that writes to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stubbi

stubbi commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Merge order note (maintenance pass, 2026-10-11). The open stubbi PRs are stacked so they apply cleanly in this order: #623 → #627 → #620 → #618 → #617 → #628 → #629. This is position 1 of 7. #623 is at the bottom because the required E2E check cannot pass without its sticky /tmp fix (OpenClaw >= 2026.9.6 crash-loops otherwise). Branch protection requires an up-to-date branch, so after each merge click "Update branch" on the next PR and let CI re-run before merging it.

@jtatum

jtatum commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Thanks—the effective-UID issue from my earlier comment is fixed. I verified that a gateway UID override of 2000 now reaches init-tmp-dir.

One separate P2 edge case remains: the helper copies mainSC.RunAsUser but hardcodes RunAsNonRoot: true. With:

security:
  containerSecurityContext:
    runAsUser: 0
    runAsNonRoot: false

the generated helper has runAsUser: 0 and runAsNonRoot: true, so kubelet refuses to start it. The existing data-writing init containers already propagate both fields from the effective main-container security context.

Please propagate the effective RunAsNonRoot setting alongside the UID here, and add a regression test for that explicit root configuration. The default non-root behavior can remain unchanged.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If this PR is still relevant, please push updates or leave a comment.

@github-actions github-actions Bot added the stale label Oct 19, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants