Skip to content

Pin and Decode Git's Quoting in repo_gate.py's ls-files Read - #1873

Merged
ptr727 merged 5 commits into
developfrom
feature/auto-1580
Sep 26, 2026
Merged

ptr727 merged 5 commits into
developfrom
feature/auto-1580

Conversation

@ptr727

@ptr727 ptr727 commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • tracked() now passes -c core.quotePath=true to git ls-files, so the output stays ASCII even when a config sets it to false. New unquote_path decodes each quoted line back to the real name, following prose_lint.py's diff_header_path. Names git quotes are no longer missed by the checks that read files, and an inherited core.quotePath=false no longer crashes the gate.

  • main switches stdout and stderr to backslashreplace, the same as prose_lint.py. A decoded name that is not valid UTF-8 now prints, where before it raised partway through the run.

  • sh() decodes with surrogateescape, so a non-UTF-8 byte in its output cannot raise.

  • New tests in TestQuotedNames and TestQuotedPlainNames cover each symptom: the quotePath=false crash, the shebang check skipping a file whose name git quotes, the strict-stdout crash, and a name quoted because it holds a quote or a backslash. Each test fails when the fix is reverted. A platform whose filesystem refuses the name skips the test.

  • tracked() also decodes git's stderr with surrogateescape. A failure message that echoes a non-UTF-8 root path used to raise; now main returns 2 as intended. This was reported by review and filed as repo_gate.py Decodes ls-files stderr Strictly, So a Non-UTF-8 Root Path Crashes It #1872.

  • New printable() escapes each C0 control, C1 control, and DEL wherever a finding or a note is printed, so a decoded name can no longer add a fake line or send escape codes to the terminal. A related case uses only printable characters: a name starting with :: can still forge a workflow command. That predates this change and is filed as Doc Gates Print Tracked Names Raw, So a Name Starting With :: Forges a Workflow Command #1874.

Closes on promotion: #1580
Closes on promotion: #1872

Verification

  • python3 -m unittest discover -s scripts/tests: 1692 tests OK.
  • ruff format and check, mypy, prose_lint.py --diff origin/develop, repo_gate.py, spec/validate.py, audit.py --selftest, build_dist.py --check: all clean.
  • A local strict review pass is recorded for the final content.

🤖 Generated with Claude Code

ptr727 and others added 2 commits September 25, 2026 21:30
tracked() read git ls-files without pinning core.quotePath, so an
inherited false emitted a non-UTF-8 name raw and the strict decode
raised, while the default quoting yielded an escaped spelling that names
no file on disk, dropping it from every check that reads the file.

Pin core.quotePath=true on the listing and decode each quoted line back
into the real name, the shape prose_lint.py's diff_header_path already
uses. sh() decodes with surrogateescape so a non-UTF-8 byte cannot raise.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…mportable on Windows

A decoded name that is not valid UTF-8 now reaches the finding text, and
a strict stdout raised on it, ending the run part way. Reconfigure both
streams to backslashreplace, as prose_lint.py already does.

Build the test's non-UTF-8 name in setUp so a platform refusing it skips
the class instead of failing the module import, and move the quote and
backslash case into a class that does not depend on that name.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Copilot AI lite review requested due to automatic review settings September 26, 2026 04:34
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: b2d91497-1de3-47cb-b3bc-847211c7372b


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

tracked() can still raise on git ls-files stderr decoding because its subprocess.run(..., encoding="utf-8") call does not set errors="surrogateescape".

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

This PR hardens .github/actions/repo-gate/repo_gate.py against Git path quoting/encoding edge cases by forcing git ls-files to use ASCII-safe quoting and then decoding quoted paths back to their real on-disk names, with targeted tests to prevent regressions.

Changes:

  • Pin core.quotePath=true for git ls-files in tracked() and decode quoted paths via the new unquote_path().
  • Make subprocess/stdout handling more resilient to non-UTF-8 bytes (sh(..., errors="surrogateescape"), stdout/stderr backslashreplace in main).
  • Add regression tests for quoted/non-UTF-8 filenames and quote/backslash quoting behavior.
