fix(OMN-20262): lane identity reads ONEX_LANE and the pr-close refusal prints an absolute CLI path

[jonahgabriel]

OMN-20262

The remote-lane runner exports only ONEX_LANE. The pr-ownership guard (and the claim CLI, which calls the guard's resolver) read ONEX_LANE_ID only, so on lab hosts the claim CLI exited 1 with no lane identity. The refusal also printed a cwd-relative CLI path.

Changes: ONEX_LANE added to the lane env lookup after ONEX_LANE_ID (one resolver serves guard and CLI). Refusal messages print an absolute CLI path resolved from the guard's own location, falling back to the canonical clone under OMNI_HOME in the plugin cache layout. Golden record normalizes the clone path. Plugin version bumped to 2.4.5.

Proof: claim CLI run with only ONEX_LANE set against a scratch state dir succeeded (rc 0, claim file written). Hot-patched both installed plugin cache copies; guard prints the absolute path. Focused unit and cache-layout tests pass; the differential test passes except two worktree-guard cases that compare /tmp with /private/tmp on macOS, unrelated to this change.

Lab pass: not runtime-affecting for services; hook and CLI only.

Evidence-Ticket: OMN-20262
Evidence-Source: OCC#12092

[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	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:qwen3-review:correctness Shell injection via unescaped target_key in _claim_command | The _claim_command function constructs a shell command string by interpolating target_key into a single-quoted string without escaping single quotes. If target_key contains a single quote (e.g., a malicious PR title or a crafted key), the shell command will break out of the quoting context, allowing arbitrary command execution. The previous implementation had the same flaw, but this PR is the point where the function is refactored and the path is
warning	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:qwen3-review:correctness _cli_path fallback returns non-existent path when OMNI_HOME is unset | When the source layout check fails (e.g., in a plugin cache environment) and OMNI_HOME is not set, _cli_path returns the source path which is known to not exist. The docstring acknowledges this fallback, but the function does not raise an error or return a sentinel value. Callers like _claim_command will generate a command pointing to a non-existent file, leading to confusing runtime failures instead of a clear diagnostic. The function s
warning	src/omniclaude/hooks/contracts/capture_redaction.yaml:1	ai-reviewer:qwen3-review:security capture_verbatim for goal_id and parent_goal_id may leak sensitive data | The capture_redaction.yaml change adds goal_id and parent_goal_id to the capture_verbatim list. While the comment states 'goal ids, never content', goal IDs in some systems may encode sensitive information (e.g., user identifiers, project codes, or sequential counters that reveal volume). If goal IDs are not purely opaque random strings, capturing them verbatim in logs or telemetry could leak information. The justification 'never cont
info	tests/unit/hooks/test_pr_ownership_guard.py:1	ai-reviewer:qwen3-review:completeness Missing test for _cli_path OMNI_HOME fallback | The new cli_path function has a fallback to $OMNI_HOME when the source layout is not available, but no test covers this code path. The existing test test_refusal_prints_absolute_existing_cli_path only verifies the source layout case. If the OMNI_HOME fallback is broken (e.g., wrong path construction), it will not be caught by the test suite. | Evidence: def test_refusal_prints_absolute_existing_cli_path() -> None: from plugins.onex.hooks.lib.pr_ownership
info	tests/hooks_system/fixtures/bash_guard_golden.json:1	ai-reviewer:qwen3-review:testing Golden test fixture uses {CLONE} placeholder that may not be resolved in all contexts | The bash_guard_golden.json fixture now uses {CLONE} as a placeholder for the repo root path. The bash_guard_corpus.py normalizer adds REPO_ROOT to the substitution pairs, but if the test runner's REPO_ROOT does not match the actual path used by the guard at runtime, the golden test will fail. The coupling between the fixture and the runtime environment is fragile. | Evidence: "reason": "REFUSED (api-pr-close) on OmniNode
info	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:qwen3-review:architecture ONEX_LANE added to _LANE_ENV_VARS without versioning or deprecation strategy | The addition of ONEX_LANE to _LANE_ENV_VARS introduces a new environment variable that is now treated as equivalent to ONEX_LANE_ID. There is no indication of a deprecation path for ONEX_LANE_ID or a versioning mechanism to distinguish between the two. If both variables are set, ONEX_LANE_ID takes precedence (as tested), but the long-term maintenance burden of supporting two variables with the same semantics is unclear. | Evidenc
warning	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:security Path construction from untrusted OMNI_HOME | The new _cli_path function builds an absolute script path using the OMNI_HOME environment variable without validation. A malicious actor could set OMNI_HOME to a directory containing a crafted pr_claim_registry_cli.py, causing the guard to execute attacker‑controlled code. | Evidence: _cli_path fallback: workspace = os.environ.get("OMNI_HOME", "").strip(); return str(Path(workspace) / "omniclaude" / relative) | Fix: Validate that OMNI_HOME points to
warning	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:correctness _cli_path assumes fixed directory depth | _cli_path accesses Path(__file__).resolve().parents[4]. If the module is moved or packaged differently, the parent chain may be shorter, raising an IndexError and crashing the guard. | Evidence: source = Path(__file__).resolve().parents[4] / relative | Fix: Compute the base directory relative to the package root using import importlib.resources or Path(__file__).parent traversal with a safety check, and raise a clear error if the expected layout is not
info	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:security Shell command not safely quoted | _claim_command builds a command string with the absolute script path inserted directly: python3 {_cli_path()} claim .... If the resolved path contains spaces or shell metacharacters, the command may be parsed incorrectly or enable injection. | Evidence: return f"python3 {_cli_path()} claim '{target_key}' --action close" | Fix: Quote the script path (e.g., shlex.quote(_cli_path())) or use a list argument form when invoking subprocesses to avoid shell parsing.
info	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:correctness _cli_path may return a non‑existent file | If neither the source location nor the OMNI_HOME fallback contains the script, _cli_path returns a path that does not exist, leading to runtime failure when the command is executed. | Evidence: if source.is_file(): return str(source); ... return str(source) | Fix: After determining the candidate path, verify Path(path).is_file() and raise a descriptive exception if the script cannot be located.
info	src/omniclaude/hooks/contracts/capture_redaction.yaml:1	ai-reviewer:gpt-oss-review:privacy Goal identifiers captured verbatim | The capture_redaction.yaml contract now records payload.goal_id and payload.parent_goal_id with capture_verbatim. These identifiers may be internal or sensitive, potentially violating data minimisation policies. | Evidence: Added lines: payload.goal_id: capture_verbatim and payload.parent_goal_id: capture_verbatim | Fix: Assess whether goal IDs need redaction; if they are sensitive, replace capture_verbatim with a hashing or masking strategy.
info	tests/hooks_system/fixtures/bash_guard_golden.json:1	ai-reviewer:gpt-oss-review:testing Golden JSON expects placeholder {CLONE} without substitution logic | Multiple entries in bash_guard_golden.json were updated to contain {CLONE} placeholder, but the test harness does not replace this token with the actual repository path, causing false negatives. | Evidence: Diff lines showing python3 {CLONE}/scripts/pr_claim_registry_cli.py in expected output. | Fix: Add a post‑processing step in the test suite to substitute {CLONE} with REPO_ROOT or update the comparison logic to ignore the pl
info	plugins/onex/hooks/lib/pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:architecture Guard library now depends on repository layout | pr_ownership_guard.py resolves the CLI script path by navigating four parent directories from its own file location. This couples the guard to a specific source tree structure, reducing reusability and breaking modular layering. | Evidence: source = Path(__file__).resolve().parents[4] / relative | Fix: Inject the CLI path via configuration or environment variable, or locate the script using package resources rather than hard‑coded relative paths.
info	tests/unit/hooks/test_pr_ownership_guard.py:1	ai-reviewer:gpt-oss-review:testing Unit test assumes CLI script exists at resolved absolute path | test_pr_ownership_guard.py asserts that the path returned by _claim_command is an existing file. In environments where the scripts directory is not present (e.g., packaged installations), this test will fail despite correct behavior. | Evidence: assert Path(cli).is_absolute(); assert Path(cli).is_file() | Fix: Mock _cli_path in the test or relax the assertion to verify that the path is correctly constructed rather than physically pres

---

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)