Skip to content

fix(scripts): extract step-down function source verbatim instead of declare -f #4775

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
cv merged 1 commit into from
Jun 4, 2026
Merged

fix(scripts): extract step-down function source verbatim instead of declare -f #4775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
122 changes: 111 additions & 11 deletions scripts/nemoclaw-start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2713,21 +2713,113 @@ NODE
fi
}

# Extract the literal source of a bash function from its defining file.
#
# Uses `shopt -s extdebug` + `declare -F` to look up the function's
# source location, then prints the function definition byte-exact from
# disk. The opener line MUST match ^<name>\(\) \{$ and the body MUST
# end with a single `}` at column 0; every function dispatched through
# run_step_down_as_sandbox follows that style.
#
# This bypasses `declare -f`'s serialiser, which mis-orders the body of
# functions whose `if`/`while`/`until` condition is a here-doc command:
# `declare -f` places the indented `then`-body command immediately after
# the `<<TAG` opener and before the here-doc body. The step-down shell
# then absorbs the displaced command into the here-doc body, leaves the
# `then` block empty, and aborts on the closing `fi` with
# syntax error near unexpected token `fi'
# Reading the source bytes off disk preserves the original layout and
# is robust to every here-doc shape, not only the
# here-doc-as-last-statement shape `declare -f` happens to round-trip.
#
# Returns 1 on any of: function not a function, source file unreadable,
# opener line shape unrecognised, or matching closing `}` not found.
_step_down_extract_function() {
local fn="$1"
local info src_lineno src_path
if ! shopt -s extdebug 2>/dev/null; then
return 1
fi
info="$(declare -F "$fn" 2>/dev/null)"
shopt -u extdebug 2>/dev/null || true
if [ -z "$info" ]; then
return 1
fi
src_lineno="${info#* }"
src_lineno="${src_lineno%% *}"
src_path="${info#* * }"
if [ -z "$src_lineno" ] || [ -z "$src_path" ] || [ ! -r "$src_path" ]; then
return 1
fi
awk -v start="$src_lineno" -v fn="$fn" '
NR == start {
# One-liner shape: `name() { body; }` — entire definition on one line.
# No heredoc is possible in this shape, so emit and stop.
if ($0 ~ "^"fn"[[:space:]]*\\(\\)[[:space:]]*\\{.*\\}[[:space:]]*$") {
print
exit 0
}
# Multi-line shape: `name() {` opener, with the matching `}` on its
# own line at column 0 at the end of the body. Both production
# call sites and the test stubs that exercise here-docs follow
# this convention.
if ($0 !~ "^"fn"[[:space:]]*\\(\\)[[:space:]]*\\{[[:space:]]*$") {
exit 1
}
in_fn = 1
print
next
}
!in_fn { next }
in_heredoc {
print
if ($0 == heredoc_tag) in_heredoc = 0
next
}
{
print
if (match($0, /<<-?[[:space:]]*['"'"'"]?[A-Za-z_][A-Za-z0-9_]*['"'"'"]?/)) {
tag = substr($0, RSTART, RLENGTH)
sub(/^<<-?[[:space:]]*/, "", tag)
sub(/^['"'"'"]/, "", tag)
sub(/['"'"'"]$/, "", tag)
in_heredoc = 1
heredoc_tag = tag
next
}
if ($0 == "}") exit
}
END { if (in_fn && in_heredoc) exit 1 }
' "$src_path"
}

