Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -480,17 +480,37 @@ RUN set -eu; \
done; \
rm -rf /root/.npm /sandbox/.npm

# Stale-base fallback for the gateway-in-sandbox-group setup (#2681).
# Newer base images already add the gateway user to the sandbox group, but
# the derived image must remain build-clean against older sandbox-base:latest
# tags too. The `id -nG` check makes this idempotent.
# hadolint ignore=DL4006
RUN if id gateway >/dev/null 2>&1 && id sandbox >/dev/null 2>&1; then \
if ! id -nG gateway | tr ' ' '\n' | grep -qx sandbox; then \
usermod -aG sandbox gateway; \
fi; \
fi

# Keep the image readable to the root entrypoint after capabilities are
# dropped. OpenShell starts the runtime as the sandbox user; the entrypoint
# restores the stricter mutable-default 600/700 permissions there.
# Shields-up applies 444 root:root + chattr +i on top.
#
# `chmod g+w` + setgid (chmod g+s on dirs) on the mutable config tree means
# both `sandbox` and `gateway` (now a member of the sandbox group) can write
# to OpenClaw config/state in default mode. New files created in setgid
# directories inherit group=sandbox regardless of which UID created them,
# so OpenClaw's mutateConfigFile path (control-UI toggles) writes succeed
# without needing an EACCES-swallow patch (#2681 supersedes #2693).
RUN chown -R sandbox:sandbox /sandbox/.openclaw \
&& chmod 755 /sandbox/.openclaw \
&& chmod 644 /sandbox/.openclaw/openclaw.json
&& chmod 644 /sandbox/.openclaw/openclaw.json \
&& chmod -R g+w /sandbox/.openclaw \
&& find /sandbox/.openclaw -type d -exec chmod g+s {} +

# Pin config hash at build time so the entrypoint can verify integrity.
RUN sha256sum /sandbox/.openclaw/openclaw.json > /sandbox/.openclaw/.config-hash \
&& chmod 644 /sandbox/.openclaw/.config-hash \
&& chmod 664 /sandbox/.openclaw/.config-hash \
&& chown sandbox:sandbox /sandbox/.openclaw/.config-hash

# DAC-protect .nemoclaw directory: /sandbox/.nemoclaw is Landlock read_write
Expand Down
11 changes: 10 additions & 1 deletion Dockerfile.base
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,15 @@ RUN arch="$(dpkg --print-architecture)" \
# Create sandbox user (matches OpenShell convention) and gateway user.
# The gateway runs as 'gateway' so the 'sandbox' user (agent) cannot
# kill it or restart it with a tampered HOME/config.
#
# `gateway` is also a member of the `sandbox` group so both users can write
# to the mutable-default OpenClaw config tree (chmod g+w + setgid below).
# This replaces the previous EACCES-swallow approach for control-UI config
# mutations — see #2681. UIDs stay distinct (security separation preserved);
# the shared group only governs the mutable-default state directory.
RUN groupadd -r gateway && useradd -r -g gateway -d /sandbox -s /usr/sbin/nologin gateway \
&& groupadd -r sandbox && useradd -r -g sandbox -d /sandbox -s /bin/bash sandbox \
&& usermod -aG sandbox gateway \
&& mkdir -p /sandbox/.nemoclaw \
&& chown -R sandbox:sandbox /sandbox

Expand All @@ -112,7 +119,9 @@ RUN mkdir -p /sandbox/.openclaw/agents/main/agent \
/sandbox/.openclaw/plugin-runtime-deps \
&& touch /sandbox/.openclaw/update-check.json \
&& touch /sandbox/.openclaw/exec-approvals.json \
&& chown -R sandbox:sandbox /sandbox/.openclaw
&& chown -R sandbox:sandbox /sandbox/.openclaw \
&& chmod -R g+w /sandbox/.openclaw \
&& find /sandbox/.openclaw -type d -exec chmod g+s {} +

