diff --git a/DESIGN.md b/DESIGN.md index b35f759..611f00c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -757,9 +757,11 @@ Kept for provenance. **None of this is the current or intended design.** examples per intent, and misroutes appended as new centroid examples. *Replaced by* LLM-as-router (`REARCH.md`): one resident model emits GBNF-constrained JSON and also phrases replies; the embedder is demoted to - a RAG hint. The classifier cascade is still the code path that runs today - (`llmrouter` is wired nil) but it is an interim stopgap, and it is the known - cause of weak RU query handling — not a design to extend. + a RAG hint. *Landed 2026-07-31:* the LLM router is on by default and set + `true` in `deploy/mavend.json`. The classifier cascade stays as the failure + floor — it runs when there is no llama-server to talk to and on any per-turn + LLM error — but routing by seed similarity is the known cause of weak RU + query handling and is not a design to extend. - **Named STT/TTS model picks.** `maven.md` picked faster-whisper small/int8 as primary STT with vosk RU for a low-latency command grammar, and silero (license unverified) as TTS with piper RU as the floor, all on @@ -769,8 +771,11 @@ Kept for provenance. **None of this is the current or intended design.** - **Small-model phrasing claim.** `maven.md` specified "lfm2.5 / sub-1b for phrasing — prompted, not trained," and `SPEC.md` named a specific resident size. Both are superseded by the RU-CPT + joint persona/router SFT plan. - *Resolved 2026-07-30 (#318):* the resident checkpoint is **Qwen3.5-0.8B** - now, with the CPT'd **Qwen3-1.7B** as the target (#122). Note the resident + *Resolved 2026-07-30 (#318), revised 2026-07-31:* the resident checkpoint is + stock **Qwen3-1.7B** (`UD-Q4_K_XL`, `n_ctx` 4096), which replaced + Qwen3.5-0.8B after measuring better on both fixtures + (`MODEL-BAKEOFF-31-07-2026.md`). The CPT'd **Qwen3-1.7B** remains the target + (#122); what stock gets wrong is the persona, not the Russian. Note the resident model is no longer described as untrained — the target is trained end-to-end, which is the substantive change from the old claim. - **sqlcipher at rest.** `maven.md` specified sqlcipher with the key read at diff --git a/internal/delivery/telegramsink/telegramsink.go b/internal/delivery/telegramsink/telegramsink.go index f6db2d2..507f2a6 100644 --- a/internal/delivery/telegramsink/telegramsink.go +++ b/internal/delivery/telegramsink/telegramsink.go @@ -29,6 +29,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -162,13 +163,13 @@ func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error { req, err := http.NewRequestWithContext(ctx, http.MethodPost, s.sendMessageURL(), bytes.NewReader(pb)) if err != nil { - return fmt.Errorf("telegramsink: build request: %w", err) + return fmt.Errorf("telegramsink: build request: %w", s.redact(err)) } req.Header.Set("Content-Type", "application/json") resp, err := s.hc.Do(req) if err != nil { - return fmt.Errorf("telegramsink: sendMessage: %w", err) + return fmt.Errorf("telegramsink: sendMessage: %w", s.redact(err)) } defer resp.Body.Close() rb, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) @@ -188,8 +189,46 @@ func (s *Sink) Send(ctx context.Context, d delivery.Sendable) error { // sendMessageURL — the bot API path. the token is in the URL path // (https://api.telegram.org/bot/sendMessage); telegram does not accept -// it anywhere else. the URL is built per-send from the resolved base — the -// token never leaves the sink, no logging. +// it anywhere else. the URL is built per-send from the resolved base and never +// stored, but it does end up inside transport errors — see redact. func (s *Sink) sendMessageURL() string { return s.base + "/bot" + s.cfg.BotToken + "/sendMessage" } + +// tokenPlaceholder — what a redacted token reads as in an error. Recognisable +// on sight, so nobody reads a redacted URL as a malformed one. +const tokenPlaceholder = "" + +// redact strips the bot token out of a transport error before it becomes a +// returned error, and from there a log line. +// +// This is not hypothetical. net/http wraps every transport failure in +// *url.Error, whose Error() prints the full request URL, and the token is IN +// that URL because telegram accepts it nowhere else. On 2026-08-01 homesrv +// could not reach api.telegram.org, so the retry wrote the whole bot token +// into the daemon log once a minute for as long as the network stayed down. +// The token lives in deploy/telegram.env specifically to stay out of the repo; +// putting it in `docker compose logs` undoes that. +// +// The structural case rewrites url.Error.URL and keeps the error's type, so +// callers matching on *url.Error still work. Anything else falls back to +// scrubbing the rendered message, which loses the type but cannot leak. +// +// There is deliberately no minimum-length guard. A one-character token would +// make this replace every occurrence of that character in the message, which +// is ugly; leaking a short token is worse. New already refuses an empty one. +func (s *Sink) redact(err error) error { + if err == nil { + return err + } + var ue *url.Error + if errors.As(err, &ue) { + clean := *ue + clean.URL = strings.ReplaceAll(clean.URL, s.cfg.BotToken, tokenPlaceholder) + err = &clean + } + if msg := strings.ReplaceAll(err.Error(), s.cfg.BotToken, tokenPlaceholder); msg != err.Error() { + return errors.New(msg) + } + return err +} diff --git a/internal/delivery/telegramsink/telegramsink_test.go b/internal/delivery/telegramsink/telegramsink_test.go index 60b66ff..fae9536 100644 --- a/internal/delivery/telegramsink/telegramsink_test.go +++ b/internal/delivery/telegramsink/telegramsink_test.go @@ -3,6 +3,7 @@ package telegramsink import ( "context" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -326,6 +327,80 @@ func TestSendConnectionRefusedReturnsError(t *testing.T) { } } +// --------------------------- token redaction -------------------------------- + +// realToken — shaped like a real BotFather token, unlike sinkCfg's "123:abc". +// The redaction tests need something long and distinctive enough that finding +// it in an error message is unambiguous. +const realToken = "7556767480:AAFh0vLU9sg8l7DwXU9y-VZQquSJKW3lsVQ" + +// A transport error renders the whole request URL, and telegram accepts the +// token nowhere but the URL path. On 2026-08-01 that put the live bot token in +// `docker compose logs mavend` once a minute while egress was down. +func TestSendTransportErrorRedactsToken(t *testing.T) { + cases := []struct { + name string + run func(*Sink) error + }{ + {"connection refused", func(s *Sink) error { + return s.Send(context.Background(), nudgeSendable(loop.Sev4, "down")) + }}, + {"context cancel", func(s *Sink) error { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + defer cancel() + return s.Send(ctx, nudgeSendable(loop.Sev4, "down")) + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := sinkCfg("http://127.0.0.1:1") + cfg.BotToken = realToken + cfg.Timeout = time.Second + sink, _ := New(cfg) + + err := tc.run(sink) + if err == nil { + t.Fatal("want a transport error") + } + if strings.Contains(err.Error(), realToken) { + t.Fatalf("token leaked into error: %v", err) + } + if !strings.Contains(err.Error(), tokenPlaceholder) { + t.Fatalf("want %q in the redacted error, got: %v", tokenPlaceholder, err) + } + }) + } +} + +// The structural branch keeps the error's type so errors.As still matches. +func TestRedactPreservesURLErrorType(t *testing.T) { + cfg := sinkCfg("http://127.0.0.1:1") + cfg.BotToken = realToken + cfg.Timeout = time.Second + sink, _ := New(cfg) + + err := sink.Send(context.Background(), nudgeSendable(loop.Sev4, "down")) + var ue *url.Error + if !errors.As(err, &ue) { + t.Fatalf("want *url.Error to survive redaction, got %T: %v", err, err) + } + if strings.Contains(ue.URL, realToken) { + t.Fatalf("token left in url.Error.URL: %s", ue.URL) + } +} + +// Nothing to redact must not disturb the error. +func TestRedactLeavesCleanErrorsAlone(t *testing.T) { + sink, _ := New(sinkCfg("http://127.0.0.1:1")) + in := errors.New("dial tcp: no route to host") + if got := sink.redact(in); got != in { + t.Fatalf("want the same error back, got %v", got) + } + if sink.redact(nil) != nil { + t.Fatal("want nil for nil") + } +} + // ----------------------------- proxy seam ----------------------------------- func TestProxyWiredIntoTransport(t *testing.T) {