feat(deny-list): per-service deny rules with --deny-ip and --deny-user-agent - #93
Merged
Conversation
…r-agent ## Summary Static deny rules for operators fronting public tenant fleets, where an allow list is definitionally impossible and a rate limit cannot express "this network gets nothing" (#88). Rules are evaluated inline in serviceRequestWithTarget as the very first gate -- deny, then allow, then TLS redirect, rate limit, basic auth -- with the client resolved through the shared forwardedResolver. Denials answer 403 through the error-page machinery, are counted in kamal_proxy_denials_total by rule kind, and persist in ServiceOptions like every other knob. User-Agent rules are RE2, compiled once at deploy, anchored to the full header value, and checked only after the IP rules pass. A missing User-Agent only matches an explicit '^$' pattern. ## Test Coverage - deny_list_test.go: parsing/validation, IP matching (exact IPv6, no /64 collapse, IPv4-mapped normalization), anchored UA matching, empty-UA semantics, trusted-proxy justification, health-check-path guard - deny_list_service_test.go: 403 ordering vs allow/redirect/rate-limit/ basic-auth, health-check + internal exemptions, forwarded-chain resolution, denial metrics by kind, old-state-file safety, JSON round-trip, fail-closed on unreadable stored rules, redeploy removal ## Verification - [x] gofmt -l internal/ cmd/ clean - [x] make test passes - [x] go vet ./... and make lint (golangci-lint) clean - [x] go test -race on the request-gate paths clean
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.
Summary
internal/server/deny_list.go: per-servicedenyListover the sharedforwardedResolver—--deny-ip(addresses/CIDRs, parsed with the sameparseIPPrefixesas the allow list) and--deny-user-agent(RE2, compiled once at deploy, anchored to the full header value)serviceRequestWithTarget: deny → allow → TLS redirect → rate limit → basic auth. A denied client never spends rate-limit budget and never sees an auth challenge; an address on both deny and allow lists is denied^$pattern/is rejected" deploy guard as allow-ip/rate-limitkamal_proxy_denials_total{service,rule}counter (ip,user_agent)ServiceOptions(deny_ips,deny_user_agents, bothomitempty) — old state files load unrestricted; unreadable stored rules fail closed--trusted-proxyis now justified bydeny-ipalone (previously required allow-ip or rate-limit); README section addedCloses #88
Test plan
deny_list_test.go— parsing/validation, IP matching (exact IPv6, IPv4-mapped normalization, zero-addr), anchored UA matching, empty-UA semantics, health-check guarddeny_list_service_test.go— 403 ordering vs every other gate, exemptions, forwarded-chain resolution, metrics by kind, state-file safety, JSON round-trip, fail-closed, redeploy removalmake test,go vet,golangci-lint,gofmt -lall clean;go test -raceon the gate paths cleanDeviations & judgment calls
Deviations
upstreamremote exists in this clone; skipped the Phase-0 ff-sync ofmainand branchedfeature/deny-listsdirectly offorigin/dash(current and clean).Discoveries
--deny-user-agentmust beStringArrayVar, notStringSliceVarlike--allow-ip: RE2 patterns legally contain commas ({1,3}), and StringSlice would split them into broken rules. Same reasoning as the header-rule flags.TestIdleController_PersistsOnlySleepAndWakeEdgesfailed once during the first fullmake test, passed 5× in isolation and on a full-package rerun — pre-existing timing flake, untouched by this change.Judgment calls
resolveIPAllowList: a block that silently lapsed would serve the very traffic the operator asked to refuse. Only corrupt state can hit this path, since deploys validate.\A(?:p)\z) — the issue says "matched against the full User-Agent" and itsBadBot/.*example only makes sense anchored.^$pattern — even.*, which technically matches the empty string, does not deny it ("absence is not a crime").unreadable, emitted only on the fail-closed path, so that state is visible in Prometheus rather than only in a log line.validateAllowIPs' trusted-proxy justification check extended to acceptdeny-ipas a third legitimate reason (deny resolves clients through the same resolver).