# Pre-create shell init files for the sandbox user.
# Runtime proxy config is written by the entrypoint to /tmp/nemoclaw-proxy-env.sh
Expand Down
36 changes: 36 additions & 0 deletions scripts/nemoclaw-start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,41 @@ _SANDBOX_HOME="/sandbox" # Home dir for the sandbox user (useradd -d /s
# mutable-default startup skips strict hash enforcement until shields-up locks
# .config-hash into a root-owned read-only trust anchor.

# ── Mutable-default permission normalize (#2681) ─────────────────
# OpenClaw's control-UI toggles (Enable Dreaming, account toggles, etc.)
# write through mutateConfigFile to /sandbox/.openclaw/openclaw.json.
# In root mode the gateway runs as the gateway UID; the file is owned
# sandbox:sandbox. Without group write, every toggle EACCESs.
#
# Make the mutable-default tree group-writable + setgid so both
# `gateway` (now a member of the sandbox group via Dockerfile.base
# usermod -aG) and `sandbox` can write. Setgid means new files
# inherit group=sandbox regardless of which UID created them, so the
# agent keeps read access and shields-up locking still works the same.
#
# Idempotent. Skips when shields are UP (config dir owned by root) so
# the lock is not weakened.
normalize_mutable_config_perms() {
# Only effective in root mode. Non-root containers can't chmod files
# they don't own; if shields are down they were normalized by an
# earlier root-mode startup.
[ "$(id -u)" -eq 0 ] || return 0

local config_dir="/sandbox/.openclaw"
[ -d "$config_dir" ] || return 0

# Detect shields-up. Config dir owned by root means shields are
# currently locked; normalizing would weaken the contract.
local config_dir_owner
config_dir_owner="$(stat -c '%U' "$config_dir" 2>/dev/null || echo unknown)"
if [ "$config_dir_owner" = "root" ]; then
return 0
fi

chmod -R g+w "$config_dir" 2>/dev/null || true
find "$config_dir" -type d -exec chmod g+s {} + 2>/dev/null || true
}

# ── Runtime model/provider override ──────────────────────────────
# Patches openclaw.json at startup when NEMOCLAW_MODEL_OVERRIDE is set,
# allowing model or provider changes without rebuilding the sandbox image.
Expand Down Expand Up @@ -2289,6 +2324,7 @@ fi
# Verify locked config integrity before starting anything. Mutable-default
# config is intentionally writable and is not a trust anchor until shields-up.
verify_config_integrity_if_locked /sandbox/.openclaw
normalize_mutable_config_perms
apply_model_override
apply_cors_override
export_gateway_token
Expand Down
37 changes: 31 additions & 6 deletions src/lib/shields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,19 @@ const HIGH_RISK_STATE_DIRS = [
];

