smallwebwaf servewaf is now a command of the binary, for #170. It runs today's checks on each question, answers rproxy, and passes nothing to an app. rproxy reports every request servewaf answered. A report counts an allowed request's bytes, and writes servewaf's second log line for any request. The "servewaf" section of SPEC.md is the exact agreement #171 builds to. smallwebwaf with no command behaves as before.
What a reader would trip over:
request now embeds decision, the part a report needs. The table holds a copy, made after the decision's line is written, and nil while a question is being handled.
A held decision includes its log line, so 10,000 requests with rproxy's largest headers hold up to about 640 MiB, when each request's path and query take up rproxy's 32 KiB, since they are held twice.
NewServeWAF overrides the size and time limits in its copy of the settings.
SWWAF_WAF_BODY_LIMIT is capped at 64 KiB less one byte, so a body cut at 64 KiB is not scored as broken JSON.
Judgement calls:
A question's body is read before any check, since Go's server holds the answer until the body arrives. Over 64 KiB gets 400.
A valid id is 1 to 128 bytes of UTF-8.
A report's bytes stop at 1 PiB, its durations at about 31 years. Unknown fields are ignored.
A third disposition, close, for SWWAF_BAN_RESPONSE=close.
A broken byte limit shows on the report line, not a process message.
Alerts raised by a request gain host, in servewaf only.
Unverified: nothing has run against a real rproxy. The stalled-body test uses a read limit shorter than 60 seconds.
Model: opus-5-5
`smallwebwaf servewaf` is now a command of the binary, for https://git.eeqj.de/sneak/smallwebwaf/issues/170. It runs today's checks on each question, answers rproxy, and passes nothing to an app. rproxy reports every request servewaf answered. A report counts an allowed request's bytes, and writes servewaf's second log line for any request. The "`servewaf`" section of `SPEC.md` is the exact agreement https://git.eeqj.de/sneak/smallwebwaf/issues/171 builds to. `smallwebwaf` with no command behaves as before.
What a reader would trip over:
- `request` now embeds `decision`, the part a report needs. The table holds a copy, made after the decision's line is written, and `nil` while a question is being handled.
- A held decision includes its log line, so 10,000 requests with rproxy's largest headers hold up to about 640 MiB, when each request's path and query take up rproxy's 32 KiB, since they are held twice.
- `NewServeWAF` overrides the size and time limits in its copy of the settings.
- `SWWAF_WAF_BODY_LIMIT` is capped at 64 KiB less one byte, so a body cut at 64 KiB is not scored as broken JSON.
Judgement calls:
- A question's body is read before any check, since Go's server holds the answer until the body arrives. Over 64 KiB gets `400`.
- A valid id is 1 to 128 bytes of UTF-8.
- A report's bytes stop at 1 PiB, its durations at about 31 years. Unknown fields are ignored.
- A third disposition, `close`, for `SWWAF_BAN_RESPONSE=close`.
- A broken byte limit shows on the `report` line, not a process message.
- Alerts raised by a request gain `host`, in servewaf only.
Unverified: nothing has run against a real rproxy. The stalled-body test uses a read limit shorter than 60 seconds.
Model: opus-5-5
The new id on allow is not the simplest correct choice.internal/proxy/servewaf.go:230 says a report cannot come while the handler still uses the request. That holds only for an id servewaf makes, since a visitor's chosen id is known before the answer leaves (:241).
A report naming that id while a new question with the same id is being handled makes finishWith swap rq.out for a writer with no connection (:326). finish then panics: the connection drops, no log line is written, smallwebwaf_requests_in_flight stays raised, and rproxy treats it as no answer.
A duplicate report or any sender on the private network can do this. So can rproxy's own report for a request dropped from the table, after a restart or past 10,000.
That same late report, arriving after the new question's answer, finishes the new request with the old request's bytes. The new request's own report is then ignored, so its bytes are never counted.
Acceptable: a report can never reach a request still being handled, and the comment is true. The plainest way I see is a random id servewaf makes for every allowed request, given only in the answer and named in the report. X-Request-ID then stays the question's, and the three cases at SPEC.md:1084 go.
Report checks do not match the agreement.internal/proxy/servewaf.go:303, SPEC.md:1116.
These get 204, though SPEC.md says 400 for a body that is not such an object: null, {}, an object with fields missing, and an object followed by other data.
An object holding only request_id finishes a held request as status 0 with zero bytes, and its true report is then ignored.
Also accepted: status of -5 or 99999 (the second creates the series status_class="999xx"), negative durations (which lower the duration sums), and byte counts whose sum overflows (which lowers the client's byte counters instead of banning).
Acceptable: 400 for anything but one object with all its fields, status0 or a real HTTP status, durations not negative, and byte counts that cannot overflow. SPEC.md says exactly this, with a test for each.
A refused report leaves no trace.internal/proxy/servewaf.go:304 writes no process message and no metric, and rproxy does not act on the answer. A bug in rproxy's report would stop the byte limits silently. Acceptable: a process warning, as the question's 400 writes at :140.
SPEC.md does not give the rest of a malformed question.SPEC.md:1047-1051, :1080.
Not stated: X-Smallwebwaf-Peer absent or unreadable is answered normally and logs peer_ip as invalid IP. X-Smallwebwaf-Content-Length unreadable or negative counts as absent. A header sent twice uses its first value.
X-Smallwebwaf-Client: 2001:db8::1%eth0 is taken as an address and escapes SWWAF_DENY_NETS (internal/proxy/servewaf.go:138), since a netblock never contains an address with a zone.
Acceptable: SPEC.md gives each case. An address with a zone gets the same 400, or has its zone removed.
A question whose body stops arriving is not described, and its log line is false.internal/proxy/servewaf.go:172, SPEC.md:1146.
With SWWAF_WAF_BODY_LIMIT set, after 60 seconds the log line says status408 and actiontimed_out, and the metrics count a hit of SWWAF_CLIENT_REQUEST_TIMEOUT, which docs/settings.md:12 says servewaf ignores.
Nothing reaches rproxy: the write time limit has already run out, so the connection closes with no answer. SPEC.md lists no 408 and does not say what happens past 60 seconds.
Acceptable: SPEC.md says such a question gets no answer, and the log line does not claim an answer that was never sent.
docs/request-log.md:123 is false. It says "writes one line for each question", but a question refused for its X-Smallwebwaf-Client writes none and is not in the metrics. Acceptable: the document says so.
The table holds each request whole.internal/proxy/servewaf.go:211-220 keeps every visitor header, cookies and Authorization included, and the question's request and response objects. At rproxy's 32 KiB header limit, 10,000 entries hold about 370 MiB, and entries whose reports are lost stay until newer ones push them out. The code lets the body go for this reason but keeps the headers. Acceptable: the table keeps only what the report's steps need, and no credentials.
Tests do not guard what this change adds. The suite still passes with any of these removed:
internal/smallwebwaf/smallwebwaf.go:71, the command itself, since nothing reaches servewaf through Main;
the bound at internal/proxy/servewaf.go:56, and the dropping of the oldest request that SPEC.md:1128 promises;
:217, letting the body go;
:149, Unmap for an IPv4 address in IPv6 form;
:159, an observe-mode refusal counted on allow;
:168, denied requests in the anomaly counters;
:323, clearing LimitHit.
Acceptable: a test for each that fails without it.
Prose in SPEC.md.
:1084: "A visitor behind a trusted proxy can choose its id, so it makes a new one when the question's id is:" reads as the visitor making the id. Name servewaf.
:1022: "It starts as smallwebwaf with no command does." has no subject to refer to, since the paragraph before it is about this section and the settings. Name servewaf.
:1096: "It reads at most 64 KiB less one byte." is false. It reads one byte past that, and request_bytes then logs 65536 (docs/request-log.md:136). Say what it inspects.
Model: opus-5-5
1. **The new id on `allow` is not the simplest correct choice.** `internal/proxy/servewaf.go:230` says a report cannot come while the handler still uses the request. That holds only for an id servewaf makes, since a visitor's chosen id is known before the answer leaves (`:241`).
- A report naming that id while a new question with the same id is being handled makes `finishWith` swap `rq.out` for a writer with no connection (`:326`). `finish` then panics: the connection drops, no log line is written, `smallwebwaf_requests_in_flight` stays raised, and rproxy treats it as no answer.
- A duplicate report or any sender on the private network can do this. So can rproxy's own report for a request dropped from the table, after a restart or past 10,000.
- That same late report, arriving after the new question's answer, finishes the new request with the old request's bytes. The new request's own report is then ignored, so its bytes are never counted.
- Acceptable: a report can never reach a request still being handled, and the comment is true. The plainest way I see is a random id servewaf makes for every allowed request, given only in the answer and named in the report. `X-Request-ID` then stays the question's, and the three cases at `SPEC.md:1084` go.
2. **Report checks do not match the agreement.** `internal/proxy/servewaf.go:303`, `SPEC.md:1116`.
- These get `204`, though `SPEC.md` says `400` for a body that is not such an object: `null`, `{}`, an object with fields missing, and an object followed by other data.
- An object holding only `request_id` finishes a held request as status `0` with zero bytes, and its true report is then ignored.
- Also accepted: `status` of `-5` or `99999` (the second creates the series `status_class="999xx"`), negative durations (which lower the duration sums), and byte counts whose sum overflows (which lowers the client's byte counters instead of banning).
- Acceptable: `400` for anything but one object with all its fields, `status` `0` or a real HTTP status, durations not negative, and byte counts that cannot overflow. `SPEC.md` says exactly this, with a test for each.
3. **A refused report leaves no trace.** `internal/proxy/servewaf.go:304` writes no process message and no metric, and rproxy does not act on the answer. A bug in rproxy's report would stop the byte limits silently. Acceptable: a process warning, as the question's `400` writes at `:140`.
4. **`SPEC.md` does not give the rest of a malformed question.** `SPEC.md:1047-1051`, `:1080`.
- Not stated: `X-Smallwebwaf-Peer` absent or unreadable is answered normally and logs `peer_ip` as `invalid IP`. `X-Smallwebwaf-Content-Length` unreadable or negative counts as absent. A header sent twice uses its first value.
- `X-Smallwebwaf-Client: 2001:db8::1%eth0` is taken as an address and escapes `SWWAF_DENY_NETS` (`internal/proxy/servewaf.go:138`), since a netblock never contains an address with a zone.
- Acceptable: `SPEC.md` gives each case. An address with a zone gets the same `400`, or has its zone removed.
5. **A question whose body stops arriving is not described, and its log line is false.** `internal/proxy/servewaf.go:172`, `SPEC.md:1146`.
- With `SWWAF_WAF_BODY_LIMIT` set, after 60 seconds the log line says `status` `408` and `action` `timed_out`, and the metrics count a hit of `SWWAF_CLIENT_REQUEST_TIMEOUT`, which `docs/settings.md:12` says servewaf ignores.
- Nothing reaches rproxy: the write time limit has already run out, so the connection closes with no answer. `SPEC.md` lists no `408` and does not say what happens past 60 seconds.
- Acceptable: `SPEC.md` says such a question gets no answer, and the log line does not claim an answer that was never sent.
6. **`docs/request-log.md:123` is false.** It says "writes one line for each question", but a question refused for its `X-Smallwebwaf-Client` writes none and is not in the metrics. Acceptable: the document says so.
7. **The table holds each request whole.** `internal/proxy/servewaf.go:211-220` keeps every visitor header, cookies and `Authorization` included, and the question's request and response objects. At rproxy's 32 KiB header limit, 10,000 entries hold about 370 MiB, and entries whose reports are lost stay until newer ones push them out. The code lets the body go for this reason but keeps the headers. Acceptable: the table keeps only what the report's steps need, and no credentials.
8. **Tests do not guard what this change adds.** The suite still passes with any of these removed:
- `internal/smallwebwaf/smallwebwaf.go:71`, the command itself, since nothing reaches `servewaf` through `Main`;
- the bound at `internal/proxy/servewaf.go:56`, and the dropping of the oldest request that `SPEC.md:1128` promises;
- `:217`, letting the body go;
- `:149`, `Unmap` for an IPv4 address in IPv6 form;
- `:159`, an observe-mode refusal counted on `allow`;
- `:168`, denied requests in the anomaly counters;
- `:323`, clearing `LimitHit`.
Acceptable: a test for each that fails without it.
9. **Prose in `SPEC.md`.**
- `:1084`: "A visitor behind a trusted proxy can choose its id, so it makes a new one when the question's id is:" reads as the visitor making the id. Name `servewaf`.
- `:1022`: "It starts as `smallwebwaf` with no command does." has no subject to refer to, since the paragraph before it is about this section and the settings. Name `servewaf`.
- `:1096`: "It reads at most 64 KiB less one byte." is false. It reads one byte past that, and `request_bytes` then logs 65536 (`docs/request-log.md:136`). Say what it inspects.
Model: opus-5-5
Request id: servewaf takes X-Request-ID as given and makes none. A question without a valid id, or with one it holds, gets 400. allow carries no id, and the three cases in SPEC.md are gone.
Logging: a report writes a second servewaf line with the decision's fields and the report's. servewaf_line is decision or report, documented in docs/request-log.md.
Reports: every answered request is held and its report gets the line. Only an allowed request is counted from its report.
Query strings: nothing redacted them, so no code changed. A test now holds them verbatim on both lines.
Findings:
Settled by the rulings and a copy of the decision, held after its line is written. A report for an id not held is ignored.
Done, with a test for each. Ceilings of 1 PiB and about 31 years keep sums and durations from overflowing.
Done.
Done. An address with a zone gets 400.
Done. The body is read before any check, so such a question gets no answer and no request log line, only a process warning.
Done.
Settled by finding 1's record.
Tests added for the command through Main, the bound and dropping, Unmap, the observe-mode refusal, and denied requests in the anomaly counters. Letting the body go and clearing LimitHit no longer exist: the record holds no body, and the third line replaced the process message.
Done, all three.
Model: opus-5-5
Rulings:
- Request id: `servewaf` takes `X-Request-ID` as given and makes none. A question without a valid id, or with one it holds, gets `400`. `allow` carries no id, and the three cases in `SPEC.md` are gone.
- Logging: a report writes a second `servewaf` line with the decision's fields and the report's. `servewaf_line` is `decision` or `report`, documented in `docs/request-log.md`.
- Reports: every answered request is held and its report gets the line. Only an allowed request is counted from its report.
- Query strings: nothing redacted them, so no code changed. A test now holds them verbatim on both lines.
Findings:
1. Settled by the rulings and a copy of the decision, held after its line is written. A report for an id not held is ignored.
2. Done, with a test for each. Ceilings of 1 PiB and about 31 years keep sums and durations from overflowing.
3. Done.
4. Done. An address with a zone gets `400`.
5. Done. The body is read before any check, so such a question gets no answer and no request log line, only a process warning.
6. Done.
7. Settled by finding 1's record.
8. Tests added for the command through `Main`, the bound and dropping, `Unmap`, the observe-mode refusal, and denied requests in the anomaly counters. Letting the body go and clearing `LimitHit` no longer exist: the record holds no body, and the third line replaced the process message.
9. Done, all three.
Model: opus-5-5
SPEC.md:1196 is false about the 60 seconds. It says "gives each request 60 seconds to arrive and its answer 60 seconds to leave". The answer has no 60 seconds of its own: its limit counts from the end of the request's headers, so the time the body and the checks take comes off it. Acceptable: a correct short sentence, such as "A request and its answer have 60 seconds together, from the request's headers."
SPEC.md:5-6 is no longer true. It says "Everything in it is built, except the last section". The servewaf section (:1013, :1041, :1121) says what rproxy does in the present tense, and no rproxy is in the tree. Acceptable: "Everything in it is built, except what the last section lists", since that section now lists rproxy.
SPEC.md:1041-1042 cannot be parsed on first reading. "rproxy removes these headers if the visitor sent them, then sets them. It adds no other header:" puts the colon after "no other header", so the list reads as the headers it does not add. Acceptable: "rproxy sets these headers, replacing any the visitor sent, and adds no others:".
servewaf's exceptions sit in the openings of documents about the one-process mode.docs/settings.md:7 is the first section, ahead of every setting. docs/metrics.md:7 opens the document, and docs/admin-endpoints.md:5 is in its opening paragraph. An exception does not go in a document's opening or before its main rule. Acceptable: a servewaf section at the end of each, as docs/request-log.md does.
Tests do not guard these. The suite still passes with all of them removed:
internal/proxy/servewaf.go:131-133, the server's read, write and idle limits. Without the read limit a stalled question holds its connection for good, and TestQuestionWhoseBodyStopsArrivingGetsNoAnswer sets its own.
:187, a denied question counted toward the error burst in enforce mode.
:270-271, removing X-Smallwebwaf-Peer and X-Smallwebwaf-Content-Length before the checks.
:64, the 1 MiB limit on a report. The test's oversized body also lacks fields, so it is refused anyway.
:110 and :114, ignoring SWWAF_CLIENT_REQUEST_TIMEOUT and SWWAF_CLIENT_RESPONSE_TIMEOUT. TestSettingsOfTheLimitsAndTheProxiesAreIgnored sets both to off, not to a short value.
internal/proxy/coreruleset.go:116 and internal/proxy/reputation.go:113, host on the waf_block and reputation_hit alerts.
Acceptable: a test for each that fails without it.
Model: opus-5-5
1. **`SPEC.md:1196` is false about the 60 seconds.** It says "gives each request 60 seconds to arrive and its answer 60 seconds to leave". The answer has no 60 seconds of its own: its limit counts from the end of the request's headers, so the time the body and the checks take comes off it. Acceptable: a correct short sentence, such as "A request and its answer have 60 seconds together, from the request's headers."
2. **`SPEC.md:5-6` is no longer true.** It says "Everything in it is built, except the last section". The `servewaf` section (`:1013`, `:1041`, `:1121`) says what `rproxy` does in the present tense, and no `rproxy` is in the tree. Acceptable: "Everything in it is built, except what the last section lists", since that section now lists `rproxy`.
3. **`SPEC.md:1041-1042` cannot be parsed on first reading.** "`rproxy` removes these headers if the visitor sent them, then sets them. It adds no other header:" puts the colon after "no other header", so the list reads as the headers it does not add. Acceptable: "`rproxy` sets these headers, replacing any the visitor sent, and adds no others:".
4. **`servewaf`'s exceptions sit in the openings of documents about the one-process mode.** `docs/settings.md:7` is the first section, ahead of every setting. `docs/metrics.md:7` opens the document, and `docs/admin-endpoints.md:5` is in its opening paragraph. An exception does not go in a document's opening or before its main rule. Acceptable: a `servewaf` section at the end of each, as `docs/request-log.md` does.
5. **Tests do not guard these.** The suite still passes with all of them removed:
- `internal/proxy/servewaf.go:131-133`, the server's read, write and idle limits. Without the read limit a stalled question holds its connection for good, and `TestQuestionWhoseBodyStopsArrivingGetsNoAnswer` sets its own.
- `:187`, a denied question counted toward the error burst in `enforce` mode.
- `:270-271`, removing `X-Smallwebwaf-Peer` and `X-Smallwebwaf-Content-Length` before the checks.
- `:64`, the 1 MiB limit on a report. The test's oversized body also lacks fields, so it is refused anyway.
- `:110` and `:114`, ignoring `SWWAF_CLIENT_REQUEST_TIMEOUT` and `SWWAF_CLIENT_RESPONSE_TIMEOUT`. `TestSettingsOfTheLimitsAndTheProxiesAreIgnored` sets both to `off`, not to a short value.
- `internal/proxy/coreruleset.go:116` and `internal/proxy/reputation.go:113`, `host` on the `waf_block` and `reputation_hit` alerts.
Acceptable: a test for each that fails without it.
Model: opus-5-5
SPEC.md now says a request and its answer have 60 seconds together, from the request's headers.
The opening says "except what the last section lists", and that section lists rproxy in one line, without its pointer to the servewaf section.
Reworded as the review gave it.
Moved to a servewaf section at the end of all three documents. docs/admin-endpoints.md now says "No check runs on those requests", since "those checks" had nothing to refer to there.
A test for each, and each fails with its code removed. The 1 MiB limit has no code to remove, so I raised it to check that one.
Disclosures:
Partially verified: the read, write and idle limits are tested by reading the server's values, not by waiting out 60 seconds.
Judgement call: without its line, the SWWAF_CLIENT_REQUEST_TIMEOUT test fails only because Go's server sees the short read deadline pass while the Core Rule Set reads the body, and the log line then says aborted. That depends on timing, and it failed every run I tried. With the line in place the test cannot fail this way.
Model: opus-5-5
1. `SPEC.md` now says a request and its answer have 60 seconds together, from the request's headers.
2. The opening says "except what the last section lists", and that section lists `rproxy` in one line, without its pointer to the `servewaf` section.
3. Reworded as the review gave it.
4. Moved to a `servewaf` section at the end of all three documents. `docs/admin-endpoints.md` now says "No check runs on those requests", since "those checks" had nothing to refer to there.
5. A test for each, and each fails with its code removed. The 1 MiB limit has no code to remove, so I raised it to check that one.
Disclosures:
- Partially verified: the read, write and idle limits are tested by reading the server's values, not by waiting out 60 seconds.
- Judgement call: without its line, the `SWWAF_CLIENT_REQUEST_TIMEOUT` test fails only because Go's server sees the short read deadline pass while the Core Rule Set reads the body, and the log line then says `aborted`. That depends on timing, and it failed every run I tried. With the line in place the test cannot fail this way.
Model: opus-5-5
docs/request-log.md:25, :156 and :166 are false for a question.duration_total says "from the question's headers being read", and time says "when the request arrived". internal/proxy/servewaf.go:161 starts the clock only after readQuestion (:153) has read the whole body. A question with a slow body logs a late time, and a duration_total without that wait. Acceptable: take the time as the handler starts, or short correct sentences, such as "from the question's body having been read to the line".
No test guards the request duration a report gives.TestMetricsCountAnAllowedRequestFromItsReport (internal/proxy/servewaf_test.go:930) checks smallwebwaf_upstream_duration_seconds_sum but not smallwebwaf_request_duration_seconds. internal/proxy/servewaf.go:459 takes that metric from the report's duration_total, and docs/metrics.md:86 promises it. Acceptable: the test also checks that metric's _sum.
SPEC.md:1201 has two unclear sentences. "Nothing routes the internet there." states as a fact what the deployment has to ensure. "These are:" has nothing to refer to. Acceptable: "The internet must not be able to reach that address. Its endpoints are:".
cmd/smallwebwaf/main.go:1-3 no longer says what the binary does. It says the binary runs between traefik and the app and passes requests through, which smallwebwaf servewaf does not do. Acceptable: a sentence for servewaf, as the doc of internal/smallwebwaf now has.
The PR body's memory figure is too low. "about 320 MiB" holds only when a header takes up rproxy's 32 KiB. When the URL does, the path and query are held twice, in the log line and in requestURI (internal/proxy/request.go:155), which comes to about 640 MiB. Acceptable: the PR body gives the larger figure.
Model: opus-5-5
1. **`docs/request-log.md:25`, `:156` and `:166` are false for a question.** `duration_total` says "from the question's headers being read", and `time` says "when the request arrived". `internal/proxy/servewaf.go:161` starts the clock only after `readQuestion` (`:153`) has read the whole body. A question with a slow body logs a late `time`, and a `duration_total` without that wait. Acceptable: take the time as the handler starts, or short correct sentences, such as "from the question's body having been read to the line".
2. **No test guards the request duration a report gives.** `TestMetricsCountAnAllowedRequestFromItsReport` (`internal/proxy/servewaf_test.go:930`) checks `smallwebwaf_upstream_duration_seconds_sum` but not `smallwebwaf_request_duration_seconds`. `internal/proxy/servewaf.go:459` takes that metric from the report's `duration_total`, and `docs/metrics.md:86` promises it. Acceptable: the test also checks that metric's `_sum`.
3. **`SPEC.md:1201` has two unclear sentences.** "Nothing routes the internet there." states as a fact what the deployment has to ensure. "These are:" has nothing to refer to. Acceptable: "The internet must not be able to reach that address. Its endpoints are:".
4. **`cmd/smallwebwaf/main.go:1-3` no longer says what the binary does.** It says the binary runs between traefik and the app and passes requests through, which `smallwebwaf servewaf` does not do. Acceptable: a sentence for `servewaf`, as the doc of `internal/smallwebwaf` now has.
5. **The PR body's memory figure is too low.** "about 320 MiB" holds only when a header takes up `rproxy`'s 32 KiB. When the URL does, the path and query are held twice, in the log line and in `requestURI` (`internal/proxy/request.go:155`), which comes to about 640 MiB. Acceptable: the PR body gives the larger figure.
Model: opus-5-5
Third failed review. Instructions that end the loop:
The rework changes exactly this, nothing else:
docs/request-log.md line 156: duration_total becomes "from servewaf having read the question's body to the line." Right after it, add "- time: when servewaf had read the question's body." Lines 166-167: "and time stays when the request arrived" becomes "and time stays as the decision line gives it". No code change: rproxy sends the question's body from memory, so the wait is short, and the documents now say what the code does.
internal/proxy/servewaf_test.go, TestMetricsCountAnAllowedRequestFromItsReport (line 930): also check that smallwebwaf_request_duration_seconds_sum equals the report's duration_total.
SPEC.md line 1201: "Nothing routes the internet there. These are:" becomes "The internet must not be able to reach that address. Its endpoints are:".
cmd/smallwebwaf/main.go lines 1-3: "Command smallwebwaf is a web application firewall for one app: it runs between traefik and the app and passes requests through within its limits, or, as smallwebwaf servewaf, answers rproxy's questions about them. Everything it does is in internal/smallwebwaf."
The PR body, line 6: "about 320 MiB" becomes "up to about 640 MiB, when each request's path and query take up rproxy's 32 KiB, since they are held twice".
Then make fmt, one commit amended, rebased onto the current next.
It leaves alone every other line of code, test and document. The three reviews' other findings are fixed; the design is sneak's.
The next review checks only: that the change since e2b214a is the five items above, any rewrapping make fmt did, and the rebase; that each item is correct; that the findings of the three reviews stay fixed; and make check on the tree rebased onto the current next. A finding on any other line stands only if it shows a bug (lost, wrong or double counting, a crash, a request reaching the checks it should not, a change to smallwebwaf with no command) or a sentence that is false. Wording the repository's documents do not settle is not a finding.
Model: opus-5-5
Third failed review. Instructions that end the loop:
**The rework changes exactly this**, nothing else:
1. `docs/request-log.md` line 156: `duration_total` becomes "from `servewaf` having read the question's body to the line." Right after it, add "- `time`: when `servewaf` had read the question's body." Lines 166-167: "and `time` stays when the request arrived" becomes "and `time` stays as the `decision` line gives it". No code change: rproxy sends the question's body from memory, so the wait is short, and the documents now say what the code does.
2. `internal/proxy/servewaf_test.go`, `TestMetricsCountAnAllowedRequestFromItsReport` (line 930): also check that `smallwebwaf_request_duration_seconds_sum` equals the report's `duration_total`.
3. `SPEC.md` line 1201: "Nothing routes the internet there. These are:" becomes "The internet must not be able to reach that address. Its endpoints are:".
4. `cmd/smallwebwaf/main.go` lines 1-3: "Command smallwebwaf is a web application firewall for one app: it runs between traefik and the app and passes requests through within its limits, or, as `smallwebwaf servewaf`, answers rproxy's questions about them. Everything it does is in internal/smallwebwaf."
5. The PR body, line 6: "about 320 MiB" becomes "up to about 640 MiB, when each request's path and query take up rproxy's 32 KiB, since they are held twice".
Then `make fmt`, one commit amended, rebased onto the current `next`.
**It leaves alone** every other line of code, test and document. The three reviews' other findings are fixed; the design is sneak's.
**The next review checks only**: that the change since `e2b214a` is the five items above, any rewrapping `make fmt` did, and the rebase; that each item is correct; that the findings of the three reviews stay fixed; and `make check` on the tree rebased onto the current `next`. A finding on any other line stands only if it shows a bug (lost, wrong or double counting, a crash, a request reaching the checks it should not, a change to `smallwebwaf` with no command) or a sentence that is false. Wording the repository's documents do not settle is not a finding.
Model: opus-5-5
`smallwebwaf servewaf` runs today's checks on the visitor's request each
question carries, answers rproxy, and passes nothing to an app. rproxy reports
every request servewaf answered. A report counts an allowed request's bytes and
writes servewaf's second log line. The table holds a copy of what a report
needs, never the request, so a report cannot reach a request being handled.
Judgement call: a question's body is read before any check; over 64 KiB gets 400.
Judgement call: a report's bytes and durations have ceilings, so they cannot overflow.
Judgement call: a `close` disposition for SWWAF_BAN_RESPONSE=close.
Model: opus-5-5
Check added. Judgement call: a finished scrape's own time also goes into that sum, so the check uses a second server in the same test, not yet scraped.
SPEC.md changed as given.
cmd/smallwebwaf/main.go changed as given.
PR body changed as given.
Model: opus-5-5
1. `docs/request-log.md` changed as given.
2. Check added. Judgement call: a finished scrape's own time also goes into that sum, so the check uses a second server in the same test, not yet scraped.
3. `SPEC.md` changed as given.
4. `cmd/smallwebwaf/main.go` changed as given.
5. PR body changed as given.
Model: opus-5-5
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.
smallwebwaf servewafis now a command of the binary, for #170. It runs today's checks on each question, answers rproxy, and passes nothing to an app. rproxy reports every request servewaf answered. A report counts an allowed request's bytes, and writes servewaf's second log line for any request. The "servewaf" section ofSPEC.mdis the exact agreement #171 builds to.smallwebwafwith no command behaves as before.What a reader would trip over:
requestnow embedsdecision, the part a report needs. The table holds a copy, made after the decision's line is written, andnilwhile a question is being handled.NewServeWAFoverrides the size and time limits in its copy of the settings.SWWAF_WAF_BODY_LIMITis capped at 64 KiB less one byte, so a body cut at 64 KiB is not scored as broken JSON.Judgement calls:
400.close, forSWWAF_BAN_RESPONSE=close.reportline, not a process message.host, in servewaf only.Unverified: nothing has run against a real rproxy. The stalled-body test uses a read limit shorter than 60 seconds.
Model: opus-5-5
The new id on
allowis not the simplest correct choice.internal/proxy/servewaf.go:230says a report cannot come while the handler still uses the request. That holds only for an id servewaf makes, since a visitor's chosen id is known before the answer leaves (:241).finishWithswaprq.outfor a writer with no connection (:326).finishthen panics: the connection drops, no log line is written,smallwebwaf_requests_in_flightstays raised, and rproxy treats it as no answer.X-Request-IDthen stays the question's, and the three cases atSPEC.md:1084go.Report checks do not match the agreement.
internal/proxy/servewaf.go:303,SPEC.md:1116.204, thoughSPEC.mdsays400for a body that is not such an object:null,{}, an object with fields missing, and an object followed by other data.request_idfinishes a held request as status0with zero bytes, and its true report is then ignored.statusof-5or99999(the second creates the seriesstatus_class="999xx"), negative durations (which lower the duration sums), and byte counts whose sum overflows (which lowers the client's byte counters instead of banning).400for anything but one object with all its fields,status0or a real HTTP status, durations not negative, and byte counts that cannot overflow.SPEC.mdsays exactly this, with a test for each.A refused report leaves no trace.
internal/proxy/servewaf.go:304writes no process message and no metric, and rproxy does not act on the answer. A bug in rproxy's report would stop the byte limits silently. Acceptable: a process warning, as the question's400writes at:140.SPEC.mddoes not give the rest of a malformed question.SPEC.md:1047-1051,:1080.X-Smallwebwaf-Peerabsent or unreadable is answered normally and logspeer_ipasinvalid IP.X-Smallwebwaf-Content-Lengthunreadable or negative counts as absent. A header sent twice uses its first value.X-Smallwebwaf-Client: 2001:db8::1%eth0is taken as an address and escapesSWWAF_DENY_NETS(internal/proxy/servewaf.go:138), since a netblock never contains an address with a zone.SPEC.mdgives each case. An address with a zone gets the same400, or has its zone removed.A question whose body stops arriving is not described, and its log line is false.
internal/proxy/servewaf.go:172,SPEC.md:1146.SWWAF_WAF_BODY_LIMITset, after 60 seconds the log line saysstatus408andactiontimed_out, and the metrics count a hit ofSWWAF_CLIENT_REQUEST_TIMEOUT, whichdocs/settings.md:12says servewaf ignores.SPEC.mdlists no408and does not say what happens past 60 seconds.SPEC.mdsays such a question gets no answer, and the log line does not claim an answer that was never sent.docs/request-log.md:123is false. It says "writes one line for each question", but a question refused for itsX-Smallwebwaf-Clientwrites none and is not in the metrics. Acceptable: the document says so.The table holds each request whole.
internal/proxy/servewaf.go:211-220keeps every visitor header, cookies andAuthorizationincluded, and the question's request and response objects. At rproxy's 32 KiB header limit, 10,000 entries hold about 370 MiB, and entries whose reports are lost stay until newer ones push them out. The code lets the body go for this reason but keeps the headers. Acceptable: the table keeps only what the report's steps need, and no credentials.Tests do not guard what this change adds. The suite still passes with any of these removed:
internal/smallwebwaf/smallwebwaf.go:71, the command itself, since nothing reachesservewafthroughMain;internal/proxy/servewaf.go:56, and the dropping of the oldest request thatSPEC.md:1128promises;:217, letting the body go;:149,Unmapfor an IPv4 address in IPv6 form;:159, an observe-mode refusal counted onallow;:168, denied requests in the anomaly counters;:323, clearingLimitHit.Acceptable: a test for each that fails without it.
Prose in
SPEC.md.:1084: "A visitor behind a trusted proxy can choose its id, so it makes a new one when the question's id is:" reads as the visitor making the id. Nameservewaf.:1022: "It starts assmallwebwafwith no command does." has no subject to refer to, since the paragraph before it is about this section and the settings. Nameservewaf.:1096: "It reads at most 64 KiB less one byte." is false. It reads one byte past that, andrequest_bytesthen logs 65536 (docs/request-log.md:136). Say what it inspects.Model: opus-5-5
ed6ba53747to4a2d57dca2Rulings:
servewaftakesX-Request-IDas given and makes none. A question without a valid id, or with one it holds, gets400.allowcarries no id, and the three cases inSPEC.mdare gone.servewafline with the decision's fields and the report's.servewaf_lineisdecisionorreport, documented indocs/request-log.md.Findings:
400.Main, the bound and dropping,Unmap, the observe-mode refusal, and denied requests in the anomaly counters. Letting the body go and clearingLimitHitno longer exist: the record holds no body, and the third line replaced the process message.Model: opus-5-5
SPEC.md:1196is false about the 60 seconds. It says "gives each request 60 seconds to arrive and its answer 60 seconds to leave". The answer has no 60 seconds of its own: its limit counts from the end of the request's headers, so the time the body and the checks take comes off it. Acceptable: a correct short sentence, such as "A request and its answer have 60 seconds together, from the request's headers."SPEC.md:5-6is no longer true. It says "Everything in it is built, except the last section". Theservewafsection (:1013,:1041,:1121) says whatrproxydoes in the present tense, and norproxyis in the tree. Acceptable: "Everything in it is built, except what the last section lists", since that section now listsrproxy.SPEC.md:1041-1042cannot be parsed on first reading. "rproxyremoves these headers if the visitor sent them, then sets them. It adds no other header:" puts the colon after "no other header", so the list reads as the headers it does not add. Acceptable: "rproxysets these headers, replacing any the visitor sent, and adds no others:".servewaf's exceptions sit in the openings of documents about the one-process mode.docs/settings.md:7is the first section, ahead of every setting.docs/metrics.md:7opens the document, anddocs/admin-endpoints.md:5is in its opening paragraph. An exception does not go in a document's opening or before its main rule. Acceptable: aservewafsection at the end of each, asdocs/request-log.mddoes.Tests do not guard these. The suite still passes with all of them removed:
internal/proxy/servewaf.go:131-133, the server's read, write and idle limits. Without the read limit a stalled question holds its connection for good, andTestQuestionWhoseBodyStopsArrivingGetsNoAnswersets its own.:187, a denied question counted toward the error burst inenforcemode.:270-271, removingX-Smallwebwaf-PeerandX-Smallwebwaf-Content-Lengthbefore the checks.:64, the 1 MiB limit on a report. The test's oversized body also lacks fields, so it is refused anyway.:110and:114, ignoringSWWAF_CLIENT_REQUEST_TIMEOUTandSWWAF_CLIENT_RESPONSE_TIMEOUT.TestSettingsOfTheLimitsAndTheProxiesAreIgnoredsets both tooff, not to a short value.internal/proxy/coreruleset.go:116andinternal/proxy/reputation.go:113,hoston thewaf_blockandreputation_hitalerts.Acceptable: a test for each that fails without it.
Model: opus-5-5
4a2d57dca2toe2b214a0f5SPEC.mdnow says a request and its answer have 60 seconds together, from the request's headers.rproxyin one line, without its pointer to theservewafsection.servewafsection at the end of all three documents.docs/admin-endpoints.mdnow says "No check runs on those requests", since "those checks" had nothing to refer to there.Disclosures:
SWWAF_CLIENT_REQUEST_TIMEOUTtest fails only because Go's server sees the short read deadline pass while the Core Rule Set reads the body, and the log line then saysaborted. That depends on timing, and it failed every run I tried. With the line in place the test cannot fail this way.Model: opus-5-5
docs/request-log.md:25,:156and:166are false for a question.duration_totalsays "from the question's headers being read", andtimesays "when the request arrived".internal/proxy/servewaf.go:161starts the clock only afterreadQuestion(:153) has read the whole body. A question with a slow body logs a latetime, and aduration_totalwithout that wait. Acceptable: take the time as the handler starts, or short correct sentences, such as "from the question's body having been read to the line".No test guards the request duration a report gives.
TestMetricsCountAnAllowedRequestFromItsReport(internal/proxy/servewaf_test.go:930) checkssmallwebwaf_upstream_duration_seconds_sumbut notsmallwebwaf_request_duration_seconds.internal/proxy/servewaf.go:459takes that metric from the report'sduration_total, anddocs/metrics.md:86promises it. Acceptable: the test also checks that metric's_sum.SPEC.md:1201has two unclear sentences. "Nothing routes the internet there." states as a fact what the deployment has to ensure. "These are:" has nothing to refer to. Acceptable: "The internet must not be able to reach that address. Its endpoints are:".cmd/smallwebwaf/main.go:1-3no longer says what the binary does. It says the binary runs between traefik and the app and passes requests through, whichsmallwebwaf servewafdoes not do. Acceptable: a sentence forservewaf, as the doc ofinternal/smallwebwafnow has.The PR body's memory figure is too low. "about 320 MiB" holds only when a header takes up
rproxy's 32 KiB. When the URL does, the path and query are held twice, in the log line and inrequestURI(internal/proxy/request.go:155), which comes to about 640 MiB. Acceptable: the PR body gives the larger figure.Model: opus-5-5
Third failed review. Instructions that end the loop:
The rework changes exactly this, nothing else:
docs/request-log.mdline 156:duration_totalbecomes "fromservewafhaving read the question's body to the line." Right after it, add "-time: whenservewafhad read the question's body." Lines 166-167: "andtimestays when the request arrived" becomes "andtimestays as thedecisionline gives it". No code change: rproxy sends the question's body from memory, so the wait is short, and the documents now say what the code does.internal/proxy/servewaf_test.go,TestMetricsCountAnAllowedRequestFromItsReport(line 930): also check thatsmallwebwaf_request_duration_seconds_sumequals the report'sduration_total.SPEC.mdline 1201: "Nothing routes the internet there. These are:" becomes "The internet must not be able to reach that address. Its endpoints are:".cmd/smallwebwaf/main.golines 1-3: "Command smallwebwaf is a web application firewall for one app: it runs between traefik and the app and passes requests through within its limits, or, assmallwebwaf servewaf, answers rproxy's questions about them. Everything it does is in internal/smallwebwaf."Then
make fmt, one commit amended, rebased onto the currentnext.It leaves alone every other line of code, test and document. The three reviews' other findings are fixed; the design is sneak's.
The next review checks only: that the change since
e2b214ais the five items above, any rewrappingmake fmtdid, and the rebase; that each item is correct; that the findings of the three reviews stay fixed; andmake checkon the tree rebased onto the currentnext. A finding on any other line stands only if it shows a bug (lost, wrong or double counting, a crash, a request reaching the checks it should not, a change tosmallwebwafwith no command) or a sentence that is false. Wording the repository's documents do not settle is not a finding.Model: opus-5-5
e2b214a0f5to872172e958docs/request-log.mdchanged as given.SPEC.mdchanged as given.cmd/smallwebwaf/main.gochanged as given.Model: opus-5-5
The review passed.
Model: opus-5-5