Retrospective: PR #636 (98 NGFW vendor catalog-extension templates) merge review

[tvna]

PR overview

PR #636 (issue #635) extended the 25-item NGFW/firewall operational-problem catalog established by #619/PR #620 with 25 new items (26-50) across Fortinet/Palo Alto Networks/SonicWall/OPNsense, adding 98 templates (25 × 4 vendors, minus 2 items where OPNsense has no valid native mechanism — owner-approved exclusion). Merged 2026-08-02T08:38:49Z by @tvna, head c04cfb5ea9a5151d2a2e56a1aa2d4d9576ba0d09, +22449/-5 across 201 files, 14 commits.

Repairs between PR open and merge

Both repairs below were made in a single commit, c04cfb5, pushed ~11 minutes after PR creation (22:58:40Z → 23:09:48Z) in direct response to CI failures — no repair happened after that commit.

1. CI failure: gitleaks false positives (recurrence of #621's unfixed finding)

Guard API Key (gitleaks, .github/workflows/reusable-test-and-build.yml) flagged 3 false positives on the initial push:

• fortinet-admin-mfa-hardening.yaml's example FortiToken Mobile hardware-token serial numbers (generic-api-key rule, entropy-only match, 2 occurrences)
• opnsense-cloud-vm-form-factor-deployment.j2's curl example using a literal **** password mask (curl-auth-user rule, matches the -u user:pass shape regardless of value)

Fixed by adding 3 documented .gitleaksignore entries, following this repo's established convention for this exact finding class.

Root cause, traced past the immediate fix: this is a direct recurrence of the finding already filed in #621 (PR #620, the direct predecessor to this PR). .pre-commit-config.yaml already configures gitleaks/gitleaks v8.24.2 as a pre-commit hook, but scripts/install-workflow-linters.sh — the SessionStart hook that provisions tooling for CLAUDE_CODE_REMOTE=true sessions — still only installs actionlint and shellcheck (confirmed by reading the current file in this session; unchanged since #621). #621's proposed durable gate (extend that script to install pre-commit and run pre-commit install) was never implemented, so every commit in this PR's session again went through raw git commit/git push with gitleaks silently inert locally, exactly as documented in #621.

Classification: missing deterministic gate — recurrence of #621, still open and unimplemented.

2. CI failure: typo (aci_version → firmware_version) caught only by CI, with zero local coverage

Guard workflow files / Check typos (crate-ci/typos@bee27e3a / v1.48.0, .github/workflows/reusable-test-and-build.yml:258-259) flagged aci_version/"ACI Version" in sonicwall-vdom-multi-tenant-segmentation.{j2,yaml} as a likely "ACPI" typo. The field actually holds a SonicOS build string ("SonicOSX 7.0.1-5165"); "ACI" had no relation to it. Fixed by renaming to firmware_version, matching the design doc's own wording and sibling SonicWall templates (sonicwall-ha-config-sync-drift.toml, sonicwall-snmp-monitoring-integration.yaml).

Root cause, traced past the immediate fix: unlike gitleaks/ruff/mypy, crate-ci/typos has no hook entry at all in .pre-commit-config.yaml (confirmed by reading the file in this session). A _typos.toml config exists at the repo root, but nothing wires it into a local hook. So this gap is independent of finding #1 above: even after #621's proposed pre-commit-activation fix ships, this specific check would still only run in CI, never locally, because the hook itself doesn't exist yet.

Classification: missing deterministic gate — new gap, not previously tracked.

Durable gates proposed

1. (Restates Retrospective: PR #620 (75 NGFW vendor-parity templates) merge review #621's still-open proposal) Extend scripts/install-workflow-linters.sh (or a sibling SessionStart hook, same pattern: pinned-tool install, CLAUDE_CODE_REMOTE-gated, idempotent) to install pre-commit and run pre-commit install so the already-configured gitleaks/ruff/mypy/lizard/ci-parity.sh hooks execute on every commit/push from a Claude Code web session, not just in CI.
2. (New) Add a crate-ci/typos hook to .pre-commit-config.yaml, pinned to the same ref CI uses (bee27e3a4fd1ea2111cf90ab89cd076c870fce14 / v1.48.0) so local and CI typo-checking stay byte-identical, per this repo's existing pin-parity convention (see flake_pin.py's actionlint/shellcheck handling).

Not implemented in this session — both are bootstrap/session-infrastructure and shared-config scoped, analogous to #621's own deferral rationale. Filing here so neither is lost; a follow-up PR can implement both together using install-workflow-linters.sh's existing idempotent-install pattern as the template for gate 1, and a straightforward .pre-commit-config.yaml addition for gate 2.

Classification summary

• Repair Bump actions/setup-node from 1 to 4 #1 (gitleaks false positives): missing deterministic gate — recurrence of Retrospective: PR #620 (75 NGFW vendor-parity templates) merge review #621, still unimplemented after two PRs.
• Repair Bump actions/checkout from 1 to 4 #2 (typo caught only by CI): missing deterministic gate — new gap, no prior tracking.

Refs #635, #621, #619, #636.