Merge remote-tracking branch 'origin/main' into fix/issue-66

Author: Shashankss1205

CHANGELOG.md

@@ -16,4 +16,12 @@ Entries are newest-last within a release, matching the order they were written.
 - a bracket anywhere in a model's **prose** hijacked JSON extraction, because only the first `{`/`[` was ever tried. `Based on the context [lines 3-5]: {…}` was rejected as unparseable, and — worse — `Analysis (note [1]): {"supported": false}` returned a perfectly valid `[1]`, substituting a fabricated value for the verifier's actual answer. Every opener is tried now, and length alone turned out not to be a safe rank — a citation list like `[101, 205, 309, …]` *longer* than the verdict still won — so object spans are tried before array spans, each longest-first; junk still returns `None`, so fail-closed is unchanged.
 - a **bare backend name was read as a model name**, because `split_spec` only consulted the backend list when the spec contained a slash. `--model claude-cli` — the backend `models --check` reports as `usable` — shelled out to `claude -p --model claude-cli` and was refused by the CLI on *every* call, and `--model mock` named the paid subscription backend and spawned the real binary, so the double documented as "never reaches a provider" reached for one. A bare backend name now resolves to that backend (`claude-cli` to its own default model, `mock` to the scripted double, which ignores the model segment anyway); `openrouter`, `openai` and `ollama` front catalogues rather than a model, so those are refused with an example spelling instead of a guess about what to bill you for. The slash forms and bare *model* names are unchanged.
 - a failing `claude -p` **reported no reason at all**. The CLI exits non-zero with an empty stderr and puts its explanation in the JSON envelope on stdout; the gateway read only stderr, so the error was `claude -p exited 1: ` — a sentence that stops at the colon. Since the wrong-model bug above presented itself exactly that way, the two hid each other. stdout is read first now, and the recovered text also feeds the transient-vs-deterministic classifier, which was previously deciding from `""`.
