[jonahgabriel]

Summary

OMN-20309. Slack alert 09:20 ET: hook capture on omninode-pc (.201) was not delivering, 1064 events queued, drainer reported no state. The drainer shipped only as a launchd plist, so nothing drained the journal on Linux.

scripts/install-hook-emit-drainer.sh gets a Linux branch (extended, no new script, per the 2026-10-01 operator ruling). launchctl on PATH keeps the launchd path unchanged; otherwise systemctl selects systemd. New scripts/systemd/ai.omninode.hook-emit-drainer.service is a systemd --user unit: Restart=always, RestartSec=30, StartLimitIntervalSec=0, Nice=10, WantedBy=default.target, no User=, so no root. Interpreter is the omniclaude project venv. Flags: install, --dry-run, --uninstall, --status (unit state, Pending, Spooled; exits non-zero when the unit is not installed).

Test plan

• tests/scripts/test_install_hook_emit_drainer_systemd.py, written red first (6 failed), then green; with the existing status tests, 13 passed.
• Lab (host omninode-pc, lane canary-and-drainer): queue read first, journal 1175 + 5 in emit_spool; the broker 192.168.86.201:19092 accepted a TCP connect as user jonah. bash scripts/install-hook-emit-drainer.sh installed the unit as uid 1000 with no root; systemctl --user reports active/running, NRestarts=0. Journal 1213 at install, 1346 at last read.

Known blocker: the queue does not drain yet

Every publish is refused by the broker ACLs: ONEX_CORE_025_PERMISSION_DENIED on onex.evt.omniclaude.tool-executed.v1 (986 of the queued records are tool.executed). The principal this host declares, dev-jonah-omninode-pc, lists 3 topics and none of the onex.evt.omniclaude topics, so it holds no WRITE grant on them. The broker is reachable; the fault is the grant. The fix is a broker-side grant (LITERAL WRITE + DESCRIBE on the omniclaude event topics for that principal), an operator-gated broker change this PR does not make. Records stay in the journal, un-acked, and replay once the grant exists.

Lab: host h201, installed as a systemd --user unit, active/running with NRestarts=0. Publishes are refused by broker ACL, so the queue is not drained yet.

Evidence-Ticket: OMN-20309
Evidence-Source: OCC#12170

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 14
Models succeeded: qwen3-review,gpt-oss-review

Findings

