Skip to content

prose_lint.py Numbers Lines From Filtered Content While Reading the File on Disk, So a clean Filter Hides a Violation #1587

Open

Description

@ptr727

Problem

changed_lines in .github/actions/prose-gate/prose_lint.py takes its line numbers from git diff, and check_file reads the file from disk. A filter.<name>.clean rewrites the worktree side before the diff is taken, so the two describe different text: the hunk headers number the filtered content while the gate reads the unfiltered file.

Unlike the settings the invocation already pins, this one cannot be pinned. git diff has no --no-filters, and --no-textconv does not reach it, so the flags that close the neighbouring routes leave this one open.

Evidence

Constructed: a repository whose core.attributesFile names *.md filter=strip, with filter.strip.clean set to a command dropping the first line, and one file gaining a duplicated word on its second line.

changed_lines()   {"doc.md": {1}}
gate run          scope: 1 of 1 file(s) read, 1 changed line(s)   exit 0
the file itself   1  Title.
                  2  It has a dupword dupword here.

The violation is on line 2, scope names line 1, and the run reports full coverage while reading the wrong line. A clean filter that drops the added line outright takes the file out of the diff altogether, leaving an empty scope, and an empty scope is falsy, so main's if scope and not matched refusal does not fire either.

Reach

core.attributesFile is a pure host setting, so a developer carrying one gets this from every local run while CI, which carries none, is unaffected and would never report it. A committed .gitattributes naming a filter reaches the same result for everyone, and a filter is ordinary tooling rather than anything exotic.

This is the more dangerous half of the class, because coverage still reads full. An emptied scope at least leaves a 0 of N file(s) read in the output for a reader to notice.

Suggested direction

No flag closes it, so the choice is between reading what the diff described and refusing when the two can differ.

  • Take the content from the same side the line numbers came from, meaning the filtered text git diff saw, rather than reading the path from disk.
  • Or detect that a changed path has a clean filter attached, via git check-attr filter, and refuse rather than report, on the same footing as the existing unresolvable-base refusal.

The second is the smaller change and states the limit honestly. The first removes the limit but changes what the gate reads for every path.

Scope

Found by a local review pass on the branch closing #1575 and #1576. It predates that branch. Left out of it because every fix in it was a flag on one invocation, and this one is neither a flag nor available as one.

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions