docs: document hook deny contract, matcher regex, tool_input keys, and provider config - #10467
Conversation
…d provider config
Fills gaps in the hooks and provider docs that made external integration
harder than it needed to be. All statements verified against the current
implementation.
hooks.md:
- Add a "Blocking a Tool Call" section covering the deny contract: only
PreToolUse and Stop can block; the stdout {"decision":"block"} and exit-2
channels; the fail-open rule for every other outcome; the model-facing
denial message; and the GOOSE_STOP_HOOK_BLOCK_CAP cap for Stop hooks.
- Warn that `matcher` is a regex, not a glob: a bare "*" is invalid and the
rule is silently skipped; use ".*" or omit the matcher. Note the match is
unanchored.
- Add a tool_input key reference for the built-in developer tools.
providers.md:
- Document OPENAI_BASE_PATH in the custom-endpoint parameter table and add a
note on reaching a LiteLLM proxy via the OpenAI provider vs. the dedicated
LiteLLM provider.
config-files.md:
- Add a callout that provider API keys are not read from config.yaml (use the
keyring, secrets.yaml, or the provider env var).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9918fdade6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Tool call denied by policy hook `<plugin>`: <reason>. Do not retry; this is a policy denial, not a transient failure. | ||
| ``` | ||
|
|
||
| **Blocking fails open.** Only a clean block signal stops a tool. Every other outcome—a spawn error, a timeout, a non-`{` stdout, or a non-zero exit code other than `2` (such as exit `1`)—is logged and treated as allow. A broken hook can never block goose. |
There was a problem hiding this comment.
Don't promise exit 1 always fails open
If a blocking PreToolUse or Stop hook writes {"decision":"block"} to stdout and then exits 1 (for example, cleanup fails after printing), the current implementation still blocks: emit_blocking passes every Output to deny_reason, and the stdout JSON path does not check status.success(). This paragraph tells hook authors that any non-2 non-zero exit is logged and allowed, so they can accidentally block tool calls despite expecting fail-open behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in b2d7b8c. deny_reason checks the stdout {"decision":"block"} signal independently of the exit code (no status.success() check), so I dropped the "exit 1 fails open" claim and now describe the two deny signals precisely, noting a printed block still blocks even if the hook later exits non-zero.
| :::note Pointing at a LiteLLM proxy | ||
| You can reach a [LiteLLM](https://docs.litellm.ai/) proxy in either of two ways—pick one, don't mix them: | ||
|
|
||
| - Use the **OpenAI provider**: set `OPENAI_HOST` to your proxy's root (no trailing path) and `OPENAI_BASE_PATH` to the path it serves (usually `v1/chat/completions`). A `401` with `No api key passed in` or a `404` on `chat/completions` usually means `OPENAI_BASE_PATH` is wrong for your proxy. |
There was a problem hiding this comment.
Correct the 401 troubleshooting for LiteLLM
In the OpenAI provider, OPENAI_BASE_PATH only controls the request URL; authentication is resolved separately from OPENAI_API_KEY and falls back to NoAuth when the key is not loaded. A 401 saying No api key passed in is therefore the expected symptom for a missing/ignored key (for example, a key placed in config.yaml), not a path problem, so this advice sends users to change the base path instead of supplying the credential.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — fixed in b2d7b8c. Auth is derived solely from OPENAI_API_KEY (falls back to NoAuth), separate from OPENAI_BASE_PATH, so a 401 No api key passed in is a missing-key symptom, not a path one. Reworded: 404 → base path, 401 → key not loaded, with a link to the config.yaml key note.
Address automated review feedback on the PR:
- hooks.md: the stdout `{"decision":"block"}` signal is checked
independently of the exit code (deny_reason does not inspect
status.success()), so a hook that prints a block and then exits non-zero
still blocks. Drop the incorrect "exit 1 fails open" claim and describe the
two deny signals precisely.
- providers.md: a `401 No api key passed in` is a missing-key problem, not a
base-path one (OPENAI_BASE_PATH only builds the URL; auth falls back to
NoAuth when the key is unset). A wrong base path surfaces as a `404`. Link
to the config.yaml API-key note.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
michaelneale
left a comment
There was a problem hiding this comment.
thanks, looks ok to me
Summary
While integrating goose as an enforcement and audit target in failproofai, we studied the hook/plugin system closely and found that several behaviors we depended on weren't documented — enough that we had to reverse-engineer them with a probe plugin before we could rely on them. This PR fills those gaps so the next person integrating against goose's hooks doesn't have to.
Every statement below was verified against the current implementation on
main(not just an installed release), so a few things that differ between releases are described as they behave today.Background — how we found these gaps
We were wiring failproofai's policy enforcement into goose via a
PreToolUsehook and its audit reader into the session database. The docs describe the events, payload shape, and matcher, but stopped short of the details an integrator needs: whether a hook can actually block a tool call and how, why a matcher silently never fired, and whichtool_inputkeys to read. We recovered those answers empirically and are contributing them back. Discovered while integrating goose in our own tool (internal tracking: FailproofAI/failproofai#508).What this changes
documentation/docs/guides/context-engineering/hooks.mdPreToolUseandStopcan block; every other event is observation-only, and a block decision from them is ignored.{"decision":"block","reason":"..."}on stdout (exit 0), or exit code2with the reason on stderr.{stdout, or a non-zero exit other than2) allows the tool — a broken hook never blocks.GOOSE_STOP_HOOK_BLOCK_CAPforStophooks.matcheris a regex, not a glob — a bare"*"is an invalid regex, so the whole rule is silently skipped. Use.*or omit the matcher. (This one cost us a debugging session: the hook simply never fired, with no obvious error.)tool_inputkey reference table for goose's built-indevelopertools — previously only the shellcommandexample existed, so anyone inspecting file paths had no reference for the input keys.documentation/docs/getting-started/providers.mdOPENAI_BASE_PATHin the custom-endpoint parameter table, and add a note on reaching a LiteLLM proxy — either via the OpenAI provider (OPENAI_HOST+OPENAI_BASE_PATH) or the dedicated LiteLLM provider (LITELLM_*).documentation/docs/guides/config-files.mdconfig.yaml— they belong in the keyring,secrets.yaml, or the provider environment variable. A key placed inconfig.yamlis ignored and surfaces asNo api key passed in.Testing
Docs-only change. Each added statement was verified against source:
crates/goose/src/hooks/mod.rs— event list, payload struct,deny_reason(), matcher handlingcrates/goose/src/agents/agent.rs—PreToolUse/Stopblock call sites, model-facing message,GOOSE_STOP_HOOK_BLOCK_CAPcrates/goose/src/agents/platform_extensions/developer/— thetool_inputkeys for eachdevelopertoolcrates/goose-providers/src/openai.rs—OPENAI_BASE_PATHdefaultcrates/goose/src/config/base.rs— secret resolution order (env → keyring →secrets.yaml)Ran the docs build locally (
cd documentation && npm run build) — it completes successfully and all three edited pages render. The build reports no new broken links or anchors from these changes.Related Issues
Discovered while integrating goose into our own policy-enforcement tool — internal tracking reference FailproofAI/failproofai#508 (private; link included for provenance).