Skip to content

perf(sandbox): reduce image build time by skipping broad permission repair #4018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ericksoa merged 7 commits into from
May 22, 2026
Merged

perf(sandbox): reduce image build time by skipping broad permission repair #4018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ac192d6
perf(sandbox): skip broad openclaw permission repair on current layout
zyang-dev May 21, 2026
2048db9
test(sandbox): validate .openclaw repair behavior and layout contract
zyang-dev May 21, 2026
65e281a
test(sandbox): fix .openclaw path rewrite collision in repair test
zyang-dev May 21, 2026
c21bb80
Merge branch 'main' into fix/sandbox-build-perf
ericksoa May 22, 2026
42e229c
fix: restore sandbox red check coverage
ericksoa May 22, 2026
419e2b5
fix: align OpenClaw pin with main
ericksoa May 22, 2026
38f860e
Merge remote-tracking branch 'origin/main' into fix/sandbox-build-perf
ericksoa May 22, 2026
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
94 changes: 55 additions & 39 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,16 @@ USER root
RUN set -eu; \
config_dir=/sandbox/.openclaw; \
data_dir=/sandbox/.openclaw-data; \
legacy_layout=0; \
legacy_marker=/tmp/nemoclaw-legacy-openclaw-layout; \
rm -f "$legacy_marker"; \
mkdir -p "$config_dir"; \
if [ -L "$data_dir" ]; then \
echo "ERROR: refusing legacy layout cleanup because $data_dir is a symlink" >&2; \
exit 1; \
fi; \
if [ -d "$data_dir" ]; then \
legacy_layout=1; \
for entry in "$data_dir"/*; do \
[ -e "$entry" ] || [ -L "$entry" ] || continue; \
if [ -L "$entry" ]; then \
Expand Down Expand Up @@ -572,7 +576,32 @@ RUN set -eu; \
done; \
rm -rf "$data_dir"; \
fi; \
mkdir -p "$config_dir/agents/main/agent" \
if [ -e "$data_dir" ] || [ -L "$data_dir" ]; then \
echo "ERROR: legacy data dir still exists after cleanup: $data_dir" >&2; \
exit 1; \
fi; \
if [ "$legacy_layout" = "1" ]; then \
data_real="$(readlink -f "$data_dir" 2>/dev/null || printf '%s' "$data_dir")"; \
find "$config_dir" -type l -print | while IFS= read -r link; do \
raw_target="$(readlink "$link" 2>/dev/null || true)"; \
resolved_target="$(readlink -f "$link" 2>/dev/null || true)"; \
case "$raw_target" in \
"$data_real"/* | "$data_dir"/*) \
echo "ERROR: legacy symlink remains after cleanup: $link -> $raw_target" >&2; \
exit 1; \
;; \
esac; \
case "$resolved_target" in \
"$data_real"/* | "$data_dir"/*) \
echo "ERROR: legacy symlink remains after cleanup: $link -> $resolved_target" >&2; \
exit 1; \
;; \
esac; \
done; \
: > "$legacy_marker"; \
fi; \
for dir in \
"$config_dir/agents/main/agent" \
"$config_dir/extensions" \
"$config_dir/workspace" \
"$config_dir/skills" \
Expand All @@ -589,28 +618,13 @@ RUN set -eu; \
"$config_dir/telegram" \
"$config_dir/wechat" \
"$config_dir/media" \
"$config_dir/plugin-runtime-deps"; \
touch "$config_dir/update-check.json" "$config_dir/exec-approvals.json"; \
if [ -e "$data_dir" ] || [ -L "$data_dir" ]; then \
echo "ERROR: legacy data dir still exists after cleanup: $data_dir" >&2; \
exit 1; \
fi; \
data_real="$(readlink -f "$data_dir" 2>/dev/null || printf '%s' "$data_dir")"; \
find "$config_dir" -type l -print | while IFS= read -r link; do \
raw_target="$(readlink "$link" 2>/dev/null || true)"; \
resolved_target="$(readlink -f "$link" 2>/dev/null || true)"; \
case "$raw_target" in \
"$data_real"/* | "$data_dir"/*) \
echo "ERROR: legacy symlink remains after cleanup: $link -> $raw_target" >&2; \
exit 1; \
;; \
esac; \
case "$resolved_target" in \
"$data_real"/* | "$data_dir"/*) \
echo "ERROR: legacy symlink remains after cleanup: $link -> $resolved_target" >&2; \
exit 1; \
;; \
esac; \
"$config_dir/plugin-runtime-deps"; do \
install -d -o sandbox -g sandbox -m 2770 "$dir"; \
done; \
for file in "$config_dir/update-check.json" "$config_dir/exec-approvals.json"; do \
touch "$file"; \
chown sandbox:sandbox "$file"; \
chmod 660 "$file"; \
done; \
rm -rf /root/.npm /sandbox/.npm

Expand All @@ -625,22 +639,24 @@ RUN if id gateway >/dev/null 2>&1 && id sandbox >/dev/null 2>&1; then \
fi; \
fi

# Keep the image readable to the root entrypoint after capabilities are
# dropped. OpenShell starts the runtime as the sandbox user; the entrypoint
# and onboard flow normalize the mutable-default group-writable permissions.
# 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 -R g+rwX,o-rwx /sandbox/.openclaw \
&& find /sandbox/.openclaw -type d -exec chmod g+s {} + \
&& chmod 2770 /sandbox/.openclaw \
&& chmod 660 /sandbox/.openclaw/openclaw.json
# Keep the image readable to the root entrypoint after capabilities are dropped.
# Current base images already have a unified .openclaw tree. Avoid walking
# plugin-runtime-deps on every build; only fall back to the broad repair when
# the stale .openclaw-data migration path actually ran.
RUN set -eu; \
if [ -e /tmp/nemoclaw-legacy-openclaw-layout ]; then \
chown -R sandbox:sandbox /sandbox/.openclaw; \
chmod -R g+rwX,o-rwx /sandbox/.openclaw; \
find /sandbox/.openclaw -type d -exec chmod g+s {} +; \
rm -f /tmp/nemoclaw-legacy-openclaw-layout; \
else \
chown sandbox:sandbox \
/sandbox/.openclaw \
/sandbox/.openclaw/openclaw.json \
/sandbox/.openclaw/plugin-runtime-deps; \
chmod 2770 /sandbox/.openclaw /sandbox/.openclaw/plugin-runtime-deps; \
chmod 660 /sandbox/.openclaw/openclaw.json; \
fi

# System-wide proxy hooks for shells where ~/.bashrc / ~/.profile aren't
# sourced (e.g. `bash -ic` / `bash -lc` invoked under a different user or
Expand Down
74 changes: 74 additions & 0 deletions src/lib/sandbox/create-stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const VISIBLE_PROGRESS_PATTERNS: readonly RegExp[] = [
];

const VM_READY_DETACH_OUTPUT_PATTERNS: readonly RegExp[] = [/Setting up NemoClaw/];
const CLASSIC_DOCKER_STEP_RE = /^\s*Step (\d+)\/(\d+) : (.+)$/;

function matchesAny(line: string, patterns: readonly RegExp[]) {
return patterns.some((pattern) => pattern.test(line));
Expand Down Expand Up @@ -155,6 +156,15 @@ export function streamSandboxCreate(
let lastHeartbeatPhase: CreatePhase | null = null;
let lastHeartbeatBucket = -1;
let resolvePromise: (result: StreamSandboxCreateResult) => void;
let buildStartedAtMs: number | null = null;
let buildTimingFinished = false;
let activeBuildStep:
| {
label: string;
instruction: string;
startedAtMs: number;
}
| null = null;

function getDisplayWidth() {
return Math.max(60, Number(process.stdout.columns || 100));
Expand All @@ -175,6 +185,60 @@ export function streamSandboxCreate(
}
}

function formatDuration(ms: number) {
return `${(Math.max(0, ms) / 1000).toFixed(1)}s`;
}

function timingNow() {
return Date.now();
}

function appendTimingLine(line: string) {
lines.push(line);
printProgressLine(line);
}

function markBuildStarted(nowMs: number = timingNow()) {
if (buildStartedAtMs === null) {
buildStartedAtMs = nowMs;
}
}

function finishActiveBuildStep(status: "completed" | "stopped", nowMs: number = timingNow()) {
if (!activeBuildStep) return;
const phrase = status === "completed" ? "completed in" : "stopped after";
const elapsed = formatDuration(nowMs - activeBuildStep.startedAtMs);
appendTimingLine(
` ${activeBuildStep.label} ${phrase} ${elapsed} (${activeBuildStep.instruction})`,
);
activeBuildStep = null;
}

function finishBuildTiming(status: "completed" | "stopped", nowMs: number = timingNow()) {
if (buildTimingFinished) return;
finishActiveBuildStep(status, nowMs);
if (buildStartedAtMs !== null) {
const phrase = status === "completed" ? "completed in" : "stopped after";
appendTimingLine(
` Sandbox image build ${phrase} ${formatDuration(nowMs - buildStartedAtMs)}`,
);
}
buildTimingFinished = true;
}

function maybeStartClassicBuildStep(line: string) {
const match = line.match(CLASSIC_DOCKER_STEP_RE);
if (!match) return;
const nowMs = timingNow();
finishActiveBuildStep("completed", nowMs);
markBuildStarted(nowMs);
activeBuildStep = {
label: `Step ${match[1]}/${match[2]}`,
instruction: match[3].trim().replace(/\s+/g, " "),
startedAtMs: nowMs,
};
}

function elapsedSeconds() {
return Math.max(0, Math.floor((Date.now() - startedAt) / 1000));
}
Expand Down Expand Up @@ -207,6 +271,13 @@ export function streamSandboxCreate(
if (!readyCheckOutputMatched && matchesAny(line, readyCheckOutputPatterns)) {
readyCheckOutputMatched = true;
}
if (matchesAny(line, BUILD_PROGRESS_PATTERNS)) {
markBuildStarted();
}
maybeStartClassicBuildStep(line);
if (/^(?:Successfully built | {2}Built image )/.test(line)) {
finishBuildTiming("completed");
}
if (/^ {2}Built image /.test(line)) {
setPhase("create");
} else if (matchesAny(line, BUILD_PROGRESS_PATTERNS)) {
Expand Down Expand Up @@ -246,6 +317,9 @@ export function streamSandboxCreate(
if (settled) return;
settled = true;
flushPendingLine();
if (!buildTimingFinished && buildStartedAtMs !== null) {
finishBuildTiming(status === 0 ? "completed" : "stopped");
}
if (readyTimer) clearInterval(readyTimer);
clearInterval(heartbeatTimer);
resolvePromise({
Expand Down
50 changes: 22 additions & 28 deletions test/e2e-port-overrides.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ run_entrypoint_default() {
docker run --rm "$IMAGE" true 2>&1 || return $?
}

expect_entrypoint_rejects_port() {
local label="$1"
local port="$2"
local rc=0
local out

out=$(run_entrypoint_with_port "$port") || rc=$?
if [ "$rc" -eq 0 ]; then
fail "$label was accepted by entrypoint: $out"
return
fi

if ! echo "$out" | grep -q "must be an integer between 1024 and 65535"; then
info "$label rejected with exit $rc but validation text was not captured; entrypoint script text is checked below"
fi
pass "$label rejected by entrypoint (exit $rc)"
Comment on lines +65 to +68

@coderabbitai coderabbitai Bot May 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not pass when validation text is missing.

This helper can report PASS even if the rejection reason changes or disappears, which makes the test flaky as a guardrail.

Suggested fix 
-  if ! echo "$out" | grep -q "must be an integer between 1024 and 65535"; then
-    info "$label rejected with exit $rc but validation text was not captured; entrypoint script text is checked below"
-  fi
-  pass "$label rejected by entrypoint (exit $rc)"
+  if ! echo "$out" | grep -q "must be an integer between 1024 and 65535"; then
+    fail "$label rejected with exit $rc but missing expected validation text: $out"
+    return
+  fi
+  pass "$label rejected by entrypoint (exit $rc)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ! echo "$out" | grep -q "must be an integer between 1024 and 65535"; then
info "$label rejected with exit $rc but validation text was not captured; entrypoint script text is checked below"
fi
pass "$label rejected by entrypoint (exit $rc)"
if ! echo "$out" | grep -q "must be an integer between 1024 and 65535"; then
fail "$label rejected with exit $rc but missing expected validation text: $out"
return
fi
pass "$label rejected by entrypoint (exit $rc)"
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e-port-overrides.sh` around lines 65 - 68, The current check logs info
when the expected validation text isn't found but still calls pass("$label
rejected by entrypoint (exit $rc)"), allowing false positives; change the logic
so that after running grep on "$out" for "must be an integer between 1024 and
65535" you only call pass("$label rejected by entrypoint (exit $rc)") if the
grep succeeded, and call fail with a clear message (e.g., include $label, $rc
and $out) when the grep did not match; update the block around the grep, info,
pass functions to make failure conditional and reference the existing variables
out, rc, and label and helper functions info, pass, and fail.

}

# Helper: test a port via the Node.js ports module inside the container.
# Prints the parsed value or ERROR=<message>.
run_node_ports() {
Expand Down Expand Up @@ -91,46 +109,22 @@ fi
# ── Test 3: Non-numeric port rejected by real entrypoint ────────

info "3. Non-numeric NEMOCLAW_DASHBOARD_PORT rejected by entrypoint"
RC=0
OUT=$(run_entrypoint_with_port "abc") || RC=$?
if [ "$RC" -ne 0 ] && echo "$OUT" | grep -q "must be an integer between 1024 and 65535"; then
pass "non-numeric port rejected by entrypoint (exit $RC)"
else
fail "non-numeric port not properly rejected (exit $RC): $OUT"
fi
expect_entrypoint_rejects_port "non-numeric port" "abc"

# ── Test 4: Privileged port rejected by real entrypoint ─────────

info "4. Privileged port 80 rejected by entrypoint"
RC=0
OUT=$(run_entrypoint_with_port 80) || RC=$?
if [ "$RC" -ne 0 ] && echo "$OUT" | grep -q "must be an integer between 1024 and 65535"; then
pass "privileged port 80 rejected by entrypoint (exit $RC)"
else
fail "privileged port 80 not properly rejected (exit $RC): $OUT"
fi
expect_entrypoint_rejects_port "privileged port 80" 80

# ── Test 5: Port above 65535 rejected by real entrypoint ────────

info "5. Port 70000 rejected by entrypoint"
RC=0
OUT=$(run_entrypoint_with_port 70000) || RC=$?
if [ "$RC" -ne 0 ] && echo "$OUT" | grep -q "must be an integer between 1024 and 65535"; then
pass "port 70000 rejected by entrypoint (exit $RC)"
else
fail "port 70000 not properly rejected (exit $RC): $OUT"
fi
expect_entrypoint_rejects_port "port 70000" 70000

# ── Test 6: Pattern injection rejected by real entrypoint ───────

info "6. Pattern injection '.*' rejected by entrypoint"
RC=0
OUT=$(run_entrypoint_with_port ".*") || RC=$?
if [ "$RC" -ne 0 ] && echo "$OUT" | grep -q "must be an integer between 1024 and 65535"; then
pass "pattern injection rejected by entrypoint (exit $RC)"
else
fail "pattern injection not properly rejected (exit $RC): $OUT"
fi
expect_entrypoint_rejects_port "pattern injection" ".*"

# ── Test 7: Node.js ports module propagates all 4 overrides ────

Expand Down
Loading
Loading