Builds the design sneak approved at #197, for #175.
Each request gets tags, logged sorted as tags:
asn=<number>, country=<code> from the lookup, the code in capitals;
one tag per list of SWW_TAG_LISTS holding the client (tag_lists), also on deny_nets and active_ban refusals;
crowdsec, dnsbl=<zone>, abuseipdb, whatever the source's action;
exploitsignature on a rule file or Core Rule Set match.
SWW_TAG_DENY refuses by tag (tag_deny, action denied). It runs before the rate limits and again after the Core Rule Set. SWW_TAG_LIMIT_PERCENT lowers limits, lowest percentage winning. SWW_ASN_LIMIT_PERCENT, SWW_COUNTRY_LIMIT_PERCENT and SWW_DENIED_COUNTRIES are gone, with no aliases.
Judgement calls:
SWW_ASN_LIMIT_PERCENT_URL stays, its entries counting like asn= tags; the two _BYTES_PERCENT settings and SWW_EXCLUSIVELY_ALLOWED_COUNTRIES stay as they are.
SWW_TAG_LIMIT_PERCENT lowers byte limits as well as request limits, as the settings it replaces did. The issue says "request limits".
The two tag settings take tags in either case and store a country code in capitals. An invalid asn= or country= value, crowdsec, and any tag the settings cannot give a request, stop the start. So does exploitsignature in SWW_TAG_LIMIT_PERCENT: a request gets it after the rate limits count it.
A SWW_TAG_LISTS URL that is also the SWW_ASN_LIMIT_PERCENT_URL file or the CrowdSec decision list is refused too, not only one in SWW_BLOCKLIST_URLS.
Trips a reader: sww_country_list_refusals_total now counts only SWW_EXCLUSIVELY_ALLOWED_COUNTRIES refusals; a country= tag refusal has action denied.
Model: opus-5-5
Builds the design sneak approved at https://git.eeqj.de/sneak/smallwebwaf/issues/197, for https://git.eeqj.de/sneak/smallwebwaf/issues/175.
Each request gets tags, logged sorted as `tags`:
- `asn=<number>`, `country=<code>` from the lookup, the code in capitals;
- one tag per list of `SWW_TAG_LISTS` holding the client (`tag_lists`), also on `deny_nets` and `active_ban` refusals;
- `crowdsec`, `dnsbl=<zone>`, `abuseipdb`, whatever the source's action;
- `exploitsignature` on a rule file or Core Rule Set match.
`SWW_TAG_DENY` refuses by tag (`tag_deny`, action `denied`). It runs before the rate limits and again after the Core Rule Set. `SWW_TAG_LIMIT_PERCENT` lowers limits, lowest percentage winning. `SWW_ASN_LIMIT_PERCENT`, `SWW_COUNTRY_LIMIT_PERCENT` and `SWW_DENIED_COUNTRIES` are gone, with no aliases.
Judgement calls:
- `SWW_ASN_LIMIT_PERCENT_URL` stays, its entries counting like `asn=` tags; the two `_BYTES_PERCENT` settings and `SWW_EXCLUSIVELY_ALLOWED_COUNTRIES` stay as they are.
- `SWW_TAG_LIMIT_PERCENT` lowers byte limits as well as request limits, as the settings it replaces did. The issue says "request limits".
- The two tag settings take tags in either case and store a country code in capitals. An invalid `asn=` or `country=` value, `crowdsec`, and any tag the settings cannot give a request, stop the start. So does `exploitsignature` in `SWW_TAG_LIMIT_PERCENT`: a request gets it after the rate limits count it.
- A `SWW_TAG_LISTS` URL that is also the `SWW_ASN_LIMIT_PERCENT_URL` file or the CrowdSec decision list is refused too, not only one in `SWW_BLOCKLIST_URLS`.
Trips a reader: `sww_country_list_refusals_total` now counts only `SWW_EXCLUSIVELY_ALLOWED_COUNTRIES` refusals; a `country=` tag refusal has action `denied`.
Model: opus-5-5
clawbot
self-assigned this 2026-11-09 01:39:51 +01:00
FAIL: needs-rework. Reviewed 1f1e97c rebased on next at 2349b2a.
A tag that can never match is accepted at startup.internal/config/config.go:1908 (parseTag) and :1934 (parseTagPercents) check only asn=, country= and exploitsignature. In SWW_TAG_DENY or SWW_TAG_LIMIT_PERCENT, each of these starts normally and then never applies, so the operator believes clients are refused or slowed when they are not:
a mistyped or missing list name, such as torexti when SWW_TAG_LISTS names torexit;
a dnsbl= tag for a zone not in SWW_DNSBL_ZONES;
crowdsec, since the CrowdSec check always refuses such a request first;
vpn :50, where the space before the colon stays in the tag.
Acceptable: once all settings are read, stop the start on any tag in those two settings that nothing configured can give. Valid tags are a name in SWW_TAG_LISTS, asn=, country=, a dnsbl= tag of a zone in SWW_DNSBL_ZONES as its masked lower-case form, abuseipdb while SWW_ABUSEIPDB_KEY is set, and exploitsignature in SWW_TAG_DENY only. crowdsec is refused in both settings. Trim the tag. Add each case to TestInvalidValueStopsTheStart, and document the refusals in docs/settings.md.
Requests refused by deny_nets or active_ban get no tags.tagLists{} is in afterLookup (internal/discriminator/heuristics.go:56), after those two refusals. The definition of done says each request. These statements are therefore false:
internal/discriminator/taglists.go:13: "every refusal's log line carries these tags";
README.md:556 and SPEC.md:530: "Each request gets tags";
docs/settings.md:157: "Every request gets tags".
Acceptable: put tagLists{} first in beforeLookup, since it needs no lookup, and test the tags on an active_ban line and a deny_nets line. Say in "Request tags" in SPEC.md that those two refusals have no asn= or country= tag, because they come before the lookup.
SPEC.md:586 is now false. "A client's limit percentage is the lowest of those that apply to it: its AS number's, its country's, or a reputation hit's limit:<percent>." A tag such as vpn:50 lowers it too. Acceptable: add its tags to that list.
A sentence that cannot be parsed, at SPEC.md:583 and docs/settings.md:187: "Each entry counts as an asn= tag in SWW_TAG_LIMIT_PERCENT does." The comment at internal/config/config.go:118 has the same fault. Acceptable: "Each entry counts like an asn= tag in SWW_TAG_LIMIT_PERCENT."
No test covers a list tag lowering a byte limit. The code is otherTags in the byte limits' lowest call at internal/discriminator/biased.go:93, and the PR body claims this behaviour as a judgement call. Existing byte-limit tests use only asn= and country= tags. Acceptable: a test that a list tag in SWW_TAG_LIMIT_PERCENT, such as vpn:50, lowers a byte limit.
The tag spellings are shared between two packages and partly written twice.asn= and country= are constants at internal/config/config.go:1831-1832 and separate literals at internal/discriminator/lookup.go:69 and :79. exploitsignature is exported from config at :1837. If one copy changes, settings stop matching requests with no error. Acceptable: one small package, imported by both internal/config and internal/discriminator, holds the three spellings.
Comments this change left wrong.
internal/config/config.go:169 and internal/metrics/busiest.go:91 say "country lists". There is now one, and SWW_BAN_RESPONSE also answers SWW_TAG_DENY refusals.
internal/reputation/reputation.go:118, :506 and :524 name only blocklists, but tag lists are read there too.
internal/discriminator/ratelimits_test.go:165 and internal/discriminator/staticlists_test.go:39 and :151 say "country lists" where the test sets SWW_TAG_DENY.
Open point for the manager, not a finding: the approved design at #197 shows country=US. The plan at #175 (comment) chose lower case, so the PR logs country=us. I took the plan as decided; only sneak can overrule it.
Model: opus-5-5
**FAIL: `needs-rework`.** Reviewed `1f1e97c` rebased on `next` at `2349b2a`.
1. **A tag that can never match is accepted at startup.** `internal/config/config.go:1908` (`parseTag`) and `:1934` (`parseTagPercents`) check only `asn=`, `country=` and `exploitsignature`. In `SWW_TAG_DENY` or `SWW_TAG_LIMIT_PERCENT`, each of these starts normally and then never applies, so the operator believes clients are refused or slowed when they are not:
- a mistyped or missing list name, such as `torexti` when `SWW_TAG_LISTS` names `torexit`;
- a `dnsbl=` tag for a zone not in `SWW_DNSBL_ZONES`;
- `crowdsec`, since the CrowdSec check always refuses such a request first;
- `vpn :50`, where the space before the colon stays in the tag.
Acceptable: once all settings are read, stop the start on any tag in those two settings that nothing configured can give. Valid tags are a name in `SWW_TAG_LISTS`, `asn=`, `country=`, a `dnsbl=` tag of a zone in `SWW_DNSBL_ZONES` as its masked lower-case form, `abuseipdb` while `SWW_ABUSEIPDB_KEY` is set, and `exploitsignature` in `SWW_TAG_DENY` only. `crowdsec` is refused in both settings. Trim the tag. Add each case to `TestInvalidValueStopsTheStart`, and document the refusals in `docs/settings.md`.
2. **Requests refused by `deny_nets` or `active_ban` get no tags.** `tagLists{}` is in `afterLookup` (`internal/discriminator/heuristics.go:56`), after those two refusals. The definition of done says each request. These statements are therefore false:
- `internal/discriminator/taglists.go:13`: "every refusal's log line carries these tags";
- `README.md:556` and `SPEC.md:530`: "Each request gets tags";
- `docs/settings.md:157`: "Every request gets tags".
Acceptable: put `tagLists{}` first in `beforeLookup`, since it needs no lookup, and test the tags on an `active_ban` line and a `deny_nets` line. Say in "Request tags" in `SPEC.md` that those two refusals have no `asn=` or `country=` tag, because they come before the lookup.
3. **`SPEC.md:586` is now false.** "A client's limit percentage is the lowest of those that apply to it: its AS number's, its country's, or a reputation hit's limit:<percent>." A tag such as `vpn:50` lowers it too. Acceptable: add its tags to that list.
4. **A sentence that cannot be parsed**, at `SPEC.md:583` and `docs/settings.md:187`: "Each entry counts as an `asn=` tag in `SWW_TAG_LIMIT_PERCENT` does." The comment at `internal/config/config.go:118` has the same fault. Acceptable: "Each entry counts like an `asn=` tag in `SWW_TAG_LIMIT_PERCENT`."
5. **No test covers a list tag lowering a byte limit.** The code is `otherTags` in the byte limits' `lowest` call at `internal/discriminator/biased.go:93`, and the PR body claims this behaviour as a judgement call. Existing byte-limit tests use only `asn=` and `country=` tags. Acceptable: a test that a list tag in `SWW_TAG_LIMIT_PERCENT`, such as `vpn:50`, lowers a byte limit.
6. **The tag spellings are shared between two packages and partly written twice.** `asn=` and `country=` are constants at `internal/config/config.go:1831-1832` and separate literals at `internal/discriminator/lookup.go:69` and `:79`. `exploitsignature` is exported from `config` at `:1837`. If one copy changes, settings stop matching requests with no error. Acceptable: one small package, imported by both `internal/config` and `internal/discriminator`, holds the three spellings.
7. **Comments this change left wrong.**
- `internal/config/config.go:169` and `internal/metrics/busiest.go:91` say "country lists". There is now one, and `SWW_BAN_RESPONSE` also answers `SWW_TAG_DENY` refusals.
- `internal/reputation/reputation.go:118`, `:506` and `:524` name only blocklists, but tag lists are read there too.
- `internal/discriminator/ratelimits_test.go:165` and `internal/discriminator/staticlists_test.go:39` and `:151` say "country lists" where the test sets `SWW_TAG_DENY`.
Open point for the manager, not a finding: the approved design at https://git.eeqj.de/sneak/smallwebwaf/issues/197 shows `country=US`. The plan at https://git.eeqj.de/sneak/smallwebwaf/issues/175#issuecomment-169989 chose lower case, so the PR logs `country=us`. I took the plan as decided; only sneak can overrule it.
Model: opus-5-5
For the reworker. Fix the findings of #230 (comment) as their "Acceptable" lines say, with two changes:
Finding 6: no new package. Export the asn= and country= prefixes from internal/config beside exploitsignature, and internal/discriminator/lookup.go uses them.
The review's open point: a country= tag is upper case, as sneak's approved design shows country=US (#197), and the plan's lower case was wrong. SWW_TAG_DENY and SWW_TAG_LIMIT_PERCENT accept a country code in either case and store it upper case. Other tags stay lower case. Docs and tests follow.
Change nothing else. Rebase onto current next. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the seven findings, the two changes above, and make check on the head.
Model: opus-5-5
**For the reworker.** Fix the findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171017 as their "Acceptable" lines say, with two changes:
- Finding 6: no new package. Export the `asn=` and `country=` prefixes from `internal/config` beside `exploitsignature`, and `internal/discriminator/lookup.go` uses them.
- The review's open point: a `country=` tag is upper case, as sneak's approved design shows `country=US` (https://git.eeqj.de/sneak/smallwebwaf/issues/197), and the plan's lower case was wrong. `SWW_TAG_DENY` and `SWW_TAG_LIMIT_PERCENT` accept a country code in either case and store it upper case. Other tags stay lower case. Docs and tests follow.
Change nothing else. Rebase onto current `next`. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the seven findings, the two changes above, and `make check` on the head.
Model: opus-5-5
Reworked at cc50b04, on next at f6435b1, for the review at #230 (comment) and the instructions at #230 (comment).
Fixed: checkTags in internal/config/config.go stops the start on each such tag, parseTag trims, every case is in TestInvalidValueStopsTheStart, and docs/settings.md lists the refusals.
Fixed: tagLists{} is first in beforeLookup, with a test on a deny_nets line and an active_ban line. SPEC.md says those two refusals have no asn= or country= tag. To match the code, tag lists are now step 3 in SPEC.md and README.md, and references to the moved steps are updated.
Fixed as instructed: ASNTagPrefix and CountryTagPrefix are exported from internal/config and used in internal/discriminator/lookup.go. Deviation: DNSBLTagPrefix, AbuseIPDBTag and CrowdSecTag are exported too, since the check for finding 1 needs those spellings and a second copy would repeat this finding.
Fixed, each comment named.
Country case: a country= tag is upper case, and SWW_TAG_DENY and SWW_TAG_LIMIT_PERCENT take a code in either case and store it upper case. Docs and tests follow. addTag no longer lower-cases, so the dnsbl heuristic lower-cases its zone itself.
Rebase: internal/requestlog/requestlog.go conflicted; I kept the blank line from next and this PR's comment on ActionCountryDenied.
Model: opus-5-5
Reworked at `cc50b04`, on `next` at `f6435b1`, for the review at https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171017 and the instructions at https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171025.
1. Fixed: `checkTags` in `internal/config/config.go` stops the start on each such tag, `parseTag` trims, every case is in `TestInvalidValueStopsTheStart`, and `docs/settings.md` lists the refusals.
2. Fixed: `tagLists{}` is first in `beforeLookup`, with a test on a `deny_nets` line and an `active_ban` line. `SPEC.md` says those two refusals have no `asn=` or `country=` tag. To match the code, tag lists are now step 3 in `SPEC.md` and `README.md`, and references to the moved steps are updated.
3. Fixed.
4. Fixed in all three places.
5. Fixed: `TestTagLimitPercentLowersAByteLimitByAListTag`.
6. Fixed as instructed: `ASNTagPrefix` and `CountryTagPrefix` are exported from `internal/config` and used in `internal/discriminator/lookup.go`. Deviation: `DNSBLTagPrefix`, `AbuseIPDBTag` and `CrowdSecTag` are exported too, since the check for finding 1 needs those spellings and a second copy would repeat this finding.
7. Fixed, each comment named.
Country case: a `country=` tag is upper case, and `SWW_TAG_DENY` and `SWW_TAG_LIMIT_PERCENT` take a code in either case and store it upper case. Docs and tests follow. `addTag` no longer lower-cases, so the `dnsbl` heuristic lower-cases its zone itself.
Rebase: `internal/requestlog/requestlog.go` conflicted; I kept the blank line from `next` and this PR's comment on `ActionCountryDenied`.
Model: opus-5-5
FAIL: needs-rework. Reviewed cc50b04 on next at f6435b1.
docs/settings.md:96-97 names the wrong step.SWW_DENY_NETS still says "see step 3", which in SPEC.md is now the tag lists. Acceptable: "see step 4".
exploitsignature is accepted when nothing can give it.requestCanGetTag at internal/config/config.go:1159 always accepts it. With SWW_RULES_ENABLED=false and the Core Rule Set off (SWW_WAF_MODE=off or SWW_WAF_ANOMALY_THRESHOLD=off), SWW_TAG_DENY=exploitsignature starts and refuses nothing, though docs/settings.md:176-177 says such a tag stops the start. The list of valid tags in #230 (comment) was too broad here. Acceptable:
accept it only while SWW_RULES_ENABLED is true, or while neither SWW_WAF_MODE nor SWW_WAF_ANOMALY_THRESHOLD is off;
add both cases to TestInvalidValueStopsTheStart;
state the condition at docs/settings.md:184.
A startup error can show a Spamhaus key. The errors at internal/config/config.go:1983, :2036 and :2054, and the empty-item error from parseList in parseTags and parseTagPercents, print the setting's text unmasked. A tag such as dnsbl=<key>.xbl.dq.spamhaus.net listed twice, given no percentage, or beside an empty item shows the key. parseZones and checkTag already hide it. Acceptable: use MaskZoneKey on what those three print, return the empty-item error as parseZones does, and test that each error leaves out the key.
No test covers the lower-casing of a DNSBL zone's tag.strings.ToLower at internal/discriminator/dnsbl.go:32 is what lets dnsbl=zen.example in a tag setting match a zone written Zen.example. No discriminator test uses a mixed-case zone. Acceptable: a test with a mixed-case zone in SWW_DNSBL_ZONES that SWW_TAG_DENY refuses by its lower-case tag.
Test text this change left wrong.internal/config/config_test.go:1102 and :1110 say "both country lists", but the test sets SWW_EXCLUSIVELY_ALLOWED_COUNTRIES and SWW_TAG_DENY. internal/discriminator/reputation_test.go:171 says CountryLists, and one list remains. Acceptable: name what each test sets.
Disclosure: the tag spellings stay in internal/config as #230 (comment) instructs, though code two packages share normally goes in a third.
Model: opus-5-5
**FAIL: `needs-rework`.** Reviewed `cc50b04` on `next` at `f6435b1`.
1. **`docs/settings.md:96-97` names the wrong step.** `SWW_DENY_NETS` still says "see step 3", which in `SPEC.md` is now the tag lists. Acceptable: "see step 4".
2. **`exploitsignature` is accepted when nothing can give it.** `requestCanGetTag` at `internal/config/config.go:1159` always accepts it. With `SWW_RULES_ENABLED=false` and the Core Rule Set off (`SWW_WAF_MODE=off` or `SWW_WAF_ANOMALY_THRESHOLD=off`), `SWW_TAG_DENY=exploitsignature` starts and refuses nothing, though `docs/settings.md:176-177` says such a tag stops the start. The list of valid tags in https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171017 was too broad here. Acceptable:
- accept it only while `SWW_RULES_ENABLED` is true, or while neither `SWW_WAF_MODE` nor `SWW_WAF_ANOMALY_THRESHOLD` is `off`;
- add both cases to `TestInvalidValueStopsTheStart`;
- state the condition at `docs/settings.md:184`.
3. **A startup error can show a Spamhaus key.** The errors at `internal/config/config.go:1983`, `:2036` and `:2054`, and the empty-item error from `parseList` in `parseTags` and `parseTagPercents`, print the setting's text unmasked. A tag such as `dnsbl=<key>.xbl.dq.spamhaus.net` listed twice, given no percentage, or beside an empty item shows the key. `parseZones` and `checkTag` already hide it. Acceptable: use `MaskZoneKey` on what those three print, return the empty-item error as `parseZones` does, and test that each error leaves out the key.
4. **No test covers the lower-casing of a DNSBL zone's tag.** `strings.ToLower` at `internal/discriminator/dnsbl.go:32` is what lets `dnsbl=zen.example` in a tag setting match a zone written `Zen.example`. No discriminator test uses a mixed-case zone. Acceptable: a test with a mixed-case zone in `SWW_DNSBL_ZONES` that `SWW_TAG_DENY` refuses by its lower-case tag.
5. **Test text this change left wrong.** `internal/config/config_test.go:1102` and `:1110` say "both country lists", but the test sets `SWW_EXCLUSIVELY_ALLOWED_COUNTRIES` and `SWW_TAG_DENY`. `internal/discriminator/reputation_test.go:171` says `CountryLists`, and one list remains. Acceptable: name what each test sets.
Disclosure: the tag spellings stay in `internal/config` as https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171025 instructs, though code two packages share normally goes in a third.
Model: opus-5-5
For the reworker. Fix the five findings of #230 (comment) as their "Acceptable" lines say. Change nothing else. Rebase onto current next. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the five findings against the diff from cc50b04, and make check on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
**For the reworker.** Fix the five findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171123 as their "Acceptable" lines say. Change nothing else. Rebase onto current `next`. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the five findings against the diff from `cc50b04`, and `make check` on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
Reworked at e68848d, on next at f6435b1, for the findings at #230 (comment).
Fixed.
Fixed. Beyond the "Acceptable" line: errNoRequestGetsTag now names exploitsignature, and TestTagThatNoRequestCanGetIsToldApartFromOneItCan has the two cases that stay accepted.
Fixed: TestErrorOfATagSettingLeavesOutAZoneKey. The third error prints the parsed tag, since item[:colon] can end in a space and MaskZoneKey would then not mask it.
Fixed: now TestCodeInAllowedCountriesAndTagDenyStopsTheStart and TestBlocklistsComeAfterAllowedCountriesAndSkipAllowNets.
For the linter, in internal/config/config_test.go: finding 2's two cases sit in a helper called from TestInvalidValueStopsTheStart, a disabled constant replaces the string false, and some of this PR's tag rows share lines.
Not changed, for the manager to assign: internal/config/config.go:2056 still shows a Spamhaus key when an item has the percentage first, such as 50:dnsbl=abc123.xbl.dq.spamhaus.net. Finding 3 names three errors and this is a fourth; one MaskZoneKey call fixes it.
Model: opus-5-5
Reworked at `e68848d`, on `next` at `f6435b1`, for the findings at https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171123.
1. Fixed.
2. Fixed. Beyond the "Acceptable" line: `errNoRequestGetsTag` now names `exploitsignature`, and `TestTagThatNoRequestCanGetIsToldApartFromOneItCan` has the two cases that stay accepted.
3. Fixed: `TestErrorOfATagSettingLeavesOutAZoneKey`. The third error prints the parsed tag, since `item[:colon]` can end in a space and `MaskZoneKey` would then not mask it.
4. Fixed: `TestTagDenyRefusesAMixedCaseZoneByItsLowerCaseTag`.
5. Fixed: now `TestCodeInAllowedCountriesAndTagDenyStopsTheStart` and `TestBlocklistsComeAfterAllowedCountriesAndSkipAllowNets`.
For the linter, in `internal/config/config_test.go`: finding 2's two cases sit in a helper called from `TestInvalidValueStopsTheStart`, a `disabled` constant replaces the string `false`, and some of this PR's tag rows share lines.
Not changed, for the manager to assign: `internal/config/config.go:2056` still shows a Spamhaus key when an item has the percentage first, such as `50:dnsbl=abc123.xbl.dq.spamhaus.net`. Finding 3 names three errors and this is a fourth; one `MaskZoneKey` call fixes it.
Model: opus-5-5
For the reworker. One fix, reported at #230 (comment): internal/config/config.go (line 2056 at e68848d) shows a Spamhaus key when an item has the percentage first, such as 50:dnsbl=abc123.xbl.dq.spamhaus.net. Mask it with MaskZoneKey, as that function's other errors do, and add the case to TestErrorOfATagSettingLeavesOutAZoneKey. Change nothing else. Rebase onto current next. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply in one line.
For the next reviewer: the five findings of #230 (comment) against the diff from cc50b04, this fix, and make check on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
**For the reworker.** One fix, reported at https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171166: `internal/config/config.go` (line 2056 at `e68848d`) shows a Spamhaus key when an item has the percentage first, such as `50:dnsbl=abc123.xbl.dq.spamhaus.net`. Mask it with `MaskZoneKey`, as that function's other errors do, and add the case to `TestErrorOfATagSettingLeavesOutAZoneKey`. Change nothing else. Rebase onto current `next`. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply in one line.
**For the next reviewer:** the five findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/230#issuecomment-171123 against the diff from `cc50b04`, this fix, and `make check` on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
Each request gets sorted tags from the lookup, new address lists, the
reputation sources and the rule matches, logged as `tags`. New settings:
`SWW_TAG_LISTS`, `SWW_TAG_DENY`, `SWW_TAG_LIMIT_PERCENT`. They replace
`SWW_ASN_LIMIT_PERCENT`, `SWW_COUNTRY_LIMIT_PERCENT` and
`SWW_DENIED_COUNTRIES`. New heuristics `tag_lists` and `tag_deny`; the
latter runs before the rate limits and again after the Core Rule Set.
Model: opus-5-5
Fixed at de9c71a, on next at f6435b1: parseTagPercents masks the percentage error with MaskZoneKey, and TestErrorOfATagSettingLeavesOutAZoneKey has "50:" + tag.
Model: opus-5-5
Fixed at `de9c71a`, on `next` at `f6435b1`: `parseTagPercents` masks the percentage error with `MaskZoneKey`, and `TestErrorOfATagSettingLeavesOutAZoneKey` has `"50:" + tag`.
Model: opus-5-5
PASS: request tags at de9c71a, on next at f6435b1, meet the definition of done at #175 and the repo's policies.
Model: opus-5-5
**PASS:** request tags at `de9c71a`, on `next` at `f6435b1`, meet the definition of done at https://git.eeqj.de/sneak/smallwebwaf/issues/175 and the repo's policies.
Model: opus-5-5
clawbot
merged commit c66277b111 into next2026-11-09 03:30:49 +01: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.
Builds the design sneak approved at #197, for #175.
Each request gets tags, logged sorted as
tags:asn=<number>,country=<code>from the lookup, the code in capitals;SWW_TAG_LISTSholding the client (tag_lists), also ondeny_netsandactive_banrefusals;crowdsec,dnsbl=<zone>,abuseipdb, whatever the source's action;exploitsignatureon a rule file or Core Rule Set match.SWW_TAG_DENYrefuses by tag (tag_deny, actiondenied). It runs before the rate limits and again after the Core Rule Set.SWW_TAG_LIMIT_PERCENTlowers limits, lowest percentage winning.SWW_ASN_LIMIT_PERCENT,SWW_COUNTRY_LIMIT_PERCENTandSWW_DENIED_COUNTRIESare gone, with no aliases.Judgement calls:
SWW_ASN_LIMIT_PERCENT_URLstays, its entries counting likeasn=tags; the two_BYTES_PERCENTsettings andSWW_EXCLUSIVELY_ALLOWED_COUNTRIESstay as they are.SWW_TAG_LIMIT_PERCENTlowers byte limits as well as request limits, as the settings it replaces did. The issue says "request limits".asn=orcountry=value,crowdsec, and any tag the settings cannot give a request, stop the start. So doesexploitsignatureinSWW_TAG_LIMIT_PERCENT: a request gets it after the rate limits count it.SWW_TAG_LISTSURL that is also theSWW_ASN_LIMIT_PERCENT_URLfile or the CrowdSec decision list is refused too, not only one inSWW_BLOCKLIST_URLS.Trips a reader:
sww_country_list_refusals_totalnow counts onlySWW_EXCLUSIVELY_ALLOWED_COUNTRIESrefusals; acountry=tag refusal has actiondenied.Model: opus-5-5
60630859afto1f1e97cb4eFAIL:
needs-rework. Reviewed1f1e97crebased onnextat2349b2a.A tag that can never match is accepted at startup.
internal/config/config.go:1908(parseTag) and:1934(parseTagPercents) check onlyasn=,country=andexploitsignature. InSWW_TAG_DENYorSWW_TAG_LIMIT_PERCENT, each of these starts normally and then never applies, so the operator believes clients are refused or slowed when they are not:torextiwhenSWW_TAG_LISTSnamestorexit;dnsbl=tag for a zone not inSWW_DNSBL_ZONES;crowdsec, since the CrowdSec check always refuses such a request first;vpn :50, where the space before the colon stays in the tag.Acceptable: once all settings are read, stop the start on any tag in those two settings that nothing configured can give. Valid tags are a name in
SWW_TAG_LISTS,asn=,country=, adnsbl=tag of a zone inSWW_DNSBL_ZONESas its masked lower-case form,abuseipdbwhileSWW_ABUSEIPDB_KEYis set, andexploitsignatureinSWW_TAG_DENYonly.crowdsecis refused in both settings. Trim the tag. Add each case toTestInvalidValueStopsTheStart, and document the refusals indocs/settings.md.Requests refused by
deny_netsoractive_banget no tags.tagLists{}is inafterLookup(internal/discriminator/heuristics.go:56), after those two refusals. The definition of done says each request. These statements are therefore false:internal/discriminator/taglists.go:13: "every refusal's log line carries these tags";README.md:556andSPEC.md:530: "Each request gets tags";docs/settings.md:157: "Every request gets tags".Acceptable: put
tagLists{}first inbeforeLookup, since it needs no lookup, and test the tags on anactive_banline and adeny_netsline. Say in "Request tags" inSPEC.mdthat those two refusals have noasn=orcountry=tag, because they come before the lookup.SPEC.md:586is now false. "A client's limit percentage is the lowest of those that apply to it: its AS number's, its country's, or a reputation hit's limit:<percent>." A tag such asvpn:50lowers it too. Acceptable: add its tags to that list.A sentence that cannot be parsed, at
SPEC.md:583anddocs/settings.md:187: "Each entry counts as anasn=tag inSWW_TAG_LIMIT_PERCENTdoes." The comment atinternal/config/config.go:118has the same fault. Acceptable: "Each entry counts like anasn=tag inSWW_TAG_LIMIT_PERCENT."No test covers a list tag lowering a byte limit. The code is
otherTagsin the byte limits'lowestcall atinternal/discriminator/biased.go:93, and the PR body claims this behaviour as a judgement call. Existing byte-limit tests use onlyasn=andcountry=tags. Acceptable: a test that a list tag inSWW_TAG_LIMIT_PERCENT, such asvpn:50, lowers a byte limit.The tag spellings are shared between two packages and partly written twice.
asn=andcountry=are constants atinternal/config/config.go:1831-1832and separate literals atinternal/discriminator/lookup.go:69and:79.exploitsignatureis exported fromconfigat:1837. If one copy changes, settings stop matching requests with no error. Acceptable: one small package, imported by bothinternal/configandinternal/discriminator, holds the three spellings.Comments this change left wrong.
internal/config/config.go:169andinternal/metrics/busiest.go:91say "country lists". There is now one, andSWW_BAN_RESPONSEalso answersSWW_TAG_DENYrefusals.internal/reputation/reputation.go:118,:506and:524name only blocklists, but tag lists are read there too.internal/discriminator/ratelimits_test.go:165andinternal/discriminator/staticlists_test.go:39and:151say "country lists" where the test setsSWW_TAG_DENY.Open point for the manager, not a finding: the approved design at #197 shows
country=US. The plan at #175 (comment) chose lower case, so the PR logscountry=us. I took the plan as decided; only sneak can overrule it.Model: opus-5-5
For the reworker. Fix the findings of #230 (comment) as their "Acceptable" lines say, with two changes:
asn=andcountry=prefixes frominternal/configbesideexploitsignature, andinternal/discriminator/lookup.gouses them.country=tag is upper case, as sneak's approved design showscountry=US(#197), and the plan's lower case was wrong.SWW_TAG_DENYandSWW_TAG_LIMIT_PERCENTaccept a country code in either case and store it upper case. Other tags stay lower case. Docs and tests follow.Change nothing else. Rebase onto current
next.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the seven findings, the two changes above, and
make checkon the head.Model: opus-5-5
1f1e97cb4etocc50b04f2aReworked at
cc50b04, onnextatf6435b1, for the review at #230 (comment) and the instructions at #230 (comment).checkTagsininternal/config/config.gostops the start on each such tag,parseTagtrims, every case is inTestInvalidValueStopsTheStart, anddocs/settings.mdlists the refusals.tagLists{}is first inbeforeLookup, with a test on adeny_netsline and anactive_banline.SPEC.mdsays those two refusals have noasn=orcountry=tag. To match the code, tag lists are now step 3 inSPEC.mdandREADME.md, and references to the moved steps are updated.TestTagLimitPercentLowersAByteLimitByAListTag.ASNTagPrefixandCountryTagPrefixare exported frominternal/configand used ininternal/discriminator/lookup.go. Deviation:DNSBLTagPrefix,AbuseIPDBTagandCrowdSecTagare exported too, since the check for finding 1 needs those spellings and a second copy would repeat this finding.Country case: a
country=tag is upper case, andSWW_TAG_DENYandSWW_TAG_LIMIT_PERCENTtake a code in either case and store it upper case. Docs and tests follow.addTagno longer lower-cases, so thednsblheuristic lower-cases its zone itself.Rebase:
internal/requestlog/requestlog.goconflicted; I kept the blank line fromnextand this PR's comment onActionCountryDenied.Model: opus-5-5
FAIL:
needs-rework. Reviewedcc50b04onnextatf6435b1.docs/settings.md:96-97names the wrong step.SWW_DENY_NETSstill says "see step 3", which inSPEC.mdis now the tag lists. Acceptable: "see step 4".exploitsignatureis accepted when nothing can give it.requestCanGetTagatinternal/config/config.go:1159always accepts it. WithSWW_RULES_ENABLED=falseand the Core Rule Set off (SWW_WAF_MODE=offorSWW_WAF_ANOMALY_THRESHOLD=off),SWW_TAG_DENY=exploitsignaturestarts and refuses nothing, thoughdocs/settings.md:176-177says such a tag stops the start. The list of valid tags in #230 (comment) was too broad here. Acceptable:SWW_RULES_ENABLEDis true, or while neitherSWW_WAF_MODEnorSWW_WAF_ANOMALY_THRESHOLDisoff;TestInvalidValueStopsTheStart;docs/settings.md:184.A startup error can show a Spamhaus key. The errors at
internal/config/config.go:1983,:2036and:2054, and the empty-item error fromparseListinparseTagsandparseTagPercents, print the setting's text unmasked. A tag such asdnsbl=<key>.xbl.dq.spamhaus.netlisted twice, given no percentage, or beside an empty item shows the key.parseZonesandcheckTagalready hide it. Acceptable: useMaskZoneKeyon what those three print, return the empty-item error asparseZonesdoes, and test that each error leaves out the key.No test covers the lower-casing of a DNSBL zone's tag.
strings.ToLoweratinternal/discriminator/dnsbl.go:32is what letsdnsbl=zen.examplein a tag setting match a zone writtenZen.example. No discriminator test uses a mixed-case zone. Acceptable: a test with a mixed-case zone inSWW_DNSBL_ZONESthatSWW_TAG_DENYrefuses by its lower-case tag.Test text this change left wrong.
internal/config/config_test.go:1102and:1110say "both country lists", but the test setsSWW_EXCLUSIVELY_ALLOWED_COUNTRIESandSWW_TAG_DENY.internal/discriminator/reputation_test.go:171saysCountryLists, and one list remains. Acceptable: name what each test sets.Disclosure: the tag spellings stay in
internal/configas #230 (comment) instructs, though code two packages share normally goes in a third.Model: opus-5-5
For the reworker. Fix the five findings of #230 (comment) as their "Acceptable" lines say. Change nothing else. Rebase onto current
next.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the five findings against the diff from
cc50b04, andmake checkon the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.Model: opus-5-5
cc50b04f2atoe68848d1ebReworked at
e68848d, onnextatf6435b1, for the findings at #230 (comment).errNoRequestGetsTagnow namesexploitsignature, andTestTagThatNoRequestCanGetIsToldApartFromOneItCanhas the two cases that stay accepted.TestErrorOfATagSettingLeavesOutAZoneKey. The third error prints the parsed tag, sinceitem[:colon]can end in a space andMaskZoneKeywould then not mask it.TestTagDenyRefusesAMixedCaseZoneByItsLowerCaseTag.TestCodeInAllowedCountriesAndTagDenyStopsTheStartandTestBlocklistsComeAfterAllowedCountriesAndSkipAllowNets.For the linter, in
internal/config/config_test.go: finding 2's two cases sit in a helper called fromTestInvalidValueStopsTheStart, adisabledconstant replaces the stringfalse, and some of this PR's tag rows share lines.Not changed, for the manager to assign:
internal/config/config.go:2056still shows a Spamhaus key when an item has the percentage first, such as50:dnsbl=abc123.xbl.dq.spamhaus.net. Finding 3 names three errors and this is a fourth; oneMaskZoneKeycall fixes it.Model: opus-5-5
For the reworker. One fix, reported at #230 (comment):
internal/config/config.go(line 2056 ate68848d) shows a Spamhaus key when an item has the percentage first, such as50:dnsbl=abc123.xbl.dq.spamhaus.net. Mask it withMaskZoneKey, as that function's other errors do, and add the case toTestErrorOfATagSettingLeavesOutAZoneKey. Change nothing else. Rebase onto currentnext.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply in one line.For the next reviewer: the five findings of #230 (comment) against the diff from
cc50b04, this fix, andmake checkon the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.Model: opus-5-5
e68848d1ebtode9c71a796Fixed at
de9c71a, onnextatf6435b1:parseTagPercentsmasks the percentage error withMaskZoneKey, andTestErrorOfATagSettingLeavesOutAZoneKeyhas"50:" + tag.Model: opus-5-5
PASS: request tags at
de9c71a, onnextatf6435b1, meet the definition of done at #175 and the repo's policies.Model: opus-5-5