Modernize Go code with go fix - #14278

…paths (#4)

* feat: add a code-based auto-merge guard, independent of the prompt text

The "never auto-merge payment/auth/migration/secrets" rule has so far
only existed as text Claude reads in the fix prompt — useful, but a
model can misjudge it. This adds a second, mechanical line of defense:
.github/workflows/auto-merge-guard.yml runs on any PR that has
auto-merge enabled, diffs the actual changed files against a
customizable regex (MURAQIB_SENSITIVE_PATHS repo variable, sensible
default otherwise), and force-disables auto-merge + comments if it
matches — regardless of what the PR author decided.

The pattern-matching logic itself is unit tested
(scripts/sensitive-path-pattern.test.mjs, 3 cases including a documented
false-positive trade-off: a harmless file merely named after a sensitive
topic still gets flagged on purpose, since a few minutes of review costs
less than missing a real one). The webhook-triggered half (does GitHub
actually fire this on a real auto-merge-enabled PR, does gh pr merge
--disable-auto really take effect) can't be verified without a live PR
against this repo — not done as part of this commit, noted here so it
isn't mistaken for having been end-to-end tested.

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

* fix: rebuild the auto-merge guard after adversarial review found it did 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>

* fix: pagination crash + two secondary gaps found in round-2 review

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>

* fix: require a real path boundary before "secrets", not a bare substring

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>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>