+- **repeating `--registry` walked a Slack user straight past the agent opt-in.** The gate reads a flag's value to decide admission and argparse's `store` action then runs the *last* occurrence, but `_flag_value` returned the *first* — so `plan … --registry grapharc.examples.plan_docs:build_registry --registry grapharc.stdlib:build_registry` was judged against the demo registry and executed against the one that builds agent kinds on the host, with `GRAPHARC_SLACK_ALLOW_AGENT` never consulted and the forced `--approve` skipped in the same step. No privilege and no special knowledge needed: typing the flag twice was the whole exploit. Repeats of any admitted flag are refused outright now — the fail-closed reading, which retires the entire first-vs-last family rather than the one flag that exposed it — with a carve-out for the options the CLI itself accumulates (`agent --allow`/`--deny`, argparse `action="append"`), where every occurrence reaches the run and nothing can diverge. A duplicated `--model` is refused on the same rule, opted in or not, and `_flag_value` reads the last occurrence regardless, so the two readers can no longer disagree. A sweep over the whole allowlist asserts the duplicated form of every gated flag, so a future gate cannot reopen the gap.
+- a **NUL byte in a path came back as silence**, the worst answer a chat bot can give: `Path(raw).resolve()` raises `ValueError`, `handle_text_live` catches only `SlackCommandError`, so `trace a\x00b` escaped the bolt listener as an unhandled exception and the requester saw no reply at all — indistinguishable from the bot being down. A NUL anywhere in the request is now a refusal in the same voice the core tools already use ("cannot name a file"), and `_confined` turns any `ValueError`/`OSError` out of the filesystem into a refusal too, for callers of its own. Folded in from the same report: the flag allowlist tested `token.startswith("--")`, so a single-dash token slipped it and was spent as a positional — `trace -h` was admitted with `-h` as the path. Any leading dash is a flag now, and one not on the list is refused like any other.
+- the `/live` **token check crashed on the strangers it exists to refuse**. `secrets.compare_digest` rejects `str` outside ASCII, and `_authorized` handed it the raw query parameter, so `?token=café` raised `TypeError` through the handler: an unauthenticated 500 with a traceback in the log on all four `/live` routes, where every ASCII guess correctly got a 401. The 500-vs-401 split was itself an oracle about how the token is compared. Both sides are encoded to UTF-8 now, which drops the ASCII restriction and keeps the constant-time comparison that is the whole reason `compare_digest` is there. A NUL byte in `?trace=` was the same shape one function over — `resolve_trace` raises `ValueError`, not the `LivePathError` the route caught — and is a 404 like any other malformed path now.
+- the `/live` **index advertised traces the reader refuses to serve**. `scan_traces` walked the live root with `rglob("*.jsonl")`, which matches a symlinked file by name, then parsed it and published its name, size, mtime and **run ids** on `GET /live/api/runs` and the HTML index — for a file outside the root that `/live/api/stream` then 404s, the 404 being the proof of intent. One contract, two code paths, and only the reader enforced it; the live root is documented as the Slack bot's working directory, i.e. somewhere other things write. `scan_traces` routes every candidate through `resolve_trace` now and skips symlinks outright, so a refactor of either check cannot reopen the leak. The reader's confinement — `../`, `%2e%2e%2f`, absolute paths, `sub/../../`, symlinked directories — is unchanged.
+- a **`deny` rule naming a tool literally failed open** when the name carried fnmatch metacharacters. `PermissionPolicy.decide` matched with `fnmatch(name, pattern)` alone, so `DENY "exfil[all]"` read as a character class, did not match the tool it spells, and evaluation fell through to whatever came next — typically a broad `ALLOW "*"`. The operator got no error, no warning and no deny; worse, `visible()` decides the same way, so the tool the operator had just forbidden was described to the model as available and then ran when it asked. The failure was inconsistent as well as silent: `DENY "tool?x"` happened to hold, because a `?` glob matches a literal `?`. This was the one place in the tree where a deny failed open — an unmatched tool, an unregistered kind and an unreachable backend all refuse. A `deny` or `ask` rule now also fires on an exact literal match. The widening is bound to those two tiers on purpose: equality can only add a rule that refuses or gates a call, never one that permits it, so it cannot loosen a policy the way the same change on `allow` could. For the `allow` case there is `PermissionRule.literal(action, name)`, which `glob.escape`s the name rather than widening the match, and which `default_harness` now uses for the registry names it allows. Glob semantics are untouched: `rm*` still spans `rmdir`, `*` still matches everything, the tier order and the `deny` default are unchanged.
+- **fan-out handed every worker the same payload object**, and never held it to the schema the worker declared. `_enter` deep-copied only a `BaseModel`, so two `Send`s built from one dict gave both parallel workers the *same live dict* — each reading the other's mutations, through a channel no node declared a write to and no trace event records, in the one place the isolation matters most. `_check_goto_target` validated `Send.node` against exactly this class of silent failure and left `Send.arg` alone, so `input_schema` — documented as typing a worker's payload — enforced nothing: a dict where a model was declared reached the worker and surfaced as a bare `AttributeError` frames away from the dispatcher that produced it, and a wrong model class sharing a field name never surfaced at all. Every payload is deep-copied now whatever its type, and one contradicting a declared `input_schema` is refused at dispatch with `StateTypeError` naming the node, the schema and what arrived. Declaring no `input_schema` stays legal — no claim, nothing to check — but the copy is unconditional.
+- **the front door was the one door the state contract did not hold.** `update_state` refuses an unknown field and `GraphARCState` forbids extras, but `invoke`/`stream`/`ainvoke`/`astream` handed `input` straight to LangGraph, which filters a dict down to known channels *before* the state model is ever constructed — so `extra="forbid"` never saw the typo. `invoke({"quesiton": …})` ran the whole graph on default values and returned a complete, plausible answer to an empty question, with nothing said to the caller: the quietest failure in the runtime, on the door every user goes through first. All four entry points, and `astream_events`, now refuse an unknown input key in the same words `update_state` uses. A wrongly *typed* input value was already loud and still raises Pydantic's `ValidationError`.
+- a node stopped by **Ctrl-C left no ending in the trace**. The sync wrapper caught `Exception` while its async twin catches `BaseException` for the reason its own comment gives — "a stop with no trace line is a stop nobody can audit afterwards" — so a `KeyboardInterrupt` or `SystemExit` inside a sync node escaped with no terminal `error` event, and `metrics.summarize` then reported `errors: 0` for a run an audit reads as having simply stopped between nodes. Ctrl-C is not an exotic ending; it is the commonest way a human stops a long run. The sync wrapper catches `BaseException` now and re-raises it untouched: only the record is new.
 - a policy document's `resource = "node"` rules were **silently discarded**. `edge_policy()` compiled the edge half and nothing compiled the other one, `AdmissionChecker` gated node kinds on registry membership alone, and `check_node` — correct, documented, advertised in the engine's own docstring — had no runtime caller anywhere. So a document denying the kind `deploy` admitted it and ran it, and the only hint that half the file had been dropped was an oblique `1 edge rule(s)` in a line that reads as a summary. The shipped `example.toml` led with exactly that shape: an operator who copied `no-shell-nodes` got a policy that denied nothing. `PolicyEngine.node_policy()` now compiles the node half as `edge_policy()` does the edge half, `AdmissionChecker(node_policy=...)` consults it for every proposed node, and a refusal comes back as `policy/node_denied` quoting the rule's own `reason` — a code the planner replans against, exactly like `edge_denied`. A document that declares *no* node rules still leaves kinds to the registry: saying nothing about nodes is not the same statement as denying all of them, and the banner now counts both halves so a reader can tell which was said.

README.md

