Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ def _relay_cli_notices(text: str) -> None:
line = line.strip()
if line and line not in _RELAYED_NOTICES:
_RELAYED_NOTICES.add(line)
print(f"commit-check: {line}", file=sys.stderr)
print(f"commit-check: {_log_safe(line)}", file=sys.stderr)


def run_check_json(
Expand Down Expand Up @@ -905,6 +905,39 @@ def _annotation_escape(text: str) -> str:
return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")


#: C0 and C1 control characters, and DEL, except the newline and tab that a
#: multi-line commit message legitimately carries.
_CONTROL_CHARS = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]")


def _log_safe(text: str) -> str:
"""Make commit-controlled text inert before it is printed to the step log.

The tree prints values from the CLI's JSON, and those are whoever wrote
the commit, branch or pull request title: a multi-line value (a whole
message on a failing rule, a sign-off fix) prints each further line on a
line of its own. The runner reads any log line beginning with ``::`` as a
workflow command, after skipping leading whitespace, so indenting it is
not enough; and it reads the older ``##[command]`` form anywhere in a
line. Left as they are, such lines can forge annotations or mask log
text. So such a line gets a backslash before its ``::``, ``##[`` is broken
up, and control characters are shown as ``\\xNN`` escapes. This is the
CLI's own ``printable()`` (commit-check#620), which covers its text
output but not the JSON this action renders itself.

Only the step log needs it: the Markdown report and the annotation
payloads are not read as commands.
"""
if text.isprintable() and "::" not in text and "##[" not in text:
return text # nothing to escape: the common case
text = _CONTROL_CHARS.sub(lambda m: f"\\x{ord(m.group()):02x}", text)
text = text.replace("##[", "#\\#[")
return "\n".join(
line.replace("::", "\\::", 1) if line.lstrip().startswith("::") else line
for line in text.split("\n")
)


def _annotation_body(scope: ScopeResult, check: dict[str, str]) -> str:
"""The message of one finding's annotation, before escaping.

Expand Down Expand Up @@ -949,7 +982,7 @@ def render_step_log(results: list[ScopeResult]) -> None:
for group_name, scopes in _grouped(results):
print(f"::group::{group_name}")
for line in _render_scopes(scopes, include_docs=True):
print(line)
print(_log_safe(line))
print("::endgroup::")

errors: list[tuple[str, str]] = []
Expand Down Expand Up @@ -1004,7 +1037,7 @@ def render_step_log(results: list[ScopeResult]) -> None:
if total and skipped == total:
print("\u2298 commit-check: all checks skipped, nothing was validated")
if _skip_reasons(results):
print(_skip_reasons(results))
print(_log_safe(_skip_reasons(results)))
elif warned or skipped:
passed = total - skipped - warned
tail = []
Expand All @@ -1016,7 +1049,7 @@ def render_step_log(results: list[ScopeResult]) -> None:
f"\u2714 commit-check: {passed} of {total} checks passed, {', '.join(tail)}"
)
if _skip_reasons(results):
print(_skip_reasons(results))
print(_log_safe(_skip_reasons(results)))
else:
print("\u2714 commit-check: all checks passed")

Expand Down
44 changes: 44 additions & 0 deletions main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,50 @@ def test_failure_prints_group_and_error_annotation(self):
output,
)

def test_commit_text_cannot_issue_workflow_commands(self):
# A failing rule prints the whole message, one line of the log per
# line of the message. The runner skips leading whitespace before
# looking for "::", and reads "##[" anywhere in a line.
scope = main.ScopeResult(
label="Commit 1/1",
checks=[
make_check(
"message",
status="fail",
rule_id="CC001",
value=(
"bad subject\n\n"
"::warning title=Forged::text\n"
" ::add-mask::secret\n"
"old ##[error]form\n"
"esc\x1b]52;c;eA==\x07"
),
error="The commit message should follow Conventional Commits.",
)
],
)
output = self._run([scope])
tree = output.split("::endgroup::")[0]
for line in tree.splitlines()[1:]: # past ::group::, our own command
self.assertFalse(line.lstrip().startswith("::"), line)
self.assertNotIn("##[", line)
self.assertNotIn("\x1b", line)
self.assertIn(" \\::warning title=Forged::text\n", tree)
self.assertIn(" \\::add-mask::secret\n", tree)
self.assertIn(" old #\\#[error]form\n", tree)
self.assertIn(" esc\\x1b]52;c;eA==\\x07\n", tree)
# The annotations are commands already; their payload is left as is.
self.assertIn("%0A::warning title=Forged::text%0A", output)

def test_log_safe_leaves_ordinary_text_alone(self):
for text in [
"feat: add login",
"value: a::b",
" ✔ Branch (feature/x)",
"tab\there",
]:
self.assertEqual(main._log_safe(text), text)

def test_annotation_names_the_commit_and_carries_the_fix(self):
output = self._run([fix_scope("Commit 2/3", sha=SHA_B)])
self.assertIn(
Expand Down
Loading