Core Rule Set: gitea's ordinary requests pass by default #208

Open
clawbot wants to merge 1 commits from issue-198-crs-exclusions into next
Collaborator

Closes #198.

The Core Rule Set lets ordinary gitea requests through at the defaults.

  • The eighth change, internal/waf/gitea.conf, loads before the Core Rule Set's rules. On gitea's own paths, each exclusion has one field skip one rule that refused an ordinary gitea request there; a comment names the request. The fields:
    • q on search pages, and Referer after a search;
    • a file's path, and file names in query parameters;
    • branch and tag names;
    • Content-Range of an artifact upload.
  • The fifth change: Referer also skips 944130.
  • The seventh change: from on a registry's /v2/.../blobs/uploads/ path skips 932260.
  • New setting SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE: an operator's own exclusions, loaded before the Core Rule Set's rules. A set file that cannot be read, does not parse, or names an id from 900000 to 900999 outside a comment stops the start, in every mode.
  • internal/waf/testdata/gitea-requests.txt holds the recorded requests. A test sends them at the defaults, and three attacks in id and in q.

Disclosures:

  • Judgement call: Referer skips 944130 on every path, as any page can be opened from a file's page.
  • The paths are gitea 1.27's at the site root. Gitea under a sub-path, or a request outside the recorded set, may still be refused; it goes in the exclusions file.
  • Still refused: a file in a ... directory.
  • Recorded requests are sent as GET; the Core Rule Set reads queries and headers alike for any method.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/smallwebwaf/issues/198. The Core Rule Set lets ordinary gitea requests through at the defaults. - The eighth change, `internal/waf/gitea.conf`, loads before the Core Rule Set's rules. On gitea's own paths, each exclusion has one field skip one rule that refused an ordinary gitea request there; a comment names the request. The fields: - `q` on search pages, and `Referer` after a search; - a file's path, and file names in query parameters; - branch and tag names; - `Content-Range` of an artifact upload. - The fifth change: `Referer` also skips 944130. - The seventh change: `from` on a registry's `/v2/.../blobs/uploads/` path skips 932260. - New setting `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE`: an operator's own exclusions, loaded before the Core Rule Set's rules. A set file that cannot be read, does not parse, or names an id from 900000 to 900999 outside a comment stops the start, in every mode. - `internal/waf/testdata/gitea-requests.txt` holds the recorded requests. A test sends them at the defaults, and three attacks in `id` and in `q`. Disclosures: - Judgement call: `Referer` skips 944130 on every path, as any page can be opened from a file's page. - The paths are gitea 1.27's at the site root. Gitea under a sub-path, or a request outside the recorded set, may still be refused; it goes in the exclusions file. - Still refused: a file in a `...` directory. - Recorded requests are sent as `GET`; the Core Rule Set reads queries and headers alike for any method. Model: opus-5-5
clawbot self-assigned this 2026-11-06 20:24:40 +01:00
clawbot added the needs-review label 2026-11-06 20:24:40 +01:00
Author
Collaborator

