Skip to content

fix(OMN-20389): hooks feed variable-size text through a process substitution, never a here-string - #2508

Merged
jonahgabriel merged 2 commits into
devfrom
jonah/omn-20389-here-string-free-hooks
Oct 3, 2026
Merged

jonahgabriel merged 2 commits into
devfrom
jonah/omn-20389-here-string-free-hooks

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Ticket: OMN-20389

Problem

Homebrew bash 5.3.9 on macOS deadlocks on a here-string or heredoc body of 512 to 65535 bytes whenever macOS pipe memory is exhausted. XNU sizes each new pipe from system-wide pipe memory use, so under load a fresh pipe holds only 512 bytes. bash writes the body into that pipe inside the process that will go on to read it (sampled stack: execute_disk_command -> do_redirections -> heredoc_write -> write), so the write blocks forever and nothing can drain it. Three lanes lost 25 to 60 minutes each on 2026-10-02: the pre-commit hooks reject-deploy-gate-skip-token.sh and validate-occ-pr-stamp.sh hung 13 to 42 minutes per run on staged files of 1.3 KB to 36 KB, and pr-handoff hung with no output.

Measured with a 512-byte pipe: 511 B ok, 512 B to 65535 B hang, 65536 B and above ok (bash falls back to a temp file). Heredocs, here-strings and the builtin read all hang. A process substitution, printf | cmd and a temp file do not, because the writer is a separate process or a regular file.

Upstream fixed this in bash53-016 (2026-07-07, PIPESIZE_DYNAMIC on darwin). Homebrew stable is 5.3.20; this host has 5.3.9, and nothing here upgrades it.

Change

All 27 here-string lines in the pre-commit hooks (.pre-commit-hooks/) and the tool hook scripts (plugins/onex/hooks/scripts/, plugins/onex/hooks/lib/) become < <(printf '%s\n' "$var"), which keeps the trailing newline a here-string added. Sixteen scripts: the six pre-commit hooks that read staged or file content, the three PreToolUse payload guards (actor line, done flip, lane liveness), the Bash guard entrypoint, the agent result verifier, cost accounting, env var sync, the Kafka poison guard, user-prompt-submit and the refusal recorder.

test_here_string_free_hooks.py adds a static ratchet (no <<< outside a comment in those three directories) and runs the real reject-deploy-gate-skip-token.sh and validate-occ-pr-stamp.sh under /opt/homebrew/bin/bash on 60 KB and 64 KB payloads with a 60 s timeout. The one existing test that asserted the refusal recorder used a here-string now asserts the process substitution.

plugins/onex/.claude-plugin/plugin.json 2.4.19 to 2.4.20 with the matching onex entry in plugins/onex-dev-marketplace.

Not changed

  • Heredocs with a constant body (python3 - <<'PY', cat > f <<EOF) carry the same hazard above 512 bytes and are a separate sweep.
  • The vendored copies of reject-deploy-gate-skip-token.sh in omnimarket, omnibase_infra, omnibase_core, onex_change_control, omnibase_compat, omnibase_spi and omnimemory each keep two here-string lines.
  • The hang needs ambient pipe pressure, so the payload tests prove the changed hooks finish and keep their verdicts, not that the pressure case is gone; the static ratchet is the guard for that.

Draft: codex=01a0fe09-d407-7c82-a2e4-f802401c5bbb (high effort), reviewed and amended by the lane.

Verification

Focused, on the launching host: 42 passed (the new file plus the skip-token, OCC stamp, defect anchor, handoff infra health and UTC discipline hook tests); 220 passed across the done-flip, lane liveness, agent result verifier, refusal row, shell input safety and actor line guard tests; pre-commit run on the changed files clean; ruff and mypy strict clean on the new test. tests/hooks_system/test_bash_guards_differential.py shows 16 mismatches that are all the /tmp versus /private/tmp macOS symlink spelling in worktree-path reasons (none mention a here-string).

Evidence-Source: OCC#12404
Evidence-Ticket: OMN-20389

…itution, never a here-string (plugin 2.4.20)

Homebrew bash 5.3.9 writes a here-string body of 512 to 65535 bytes into a pipe inside
the process that will read it. When macOS pipe memory is exhausted a fresh pipe holds
512 bytes, the write blocks forever and the hook hangs with no output. Replaces all 27
here-string lines in the pre-commit hooks and the tool hook scripts with
< <(printf '%s\n' "$var"), whose writer is a separate process. Adds a static ratchet
and 60 KB and 64 KB payload runs of reject-deploy-gate-skip-token.sh and
validate-occ-pr-stamp.sh under Homebrew bash.

