Repository navigation
fix(stats): count only true blocks (BLOCKED) as blocked - #2161
Merged
Merged
Conversation
FILTERED (query-type filtering, e.g. AAAA via filtering.queryTypes) and NOTFQDN responses were categorized as "blocked", so they inflated the Blocked summary and polluted the Top Blocked table with domains that actually resolve. They now map to a separate "filtered" bucket, and isBlockedRType (which feeds Top Blocked and the per-hour blocked series) matches BLOCKED only. Adds a filtered field to the stats API summary (regenerated) and a Filtered row to the blocky stats CLI output. Fixes 0xERR0R#2151
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2161 +/- ##
==========================================
+ Coverage 87.88% 87.91% +0.02%
==========================================
Files 124 124
Lines 9816 9822 +6
==========================================
+ Hits 8627 8635 +8
+ Misses 925 923 -2
Partials 264 264 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0xERR0R
reviewed
Jul 13, 2026
| model.ResponseTypeNOTFQDN.String(): | ||
| case model.ResponseTypeBLOCKED.String(): | ||
| return categoryBlocked | ||
| case model.ResponseTypeFILTERED.String(), model.ResponseTypeNOTFQDN.String(): |
Owner
There was a problem hiding this comment.
here is a small problem: filtered is overloaded and can be "filtered out by type" or also rebind protection. I think, I will introduce new dedicated response type later, we can leave it as is for now
0xERR0R
enabled auto-merge (squash)
July 13, 2026 19:32
0xERR0R
added a commit
that referenced
this pull request
Jul 13, 2026
#2166) * fix(stats): give DNS rebinding protection its own REBIND response type Rebinding protection returned ResponseTypeFILTERED, the same type used for query-type filtering (filtering.queryTypes). Since #2161 excluded FILTERED from the "blocked" stats, a rebinding hit — an actual attack being stopped — was counted as a benign filter: absent from summary.blocked, topBlockedDomains and the per-hour blocked series, and reported instead under "filtered", whose API description explicitly claims it holds only query-type/NOTFQDN responses. FILTERED was overloaded, so no stats-layer mapping could tell the two apart. Split them at the source: - add model.ResponseTypeREBIND (appended to the enum, so existing iota values are unchanged) and return it from RebindingProtectionResolver - count REBIND as blocked in the stats (summary, Top Blocked, per-hour series) - report REBIND to clients as EDE 15 (Blocked) rather than 17 (Filtered). Per RFC 8914, Blocked is blocking by operator security policy and Filtered is blocking requested by the client; rebinding protection is operator policy. - FILTERED now means exactly what its enum comment says: filtered by query type Also fixes gaps left by #2161: - perHour gains a `filtered` series, so the volume the blocked series no longer carries stays observable instead of vanishing from the API - docs/interfaces.md still documented blocked = BLOCKED + FILTERED + NOTFQDN - cover the Filtered mapping in toAPIStats, the CLI summary row and the per-hour blocked series, none of which were tested BREAKING CHANGE: rebinding-protection hits change response type FILTERED -> REBIND and reason "FILTERED (rebinding protection)" -> "REBIND (rebinding protection)" in query logs and Prometheus labels, and their EDE code changes from 17 (Filtered) to 15 (Blocked). * fix(stats): give DNSSEC validation failures their own BOGUS response type DNSSEC-bogus SERVFAILs reused ResponseTypeBLOCKED, which had two effects. In the statistics they were counted as blocks: they inflated summary.blocked and perHour[].blocked, and a domain whose operator had merely misconfigured DNSSEC showed up in topBlockedDomains. That contradicts the definition of "blocked" as queries blocked to protect the client. Worse, EdeResolver sits above the DNSSEC resolver and rewrites the EDE option from the response type, and util.SetEdns0Option replaces an existing option of the same code. So the ExtendedErrorCodeDNSBogus (6) that the DNSSEC resolver sets on its SERVFAIL was overwritten with Blocked (15) before the answer reached the client. The existing test missed this because it exercises the resolver in isolation, with no EDE resolver above it. Give validation failures their own response type, mirroring REBIND: BOGUS maps to ExtendedErrorCodeDNSBogus, so EdeResolver now reproduces the correct code instead of clobbering it, and it is categorized as an error rather than a block. A SERVFAIL is a resolution error - blocky could not obtain a trustworthy answer - so it joins the chain-level failures in summary.errors. The curated categories now partition every response type, so they always add up to queries. Also point the shipped Grafana dashboard's blocked panels (block rate, blocked/sec, block ratio, top block reasons) at BLOCKED|REBIND. They hard-coded response_type="BLOCKED", so they would have disagreed with /api/stats about what "blocked" means, and they silently counted DNSSEC failures as blocks. The query-log dashboards keep filtering on BLOCKED alone: that panel is "Blocked by Denylist" and is correctly scoped to denylist groups. * docs: fix stale FILTERED terminology and a wrong symbol reference Addresses the two Copilot review comments on #2166. The categorize comment referred to `model.ToExtendedErrorCode`, but there is no package-level function of that name - it is a method on `model.ResponseType`, which makes the behavior hard to trace from the comment. The rebinding docs and the RebindingProtectionResolver.Resolve comment still described the response as FILTERED, mixing the old terminology with the new REBIND semantics introduced in 66784f6. * refactor(stats): derive the per-hour series from curatedSummary perHour re-implemented curatedSummary's category-summing loop, so every new category had to be taught to two places: adding "filtered" in 66784f6 needed an edit in both, and a future one that updated only the summary would leave the per-hour series silently disagreeing about what counts as a block. Derive the point from curatedSummary instead, so categorize() stays the single switch. Correct the isBlockedRType comment while here: since perHour no longer calls it, its only caller is the Top Blocked table. Also reset the shared mock in the EdeResolver suite. `m` is a closure variable that nothing set back to nil, so a resolver installed by one spec's BeforeEach survived into the next and was reused by the `if m == nil` guard, leaking both its response type and its accumulated Calls. It passed only because Ginkgo runs specs in declaration order and the specs relying on the default mock happen to be declared first; under --randomize-all it fails on every seed tried. Finally, cover REBIND and BOGUS in the CLI stats fixture, which exercised neither of the response types split out of BLOCKED.
dadezzz
pushed a commit
to dadezzz/infra_docker-compose
that referenced
this pull request
Jul 28, 2026
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/0xerr0r/blocky](https://github.com/0xERR0R/blocky) | minor | `v0.33.0` → `v0.34.0` | --- ### Release Notes <details> <summary>0xERR0R/blocky (ghcr.io/0xerr0r/blocky)</summary> ### [`v0.34.0`](https://github.com/0xERR0R/blocky/releases/tag/v0.34.0) [Compare Source](0xERR0R/blocky@v0.33.0...v0.34.0) #### Changelog ##### Bug fixes - [`91a8f44`](0xERR0R/blocky@91a8f44): fix(bootstrap): fall back to other resolved addresses when a dial fails ([#​2190](0xERR0R/blocky#2190)) ([@​maximilize](https://github.com/maximilize)) - [`0a20671`](0xERR0R/blocky@0a20671): fix(cache): set DO bit on prefetch reloads to keep DNSSEC-signed entry valid ([#​2158](0xERR0R/blocky#2158)) ([@​logingood](https://github.com/logingood)) - [`d55cd4f`](0xERR0R/blocky@d55cd4f): fix(docker): pre-create writable /app/cache and /logs owned by UID 100 ([#​2172](0xERR0R/blocky#2172)) ([@​0xERR0R](https://github.com/0xERR0R)) - [`dc958ef`](0xERR0R/blocky@dc958ef): fix(lists): skip a lone "/" blocklist line instead of panicking ([#​2156](0xERR0R/blocky#2156)) ([@​maximilize](https://github.com/maximilize)) - [`1d450af`](0xERR0R/blocky@1d450af): fix(resolver): match custom DNS PTR queries case insensitively ([#​2191](0xERR0R/blocky#2191)) ([@​maximilize](https://github.com/maximilize)) - [`472bac9`](0xERR0R/blocky@472bac9): fix(stats): attribute filtered and non-FQDN queries to the client name ([#​2167](0xERR0R/blocky#2167)) ([@​0xERR0R](https://github.com/0xERR0R)) - [`c07bd55`](0xERR0R/blocky@c07bd55): fix(stats): count only true blocks (BLOCKED) as blocked ([#​2161](0xERR0R/blocky#2161)) ([@​maximilize](https://github.com/maximilize)) - [`8435342`](0xERR0R/blocky@8435342): fix(stats): give DNS rebinding protection its own REBIND response type ([#​2166](0xERR0R/blocky#2166)) ([@​0xERR0R](https://github.com/0xERR0R)) ##### Build and dependencies - [`c163c75`](0xERR0R/blocky@c163c75): build(deps): bump actions/setup-go from 6 to 7 ([#​2168](0xERR0R/blocky#2168)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`958760d`](0xERR0R/blocky@958760d): build(deps): bump github.com/breml/rootcerts from 0.3.6 to 0.3.7 ([#​2181](0xERR0R/blocky#2181)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`d94652f`](0xERR0R/blocky@d94652f): build(deps): bump github.com/go-chi/chi/v5 from 5.3.0 to 5.3.1 ([#​2157](0xERR0R/blocky#2157)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`d784ac5`](0xERR0R/blocky@d784ac5): build(deps): bump github.com/jedib0t/go-pretty/v6 from 6.8.1 to 6.8.2 ([#​2154](0xERR0R/blocky#2154)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`23575a4`](0xERR0R/blocky@23575a4): build(deps): bump github.com/jedib0t/go-pretty/v6 from 6.8.2 to 6.8.3 ([#​2183](0xERR0R/blocky#2183)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`b1f8a72`](0xERR0R/blocky@b1f8a72): build(deps): bump github.com/oapi-codegen/runtime from 1.4.2 to 1.5.0 ([#​2164](0xERR0R/blocky#2164)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`4fda662`](0xERR0R/blocky@4fda662): build(deps): bump github.com/oapi-codegen/runtime from 1.5.0 to 1.6.0 ([#​2170](0xERR0R/blocky#2170)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`7c8aeb4`](0xERR0R/blocky@7c8aeb4): build(deps): bump github.com/pires/go-proxyproto from 0.12.0 to 0.13.0 ([#​2153](0xERR0R/blocky#2153)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`77c115b`](0xERR0R/blocky@77c115b): build(deps): bump github.com/pires/go-proxyproto from 0.13.0 to 0.14.0 ([#​2155](0xERR0R/blocky#2155)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`6154af2`](0xERR0R/blocky@6154af2): build(deps): bump github.com/pires/go-proxyproto from 0.14.0 to 0.15.0 ([#​2162](0xERR0R/blocky#2162)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`bd9e3ae`](0xERR0R/blocky@bd9e3ae): build(deps): bump github.com/prometheus/client\_golang from 1.23.2 to 1.24.0 ([#​2185](0xERR0R/blocky#2185)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`6fd79dd`](0xERR0R/blocky@6fd79dd): build(deps): bump github.com/prometheus/client\_golang from 1.24.0 to 1.24.1 ([#​2193](0xERR0R/blocky#2193)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`82777ca`](0xERR0R/blocky@82777ca): build(deps): bump github.com/quic-go/quic-go from 0.60.0 to 0.61.0 ([#​2192](0xERR0R/blocky#2192)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`59f0b0f`](0xERR0R/blocky@59f0b0f): build(deps): bump golang.org/x/net from 0.56.0 to 0.57.0 ([#​2159](0xERR0R/blocky#2159)) ([@​dependabot](https://github.com/dependabot)\[bot]) - [`288ff55`](0xERR0R/blocky@288ff55): build(deps): bump oapi-codegen to 2.7.1 and kin-openapi to 0.144.0 ([#​2195](0xERR0R/blocky#2195)) ([@​0xERR0R](https://github.com/0xERR0R)) ##### Misc - [`541860d`](0xERR0R/blocky@541860d): ci: automate sponsor list in README ([#​2173](0xERR0R/blocky#2173)) ([@​0xERR0R](https://github.com/0xERR0R)) - [`493142d`](0xERR0R/blocky@493142d): ci: build sponsor avatar URLs from the login to avoid HTML escaping ([#​2177](0xERR0R/blocky#2177)) ([@​0xERR0R](https://github.com/0xERR0R)) - [`d9e2827`](0xERR0R/blocky@d9e2827): ci: fail the sponsors job instead of silently wiping the sponsor list ([#​2175](0xERR0R/blocky#2175)) ([@​0xERR0R](https://github.com/0xERR0R)) *** #### ❤️ Support Blocky Blocky is free and open source, developed in my spare time — no telemetry, no ads, no hidden filtering. If it's useful to you, please consider supporting its continued development: [GitHub Sponsors](https://github.com/sponsors/0xERR0R) · [thanks.dev](https://thanks.dev/u/gh/0xERR0R) · [Liberapay](https://liberapay.com/spx01) · [Ko-fi](https://ko-fi.com/0xerr0r) · [PayPal](https://paypal.me/spx01) Thank you to everyone supporting Blocky! 🙏 </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjI4MS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2151.
Following the direction in #2151 (blocked should mean
BLOCKEDonly),categorize()no longer folds query-type-filtered responses into "blocked":FILTERED(e.g. anAAAAlookup dropped byfiltering.queryTypes) andNOTFQDNnow map to a newfilteredcategory instead ofblocked.isBlockedRType— which feeds the Top Blocked table and the per-hourblockedseries — now matchesBLOCKEDonly. Domains that resolve fine on theirArecord but had a filteredAAAAquery (the case reported in Possible bug:blocky statsTop Blocked table cannot be trusted #2151) no longer show up as blocked.User-facing:
summarygains afilteredfield (api/*.gen.goregenerated viago generate).blocky statsprints a new Filtered row.Alternative, if a smaller API surface is preferred: drop the separate
filteredfield and let those responses fall into the unnamed remainder instead. That is a one-line change — say which you want.Tested (
go test ./stats/... ./cmd/... ./api/...green): new specs assertBlockedcountsBLOCKEDonly,FilteredcountsFILTERED+NOTFQDN, and Top Blocked excludes filtered domains. gofmt / vet / golangci-lint v2.12.2 clean.