diff --git a/docs/guide/determinism.en.md b/docs/guide/determinism.en.md index e900b9d8..8ebd31be 100644 --- a/docs/guide/determinism.en.md +++ b/docs/guide/determinism.en.md @@ -15,6 +15,25 @@ factlog enforces freshness through two distinct mechanisms: These two levels are complementary: the hook closes the deterministic gap; the SKILL discipline covers the narration layer where engineering enforcement is not possible. +> **An existing KB needs one `/factlog check` first.** In earlier versions the +> hook could not read Claude Code's real payload, so this deny never fired. From +> this fix on, a KB that already has `facts/accepted.dl` or `facts/query.dl` +> while `facts/logic_report.txt` is missing or older must be refreshed with +> `/factlog check` before its engine inputs can be edited. + +The hook denies for **one more reason** besides staleness. It reads the target +path out of the tool payload Claude Code sends; if the call is a `Write`/`Edit` +but the payload shape has changed such that no path can be read — so the write +cannot be shown *not* to target an engine input — the hook denies rather than +letting it through. Only in that situation can you skip the check with +`FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1`. As the name says, it exempts the +unreadable-payload branch and nothing else — it does **not** release the +staleness deny. It is not something the model can do from inside the session: a hook +inherits the Claude Code process environment, so **a human** has to set it there +— in the `env` block of `settings.json`, or exported before launching Claude +Code — and then start a new session. Please also report the payload shape +upstream. + ### Scale & performance **You don't need to empty the KB for performance.** The logic-check cost depends diff --git a/docs/guide/determinism.md b/docs/guide/determinism.md index 5b50cba4..b9ad75a5 100644 --- a/docs/guide/determinism.md +++ b/docs/guide/determinism.md @@ -20,6 +20,25 @@ factlog는 두 가지 서로 다른 메커니즘으로 신선도(freshness)를 이 두 단계는 상호 보완적입니다. 훅은 결정론적 빈틈을 메우고, SKILL 규율은 엔지니어링적 강제가 불가능한 서술(narration) 계층을 담당합니다. +> **기존 KB 를 쓰고 있었다면 한 번은 `/factlog check` 가 필요합니다.** 이전 +> 버전에서는 훅이 Claude Code 의 실제 페이로드를 읽지 못해 이 거부가 한 번도 +> 발동하지 않았습니다. 이번 수정 이후로는, `facts/accepted.dl` 이나 +> `facts/query.dl` 이 이미 있는데 `facts/logic_report.txt` 가 없거나 낡은 KB +> 에서는 `/factlog check` 로 리포트를 먼저 갱신해야 엔진 입력을 편집할 수 +> 있습니다. + +훅은 신선도 외에 **한 가지 사유로 더** 거부합니다. 훅은 Claude Code 가 보내는 +도구 페이로드에서 대상 경로를 읽어 판정하는데, `Write`/`Edit` 호출이면서 페이로드 +구조가 바뀌어 그 경로를 전혀 읽을 수 없으면 — 즉 그 쓰기가 엔진 입력을 겨냥하지 +*않는다*는 것을 보일 수 없으면 — 통과시키지 않고 거부합니다. 이 상황에서만 +`FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1` 로 해당 검사를 건너뛸 수 있습니다. 이름 +그대로 **읽을 수 없는 페이로드** 한 갈래만 면제하며, 이 변수로 **신선도 거부는 +풀리지 않습니다.** 이건 세션 안에서 모델이 스스로 할 수 있는 일이 아닙니다 +— 훅은 Claude Code 프로세스의 환경을 물려받으므로, **사람이** Claude Code 환경에 +설정해야 합니다. `settings.json` 의 `env` 블록에 넣거나 Claude Code 를 띄우기 전에 +export 한 뒤, 새 세션을 시작하세요. 그리고 읽지 못한 페이로드 형태를 업스트림에 +알려 주세요. + ### 규모와 성능 **성능 때문에 KB를 비울 필요는 없습니다.** 로직 체크 비용은 총 사실 수보다 diff --git a/hooks/gate_check.sh b/hooks/gate_check.sh index 9e7faebb..daa97f02 100755 --- a/hooks/gate_check.sh +++ b/hooks/gate_check.sh @@ -23,6 +23,10 @@ # DENY (exit 2) otherwise, i.e. TARGET is an engine input AND NOT bootstrap # AND (report absent OR report stale). # +# TARGET itself is read from the hook payload; when it cannot be read at all +# the predicate above is undefined, and the narrow fail-closed rule described +# under "fail-closed branches" below decides instead. +# # This predicate is falsifiable in both directions: # - Bootstrap is allowed: creating facts/query.dl in a freshly `factlog init` # KB (no logic_report.txt, no pre-existing query.dl) returns exit 0. @@ -48,10 +52,53 @@ # That degrade is made OBSERVABLE: when Python is available but the resolver # returns empty (package import failure), a one-line stderr note is emitted so # the silent permissive fallback is visible to an operator (see below). -# The only true fail-closed here is the python-availability check below (which -# DENYs when no usable Python 3.11+ is present, since the predicate cannot then -# be evaluated). Target-path extraction failures for engine-input-shaped payloads -# likewise deny. +# THREE branches DENY without evaluating the freshness predicate: +# 1. The python-availability check below DENYs when no usable Python 3.11+ is +# present, since the predicate cannot then be evaluated. Escape hatch: +# FACTLOG_PYTHON (point it at a usable interpreter). +# 2. Target-path extraction DENYs only in the narrow case where the payload +# carries a `tool_input` JSON OBJECT, `tool_name` is one of the write-class +# tools this hook is registered for, and NO usable path can be read from +# either `tool_input` or the top level. That combination means the payload +# schema drifted out from under the extractor while a write was in flight, +# so the predicate cannot be evaluated for a write that may well target an +# engine input. Escape hatch: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1, +# which only a human can set (see the deny message below). The name states +# the one branch it releases: it must NOT grow into a switch that also +# releases branch 1 or 3, or the freshness predicate itself. +# 3. _mtime() DENYs when `stat` cannot report a file's mtime. That branch has +# NO escape hatch; it is only reachable for a file this script has already +# seen pass `-f`, i.e. a race or an unreadable filesystem. +# Only branches 1 and 2 carry an escape hatch, and only branches 1 and 2 are +# about the payload/interpreter layer. +# +# Everything else fails OPEN (exit 0). Three of those branches are a check the +# gate SKIPPED because it could not read the call, and each emits a one-line +# stderr note — #323 was filed partly because such skips were silent and left +# the operator believing the gate was running (same rule as the resolver +# degrade, #244): +# - an unparseable payload; +# - an INCOMPLETE record from the extractor — the interpreter died, or wrote +# something that is not three NUL-terminated fields. Note this lands the +# OPPOSITE way from branch 2's reasoning, deliberately: with no record at +# all we cannot tell that the call is even a write, so denying would block +# every tool call in the session on evidence we do not have. Branch 2 denies +# because we positively know it IS a write we cannot evaluate. +# - a write-class `tool_name` whose `tool_input` is absent or is not a JSON +# object. The JSON parses and the record is complete, so the two notes above +# do not cover it, yet the gate is just as blind. Under the current schema +# Write/Edit always send a `tool_input` object, so this fires zero times in +# normal operation. +# Two branches stay silent, and neither is a skipped check: +# - a `tool_name` outside the write-class list (a Read is not this gate's +# business); +# - a payload with no `tool_name` at all. +# One more permissive degrade exists further down: _canon() falls back to the +# raw path string if the canonicaliser cannot run, which can only make a match +# LESS likely (i.e. more permissive). It is silent, and predates #323. +# +# A gate protecting two files in one KB must not become a global Write/Edit +# outage, so anything short of positive knowledge falls open. set -euo pipefail @@ -98,17 +145,173 @@ else echo "[factlog GATE] note: factlog config resolver unavailable; freshness gate falling back to \${FACTLOG_ROOT:-cwd} (KB_ROOT=$KB_ROOT)" >&2 fi -# Extract the tool target from the hook payload. -# Claude Code sends the tool input as JSON on stdin. -# The relevant field is "file_path" for Write and "file_path" for Edit. -target_path="$(printf '%s' "$payload" | "${PYTHON_RUNNER[@]}" -c \ - "import json,sys; d=json.load(sys.stdin); print(d.get('file_path','') or d.get('path',''))" \ - 2>/dev/null || true)" +# Extract the tool target from the hook payload (issue #323). +# +# Claude Code sends an ENVELOPE on stdin, not the bare tool input: +# {"session_id":..,"cwd":..,"hook_event_name":"PreToolUse","tool_name":"Write", +# "tool_input":{"file_path":..,"content":..},"tool_use_id":..} +# so the target path lives under `tool_input`, which the previous extractor +# never looked at — every real payload fell through to the fail-open branch. +# +# Key precedence: `tool_input` first, then the TOP LEVEL as a fallback. No real +# Claude Code payload puts `file_path` at the top level; that fallback exists to +# keep the flat fixture shape used by tests/test_gate_check.sh working. +# `notebook_path` is defensive only: hooks.json registers the matcher "Write|Edit", +# which Claude Code compares by exact tool name, so NotebookEdit (and MultiEdit) +# never reach this hook. It costs nothing and covers a user who widens the +# matcher in their own settings.json. +# +# The extractor pulls each field under its OWN try/except and always writes +# exactly three NUL-terminated fields, so a failure in one field cannot truncate +# the others. NUL is the separator because a path may legally contain a newline, +# and a newline separator misreads such a path (or, with a leading newline, +# denies a perfectly legal write). A JSON string CAN itself contain a NUL, which +# shifts the remaining fields; that is a wash here — a truncated engine-input +# path still matches the engine input (deny), and a NUL-prefixed one is a path +# the OS cannot write to anyway. +# +# Three fields, not two: `tool_name` is carried raw so the write-class decision +# stays a readable `case` in shell right next to hooks.json's matcher, and so +# the deny message can name the offending tool. Collapsing the tool name and the +# tool_input shape into one field is possible, but it moves that decision into +# the embedded Python where it is harder to audit. +# +# Fields are read straight off a pipe: bash command substitution silently drops +# NUL bytes, so `$(...)` cannot capture this. A shell that cannot parse process +# substitution would raise a SYNTAX error, and bash exits 2 on one — which +# PreToolUse reads as DENY, not as a fail-open. That is unreachable in practice +# (bash falls back to FIFOs where /dev/fd is missing, and macOS, Linux and Git +# Bash all support it), so the code is left as is; it is noted only so the +# failure direction is not misdescribed. +GATE_EXTRACT_PY=" +import json, sys +PATH_KEYS = (\"file_path\", \"path\", \"notebook_path\") +UNPARSED = object() +try: + payload = json.load(sys.stdin) +except Exception: + payload = UNPARSED +try: + name = payload.get(\"tool_name\") + tool_name = name if isinstance(name, str) else \"\" +except Exception: + tool_name = \"\" +try: + if payload is UNPARSED: + input_kind = \"unparsed\" + elif not isinstance(payload, dict) or \"tool_input\" not in payload: + input_kind = \"absent\" + elif isinstance(payload[\"tool_input\"], dict): + input_kind = \"object\" + else: + input_kind = \"other\" +except Exception: + input_kind = \"absent\" +try: + target = \"\" + nested = payload.get(\"tool_input\") if isinstance(payload, dict) else None + for source in (nested, payload): + if not isinstance(source, dict): + continue + for key in PATH_KEYS: + value = source.get(key) + if isinstance(value, str) and value: + target = value + break + if target: + break +except Exception: + target = \"\" +sys.stdout.write(tool_name + \"\\0\" + target + \"\\0\" + input_kind + \"\\0\") +" + +tool_name="" +target_path="" +tool_input_kind="absent" +if ! { IFS= read -r -d '' tool_name \ + && IFS= read -r -d '' target_path \ + && IFS= read -r -d '' tool_input_kind; } \ + < <(printf '%s' "$payload" | "${PYTHON_RUNNER[@]}" -c "$GATE_EXTRACT_PY" 2>/dev/null); then + # The extractor produced no complete record (the interpreter died, or wrote + # something that is not three NUL-terminated fields). Treat it as an + # unparseable payload: fail OPEN, the pre-#323 behaviour. See the header for + # why this lands opposite to the narrow fail-closed branch below. + tool_name="" + target_path="" + tool_input_kind="incomplete" +fi + +# Write-class tool names, matched EXACTLY. A user may register this hook with a +# broader matcher in their own settings.json, so the deny branch below must key +# off the tool name rather than assume only Write/Edit arrive. Anything outside +# this list — including an absent tool_name, which is what the flat test +# fixtures send — is not a write we can reason about, and falls open. +_is_write_tool() { + case "$1" in + Write|Edit) return 0 ;; + *) return 1 ;; + esac +} -# If we could not extract a path, allow the tool to proceed (fail open). -# An empty/unparseable payload cannot target an engine input, so allowing here -# does not weaken the engine-input guard below. if [ -z "$target_path" ]; then + if [ "$tool_input_kind" = "object" ] && _is_write_tool "$tool_name"; then + # Narrow fail-closed: a write-class call carrying a structured tool_input + # from which no path key could be read. Usually that means the payload + # schema drifted; a present-but-empty file_path lands here too, and is + # denied regardless of report freshness because this branch runs before the + # engine-input match. Either way we cannot tell whether it targets an + # engine input. + # + # The escape hatch is read from THIS process's environment, which a hook + # inherits from the Claude Code process. A model cannot set it for its own + # tool calls from inside the session, so the deny message is addressed to a + # human operator and says where to set it. + if [ "${FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD:-}" = "1" ]; then + echo "[factlog GATE] note: could not read a target path from the $tool_name payload;" >&2 + echo " FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 is set, so the write is allowed unchecked." >&2 + exit 0 + fi + echo "[factlog GATE] DENIED: could not read a target path from the $tool_name tool payload." >&2 + echo " Either the tool call carried an empty path — in which case the write itself is" >&2 + echo " malformed and nothing needs configuring — or the hook payload schema changed, so" >&2 + echo " the freshness predicate cannot be evaluated and this write cannot be shown to" >&2 + echo " miss facts/accepted.dl or facts/query.dl." >&2 + echo " If it is the schema: this cannot be worked around from inside the session. Ask" >&2 + echo " the operator to set FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 in the Claude Code" >&2 + echo " environment (the \"env\" block of settings.json, or export it before launching" >&2 + echo " Claude Code) and start a new session. That bypasses only this check — the" >&2 + echo " freshness deny and the Python-availability deny still apply." >&2 + echo " Please also report the payload shape upstream." >&2 + exit 2 + fi + # Fail OPEN for everything else. Three of those branches mean the gate SKIPPED + # a check because it could not read the call, which is exactly the "operator + # believes the gate is running" half of #323, so each emits one stderr line + # the way the resolver degrade does (#244): + # - the payload was not parseable JSON; + # - the extractor returned no complete record; + # - the call IS a write-class tool, but its `tool_input` is missing or is not + # a JSON object (a renamed key, a changed nesting). The JSON parses and the + # record is complete here, so neither note above covers it — and under the + # current schema Write/Edit always send a `tool_input` object, so this note + # fires zero times in normal operation. + # Only two branches stay silent, and neither is a skipped check: a tool outside + # the write-class list (a Read is not this gate's business) and a payload with + # no `tool_name` at all. Those are ordinary traffic; a note on each would be + # noise on every tool call. + case "$tool_input_kind" in + unparsed) + echo "[factlog GATE] note: hook payload was not parseable JSON; the freshness gate was skipped for this call (fail-open)." >&2 + ;; + incomplete) + echo "[factlog GATE] note: the payload extractor returned no complete record; the freshness gate was skipped for this call (fail-open)." >&2 + ;; + absent|other) + if _is_write_tool "$tool_name"; then + echo "[factlog GATE] note: the $tool_name payload carried no tool_input object, so no target path could be read; the freshness gate was skipped for this call (fail-open)." >&2 + fi + ;; + esac exit 0 fi @@ -163,6 +366,11 @@ if [ ! -f "$report" ]; then exit 2 fi +# Fail-closed branch 3 (see header): if stat cannot report an mtime we cannot +# compare freshness, so we deny. The `exit 2` inside this command substitution +# ends the whole script under `set -e`. There is no escape hatch here — the file +# has already passed `-f` a line earlier, so reaching this needs a race or an +# unreadable filesystem, not a schema change someone has to work around. _mtime() { local value if value="$(stat -c %Y "$1" 2>/dev/null)" || value="$(stat -f %m "$1" 2>/dev/null)"; then diff --git a/tests/test_gate_check.sh b/tests/test_gate_check.sh index 454bd78c..c6a37613 100644 --- a/tests/test_gate_check.sh +++ b/tests/test_gate_check.sh @@ -42,6 +42,29 @@ run_case() { fi } +# --------------------------------------------------------------------------- +# Helper: run gate for a given KB root with a VERBATIM payload and expected exit. +# Used by the envelope cases (#323), where the payload shape is the thing under +# test and cannot be derived from a path alone. +# --------------------------------------------------------------------------- +run_payload_case() { + local desc="$1" + local kb_root="$2" + local payload="$3" + local expected_exit="$4" + + local actual_exit=0 + FACTLOG_ROOT="$kb_root" bash "$GATE" <<< "$payload" >/dev/null 2>&1 || actual_exit=$? + + if [ "$actual_exit" -eq "$expected_exit" ]; then + echo "PASS: $desc (exit $actual_exit)" + pass=$((pass + 1)) + else + echo "FAIL: $desc — expected exit $expected_exit, got $actual_exit" + fail=$((fail + 1)) + fi +} + # --------------------------------------------------------------------------- # Setup helpers # --------------------------------------------------------------------------- @@ -468,6 +491,398 @@ else fi rm -rf "$KB_OK" "$ok_err" +# =========================================================================== +# CASES 18-38: REAL HOOK ENVELOPE (#323) +# +# Claude Code does not send the bare tool input; it sends an envelope with the +# tool input nested under `tool_input`. CASES 1-17 above all use the FLAT +# fixture shape, which no production payload has, so before the #323 fix the +# gate returned exit 0 for every envelope payload — the freshness guard never +# fired in production and the harness never noticed. +# +# Each case below is labelled with its PRE-FIX result. "vacuous pre-fix" means +# the old gate passed the case only because it allowed everything; those cases +# pin behaviour but are NOT evidence of the defect. The cases that genuinely +# failed before the fix are marked "PRE-FIX FAIL". +# =========================================================================== + +kb_no_report() { + # Existing engine input, no report → the stale-guard must DENY. + local root="$1" + make_kb "$root" + touch_file "$root/facts/accepted.dl" +} + +kb_stale() { + # Report older than accepted.dl → DENY. + local root="$1" + make_kb "$root" + touch_file "$root/facts/logic_report.txt" + set_mtime_past "$root/facts/logic_report.txt" + touch_file "$root/facts/accepted.dl" +} + +kb_fresh() { + # Report newer than accepted.dl → ALLOW. + local root="$1" + make_kb "$root" + touch_file "$root/facts/accepted.dl" + set_mtime_past "$root/facts/accepted.dl" + touch_file "$root/facts/logic_report.txt" +} + +envelope() { + # The payload Claude Code actually sends for a Write/Edit PreToolUse hook. + local tool_name="$1" + local target="$2" + printf '{"session_id":"s","cwd":"/tmp","hook_event_name":"PreToolUse","tool_name":"%s","tool_input":{"file_path":"%s","content":"x"},"tool_use_id":"u"}' \ + "$tool_name" "$target" +} + +# --------------------------------------------------------------------------- +# CASE 18: envelope, existing engine input, report absent — DENY. +# PRE-FIX FAIL (old gate returned 0). This is the issue's core reproduction. +# --------------------------------------------------------------------------- +KB_ENV1="$(mktemp -d)" +kb_no_report "$KB_ENV1" +run_payload_case "envelope: engine input, report absent — deny" \ + "$KB_ENV1" "$(envelope Write "$KB_ENV1/facts/accepted.dl")" 2 +rm -rf "$KB_ENV1" + +# --------------------------------------------------------------------------- +# CASE 19: envelope, report stale — DENY. +# PRE-FIX FAIL (old gate returned 0). +# --------------------------------------------------------------------------- +KB_ENV2="$(mktemp -d)" +kb_stale "$KB_ENV2" +run_payload_case "envelope: engine input, report stale — deny" \ + "$KB_ENV2" "$(envelope Edit "$KB_ENV2/facts/accepted.dl")" 2 +rm -rf "$KB_ENV2" + +# --------------------------------------------------------------------------- +# CASE 20: envelope, report fresh — ALLOW (acceptance criterion; vacuous pre-fix). +# --------------------------------------------------------------------------- +KB_ENV3="$(mktemp -d)" +kb_fresh "$KB_ENV3" +run_payload_case "envelope: engine input, report fresh — allow" \ + "$KB_ENV3" "$(envelope Write "$KB_ENV3/facts/accepted.dl")" 0 +rm -rf "$KB_ENV3" + +# --------------------------------------------------------------------------- +# CASE 21: envelope bootstrap — fresh KB creating query.dl — ALLOW. +# Acceptance criterion; vacuous pre-fix. Guards against a fix that denies the +# first write in a fresh KB and deadlocks the question→query-draft flow. +# --------------------------------------------------------------------------- +KB_ENV4="$(mktemp -d)" +make_kb "$KB_ENV4" +run_payload_case "envelope: bootstrap fresh KB creating query.dl — allow" \ + "$KB_ENV4" "$(envelope Write "$KB_ENV4/facts/query.dl")" 0 +rm -rf "$KB_ENV4" + +# --------------------------------------------------------------------------- +# CASE 22: envelope, target is not an engine input — ALLOW (vacuous pre-fix). +# --------------------------------------------------------------------------- +KB_ENV5="$(mktemp -d)" +kb_stale "$KB_ENV5" +run_payload_case "envelope: non-engine-input target in a stale KB — allow" \ + "$KB_ENV5" "$(envelope Write "$KB_ENV5/notes.md")" 0 +rm -rf "$KB_ENV5" + +# --------------------------------------------------------------------------- +# CASE 23: NARROW FAIL-CLOSED — write-class tool, `tool_input` IS an object, but +# it carries no known path key — DENY. +# PRE-FIX FAIL (old gate returned 0). +# +# This is the schema-drift branch: the payload is a write we cannot evaluate, so +# we cannot show it misses the engine inputs. +# --------------------------------------------------------------------------- +KB_ENV6="$(mktemp -d)" +kb_stale "$KB_ENV6" +run_payload_case "envelope: Write with no path key in tool_input — fail-closed deny" \ + "$KB_ENV6" '{"tool_name":"Write","tool_input":{"content":"x"}}' 2 + +# --------------------------------------------------------------------------- +# CASE 24: the fail-closed branch requires a write-class `tool_name`. With NO +# tool_name (the shape CASES 1-17 send) the gate must ALLOW — otherwise the +# entire flat harness flips to DENY. Vacuous pre-fix; kills a mutant that denies +# on every empty path. +# --------------------------------------------------------------------------- +run_payload_case "envelope: no tool_name, no path key — allow (not a known write)" \ + "$KB_ENV6" '{"tool_input":{"content":"x"}}' 0 + +# --------------------------------------------------------------------------- +# CASE 25: tool_name outside the write-class list — ALLOW (vacuous pre-fix). +# Pins the exact-match small list: a user who widens the matcher in their own +# settings.json must not have unrelated tools denied. +# --------------------------------------------------------------------------- +run_payload_case "envelope: non-write tool_name, no path key — allow" \ + "$KB_ENV6" '{"tool_name":"Read","tool_input":{"content":"x"}}' 0 +rm -rf "$KB_ENV6" + +# --------------------------------------------------------------------------- +# CASE 26: KEY PRECEDENCE — `tool_input.file_path` wins over a top-level +# `file_path`. Here the nested path IS the engine input and the top-level one is +# not, so the gate must DENY. PRE-FIX FAIL (old gate read the top level → 0). +# --------------------------------------------------------------------------- +KB_ENV7="$(mktemp -d)" +kb_stale "$KB_ENV7" +run_payload_case "envelope: tool_input.file_path wins over top-level — deny" \ + "$KB_ENV7" \ + "$(printf '{"tool_name":"Write","file_path":"%s","tool_input":{"file_path":"%s"}}' \ + "$KB_ENV7/notes.md" "$KB_ENV7/facts/accepted.dl")" 2 + +# --------------------------------------------------------------------------- +# CASE 27: the same precedence in the opposite direction — the nested path is +# NOT an engine input while the top-level one is, so the gate must ALLOW. +# PRE-FIX FAIL (old gate read the top level → 2). Together with CASE 26 this +# pins the precedence in both directions; either case alone is passed by an +# implementation that simply merges the two dicts. +# --------------------------------------------------------------------------- +run_payload_case "envelope: top-level file_path ignored when tool_input has one — allow" \ + "$KB_ENV7" \ + "$(printf '{"tool_name":"Write","file_path":"%s","tool_input":{"file_path":"%s"}}' \ + "$KB_ENV7/facts/accepted.dl" "$KB_ENV7/notes.md")" 0 + +# --------------------------------------------------------------------------- +# CASE 28: top-level FALLBACK is still consulted when `tool_input` carries no +# path key. No production payload has a top-level file_path; this keeps the flat +# fixture shape of CASES 1-17 working. Vacuous pre-fix. +# --------------------------------------------------------------------------- +run_payload_case "envelope: falls back to top-level file_path — deny" \ + "$KB_ENV7" \ + "$(printf '{"file_path":"%s","tool_input":{"content":"x"}}' "$KB_ENV7/facts/accepted.dl")" 2 + +# --------------------------------------------------------------------------- +# CASES 29-31: `tool_input` present but NOT an object (null / string / array). +# That is not the narrow fail-closed condition, so the gate must ALLOW rather +# than deny — and, more importantly, the extractor must not die partway through +# and leave the shell reading a truncated record. Vacuous pre-fix; these are the +# cases that kill an extractor which calls .get() on a non-dict. +# +# CASE 31 deliberately hides an engine-input path inside an ARRAY: a path is +# only honoured from an object, so this must still ALLOW. +# --------------------------------------------------------------------------- +run_payload_case "envelope: tool_input is null — allow (not the fail-closed shape)" \ + "$KB_ENV7" '{"tool_name":"Write","tool_input":null}' 0 +run_payload_case "envelope: tool_input is a string — allow (not the fail-closed shape)" \ + "$KB_ENV7" '{"tool_name":"Write","tool_input":"oops"}' 0 +run_payload_case "envelope: tool_input is an array — allow (not the fail-closed shape)" \ + "$KB_ENV7" \ + "$(printf '{"tool_name":"Write","tool_input":[{"file_path":"%s"}]}' "$KB_ENV7/facts/accepted.dl")" 0 + +# --------------------------------------------------------------------------- +# CASE 32: MUTATION PIN (vacuous pre-fix) — the engine-input path appears only +# inside `content`, while the actual target is an unrelated file → ALLOW. +# +# A grep-the-whole-payload "fix" (the shape hooks/gate_reminder.sh:17 uses for +# its non-blocking nudge) would DENY here and make every write that merely +# mentions facts/accepted.dl unblockable without a logic check. +# --------------------------------------------------------------------------- +run_payload_case "envelope: engine-input path only inside content — allow (no payload grep)" \ + "$KB_ENV7" \ + "$(printf '{"tool_name":"Write","tool_input":{"file_path":"%s","content":"see %s"}}' \ + "$KB_ENV7/notes.md" "$KB_ENV7/facts/accepted.dl")" 0 + +# --------------------------------------------------------------------------- +# CASE 33: NUL SEPARATOR — MUTATION PIN. +# +# The extractor separates its three fields with NUL because a path may legally +# contain a newline. Switching both the write and the read back to "\n" passes +# every other case in this file, so without this one that decision is unpinned. +# +# The target is the engine input path with a newline and one more segment glued +# on. That is NOT the engine input, and the KB is stale, so the gate must ALLOW. +# Under a newline separator the record splits at the newline, field 2 becomes +# exactly "/facts/accepted.dl", the stale-report guard fires, and the case +# gets exit 2 instead. +# --------------------------------------------------------------------------- +run_payload_case "envelope: newline inside file_path — allow (NUL separator pin)" \ + "$KB_ENV7" \ + "$(printf '{"tool_name":"Write","tool_input":{"file_path":"%s\\nsuffix"}}' \ + "$KB_ENV7/facts/accepted.dl")" 0 + +# --------------------------------------------------------------------------- +# CASE 34: EMPTY file_path — DENY, and pin it. +# +# `tool_input` is an object and `tool_name` is write-class, but the path is the +# empty string, so no target can be read and the narrow fail-closed branch +# fires. Note this deny happens BEFORE the engine-input match, so it holds even +# in a KB whose report is fresh — hence the second call below against a fresh +# KB. The deny text must therefore not diagnose "schema changed" as the only +# possible cause. +# --------------------------------------------------------------------------- +run_payload_case "envelope: empty file_path in a stale KB — deny" \ + "$KB_ENV7" '{"tool_name":"Write","tool_input":{"file_path":"","content":"x"}}' 2 + +KB_EMPTY_FRESH="$(mktemp -d)" +kb_fresh "$KB_EMPTY_FRESH" +run_payload_case "envelope: empty file_path in a FRESH KB — deny (runs before the match)" \ + "$KB_EMPTY_FRESH" '{"tool_name":"Write","tool_input":{"file_path":""}}' 2 +rm -rf "$KB_EMPTY_FRESH" + +# --------------------------------------------------------------------------- +# CASE 35: WRITE-CLASS CALL WITH NO tool_input OBJECT — ALLOW, but say so. +# +# The JSON parses and the extractor record is complete, so neither the +# unparseable nor the incomplete note covers these; the gate is nevertheless +# blind to the target. That is a skipped check, not ordinary traffic, so it must +# be observable. Under the current schema Write/Edit always send a `tool_input` +# object, so in normal operation this note never fires. +# +# Three shapes: tool_input missing entirely, a renamed key (the most plausible +# schema drift), and a tool_input that is not an object. +# --------------------------------------------------------------------------- +blind_note_case() { + local desc="$1" + local payload="$2" + + local err + err="$(mktemp)" + local actual_exit=0 + FACTLOG_ROOT="$KB_ENV7" bash "$GATE" <<< "$payload" >/dev/null 2>"$err" || actual_exit=$? + + if [ "$actual_exit" -eq 0 ] && grep -qF "carried no tool_input object" "$err"; then + echo "PASS: $desc — allow with a fail-open note (exit $actual_exit)" + pass=$((pass + 1)) + else + echo "FAIL: $desc — expected allow (exit 0) with a fail-open note; exit=$actual_exit stderr=$(cat "$err")" + fail=$((fail + 1)) + fi + rm -f "$err" +} + +blind_note_case "write-class call with no tool_input at all" \ + '{"tool_name":"Write"}' +blind_note_case "write-class call with a renamed tool_input key" \ + "$(printf '{"tool_name":"Write","toolInput":{"file_path":"%s"}}' "$KB_ENV7/facts/accepted.dl")" +blind_note_case "write-class call with a non-object tool_input" \ + '{"tool_name":"Edit","tool_input":"oops"}' + +# A non-write tool in the same shape must stay SILENT — the note must not become +# noise on every Read/Grep/Bash call in a session. +quiet_err="$(mktemp)" +quiet_exit=0 +FACTLOG_ROOT="$KB_ENV7" bash "$GATE" <<< '{"tool_name":"Read","tool_input":"oops"}' \ + >/dev/null 2>"$quiet_err" || quiet_exit=$? +if [ "$quiet_exit" -eq 0 ] && [ ! -s "$quiet_err" ]; then + echo "PASS: non-write tool with no tool_input object — allow, and silent (exit $quiet_exit)" + pass=$((pass + 1)) +else + echo "FAIL: non-write tool — expected a silent allow; exit=$quiet_exit stderr=$(cat "$quiet_err")" + fail=$((fail + 1)) +fi +rm -f "$quiet_err" + +# --------------------------------------------------------------------------- +# CASE 36: UNPARSEABLE PAYLOAD — ALLOW, but say so. +# +# The header enumerates five fail-open branches; before this case none of the +# harness exercised the two that mean "the gate skipped a check it could not +# read". #323 survived precisely because a documented contract had no case +# riding it, so both are pinned here: exit 0 AND the one-line stderr note that +# keeps the skip visible to an operator (#244's rule). +# +# Garbage payload → json.load raises → fail open with a note. +# Empty payload → same branch (json.load("") raises too). +# --------------------------------------------------------------------------- +run_payload_case "unparseable payload — allow (fail-open)" \ + "$KB_ENV7" 'not json at all' 0 +run_payload_case "empty payload — allow (fail-open)" \ + "$KB_ENV7" '' 0 + +unparsed_err="$(mktemp)" +FACTLOG_ROOT="$KB_ENV7" bash "$GATE" <<< 'not json at all' >/dev/null 2>"$unparsed_err" || true +if grep -qF "hook payload was not parseable JSON" "$unparsed_err"; then + echo "PASS: unparseable payload emits a one-line fail-open note" + pass=$((pass + 1)) +else + echo "FAIL: unparseable payload — expected a fail-open note, got: $(cat "$unparsed_err")" + fail=$((fail + 1)) +fi +rm -f "$unparsed_err" + +# --------------------------------------------------------------------------- +# CASE 37: INCOMPLETE EXTRACTOR RECORD — ALLOW, but say so. +# +# The extractor writes exactly three NUL-terminated fields; if it dies or writes +# anything else, the shell has no record to reason about. That must fail OPEN +# (we cannot even tell the call is a write) and must NOT be silent. +# +# Simulated hermetically with a FACTLOG_PYTHON_RUNNER shim that answers the +# `import sys` availability probe and the resolver, then emits a record with no +# NUL terminators. The KB is stale and the payload targets an engine input, so a +# working extractor would DENY — reaching exit 0 proves the incomplete-record +# branch was taken. +# --------------------------------------------------------------------------- +TRUNC_SHIM="$(mktemp -d)" +cat > "$TRUNC_SHIM/runner.sh" <<'SH' +#!/usr/bin/env bash +# Behave like tools/factlog_python.sh for the probes the gate makes first, then +# hand back a record that is not three NUL-terminated fields. +for arg in "$@"; do + case "$arg" in + "import sys") exit 0 ;; + *resolve_root*) printf '%s' ""; exit 0 ;; + esac +done +printf 'Write no-nul-here' +exit 0 +SH +chmod +x "$TRUNC_SHIM/runner.sh" + +trunc_err="$(mktemp)" +trunc_exit=0 +FACTLOG_PYTHON_RUNNER="$TRUNC_SHIM/runner.sh" FACTLOG_ROOT="$KB_ENV7" \ + bash "$GATE" <<< "$(envelope Write "$KB_ENV7/facts/accepted.dl")" \ + >/dev/null 2>"$trunc_err" || trunc_exit=$? +if [ "$trunc_exit" -eq 0 ]; then + echo "PASS: incomplete extractor record — allow (fail-open) (exit $trunc_exit)" + pass=$((pass + 1)) +else + echo "FAIL: incomplete extractor record — expected fail-open (exit 0), got $trunc_exit" + fail=$((fail + 1)) +fi +if grep -qF "returned no complete record" "$trunc_err"; then + echo "PASS: incomplete extractor record emits a one-line fail-open note" + pass=$((pass + 1)) +else + echo "FAIL: incomplete extractor record — expected a fail-open note, got: $(cat "$trunc_err")" + fail=$((fail + 1)) +fi +rm -rf "$TRUNC_SHIM" "$trunc_err" + +# --------------------------------------------------------------------------- +# CASE 38: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 escape hatch. +# +# (a) It releases the narrow schema-drift deny of CASE 23 — without an escape +# hatch, a payload-schema change turns this gate into a global Write/Edit +# outage, and the deny stderr is fed back to the model as a retry loop. +# (b) It must NOT release the freshness deny, which is the gate's whole purpose. +# Vacuous pre-fix for (a); PRE-FIX FAIL for (b) (old gate returned 0). +# --------------------------------------------------------------------------- +hatch_exit=0 +FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 FACTLOG_ROOT="$KB_ENV7" bash "$GATE" \ + <<< '{"tool_name":"Write","tool_input":{"content":"x"}}' >/dev/null 2>&1 || hatch_exit=$? +if [ "$hatch_exit" -eq 0 ]; then + echo "PASS: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 releases the schema-drift deny (exit $hatch_exit)" + pass=$((pass + 1)) +else + echo "FAIL: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 — expected allow (exit 0), got $hatch_exit" + fail=$((fail + 1)) +fi + +hatch_fresh_exit=0 +FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 FACTLOG_ROOT="$KB_ENV7" bash "$GATE" \ + <<< "$(envelope Write "$KB_ENV7/facts/accepted.dl")" >/dev/null 2>&1 || hatch_fresh_exit=$? +if [ "$hatch_fresh_exit" -eq 2 ]; then + echo "PASS: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 does NOT release the freshness deny (exit $hatch_fresh_exit)" + pass=$((pass + 1)) +else + echo "FAIL: FACTLOG_GATE_ALLOW_UNREADABLE_PAYLOAD=1 — freshness deny must still fire (exit 2), got $hatch_fresh_exit" + fail=$((fail + 1)) +fi +rm -rf "$KB_ENV7" + # --------------------------------------------------------------------------- # Summary # ---------------------------------------------------------------------------