lablup / lablup/backend.ai

Batch commands are executed by /bin/sh regardless of the SHELL the runner declares

Open
#13,145 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
670
Forks
183
Avg merge
15h 13m
Merged PRs (30d)
368

Description

## Summary

A batch session's `startup_command` — and every other user-authored command string passed to
`BaseRunner.run_subproc()` — is executed by `/bin/sh`, which is **dash** on Debian/Ubuntu-based
images. The runner itself declares a different shell to the child process: `child_env["SHELL"]`
is set to `/bin/bash` (or `/bin/ash`) at `src/ai/backend/kernel/base.py:152,186`. Nothing reads
that value when the command is actually spawned.

The result is that ordinary bash syntax users write in a batch command silently does not work,
and in the worst cases the kernel still exits 0, so the session is recorded as a success.

## Current behavior

```
agent/agent.py:2384 execute_batch() → mode="batch", opts={"exec": startup_command}
kernel/base.py:494 _execute() → run_subproc(exec_cmd, batch=True)
kernel/base.py:1040 asyncio.create_subprocess_shell(str(cmd))
```

`create_subprocess_shell` is `shell=True`, and CPython hardcodes the interpreter
(`subprocess.py`: `unix_shell = '/bin/sh'`). `$SHELL` is never consulted.

`run_subproc` picks its spawner by argument type — list → `create_subprocess_exec`
(internally built argv: the bootstrap script, pip/`main.py` invocations), string →
`create_subprocess_shell` (user-authored command lines). That split is correct in itself; the
issue is only which interpreter the string branch gets.

## Reproduction

Image `cr.backend.ai/multiarch/python:3.9-ubuntu20.04` (`/bin/sh -> dash`), driving the same
`asyncio.create_subprocess_shell` call the runner uses:

```
--- multi-line command, exit=0 ---
/bin/sh: 2: source: not found
VIRTUAL_ENV=
/usr/bin/python ← subsequent lines run against the system Python
```

A control `export FOO=persisted` on line 1 is still visible on line 4, so this is not a
state-loss problem — the lines share one shell. dash simply has no `source` builtin.

A 41-case dash-vs-bash matrix was run in the same image. Failures fall into three classes:

**Class A — silent, exit 0:** `source`; `((i++))`, `let`, `v+=b` (value unchanged);
`{1..5}` (literal); `echo -e` (prints `-e`); `$'a\tb'`; `[[ ... ]]` (`[[: not found` → condition
evaluates false, so the *else* branch is taken); `trap ... ERR`; `mapfile`; `$RANDOM`.

Most severe is `&>`. dash parses `python train.py &> train.log` as `python train.py &` plus
`> train.log`, so the command is **backgrounded**, the log is left empty, and the next line runs
immediately. Measured:

```
== /bin/dash
next line reached after 0 ms
log size: 0
== /bin/bash
next line reached after 3003 ms
```

A batch job written this way exits 0 while its real work is still running, and the container is
then torn down.

**Class B — visible error, still exit 0:** `pushd`/`popd`, `export -f`, `shopt`, `wait -n`,
`[ "$a" == "b" ]`, `time cmd`.

**Class C — hard abort (rc 2), lines before it already ran:** `arr=(a b c)`,
`function f() {}`, `<(...)`, `<<<`, `${v,,}`, `${!name}`, and `set -o pipefail` — so the common
`set -euo pipefail` prologue kills the whole job before line 2.

## Why this is hard for users to diagnose

The behavior varies by base image, so the same command works for one user and not another.
Measured across three real images:

| | dash (`ubuntu20.04`) | busybox ash (`alpine:3.22`) | bash (`almalinux:9`) |
|---|---|---|---|
| `/bin/sh` → | `dash` | `busybox` | `/usr/bin/bash` |
| `source` | ✗ | ✓ | ✓ |
| `[[ ]]` | ✗ | ✓ | ✓ |
| `set -o pipefail` | ✗ (aborts) | ✓ | ✓ |
| `let` | ✗ | ✓ | ✓ |
| `cmd &> log` | ✗ (backgrounds) | ✓ | ✓ |
| arrays, `${v,,}`, `((i++))`, `{1..5}`, `v+=b` | ✗ | ✗ | ✓ |

## Secondary defect: `run_subproc`'s return contract differs by branch

The comment at `kernel/base.py:1035` states that "command not found" is handled by the spawned
shell and terminates with 127. That holds only for the string branch. In the list branch a
missing binary raises `FileNotFoundError` in the parent, which is swallowed by
`except Exception` at `:1070` and returned as `-1`. Verified:

```
exec: FileNotFoundError [Errno 2] No such file or directory
shell: rc 127
```

## Proposal

1. **Execute user-authored command strings with a resolved shell.** Replace
`create_subprocess_shell(str(cmd))` with `create_subprocess_exec(shell, "-c", str(cmd))`,
resolving `shell` as: validated `child_env["SHELL"]` → `/bin/bash` → `/bin/ash` → `/bin/sh`.
Log the resolved interpreter once so it is visible in the task log.

**`$SHELL` must be validated, not trusted.** `child_env` merges the session environment and
`/home/config/environ.txt` (`kernel/base.py:188-192`), both user-controlled. An unvalidated
value lets a user swap the interpreter for a non-POSIX shell (`fish`, `csh`) or point it at a
nonexistent path, breaking every batch job on that image. Require an absolute path that
exists and is executable, with a basename in `{bash, ash, dash, sh, zsh, ksh}`; otherwise
fall through to the fixed candidates.

2. **Allow the interpreter to be declared explicitly.** A `shell` field on the batch execution
spec, carried through to the runner and used verbatim when set, would let callers pin the
interpreter instead of depending on what a given image happens to ship. This is the part that
makes the behavior predictable at authoring time rather than at run time; candidate semantics:
an explicit value fails loudly (exit 127 with a clear message) if that interpreter is absent,
while the default keeps the resolution order in (1).

3. **Honor the documented return code.** Catch `FileNotFoundError`/`PermissionError` in the exec
branch and return 127 rather than falling through to `-1`, and update the comment to describe
both branches.

`_bootstrap`'s explicit `["/bin/sh", str(script_path)]` at `:449` should stay as it is — that is
a deliberate POSIX contract for bootstrap scripts, not an accident of `shell=True`.

## Compatibility

(1) is a behavior change and deserves an explicit decision. Commands that silently no-op today
would start working, which is the point — but the `&>` case also changes timing: a job that
currently "finishes" instantly because dash backgrounded its main process would begin to block
until that process actually exits. That is the correct behavior, and it is what the author
intended when they wrote `&>`, but it will look like a regression to anyone whose pipeline
timing depended on the accident. Gating it behind a config flag for one release, or landing it
in a major version, are both reasonable.

Contributor guide

Open the contributing guide

Research direction

Start in src/ai/backend/kernel/base.py at child_env construction, _execute(), _bootstrap(), and BaseRunner.run_subproc(), then trace agent/agent.py:2384 execute_batch(). Compare the string and list subprocess branches against the documented return contract and the reproduction cases. Done means user-authored strings use the resolved declared shell, missing executables return 127, and the explicit /bin/sh bootstrap path remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
bash, python
Domain
backend, devops
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.