Skip to content

fix: neutralize workflow-command injection in info logs - #776

Merged
EndBug merged 2 commits into
mainfrom
cursor/61cc6e37
Aug 16, 2026
Merged

fix: neutralize workflow-command injection in info logs#776
EndBug merged 2 commits into
mainfrom
cursor/61cc6e37

Conversation

@EndBug

@EndBug EndBug commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • core.info writes raw stdout, so user-controlled inputs (message, author, cwd, etc.) could inject GitHub Actions workflow commands via a newline (::stop-commands::, ::error::, …).
  • Route all info logs through safeInfo(), which applies neutralizeLogString() first. Git commit/author/cwd values are unchanged; only the log line is escaped.
  • Ban direct core.info with ESLint, cover U+2028/U+2029, and add a regression test that the payload is escaped in stdout but kept raw in the commit.

Test plan

  • npm test (78 tests, including the newline + ::stop-commands:: integration case)
  • Confirm a workflow using message: ${{ github.event.issue.title }} no longer emits a :: command line when the title contains a newline

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes

    • Improved log safety by preventing user-provided text from injecting misleading lines or workflow commands into action logs.
    • Sanitized paths, branch names, commit messages, and other displayed values while preserving their original committed content.
  • Tests

    • Added coverage for unsafe Unicode characters and workflow-command payloads in logs.
    • Verified multiline commit messages remain unchanged when committed.

core.info writes raw stdout, so inputs like message could inject
::stop-commands:: via a newline. Route info through safeInfo().

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f3120fa-17e2-45d8-bc00-a5c0bf72793f

📥 Commits

Reviewing files that changed from the base of the PR and between 39f3329 and 2a4d3d6.

📒 Files selected for processing (4)
  • lib/index.js
  • src/util.ts
  • test/integration/action.test.ts
  • test/util.test.ts

📝 Walkthrough

Walkthrough

The change adds safeInfo, expands log-string neutralization, sanitizes user-controlled values in errors, replaces direct informational logging, adds ESLint enforcement, and adds unit and integration coverage.

Changes

Log Safety

Layer / File(s) Summary
Sanitization utilities and validation
src/util.ts, test/util.test.ts
safeInfo now neutralizes unsafe log content. Validation and diagnostic errors sanitize paths, branch names, arguments, and file names.
Informational logging migration
.eslintrc.json, src/io.ts, src/main.ts
Informational output now uses safeInfo. ESLint rejects direct core.info() calls.
Error output sanitization and integration coverage
src/io.ts, src/main.ts, test/integration/action.test.ts
Validation and pathspec errors sanitize interpolated values. Integration tests verify escaped action-log output and unchanged commit content.

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

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 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 primary change: preventing workflow-command injection in informational logs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/61cc6e37

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.

@EndBug
EndBug marked this pull request as ready for review August 16, 2026 23:28
@EndBug
EndBug enabled auto-merge (squash) August 16, 2026 23:38
Rebuild lib/ after overlapping ncc bundle changes and neutralize the
new --pathspec-from-file error string.

Co-authored-by: Cursor <cursoragent@cursor.com>
@EndBug
EndBug merged commit 60ff4bb into main Aug 16, 2026
9 of 10 checks passed
@EndBug
EndBug deleted the cursor/61cc6e37 branch August 16, 2026 23:41
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.

1 participant