feat: code-based auto-merge guard for payment/auth/migration/secrets paths - #4

…id not work

Ran the first version through a dedicated adversarial + edge-case review
before pushing anywhere. Both reviewers independently found it
fundamentally broken, not just rough around the edges:

1. Used `pull_request` instead of `pull_request_target` — meant a PR could
   weaken the guard (empty its pattern, disable the job) in the same diff
   as a sensitive-path change, and get checked against its own
   already-neutered copy. `pull_request_target` always reads the workflow
   from the base branch, which the PR can't alter.
2. The unit tests exercised a JS RegExp hand-duplicated from a separate
   `grep -E` (POSIX ERE) string actually used in the workflow — the two
   dialects can disagree, so green tests didn't guarantee the production
   bash behaved the same way.
3. An invalid custom MURAQIB_SENSITIVE_PATHS pattern failed OPEN (grep
   silently treated it as "no match") instead of blocking.
4. The pattern never actually included "auth" despite every doc claiming
   it covered auth changes.
5. No documentation of the real requirement: this only actually blocks a
   merge if configured as a required status check in branch protection,
   since the job itself runs async and can't stop native auto-merge from
   completing first.

Rebuilt: all matching logic now lives in one place
(scripts/check-sensitive-paths.mjs), imported by both the workflow (via a
plain `node` invocation, no more grep) and its own test suite
(check-sensitive-paths.test.mjs, 7 cases: default-pattern matches
including auth and the workflow file itself, unrelated files pass, empty
diff passes, invalid custom pattern fails closed, valid custom pattern
overrides the default, non-ASCII filenames match correctly, matching is
case-insensitive). Changed files are read via the GitHub API, never by
checking out the PR's own ref. README/SECURITY.md/LESSONS.md updated to
document the required-status-check requirement and the full history of
what was wrong with the first version.

The webhook-triggered half (does pull_request_target actually behave as
documented on a live PR, does the required-status-check race close in
practice) still can't be verified without a real test PR against this
repo — noted explicitly, not claimed as tested.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Both reviewers in the second review round independently found and
reproduced the same critical bug against real large PRs (nodejs/node#64199,
293 files; cli/cli#14278, 252 files): `gh api ... --paginate --jq
"[.[].filename]"` crashes on any response spanning more than one page,
because gh applies --jq per-page before merging, producing several
concatenated JSON array literals instead of one valid document. The
uncaught JSON.parse meant the guard crashed before it could evaluate
whether the PR even needed blocking — on precisely the large-refactor PRs
where an accidental sensitive-path touch is most likely.

Fixed by dropping --jq entirely from both --paginate calls (file list and
existing-comments lookup) and moving the parsing into two testable
functions: parsePaginatedArrayOutput (defensively flattens an
array-of-page-arrays, in case anything ever produces that shape again)
and extractCheckablePaths (also pulls previous_filename for renamed
files, closing a smaller round-2 finding: a rename with no content change
would otherwise evade the guard under its old, possibly-sensitive name).

Verified against real production data, not just reasoning: reproduced the
old crash and confirmed the fix against the actual nodejs/node#64199 PR
(293 files) before writing this commit.

Also fixed: secrets?[._-] required "secrets" to be followed immediately
by ".", "_" or "-" — a bare secrets/ directory (k8s/secrets/prod.yaml)
slid through undetected. Widened to secrets?(/|[._-]|$).

6 new test cases (13 total in this file): multi-page parsing, defensive
flatten, malformed-JSON still throws (not silently empty), rename
old-path inclusion, bare secrets/ directory, and one true end-to-end case
combining all of the above against a simulated 151-file PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Round-3 review confirmed the pagination fix holds (re-verified against
the real 293-file PR, 18/18 tests green) and found one remaining gap:
secrets?(/|[._-]|$) fixed the right-hand boundary (a bare secrets/
directory now matches) but never required a boundary on the left, so
"notsecrets.txt" or "topsecretsauce.md" would also trip the guard.

Fails toward caution, not away from it (unnecessary manual review, not a
missed real secret), so this was assessed as low-severity — fixed anyway
since it was already found and the correction is a one-line, fully
verified change: (^|[/._-])secrets?(/|[._-]|$). New test covers both
directions (false positives removed, true positives retained).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>