Draft: codex=01a0fe09-d407-7c82-a2e4-f802401c5bbb
Onex-Lane: mac-bash-herestring-2a21
Onex-Session: bf22f698e0ec441e8ce65c6e67d86040
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Lab: head=e1e7927566e837a86d2a01d44d64eec84fcd437b host=launching-mac (Darwin 25.6, /opt/homebrew/bin/bash 5.3.9) lane=mac-bash-herestring-2a21 command=during ambient pipe pressure (a fresh pipe measured at 512 bytes before and after each run), /opt/homebrew/bin/bash .pre-commit-hooks/reject-deploy-gate-skip-token.sh on a 61440-byte markdown file, this head versus origin/dev's copy of the hook observed=this head exited 0 in three of three pressure windows; the origin/dev copy hung to the 5 s timeout in three of three (and in the earlier window at 8 s). Focused suites: 42 passed (new ratchet and 60/64 KB payload tests plus the five hook suites), 220 passed across the guard suites. No runtime or tenant path is touched, so no onex-api surface applies.

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

✅ Hostile Reviewer — PASSED

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

Findings

Severity Location Finding
error plugins/onex/hooks/scripts/post_tool_use_agent_result_verifier.sh:1 ai-reviewer:qwen3-review:correctness Process substitution breaks command substitution exit code capture | In post_tool_use_agent_result_verifier.sh, the change from <<< "$TURN_BODY" to < <(printf '%s\n' "$TURN_BODY") inside a command substitution $( ... ) alters how the exit status is captured. With here-strings, the exit code of the last command in the pipeline (the Python invocation) is what VERIFIER_EXIT=$? captures. With process substitution, the shell forks a subshell for <(...), and the exit status of the process substitution i
error plugins/onex/hooks/scripts/post_tool_use_cost_accounting.sh:1 ai-reviewer:qwen3-review:correctness Process substitution breaks exit code capture in cost accounting hook | In post_tool_use_cost_accounting.sh, the same pattern applies: INJECTION_OUTPUT="$( ... "$PYTHON_BIN" -m ... < <(printf '%s\n' "$HOOK_EVENT") 2>>"$LOG_FILE" )" followed by PYTHON_EXIT=$?. The process substitution <(...) spawns a background subshell whose exit status is not the same as the foreground Python process's exit status. If the Python process fails, PYTHON_EXIT may not reflect that failure, causing cost accounting to sil
warning plugins/onex/hooks/scripts/pre_tool_use_bash_guards.sh:1 ai-reviewer:qwen3-review:correctness Process substitution in source command may not propagate exit code | In pre_tool_use_bash_guards.sh, the line _obg_out="$(source "${_OBG_GUARDS[$1]}" < <(printf '%s\n' "$_OBG_INPUT"))" uses process substitution to feed input to source. The source builtin reads from the process substitution, but the exit code of source when fed via process substitution may not be reliably captured by _obg_rc=$?. The original here-string form guaranteed that the exit code of source was the exit code of the last co
warning .pre-commit-hooks/check-utc-timestamp-discipline.sh:1 ai-reviewer:qwen3-review:correctness Process substitution in while-read loop may lose trailing newline semantics | In check-utc-timestamp-discipline.sh, the change from done <<<"$content" to done < <(printf '%s\n' "$content") alters the input to the while loop. Here-strings append a trailing newline if the variable does not already end with one. printf '%s\n' always appends a newline. If $content already ends with a newline, printf '%s\n' adds a second one, creating an empty final line. The while loop will process this empty line, wh
warning .pre-commit-hooks/check-utc-timestamp-discipline.sh:1 ai-reviewer:qwen3-review:performance Process substitution spawns subshells in tight loops | In check-utc-timestamp-discipline.sh, the while loop now spawns a process substitution for every line: if grep -qiE "$MTIME_MARKER" < <(printf '%s\n' "$line") && grep -qE "$Z_TIME" < <(printf '%s\n' "$line"). For a file with N lines, this spawns 2N subshells plus N process substitutions. For large files, this is a significant performance regression compared to the here-string form, which used no subshells. The same pattern appears in check-defect-anch
warning plugins/onex/hooks/lib/hook_refusal.sh:1 ai-reviewer:qwen3-review:security Process substitution in hook_refusal.sh may leak payload to /dev/null | In hook_refusal.sh, the change to < <(printf '%s\n' "$payload") inside a command substitution that is backgrounded with & and has its stdout/stderr redirected to "${LOG_FILE:-/dev/null}" means the process substitution's subshell runs in the background. If the process substitution fails to start (e.g., due to resource limits), the error may be silently swallowed. More importantly, the payload is now passed through a subshell that i
warning tests/hooks/test_here_string_free_hooks.py:1 ai-reviewer:qwen3-review:testing Test only covers macOS Homebrew bash, not other affected environments | The new test file test_here_string_free_hooks.py uses a fixture that prefers /opt/homebrew/bin/bash and falls back to shutil.which("bash"). This only tests the specific macOS Homebrew bash that is affected by the pipe deadlock. The process substitution change may introduce different issues on Linux bash, zsh, or other shells. The test does not verify that the hooks work correctly on non-macOS systems. Additionally, the test only che
warning tests/hooks/test_here_string_free_hooks.py:1 ai-reviewer:qwen3-review:completeness Test does not cover all modified hooks | The new test file only tests three hooks: reject-deploy-gate-skip-token.sh, validate-occ-pr-stamp.sh, and the ratchet test for here-strings. The diff modifies at least 10 other hook scripts (check-defect-anchor.sh, check-handoff-infra-health.sh, check-utc-timestamp-discipline.sh, reject-imperative-skill-patterns.sh, post_tool_use_agent_result_verifier.sh, post_tool_use_cost_accounting.sh, post_tool_use_env_var_sync.sh, post_tool_use_kafka_poison_message_guard.sh, pre
info plugins/onex/hooks/scripts/user-prompt-submit.sh:1 ai-reviewer:qwen3-review:correctness Process substitution in user-prompt-submit.sh may change jq exit code semantics | In user-prompt-submit.sh, the change from jq -e ... <<< "$_POLL_RESULT" to jq -e ... < <(printf '%s\n' "$_POLL_RESULT") may alter the exit code behavior. jq -e returns exit code 1 if the output is false or null. With process substitution, the exit code of jq is still captured by the if condition, but the process substitution's subshell may interfere with signal handling or error reporting. The `>/dev/null 2>/dev/null
hint .pre-commit-hooks/check-defect-anchor.sh:1 ai-reviewer:qwen3-review:style Inconsistent quoting in process substitution | Some process substitutions use single quotes around the printf format string (< <(printf '%s\n' "$var")) while the original here-strings had no quoting issues. This is consistent, but the mix of single and double quotes in the same expression can be confusing. More importantly, the printf '%s\n' format string is not quoted in some places, which could be a problem if the variable contains glob characters. However, since the variable is in double quotes, this
info plugins/onex/hooks/scripts/post_tool_use_env_var_sync.sh, check-utc-timestamp-discipline.sh, and other scripts using the same pattern:1 ai-reviewer:gpt-oss-review:correctness Empty input handling changed for while-read loops | Several scripts replace here-strings (<<<"$content") with process substitution using printf. A here-string always supplies a trailing newline, causing a read loop to execute once even when the variable is empty. printf without a newline yields no output, so the loop runs zero times. This alters semantics for empty files or empty variables, potentially skipping validation checks that rely on at least one iteration. | Evidence: In check-utc-timestamp-discipl
info Multiple .pre-commit-hooks/*.sh files:1 ai-reviewer:gpt-oss-review:performance Unnecessary subshells introduced by process substitution | Replacing simple here-strings with < <(printf ...) spawns an extra subshell for each grep or read operation. While functionally equivalent, the added process creation incurs modest CPU and memory overhead, especially in hooks that run on many files. | Evidence: All pre-commit hook scripts now use grep -qE "$PATTERN" < <(printf '%s\n' "$var") instead of grep -qE "$PATTERN" <<<"$var". | Fix: Retain here-strings for read‑only pattern matching, or
info plugins/onex/hooks/lib/hook_refusal.sh:1 ai-reviewer:gpt-oss-review:security Potential format‑string exposure in printf usage | The new pattern uses printf '%s\n' "$payload". If future modifications replace the format string with a variable, untrusted payload could be interpreted as a format string, leading to information disclosure or crashes. | Evidence: hook_refusal.sh now contains printf '%s\n' "$payload" within a process substitution. | Fix: Document that the format string must remain a constant literal and enforce linting rules to prevent accidental substitution with user‑

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)

Merging origin/dev (5e71656) into jonah/omn-20389-here-string-free-hooks for PR 2508 (ticket OMN-20389). Two version-line conflicts resolved in plugins/onex/.claude-plugin/plugin.json and plugins/onex-dev-marketplace/.claude-plugin/marketplace.json. Branch had plugin version 2.4.20; dev has 2.4.22. Version set to 2.4.23, one above dev, keeping the PR's version bump. No hook script conflicts.
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Lab: head=06346da4663c462f7ff6527d46cc7aa23fe1697a. Host h201 (Linux lab host) executed the command "pytest tests/hooks/test_here_string_free_hooks.py tests/hooks/test_refusal_row_lane_omn19381.py" in a worktree of this head for lane claude-2508-conflict-2a21. The run observed 76 passed in 1.44s. This head is a merge of origin/dev (5e71656) into the branch; the only conflicts were the plugin version lines (plugin.json and marketplace.json, resolved to 2.4.23), so the hook scripts are unchanged from the head e1e7927 whose Mac pipe-pressure measurement is in the earlier Lab comment. No runtime or tenant path is touched, so no onex-api surface applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant