[AUTOMATION] fix(clawpatch): address daily finding#258
Conversation
Finding: Hook install is not rolled back when daemon startup fails Severity: medium Clawpatch ID: fnd_sig-feat-cli-command-56a5b2eae3-_0b3f60c190
Greptile SummaryThis PR delays Claude hook installation until daemon startup has passed more checks. The main changes are:
Confidence Score: 4/5This is close, but the timeout regression should be fixed before merging.
|
| Filename | Overview |
|---|---|
| internal/guard/cli/cli.go | Moves hook installation later in daemon startup and changes dashboard serving to use a pre-bound listener. |
| internal/guard/cli/cli_test.go | Adds a regression test for preventing Claude settings writes when daemon startup fails. |
Reviews (1): Last reviewed commit: "fix(clawpatch): address daily finding" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _ = browser.OpenURL("http://" + *addr) | ||
| } | ||
| return localServer.ListenAndServe(*addr) | ||
| return http.Serve(listener, localServer.Handler()) |
There was a problem hiding this comment.
This change bypasses localServer.ListenAndServe, which was setting ReadHeaderTimeout to 5 seconds. The package-level http.Serve uses a zero-value server, so a client that connects to the dashboard address and slowly sends headers can keep connections open indefinitely, especially when --addr or KONTEXT_ADDR exposes the daemon beyond loopback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a3521ead3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ = browser.OpenURL("http://" + *addr) | ||
| } | ||
| return localServer.ListenAndServe(*addr) | ||
| return http.Serve(listener, localServer.Handler()) |
There was a problem hiding this comment.
When the daemon is run with a non-loopback --addr/KONTEXT_ADDR or there is an untrusted local client, this replacement drops the ReadHeaderTimeout: 5 * time.Second that server.ListenAndServe used to apply, so slow clients can hold HTTP connections open indefinitely by trickling request headers. Please serve the pre-bound listener through an http.Server configured with the same timeout instead of calling http.Serve directly.
Useful? React with 👍 / 👎.
michiosw
changed the title
1 participant
Where We Are
kontext guard startinstalled Claude hooks before the daemon proved it could bind and serve. If startup failed after that write, Claude was left pointing at a dead local Guard hook.Where We Want To Go
Daemon startup should only install Claude hooks after the local runtime and HTTP listener are ready. A failed start should leave
~/.claude/settings.jsonunchanged.How do we get there
Bind the TCP listener before hook installation, then serve with that pre-opened listener. Add a regression test that starts the daemon with an invalid listen address and proves no Claude settings file is created. Verified with
go test ./...,go vet ./...,npm exec --yes --package pnpm@10.0.0 -- pnpm install --frozen-lockfile,npm exec --yes --package pnpm@10.0.0 -- pnpm --dir web/guard-dashboard typecheck, andgit diff --check.