Severity	Location	Finding
error	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:qwen3-review:security Shell injection via unescaped sed substitution in systemd unit rendering | The installer renders the systemd unit by piping OMNI_HOME_RESOLVED and LINUX_PYTHON through sed with the 's|TOKEN|VALUE|g' pattern. The substitution value is not escaped for sed's replacement syntax. If OMNI_HOME_RESOLVED contains a pipe character, ampersand, or backslash, the rendered unit file will contain corrupted or arbitrary text. More critically, if the path contains a newline, the rendered unit will be split across lines
warning	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:qwen3-review:correctness systemd-analyze verify check is ineffective and misleading | The installer runs 'systemd-analyze --user verify' and only treats it as a failure if the output matches a narrow grep for 'unknown section|unknown key|failed to parse|bad unit'. systemd-analyze verify reports many other error conditions (missing ExecStart binary, invalid Environment syntax, unknown directives in newer systemd versions) that will not match this grep. The comment claims 'only a unit that systemd itself cannot parse is refused here'
warning	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:qwen3-review:correctness Redundant restart after enable --now is a race condition on first install | The installer calls 'systemctl --user enable --now' followed immediately by 'systemctl --user restart'. On a first install, the unit is not yet running, so 'enable --now' starts it. The subsequent 'restart' then stops and starts the service again, causing a brief window where the drainer is not running. On a reinstall, the comment claims 'enable --now on an already-running unit leaves the old process', but systemctl enable --now on
warning	tests/scripts/test_install_hook_emit_drainer_systemd.py:1	ai-reviewer:qwen3-review:completeness No test for the sed rendering corruption path or special characters in OMNI_HOME | The test suite covers the happy path, missing interpreter, uninstall, and status. There is no test that exercises OMNI_HOME values containing characters that would break the sed substitution (pipe, ampersand, backslash, newline). Given the critical security finding above, the absence of a regression test for this input class means the vulnerability is untested and likely to persist. | Evidence: def test_install_writes_a_user_
info	scripts/systemd/ai.omninode.hook-emit-drainer.service:1	ai-reviewer:qwen3-review:performance Unbounded log file growth via StandardOutput=append with no rotation | The systemd unit configures StandardOutput and StandardError to append to a log file with no size limit or rotation mechanism. Over time, this log will grow without bound, consuming disk space. The launchd plist counterpart likely has a similar limitation, but the new systemd unit does not reference any logrotate configuration or systemd journal integration that would mitigate this. | Evidence: StandardOutput=append:OMNI_HOME/.onex_s
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:qwen3-review:correctness print_backlog uses nullglob but does not handle the case where the journal directory does not exist | The print_backlog function uses 'shopt -s nullglob' and globs the journal directory. If the journal directory does not exist, the glob expands to an empty array, which is correct. However, the function also echoes the journal path unconditionally, which is fine. The issue is that the function is called from --status, which is expected to work even when the drainer has never been installed. The function does
info	tests/scripts/test_install_hook_emit_drainer_systemd.py:1	ai-reviewer:qwen3-review:testing Test for status when unit is missing does not verify the exit code is 1 | The test test_status_fails_when_the_unit_was_never_installed asserts that the return code is non-zero and that the output contains the installer command. It does not assert the specific exit code of 1, which is what the installer script produces. This is a minor gap in test precision. | Evidence: def test_status_fails_when_the_unit_was_never_installed(tmp_path: Path) -> None: """A missing unit is a red status naming the repair, ne
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:qwen3-review:architecture Supervisor selection by command presence is fragile in containerized or minimal environments | The installer selects the supervisor by checking for launchctl or systemctl on PATH. In a container or minimal Linux environment, systemctl may be present but systemd may not be running as PID 1, in which case 'systemctl --user' will fail with 'System has not been booted with systemd'. The installer does not check whether systemd is actually the init system before proceeding. This will produce a confusing error at
warning	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:security Unsanitized path values injected into rendered systemd unit | The script substitutes OMNI_HOME and PYTHON directly into the unit file via sed without quoting or validation. If either variable contains spaces, shell metacharacters, or malicious content, the resulting unit may execute unintended commands or fail to start, constituting a command injection risk. | Evidence: Lines 71‑78: sed -e "s|OMNI_HOME|${OMNI_HOME_RESOLVED}|g" -e "s|PYTHON|${LINUX_PYTHON}|g" "${SRC_UNIT}" > "${RENDERED}" | F
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:correctness Supervisor detection prefers launchctl even when systemctl is also present | The detection logic selects launchctl if it exists, ignoring the possibility that a macOS host may also have systemctl installed (e.g., via Homebrew). This can lead to the script taking the launchd path when the user intended the systemd branch, causing unexpected behavior. | Evidence: Lines 45‑50: if command -v launchctl >/dev/null 2>&1; then SUPERVISOR=launchd elif command -v systemctl >/dev/null 2>&1; then SUPERVISOR=systemd | F
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:correctness Missing error handling after enabling the systemd unit | The script calls systemctl --user enable --now "${UNIT}" without checking its exit status before proceeding to systemctl --user restart "${UNIT}". If enabling fails, the subsequent restart may produce confusing errors or leave the unit inactive. | Evidence: Lines 124‑127: systemctl --user enable --now "${UNIT}"; systemctl --user restart "${UNIT}" | Fix: Capture the exit code of the enable command and abort or report a clear error if it is non‑zero
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:performance Potential memory blow‑up when printing backlog for large journal directories | The print_backlog function expands all *.json files in the journal and spool directories into Bash arrays, which loads the full list into memory. With a very large number of pending or spooled files this could exhaust memory. | Evidence: Lines 58‑66: shopt -s nullglob; local pending=("${journal}"/.json); local spooled=("${spool}"/.json) | Fix: Iterate over the directory entries with a loop (e.g., `for f in "${journal}"/*.js
warning	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:testing Systemd‑analyze verification path not covered by tests | The installer invokes systemd-analyze --user verify when the binary is present, but the test suite does not stub or control this binary. On CI runners where systemd-analyze exists, the verification may run against the rendered unit and cause test failures or mask errors, leaving an untested execution path. | Evidence: Lines 92‑101: if command -v systemd-analyze >/dev/null 2>&1 && ! systemd-analyze --user verify "${RENDERED}" >/dev/null 2>&1; then
info	scripts/install-hook-emit-drainer.sh:1	ai-reviewer:gpt-oss-review:security Potential overwrite of existing user unit without ownership check | The installer writes the rendered unit file to ${XDG_CONFIG_HOME:-${HOME}/.config}/systemd/user/${UNIT} unconditionally, overwriting any existing file regardless of its ownership or permissions. A malicious user could pre‑create a unit with elevated privileges that the installer would silently replace, possibly leading to privilege escalation if the attacker later restores the original file. | Evidence: Lines 118‑121: mkdir -p "${UNIT_DIR

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	Reviewer ran but every model failed mid-run	No (pilot)
infra_error	cli_review failed to run at all (install/crash) after 2 attempts	Yes
unavailable_fork_isolation	Fork PR — the LAN-only model is unreachable and the fleet is off limits	Yes

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)