@@ -482,7 +482,7 @@ Re-derived on 2026-07-28 by running each item, not by reading the commit log.
 - **Cost is recorded when a backend reports one, estimated when it does not.** Both gateways publish the provider's `cost_usd` through the same `llm_output` envelope, the runtime's usage callback writes it onto the node's `end` event, and an agent's `model` events carry the per-call breakdown. A backend that reports no price still falls back to a `RateCard` estimate, and the two figures stay apart — `recorded_cost_usd` is never a guess. Still missing: no tenant on a trace event, so per-tenant attribution is not offered.
 - **A node's tokens are its own, not the run's movement while it ran.** Worth stating because it was the other way round: an `end` event carried the difference between two readings of the run's *shared* meter, so under fan-out the workers' windows overlapped and each was credited with its siblings' concurrent spend. Three workers costing 8 tokens each traced as 24/16/8, and `metrics` and `cost` agreed on 48 for 24 tokens of real work — doubling the estimated bill purely because the work ran in parallel. Attribution now comes from a per-node scope on the meter, so the same work costs the same serially and in parallel; a hand charge the usage callback never saw still lands on the node that made it.
 - **A planning round is an envelope, not a measurement.** A `round` event used to carry the planner's `tokens` and the round's `duration_ms`, both of which `metrics`, `cost` and `replay` add on top of node totals — and the planner's spend was already reported by its own `plan` event, so it was counted twice, and a round's duration encloses the plan plus every node it ran. Neither is on the event now; both are on its `state_delta` as `round_tokens` / `round_iterations` / `round_duration_ms`, where no reader sums them. `RoundRecord.iterations` also holds a figure now rather than always `0`.
-- **The Claude CLI backend is completion-only.** Tool calling and structured output need one of the OpenAI-wire backends: `openrouter`, `openai`, or a local `ollama`.
+- **The Claude CLI backend is completion-only, and an agent node on it is *delegated* rather than governed.** The CLI has no tool-calling wire format, so GraphARC cannot run its own gated loop over it. Rather than refuse, `AgentNode` hands the whole loop to Claude Code's headless agent — which means every tool Claude Code has, under its `bypassPermissions` mode: those calls are not checked by this graph's permission policy, not confined by the sandbox executor, and the token figure is the sub-agent's own rather than one GraphARC metered call by call. The workspace boundary and the wall-clock ceiling still hold. It warns on `DelegatedToolUseWarning` at construction and marks every trace event `executor=delegated`, so a run stays auditable as delegated; filter that warning to an error to get the old refusal back. Structured output still needs an OpenAI-wire backend: `openrouter`, `openai`, or a local `ollama`.
 - **A session turn is synchronous**, and a runner claim is a claim rather than a lease — nothing reclaims a session whose runner died holding it.
 - **`.env` is found by walking up parent directories; `grapharc.toml` is not.** The config layer refuses an upward search on purpose — a run must not be governed by a file you did not know about. The credential loader predates that decision and still searches upward, so the thing that *spends money* is discovered more eagerly than the thing that *constrains* it.
 - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.

docs/cookbook/03-agents-and-tools.md

@@ -546,6 +546,16 @@ Patterns match the **tool name** only, never its arguments. `DENY "run_command"`
 stops the shell tool entirely; it cannot express "deny `rm` but allow `ls`". That
 distinction belongs in a pre-hook, two recipes down.
 
+**A name that is also a pattern.** `fnmatch` reads `[`…`]` as a character class,
+so a tool called `exfil[all]` — or an MCP-style `mcp__srv__do[all]` — is not the
+same string as the pattern that spells it. A `deny` or `ask` rule therefore also
+fires on an **exact literal match**, so pasting a tool's name into a rule refuses
+it whatever characters it holds. That fallback is deliberately not extended to
+`allow`: equality can only ever add a refusal, never a grant. To *allow* one tool
+whose name carries `*`, `?` or `[`, build the rule with
+`PermissionRule.literal(Decision.ALLOW, name)`, which escapes the name instead of
+widening the match.
+
 ---
 
 ## How do I make sure a denied tool is never even offered to the model?

docs/cookbook/07-slack.md

@@ -27,6 +27,7 @@ an afterthought. The defaults:
 | The budget, policy and trace flags each command already has | `--registry` (imports an arbitrary module), `--config`, `--json`, `--no-color` |
 | `plan --registry`, for exactly the two registries the package ships | any other `--registry` value |
 | `agent`, only behind the double opt-in below | `--model` / `--reviewer-model`, unless the operator opts in |
+| Each admitted flag, once; `agent --allow`/`--deny` accumulate as the CLI does | The same flag twice (`--registry <demo> --registry <stdlib>`), because the gate would judge one occurrence and the CLI would run the other |
 
 With `--model` off, every reachable command runs the scripted, spend-free
 path. The default answer to "can someone in Slack cost me money?" is **no**;

grapharc/cli/config.py

@@ -161,8 +161,12 @@ def load(explicit: Path | None = None, *, cwd: Path | None = None) -> Settings:
             return Settings()
 
     try:
+        # `UnicodeDecodeError` is a `ValueError`, so it belongs in this tuple
+        # explicitly: without it a stray binary `grapharc.toml` in the working
+        # directory tracebacks out of every configurable command, because this
+        # file is picked up implicitly rather than named by the operator.
         document = tomllib.loads(path.read_text(encoding="utf-8"))
-    except (OSError, tomllib.TOMLDecodeError) as exc:
+    except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError) as exc:
         raise ConfigError(f"{path}: {exc}") from exc
 
     table = document.get(TABLE, document)