function applyStateDirLockMode(sandboxName: string, configDir: string, owner: string): void {
// Locking (shields-up) strips group + world write. Unlocking (shields-down)
// re-adds group write and strips world write, plus setgid so the gateway
// UID — now in the sandbox group via Dockerfile.base — can write to
// OpenClaw's mutable config tree (#2681).
//
// The unlock variant uses `g+w,o-w` (not `o-w` alone) because a prior
// lock already stripped g+w from descendants via `chmod -R go-w`. Without
// re-adding g+w explicitly, shields-down would leave nested files at
// 644 — group-write gone, contract broken.
const isLocking = owner === "root:root";
const writeStrip = isLocking ? "go-w" : "g+w,o-w";
const dirMode = isLocking ? "755" : "2775";

for (const dirName of HIGH_RISK_STATE_DIRS) {
const dirPath = `${configDir}/${dirName}`;
try {
Expand All @@ -265,8 +278,8 @@ function applyStateDirLockMode(sandboxName: string, configDir: string, owner: st
// Directory may not exist for this agent — silently skip
}
try {
kubectlExec(sandboxName, ["chmod", "755", dirPath]);
kubectlExec(sandboxName, ["chmod", "-R", "go-w", dirPath]);
kubectlExec(sandboxName, ["chmod", dirMode, dirPath]);
kubectlExec(sandboxName, ["chmod", "-R", writeStrip, dirPath]);
} catch {
// Silently skip
}
Expand All @@ -282,16 +295,20 @@ function applyStateDirLockMode(sandboxName: string, configDir: string, owner: st
set -u
config_dir="$1"
owner="$2"
write_strip="$3"
dir_mode="$4"
for dir in "$config_dir"/workspace-*; do
[ -d "$dir" ] || continue
chown -R "$owner" "$dir" 2>/dev/null || true
chmod 755 "$dir" 2>/dev/null || true
chmod -R go-w "$dir" 2>/dev/null || true
chmod "$dir_mode" "$dir" 2>/dev/null || true
chmod -R "$write_strip" "$dir" 2>/dev/null || true
done
`,
"sh",
configDir,
owner,
writeStrip,
dirMode,
]);
} catch {
// Best effort; verification below catches the primary config lock.
Expand Down Expand Up @@ -337,8 +354,16 @@ function unlockAgentConfig(
): void {
const errors: string[] = [];
const filesToUnlock = [target.configPath, ...(target.sensitiveFiles || [])];
const fileMode = target.agentName === "hermes" ? "640" : "600";
const dirMode = target.agentName === "hermes" ? "750" : "700";
// Mutable-default mode for OpenClaw: group-writable + setgid on the
// config dir so the gateway UID (a member of the sandbox group via
// Dockerfile.base) can write to OpenClaw config files. Without this,
// control-UI mutations (Enable Dreaming, account toggles) EACCES
// against sandbox:sandbox 600 even after shields-down
// (#2681 supersedes #2693).
// Hermes is unchanged — its sandbox does not run a separate gateway UID,
// so the shared-group contract does not apply.
const fileMode = target.agentName === "hermes" ? "640" : "660";
const dirMode = target.agentName === "hermes" ? "750" : "2770";
for (const f of filesToUnlock) {
try {
kubectlExec(sandboxName, ["chattr", "-i", f]);
Expand Down
120 changes: 120 additions & 0 deletions test/repro-2681-group-writable.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* Regression guards for the group-writable mutable-default contract (#2681).
*
* Before this PR, control-UI config mutations in the OpenClaw sandbox
* (Enable Dreaming, account toggles, etc.) wrote through mutateConfigFile,
* which targeted /sandbox/.openclaw/openclaw.json — owned sandbox:sandbox
* mode 600. The gateway runs as a separate UID, so every mutation EACCES'd.
*
* The previous proposal (#2693) wrapped mutateConfigFile in a try/catch
* that swallowed EACCES, making the mutation a silent no-op. That made
* toggles non-functional in the sandbox.
*
* This PR replaces that approach with proper Unix group permissions:
* 1. `gateway` is a member of the `sandbox` group (Dockerfile.base usermod).
* 2. /sandbox/.openclaw is group-writable + setgid (chmod g+w + g+s).
* 3. nemoclaw-start.sh normalizes those perms before gateway launch when
* shields are not UP.
* 4. `shields down` restores 660/2770 instead of the old 600/700.
*
* Result: writes succeed in default mode without an EACCES swallow.
*
* These tests lock the structural invariants so a future change can't
* silently regress to the swallow approach.
*/

import fs from "node:fs";
import path from "node:path";
import { describe, expect, it } from "vitest";

const ROOT = path.join(import.meta.dirname, "..");
const DOCKERFILE_BASE = fs.readFileSync(path.join(ROOT, "Dockerfile.base"), "utf-8");
const DOCKERFILE = fs.readFileSync(path.join(ROOT, "Dockerfile"), "utf-8");
const NEMOCLAW_START = fs.readFileSync(
path.join(ROOT, "scripts", "nemoclaw-start.sh"),
"utf-8",
);
const SHIELDS_TS = fs.readFileSync(path.join(ROOT, "src", "lib", "shields.ts"), "utf-8");

describe("Issue #2681 — group-writable mutable-default contract", () => {
it("Dockerfile.base adds gateway to the sandbox group", () => {
expect(DOCKERFILE_BASE).toMatch(/usermod\s+-aG\s+sandbox\s+gateway/);
});

it("Dockerfile.base makes /sandbox/.openclaw group-writable + setgid", () => {
expect(DOCKERFILE_BASE).toMatch(/chmod\s+-R\s+g\+w\s+\/sandbox\/\.openclaw/);
expect(DOCKERFILE_BASE).toMatch(
/find\s+\/sandbox\/\.openclaw\s+-type\s+d\s+-exec\s+chmod\s+g\+s/,
);
});

it("Dockerfile has stale-base fallback that idempotently adds gateway to sandbox group", () => {
// Older base images won't have the usermod yet; the derived image must
// add it at build time so PR images work even before sandbox-base is
// rebuilt.
expect(DOCKERFILE).toMatch(/id\s+gateway/);
expect(DOCKERFILE).toMatch(/usermod\s+-aG\s+sandbox\s+gateway/);
});

it("Dockerfile applies group-writable + setgid in the production image too", () => {
expect(DOCKERFILE).toMatch(/chmod\s+-R\s+g\+w\s+\/sandbox\/\.openclaw/);
expect(DOCKERFILE).toMatch(
/find\s+\/sandbox\/\.openclaw\s+-type\s+d\s+-exec\s+chmod\s+g\+s/,
);
});

it("Dockerfile creates .config-hash group-writable (664), not read-only-for-group (644)", () => {
// Aaron's spec item 3 explicitly calls out "group-writable config/hash
// files". The .config-hash sha256 is created AFTER the recursive chmod
// g+w pass, so it gets its own explicit chmod. Lock it to 664 so a
// future change can't silently revert to 644 and break gateway writes.
expect(DOCKERFILE).toMatch(/chmod\s+664\s+\/sandbox\/\.openclaw\/\.config-hash/);
expect(DOCKERFILE).not.toMatch(/chmod\s+644\s+\/sandbox\/\.openclaw\/\.config-hash/);
});

it("Dockerfile does NOT introduce a mutateConfigFile EACCES swallow patch", () => {
// The PR explicitly replaces #2693's approach. If a future PR adds
// Patch 4b back, this test fails and forces re-evaluation.
expect(DOCKERFILE).not.toMatch(/Patch\s+4b/);
expect(DOCKERFILE).not.toMatch(/mutateConfigFile.*EACCES/);
expect(DOCKERFILE).not.toMatch(/mutation not persisted/);
});

it("nemoclaw-start.sh defines normalize_mutable_config_perms", () => {
expect(NEMOCLAW_START).toMatch(/normalize_mutable_config_perms\s*\(\)/);
});

it("normalize_mutable_config_perms skips when shields are UP (root-owned config dir)", () => {
// The function must check ownership before chmod-ing; if shields are
// up the dir is root-owned and normalizing would weaken the lock.
const fnIdx = NEMOCLAW_START.indexOf("normalize_mutable_config_perms()");
expect(fnIdx).toBeGreaterThan(0);
const fnBody = NEMOCLAW_START.slice(fnIdx, fnIdx + 1500);
expect(fnBody).toMatch(/stat\s+-c\s+'%U'/);
expect(fnBody).toMatch(/= "root"/);
});

it("normalize_mutable_config_perms is called in the root-mode startup path", () => {
expect(NEMOCLAW_START).toMatch(/normalize_mutable_config_perms\b[^(]/);
});

it("shields.ts unlock path uses group-writable file mode (660) + setgid dir (2770) for openclaw", () => {
// Pre-#2681 openclaw unlock used 600/700 which stripped group-write.
// After this PR openclaw uses 660/2770 so the gateway UID (member of
// sandbox group) can write OpenClaw config. Hermes is left unchanged
// (no separate gateway UID, so the shared-group contract doesn't apply).
expect(SHIELDS_TS).toMatch(/agentName === "hermes" \? "640" : "660"/);
expect(SHIELDS_TS).toMatch(/agentName === "hermes" \? "750" : "2770"/);
});

it("applyStateDirLockMode re-adds group-write when unlocking (shields down)", () => {
// The chmod in the unlock path must explicitly RE-ADD group-write,
// not just preserve it. A prior `chmod -R go-w` from shields-up
// already stripped g+w from descendants, so unlock must use `g+w,o-w`
// to restore the group-writable contract on the whole tree.
expect(SHIELDS_TS).toMatch(/isLocking\s*\?\s*"go-w"\s*:\s*"g\+w,o-w"/);
});
});
Loading