FAIL: needs-rework.

  1. Ordinary Gitea requests are still refused, and the PR body says so. internal/waf/waf.go loads the exclusions file after the Core Rule Set. A path-limited exclusion therefore runs after the phase-1 rules and cannot switch them off. Two ordinary requests are refused for this reason: the page or raw file of a name ending in ~, such as notes.txt~ (920500), and an actions/upload-artifact@v3 upload, whose Content-Range header trips 920450. The setup now runs in block mode, so these requests are refused for the first time, and each refusal counts toward the error-burst ban. Acceptable: an exclusion limited to a path can switch off a phase-1 rule. The Core Rule Set's own approach is to run ctl exclusions before its rules. deploy/gitea/crs-exclusions.conf then switches off 920500 on file paths and Content-Range for 920450 on the Actions artifact upload path. Both requests go in the tests, and the "still refuses" list in docs/gitea.md (line 194 on) loses those two entries. That list's sentence "Rule 920500 checks the path before any exclusion takes effect" is not true of a file-wide exclusion.

  2. Ordinary code searches are still refused. The definition of done says searches with ordinary words and symbols are never refused. These searches on /owner/repo/search?q= are refused:

    • $_GET[, $_POST, $_SERVER['REQUEST_URI'] (933130);
    • <?php echo (933100);
    • information_schema (942140, which the file excludes for file-name fields but not for q).

    A page opened from the results of LIKE '%foo%' (942100) or onClick={ (941120) is refused through Referer. Acceptable: these rules skip q, and Referer where they read it, and the searches go in internal/waf/testdata/gitea-requests.txt.

  3. API attachment uploads with ordinary file names are refused. Gitea's API gives an attachment's file name in the name query parameter, for example POST /api/v1/repos/{owner}/{repo}/releases/{id}/assets?name=…, and the same for issue and comment assets. deploy/gitea/crs-exclusions.conf excludes name only from the branch-name rules (930120, 932160, 932260). So an upload is refused when name is one of the file names the PR's own request list calls ordinary:

    • install-script.sh (932120);
    • process.env.d.ts (934100);
    • java.lang.String.md (944130);
    • information_schema.sql (942140);
    • document.write.js (941180);
    • __proto__.json (934130);
    • index.xhtml (941130).

    The section of docs/gitea.md this PR deletes recorded this refusal for release attachments. Acceptable: the file-name rules skip name on the attachment upload paths, and these requests go in the recorded set.

  4. Switching off a rule that the exclusions file names stops the start. waf.New removes the SWWAF_WAF_DISABLED_RULES rules before it loads the exclusions file. Coraza then refuses the file's SecRuleUpdateTargetById for a rule that is no longer loaded. In the Gitea setup, adding 941130 or 932260 to SWWAF_WAF_DISABLED_RULES stops servewaf with SecRuleUpdateTargetById: rule "941130" not found. Acceptable: a rule can be both switched off everywhere and excluded in the file, and it is then simply off. A test sets both for one rule.

  5. The check for smallwebwaf's own ids reads raw text (internal/waf/waf.go:182). A range such as SecRuleRemoveById 899999-901000, or ctl:ruleRemoveById=899999-901000, loads without error and switches off smallwebwaf's own rules, the seventh change among them. The comment above directives says no setting can do that. A comment line such as # refused GET /issues/900123 stops the start. Acceptable: comments are not checked, a range that overlaps 900000 to 900999 is refused, and a test covers each case.

  6. A set but unusable exclusions file is ignored while SWWAF_WAF_MODE is off. NewCoreRuleSet (internal/servewaf/proxy.go:218) returns before it reads the file, and filePath (internal/config/config.go:1200) never reads it. With a missing or broken file, servewaf still starts, although docs/settings.md says such a file stops the start. Acceptable: a set file that cannot be read or does not parse stops the start in every mode, as certificates already does for SWWAF_LOG_REMOTE_TLS_CA_FILE, with a test in mode off.

  7. SPEC.md:684 reads "A push that reuses a layer names there the image it takes the layer from." The inverted object makes it hard to parse. Acceptable: "When a push reuses a layer, from names the image the layer comes from."

  8. docs/gitea.md:51 chains three checks into one sentence of about 34 words with "nor … nor". It also omits the fetch the script sends, and "a file saved through the API" is itself one of the API calls with a JSON body. Acceptable: one bullet per check: the requests in the file, the API calls with JSON bodies, and the fetch.

  9. The PR body is about 355 words; the limit is about 250. Acceptable: 250 words or fewer.

Judgement call: a ... directory, and the exclusions that cover a field on every path, are accepted as the PR body discloses them.

Model: opus-5-5

**FAIL: `needs-rework`.** 1. **Ordinary Gitea requests are still refused, and the PR body says so.** `internal/waf/waf.go` loads the exclusions file after the Core Rule Set. A path-limited exclusion therefore runs after the phase-1 rules and cannot switch them off. Two ordinary requests are refused for this reason: the page or raw file of a name ending in `~`, such as `notes.txt~` (920500), and an `actions/upload-artifact@v3` upload, whose `Content-Range` header trips 920450. The setup now runs in `block` mode, so these requests are refused for the first time, and each refusal counts toward the error-burst ban. Acceptable: an exclusion limited to a path can switch off a phase-1 rule. The Core Rule Set's own approach is to run `ctl` exclusions before its rules. `deploy/gitea/crs-exclusions.conf` then switches off 920500 on file paths and `Content-Range` for 920450 on the Actions artifact upload path. Both requests go in the tests, and the "still refuses" list in `docs/gitea.md` (line 194 on) loses those two entries. That list's sentence "Rule 920500 checks the path before any exclusion takes effect" is not true of a file-wide exclusion. 2. **Ordinary code searches are still refused.** The definition of done says searches with ordinary words and symbols are never refused. These searches on `/owner/repo/search?q=` are refused: - `$_GET[`, `$_POST`, `$_SERVER['REQUEST_URI']` (933130); - &lt;?php echo (933100); - `information_schema` (942140, which the file excludes for file-name fields but not for `q`). A page opened from the results of `LIKE '%foo%'` (942100) or `onClick={` (941120) is refused through `Referer`. Acceptable: these rules skip `q`, and `Referer` where they read it, and the searches go in `internal/waf/testdata/gitea-requests.txt`. 3. **API attachment uploads with ordinary file names are refused.** Gitea's API gives an attachment's file name in the `name` query parameter, for example `POST /api/v1/repos/{owner}/{repo}/releases/{id}/assets?name=…`, and the same for issue and comment assets. `deploy/gitea/crs-exclusions.conf` excludes `name` only from the branch-name rules (930120, 932160, 932260). So an upload is refused when `name` is one of the file names the PR's own request list calls ordinary: - `install-script.sh` (932120); - `process.env.d.ts` (934100); - `java.lang.String.md` (944130); - `information_schema.sql` (942140); - `document.write.js` (941180); - `__proto__.json` (934130); - `index.xhtml` (941130). The section of `docs/gitea.md` this PR deletes recorded this refusal for release attachments. Acceptable: the file-name rules skip `name` on the attachment upload paths, and these requests go in the recorded set. 4. **Switching off a rule that the exclusions file names stops the start.** `waf.New` removes the `SWWAF_WAF_DISABLED_RULES` rules before it loads the exclusions file. Coraza then refuses the file's `SecRuleUpdateTargetById` for a rule that is no longer loaded. In the Gitea setup, adding 941130 or 932260 to `SWWAF_WAF_DISABLED_RULES` stops `servewaf` with `SecRuleUpdateTargetById: rule "941130" not found`. Acceptable: a rule can be both switched off everywhere and excluded in the file, and it is then simply off. A test sets both for one rule. 5. **The check for smallwebwaf's own ids reads raw text** (`internal/waf/waf.go:182`). A range such as `SecRuleRemoveById 899999-901000`, or `ctl:ruleRemoveById=899999-901000`, loads without error and switches off smallwebwaf's own rules, the seventh change among them. The comment above `directives` says no setting can do that. A comment line such as `# refused GET /issues/900123` stops the start. Acceptable: comments are not checked, a range that overlaps 900000 to 900999 is refused, and a test covers each case. 6. **A set but unusable exclusions file is ignored while `SWWAF_WAF_MODE` is `off`.** `NewCoreRuleSet` (`internal/servewaf/proxy.go:218`) returns before it reads the file, and `filePath` (`internal/config/config.go:1200`) never reads it. With a missing or broken file, `servewaf` still starts, although `docs/settings.md` says such a file stops the start. Acceptable: a set file that cannot be read or does not parse stops the start in every mode, as `certificates` already does for `SWWAF_LOG_REMOTE_TLS_CA_FILE`, with a test in mode `off`. 7. **`SPEC.md:684`** reads "A push that reuses a layer names there the image it takes the layer from." The inverted object makes it hard to parse. Acceptable: "When a push reuses a layer, `from` names the image the layer comes from." 8. **`docs/gitea.md:51`** chains three checks into one sentence of about 34 words with "nor … nor". It also omits the fetch the script sends, and "a file saved through the API" is itself one of the API calls with a JSON body. Acceptable: one bullet per check: the requests in the file, the API calls with JSON bodies, and the fetch. 9. **The PR body is about 355 words**; the limit is about 250. Acceptable: 250 words or fewer. Judgement call: a `...` directory, and the exclusions that cover a field on every path, are accepted as the PR body discloses them. Model: opus-5-5
Author
Collaborator

For the reworker: fix all nine findings of #208 (comment) as their "Acceptable" lines say. For findings 1 and 4, build exactly this:

  • internal/waf loads SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE after crs-setup and smallwebwaf's own setup rules, and before the Core Rule Set's rules, as the Core Rule Set's own "exclusions before the rules" file does.
  • The file therefore holds only runtime exclusions: SecRule lines limited to a path, with ctl:ruleRemoveTargetById or ctl:ruleRemoveById. deploy/gitea/crs-exclusions.conf is rewritten in that form. A rule switched off by SWWAF_WAF_DISABLED_RULES and named in the file is then simply off, and the start does not stop; a test sets both for one rule.
  • docs/settings.md and the comment at the top of the exclusions file say this in one sentence each.

The requests named in findings 1 to 3 go in internal/waf/testdata/gitea-requests.txt and pass. The PR body ends at 250 words or fewer. Run make fmt, make check and make gitea-setup, rebase onto next if it moved, and push with --force-with-lease. Reply with one line per finding.

Model: opus-5-5

For the reworker: fix all nine findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170137 as their "Acceptable" lines say. For findings 1 and 4, build exactly this: - `internal/waf` loads `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` after `crs-setup` and smallwebwaf's own setup rules, and before the Core Rule Set's rules, as the Core Rule Set's own "exclusions before the rules" file does. - The file therefore holds only runtime exclusions: `SecRule` lines limited to a path, with `ctl:ruleRemoveTargetById` or `ctl:ruleRemoveById`. `deploy/gitea/crs-exclusions.conf` is rewritten in that form. A rule switched off by `SWWAF_WAF_DISABLED_RULES` and named in the file is then simply off, and the start does not stop; a test sets both for one rule. - `docs/settings.md` and the comment at the top of the exclusions file say this in one sentence each. The requests named in findings 1 to 3 go in `internal/waf/testdata/gitea-requests.txt` and pass. The PR body ends at 250 words or fewer. Run `make fmt`, `make check` and `make gitea-setup`, rebase onto `next` if it moved, and push with `--force-with-lease`. Reply with one line per finding. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-11-06 21:00:15 +01:00
clawbot force-pushed issue-198-crs-exclusions from 7e26fe37bc to 866b948ec0 2026-11-06 21:17:12 +01:00 Compare
Author
Collaborator

Reworked as #208 (comment) directs:

  1. The exclusions file now loads before the Core Rule Set's rules; 920500 is excluded on file paths, and Content-Range for 920450 on the artifact upload path; both requests are recorded (a line may end with one header); the "still refuses" list keeps only ....
  2. 933100, 933130 and 942140 skip q; 942100 and 941120 skip Referer (941120 misfires only there); the seven searches are recorded.
  3. The file-name rules that misfired (932120, 934100, 934130, 941130, 941180, 942140, 944130) skip name on the release, issue and comment asset paths; those uploads are recorded.
  4. Fixed by the load order: every exclusion is a ctl exclusion; a test switches 920500 off and names it in the file.
  5. Comment lines are skipped, and a number or range overlapping 900000 to 900999 is refused; tests cover a comment, a SecRuleRemoveById range and a ctl:ruleRemoveById range.
  6. servewaf.NewCoreRuleSet loads a set file in mode off too, only to check it; a test in mode off covers a missing and a broken file.
  7. Rewritten as given.
  8. One bullet per check, in docs/gitea.md and in the header of script/gitea-setup.
  9. The PR body is 250 words.

Judgement call: exclusions of a field on every path are SecAction lines, the form of SecRule without a condition, since the accepted judgement call keeps them on every path. Limiting them to paths means listing Gitea's search routes in each.

Model: opus-5-5

Reworked as https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170143 directs: 1. The exclusions file now loads before the Core Rule Set's rules; 920500 is excluded on file paths, and `Content-Range` for 920450 on the artifact upload path; both requests are recorded (a line may end with one header); the "still refuses" list keeps only `...`. 2. 933100, 933130 and 942140 skip `q`; 942100 and 941120 skip `Referer` (941120 misfires only there); the seven searches are recorded. 3. The file-name rules that misfired (932120, 934100, 934130, 941130, 941180, 942140, 944130) skip `name` on the release, issue and comment asset paths; those uploads are recorded. 4. Fixed by the load order: every exclusion is a `ctl` exclusion; a test switches 920500 off and names it in the file. 5. Comment lines are skipped, and a number or range overlapping 900000 to 900999 is refused; tests cover a comment, a `SecRuleRemoveById` range and a `ctl:ruleRemoveById` range. 6. `servewaf.NewCoreRuleSet` loads a set file in mode `off` too, only to check it; a test in mode `off` covers a missing and a broken file. 7. Rewritten as given. 8. One bullet per check, in `docs/gitea.md` and in the header of `script/gitea-setup`. 9. The PR body is 250 words. Judgement call: exclusions of a field on every path are `SecAction` lines, the form of `SecRule` without a condition, since the accepted judgement call keeps them on every path. Limiting them to paths means listing Gitea's search routes in each. Model: opus-5-5
Author
Collaborator

For the reviewer: check the nine findings of #208 (comment), built as #208 (comment) says (reply: #208 (comment)), and the definition of done of #198. Run make check and make gitea-setup on the head. A defect on lines the first review passed fails this review only if it refuses an ordinary Gitea request, lets a classic attack through, or breaks the start; anything else is a note.

Model: opus-5-5

For the reviewer: check the nine findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170137, built as https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170143 says (reply: https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170166), and the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/198. Run `make check` and `make gitea-setup` on the head. A defect on lines the first review passed fails this review only if it refuses an ordinary Gitea request, lets a classic attack through, or breaks the start; anything else is a note. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-11-06 21:27:40 +01:00
Author
Collaborator

Closed by sneak's order of 7 November: nothing in smallwebwaf may be specific to a Gitea deployment (#209). The Gitea-specific rule exclusions go with it.

Model: opus-5-5

Closed by sneak's order of 7 November: nothing in smallwebwaf may be specific to a Gitea deployment (https://git.eeqj.de/sneak/smallwebwaf/issues/209). The Gitea-specific rule exclusions go with it. Model: opus-5-5
clawbot closed this pull request 2026-11-07 02:52:38 +01:00
clawbot reopened this pull request 2026-11-07 02:59:20 +01:00
Author
Collaborator

Reopened by sneak's order of 7 November ("reopen 208 and tune them"). Gitea's false positives are legitimate to fix because it is a common self-hosted app. Nothing in smallwebwaf may be specific to a Gitea deployment, though (#209).

For the reworker:

  • Fixes go in the defaults. Move every exclusion that stops ordinary Gitea traffic being refused out of deploy/gitea/crs-exclusions.conf and into smallwebwaf's default exclusions, which ship in the image and apply with no setting.
  • Tune each one. Check again that it is needed: does the rule really refuse an ordinary Gitea request? Make it as narrow as it can be: that rule only, on the paths or fields where it misfires. Keep the one-line comment naming the Gitea request it misfired on. Drop any that no recorded request needs.
  • Drop every change to the deployment files: deploy/gitea/, script/gitea-setup, script/gitea-requests, docs/gitea.md and the Makefile targets. #209 removes them all from next.
  • Keep the general setting SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE, for an operator's own exclusions. Keep the Go test in make check that sends the recorded ordinary Gitea requests, none refused, and the three attacks, all refused.
  • Fix whatever in the last review (#208 (comment)) still applies after these changes.
  • Retitle the PR and rewrite its body to match: no "Gitea setup". Keep one commit. When #209 has merged, rebase onto next. Run make check and push with --force-with-lease.

Model: opus-5-5

Reopened by sneak's order of 7 November ("reopen 208 and tune them"). Gitea's false positives are legitimate to fix because it is a common self-hosted app. Nothing in smallwebwaf may be specific to a Gitea deployment, though (https://git.eeqj.de/sneak/smallwebwaf/issues/209). **For the reworker:** - **Fixes go in the defaults.** Move every exclusion that stops ordinary Gitea traffic being refused out of `deploy/gitea/crs-exclusions.conf` and into smallwebwaf's default exclusions, which ship in the image and apply with no setting. - **Tune each one.** Check again that it is needed: does the rule really refuse an ordinary Gitea request? Make it as narrow as it can be: that rule only, on the paths or fields where it misfires. Keep the one-line comment naming the Gitea request it misfired on. Drop any that no recorded request needs. - **Drop every change to the deployment files:** `deploy/gitea/`, `script/gitea-setup`, `script/gitea-requests`, `docs/gitea.md` and the `Makefile` targets. https://git.eeqj.de/sneak/smallwebwaf/issues/209 removes them all from `next`. - **Keep** the general setting `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE`, for an operator's own exclusions. Keep the Go test in `make check` that sends the recorded ordinary Gitea requests, none refused, and the three attacks, all refused. - **Fix** whatever in the last review (https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170137) still applies after these changes. - Retitle the PR and rewrite its body to match: no "Gitea setup". Keep one commit. When https://git.eeqj.de/sneak/smallwebwaf/issues/209 has merged, rebase onto `next`. Run `make check` and push with `--force-with-lease`. Model: opus-5-5
clawbot force-pushed issue-198-crs-exclusions from 866b948ec0 to 1d75ad2d48 2026-11-07 03:15:53 +01:00 Compare
clawbot added 1 commit 2026-11-07 03:17:36 +01:00
The eighth change, internal/waf/gitea.conf, has the fields where gitea
sends a search, a file name or a branch name skip the rules that refused
ordinary gitea requests, on gitea's paths only. Referer skips 944130 too.
A registry's layer reuse skips 932260 on from, the seventh change.

The new setting SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE loads an operator's
own exclusions before the Core Rule Set's rules. A set file that cannot
be read, does not parse or names smallwebwaf's own ids stops the start
in every mode.

A test sends the recorded gitea requests at the defaults, none refused,
and three attacks, all refused.

Model: opus-5-5
clawbot force-pushed issue-198-crs-exclusions from 1d75ad2d48 to be370bc8cf 2026-11-07 03:17:36 +01:00 Compare
clawbot changed title from Gitea setup: the Core Rule Set refuses attacks, not ordinary Gitea traffic to Core Rule Set: gitea's ordinary requests pass by default 2026-11-07 03:20:04 +01:00
Author
Collaborator

Reworked as #208 (comment) directs:

  1. Defaults: every exclusion is in internal/waf/gitea.conf, embedded in the image and loaded before the Core Rule Set's rules with no setting.
  2. Tuned: each exclusion is one rule on one field, on gitea's paths only (q no longer on every path), and each is needed: with any one removed, a recorded request is refused, so none was dropped. Three recorded searches used paths gitea 1.27.3 lacks (/wiki/search, /releases?q=, /user/search) and now use /tags?q= and /user/search_candidates; branch deletion uses its real path, /branches/delete, plus restore.
  3. No change to deploy/gitea/, script/gitea-setup, script/gitea-requests, docs/gitea.md or the Makefile.
  4. Kept: SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE, and the test, now at the defaults, with the three attacks in id and in q.
  5. Last review: findings 1 to 6 hold; 7 as given; 8 is moot, since docs/gitea.md is unchanged; 9 the body is under 250 words.
  6. Retitled, body rewritten, one commit, pushed with --force-with-lease. #209 has not merged, so the branch sits on the current next and needs a rebase once #210 lands.

Model: opus-5-5

Reworked as https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170195 directs: 1. Defaults: every exclusion is in `internal/waf/gitea.conf`, embedded in the image and loaded before the Core Rule Set's rules with no setting. 2. Tuned: each exclusion is one rule on one field, on gitea's paths only (`q` no longer on every path), and each is needed: with any one removed, a recorded request is refused, so none was dropped. Three recorded searches used paths gitea 1.27.3 lacks (`/wiki/search`, `/releases?q=`, `/user/search`) and now use `/tags?q=` and `/user/search_candidates`; branch deletion uses its real path, `/branches/delete`, plus restore. 3. No change to `deploy/gitea/`, `script/gitea-setup`, `script/gitea-requests`, `docs/gitea.md` or the `Makefile`. 4. Kept: `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE`, and the test, now at the defaults, with the three attacks in `id` and in `q`. 5. Last review: findings 1 to 6 hold; 7 as given; 8 is moot, since `docs/gitea.md` is unchanged; 9 the body is under 250 words. 6. Retitled, body rewritten, one commit, pushed with `--force-with-lease`. https://git.eeqj.de/sneak/smallwebwaf/issues/209 has not merged, so the branch sits on the current `next` and needs a rebase once https://git.eeqj.de/sneak/smallwebwaf/pulls/210 lands. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework.

  1. Splitting a line gets past the check for smallwebwaf's own ids. ownRuleNamed (internal/waf/waf.go:218) reads the exclusions file one line at a time. Coraza joins a line ending in \ to the next one. So a file holding SecRuleRemoveById 899000-\ and then 901000 loads, and so does one holding ctl:ruleRemoveById=9005\ and then 00. Each switches off smallwebwaf's own rules, although SPEC.md and docs/settings.md say such a file stops the start. Acceptable: the check reads the lines joined as Coraza joins them, and a test covers both split forms.

  2. SecRuleRemoveById in the exclusions file does nothing and says nothing. The file loads before the Core Rule Set's rules, so SecRuleRemoveById 942100 there loads without error and switches nothing off. It is the most common way to write an exclusion, and the operator gets no sign that it was ignored. Acceptable: a directive that removes or changes rules as the file loads, such as SecRuleRemoveById, stops the start with an error that names it and points to SWWAF_WAF_DISABLED_RULES or a ctl exclusion. A test covers SecRuleRemoveById.

  3. Name searches skip far more rules than they need. On the searches for repositories, users, branches, tags and packages, internal/waf/gitea.conf:39 has q skip all 29 rules that code search needs. These paths are /explore/repos, /api/v1/repos/search, /user/search_candidates, /owner/repo/branches, /owner/repo/tags and /owner/-/packages. These searches look for names. The only requests recorded for them (internal/waf/testdata/gitea-requests.txt:49-50 and :69-78) search for SELECT * FROM users WHERE and an img tag, which nobody types to find a name. Ordinary names such as python3-app or docker-build trip only 932260. The definition of done of #198 switches each rule off only where it misfires. Acceptable:

    • On those six paths, q skips only the rules an ordinary name search trips there, and a recorded name search covers each of those rules.
    • The SELECT and img records for those paths are removed.
    • Referer after those searches (gitea.conf:82) is cut down the same way.
    • SPEC.md:703 lists the name searches separately.
  4. SPEC.md:593: "The Core Rule Set puts its own file of exclusions that run before the rules in the same place." "In the same place" comes after a clause that seems to end the sentence. Acceptable: "The Core Rule Set loads its own REQUEST-900-EXCLUSION-RULES-BEFORE-CRS.conf at the same point."

  5. SPEC.md:686: "The Core Rule Set takes some page addresses for attacks, such as a search for env, OpenJDK's Runtime.java, whose path holds both runtime and java., or a file named java.lang.String.md." It is 30 words long and lists its examples with commas, one of them with a clause inside it. Acceptable: end the sentence at "for attacks:" and give the three examples as bullets.

  6. SPEC.md:699: "On gitea's own paths, the fields where gitea sends a search, a file name or a branch name skip the rules that refused ordinary gitea requests there." The verb comes after a list, so "a branch name skip" reads as the clause. Acceptable: "On gitea's paths, some fields skip the rules that refused ordinary gitea requests there. They are the fields where gitea sends a search, a file name or a branch name."

  7. SPEC.md:711: "A file name in a query parameter: file of a commit graph, path of the API's list of commits, body of the API's call for several files, template of a new issue, redirect_to of the sign-in page, and name of an attachment uploaded through the API." It is about 45 words long and chains six items with commas. Acceptable: nested bullets, one for each parameter.

  8. SPEC.md:716: "gitea sends one to delete or restore a branch, to start a release from a tag, to edit a branch protection rule, and to list a directory's last commits." It chains four items with commas. Acceptable: nested bullets, one for each request.

Open point for sneak: the search and file-path exclusions also match the same paths on any other app, such as /api/v1/search?q=, where script injection in q now gets through at the defaults. I took this as following his order to put gitea's fixes in the defaults, and SPEC.md says so.

Model: opus-5-5

**FAIL: `needs-rework`.** 1. **Splitting a line gets past the check for smallwebwaf's own ids.** `ownRuleNamed` (`internal/waf/waf.go:218`) reads the exclusions file one line at a time. Coraza joins a line ending in `\` to the next one. So a file holding `SecRuleRemoveById 899000-\` and then `901000` loads, and so does one holding `ctl:ruleRemoveById=9005\` and then `00`. Each switches off smallwebwaf's own rules, although `SPEC.md` and `docs/settings.md` say such a file stops the start. Acceptable: the check reads the lines joined as Coraza joins them, and a test covers both split forms. 2. **`SecRuleRemoveById` in the exclusions file does nothing and says nothing.** The file loads before the Core Rule Set's rules, so `SecRuleRemoveById 942100` there loads without error and switches nothing off. It is the most common way to write an exclusion, and the operator gets no sign that it was ignored. Acceptable: a directive that removes or changes rules as the file loads, such as `SecRuleRemoveById`, stops the start with an error that names it and points to `SWWAF_WAF_DISABLED_RULES` or a `ctl` exclusion. A test covers `SecRuleRemoveById`. 3. **Name searches skip far more rules than they need.** On the searches for repositories, users, branches, tags and packages, `internal/waf/gitea.conf:39` has `q` skip all 29 rules that code search needs. These paths are `/explore/repos`, `/api/v1/repos/search`, `/user/search_candidates`, `/owner/repo/branches`, `/owner/repo/tags` and `/owner/-/packages`. These searches look for names. The only requests recorded for them (`internal/waf/testdata/gitea-requests.txt:49-50` and `:69-78`) search for `SELECT * FROM users WHERE` and an `img` tag, which nobody types to find a name. Ordinary names such as `python3-app` or `docker-build` trip only 932260. The definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/198 switches each rule off only where it misfires. Acceptable: - On those six paths, `q` skips only the rules an ordinary name search trips there, and a recorded name search covers each of those rules. - The `SELECT` and `img` records for those paths are removed. - `Referer` after those searches (`gitea.conf:82`) is cut down the same way. - `SPEC.md:703` lists the name searches separately. 4. **`SPEC.md:593`:** "The Core Rule Set puts its own file of exclusions that run before the rules in the same place." "In the same place" comes after a clause that seems to end the sentence. Acceptable: "The Core Rule Set loads its own `REQUEST-900-EXCLUSION-RULES-BEFORE-CRS.conf` at the same point." 5. **`SPEC.md:686`:** "The Core Rule Set takes some page addresses for attacks, such as a search for `env`, OpenJDK's `Runtime.java`, whose path holds both `runtime` and `java.`, or a file named `java.lang.String.md`." It is 30 words long and lists its examples with commas, one of them with a clause inside it. Acceptable: end the sentence at "for attacks:" and give the three examples as bullets. 6. **`SPEC.md:699`:** "On gitea's own paths, the fields where gitea sends a search, a file name or a branch name skip the rules that refused ordinary gitea requests there." The verb comes after a list, so "a branch name skip" reads as the clause. Acceptable: "On gitea's paths, some fields skip the rules that refused ordinary gitea requests there. They are the fields where gitea sends a search, a file name or a branch name." 7. **`SPEC.md:711`:** "A file name in a query parameter: `file` of a commit graph, `path` of the API's list of commits, `body` of the API's call for several files, `template` of a new issue, `redirect_to` of the sign-in page, and `name` of an attachment uploaded through the API." It is about 45 words long and chains six items with commas. Acceptable: nested bullets, one for each parameter. 8. **`SPEC.md:716`:** "gitea sends one to delete or restore a branch, to start a release from a tag, to edit a branch protection rule, and to list a directory's last commits." It chains four items with commas. Acceptable: nested bullets, one for each request. Open point for sneak: the search and file-path exclusions also match the same paths on any other app, such as `/api/v1/search?q=`, where script injection in `q` now gets through at the defaults. I took this as following his order to put gitea's fixes in the defaults, and `SPEC.md` says so. Model: opus-5-5
Author
Collaborator

For the reworker: fix every finding of the review above (#208 (comment)) as it says. The Gitea-deployment removal is now on next (#210), so rebase onto next first: this PR no longer merges. Drop anything that touches the removed deployment files. Keep one commit, run make check, and push with --force-with-lease.

Model: opus-5-5

**For the reworker:** fix every finding of the review above (https://git.eeqj.de/sneak/smallwebwaf/pulls/208#issuecomment-170242) as it says. The Gitea-deployment removal is now on `next` (https://git.eeqj.de/sneak/smallwebwaf/pulls/210), so rebase onto `next` first: this PR no longer merges. Drop anything that touches the removed deployment files. Keep one commit, run `make check`, and push with `--force-with-lease`. Model: opus-5-5
Some required checks failed
check / check (push) Failing after 3m17s
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-198-crs-exclusions:issue-198-crs-exclusions
git checkout issue-198-crs-exclusions
Sign in to join this conversation.