-
Notifications
You must be signed in to change notification settings - Fork 1
fix(workflows): isolate free-text workflow inputs #3020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
17d595c
fix(workflows): isolate free-text workflow inputs
codex-automation d41b32a
chore(autofix): formatting/lint
github-actions[bot] 429eee3
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] b6c7960
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 52c8479
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 4287d60
fix(workflows): harden interpolation guards
codex-automation 3c2bb52
chore(autofix): formatting/lint
github-actions[bot] 37dee28
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 25fceb6
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 3547f91
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 151b7a7
chore(codex-autofix): apply updates (PR #3020)
github-actions[bot] 925e3f6
test(workflows): parse quoted Actions expressions safely
codex-automation 0cee600
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] dd13890
chore(codex-autofix): apply updates (PR #3020)
github-actions[bot] 03d990e
test(workflows): exercise untrusted-input detection per expression
codex-automation c3e71dc
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] bba6985
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 464e6ec
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 752a3ad
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 64e51fa
chore(codex-autofix): apply updates (PR #3020)
github-actions[bot] 0d470a8
chore(workflows): remove unrelated runner timestamp
codex-automation 8c4f781
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] a712232
chore(workflows): drop out-of-scope runner artifact from PR diff
codex-automation f207448
chore(codex-keepalive): apply updates (PR #3020)
github-actions[bot] 2be5c1c
fix(workflows): keep worker artifact out of issue diff
codex-automation File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| """Regression guard for free-text Actions values embedded in scripts. | ||
|
|
||
| Workflow expressions are evaluated before a shell or github-script body runs. | ||
| The listed values are free-form workflow-dispatch or runner inputs, so placing | ||
| them directly in a ``run:``/``with.script:`` body can turn quotes or shell | ||
| metacharacters into source code. They must cross that boundary through a | ||
| step-level ``env:`` value instead. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| from collections.abc import Iterable | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
| import yaml | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[2] | ||
| WORKFLOW_GLOBS = (".github/workflows/*.yml", ".github/workflows/*.yaml") | ||
|
|
||
| # Keep this deliberately small. Other expressions require a file-by-file | ||
| # constrained-value review; expanding this set is not a substitute for that | ||
| # review. | ||
| UNTRUSTED_EXPRESSIONS = frozenset({"inputs.commit_message", "inputs.codex_args", "inputs.repos"}) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def _workflow_paths() -> Iterable[Path]: | ||
| for pattern in WORKFLOW_GLOBS: | ||
| yield from sorted(ROOT.glob(pattern)) | ||
|
|
||
|
|
||
| def _script_values(path: Path) -> Iterable[tuple[str, str]]: | ||
| data = yaml.safe_load(path.read_text(encoding="utf-8")) or {} | ||
| for job_name, job in (data.get("jobs") or {}).items(): | ||
| if not isinstance(job, dict): | ||
| continue | ||
| for index, step in enumerate(job.get("steps") or []): | ||
| if not isinstance(step, dict): | ||
| continue | ||
| run = step.get("run") | ||
| if isinstance(run, str): | ||
| yield f"{job_name}/step-{index}/run", run | ||
| script = (step.get("with") or {}).get("script") | ||
| if isinstance(script, str): | ||
| yield f"{job_name}/step-{index}/with.script", script | ||
|
|
||
|
|
||
| def _actions_expression_bodies(script: str) -> Iterable[str]: | ||
| """Yield Actions expression bodies without ending quoted brace literals early.""" | ||
|
|
||
| start = 0 | ||
| while (opening := script.find("${{", start)) != -1: | ||
| index = opening + 3 | ||
| quote: str | None = None | ||
| escaped = False | ||
| while index < len(script) - 1: | ||
| character = script[index] | ||
| if quote: | ||
| if escaped: | ||
| escaped = False | ||
| elif character == "\\": | ||
| escaped = True | ||
| elif character == quote: | ||
| quote = None | ||
| elif character in {"'", '"'}: | ||
| quote = character | ||
| elif script[index : index + 2] == "}}": | ||
| yield script[opening + 3 : index] | ||
| start = index + 2 | ||
| break | ||
| index += 1 | ||
| else: | ||
| start = opening + 3 | ||
|
|
||
|
|
||
| def _references_untrusted_input(body: str, expression: str) -> bool: | ||
| """Recognize equivalent property and bracket references in an expression.""" | ||
|
|
||
| _, property_name = expression.split(".", maxsplit=1) | ||
| return bool( | ||
| re.search( | ||
| rf"\binputs\s*(?:\.\s*{re.escape(property_name)}\b|\[\s*['\"]{re.escape(property_name)}['\"]\s*\])", | ||
| body, | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ) | ||
|
|
||
|
|
||
| def _untrusted_references(script: str) -> list[str]: | ||
| """Return free-text inputs referenced anywhere in Actions expressions.""" | ||
|
|
||
| expression_bodies = list(_actions_expression_bodies(script)) | ||
| return sorted( | ||
| expression | ||
| for expression in UNTRUSTED_EXPRESSIONS | ||
| if any(_references_untrusted_input(body, expression) for body in expression_bodies) | ||
| ) | ||
|
|
||
|
|
||
| def test_no_untrusted_expressions_in_script_bodies() -> None: | ||
| """Free-text inputs must not be interpolated into shell or JS source.""" | ||
| violations: list[str] = [] | ||
| for workflow in _workflow_paths(): | ||
| for location, script in _script_values(workflow): | ||
| for expression in _untrusted_references(script): | ||
| violations.append(f"{workflow.relative_to(ROOT)}:{location}: {expression}") | ||
| assert not violations, ( | ||
| "Pass untrusted workflow values through step env and consume the env " | ||
| "variable in the script:\n" + "\n".join(violations) | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("body", "expression", "expected"), | ||
| [ | ||
| ("inputs.commit_message", "inputs.commit_message", True), | ||
| ("inputs['commit_message']", "inputs.commit_message", True), | ||
| ("inputs.repos || 'all'", "inputs.repos", True), | ||
| ("inputs['repos']", "inputs.repos", True), | ||
| ("inputs.codex_args", "inputs.codex_args", True), | ||
| ("inputs.safe_field", "inputs.commit_message", False), | ||
| ], | ||
| ) | ||
| def test_references_untrusted_input(body: str, expression: str, expected: bool) -> None: | ||
| assert _references_untrusted_input(body, expression) is expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("expression", sorted(UNTRUSTED_EXPRESSIONS)) | ||
| def test_untrusted_expression_guard_detects_listed_inputs(expression: str) -> None: | ||
| """Each listed field must be detectable via _untrusted_references.""" | ||
|
|
||
| property_name = expression.split(".", 1)[1] | ||
| dot_form = f"echo ${{{{ inputs.{property_name} }}}}" | ||
| bracket_form = f"echo ${{{{ inputs['{property_name}'] }}}}" | ||
| assert expression in _untrusted_references(dot_form) | ||
| assert expression in _untrusted_references(bracket_form) | ||
|
|
||
|
|
||
| def test_untrusted_expression_guard_matches_default_and_wrapper_forms() -> None: | ||
| assert _untrusted_references("echo ${{ inputs.repos || 'all' }}") == ["inputs.repos"] | ||
| assert _untrusted_references("const v = '${{ format('{0}', inputs.codex_args) }}';") == [ | ||
| "inputs.codex_args" | ||
| ] | ||
| assert _untrusted_references("echo ${{ inputs['repos'] }}") == ["inputs.repos"] | ||
| assert _untrusted_references("${{ format('{{prefix}} {0}', inputs.codex_args) }}") == [ | ||
| "inputs.codex_args" | ||
| ] | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.