spawn_test: non-string env values raise in the tester thread — Test launch / Test all stuck disabled #34

New Issue

2026-07-12T12:44:29-04:00

the_og commented

2026-07-12 12:44:29 -04:00

Found in audit of main @ 06326e5.

spawn_test() (bcc_core.py ~L1584) builds merged_env = {**os.environ, "PATH": augmented_path()} then merged_env.update(data.get("env") or {}) and passes it to subprocess.Popen. If a server's env contains a non-string value — trivially reachable by pasting JSON like "env": {"PORT": 8080} and saving without ever opening the editor (the KeyValueTable str()-casts on load, but paste/import doesn't round-trip through it) — Popen raises TypeError, which is not in the except (FileNotFoundError, OSError) clause.

The exception escapes SpawnTester.run() / the Test-all worker: done is never emitted, so:

Test launch button stays disabled saying "Launching…" forever;
Test all stalls mid-queue with the button stuck on "Testing N/M…".

Args are already str-cast ([str(a) for a in …]); env is the gap. args containing non-str is handled; command non-str is .strip()ed — a non-str command (e.g. number) would also raise AttributeError before that ((data.get("command") or "").strip() breaks on int → actually or "" passes the int through; .strip() → AttributeError). Same escape path.

Proposed fix

In spawn_test: coerce env keys/values with str() when merging; coerce command via str(data.get("command") or "").strip().
Defense in depth: wrap the body of spawn_test (or SpawnTester.run / the test-all worker) so any unexpected exception returns/emits an {"outcome": "crashed", "detail": repr(e)}-style result instead of dying silently — the UI must always get a result.

Acceptance

Unit test: spawn_test({"command": sys.executable, "args": ["-c", "pass"], "env": {"PORT": 8080}}) returns a result dict (no exception), and the env value is passed as "8080".
Unit test: a data dict engineered to raise inside spawn_test still yields an outcome dict.
Manual: paste a server with numeric env value → Test launch completes and re-enables the button.

**Found in audit of `main` @ 06326e5.** `spawn_test()` (`bcc_core.py` ~L1584) builds `merged_env = {**os.environ, "PATH": augmented_path()}` then `merged_env.update(data.get("env") or {})` and passes it to `subprocess.Popen`. If a server's `env` contains a non-string value — trivially reachable by pasting JSON like `"env": {"PORT": 8080}` and saving without ever opening the editor (the KeyValueTable str()-casts on load, but paste/import doesn't round-trip through it) — `Popen` raises `TypeError`, which is **not** in the `except (FileNotFoundError, OSError)` clause. The exception escapes `SpawnTester.run()` / the Test-all worker: `done` is never emitted, so: - **Test launch** button stays disabled saying "Launching…" forever; - **Test all** stalls mid-queue with the button stuck on "Testing N/M…". Args are already str-cast (`[str(a) for a in …]`); env is the gap. `args` containing non-str is handled; `command` non-str is `.strip()`ed — a non-str `command` (e.g. number) would also raise `AttributeError` before that (`(data.get("command") or "").strip()` breaks on int → actually `or ""` passes the int through; `.strip()` → AttributeError). Same escape path. **Proposed fix** - In `spawn_test`: coerce env keys/values with `str()` when merging; coerce `command` via `str(data.get("command") or "").strip()`. - Defense in depth: wrap the body of `spawn_test` (or `SpawnTester.run` / the test-all worker) so any unexpected exception returns/emits an `{"outcome": "crashed", "detail": repr(e)}`-style result instead of dying silently — the UI must always get a result. **Acceptance** - Unit test: `spawn_test({"command": sys.executable, "args": ["-c", "pass"], "env": {"PORT": 8080}})` returns a result dict (no exception), and the env value is passed as `"8080"`. - Unit test: a data dict engineered to raise inside spawn_test still yields an outcome dict. - Manual: paste a server with numeric env value → Test launch completes and re-enables the button.

the_og added the P1 label 2026-07-12 12:44:29 -04:00

the_og referenced this issue from a commit

2026-07-12 12:58:46 -04:00

fix: spawn_test never raises — coerce env/command to str, wrap unexpected errors (#34)

the_og referenced a pull request that will close this issue

2026-07-12 12:58:56 -04:00

fix: spawn_test never raises — str-coerce env/command, wrap unexpected errors (#34) #41

the_og referenced this issue from a commit

2026-07-12 12:59:21 -04:00

fix: spawn_test never raises — coerce env/command to str, wrap unexpected errors (#34)

the_og closed this issue

2026-07-12 13:00:14 -04:00