Skip to content

marketplace: two unconditional in-place rewriters race on every Write with no defined precedence, nobody sums the always-on per-turn cost, and thirteen CRITICALs need an ordering #1809

Description

@kyle-sexton

Filed by an agent session that audited six components in one run. These three items belong to no single plugin — each was noticed by two or more audits and owned by none. Following the precedent in issue #1682, which bundles cross-plugin observations "rather than silently mis-filed" and invites a maintainer to split them.

1. Two unconditional in-place rewriters race on every Write, and nobody owns it

typos-format 0.4.2 and markdown-format 0.8.4 both register PostToolUse with matcher: "Write|Edit", unconditionally, and both rewrite file content in place.

The hooks reference states matching hooks "run in parallel, and identical handlers are deduplicated automatically", and separately that when multiple hooks return competing results "the last one to finish takes effect. Since hooks run in parallel, the order is non-deterministic."

So on any Markdown write, two independent processes read, rewrite, and write back the same file with no defined precedence and no locking.

Both audits noticed this. typos-format discloses it in its own script header and README as a fleet concern; markdown-format's audit reached the same conclusion. Both classified it as something neither plugin can solve alone — so it is recorded twice and owned zero times, which is precisely how a defect survives an audit.

Disclosure is honest but is not a fix. This needs a named owner and one of: a shared advisory lock, a declared ordering, a single serialising formatter dispatcher, or an explicit decision that last-writer-wins is acceptable and why.

Observed evidence that the risk is not theoretical: during this audit both hooks mutated the same class of files, and three separate content-destroying rewrites were captured — a dictionary autocorrect renaming Win32 identifiers, MD004 falsifying a quoted changelog line, and MD018 destroying a line-leading issue reference while fabricating a heading. Each was individually recoverable; two of them landing on one file in undefined order is not something any audit has bounded.

2. Nobody sums the always-on cost, and the consumer is the one who feels it

This single session carried, on every turn or every tool call, in every consumer session regardless of whether the plugin was in use:

  • disk-hygiene's plugin-level engine gate — on every Bash and PowerShell command. Its marker-free fallback calls os.path.samefile on every separator-carrying word of every command, including commands with no relation to the plugin. Its own docstring names this as the strongest candidate for an observed 17-second stall.
  • disk-hygiene's Stop detector — once per turn. Its amortisation claim holds only after a failure has been reported, because the marker is written only when a warning fires. On the happy path every turn pays interpreter startup (the plugin's own ADR measures python3 -c 'pass' at 396 ms on this host) plus a seek-and-read of up to 2 MB.
  • four guardrails classifiers — on every Bash and PowerShell command.
  • two PostToolUse formatters — on every Write and Edit.

Each plugin accounts for its own share honestly where it accounts at all. Nobody adds them up. A per-turn and per-tool-call cost budget across the marketplace is the missing question, and the aggregate is what a user experiences — the 17-second stall that disk-hygiene's guard docstring investigates is a symptom of the sum, not of any single component.

Suggested shape: a stated budget (milliseconds per turn, milliseconds per tool call) that every always-on hook must fit inside, plus a measured accounting per plugin against it. Without a shared ceiling, each plugin's individually-reasonable cost composes into something no one reviewed.

3. Sequence the batch — this is worth more than the individual reports

This audit produced thirteen CRITICALs across five plugins, all destined for this repository at once, with no ordering. Several of the highest-value fixes are one-liners, and none requires agreeing with the deeper structural findings first:

  1. Flip typos_format_write_changes to false in typos-format's plugin.json. Write mode becomes opt-in. Removes the entire silent-corruption class immediately — including the already-filed issue typos-format: hook silently corrupts abbreviated git SHAs, including inside backticks #1257 — while preserving the zero-config reporting win that 0.2.0 was protecting. Verified working: with it false, the file is byte-identical afterwards and findings still surface.
  2. Make markdown-format report-only when no markdownlint config is discoverable. This is the same author's own doctrine elsewhere in this marketplace: bash-format refuses to run when the repo has no shfmt config, "so the plugin never imposes a style you did not [choose]". Applying it here closes the quote falsification, the issue-reference destruction, the unreachable remediation advice, and roughly 115 unactionable MD013 findings in one change, using machinery collect_risky_configs already has.
  3. Give block-noncanonical-commit an exit 0 on rc=2, following the precedent already present at block-convention-violation.sh:375-383. Coverage cost is provably zero — both guards call ps::classify_git_command with identical arguments, corroborated across a 12-case corpus where every commit-guard block was also a dangerous-git block carrying that guard's accurate message.

An ordered "these three land most of the CRITICAL surface today" list is more actionable than five independent reports arriving together.

One hard ordering constraint

A typos-format audit found a write-bypass leak in guardrails' block-hook-bypass.sh: a Python one-liner that opens a path and writes it evades the Write/Edit PostToolUse hooks entirely, while the guard refuses the echo/printf redirection form.

Closing that leak removes the last escape hatch from typos-format's unconditional write mode. So item 1 must land before the bypass is closed, or the situation gets strictly worse for anyone whose vocabulary the dictionary mangles.

4. A process finding, since it affects every future audit in this repo

There is no verification step anywhere between "auditor writes evidence packet" and "drafter reads packet", while the plugin-quality workflow instructs a fresh auditor to treat the packet as ground truth for behavioural claims.

In this run: packet files were mutated in place by sibling hooks three times; one packet's evidence.md carried three false factual claims; and an independent review gate found four further factual errors that had survived into the locked-contract layer in two cases. One of those was load-bearing — an incorrect explanation of a measured attribute discrepancy that would have caused a maintainer to dismiss a CRITICAL on first verification.

Content-hash-at-write-time is the right primitive and is currently one clause inside a bundled remediation in the plugin-quality issue. Raising it here because the exposure is not specific to that plugin: any workflow in this marketplace that writes durable evidence for a later reader, while unconditional in-place formatters are installed, has the same hole.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions