Skip to content

Stop the Doc Gates' Printed Names Forging Workflow Commands - #3116

Merged
ptr727 merged 3 commits into
developfrom
feature/auto-1874
Oct 16, 2026
Merged

ptr727 merged 3 commits into
developfrom
feature/auto-1874

Conversation

@ptr727

@ptr727 ptr727 commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

The Actions runner reads a stdout or stderr line as a workflow command in two ways, and a tracked name or a file's own text could reach either one through the output of both doc gates.

  • The current form opens on :: once the runner trims leading whitespace, so the indent in front of a repo_gate.py finding gives no protection.
  • The legacy form is ##[ anywhere in the line. The runner's ActionCommand.TryParse finds it with IndexOf and still runs it when a registered command name follows up to ]. The issue did not name this form, and it is fixed here as well.

Changes

  • repo_gate.py's printable() now spells the colon opening such a line as \x3a, and the bracket of each ##[ as \x5b. That matches how it already spells each control character. Every finding and note passes through it, so no call site changed.
  • prose_lint.py gains the same printable(), control escaping included, and applies it to every line carrying a path or file text. That covers the finding line, --list-files, the scope note, the summary's per-file lines, the filesystem-walk warning, and the error lines that quote paths or the diff base. The finding line opened on the raw tracked name with no indent, so a name opening on :: forged a command there directly. A name holding a newline also printed raw, which let it start a line of its own.
  • The prose gate's composite action echoed each line of the tracked .github/prose-gate-excludes raw, before prose_lint.py ran. A fork controls that file, so an exclusion holding ##[, or a carriage return followed by ::, forged a command there. The echo now spells the bracket as \x5b and a carriage return as \x0d.
  • tests/runner_commands.py models which lines the runner would pass to a command parser. It follows the runner's two parsers rather than the gates' own escaping. Both gates' tests use it, through printable() and through a real run over tracked names built to forge each form. The run covers the finding lines, --summary, --list-files, and a name the scope note prints on stderr. A parity test keeps the two gates' printable() copies equal, since each gate is fetched downstream as one file and cannot import the other.

Design choice

The issue left a choice: neutralize the printed lines, or wrap the findings in a ::stop-commands:: block with a random token. This change neutralizes the lines, for three reasons:

  • Neutralizing covers the legacy ##[ form along with the :: form, in a single helper each gate already routes its output through.
  • The output stays deterministic, so a test can check it without a runner, and a local run shows no token lines.
  • A stop-commands block depends on its resume line being printed. A gate killed mid-run would leave command processing off for the rest of the step, and that includes the composite action's own ::error:: lines.

Verification

  • The new tests fail with the two neutralizing lines removed from both gates (14 failures). Removing a single call site also fails a test: the finding line's, the --summary line's, or the --list-files line's printable(), or either substitution in the action's echo.

  • python3 -m unittest discover -s tests passes with 2867 tests and 7 skipped. ruff format, ruff check, and mypy are clean, as are prose_lint.py --diff origin/develop, repo_gate.py, and spec/validate.py.

  • actionlint, run through scripts/docker_lint.py, is clean.

  • The local strict review ran three rounds. Round 1 found the action's echo and the untested output modes. Round 2 found the carriage-return route. Round 3 raised no findings.

Follow-up

Closes on promotion: #1874

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Text in validation messages and file paths that resembles workflow commands is now displayed safely rather than being interpreted as a command by the automation runner.
    • Control characters and invalid text are escaped in output to prevent unexpected line breaks or command-like behavior. Validation results and outcomes remain unchanged.

The Actions runner reads a line opening on `::` after leading whitespace
is trimmed as a workflow command, and also a line holding `##[` anywhere,
which is its legacy parser. A tracked name or file text shaped like
either reached the output of both doc gates unchanged.

repo_gate.py's printable() now spells the colon opening such a line and
the bracket of each `##[` as escapes. prose_lint.py gains the same
printable(), control escaping included, and applies it to every line
carrying a path or file text, the finding line among them, which opened
on the raw tracked name with no indent and printed a newline in a name
raw.
The composite action echoed each line of the tracked
.github/prose-gate-excludes raw, so an exclusion holding `##[` forged a
legacy workflow command before prose_lint.py ran. The echo now escapes
the bracket the way printable() does.

The tests now cover the --summary and --list-files output and a name
the scope note carries on stderr, and they share their case lists
through tests/runner_commands.py. A parity test keeps the two gates'
printable() copies equal, since each gate is fetched as one file.
The runner ends a line at a lone carriage return, and the trim keeps one
inside an exclusion line, so an exclusion holding one could still open a
workflow command on the line after it. The echo now spells it as \x0d.
Copilot AI lite review requested due to automatic review settings October 16, 2026 03:51
@ptr727

ptr727 commented Oct 16, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head b0ce631d285b0dcdad13145d82066eb36f8963de, the content this pull request carries at that commit against develop, and it recorded 0 findings.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bca81062-8744-41e6-8782-2b9a5e3fbfd1
📥 Commits

Reviewing files that changed from the base of the PR and between c181165 and b0ce631.

📒 Files selected for processing: 6
  • .github/actions/prose-gate/action.yml
  • .github/actions/prose-gate/prose_lint.py
  • .github/actions/repo-gate/repo_gate.py
  • tests/runner_commands.py
  • tests/test_prose_lint.py
  • tests/test_repo_gate.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The prose gate and repository gate escape workflow-command syntax and control characters in printed values. Tests cover the escaping helpers and output generated from command-shaped paths and content.

Changes: Workflow command output escaping

Layer / File(s) Summary
Escape workflow-command syntax
.github/actions/prose-gate/prose_lint.py, .github/actions/repo-gate/repo_gate.py, tests/runner_commands.py, tests/test_repo_gate.py
Both gates escape workflow-command markers in printable output. Shared test helpers identify command-shaped lines, and tests check escaping and consistency between the gates.
Sanitize printed paths and diagnostics
.github/actions/prose-gate/action.yml, .github/actions/prose-gate/prose_lint.py, tests/test_prose_lint.py, tests/test_repo_gate.py
The prose gate and composite action escape paths and diagnostic output. Tests check command-shaped filenames, content, findings, summaries, and file lists.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to b0ce6

No merge-blocking issue is established; the investigated exclusion-log path does not forge a workflow command.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 5 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing Doc Gates output from being interpreted as GitHub Actions workflow commands.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

🟡 Changes recommended

The scan-path summary still prints requested paths raw and can forge a legacy workflow command.

1 open finding
What changed in this PR

Hardens documentation gates against GitHub Actions workflow-command injection in paths, findings, file content, and exclusions.

Changes:

  • Escapes current and legacy command markers in both gates.
  • Sanitizes exclusion output in the composite action.
  • Adds runner-command modeling and regression tests.
File Description
tests/​test_repo_gate.py Tests repository-gate output escaping.
tests/​test_prose_lint.py Tests prose-gate paths, findings, and exclusions.
tests/​runner_commands.py Models runner command parsing.
.github/​actions/​repo-gate/​repo_gate.py Escapes command-shaped output.
.github/​actions/​prose-gate/​prose_lint.py Escapes paths and file text.
.github/​actions/​prose-gate/​action.yml Escapes exclusion output.

🧠 Review effort: Lite


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/actions/prose-gate/action.yml
@codecov

codecov Bot commented Oct 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@c181165). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #3116   +/-   ##
==========================================
  Coverage           ?   63.94%           
==========================================
  Files              ?       17           
  Lines              ?     9145           
  Branches           ?        0           
==========================================
  Hits               ?     5848           
  Misses             ?     3297           
  Partials           ?        0           
Flag Coverage Δ
python-3.13 63.94% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ptr727

ptr727 commented Oct 16, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ptr727
ptr727 merged commit 5d369fd into develop Oct 16, 2026
14 checks passed
@ptr727
ptr727 deleted the feature/auto-1874 branch October 16, 2026 04:24
ptr727 added a commit that referenced this pull request Oct 16, 2026
… Fixes to Main (#3146)

- #3135 makes `gh-write-guard` read a gh write's subcommand and target
the way gh parses them: the subcommand is found past flags before the
command and before the subcommand, an `R` inside a short-option cluster
is read as a target, `GH_REPO` is read as a target, a run-time
repository owner on a write fails closed, and a run-time word where `pr`
or `merge` goes fails closed beside `--admin` or where word splitting
may split it
- #3139 releases the bootstrap lock when a `bootstrap.ps1` run ends
rather than when its process does, so a second run typed at the same
pwsh prompt is no longer refused the lock
- #3130 matches tool names and the `all` keyword by case in the Windows
agents loaders, as the Linux loaders already do
- #3128 reads CodeRabbit's rate limit on a reply to an explicit review
command in `pr_review.py`
- #3126 reconciles VSCode-Server-DotNetCore in the hub after its resync
- #3124 logs the Docker task's build-base leg in to Docker Hub
- #3122 names the carried skill and the skipped review correctly
- #3120 states the subprocess encoding rule in `CODESTYLE.md`, where
carriers read Python style
- #3118 and #3111 move the release-guard tests' workflow-command checks
onto `runner_commands`, and test the alternatives wiring at the
`audit_repo` and `fidelity_pass` call sites
- #3116 stops the doc gates' printed names forging workflow commands
- #3113 adds a catalog snippet for the release-with-smoke caller
- #3109 moves the `workflow_reuse` fetch onto `audit.contents_text`
- #3105 accepts `.codecov.yml` as an alternative to `codecov.yml`
- #3103 adds a pip ecosystem to the carried Dependabot snippet
- #3100 prefers the venv's ruff and pyright for a declared pip directory
- #3098 tracks composite action pins in Dependabot
- #3096 reads an uppercase Markdown suffix as Markdown at every prose
gate branch

After this merges, the host-safety installer payload changes, so the
maintainer runs the installer after promotion.

Closes #1416
Closes #1425
Closes #1578
Closes #1874
Closes #1892
Closes #1899
Closes #2804
Closes #2850
Closes #2852
Closes #2854
Closes #2996
Closes #3065
Closes #3067
Closes #3069
Closes #3079
Closes #3080
Closes #3089
Closes #3093
Closes #3101
Closes #3106
Closes #3107
Closes #3137

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added a pull-request smoke build that runs when relevant release files
change and reports the combined validation and build status.
* Repository audits now recognize `.codecov.yml` as an alternative to
`codecov.yml`.

* **Improvements**
* Python linting and type checks use tools from declared project
environments when available, with fallbacks for missing tools.
* Workflow and lint reports safely display unusual filenames and
command-like text.
* GitHub write-target checks now account for additional command forms
and fail closed when targets cannot be verified.
* Rate-limit detection now recognizes another CodeRabbit response
format.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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