fix(gate-48): the frontend-signal pathspec could not see a file directly under src/ (#428) - #436
Merged
Merged
Conversation
…tly under src/ (#428) In a PLAIN git pathspec there is no `**` operator — it is two ordinary `*`s, and a plain `*` matches `/`. So `src/**/*.js` reads as "src/, anything, /, anything, .js": it REQUIRES at least one directory below src/, and every file at `src/foo.js` was invisible to gate-48's CSRF-signal scan. Reproduced on c26f9a3, one repo, one commit, nothing varying but the path: git diff --name-only HEAD~1...HEAD -- 'src/**/*.js' -> (empty) git diff --name-only HEAD~1...HEAD -- ':(glob)src/**/*.js' -> src/thing.js `:(glob)` magic makes `**` mean "zero or more directory components" and stops a single `*` crossing `/`. It is a strict superset of the old spelling — src/sub/thing.js and src/a/b/c.js still match. DIRECTION: a missed signal made the count 0, which routes to the caller-state check (the conservative branch), so this was never letting a CSRF removal through. It made the count a FLOOR rather than a count, and the NOTE gate-48 prints about the frontend diff could be wrong about what it had read. Fixing it therefore REMOVES findings, it does not add them. FAMILY I in test_gate_45_to_55_acceptance.sh: I1 is the evidence arm (FAIL on origin/main, PASS here); I2 (same signal one directory down) and I3 (anti-widening: a top-level src/ edit with NO signal must still FAIL) hold their verdict in BOTH arms and are labelled CONTROLs in the file. FAMILY H's header already warned about this pathspec and worked AROUND it; that note now points at the fix instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #428.
The defect
In a plain git pathspec there is no
**operator: it is two ordinary*s, and a plain*matches/. Sosrc/**/*.jsreads as "src/, then anything, then/, then anything, then.js" — it requires at least one directory belowsrc/, and every file sitting atsrc/foo.jswas invisible to gate-48's CSRF-signal scan.Reproduced on c26f9a3 — one repo, one commit, nothing varying but the path
src/sub/thing.jsmatches under both spellings, so:(glob)is a strict superset — nothing that matched before stops matching.Direction — this REMOVES findings, it does not add them
The task brief expected this to increase gate-48's findings. It does the opposite, and the mechanism is one-way: the pathspec feeds only
_csrf_fe_signals, and a signal count of0routes to the caller-state check (check_csrf_callers.py), the conservative branch. So the blind spot was never letting a CSRF removal through — it made the count a floor rather than a count, and theNOTEgate-48 prints about the frontend diff could be wrong about what it had read. Un-blinding it moves runs off the fallback branch.Acceptance — FAMILY I in
test_gate_45_to_55_acceptance.shFAMILY H's own header already warned about this pathspec and worked around it ("the fixture file must not sit directly in
src/"). The workaround was correct; the blind spot was left in the gate. FAMILY I removes it.Against this branch: whole suite green. Against
origin/main's runner (a real revert viaHYDRA_GATES_RUNNER_UNDER_TEST=, notgit checkout --): 1 failure, the evidence arm.requesttokenadded insrc/thing.jsis seensrc/sub/thing.jssrc/edit carrying no signalI3 is what separates "make the top level visible" from "count any top-level edit".
Fleet numbers, and the layer they come from
developmentHEAD there is no delta base, so it reportsNOT APPLICABLE— a whole-tree fleet run cannot exercise it and would producenain both arms, which is not a measurement. So the exposure is reported structurally, over six freshdevelopmentclones:src/*.{js,ts,vue}(was invisible)src/**/*(was visible)src/customComponents.js)src/App.vue)src/setPublicPath.js)63 files across six repos sat in the blind spot, and they are not obscure ones:
App.vue,main.js,settings.js,registry.jsand every dashboard-widget entry point — exactly the files where@nextcloud/axiosor arequesttokenheader gets added. Three of them carry a signal right now.The verdict-layer evidence is FAMILY I above, which is the same shape driven through
bin/hydra-gates.Real-tree delta measurement (verdict layer)
Since a whole-tree run reports
na, a real diff was built on three real trees: drop@NoCSRFRequired/#[NoCSRFRequired]from a controller that actually carries one, and add a realrequesttoken+@nextcloud/axiosto the repo's existing top-levelsrc/settings.js.lib/Controller/StufController.phpFAILPASSlib/Controller/HealthController.phpFAILPASSlib/Controller/SettingsController.phpPASSPASS0 new findings. 2 findings removed, and both were false — hand-characterised, not counted:
procest, base log:
@NoCSRFRequired removed but no frontend CSRF-signal added in diff. That sentence is untrue of that diff; the same commit addedrequesttokeninsrc/settings.js. The gate could not see it, fell through to the caller-state check, and blocked on an unrelated pre-existingsrc/store/modules/workflow.js:828 — fetch() POST with no CSRF signal.docudesk, same shape, blocking on
src/views/settings/Settings.vue:816/879/943.larpingapp is the arm that does not agree with the other two, and it shows the second harm in the issue directly. Base passes via the advisory branch and prints:
[gate-48] NOTE: no CSRF signal was ADDED by this diff, and none was needed…— wrong about what it read. On this branch the NOTE is gone, because the signal was seen.The two removed findings were not "app debt this fix hides": the underlying unprotected callers are still reported by
check_csrf_callers.pywhenever the signal count is genuinely zero. What changed is that the gate no longer says a signal was absent when it was present.