fix(typos-format): gate --write-changes on extension allowlist

[kyle-sexton]

Closes #2650

Summary

Gates opt-in --write-changes on an explicit source/prose/config extension allowlist so unknown extensions, extensionless paths, and fixture/lock-adjacent types stay report-only even when typos_format_write_changes is true.

Fix

• Add typos_write_ext_allowed allowlist gate before --write-changes
• Keep the read-only scan language-agnostic; denied paths still report findings with an allowlist skip note
• Stub contract coverage for .snap / extensionless deny and .md allow
• Bump typos-format to 0.6.16 with CHANGELOG + README

Verification

bash plugins/typos-format/hooks/typos-format.test.sh
# PASS=112 FAIL=0

Related

Refs the silent --write-changes tool behavior documented in the hook comments.

  

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 1m 45s —— View job

---

Security review (/review:security-review)

• Gather context (PR diff, changed files)
• Understand the request (/review:security-review)
• Review changed files for security issues
• Post findings / summary

No security issues found.

This PR is itself a hardening fix (closes #2650): it gates typos-format's opt-in --write-changes mutation behind an explicit extension allowlist (typos_write_ext_allowed) at plugins/typos-format/hooks/typos-format.sh#L231-L262, applied at L258-L262.

Reviewed for logic/trust-boundary issues (not statically-catchable style/lint noise):

• Fail-closed by design: the case match is case-sensitive (no nocasematch/extglob set) and extensionless paths fall through to the deny arm, so an unmatched or oddly-cased extension (.MD, LICENSE, .env, .pem, .lock) stays report-only rather than silently being writable — the safe direction for an unrecognized pattern.
• No new mutation surface: the allowlist only narrows which files --write-changes can touch versus current main (previously unrestricted); it does not widen write access to anything that wasn't already writable, and it doesn't change the read-only scan path.
• No injection surface: the gate is a pure Bash case pattern match against $FILE, not eval/interpolated into a command — a filename with shell/glob metacharacters is only ever compared, never executed.
• Double-extension edges (e.g. foo.sh.md) key off the real trailing extension, consistent with how the file will actually be interpreted — not a bypass of the allowlist's intent.
• Version bump, CHANGELOG, README, and new .snap/extensionless-deny/.md-allow contract tests in the diff are documentation/test-only and introduce no new runtime surface.

Per the security-review skip criteria, supply-chain/permissions/pinning concerns are out of scope for this lane (zizmor's advisory lane) — not applicable here regardless, as no workflow or dependency files changed.

Note: I was unable to execute the hook's test suite (bash plugins/typos-format/hooks/typos-format.test.sh) in this sandbox due to command-execution restrictions, so the PR's own stated PASS=112 FAIL=0 verification was not independently re-run — the code-level review above is unaffected by this.

[github-actions]

Last security-reviewed head: 4ecf88e3049aca3b81c1414e823c3657049fa2bf. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @kyle-sexton's task in 1m 2s —— View job

---

Security review

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for security-relevant surface
• Adversarial validation of any candidates
• Post final findings

No security issues found.

This PR narrows an existing write path rather than expanding one: --write-changes (a silent mutator — typos-format.sh:270-276) is now gated by an explicit extension allowlist (typos_write_ext_allowed, typos-format.sh:231-262) before it's allowed to run, closing off unknown/extensionless/fixture-adjacent paths from silent autocorrect even when the opt-in is on.

Checked for:

• Injection: the allowlist is a plain bash case/glob match against $FILE — no eval, no string interpolation into a shell command. $FILE/$TYPOS_ARG are passed as quoted arguments to typos throughout, unchanged by this diff.
• Fail-open risk: the gate is deny-by-default (case ... *) return 1), and the match is case-sensitive, so an unexpected extension (or unusual casing) degrades to report-only rather than to a write — the safe direction.
• Scope of the allowlist: source/prose/hand-edited config extensions only; explicitly excludes lock/binary/cert/fixture types per the PR description, consistent with the code.
• No permission, workflow, or dependency-pin changes — this PR touches only the plugin hook, its test, README/CHANGELOG, and a version bump in plugin.json.

No logic flaws, trust-boundary gaps, or supply-chain concerns identified in the diff.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ecf88e304

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 7m 43s —— View job

---

Code review complete

• Gather context (PR diff, CLAUDE.md, REVIEW.md)
• Apply skip gate (PR is open, not draft, meaningful diff — review proceeded)
• Review changed files for correctness / maintainability
• Post findings

Summary

Reviewed the diff (git diff origin/main...HEAD) across all 5 changed files. The allowlist-gating logic itself (typos_write_ext_allowed, the WRITE_SKIPPED_EXT flag, and the report-only messaging branch in typos-format.sh#L534-L539) is sound, and the new test cases (.snap deny, extensionless deny, .md allow) exercise it correctly. plugin.json, CHANGELOG.md, and README.md are consistent with the behavior change.

One blocking finding, posted inline on typos-format.sh#L242:

The allowlist's case statement has a bash syntax error — \ # spellchecker:disable-line is a backslash followed by a space and a comment, not a valid line-continuation (the \ must be immediately followed by the newline). Every other continuation line in that same block ends bare in \. This leaves the pattern list unterminated and, on my static read, prevents the whole script from parsing — breaking the hook entirely (both the write-mode gate and the always-on read-only scan), not just the new allowlist behavior.

I could not run bash -n or the test suite myself in this review sandbox (only read-only git commands are permitted here), so I can't independently confirm the author-claimed PASS=112 FAIL=0 test run — that claim is unverified from my side. Please run bash -n plugins/typos-format/hooks/typos-format.sh locally to confirm before merging; if it does error, the suggested one-line fix on the inline comment resolves it (drop the stray backslash — a bare trailing | already continues a case pattern across lines).
· Branch

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 4m 38s —— View job

---

Code review (/review:code-review)

• Gather context (PR diff, changed files)
• Apply skip gate
• Review changed files for correctness/maintainability
• Adversarial validation of candidates
• Post findings

Scope: correctness/maintainability only — security is scoped to the claude-security-review lane, which already reviewed this PR twice (both clean, though the second pass predates the lockfile-basename fix now in 989e6f7).

Findings

1 finding posted inline on typos-format.sh:552: the report-only disclosure message always says "this extension is outside the write allowlist," but WRITE_SKIPPED_EXT (set at L274-278) covers three distinct deny reasons — unknown extension, no extension, and known-lockfile-basename (e.g. package-lock.json, whose .json extension is allowlisted). For the lockfile case specifically, the message blames "the extension" when it's really the basename denylist, which works against this hook's own stated goal of a trustworthy disclosure channel, and isn't caught by the new #2650 tests (they only grep for the substring allowlist, not the specific wording).

Not filed as a finding, noted for awareness: typos_write_lockfile_denied (L234-245) matches basenames case-sensitively (bash case, no nocasematch). For most listed lockfiles this is moot — their extensions (.lock, .sum, .resolved) already fail typos_write_ext_allowed regardless of case. But for the .json/.yaml ones (package-lock.json, pnpm-lock.yaml, packages.lock.json, project.assets.json), a differently-cased basename would skip the lockfile denylist and still pass the extension allowlist, becoming writable. Real-world impact is low — npm/pnpm/etc. always emit these with their canonical exact casing, and any renamed copy wouldn't function as a real lockfile anyway — so this is a defense-in-depth gap, not a live vulnerability.

Everything else checked out:

• Doc/version consistency: plugin.json (0.6.15 → 0.6.16), CHANGELOG.md, and README.md all accurately describe the new gate and stay in sync with the code.
• Gate ordering is correct: WRITE_CHANGES is downgraded to false before both the scan and write passes, so a denied file never reaches --write-changes, and the read-only scan (unaffected by the gate) still runs and reports findings.
• New tests (.snap deny, package-lock.json deny, extensionless LICENSE deny, .md allow) exercise the gate's main branches and pass per the PR's own stated run — this could not be independently re-executed in this sandbox (no shell access beyond git/gh in this lane), so that pass/fail is author-claimed, not independently verified here.

[claude]

Claude finished @cursor[bot]'s task in 5m 12s —— View job

---

Code review (/review:code-review)

• Gather context (PR diff, CLAUDE.md, REVIEW.md, prior review threads)
• Apply skip gate
• Review changed files for correctness/maintainability
• Adversarial validation of candidates
• Post findings

Scope: correctness/maintainability only — security stays with the claude-security-review lane, which has already reviewed this PR twice (both clean).

Summary

This is a re-review at the current tip (5445681, "docs: update write-skip comment for WRITE_SKIP_REASON"), after the two blocking issues found in earlier rounds on this PR were fixed:

• The bash syntax break on the *.edn continuation line (backslash-space-comment) — *.edn now sits on its own complete case arm (typos-format.sh:248), and every remaining multi-line pattern-list line ends bare in \ with no trailing comment. I traced the continuations by hand and confirmed the pattern list closes correctly; I could not run bash -n/the test suite myself in this sandbox (only read-only git and gh pr diff are available here), so this is a static read, not an independent execution — please keep the author's bash -n + PASS=112 FAIL=0 verification as the authoritative check.
• The lockfile bypass (package-lock.json, pnpm-lock.yaml, etc. slipping through the *.json/*.yaml allowlist) — typos_write_lockfile_denied() now runs before the extension allowlist (typos-format.sh:231-244), with a dedicated deny test (stub/write-lockfile-deny).
• The disclosure-message misattribution (always blaming "the extension" even for lockfile/extensionless denies) — WRITE_SKIP_REASON now threads through lockfile / extensionless / extension to distinct wording (typos-format.sh:562-576).

I reviewed the current diff (gh pr diff 2653, all 5 files) for new issues and found none that clear the high-signal bar:

• Gate ordering is correct: WRITE_CHANGES is downgraded to false before both typos_write_lockfile_denied and typos_write_ext_allowed are consulted, and before the write pass runs — a denied file never reaches --write-changes, and the read-only scan is unaffected.
• New tests (.snap deny, package-lock.json deny, extensionless LICENSE deny, .md allow) exercise the three WRITE_SKIP_REASON branches and the positive-control path. Pass/fail is author-claimed (PASS=112 FAIL=0) and not independently re-run here.
• plugin.json (0.6.15 → 0.6.17), CHANGELOG.md, and README.md stay in sync with the behavior change across both fix commits.
• Checked two edge cases by hand, neither rises to a blocking finding: a hidden dotfile (e.g. .env) gets labeled extension rather than extensionless in the skip message (cosmetic only — it's still denied either way), and the new extensionless-check recomputes base=$(basename -- "$FILE") where an existing $BASE likely already holds the same value earlier in the script — a trivial redundant call, not a correctness issue.

No inline comments filed — nothing here would block on a careful senior read.