Skip to content

fix(claude-goal-mode): refuse an unreadable PreToolUse event instead of exiting with no decision - #6227

Merged
huangruiteng merged 4 commits into
loopx-project:mainfrom
JasonBuildAI:codex/claude-goal-hook-fail-closed-c2w9
Oct 12, 2026
Merged

huangruiteng merged 4 commits into
loopx-project:mainfrom
JasonBuildAI:codex/claude-goal-hook-fail-closed-c2w9

Conversation

@JasonBuildAI

@JasonBuildAI JasonBuildAI commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / optional anchor: reproduced defect — [Bug]: goal-mode PreToolUse gate exits rc=1 with no decision on an unreadable event instead of the documented fail-closed deny #6225; the class is a named, deliberately deferred successor of merged PR fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166.

  • Goal/source and gap: the opt-in --harden Claude Code PreToolUse gate (loopx/claude_goal_mode/hooks/goal_policy.py) documents itself as fail-closed: when the deterministic should_run probe cannot be reached, a non-read-only tool is denied "so a flaky/broken control plane pauses delivery instead of running ungated". The event read was the one boundary outside that rule. Four input classes made the hook exit rc=1 with a traceback and print no decision: an undecodable stdin byte (strict stdin pin, fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166), malformed JSON, JSON that is not an object, and an object with no usable session cwd (including a non-object workspace). Independent review then found a fifth, same-class input on the gated tools: a truthy non-object tool_input made decide crash on ti.get whenever should_run answered true, the same rc=1 fail-open signature (f5505fc7). Review of that head then found two retained gaps at the same read boundary: a decoded session cwd string that is not a resolvable path (an embedded NUL raises ValueError inside active_context) still exited rc=1 with no decision, and ev.get("tool_input", {}) or {} coerced a present falsy non-object ([] / false / 0 / "") into {}, so an armed Bash call with the gate open was allowed as if it carried no input. Probing the same read-boundary rule then added the remaining untyped values: a missing/non-string tool_name crashed the read-only/gated membership tests, a non-string command crashed .lower(), and a non-string file_path / notebook_path silently fell back to the project root (inside write_scope). All are fixed in 0dfec2170, which validates every event value decide uses before consulting the probe. Claude Code treats a hook that exits with a code other than 0 or 2 as a non-blocking error, so the tool call was not blocked and the gate silently disappeared exactly when the hook could not verify anything.

  • Observable before → after, with the validation row that proves it: tests/test_claude_goal_mode_hook_fail_closed.py runs the shipped script as Claude Code invokes it (own process, event on stdin, decision read back as JSON) plus decision-level cases against an armed context with should_run stubbed true. At base adb247e5f with this revision's test file: 26 failed / 3 passed — the unreadable payloads and untyped values still exit rc=1 with empty stdout and a traceback (UnicodeDecodeError; TypeError: argument should be a str or an os.PathLike object where __fspath__ returns a str, not 'NoneType'; AttributeError: 'list' object has no attribute 'get'; ValueError: embedded null character in path), while the falsy-tool_input and untyped-value cases are answered by the probe/allow branch instead of the unreadable-input rule; the three base passes are exactly the two preserved defaults (the read-only exception and the absent-tool_input default) and the unarmed no-op. At this revision: 29 passed — every case answers an explicit decision (deny for an unreadable event, allow for the read-only and absent-tool_input defaults, {} for the unarmed project).

  • Issue/task and intended base: Closes [Bug]: goal-mode PreToolUse gate exits rc=1 with no decision on an unreadable event instead of the documented fail-closed deny #6225 (follow-up successor named by fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166). Intended base: main at adb247e5f.

Author Declaration

  • Written by: model_agent — an OpenAI Codex model (GPT-5-family) running in a terminal, directed and reviewed by the GitHub account owner.

AI disclosure: this change was drafted with AI assistance at my request, under my direction and review.

Implemented against

Criterion (spec clause) Disposition Symbol / path Test or command
An event the hook cannot read is answered with an explicit deny instead of rc=1 with no decision (issue "Expected behavior") implemented main, decide in loopx/claude_goal_mode/hooks/goal_policy.py tests/test_claude_goal_mode_hook_fail_closed.py::test_unreadable_event_is_refused_with_an_explicit_deny (6 payload ids); ...::test_non_object_tool_input_for_a_gated_tool_is_refused
A session cwd that cannot be used as a path is answered with an explicit deny instead of exiting with no decision (review P1) implemented _usable_cwd, decide in loopx/claude_goal_mode/hooks/goal_policy.py ...::test_unusable_cwd_is_refused_with_an_explicit_deny (2 payload ids, real entry); ...::test_decide_refuses_an_unusable_cwd_before_resolving_the_project
tool_input is type-checked before any default; a falsy non-object is refused while the read-only exception and the absent-field default stay (review P2) implemented decide ...::test_falsy_non_object_tool_input_is_refused_not_coerced (4 ids); ...::test_decide_refuses_falsy_non_object_tool_input_when_the_gate_is_open (4); ...::test_decide_keeps_the_empty_input_default_for_an_absent_tool_input; ...::test_decide_keeps_the_read_only_exception_for_unreadable_input
Every other event value decide uses is typed before use (tool_name, command, file_path / notebook_path), refused the same way implemented decide ...::test_unreadable_event_value_is_refused_with_an_explicit_deny (4 ids); ...::test_decide_refuses_unreadable_event_values_when_the_gate_is_open (4)
A valid event in a project where goal-mode is off keeps answering {} (defer to the normal permission flow) implemented same ...::test_unarmed_project_still_defers_to_the_normal_flow
Strict stdin stays strict: a non-UTF-8 byte is never repaired into a different event (#6166 contract) implemented main catches ValueError (which covers UnicodeDecodeError) and answers the deny; stdio_utf8.pin_utf8_stdio unchanged the undecodable-stdin payload above; tests/test_claude_goal_mode_stdio_utf8.py (5 passed)
The documented behavior list and README stay in sync with the code implemented module docstring; README.md "Optional hardening" section examples/claude-goalmode-hook-policy-smoke.py (extended with the denial row, the unusable-cwd check, the falsy/untyped-value checks and the preserved defaults)
No new dependency, protocol field, persisted state or vocabulary implemented diff scope python scripts/generate_semantic_inventory.py --changed-from HEAD -> "No supported new vocabulary carriers were detected."
  • Self-check before submission: ran the reproduction against a clean checkout of the base commit and at this revision, with the shipped script as a subprocess; ran the adjacent stdio suite, the hook-policy smoke, the CI ruff scope, the architecture suites that bound the area, and the public-safe-outbound diff scrub. Deliberately left out: driving a live Claude Code session (its real events are well-formed UTF-8 JSON, so it cannot emit the reproduction inputs); the launcher residual fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166 declared (the installer baking a bare python3 command) is untouched.

Scope And Continuation

  • Completed scope and remaining work: the successor fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166 named — an unreadable gate event now produces the explicit fail-closed deny for both entry paths (main for the CLI hook, decide for the in-process Agent SDK hook). Remaining, out of scope: the onFailure: "block" per-hook setting that Claude Code v2.1.295+ offers (the hook-level deny is version-independent, reaches existing hardened installs without re-running --harden, and also covers the SDK path), and the bare-python3 launcher residual named by fix(claude-goal-mode): pin the Claude Code goal-mode entry stdio to UTF-8 #6166.
  • Slice boundary / successor: complete within this scope; no successor is required for the repair itself. The change is three commits touching the gate, its documented contract, one smoke and one regression file, so it is reversible by reverting them; the review-response commit 0dfec2170 is independent of the two preceding ones.

Validation

  • Tested revision: 0dfec21
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
static passed uv run --extra test python -m ruff check tests loopx/canary loopx/control_plane loopx/domain_packs loopx/presentation loopx/claude_goal_mode -> "All checks passed!". mypy is not applicable: loopx/claude_goal_mode is outside its configured files list in pyproject.toml.
regression_parity passed pytest tests/test_claude_goal_mode_hook_fail_closed.py against a clean checkout of base adb247e5f -> 26 failed, 3 passed; at this revision -> 29 passed. Same file, same payloads, only the revision differs; the three base passes are the two preserved defaults and the unarmed no-op.
regression_parity passed decide-level differential (gate open): at base adb247e5f, tool_input [] / false / 0 / "" answer allow / bash permitted under goal policy, a non-string command and an unhashable tool_name raise, and a NUL cwd raises ValueError out of decide; at this revision every one answers the explicit deny naming the unreadable value. The read-only exception and the absent-tool_input default still answer allow on both revisions.
real_entrypoint passed Every case runs the shipped python loopx/claude_goal_mode/hooks/goal_policy.py in its own process with the event on stdin and reads the decision from stdout - the way the installer wires it into Claude Code - not an in-process approximation.
unit passed pytest tests/test_claude_goal_mode_hook_fail_closed.py tests/test_claude_goal_mode_stdio_utf8.py -q -> 34 passed (29 + 5); python examples/claude-goalmode-hook-policy-smoke.py -> claude-goalmode-hook-policy-smoke ok.
unit passed pytest tests/architecture/test_top_level_module_budget.py tests/architecture/test_control_plane_import_boundaries.py tests/architecture/test_project_registry_io_census.py -q -> 32 passed (the registry-I/O census needs npm ci --ignore-scripts once, installed here).
manual passed `git diff ...HEAD
  • Coverage and gaps: the changed paths are the gate's two entry points (main, decide), the documented behavior list and README, one example smoke, and one regression file that covers the unreadable-input classes, the unusable-path cwd, the falsy/untyped event values, the read-only exception, the absent-tool_input default and the unarmed-project no-op; the existing stdio suite still passes. The changed paths stay inside loopx/claude_goal_mode, so an unusable cwd is refused before the shared registry resolver is called. Untested path: a live Claude Code process (its events are always well-formed, so they cannot exercise the unreadable branch) and the hook's decision display inside it; loopx check could not run on this host because the LoopX effect runtime requires Node 22.22.3+ and only 22.2.0 is installed, so the diff-level public-safe-outbound scan above covers the same boundary for this diff. Residual risk: the rc/stdout contract is asserted against the shipped script only, not against a real Claude Code client.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: none
  • Source data: none
  • Attention review: N/A - no user-visible surface changes; the README bullet describes a gate decision, and this change only adds a refusal path.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: N/A - bounded host-adapter defect repair; no roadmap card, RFC section or acceptance id is claimed.

Shared-authority RFC fixture impact

  • N/A - no progress is claimed against the TypeScript control-plane migration or the shared Goal Authority RFC.

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths (including .loopx/, .codex/goals/, and live ACTIVE_GOAL_STATE.md).
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer (git commit -s).

…of exiting with no decision

The opt-in --harden gate documents itself as fail-closed around the should_run
probe, but the event read was outside that rule: an undecodable byte raised
UnicodeDecodeError at the strict stdin boundary, and malformed JSON, non-object
JSON or an object without a usable session cwd raised inside active_context.
Each case exited rc=1 with a traceback and no decision, which Claude Code
reports as a non-blocking hook error and follows with an ungated tool call.

An event the hook cannot read is now answered with an explicit deny - the same
fail-closed rule the unreachable should_run probe already applies - and a valid
event in a project where goal-mode is off still answers {} (defer to the normal
permission flow).

tests/test_claude_goal_mode_hook_fail_closed.py runs the shipped script the way
Claude Code invokes it (own process, event on stdin, answer read back as JSON):
6 failed / 1 passed at adb247e, 7 passed with this change.

Follow-up to loopx-project#6166, which pinned the adapter stdio and listed this as its
deliberately deferred successor. Fixes loopx-project#6225.

Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
decide() reads the tool input with `ti.get` for the gated tools, so a
PreToolUse event whose `tool_input` is a truthy non-object crashed there
(AttributeError, rc=1, no decision) as soon as should_run was true - the
same fail-open hook error the unreadable-event refusal removed. Refuse
that event with the documented fail-closed deny instead; read-only tools
keep their unconditional allow.

This is the same input boundary as the unreadable-event fix and was
found during the independent review of this branch's change.

Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
@JasonBuildAI

Copy link
Copy Markdown
Contributor Author

Follow-up commit f5505fc7 from an independent, read-only review of this branch (verdict: keep; two non-blocking items). One item was the same input class as this fix: a truthy non-object tool_input on a gated tool still crashed decide on ti.get with rc=1 whenever should_run answered true - the same fail-open signature on a narrower input. It is fixed in f5505fc7, with test_non_object_tool_input_for_a_gated_tool_is_refused and the extended smoke; the deny-reason assertion fails against the pre-fix hook on any host (base either crashes when the probe is open or answers with the probe branch instead of the unreadable-input refusal). The second item, an ambiguous README/doc sentence about read-only tools, is addressed in the same commit.

Validation rows in the description are updated for head f5505fc7: base adb247e5f 7 failed / 1 passed; head 8 passed (13 with the stdio suite); ruff clean; architecture trio 32 passed; semantic-inventory advisory reports no new vocabulary carriers.

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent; model=gpt-6.1-sol; provider=OpenAI; runtime_reported; reasoning_effort=xhigh

动机

评审 head:f5505fc790c01188c7ce76090764e76ec64eb5c2;原基线:adb247e5f3fac8e2e3f7152239955b49480d0b9f。

使用 Claude Code 可选 --harden 的用户,遇到损坏的工具调用事件时,需要收到明确的拒绝,而不是失去门禁决定。原脚本在这些事件上异常退出,用户看到错误后,工具仍进入通常的权限流程。这个 PR 把六种无法解析或缺少目录的事件,以及非空的非对象工具输入,改为明确拒绝;合法写入、只读工具及未启用项目的正常流程应保持。它修复的是可选 hook 的输入边界,不是操作系统沙箱、完整 Claude Code 运行时或新的权限授权。

这个方向有实际价值:损坏事件不再悄悄移除门禁,且修正输入后能继续工作。不过,现有测试通过时仍有两个同一输入边界的遗漏,下面分别列出。它们在原基线也存在,属于本次修复承诺的剩余缺口,不是新引入的回归。

改动思路

既有 main → decide → active_context / should_run 是合适的 owner;无需新增 Core 状态、并行判断源或更换语言。main 将解码/JSON 失败转为缺少目录的事件,decide 提前构造 JSON 拒绝;非空的非对象 tool_input 在进入 should_run 前拒绝,已知只读工具保持放行。异常输出变为明确的决定,符合 Claude Code hooks 的退出码契约:没有有效决定的 exit 1 是非阻断错误。这里验证的是脚本的实际 rc/stdout,不声称已在真实 Claude Code 会话中执行了损坏事件。

归属和体量合理:4 个文件,248 行新增/11 行删除,其中 177 行为 subprocess 回归测试,运行时代码新增 46 行/删除 11 行。未来修改更容易的做法,是在同一事件读取边界区分原始值的类型、缺失与合法默认值;先补齐下面的两项,不引入通用框架。既有 Bash 子串 denylist 的非沙箱局限没有被本 PR 改变,不能将这次输入修复包装成强隔离。

验收参考是 #6225 和修改前 loopx/claude_goal_mode/README.md 的 Optional hardening 契约,版本 adb247e;没有使用本 PR 改写后的 README 为自身背书。条目:HC-1 不可读事件给出明确拒绝;HC-2 合法未启用项目保持正常权限流程;HC-3 严格 UTF-8 不把坏字节修成另一个事件;HC-4 README、实际行为和测试一致。HC-2/HC-3 已核验,HC-1/HC-4 有以下剩余缺口。已有恢复类经验的“先判断真实结果,再看技术覆盖”方法可参考,但本 PR 是既有 host 输入修复,不能继承另一 RFC 的结论或要求它补 unrelated 产品阶段。

具体改动

  • hooks/goal_policy.py:将局部 JSON 决定 helper 提前;检查 cwd/备用 workspace.current_dir 的类型和非空性;为非对象工具输入加检查;main 捕获解码/JSON 的 ValueError 并处理非对象事件。既有 registry、armed marker、Agent 参数转发、write_scope 和 quota owner 保持。
  • tests/test_claude_goal_mode_hook_fail_closed.py:新增 8 个实际 subprocess 用例。独立运行与 stdio 套件合计 13 passed;同一回归文件指向原基线为 7 failed / 1 passed,证明已覆盖的修复有敏感性。
  • examples/claude-goalmode-hook-policy-smoke.py:增加缺少目录、非空非对象输入、只读例外的轻量断言;smoke 通过。它补充真实入口测试,不代替它。
  • README.md:说明新增拒绝路径;仍需避免把“所有非对象输入/不可用目录均已处理”说得比实现更宽。

[P1] HC-1:非空字符串仍可能不是可用的目录。

cwd 为 "/bad\u0000cwd" 的已解码 JSON 通过第 180 行检查,随后第 188 行调用 active_context(cwd);实际 Path.resolve() 抛出 ValueError: embedded null character in path。当前 head 的真实 hook 进程依然是 exit 1、空 stdout、无 permissionDecision,和原基线一样。这是本次“不可用 cwd 应显式拒绝”的同一边界,不是要求正常客户端生成坏输入,也不是要求改外部 Claude Code。请在现有输入边界处理无法作为路径使用的字符串/相应校验失败,输出明确拒绝,并增加 CLI 与 decide 的对应用例;不要让异常逃出决定入口。

[P2] HC-4:类型检查前的 or {} 隐藏了非对象输入。

第 197 行把显式 tool_input: []、false、0 或空字符串转成 {},第 202 行的新检查随后通过。真实 armed 项目、Bash、合法 quota 回答 true 时,tool_input: [] 实测返回 allow / bash permitted under goal policy;原基线也如此。新增 README 却承诺非只读工具的非对象输入会拒绝。请先检查原始字段的类型,再处理允许的缺失默认值;保留已知只读工具的既有例外,并覆盖这些 falsy 非对象值。这是同一个小范围修复,无需另造任务或解析框架。

对主干的风险

验证了真实 registry/armed marker 的合法写入、关闭 gate、未启用及新项目、只读例外、越界写入,以及损坏事件修正后连续调用、关闭再启用的恢复。比较使用相同解释器与合成 fixture;只有未改变的 should_run 回答固定为测试 stub,事件解析、上下文解析、决定和退出码来自实际脚本。默认安装确实不写 hooks;--harden 才生成指向当前脚本的配置,未触碰用户配置或实际项目。未知/损坏事件的新增明确拒绝属于 #6225 的有意改变,合法 off 路径保持 {};没有把无效输入的旧异常当成需要保留的正确行为。

相关检查:13 个 hook/stdio 测试、25 个架构测试、原 smoke、CI 所用 Ruff 范围、语义 advisory 后全树 semantic drift 和公开边界扫描均通过。未查询或等待 CI。两个遗漏独立于这些绿灯;修复后应重跑相同正负例。新增值仍复用既有 allow/deny 输出词汇,没有新增持久状态或共享协议。没有发现跨领域措辞或把强制拒绝叫成建议的问题。

还实际构建并非 editable 安装了当前 head 的干净 wheel,在源码目录外调用其中同字节的 hook:合法写入和 malformed JSON 的显式拒绝保持,NUL cwd 的 exit 1/无决定、空数组输入的 allow 也同样复现。构建前缺少 frontend bundle 的准备失败已保留,按原仓库 dashboard build:chat 流程准备后成功;没有将环境准备失败归因于此 PR。

未验:真实 Claude Code 会话中的决定展示、原生 Windows 控制台和长期调用成本。合法任务/恢复路径的结果保持,局部异常保护方向正向,但不能把这一轮测试数量称为长期效益。两项当前遗漏仍可能让“不可信事件必须给决定”的目标不成立。回退可撤销此独立两提交;更小、合适的当前修复是补齐同一入口的校验,而不是改安装方式或另加 authority。

我的整体评价

REQUEST_CHANGES。问题、owner 和改动规模都合理,已覆盖的入口修复成立;补齐 HC-1 的非法路径字符串和 HC-4 的原始非对象输入即可继续。保持合法/off/只读/作用域及恢复反例,收窄当前输入读取边界,不需要扩展为完整客户端改造。此前独立意见和本次绿灯不能代替这两项真实反例;本文没有合并或升级结论。

English verdict: REQUEST_CHANGES — head f5505fc. The scoped hook repair is useful, but an unusable string cwd still exits 1 without a decision, and falsy non-object tool_input is coerced to {} and allowed. Both are retained gaps, not introduced regressions. Source tests, paired real-hook calls, off/scope/recovery cases and static checks support the findings; no live Claude Code or native Windows claim.

Comment thread loopx/claude_goal_mode/hooks/goal_policy.py
@@ -168,15 +196,17 @@ def decide(ev: dict) -> dict:
tool = ev.get("tool_name", "")
ti = ev.get("tool_input", {}) or {}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The or {} coercion happens before the new dict check. Explicit tool_input: [] (also false/0/empty string) becomes an object, so an armed Bash call with should_run=true is actually allowed rather than refused as the new README promises. Check the original value type before applying the permitted missing-field default, retaining the known read-only exception, and cover falsy non-object inputs. The behavior also exists at the baseline; it is another gap in this input-boundary repair.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复(0dfec2170)。tool_input 改为先看原始值再应用允许的缺失默认:仅缺失/null 取 {},其余值原样进入 isinstance(ti, dict) 检查,[] / false / 0 / "" 不再被 or {} 吞掉;只读例外仍先于类型检查。回归覆盖:真实入口 ...::test_falsy_non_object_tool_input_is_refused_not_coerced(四个 falsy 值,断言 reason 含 tool_input,证明来自不可读规则而非关闭的 gate);decide 级(should_run 置 true)...::test_decide_refuses_falsy_non_object_tool_input_when_the_gate_is_open(四个值,修复前正是 allow / bash permitted under goal policy);保留默认的两条 ...::test_decide_keeps_the_empty_input_default_for_an_absent_tool_input 与 ...::test_decide_keeps_the_read_only_exception_for_unreadable_input。

同一读取边界排查还发现并一并修复了同类未校验值:非字符串 tool_name 会让只读/受门禁工具的集合判断抛异常,非字符串 command 在 .lower() 崩溃,非字符串 file_path / notebook_path 会回落到项目根目录(落在 scope 内)。这些值现在都在 probe 之前被明确拒绝,真实入口与 decide 两级各有 4 个 payload 用例。

@loopx-agent

Copy link
Copy Markdown
Collaborator

Reproduction clarification for review 5491241028 at head f5505fc: the publication readback normalized the escaped NUL example in HC-1 to a visual ^@ representation. Construct the actual JSON value in Python as cwd = "/bad" + chr(0) + "cwd", then serialize it with json.dumps and send UTF-8 bytes to the shipped hook. Do not paste the visually normalized representation as an ordinary path. The source and noneditable-wheel processes return exit 1 and no decision, with an embedded-null-path ValueError. The other text, both inline findings and the REQUEST_CHANGES conclusion are unchanged. This is an input-repair gap already present at the original baseline, not a claim that a normal Claude Code event contains NUL.

Independent review of this branch's fail-closed refusal found two retained
gaps at the same event-read boundary, and probing that same rule found two
more values of the class:

- a decoded session cwd string that is not a resolvable path (an embedded
  NUL raises ValueError inside active_context) still exited rc=1 with no
  decision, so the tool ran with no gate;
- `ev.get("tool_input", {}) or {}` coerced a present falsy non-object
  (`[]`, false, 0, "") into {}, so an armed Bash call with the gate open
  was allowed as if it carried no input;
- a missing/non-string tool_name crashed the read-only/gated membership
  tests, a non-string command crashed `.lower()`, and a non-string file
  path silently resolved to the project root (inside write_scope);

decide now validates every event value it uses (cwd resolvable as a path,
tool_name a non-empty string, tool_input an object or absent/null,
command/file_path/notebook_path strings or absent/null) and answers the
documented explicit deny before consulting the probe. The read-only
exception and the absent-tool_input default are unchanged, and both the
real hook entry point and the decide level pin every case.

Signed-off-by: JasonBuildAI <jasonbuildai@gmail.com>
@JasonBuildAI

Copy link
Copy Markdown
Contributor Author

评审响应对应 head 0dfec2170a0454301c2ff6986807087f682d1105(在 f5505fc7 之上新增 1 个提交)。

P1 不可用 cwd:decide 在 active_context 之前用 _usable_cwd 校验 cwd 是否可作为路径解析(NUL / 代理项 / 解析失败均覆盖),不可用即输出明确拒绝 session cwd is not a usable path。真实进程在该 payload 上由 rc=1、空 stdout、ValueError: embedded null character in path 变为 rc=0 + deny。覆盖:真实入口 2 个 payload(cwd、workspace.current_dir)+ decide 级 1 例(断言 active_context 不被调用)。

P2 原始类型检查:tool_input 仅缺失/null 取 {} 默认,[] / false / 0 / "" 原样进入对象检查并被拒绝;只读例外与缺失默认保持。覆盖:真实入口 4 例 + decide 级 4 例(should_run=true,修复前为 allow)+ 2 条保留默认的用例。

同类排查:同一读取边界的其余未校验值(非字符串 tool_name、非字符串 command、非字符串 file_path / notebook_path)也改为在 probe 之前明确拒绝;真实入口与 decide 级各 4 个 payload。所有校验都发生在 should_run 之前,且不改变合法事件、只读工具、未启用项目的既有行为。

证据:tests/test_claude_goal_mode_hook_fail_closed.py 同一文件在基线 adb247e5f 为 26 failed / 3 passed(3 个 pass 恰为被保留的两个默认与未启用 no-op),本 head 为 29 passed;与 stdio 套件合计 34 passed。smoke、CI ruff 范围、3 个架构套件(32 passed,需 npm ci --ignore-scripts 一次)、公开边界 diff 扫描(PASS: public-safe)均通过。语义 advisory:无新增词汇载体。未验项与此前一致:真实 Claude Code 会话内的决定展示;loopx check 因本机 Node 22.2.0 < 22.22.3 仍不可运行。改动仅在 loopx/claude_goal_mode 内,共享 registry resolver 未改。

请按新 head 继续评审;本地复现命令:uv run --extra test python -m pytest tests/test_claude_goal_mode_hook_fail_closed.py -q。

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=xhigh

评审 head:0dfec2170a0454301c2ff6986807087f682d1105;上轮 f5505fc790c01188c7ce76090764e76ec64eb5c2。原两项遗漏已独立复核解决,未发现阻塞代码问题;另有一项非阻塞文档建议。

动机

使用 Claude Code 可选工具检查的用户,需要损坏的调用事件得到明确拒绝,同时合法任务和未启用项目仍可正常工作。原来不可读事件会异常退出,或把空数组等非对象输入转换成空对象放行;连续调用时用户会失去可靠的工具检查结果。新 head 将这些事件转成明确拒绝,修正输入后可继续执行;只读例外、合法空值默认和未启用项目的原流程保持。本次修复只覆盖已有 hook 的事件读取边界,不是操作系统沙箱,也不新增 Goal、权限或 Claude 运行循环。不修复这些入口,单次测试绿灯仍可能掩盖连续调用中丢失决定或错误放行的缺口。

改动思路

现有 Python 宿主事件入口已经拥有输入解码和拒绝,局部路径检查与显式默认值即可完成修复,无需新增控制面状态或另一决策源。本 PR 边界是同一个 hook 的不可读事件处理、对应回归与说明;默认安装、身份、quota、作用域与实际工具执行仍由原 owner 负责。本轮没有继承上轮结论:同时检查整个 PR 和 f550 到当前新增的修复。路径能解码成字符串,不代表可供文件系统解析;原始值的类型检查也必须先于默认值处理。保留可恢复的合法默认、只读例外、项目标记及原 quota owner,避免简单加强拒绝导致正常工作被挡住。

具体改动

验收依据是修改前 loopx/claude_goal_mode/README.md,版本 adb247e5f3fac8e2e3f7152239955b49480d0b9f,以及 #6225 和上轮评审回应。HC-1 已实现:不可读事件/不可用路径/原始类型错误显式拒绝。HC-2 已实现:合法未启用项目正常流程保持。HC-3 已实现:严格 UTF-8 和不可修复坏字节约定保持。HC-4 已实现:只读、quota、write_scope 及非沙箱边界保持。

关键代码讲解

_usable_cwd(97行)在真正上下文解析前捕获路径解析失败,NUL 示例应以 Python chr(0) 构造,避免把显示转义当实际输入。decide(177行)仅将缺失/null 输入设为空对象;空数组、false、0、空字符串保留原始类型并拒绝。非字符串工具名及 gated 工具的命令/文件字段也在 probe 前校验,合法只读例外仍先执行。main(281行)把严格解码/JSON失败及非对象事件送到同一拒绝路径。

全 PR 四文件共 +567/-13:runtime +97/-13、现有 smoke +53、README +6、焦点测试 +411。原生测试和 smoke 继续覆盖现有稳定边界,没有新增框架或持久协议。上轮 P1 的不可用 cwd、P2 的 falsy 输入,以及同类未校验字段,本次都通过真实入口复核。

对主干的风险

独立运行当前 hook/stdio 套件 34 passed。同一当前29项回归文件在原基线为 26 failed/3 passed,在上轮 f550 为 19 failed/10 passed,当前29项通过;这些基线失败保留为修复敏感性证据。再以本机 authority 的10个Goal、20个注册Agent背景构造隔离场景,实际调用三版 hook、真实 File registry/armed marker和实际agent参数传输:340项语义检查符合当前预期,覆盖旧错误、合法/null/只读、off、越界、closed/unavailable probe、新主体显式启用及修正后连续工作。只有未改动的 quota 回答被明确stub;这些注册标签不等于20个真实Claude/业务会话,不证明各领域业务验收。

当前干净 wheel 也实际非editable安装到独立环境、在源码目录外调用同字节hook:合法写入、NUL拒绝、空数组拒绝、只读例外和非字符串命令共5项通过。构建所需未改动frontend bundle复用前先验证完整 f550→head 只有上述四文件;没有声称重新验收App。repository Ruff、模块Ruff、advisory后semantic drift、原hook/opt-in安装smoke、diff及公开边界检查通过,未读取或等待CI。当前PR复用既有allow/deny词汇,本地输入校验没有新增共享状态;没有域特定Core义务或把硬拒绝叫建议的变更。

[P2,非阻塞] README第58行应明确null默认的例外。 “present but not a string”目前比实现更严格:command: null在三版实际入口均按原空默认允许。建议改成present non-null/non-string并说明缺失/null默认;保留已验证的代码兼容行为。这里是说明精度问题,不要求扩大拒绝范围或新增任务。

我的整体评价

APPROVE。长程效果 improved:已验证的不可读事件有明确决定,修正后继续工作,原合法/off/只读/身份及quota边界保持;体验 improved:修复旧异常和错误默认后,用户得到明确拒绝而不是失去检查,且合法调用没有新增步骤。真实Claude决定展示、Windows及自然长期成本仍未测,不能由测试数量或经验召回推断净收益。机制比例合适;未来重构检查保留一个局部owner和路径helper,无需引入控制面框架。已有恢复RFC经验只作为参考,没有继承其验收或结论;也没有声称记忆造成了本次改善。#6250的解释器命令问题及其它Goal缺口不由本PR关闭。

English verdict: APPROVE - 0dfec21; the prior unusable-cwd and falsy-input gaps are independently resolved while legal, readonly, scoped/off and correction paths remain. 34 native tests, three-revision sensitivity checks,340 metadata-derived semantic cases,5 current noneditable-wheel cases and local static/smoke checks passed. Nullable-field README wording is a non-blocking P2; live Claude/native Windows and natural long-run utility remain untested.

- **denies** an event it cannot read at all (undecodable stdin, malformed or
non-object JSON, a missing or unusable session `cwd`, a missing/non-string
tool name, or - for a non-read-only tool - a non-object `tool_input` or a
`command` / `file_path` / `notebook_path` field that is present but not a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2, non-blocking] Clarify the allowed null/default exception. This wording says a present non-string command/file field is refused, but decide explicitly skips type refusal for None and retains its existing empty default. A real armed hook with command:null and an open unchanged quota answer returns allow in the original baseline, previous head and current head. Please say present non-null/non-string and describe the permitted absent/null defaults; keep the code behavior and legal-path regression. This is documentation precision, not a blocker or a request to widen enforcement.

@huangruiteng
huangruiteng merged commit 9d87c42 into loopx-project:main Oct 12, 2026
4 of 6 checks passed
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.

[Bug]: goal-mode PreToolUse gate exits rc=1 with no decision on an unreadable event instead of the documented fail-closed deny

3 participants