What is wrong.State.GetSnapshot copies the Snapshot struct but not its maps, so the value it returns shares Domains, Hostnames, Ports and Certificates with the live state. The dashboard and /api/v1/status range over those maps after the read lock is released, while the watcher goroutine writes them under the lock. That is a data race, and when a write lands during an iteration Go aborts the process with fatal error: concurrent map iteration and map write.
Readers: internal/handlers/status.go, HandleStatus line 85, then splitHostnames line 131, buildDomains 147, buildPorts 209, buildCertificates 239; internal/handlers/dashboard.go, HandleDashboard line 69, and the template's ranges over .Snapshot.Domains, .Snapshot.Ports and .Snapshot.Certificates.
Writers: internal/watcher/watcher.go, SetDomainState line 330, SetHostnameState 498, SetPortState 1186, SetCertificateState 1275 and 1298, and the Delete*State calls.
TestGetSnapshot_ReturnsCopy (internal/state/state_test.go line 999) adds a key to the returned map and then checks something the shared map satisfies anyway, so it does not catch this.
Scenario. The dashboard is open (it refreshes every 30 seconds), or something polls /api/v1/status, and a check cycle saves a hostname's state while a request is rendering the hostnames table. Confirmed with the race detector: a test that ranges over GetSnapshot().Hostnames while another goroutine calls SetHostnameState reports WARNING: DATA RACE (mapassign_faststr in SetHostnameState against mapIterStart and mapIterNext in the reader). Each overlap window is short, so the crash is rare per cycle, but the daemon runs for months and every crash loses the checks since the last save.
Correct. The README promises an unauthenticated, read-only dashboard; reading it must not be able to stop monitoring. GetSnapshot returns copies of the four maps (as the code stands, entries in them are replaced, never changed in place, so copying the maps themselves is enough), or the handlers build their response while holding the read lock.
Done when:GetSnapshot returns maps that are not the live ones, or the handlers read under the lock; a test ranges over a snapshot while the state is being written and passes under -race; TestGetSnapshot_ReturnsCopy checks that the returned map is not the live one.
Severity: critical
Model: fable-5-1
**What is wrong.** `State.GetSnapshot` copies the `Snapshot` struct but not its maps, so the value it returns shares `Domains`, `Hostnames`, `Ports` and `Certificates` with the live state. The dashboard and `/api/v1/status` range over those maps after the read lock is released, while the watcher goroutine writes them under the lock. That is a data race, and when a write lands during an iteration Go aborts the process with `fatal error: concurrent map iteration and map write`.
**Where** (at `e8b4006`):
- `internal/state/state.go`, `GetSnapshot`, lines 274-279 (`return *s.snapshot`).
- Readers: `internal/handlers/status.go`, `HandleStatus` line 85, then `splitHostnames` line 131, `buildDomains` 147, `buildPorts` 209, `buildCertificates` 239; `internal/handlers/dashboard.go`, `HandleDashboard` line 69, and the template's ranges over `.Snapshot.Domains`, `.Snapshot.Ports` and `.Snapshot.Certificates`.
- Writers: `internal/watcher/watcher.go`, `SetDomainState` line 330, `SetHostnameState` 498, `SetPortState` 1186, `SetCertificateState` 1275 and 1298, and the `Delete*State` calls.
- `TestGetSnapshot_ReturnsCopy` (`internal/state/state_test.go` line 999) adds a key to the returned map and then checks something the shared map satisfies anyway, so it does not catch this.
**Scenario.** The dashboard is open (it refreshes every 30 seconds), or something polls `/api/v1/status`, and a check cycle saves a hostname's state while a request is rendering the hostnames table. Confirmed with the race detector: a test that ranges over `GetSnapshot().Hostnames` while another goroutine calls `SetHostnameState` reports `WARNING: DATA RACE` (`mapassign_faststr` in `SetHostnameState` against `mapIterStart` and `mapIterNext` in the reader). Each overlap window is short, so the crash is rare per cycle, but the daemon runs for months and every crash loses the checks since the last save.
**Correct.** The README promises an unauthenticated, read-only dashboard; reading it must not be able to stop monitoring. `GetSnapshot` returns copies of the four maps (as the code stands, entries in them are replaced, never changed in place, so copying the maps themselves is enough), or the handlers build their response while holding the read lock.
**Done when:** `GetSnapshot` returns maps that are not the live ones, or the handlers read under the lock; a test ranges over a snapshot while the state is being written and passes under `-race`; `TestGetSnapshot_ReturnsCopy` checks that the returned map is not the live one.
Severity: critical
Model: fable-5-1
clawbot
added this to the 1.0 milestone 2026-10-13 12:32:08 +02:00
clawbot
self-assigned this 2026-10-13 12:32:08 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What is wrong.
State.GetSnapshotcopies theSnapshotstruct but not its maps, so the value it returns sharesDomains,Hostnames,PortsandCertificateswith the live state. The dashboard and/api/v1/statusrange over those maps after the read lock is released, while the watcher goroutine writes them under the lock. That is a data race, and when a write lands during an iteration Go aborts the process withfatal error: concurrent map iteration and map write.Where (at
e8b4006):internal/state/state.go,GetSnapshot, lines 274-279 (return *s.snapshot).internal/handlers/status.go,HandleStatusline 85, thensplitHostnamesline 131,buildDomains147,buildPorts209,buildCertificates239;internal/handlers/dashboard.go,HandleDashboardline 69, and the template's ranges over.Snapshot.Domains,.Snapshot.Portsand.Snapshot.Certificates.internal/watcher/watcher.go,SetDomainStateline 330,SetHostnameState498,SetPortState1186,SetCertificateState1275 and 1298, and theDelete*Statecalls.TestGetSnapshot_ReturnsCopy(internal/state/state_test.goline 999) adds a key to the returned map and then checks something the shared map satisfies anyway, so it does not catch this.Scenario. The dashboard is open (it refreshes every 30 seconds), or something polls
/api/v1/status, and a check cycle saves a hostname's state while a request is rendering the hostnames table. Confirmed with the race detector: a test that ranges overGetSnapshot().Hostnameswhile another goroutine callsSetHostnameStatereportsWARNING: DATA RACE(mapassign_faststrinSetHostnameStateagainstmapIterStartandmapIterNextin the reader). Each overlap window is short, so the crash is rare per cycle, but the daemon runs for months and every crash loses the checks since the last save.Correct. The README promises an unauthenticated, read-only dashboard; reading it must not be able to stop monitoring.
GetSnapshotreturns copies of the four maps (as the code stands, entries in them are replaced, never changed in place, so copying the maps themselves is enough), or the handlers build their response while holding the read lock.Done when:
GetSnapshotreturns maps that are not the live ones, or the handlers read under the lock; a test ranges over a snapshot while the state is being written and passes under-race;TestGetSnapshot_ReturnsCopychecks that the returned map is not the live one.Severity: critical
Model: fable-5-1