fix: harden gossip, HLC restarts, watcher races, MCP args, pagination (#3)

Implements the top findings from the codebase review, verified with tests and live CLI/MCP checks.

**Gossip integrity**
- Push validation: 4 MiB body cap, 1000-row batch cap; rows claiming the local node id (vector-poisoning), empty node ids, and negative HCLs rejected (internal/watch/gossip.go, internal/db/gossip.go)
- Reconcile-on-pull: Run returns the pulled count, syncGossip rebuilds derived state when > 0 — entry-count comparison could never fire, so synced observations never materialized into searchable entries

**Data-layer safety**
- HLC resumed from MAX(hcl) at Open (hlc.SeekTo): a restart with a regressed wall clock cannot reissue values the (node_id, hcl) locator and pull cursors depend on
- Writer serialization: _txlock=immediate DSN + SetMaxOpenConns(1) + per-KnoxDB mutex around RecordObservation's check-then-insert dedup (closes duplicate-row race)

**Watch daemon**
- Ticker guard flags now atomic.Bool (was a cross-goroutine data race)
- Trailing-edge per-path debounce (timer-based, pruned on fire/delete)
- Recursive watches (startup tree walk + watcher.Add on dir Create); Rename re-ingests, Remove cancels pending ingests

**MCP + CLI**
- Strict arg validation, no silent clamping: thread_id 0 errors instead of renaming thread #1; empty knox_thread_link {} errors instead of false success; thread existence checked before writes; golden-thread tool nil-safe
- --page 0 errors instead of panicking; query/recent pagination actually pages (page x limit)

**Tests** (new internal/hlc and internal/db packages): SeekTo monotonicity, concurrent dedup race, push validation, reopen HCL monotonicity, batch caps, self-spoof rejection, idempotency on observation counts.

Verified: go build, go vet, full suite with -race, live MCP stdio transcripts against a scratch DB.
Reviewed-on: #3
Co-authored-by: David Gwilliam <dhgwilliam@gmail.com>
Co-committed-by: David Gwilliam <dhgwilliam@gmail.com>
This commit was merged in pull request #3.
This commit is contained in:
2026-09-17 09:06:08 +00:00
committed by david
parent d6d2a24ddc
commit bb852faa27
10 changed files with 733 additions and 116 deletions
+108 -4
View File
@@ -1,8 +1,12 @@
package watch
import (
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
"path/filepath"
"strings"
"testing"
"github.com/david/knox/internal/db"
@@ -228,16 +232,116 @@ func TestGossipIdempotent(t *testing.T) {
defer sb.Close()
Run(b, nil, []string{sa.URL})
before, err := b.EntryCount()
entryBefore, err := b.EntryCount()
if err != nil {
t.Fatal(err)
}
stats, err := b.Stats()
if err != nil {
t.Fatal(err)
}
obsBefore, _ := stats["total_observations"].(int)
Run(b, nil, []string{sa.URL})
after, err := b.EntryCount()
entryAfter, err := b.EntryCount()
if err != nil {
t.Fatal(err)
}
if before != after {
t.Errorf("second sweep changed entry count: %d -> %d", before, after)
if entryBefore != entryAfter {
t.Errorf("second sweep changed entry count: %d -> %d", entryBefore, entryAfter)
}
stats, err = b.Stats()
if err != nil {
t.Fatal(err)
}
obsAfter, _ := stats["total_observations"].(int)
if obsBefore != obsAfter {
t.Errorf("second sweep duplicated observations: %d -> %d", obsBefore, obsAfter)
}
}
// TestHandleBatchRejectsOversizedBatch: more than maxBatchRows in one POST
// must be refused up front, before any insert work.
func TestHandleBatchRejectsOversizedBatch(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := make([]db.GossipObservation, maxBatchRows+1)
for i := range rows {
rows[i] = db.GossipObservation{
NodeID: "0123456789abcdef0123456789abcdef", HCL: int64(i + 1),
Fingerprint: "fp:oversized", SourceID: "test", Title: "t", Summary: "s",
CollectedAt: "2026-08-29T00:00:00Z",
}
}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("oversized batch: want 400, got %d", resp.StatusCode)
}
}
// TestHandleBatchRejectsHugeBody: an oversized body (beyond the 4 MiB cap)
// must be refused even when the row count is small.
func TestHandleBatchRejectsHugeBody(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := []db.GossipObservation{{
NodeID: "0123456789abcdef0123456789abcdef", HCL: 1,
Fingerprint: "fp:huge", SourceID: "test", Title: "t",
Summary: strings.Repeat("x", 5<<20), // 5 MiB summary
CollectedAt: "2026-08-29T00:00:00Z",
}}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("huge body: want 400, got %d", resp.StatusCode)
}
}
// TestHandleBatchSkipsSelfRows: rows claiming the receiver's own node_id are
// dropped at the HTTP layer too (the poisoning vector), reported as conflicts.
func TestHandleBatchSkipsSelfRows(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := []db.GossipObservation{
{NodeID: b.NodeID(), HCL: 1, Fingerprint: "fp:self1", SourceID: "test", Title: "t", Summary: "s", CollectedAt: "2026-08-29T00:00:00Z"},
{NodeID: b.NodeID(), HCL: 2, Fingerprint: "fp:self2", SourceID: "test", Title: "t", Summary: "s", CollectedAt: "2026-08-29T00:00:00Z"},
}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("want 200, got %d", resp.StatusCode)
}
var out struct {
Accepted int `json:"accepted"`
Conflict int `json:"conflict"`
}
if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
t.Fatalf("decode: %v", err)
}
if out.Accepted != 0 || out.Conflict != 2 {
t.Errorf("self rows: want accepted=0 conflict=2, got accepted=%d conflict=%d", out.Accepted, out.Conflict)
}
}