# Run one or more locally-defined bash functions as the sandbox user
# without round-tripping through `bash -c "$(declare -f ...) ..."`.
# without round-tripping through `bash -c "$(declare -f ...) ..."` and
# without going through `declare -f`'s serialiser at all.
#
# The interpolated form is fragile under restricted runtimes: the
# step-down shell cannot always re-parse a heredoc-bearing function
# body carried through `bash -c`'s argv. Writing the declarations plus
# the trailing invocation to a temp script and invoking `bash <file>`
# instead lets the step-down shell read the literal source bytes from
# disk so the argv/quoting round-trip is gone.
# The interpolated argv form was fragile because the step-down shell
# could not always re-parse a here-doc-bearing function body carried
# through `bash -c`'s argv. The earlier in-house fix routed function
# bodies through `declare -f` plus a temp file, which removed the argv
# round-trip but kept `declare -f`'s body-reordering bug for here-doc
# `if` conditions. This helper now copies each named function's source
# verbatim from `${BASH_SOURCE[0]}` (resolved per function via the
# extdebug machinery), so every here-doc shape — condition, body,
# trailing — survives the dispatch unchanged.
#
# The temp script lives directly under /tmp (sticky-bit, world-writable
# but unlink-protected) with an unguessable mktemp suffix, so an
# attacker cannot swap the file between mktemp and the step-down bash
# invocation. The directory is intentionally not configurable.
#
# A `bash -n` syntax check runs on the assembled script before the
# step-down invocation. It is a fail-closed guard: if a future change
# ever produces a malformed temp script (for example, a dispatched
# function that violates the opener/closer style assumption), we abort
# before handing the broken script to step-down, surfacing a clean
# error instead of the obscure `unexpected token 'fi'` failure that
# this helper exists to prevent.
#
# Usage: run_step_down_as_sandbox <invocation-snippet> <fn>...
#
# SECURITY CONTRACT: <invocation-snippet> is appended verbatim to the
Expand All @@ -2747,14 +2839,22 @@ run_step_down_as_sandbox() {
rm -f "$script" 2>/dev/null || true
return 1
fi
{
if ! (
printf 'set -euo pipefail\n'
declare -f "$@"
for fn in "$@"; do
_step_down_extract_function "$fn" || exit 1
done
printf '%s\n' "$invocation"
} >"$script" || {
) >"$script"; then
rm -f "$script" 2>/dev/null || true
printf '[step-down] failed to assemble dispatch script\n' >&2
return 1
}
fi
if ! bash -n "$script" 2>/dev/null; then
rm -f "$script" 2>/dev/null || true
printf '[step-down] generated dispatch script failed bash -n syntax check\n' >&2
return 1
fi
local rc=0
"${STEP_DOWN_PREFIX_SANDBOX[@]}" bash "$script" || rc=$?
rm -f "$script" 2>/dev/null || true
Expand Down
84 changes: 80 additions & 4 deletions test/nemoclaw-start.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2701,7 +2701,10 @@ describe("seed_default_workspace_templates (#3240)", () => {
const configPath = path.join(tmpDir, "openclaw.json");
fs.writeFileSync(configPath, JSON.stringify({ agents: { defaults: { skipBootstrap: true } } }));
const scriptPath = path.join(tmpDir, "seed-as-sandbox.sh");
const runStepDown = extractShellFunctionFromSource(src, "run_step_down_as_sandbox");
const runStepDown = [
extractShellFunctionFromSource(src, "_step_down_extract_function"),
extractShellFunctionFromSource(src, "run_step_down_as_sandbox"),
].join("\n");
const seedAsSandbox = extractShellFunctionFromSource(
src,
"seed_default_workspace_templates_as_sandbox",
Expand Down Expand Up @@ -4111,7 +4114,10 @@ describe("openclaw.json baseline + recovery (#3118)", () => {

describe("run_step_down_as_sandbox", () => {
const src = fs.readFileSync(START_SCRIPT, "utf-8");
const helper = extractShellFunctionFromSource(src, "run_step_down_as_sandbox");
const helper = [
extractShellFunctionFromSource(src, "_step_down_extract_function"),
extractShellFunctionFromSource(src, "run_step_down_as_sandbox"),
].join("\n");

it("dispatches via a temp script and cleans up after success", () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-step-down-helper-"));
Expand Down Expand Up @@ -4171,6 +4177,70 @@ describe("run_step_down_as_sandbox", () => {
}
});

it("survives a heredoc used as an if-condition's command without bash declare -f reordering the then-body into the heredoc", () => {
// Regression: bash `declare -f` serialises a function whose `if`
// condition is a heredoc-bearing command by placing the indented
// `then`-body command BEFORE the heredoc closer. When the
// step-down shell re-parses that output, it consumes the displaced
// command as part of the heredoc body, leaves the `then` block
// empty, and aborts on the closing `fi` with
// syntax error near unexpected token `fi'
// (the exact text NV QA reported on v0.0.58 after the earlier fix
// that handled only the heredoc-as-last-statement shape). The new
// helper bypasses `declare -f` and reads the function source
// verbatim from disk via `shopt -s extdebug` + `declare -F`, so
// every here-doc placement survives intact.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-step-down-heredoc-if-"));
const stepDownLog = path.join(tmpDir, "step-down.log");
const sentinel = path.join(tmpDir, "ran.txt");
const scriptPath = path.join(tmpDir, "run.sh");
fs.writeFileSync(
scriptPath,
[
"#!/usr/bin/env bash",
"set -euo pipefail",
`STEP_DOWN_PREFIX_SANDBOX=(bash -c 'printf "%s\\n" "$2" >${JSON.stringify(stepDownLog)}; exec "$@"' sandbox-step-down)`,
`SENTINEL=${JSON.stringify(sentinel)}`,
// Mirror seed_default_workspace_templates' broken shape exactly:
// a heredoc-bearing `node` invocation as the `if` condition,
// with a `then`-body command, followed by `fi`. This is the
// shape `declare -f` mangles in bash 5.x.
"heredoc_in_if_condition() {",
" local marker=\"$1\"",
" if ! node - \"$marker\" <<'NODE' >/dev/null 2>&1; then",
"const fs = require(\"fs\");",
"const target = process.argv[2];",
"fs.writeFileSync(target, \"ran-via-heredoc-if\\n\");",
"process.exit(0);",
"NODE",
" return 0",
" fi",
"}",
helper,
"run_step_down_as_sandbox 'heredoc_in_if_condition \"$SENTINEL\"' heredoc_in_if_condition",
].join("\n"),
{ mode: 0o700 },
);
try {
const result = spawnSync("bash", [scriptPath], {
encoding: "utf-8",
env: { ...process.env, SENTINEL: sentinel },
timeout: 5000,
});
expect(result.status, result.stderr || result.stdout).toBe(0);
expect(result.stderr).not.toContain("syntax error near unexpected token `fi'");
expect(result.stderr).not.toContain("bash -n syntax check");
// The heredoc body ran in the step-down shell: it wrote the sentinel.
expect(fs.existsSync(sentinel)).toBe(true);
expect(fs.readFileSync(sentinel, "utf-8")).toBe("ran-via-heredoc-if\n");
const tempScriptPath = fs.readFileSync(stepDownLog, "utf-8").trim();
expect(tempScriptPath).toMatch(/^\/tmp\/nemoclaw-step-down-[A-Za-z0-9]{6}\.sh$/);
expect(fs.existsSync(tempScriptPath)).toBe(false);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});

it("survives heredoc-bearing function bodies through the temp-script round-trip", () => {
// The production caller passes functions whose bodies contain a
// `<<'TAG'` heredoc (e.g. `python3 - <<'PYAUTH' ...`). This test
Expand Down Expand Up @@ -4239,7 +4309,10 @@ describe("run_step_down_as_sandbox", () => {

describe("setup_auth_profile_as_sandbox", () => {
const src = fs.readFileSync(START_SCRIPT, "utf-8");
const helper = extractShellFunctionFromSource(src, "run_step_down_as_sandbox");
const helper = [
extractShellFunctionFromSource(src, "_step_down_extract_function"),
extractShellFunctionFromSource(src, "run_step_down_as_sandbox"),
].join("\n");
const setup = extractShellFunctionFromSource(src, "setup_auth_profile_as_sandbox");

it("runs the auth-profile setup under HOME=/sandbox even when the parent env has HOME=/root", () => {
Expand Down Expand Up @@ -4506,7 +4579,10 @@ describe("direct-root entrypoint composition under CAP_DAC_OVERRIDE drop", () =>
const writeRuntimeEnv = src
.slice(writeRuntimeStart, writeRuntimeEnd)
.replaceAll("/tmp/nemoclaw-proxy-env.sh", proxyEnvFile);
const helper = extractShellFunctionFromSource(src, "run_step_down_as_sandbox");
const helper = [
extractShellFunctionFromSource(src, "_step_down_extract_function"),
extractShellFunctionFromSource(src, "run_step_down_as_sandbox"),
].join("\n");
const setupAuth = extractShellFunctionFromSource(src, "setup_auth_profile_as_sandbox");
fs.writeFileSync(
scriptPath,
Expand Down
Loading