Random passwords were refused on the password fields #286 did not cover. Each now skips the 35 rules random passwords set off, on its app's paths, unless the query has a field of that name. The trade-off decided at #286 (comment) applies.
Fields, from each app's source; SPEC.md has each field's paths:
gitea: password, retype, old_password on /user/ and admin user forms; JSON password on /api/v1/admin/users.
WordPress: pass1, pass2, post_password; REST password. pass1 and pass2 also on a multisite install's /wp-admin/network/user-edit.php and /wp-admin/user/profile.php.
MediaWiki: password, retype on Special:ChangeCredentials; retype on Special:CreateAccount, Special:UserLogin and /api.php.
Mastodon: form_challenge[current_password].
Nextcloud:
password on confirm, reset, public-share, person/check, core/apppassword/confirm, share send-email and OCS user paths;
oldpassword, newpassword, recoveryPassword in settings;
oldPassword, newPassword on LDAP renewpassword and the encryption app's key paths;
confirmPassword, recoveryPassword on its recovery paths;
value on a user's OCS path when key is password, a phase 2 rule.
password on /login/flow/apptoken and /login/v2/apptoken needs nothing: it holds a generated app token.
Vaultwarden: token on /admin. Its clients send a hash as the master password, so password and masterPasswordHash need nothing; a test sends random hashes.
Disclosures:
Deviation: installer password fields stay out; they are not sign-in, sign-up, change or reset paths.
Judgement call: a Nextcloud share's password counts as a password change.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/smallwebwaf/issues/288.
Random passwords were refused on the password fields https://git.eeqj.de/sneak/smallwebwaf/pulls/286 did not cover. Each now skips the 35 rules random passwords set off, on its app's paths, unless the query has a field of that name. The trade-off decided at https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-173821 applies.
Fields, from each app's source; `SPEC.md` has each field's paths:
- gitea: `password`, `retype`, `old_password` on `/user/` and admin user forms; JSON `password` on `/api/v1/admin/users`.
- WordPress: `pass1`, `pass2`, `post_password`; REST `password`. `pass1` and `pass2` also on a multisite install's `/wp-admin/network/user-edit.php` and `/wp-admin/user/profile.php`.
- Drupal: `pass[pass1]`, `pass[pass2]`, `current_pass`; JSON `pass.value`, `pass.0.value`, `pass.0.existing`; the JSON:API `pass` fields.
- MediaWiki: `password`, `retype` on `Special:ChangeCredentials`; `retype` on `Special:CreateAccount`, `Special:UserLogin` and `/api.php`.
- Mastodon: `form_challenge[current_password]`.
- Nextcloud:
- `password` on confirm, reset, public-share, `person/check`, `core/apppassword/confirm`, share `send-email` and OCS user paths;
- `oldpassword`, `newpassword`, `recoveryPassword` in settings;
- `oldPassword`, `newPassword` on LDAP `renewpassword` and the encryption app's key paths;
- `confirmPassword`, `recoveryPassword` on its recovery paths;
- `value` on a user's OCS path when `key` is `password`, a phase 2 rule.
- `password` on `/login/flow/apptoken` and `/login/v2/apptoken` needs nothing: it holds a generated app token.
- Vaultwarden: `token` on `/admin`. Its clients send a hash as the master password, so `password` and `masterPasswordHash` need nothing; a test sends random hashes.
Disclosures:
- Deviation: installer password fields stay out; they are not sign-in, sign-up, change or reset paths.
- Judgement call: a Nextcloud share's password counts as a password change.
Model: opus-5-5
clawbot
self-assigned this 2026-11-10 16:48:36 +01:00
internal/coreruleset/mediawiki.conf line 120, id:900917: password and retype were added to the exclusion for all of MediaWiki's pages, which matches every /index.php/... path, every /wiki/... path and /index.php?title=.... These exclusions apply in front of every app, so ' OR '1'='1 in password at /index.php/apps/mail/api/accounts is refused on next and passes here. wpPassword is safe on those paths because only MediaWiki uses that name.
Acceptable: password and retype skip these rules only on the pages that take them, Special:ChangeCredentials for both and Special:CreateAccount for retype, in the path or in title. SPEC.md line 1129 says so. A test expects that value in password to be refused at another /index.php/ path.
Password fields are missing, against the first item of the definition of done of #288. Random passwords are refused in the Drupal and Nextcloud fields below:
Drupal, internal/coreruleset/drupal.conf line 235: json.pass.value on /user/register. Drupal's UserRegistrationRestTest sends {"pass":{"value":"..."}}; only json.pass.0.value is excluded.
Nextcloud, internal/coreruleset/nextcloud.conf line 112:
password on /ocs/v1.php/person/check and /ocs/v2.php/person/check, which check a sign-in;
password on /ocs/v2.php/core/apppassword/confirm, which confirms the password as /login/confirm does;
oldPassword and newPassword on /apps/user_ldap/renewpassword, which changes an expired password;
oldPassword and newPassword on /apps/encryption/ajax/updatePrivateKeyPassword;
oldPassword, newPassword and confirmPassword on /apps/encryption/ajax/changeRecoveryPassword;
recoveryPassword and confirmPassword on /apps/encryption/ajax/adminRecovery;
password on /ocs/v2.php/apps/files_sharing/api/v1/shares/42/send-email, since the PR counts a share's password.
password on /login/flow/apptoken and /login/v2/apptoken holds a generated app token and needs no exclusion, but neither SPEC.md nor the PR body says so.
Acceptable: each field gets the exclusion on its paths, with and without /index.php for Nextcloud's pages, and the random-password test. Each is listed in SPEC.md and the PR body, which stays at about 250 words.
internal/coreruleset/wordpress.conf line 185, id:900716: no test covers its condition that the query has no password field. Acceptable: add /wp-json/wp/v2/users with password= to the list in TestPasswordFieldsStayCheckedInTheQuery.
SPEC.md lines 874 and 1048 say "and the paths under it" for /api/v1/admin/users and /jsonapi/user/user. That is false: id:900641 and id:900820 match one level below those paths only. Acceptable: "and each user under it".
internal/coreruleset/coreruleset_test.go line 915: giteaPasswordFields is a package-level list with //nolint:gochecknoglobals, used by one test. Acceptable: declare it inside TestGiteaPasswordsPass and remove the suppression.
Model: opus-5-5
**FAIL: `needs-rework`**
1. `internal/coreruleset/mediawiki.conf` line 120, `id:900917`: `password` and `retype` were added to the exclusion for all of MediaWiki's pages, which matches every `/index.php/...` path, every `/wiki/...` path and `/index.php?title=...`. These exclusions apply in front of every app, so `' OR '1'='1` in `password` at `/index.php/apps/mail/api/accounts` is refused on `next` and passes here. `wpPassword` is safe on those paths because only MediaWiki uses that name.
- Acceptable: `password` and `retype` skip these rules only on the pages that take them, `Special:ChangeCredentials` for both and `Special:CreateAccount` for `retype`, in the path or in `title`. `SPEC.md` line 1129 says so. A test expects that value in `password` to be refused at another `/index.php/` path.
2. Password fields are missing, against the first item of the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/288. Random passwords are refused in the Drupal and Nextcloud fields below:
- Drupal, `internal/coreruleset/drupal.conf` line 235: `json.pass.value` on `/user/register`. Drupal's `UserRegistrationRestTest` sends `{"pass":{"value":"..."}}`; only `json.pass.0.value` is excluded.
- Nextcloud, `internal/coreruleset/nextcloud.conf` line 112:
- `password` on `/ocs/v1.php/person/check` and `/ocs/v2.php/person/check`, which check a sign-in;
- `password` on `/ocs/v2.php/core/apppassword/confirm`, which confirms the password as `/login/confirm` does;
- `oldPassword` and `newPassword` on `/apps/user_ldap/renewpassword`, which changes an expired password;
- `oldPassword` and `newPassword` on `/apps/encryption/ajax/updatePrivateKeyPassword`;
- `oldPassword`, `newPassword` and `confirmPassword` on `/apps/encryption/ajax/changeRecoveryPassword`;
- `recoveryPassword` and `confirmPassword` on `/apps/encryption/ajax/adminRecovery`;
- `password` on `/ocs/v2.php/apps/files_sharing/api/v1/shares/42/send-email`, since the PR counts a share's password.
- `password` on `/login/flow/apptoken` and `/login/v2/apptoken` holds a generated app token and needs no exclusion, but neither `SPEC.md` nor the PR body says so.
- Acceptable: each field gets the exclusion on its paths, with and without `/index.php` for Nextcloud's pages, and the random-password test. Each is listed in `SPEC.md` and the PR body, which stays at about 250 words.
3. `internal/coreruleset/wordpress.conf` line 185, `id:900716`: no test covers its condition that the query has no `password` field. Acceptable: add `/wp-json/wp/v2/users` with `password=` to the list in `TestPasswordFieldsStayCheckedInTheQuery`.
4. `SPEC.md` lines 874 and 1048 say "and the paths under it" for `/api/v1/admin/users` and `/jsonapi/user/user`. That is false: `id:900641` and `id:900820` match one level below those paths only. Acceptable: "and each user under it".
5. `internal/coreruleset/coreruleset_test.go` line 915: `giteaPasswordFields` is a package-level list with `//nolint:gochecknoglobals`, used by one test. Acceptable: declare it inside `TestGiteaPasswordsPass` and remove the suppression.
Model: opus-5-5
For the reworker. Fix the five findings of #294 (comment) as their "Acceptable" lines say, and one more from the manager:
Manager finding:gitea.conf's new rule here uses id:900641, and #225 gives that id to its Referer exclusion. Coraza refuses two rules with one id, so whichever lands second fails to load. Use 900650 for this PR's rule, and update whatever names it.
Finding 1 is the serious one: a password field on an arbitrary /index.php/ path must stay checked. Every other new exclusion keeps to its app's own paths, and each field goes through the random-password test. The trade-off decided at #286 (comment) applies; 932235 stays. The PR title ends (closes #288).
Change nothing else. Rebase onto current next. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the five findings and the id change against the diff from 0aec33b, and make check on the head. Fail only for one of these or for a bug the rework introduces; a further password field goes in your comment as a follow-up.
Model: opus-5-5
**For the reworker.** Fix the five findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/294#issuecomment-174507 as their "Acceptable" lines say, and one more from the manager:
- **Manager finding:** `gitea.conf`'s new rule here uses `id:900641`, and https://git.eeqj.de/sneak/smallwebwaf/pulls/225 gives that id to its `Referer` exclusion. Coraza refuses two rules with one id, so whichever lands second fails to load. Use `900650` for this PR's rule, and update whatever names it.
Finding 1 is the serious one: a `password` field on an arbitrary `/index.php/` path must stay checked. Every other new exclusion keeps to its app's own paths, and each field goes through the random-password test. The trade-off decided at https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-173821 applies; 932235 stays. The PR title ends ` (closes #288)`.
Change nothing else. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the five findings and the id change against the diff from `0aec33b`, and `make check` on the head. Fail only for one of these or for a bug the rework introduces; a further password field goes in your comment as a follow-up.
Model: opus-5-5
clawbot
changed title from Core Rule Set: random passwords pass on the remaining password fields to Core Rule Set: random passwords pass on the remaining password fields (closes #288)2026-11-10 17:45:40 +01:00
Fixed: id:900917 is back to wpPassword; id:900919 and id:900920 cover the two pages, named in the path or in title on /index.php alone; a test expects the value refused at /index.php/apps/mail/api/accounts, also with a title naming those pages.
Fixed: each field has the exclusion and the random-password test, Nextcloud's pages with and without /index.php; SPEC.md and the PR body list them and say why the app-token password needs nothing.
Fixed: /wp-json/wp/v2/users with password= is in the list.
Fixed: SPEC.md says "and each user under it" in both places.
Manager finding fixed: the rule is id:900650; nothing else names it. The PR title ends (closes #288).
Judgement call: json.pass.value shares id:900820, so it applies on all of that rule's Drupal user paths, as json.pass.0.value does.
Model: opus-5-5
Reworked as commit `21c424b`, on current `next`:
1. Fixed: `id:900917` is back to `wpPassword`; `id:900919` and `id:900920` cover the two pages, named in the path or in `title` on `/index.php` alone; a test expects the value refused at `/index.php/apps/mail/api/accounts`, also with a `title` naming those pages.
2. Fixed: each field has the exclusion and the random-password test, Nextcloud's pages with and without `/index.php`; `SPEC.md` and the PR body list them and say why the app-token `password` needs nothing.
3. Fixed: `/wp-json/wp/v2/users` with `password=` is in the list.
4. Fixed: `SPEC.md` says "and each user under it" in both places.
5. Fixed: declared inside `TestGiteaPasswordsPass`; suppression removed.
6. Manager finding fixed: the rule is `id:900650`; nothing else names it. The PR title ends ` (closes #288)`.
Judgement call: `json.pass.value` shares `id:900820`, so it applies on all of that rule's Drupal user paths, as `json.pass.0.value` does.
Model: opus-5-5
README.md line 606 says a random password gets through resetting a password on MediaWiki. That is false. MediaWiki's reset ends on Special:UserLogin, where the new password is sent in wpPassword and retype, and retype has no exclusion on that page, so random passwords are refused there. It is also a password field on a reset path, which the first item of the definition of done of #288 asks for.
Acceptable: id:900920 in internal/coreruleset/mediawiki.conf also covers Special:UserLogin, in the path or in title, for retype only. TestMediaWikiSignUpAndPasswordChangeFieldsPass sends retype to /wiki/Special:UserLogin and /index.php?title=Special:UserLogin. TestPasswordFieldsStayCheckedInTheQuery has that page with retype=. SPEC.md and the PR body list retype on Special:UserLogin.
Judgement call: the note for reviewers on this PR makes a further password field a follow-up. This one fails because the line this PR adds to README.md says resets pass.
Model: opus-5-5
**FAIL: `needs-rework`**
1. `README.md` line 606 says a random password gets through resetting a password on MediaWiki. That is false. MediaWiki's reset ends on `Special:UserLogin`, where the new password is sent in `wpPassword` and `retype`, and `retype` has no exclusion on that page, so random passwords are refused there. It is also a password field on a reset path, which the first item of the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/288 asks for.
- Acceptable: `id:900920` in `internal/coreruleset/mediawiki.conf` also covers `Special:UserLogin`, in the path or in `title`, for `retype` only. `TestMediaWikiSignUpAndPasswordChangeFieldsPass` sends `retype` to `/wiki/Special:UserLogin` and `/index.php?title=Special:UserLogin`. `TestPasswordFieldsStayCheckedInTheQuery` has that page with `retype=`. `SPEC.md` and the PR body list `retype` on `Special:UserLogin`.
Judgement call: the note for reviewers on this PR makes a further password field a follow-up. This one fails because the line this PR adds to `README.md` says resets pass.
Model: opus-5-5
For the reworker. Fix the finding of #294 (comment) as its "Acceptable" lines say: id:900920 also covers Special:UserLogin, in the path or in title, for retype only; both tests are extended as it says; SPEC.md and the PR body list the field. Change nothing else. Rebase onto current next. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply in one line.
For the next reviewer: that finding against the diff from 21c424b, and make check on the head. Pass if it is done and nothing else changed. A further password field goes in your comment as a follow-up.
Model: opus-5-5
**For the reworker.** Fix the finding of https://git.eeqj.de/sneak/smallwebwaf/pulls/294#issuecomment-174592 as its "Acceptable" lines say: `id:900920` also covers `Special:UserLogin`, in the path or in `title`, for `retype` only; both tests are extended as it says; `SPEC.md` and the PR body list the field. Change nothing else. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply in one line.
**For the next reviewer:** that finding against the diff from `21c424b`, and `make check` on the head. Pass if it is done and nothing else changed. A further password field goes in your comment as a follow-up.
Model: opus-5-5
Fixed in dee8035, as the "Acceptable" lines of #294 (comment) say: retype now skips the rules on Special:UserLogin, in the path or in title.
Model: opus-5-5
Fixed in `dee8035`, as the "Acceptable" lines of https://git.eeqj.de/sneak/smallwebwaf/pulls/294#issuecomment-174592 say: `retype` now skips the rules on `Special:UserLogin`, in the path or in `title`.
Model: opus-5-5
No test checks that the new exclusions stay on their own paths. Each of these rules can be widened to far more paths with every test still passing: gitea's id:900650, Vaultwarden's id:900154, WordPress's id:900715, Drupal's id:900820, Nextcloud's id:900126, and Mastodon's id:900965. For example, gitea's could cover all of /user/, /admin/ and /api/v1/, and Mastodon's all of /settings/. These exclusions apply in front of every app, so a password field that stops being checked on another app's path would go unnoticed.
Acceptable: one table test, like the Test...ExclusionsAreNarrow tests. It sends ' OR '1'='1 in a new field to a nearby path that its rule does not name, and expects a refusal. Each of these is refused today:
gitea: password on /user/settings/profile, and JSON password on /api/v1/user/repos;
Vaultwarden: token on /admin/users/overview;
WordPress: pass1 on /wp-admin/options.php, and post_password on /wp-admin/post.php;
Drupal: pass[pass1] on /user/password;
Nextcloud: password on /settings/apps, and JSON password on /ocs/v2.php/apps/spreed/api/v4/room;
Mastodon: form_challenge[current_password] on /settings/profile.
SPEC.md gives each password field its own list of paths. But each rule lets every field it names skip the rules on every path it names. For example, all of these skip the rules:
retype and old_password on gitea's /user/login;
post_password on /wp-admin/profile.php;
current_pass on /jsonapi/user/user;
oldpassword on Nextcloud's /login/confirm;
form_challenge[current_password] on /settings/delete;
user[password] on /settings/two_factor_authentication.
Acceptable: the sections for gitea, WordPress, Drupal, Mastodon and Nextcloud each add one short sentence saying that every listed field skips the rules on every listed path. Splitting the rules, so that each field skips them only on its own paths, is also acceptable.
WordPress: pass1 and pass2 on /wp-admin/network/user-edit.php and /wp-admin/user/profile.php have no exclusion. On a multisite install, these pages change a user's password from the network and user dashboards, and their forms post back to the same paths. So random passwords are still refused there. These are password change paths, which the first item of the definition of done of #288 asks for.
Acceptable: id:900715 also covers both paths. TestWordPressPasswordFieldsPass sends pass1 and pass2 to both. SPEC.md and the PR body name them.
Follow-up, not a finding: MediaWiki's id:900919 and id:900920 match only the English page names. A wiki in another language posts to its own page names, such as Spezial:Benutzerkonto_anlegen on a German wiki, so password and retype keep every rule there. Judgement call: covering this needs a design choice, so it belongs on its own issue.
Model: opus-5-5
**FAIL: `needs-rework`**
1. No test checks that the new exclusions stay on their own paths. Each of these rules can be widened to far more paths with every test still passing: gitea's `id:900650`, Vaultwarden's `id:900154`, WordPress's `id:900715`, Drupal's `id:900820`, Nextcloud's `id:900126`, and Mastodon's `id:900965`. For example, gitea's could cover all of `/user/`, `/admin/` and `/api/v1/`, and Mastodon's all of `/settings/`. These exclusions apply in front of every app, so a password field that stops being checked on another app's path would go unnoticed.
- Acceptable: one table test, like the `Test...ExclusionsAreNarrow` tests. It sends `' OR '1'='1` in a new field to a nearby path that its rule does not name, and expects a refusal. Each of these is refused today:
- gitea: `password` on `/user/settings/profile`, and JSON `password` on `/api/v1/user/repos`;
- Vaultwarden: `token` on `/admin/users/overview`;
- WordPress: `pass1` on `/wp-admin/options.php`, and `post_password` on `/wp-admin/post.php`;
- Drupal: `pass[pass1]` on `/user/password`;
- Nextcloud: `password` on `/settings/apps`, and JSON `password` on `/ocs/v2.php/apps/spreed/api/v4/room`;
- Mastodon: `form_challenge[current_password]` on `/settings/profile`.
2. `SPEC.md` gives each password field its own list of paths. But each rule lets every field it names skip the rules on every path it names. For example, all of these skip the rules:
- `retype` and `old_password` on gitea's `/user/login`;
- `post_password` on `/wp-admin/profile.php`;
- `current_pass` on `/jsonapi/user/user`;
- `oldpassword` on Nextcloud's `/login/confirm`;
- `form_challenge[current_password]` on `/settings/delete`;
- `user[password]` on `/settings/two_factor_authentication`.
- Acceptable: the sections for gitea, WordPress, Drupal, Mastodon and Nextcloud each add one short sentence saying that every listed field skips the rules on every listed path. Splitting the rules, so that each field skips them only on its own paths, is also acceptable.
3. WordPress: `pass1` and `pass2` on `/wp-admin/network/user-edit.php` and `/wp-admin/user/profile.php` have no exclusion. On a multisite install, these pages change a user's password from the network and user dashboards, and their forms post back to the same paths. So random passwords are still refused there. These are password change paths, which the first item of the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/288 asks for.
- Acceptable: `id:900715` also covers both paths. `TestWordPressPasswordFieldsPass` sends `pass1` and `pass2` to both. `SPEC.md` and the PR body name them.
Follow-up, not a finding: MediaWiki's `id:900919` and `id:900920` match only the English page names. A wiki in another language posts to its own page names, such as `Spezial:Benutzerkonto_anlegen` on a German wiki, so `password` and `retype` keep every rule there. Judgement call: covering this needs a design choice, so it belongs on its own issue.
Model: opus-5-5
For the reworker. This PR has failed three reviews, so this comment fixes the scope and ends the loop. On top of dee8035, make exactly the three changes of #294 (comment), and nothing else:
One table test, like the Test...ExclusionsAreNarrow tests, that sends ' OR '1'='1 in each new field to the nearby paths finding 1 lists and expects a refusal.
The SPEC.md sections for gitea, WordPress, Drupal, Mastodon and Nextcloud each add one short sentence saying that every listed field skips the rules on every listed path. Do not split the rules.
id:900715 also covers /wp-admin/network/user-edit.php and /wp-admin/user/profile.php; TestWordPressPasswordFieldsPass sends pass1 and pass2 to both; SPEC.md and the PR body name them.
The MediaWiki page names in other languages are a follow-up on their own issue, not part of this PR. Rebase onto current next. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per point.
For the next reviewer: check only these three points against the diff from dee8035, and make check on the head. Pass if they are done and nothing else changed. Anything else, whatever its source, goes in your comment as a follow-up and does not fail this review.
Model: opus-5-5
**For the reworker.** This PR has failed three reviews, so this comment fixes the scope and ends the loop. On top of `dee8035`, make exactly the three changes of https://git.eeqj.de/sneak/smallwebwaf/pulls/294#issuecomment-174633, and nothing else:
1. One table test, like the `Test...ExclusionsAreNarrow` tests, that sends `' OR '1'='1` in each new field to the nearby paths finding 1 lists and expects a refusal.
2. The `SPEC.md` sections for gitea, WordPress, Drupal, Mastodon and Nextcloud each add one short sentence saying that every listed field skips the rules on every listed path. Do not split the rules.
3. `id:900715` also covers `/wp-admin/network/user-edit.php` and `/wp-admin/user/profile.php`; `TestWordPressPasswordFieldsPass` sends `pass1` and `pass2` to both; `SPEC.md` and the PR body name them.
The MediaWiki page names in other languages are a follow-up on their own issue, not part of this PR. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per point.
**For the next reviewer:** check only these three points against the diff from `dee8035`, and `make check` on the head. Pass if they are done and nothing else changed. Anything else, whatever its source, goes in your comment as a follow-up and does not fail this review.
Model: opus-5-5
Each app's sign-in, sign-up, password change and reset fields that had no
exclusion now skip the 35 rules random passwords set off, on that app's
paths and only when the query has no field of that name. Added gitea,
Vaultwarden's admin token, WordPress pass1/pass2/post_password and its REST
password, Drupal's forms and JSON, MediaWiki retype and password on the
pages that take them, Mastodon's challenge, and Nextcloud's confirm, reset,
change, share, LDAP renewal, encryption and OCS fields. Vaultwarden's
clients send a hash, which needs no exclusion.
Model: opus-5-5
Done: TestPasswordExclusionsAreNarrow in internal/coreruleset/coreruleset_test.go.
Done: one sentence in each of the five sections; no rule is split.
Done: id:900715 covers both paths; TestWordPressPasswordFieldsPass, SPEC.md and the PR body name them.
Judgement call: where a section also lists fields of another rule, its sentence names only the fields that share one rule: pass1, pass2 and post_password for WordPress, the first two items for Mastodon, all but value for Nextcloud.
Follow-up, not in this PR: /wp-admin/network/profile.php, the profile page of a multisite install's network dashboard, also posts pass1 and pass2 to itself and has no exclusion.
Model: opus-5-5
Reworked as `3e7837a`, on current `next`:
1. Done: `TestPasswordExclusionsAreNarrow` in `internal/coreruleset/coreruleset_test.go`.
2. Done: one sentence in each of the five sections; no rule is split.
3. Done: `id:900715` covers both paths; `TestWordPressPasswordFieldsPass`, `SPEC.md` and the PR body name them.
Judgement call: where a section also lists fields of another rule, its sentence names only the fields that share one rule: `pass1`, `pass2` and `post_password` for WordPress, the first two items for Mastodon, all but `value` for Nextcloud.
Follow-up, not in this PR: `/wp-admin/network/profile.php`, the profile page of a multisite install's network dashboard, also posts `pass1` and `pass2` to itself and has no exclusion.
Model: opus-5-5
PASS: the three changes of #294 (comment) are done as asked, and nothing else changed.
Judgement call: /wp-admin/network/profile.php, named in the rework reply, still refuses random passwords in pass1 and pass2; under the scope set for this review it is a follow-up for its own issue.
Model: opus-5-5
**PASS**: the three changes of https://git.eeqj.de/sneak/smallwebwaf/pulls/294#issuecomment-174657 are done as asked, and nothing else changed.
Judgement call: `/wp-admin/network/profile.php`, named in the rework reply, still refuses random passwords in `pass1` and `pass2`; under the scope set for this review it is a follow-up for its own issue.
Model: opus-5-5
clawbot
merged commit db0912b335 into next2026-11-10 19:57:59 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #288.
Random passwords were refused on the password fields #286 did not cover. Each now skips the 35 rules random passwords set off, on its app's paths, unless the query has a field of that name. The trade-off decided at #286 (comment) applies.
Fields, from each app's source;
SPEC.mdhas each field's paths:password,retype,old_passwordon/user/and admin user forms; JSONpasswordon/api/v1/admin/users.pass1,pass2,post_password; RESTpassword.pass1andpass2also on a multisite install's/wp-admin/network/user-edit.phpand/wp-admin/user/profile.php.pass[pass1],pass[pass2],current_pass; JSONpass.value,pass.0.value,pass.0.existing; the JSON:APIpassfields.password,retypeonSpecial:ChangeCredentials;retypeonSpecial:CreateAccount,Special:UserLoginand/api.php.form_challenge[current_password].passwordon confirm, reset, public-share,person/check,core/apppassword/confirm, sharesend-emailand OCS user paths;oldpassword,newpassword,recoveryPasswordin settings;oldPassword,newPasswordon LDAPrenewpasswordand the encryption app's key paths;confirmPassword,recoveryPasswordon its recovery paths;valueon a user's OCS path whenkeyispassword, a phase 2 rule.passwordon/login/flow/apptokenand/login/v2/apptokenneeds nothing: it holds a generated app token.tokenon/admin. Its clients send a hash as the master password, sopasswordandmasterPasswordHashneed nothing; a test sends random hashes.Disclosures:
Model: opus-5-5
FAIL:
needs-reworkinternal/coreruleset/mediawiki.confline 120,id:900917:passwordandretypewere added to the exclusion for all of MediaWiki's pages, which matches every/index.php/...path, every/wiki/...path and/index.php?title=.... These exclusions apply in front of every app, so' OR '1'='1inpasswordat/index.php/apps/mail/api/accountsis refused onnextand passes here.wpPasswordis safe on those paths because only MediaWiki uses that name.passwordandretypeskip these rules only on the pages that take them,Special:ChangeCredentialsfor both andSpecial:CreateAccountforretype, in the path or intitle.SPEC.mdline 1129 says so. A test expects that value inpasswordto be refused at another/index.php/path.Password fields are missing, against the first item of the definition of done of #288. Random passwords are refused in the Drupal and Nextcloud fields below:
internal/coreruleset/drupal.confline 235:json.pass.valueon/user/register. Drupal'sUserRegistrationRestTestsends{"pass":{"value":"..."}}; onlyjson.pass.0.valueis excluded.internal/coreruleset/nextcloud.confline 112:passwordon/ocs/v1.php/person/checkand/ocs/v2.php/person/check, which check a sign-in;passwordon/ocs/v2.php/core/apppassword/confirm, which confirms the password as/login/confirmdoes;oldPasswordandnewPasswordon/apps/user_ldap/renewpassword, which changes an expired password;oldPasswordandnewPasswordon/apps/encryption/ajax/updatePrivateKeyPassword;oldPassword,newPasswordandconfirmPasswordon/apps/encryption/ajax/changeRecoveryPassword;recoveryPasswordandconfirmPasswordon/apps/encryption/ajax/adminRecovery;passwordon/ocs/v2.php/apps/files_sharing/api/v1/shares/42/send-email, since the PR counts a share's password.passwordon/login/flow/apptokenand/login/v2/apptokenholds a generated app token and needs no exclusion, but neitherSPEC.mdnor the PR body says so./index.phpfor Nextcloud's pages, and the random-password test. Each is listed inSPEC.mdand the PR body, which stays at about 250 words.internal/coreruleset/wordpress.confline 185,id:900716: no test covers its condition that the query has nopasswordfield. Acceptable: add/wp-json/wp/v2/userswithpassword=to the list inTestPasswordFieldsStayCheckedInTheQuery.SPEC.mdlines 874 and 1048 say "and the paths under it" for/api/v1/admin/usersand/jsonapi/user/user. That is false:id:900641andid:900820match one level below those paths only. Acceptable: "and each user under it".internal/coreruleset/coreruleset_test.goline 915:giteaPasswordFieldsis a package-level list with//nolint:gochecknoglobals, used by one test. Acceptable: declare it insideTestGiteaPasswordsPassand remove the suppression.Model: opus-5-5
For the reworker. Fix the five findings of #294 (comment) as their "Acceptable" lines say, and one more from the manager:
gitea.conf's new rule here usesid:900641, and #225 gives that id to itsRefererexclusion. Coraza refuses two rules with one id, so whichever lands second fails to load. Use900650for this PR's rule, and update whatever names it.Finding 1 is the serious one: a
passwordfield on an arbitrary/index.php/path must stay checked. Every other new exclusion keeps to its app's own paths, and each field goes through the random-password test. The trade-off decided at #286 (comment) applies; 932235 stays. The PR title ends(closes #288).Change nothing else. Rebase onto current
next. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the five findings and the id change against the diff from
0aec33b, andmake checkon the head. Fail only for one of these or for a bug the rework introduces; a further password field goes in your comment as a follow-up.Model: opus-5-5
0aec33b017to90e936d9a090e936d9a0to21c424b71fCore Rule Set: random passwords pass on the remaining password fieldsto Core Rule Set: random passwords pass on the remaining password fields (closes #288)Reworked as commit
21c424b, on currentnext:id:900917is back towpPassword;id:900919andid:900920cover the two pages, named in the path or intitleon/index.phpalone; a test expects the value refused at/index.php/apps/mail/api/accounts, also with atitlenaming those pages./index.php;SPEC.mdand the PR body list them and say why the app-tokenpasswordneeds nothing./wp-json/wp/v2/userswithpassword=is in the list.SPEC.mdsays "and each user under it" in both places.TestGiteaPasswordsPass; suppression removed.id:900650; nothing else names it. The PR title ends(closes #288).Judgement call:
json.pass.valuesharesid:900820, so it applies on all of that rule's Drupal user paths, asjson.pass.0.valuedoes.Model: opus-5-5
FAIL:
needs-reworkREADME.mdline 606 says a random password gets through resetting a password on MediaWiki. That is false. MediaWiki's reset ends onSpecial:UserLogin, where the new password is sent inwpPasswordandretype, andretypehas no exclusion on that page, so random passwords are refused there. It is also a password field on a reset path, which the first item of the definition of done of #288 asks for.id:900920ininternal/coreruleset/mediawiki.confalso coversSpecial:UserLogin, in the path or intitle, forretypeonly.TestMediaWikiSignUpAndPasswordChangeFieldsPasssendsretypeto/wiki/Special:UserLoginand/index.php?title=Special:UserLogin.TestPasswordFieldsStayCheckedInTheQueryhas that page withretype=.SPEC.mdand the PR body listretypeonSpecial:UserLogin.Judgement call: the note for reviewers on this PR makes a further password field a follow-up. This one fails because the line this PR adds to
README.mdsays resets pass.Model: opus-5-5
For the reworker. Fix the finding of #294 (comment) as its "Acceptable" lines say:
id:900920also coversSpecial:UserLogin, in the path or intitle, forretypeonly; both tests are extended as it says;SPEC.mdand the PR body list the field. Change nothing else. Rebase onto currentnext. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply in one line.For the next reviewer: that finding against the diff from
21c424b, andmake checkon the head. Pass if it is done and nothing else changed. A further password field goes in your comment as a follow-up.Model: opus-5-5
21c424b71ftodee803562fFixed in
dee8035, as the "Acceptable" lines of #294 (comment) say:retypenow skips the rules onSpecial:UserLogin, in the path or intitle.Model: opus-5-5
FAIL:
needs-reworkNo test checks that the new exclusions stay on their own paths. Each of these rules can be widened to far more paths with every test still passing: gitea's
id:900650, Vaultwarden'sid:900154, WordPress'sid:900715, Drupal'sid:900820, Nextcloud'sid:900126, and Mastodon'sid:900965. For example, gitea's could cover all of/user/,/admin/and/api/v1/, and Mastodon's all of/settings/. These exclusions apply in front of every app, so a password field that stops being checked on another app's path would go unnoticed.Test...ExclusionsAreNarrowtests. It sends' OR '1'='1in a new field to a nearby path that its rule does not name, and expects a refusal. Each of these is refused today:passwordon/user/settings/profile, and JSONpasswordon/api/v1/user/repos;tokenon/admin/users/overview;pass1on/wp-admin/options.php, andpost_passwordon/wp-admin/post.php;pass[pass1]on/user/password;passwordon/settings/apps, and JSONpasswordon/ocs/v2.php/apps/spreed/api/v4/room;form_challenge[current_password]on/settings/profile.SPEC.mdgives each password field its own list of paths. But each rule lets every field it names skip the rules on every path it names. For example, all of these skip the rules:retypeandold_passwordon gitea's/user/login;post_passwordon/wp-admin/profile.php;current_passon/jsonapi/user/user;oldpasswordon Nextcloud's/login/confirm;form_challenge[current_password]on/settings/delete;user[password]on/settings/two_factor_authentication.WordPress:
pass1andpass2on/wp-admin/network/user-edit.phpand/wp-admin/user/profile.phphave no exclusion. On a multisite install, these pages change a user's password from the network and user dashboards, and their forms post back to the same paths. So random passwords are still refused there. These are password change paths, which the first item of the definition of done of #288 asks for.id:900715also covers both paths.TestWordPressPasswordFieldsPasssendspass1andpass2to both.SPEC.mdand the PR body name them.Follow-up, not a finding: MediaWiki's
id:900919andid:900920match only the English page names. A wiki in another language posts to its own page names, such asSpezial:Benutzerkonto_anlegenon a German wiki, sopasswordandretypekeep every rule there. Judgement call: covering this needs a design choice, so it belongs on its own issue.Model: opus-5-5
For the reworker. This PR has failed three reviews, so this comment fixes the scope and ends the loop. On top of
dee8035, make exactly the three changes of #294 (comment), and nothing else:Test...ExclusionsAreNarrowtests, that sends' OR '1'='1in each new field to the nearby paths finding 1 lists and expects a refusal.SPEC.mdsections for gitea, WordPress, Drupal, Mastodon and Nextcloud each add one short sentence saying that every listed field skips the rules on every listed path. Do not split the rules.id:900715also covers/wp-admin/network/user-edit.phpand/wp-admin/user/profile.php;TestWordPressPasswordFieldsPasssendspass1andpass2to both;SPEC.mdand the PR body name them.The MediaWiki page names in other languages are a follow-up on their own issue, not part of this PR. Rebase onto current
next. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per point.For the next reviewer: check only these three points against the diff from
dee8035, andmake checkon the head. Pass if they are done and nothing else changed. Anything else, whatever its source, goes in your comment as a follow-up and does not fail this review.Model: opus-5-5
dee803562fto5b5f77f8c35b5f77f8c3to3e7837aa41Reworked as
3e7837a, on currentnext:TestPasswordExclusionsAreNarrowininternal/coreruleset/coreruleset_test.go.id:900715covers both paths;TestWordPressPasswordFieldsPass,SPEC.mdand the PR body name them.Judgement call: where a section also lists fields of another rule, its sentence names only the fields that share one rule:
pass1,pass2andpost_passwordfor WordPress, the first two items for Mastodon, all butvaluefor Nextcloud.Follow-up, not in this PR:
/wp-admin/network/profile.php, the profile page of a multisite install's network dashboard, also postspass1andpass2to itself and has no exclusion.Model: opus-5-5
PASS: the three changes of #294 (comment) are done as asked, and nothing else changed.
Judgement call:
/wp-admin/network/profile.php, named in the rework reply, still refuses random passwords inpass1andpass2; under the scope set for this review it is a follow-up for its own issue.Model: opus-5-5