File Description
.github/​actions/​repo-gate/​repo_gate.py Pins Git quoting for ls-files, decodes C-quoted paths, and makes output handling tolerant of non-UTF-8.
scripts/​tests/​test_repo_gate.py Adds coverage for core.quotePath=false crash prevention, quoted-name coverage, and strict-output printing behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/actions/repo-gate/repo_gate.py Outdated
Git's error for a root that cannot be entered echoes the path raw, and
the strict decode of that stderr raised instead of letting main return 2.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings September 26, 2026 04:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Decoding C-quoted paths can reintroduce literal control characters into paths that are later printed, enabling malformed or injectable gate output unless display escaping is added.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Sanitize decoded path output before formatting gate messages

.github/​actions/​repo-gate/​repo_gate.py:176

Decoding git's C-quoted form here can reintroduce literal control characters (e.g. newline, ESC) into the returned path. Downstream checks interpolate tracked paths directly into human-readable output (for example, check_eol_coverage formats f"{path}: tracked shebang path ..."), so a malicious filename containing control bytes can inject extra lines or terminal escapes into the gate output. Keep the filesystem path decoding, but ensure every path that reaches stdout/stderr is escaped/sanitized for display (for example, a dedicated display helper that uses unicode_escape/backslashreplace for control bytes) before formatting messages.

A decoded tracked name can hold a newline or an escape sequence, which
printed raw forges a line of the gate's output or drives the terminal.
Spell each C0 or C1 control and DEL as an escape where a finding or a
note is printed.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings September 26, 2026 04:48
@ptr727

ptr727 commented Sep 26, 2026

Copy link
Copy Markdown
Owner Author

Sanitize decoded path output before formatting gate messages (repo_gate.py:176, previously missed)

Fixed in f1ba017. A new printable() escapes each C0 control, C1 control, and DEL wherever main prints a finding or a note. test_a_control_character_in_a_name_prints_escaped_rather_than_raw covers this and fails when the fix is reverted. A related case uses only printable characters: a name that begins with :: can still forge a workflow command. That predates this change and is filed as #1874.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

tracked() can still raise on its error-path print when called outside main() because surrogate-escaped stderr may not be encodable on a strict sys.stderr.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity tracked() can raise UnicodeEncodeError when printing surrogate escapes

.github/​actions/​repo-gate/​repo_gate.py:155

tracked() decodes git ls-files stderr with errors="surrogateescape", but on the error path it prints reason directly to sys.stderr. If tracked() is called outside main() (so report_paths_that_are_not_utf8() has not reconfigured stderr), a failure message containing surrogate escapes can still raise UnicodeEncodeError at this print.

Consider making the error message ASCII-safe (for example, UTF-8 encode with backslashreplace) before printing so tracked() is safe regardless of caller.

git's stderr is now decoded with surrogateescape, so a caller that has
not reconfigured stderr could still raise printing it. printable() now
also spells a lone surrogate as an escape, and the failure line goes
through it.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@ptr727

ptr727 commented Sep 26, 2026

Copy link
Copy Markdown
Owner Author

tracked() can raise UnicodeEncodeError when printing surrogate escapes (repo_gate.py:155, previously missed)

Fixed in 1232c19. printable() now also backslash-escapes a lone surrogate, and tracked()'s failure line is printed through it, so the print cannot raise even when tracked() is called outside main. test_a_missing_root_named_in_bytes_that_are_not_utf8_fails_without_raising now uses a strict stderr and fails when the fix is reverted.

Copilot AI review requested due to automatic review settings September 26, 2026 04:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The changes are well-scoped, align with the documented reference implementation in prose_lint.py, and are backed by targeted regression tests for the previously failing cases.

Review effort: Lite
Findings: None

@ptr727
ptr727 merged commit 6e37b7a into develop Sep 26, 2026
9 checks passed
@ptr727
ptr727 deleted the feature/auto-1580 branch September 26, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants