← Back to PRs

#20355: fix(gateway): enforce commands.restart guard for config.apply and config.patch

by Clawborn open 2026-02-18 20:38 View on GitHub →
agents size: S trusted-contributor
## Problem `config.apply` and `config.patch` both trigger a SIGUSR1 restart as a side effect, but unlike the `restart` action they did not check `commands.restart`. This allowed agents to bypass the restart guard by using `config.patch` to set `commands.restart: true`, which itself caused a restart. ## Fix Now all three write-triggered-restart actions (`restart`, `config.apply`, `config.patch`) require `commands.restart: true`, with a clear error message explaining why (`config.apply` and `config.patch` trigger a restart as a side effect). ## Tests Added 4 test cases to `gateway-tool.test.ts`: - `config.apply` blocked when `commands.restart` is false - `config.apply` allowed when `commands.restart` is true - `config.patch` blocked when `commands.restart` is false - `config.patch` allowed when `commands.restart` is true Fixes #20245 Co-authored-by: Clawborn <tianrun.yang103@gmail.com> <!-- greptile_comment --> <h3>Greptile Summary</h3> Closes a security gap where agents could bypass the `commands.restart` guard by using `config.apply` or `config.patch` (which trigger a SIGUSR1 restart as a side effect) instead of the guarded `restart` action. The fix adds the same `commands.restart !== true` check to both `config.apply` and `config.patch` in `gateway-tool.ts`, with clear error messages explaining the side-effect relationship. - `config.apply` and `config.patch` now require `commands.restart: true`, consistent with the `restart` action - Error messages include an explanation that these actions trigger a restart as a side effect - New test file covers the blocking cases (false, unset) for all three actions - **Missing:** The PR description claims success-path tests for `config.apply` and `config.patch` (when `commands.restart` is true), but these are absent from the test file. The current mock (`callGatewayTool` returning `{ ok: true }`) lacks a `hash` field, so the success path would fail at `resolveConfigWriteParams` — the mock would need to be extended to support these tests. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — the implementation is correct and closes a real security gap, though test coverage has a notable gap. - The implementation changes are minimal, well-placed (fail-fast before side effects), and follow the existing pattern used by the restart action. The security fix is sound. Score is 4 instead of 5 because the PR description lists success-path tests that are missing from the actual code, leaving incomplete coverage for the happy path of config.apply and config.patch when commands.restart is enabled. - `src/agents/tools/gateway-tool.test.ts` — missing the success-path tests described in the PR <sub>Last reviewed commit: 4376c64</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs