Files
Dimitrije DimitrijevicandClaude Opus 5.5 54f9626121 Add 80% limit warning, stop repeat notifications, fix review findings
Notifications:
- Warn once when a counter reaches warn_percent of its limit, through
  separate warn_*_url endpoints.
- Write ntf/ markers so a lost Redis reply cannot re-send a breach.
- Retry only when the request never left or the web app answered 429/5xx.

Fixes:
- Retry transactions Redis rejected; treat pool timeouts as definite.
- Install the signal handler before startup work.
- Stop logging the login suffix; redact notify URL credentials.
- Reject corrupt or empty CDBs, truncated hash tables included.
- Require a Sentinel password, refuse example placeholders, and default
  admin_addr to localhost.
- Normalise /counters domains, count on the arrival day, log rejections,
  hide internals in errors and exit 1 on a failed drain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-09 09:01:56 +01:00
..
2026-10-15 02:12:18 +02:00
2026-10-15 02:12:18 +02:00

Proof tests from the 2026-10-14 code review

These are the tests the two reviewers wrote to demonstrate their findings. They live under a directory whose name starts with _, so go build ./..., go test ./... and go vet ./... ignore them. They are evidence, not part of the suite. When a finding is fixed, turn its proof test into a real regression test in the package it belongs to.

Dir Reviewer Style
a/ A — correctness and runtime Demonstrations: the tests pass and print the evidence with t.Log (run with -v).
b/ B — security, contracts, ops Assertions: the tests fail while the defect exists (a few PASSes only log evidence).

Running

Copy the files into a scratch copy of the repo, so the real packages stay clean:

rm -rf /tmp/mc_proof && cp -R /Users/dice/Work/migadu_counters /tmp/mc_proof && cd /tmp/mc_proof
cp -R review/_proof_tests/a/internal/. internal/          # reviewer A
cp -R review/_proof_tests/b/internal/. internal/          # reviewer B
cp -R review/_proof_tests/b/cmd/. cmd/
go test -count=1 -v -run 'TestReview' ./internal/... ./cmd/...
go test -run '^$' -bench Review -benchmem ./internal/txlog/   # A's CDB parsing benchmarks

Requirements and notes:

  • a/internal/store/review_test.go (TestReview_TimeoutAfterWriteStillCounts) needs a real Redis on port 16401: redis-server --port 16401 --save '' --appendonly no. The test uses DEBUG SLEEP to make the reply arrive after the client timeout. It fails to connect if no Redis is running.
  • b/bad.cdb is a three-row fixture with one malformed row (bad.com x12|A/i=5). It is used by b/internal/server/review_b4_test.go, whose path was updated to ../../review/_proof_tests/b/bad.cdb.
  • a/run/ holds the config and the app log from reviewer A's manual reproduction of "SIGHUP during startup kills the daemon" (merged item #10). The paths in config.toml point at the reviewer's former scratch copy, so edit them before reuse. The log stops after domains db loaded, which is the evidence.

Map to the merged report (../README.md)

Merged # Proof
1 a/internal/server/review_test.go TestReview_ClientDisconnectLosesCount, TestReview_ClientDisconnectNoDelay
2 b/internal/server/review_b_test.go TestReviewB_InMailFromDropsRecipientCount, TestReviewB_OutRecipientDropsSenderCount; a/… TestReview_MasterLoginAndOddSenders
3 a/internal/server/review_test.go TestReview_MasterLoginAndOddSenders (first case)
4 b/internal/server/review_b2_test.go TestReviewB_LostCrossingOnReplyTimeout; a/internal/server/review_test.go TestReview_PartialExecFailure; a/internal/store/review_test.go (real Redis)
5 a/internal/txlog/review_bench_test.go
6 b/internal/config/review_b_test.go TestReviewB_MisconfigStarts; b/…/review_b_test.go TestReviewB_EmptyTokenOpen
7 b/internal/server/review_b_test.go TestReviewB_PlusTagSeparateCounters
8 b/internal/server/review_b4_test.go; a/internal/counting/review_test.go
10 a/run/ (manual reproduction)
11 b/internal/server/review_b_test.go TestReviewB_RejectLogHasNoTraceID
13 b/internal/server/review_b3_test.go
14 b/internal/server/review_b_test.go TestReviewB_ErrorLeak
15 b/internal/config/review_b_test.go (case "sentinel without password")
16 b/cmd/counterctl/review_b_test.go TestReviewB_31DayWarningOffByOne
17 b/internal/server/review_b_test.go TestReviewB_PanicNotInMetrics

Fixed in round 1 (2026-10-14) — regression tests now in the suite

Merged # Regression test
1 internal/server TestClientDisconnectStillCounts, TestAcceptedBeforeCounting, TestQueueFull, TestDrainCountsEverything; internal/counting/queue_test.go
2 internal/server TestIn_OddMailFromStillCountsRecipient, TestOut_OddRecipientStillCountsSender; internal/api TestValidateOut, TestValidateIn, TestNormalizeEmail
3 internal/server TestOut_LoginSuffixCountsBaseAddress; internal/api TestNormalizeAuthUser
4 internal/server TestPartialExecFailureStillNotifies, TestLostReplyStillNotifies; internal/counting TestCounter_PartialExecFailure, TestCounter_LostReplyReReadsMaster; internal/store TestIncr_PartialFailure, TestOutcomeUnknown
5 internal/domains TestLookup_CachesParsedRecord, TestLookup_CacheBounded, BenchmarkLookupSmall/BenchmarkLookupHuge
7 internal/server TestIn_PlusTagCountsMailbox; internal/counting TestPlan_PlusTagCountsMailbox; internal/domains TestMailbox (decision 2026-10-15: strip the tag, as rcptd does)
8 internal/server TestMalformedRowCountsOtherSide; internal/counting TestPlan_MalformedRowIsNotHosted; internal/domains TestLookup_MalformedRow; internal/txlog TestTrail_MalformedRow
6 internal/config TestValidate_Errors (auth_token / unknown keys), TestLoad_TokenFromEnvOnly, TestLoad_Defaults (:5555); cmd/migadu_counters TestRun_FlagsAndExitCodes

Fixed in round 2 (2026-11-09) — regression tests now in the suite

Merged # Regression test
9 internal/notify TestNotify_TimeoutAfterWriteIsNotRetried, TestNotify_BackoffReleasesSlot, TestNotify_ShutdownFinishesPendingRetries, TestNotify_ShutdownDeadlineReportsRealAttempts
10 cmd/migadu_counters TestRun_SIGHUPDuringStartupIsQueued
11 internal/server TestReject_LeavesTraceInBothLogs
12 internal/domains TestParseLimits_OverrideKeysLowercased
13 internal/txlog TestNotificationLines_RedactCredentials; internal/config TestWarnings_PlainHTTPWithBasicAuth
14 internal/server TestErrors_DoNotExposeInternals; internal/config TestLoad_Defaults
15 internal/config TestValidate_Errors (case "sentinel needs password"), TestLoad_SentinelPasswordOptional
16 cmd/counterctl TestDateWarning_Boundary
17 internal/server TestPanic_CountedInRequestMetrics

The proof files stay here as the review record; they no longer compile against the current API (CountResponse, store.Incr) and are not meant to.