#20355: fix(gateway): enforce commands.restart guard for config.apply and config.patch
agents
size: S
trusted-contributor
Cluster:
Gateway Hot-Reload Improvements
## 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
#13408: fix(gateway): skip SIGUSR1 restart in config.patch for noop reload ...
by rwmjhb · 2026-02-10
82.6%
#17221: fix(agents): prevent agents from using exec for gateway management
by CornBrother0x · 2026-02-15
80.3%
#16170: fix: restart service manager after update.run
by Swader · 2026-02-14
80.0%
#21120: Security/Gateway: guard dangerous HTTP /tools/invoke re-enables
by bmendonca3 · 2026-02-19
78.8%
#21651: fix(gateway): token fallback + operator.admin scope superset in pai...
by lan17 · 2026-02-20
78.7%
#19885: test(gateway,browser): isolate tests from ambient OPENCLAW_GATEWAY_...
by NewdlDewdl · 2026-02-18
78.2%
#19937: fix(gateway): validate token/password auth modes and isolate gatewa...
by NewdlDewdl · 2026-02-18
77.6%
#11455: fix(gateway): default gateway.mode to local when unset
by AnonO6 · 2026-02-07
77.6%
#16845: fix(daemon): gateway auto-restart on SIGTERM + agent restart guidel...
by kiminbean · 2026-02-15
77.2%
#12953: fix: defer gateway restart until all replies are sent
by zoskebutler · 2026-02-10
77.0%