fix(gate-48): a net-zero MOVE is not a removal, and 'no co-change in the diff' is not 'no co-change needed'

[rubenvdlinde]

Closes #303.

Two defects, both measured on larpingapp.

1. A net-zero MOVE counted as a removal

check_csrf_removal.py scanned only ^- lines and never asked whether an identical ^+ line existed, so relocating a docblock line inside a file read as deleting the annotation.

larpingapp#297 moved one comment line while reordering SettingsController's docblocks:

-     * @NoCSRFRequired removed to close the CSRF-forgery surface (closes #206).
+     * instance-wide configuration write needs. `@NoCSRFRequired` was removed
+     * @NoCSRFRequired removed to close the CSRF-forgery surface (closes #206).

Byte-identity check on that diff: identical line also ADDED: True. Nothing about CSRF changed, and gate-48 kept Hydra Gates red on that repo's development from then on.

Cancellation is per file and by multiset:

• per file, because a line deleted from one controller and added to another is a real posture change for the first one;
• by multiset, because a diff that removes a tag twice and restores it once has removed it once;
• and whitespace is not normalised — a re-indented line is not the same line, and treating it as a move would let a reformat swallow a genuine deletion.

pre-fix   removals() on the #297 diff -> 1
post-fix  removals() on the #297 diff -> 0

Seven new tests pin this, including the cross-file arm, the remove-twice-restore-once arm, the re-indent arm, and the +++ b/... header (which starts with + and must not be pooled as an added line).

2. The gate could not see a caller that was already compliant

The co-change question was asked of the diff. A PR whose callers have always sent a token has no signal to add, so it could not pass — the only exits were a waiver or staying red.

Measured on larpingapp#298, which closes a live CSRF-forgery hole. SettingsController::create() and reimport() carried

     * @NoCSRFRequired removed to close the CSRF-forgery surface (closes #206).

at docblock-tag position, where Nextcloud's ControllerMethodReflector parses annotations with

preg_match_all('/^\h+\*\h+@(?P<annotation>[A-Z]\w+)((?P<parameter>.*))?$/m', $docs, $matches);

— so the sentence announcing the removal was the annotation, and SecurityMiddleware::isValidCSRF() returned early. CSRF was never enforced on either state-mutating admin POST. Deleting that line is the fix.

Every frontend caller already sent a token (requesttoken on all three fetch calls; CnAdminSettingsShell uses @nextcloud/axios, which injects it), and OC.requestToken is kept current rather than snapshotted — core/src/OC/index.js subscribes to csrf-token-update and reassigns it. So the co-change did not exist to be made, and the cheapest route to green would have been a cosmetic edit under src/ containing the word requesttoken.

The question it asks instead

New scripts/lib/check_csrf_callers.py: is any mutating caller unprotected right now? Conservative in the direction that matters:

• every mutating call site protected ⇒ whichever one reaches the endpoint is protected, so enforcing CSRF cannot break it → pass, stating the claim on stdout rather than passing silently;
• any call site unprotected ⇒ we cannot show it is not this endpoint's caller → the removal still blocks, and the log now names the call site.

Protection means, within the call expression itself: requesttoken / OCS-APIRequest (both case-insensitive — HTTP header names are) or getRequestToken; or the call goes through @nextcloud/axios, imported in that file. Extraction is paren-balanced, so a correct call cannot vouch for an incorrect one beside it — pinned by a test.

Both arms, end-to-end through the runner on larpingapp#298

arm 1  every caller protected
       [gate-48] no CSRF signal was ADDED by this diff, and none was needed:
                 every mutating call site under src/ already carries one …
       [gate-48] csrf-cochange: PASS

arm 2  one unprotected fetch() DELETE added — the opencatalogi#79 shape
       [gate-48] csrf-cochange: FAIL
       UNPROTECTED mutating call site(s) — these are why the removal blocks:
       src/services/__armtwo_probe.js:3 — fetch() DELETE with no CSRF signal

Arm 2 is the one that proves this did not switch the gate off. opencatalogi#79 — the delete-modal fetch() with no CSRF header that gate-48 was built for — is still caught, and its shape is pinned as a test.

Suites

scripts/lib/test_* files 63 → 64; test_check_csrf_callers.py is auto-discovered by run-helper-suites.sh and passes. Full run: passed 62, quarantined 2, failed 0.

Fallback behaviour

If check_csrf_callers.py is absent or the app has no src/, the gate falls back to the previous diff-only question and says so in the log — a missing helper must not report pass (#147).