Leave SWWAF_RATE_LIMIT_EXEMPT_PATHS out of the request rate limits (closes #77)

A request is neither counted nor refused by the request rate limits
when the path the app will act on starts with one of the
comma-separated prefixes in SWWAF_RATE_LIMIT_EXEMPT_PATHS. That path is
the request's path percent-decoded, with its . and .. segments and
repeated slashes resolved, so /assets/..%2Flogin is /login and is
counted. The static lists, bans and the country lists still apply. The
setting is empty by default, and a prefix that does not start with /
stops the start. README.md documents it.

Model: opus-5-5

README.md

@@ -81,14 +81,13 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set.
   takes the client over one of the rate limits below is refused with
   `SWWAF_BAN_RESPONSE`, `403` by default, before anything reaches the app, and
   bans the client. A request whose path starts with one of
-  `SWWAF_RATE_LIMIT_EXEMPT_PATHS`, as that setting below describes, is neither
-  counted nor refused by the rate limits; the static lists, bans and the country
-  lists still apply to it. A client is one IPv4 address, or one IPv6 /64, since
-  one abuser usually holds a whole /64. Each window is counted in two fixed
-  buckets, the earlier one weighted by how much of it the window still covers.
-  At most 20,000 clients are kept, the least recently seen dropped first, with
-  their history, and a restart gives no client a fresh allowance (see "State
-  files" below).
+  `SWWAF_RATE_LIMIT_EXEMPT_PATHS` is neither counted nor refused by the rate
+  limits; the static lists, bans and the country lists still apply to it. A
+  client is one IPv4 address, or one IPv6 /64, since one abuser usually holds a
+  whole /64. Each window is counted in two fixed buckets, the earlier one
+  weighted by how much of it the window still covers. At most 20,000 clients are
+  kept, the least recently seen dropped first, with their history, and a restart
+  gives no client a fresh allowance (see "State files" below).
 - Bans a client that breaks a rate limit, as "Bans" in [`SPEC.md`](SPEC.md)
   describes: the first ban lasts an hour, and a limit broken again within a day
   of a ban ending bans for three times as long as that ban, so 1, 3, 9, 27 and
@@ -197,14 +196,14 @@ it, and the effective settings are logged at start.
   page makes a few hundred requests and several people often share one address.
 - `SWWAF_RATE_LIMIT_EXEMPT_PATHS` (default empty): path prefixes whose requests
   the rate limits neither count nor refuse, such as `/assets/` for static
-  assets; each starts with `/`. A request whose path, percent-decoded, contains
-  `..` anywhere or a backslash, or whose path as sent holds an encoded slash
-  (`%2F` or `%2f`), is never exempt, since the app may act on it as a path
-  outside every prefix: `/assets/..%2Flogin` as `/login`. Any other request is
-  exempt when its path, percent-decoded and before any query string, starts with
-  a prefix, character for character. `/assets/` matches `/assets/app.js` and
-  `/assets/`, but not `/assets`, `/Assets/app.js`, `/static/assets/app.js`,
-  `/static/../assets/app.js` or `/assets%2Fapp.js`. A prefix is written without
+  assets; each starts with `/`. A prefix is compared, character for character,
+  with the start of the path the app will act on: the request's path, before any
+  query string, percent-decoded, with its `.` and `..` segments and repeated
+  slashes resolved and without a trailing slash, which is not always what the
+  request log's `path` shows. `/assets/` matches `/assets/app.js`,
+  `/assets//img/logo.png` and `/static/../assets/app.js`, but not `/assets/`
+  itself, `/assets`, `/Assets/app.js`, `/static/assets/app.js` or
+  `/assets/..%2Flogin`, which is `/login`. A prefix is written without
   percent-encoding, and there are no wildcards: `*` is a character like any
   other.
 - `SWWAF_DENIED_COUNTRIES` (default empty): countries whose clients are refused,
@@ -752,9 +751,9 @@ so that they run in minimal containers.
 ## TODO
 
 - The rest of milestone 3: taking in an admin's edits to the state files
-  (https://git.eeqj.de/sneak/smallwebwaf/issues/68) and the rest of the request
-  log's fields; then the rest of the design, in the order of the build order in
-  [`SPEC.md`](SPEC.md).
+  (https://git.eeqj.de/sneak/smallwebwaf/issues/68), exemptions and the rest of
+  the request log's fields; then the rest of the design, in the order of the
+  build order in [`SPEC.md`](SPEC.md).
 
 ## Documents
 

internal/proxy/ratelimits_test.go

@@ -92,9 +92,12 @@ func TestRateLimitExemptPathsAreNeitherCountedNorRefused(t *testing.T) {
 	// With a limit of one request a minute, the requests for paths under a
 	// prefix are not counted, so client's first request for / is within
 	// the limit; and once client has reached it, they are not refused.
+	// The app acts on /static/../assets/app.js as /assets/app.js.
 	s.request(client, "/assets/app.js", http.StatusOK, requestlog.ActionForward)
 	s.request(client, "/favicon.ico?v=2", http.StatusOK, requestlog.ActionForward)
 	s.get(client, http.StatusOK, requestlog.ActionForward)
+	s.request(client, "/static/../assets/app.js",
+		http.StatusOK, requestlog.ActionForward)
 
 	line := s.request(client, "/assets/app.js", http.StatusOK, requestlog.ActionForward)
 	if line.LimitHit != "" {
@@ -113,24 +116,17 @@ func TestRateLimitExemptPathsAreNeitherCountedNorRefused(t *testing.T) {
 		http.StatusForbidden, requestlog.ActionCountryDenied)
 }
 
-func TestRateLimitCountsPathsThatAreNotExempt(t *testing.T) {
+func TestRateLimitCountsPathsOutsideEveryExemptPrefix(t *testing.T) {
 	t.Parallel()
 
+	// A prefix matches only at the start of the path, and the app acts on
+	// the last three paths as /login, once they are percent-decoded and
+	// their .. segments resolved.
 	for _, sent := range []string{
-		// A prefix matches only at the start of the path.
 		"/static/assets/app.js",
-		// .. once percent-decoded: an app may act on these as /login, the
-		// last as a path under /sneak/app/ or as /assets/x.
 		"/assets/../login",
 		"/assets/%2e%2e/login",
 		"/assets/..%2Flogin",
-		"/assets/..;/login",
-		"/sneak/app/src/branch/main/..%2F..%2F..%2F..%2F..%2F..%2Fassets/x",
-		// An encoded slash or a backslash: Go's router takes /assets%2Fx
-		// for one path segment, not a path under /assets/.
-		"/assets%2Fx",
-		"/assets%2fx",
-		`/assets/x\y`,
 	} {
 		t.Run(sent, func(t *testing.T) {
 			t.Parallel()

internal/proxy/request.go

@@ -7,8 +7,8 @@ import (
 	"net/http/httptrace"
 	"net/http/httputil"
 	"net/netip"
-	"net/url"
 	"os"
+	"path"
 	"slices"
 	"strings"
 	"sync"
@@ -148,9 +148,9 @@ func (rq *request) check(ctx context.Context) *refusal {
 // client either refuses is not looked up, and then the country lists; a
 // request any of them refuses is not counted for the rate limits. Then
 // come the rate limits, unless the client is in
-// SWWAF_RATE_LIMIT_EXEMPT_NETS or the request's path is exempt under
-// SWWAF_RATE_LIMIT_EXEMPT_PATHS, so that every other request is counted.
-// ctx is the request's own context.
+// SWWAF_RATE_LIMIT_EXEMPT_NETS or the path the app will act on starts
+// with one of SWWAF_RATE_LIMIT_EXEMPT_PATHS, so that every other request
+// is counted. ctx is the request's own context.
 func (rq *request) checkClient(ctx context.Context) string {
 	cfg := rq.h.config
 	if isInside(rq.client, cfg.AllowNets) {
@@ -171,8 +171,13 @@ func (rq *request) checkClient(ctx context.Context) string {
 		return requestlog.ActionCountryDenied
 	}
 
+	// The prefixes are matched against the path the app will act on:
+	// URL.Path is the request's path percent-decoded, and path.Clean
+	// resolves its . and .. segments and repeated slashes, and drops a
+	// trailing slash. So /assets/..%2Flogin is /login, outside /assets/,
+	// whatever the log line's path shows.
 	exempt := isInside(rq.client, cfg.RateLimitExemptNets) ||
-		pathExempt(rq.in.URL, cfg.RateLimitExemptPaths)
+		startsWithAny(path.Clean(rq.in.URL.Path), cfg.RateLimitExemptPaths)
 	if !exempt && rq.limitBroken(now) {
 		return requestlog.ActionRateLimited
 	}
@@ -180,25 +185,10 @@ func (rq *request) checkClient(ctx context.Context) string {
 	return ""
 }
 
-// pathExempt reports whether the rate limits leave out a request for u
-// because of SWWAF_RATE_LIMIT_EXEMPT_PATHS: whether its path,
-// percent-decoded, starts with one of prefixes. A request whose decoded
-// path contains .. anywhere or a backslash, or whose path as sent holds
-// an encoded slash (%2F or %2f), never is, since an app may act on it as
-// a path outside every prefix: /assets/..%2Flogin as /login, or
-// /assets%2Fx as one path segment, as Go's router does.
-func pathExempt(u *url.URL, prefixes []string) bool {
-	decoded := u.Path
-	// EscapedPath is the path as the app receives it, not decoded.
-	sent := strings.ToLower(u.EscapedPath())
-
-	if strings.Contains(decoded, "..") || strings.Contains(decoded, `\`) ||
-		strings.Contains(sent, "%2f") {
-		return false
-	}
-
+// startsWithAny reports whether s starts with one of prefixes.
+func startsWithAny(s string, prefixes []string) bool {
 	return slices.ContainsFunc(prefixes, func(prefix string) bool {
-		return strings.HasPrefix(decoded, prefix)
+		return strings.HasPrefix(s, prefix)
 	})
 }