Skip to content

ci(tools): parse the scripts inside single-quoted bash -c arguments #893

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.

Sign up for GitHub

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

Open
gHashTag wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci(tools): parse the scripts inside single-quoted bash -c arguments #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
86 changes: 82 additions & 4 deletions tools/check_run_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@

`bash -n` reads syntax only: a script inside a quoted `bash -c '...'` argument
is one string to it, so an error INSIDE that string is not seen -- unless, as
in both cases above, it breaks the quoting of the outer script.
in both cases above, it breaks the quoting of the outer script. So once the
outer script parses, every single-quoted `bash -c '...'` / `sh -c '...'`
argument in it (the `docker run ... bash -c '` scripts, 24 on main) is read
out as the outer shell reads it -- `'\''` is a quote, and a splice of an outer
value, `'"$PART"'` or `'$X'`, is one word -- and parsed with `bash -n` / `sh -n`
in turn. A quote followed by anything else is "cannot tell". A double-quoted
`bash -c "..."` is not read: the outer shell expands it first.

Exit 0 clean, 1 a step that does not parse, 2 a workflow this could not read
(cannot tell is not clean). tools/test_check_run_syntax.py plants every case.
Expand Down Expand Up @@ -165,7 +171,58 @@ def _shell_named(step: dict, job: dict, wf: dict) -> tuple[str, ...]:
return () # pwsh, python, cmd, a custom shell: not ours to parse


# `bash -c '`, `sh -lc '`, `bash -euo pipefail -c '`: the opening quote of a
# script argument. `\b` keeps `zsh -c` and `fish -c` out.
INNER_OPEN = re.compile(r"\b(bash|sh)(?:[ \t]+(?:-[a-zA-Z]*o[ \t]+[a-z]+|-[a-zA-Z]+))*?"
r"[ \t]+-[a-zA-Z]*c[a-zA-Z]*[ \t]+'")
# What may sit between a closing quote and the reopening one: an outer value.
SPLICE = re.compile(r'"(?:[^"\\]|\\.)*"|\$\{[^}]*\}|\$[A-Za-z_][A-Za-z0-9_]*')
# Stands in for a spliced value: bash -n needs a word there, not its contents.
OUTER_VALUE = "__outer__"
WORD_BREAK = " \t\n;|&)"
# How many -c scripts check() parsed since main() started, for its summary line.
_INNER_SEEN = 0


def inner_scripts(script: str) -> list[tuple[str, str]]:
"""(interpreter, text) of every single-quoted -c script argument in `script`."""
found = []
for m in INNER_OPEN.finditer(script):
i, buf = m.end(), []
while True:
j = script.find("'", i)
unclosed = j < 0
if unclosed:
raise CannotTell(f"a {m.group(1)} -c '...' script with no closing quote")
buf.append(script[i:j])
k = j + 1
escaped_quote = script.startswith("\\''", k) # '\'' : a quote, then reopen
if escaped_quote:
buf.append("'")
i = k + 3
continue
spliced = False
s = SPLICE.match(script, k)
while s:
spliced = True
k = s.end()
s = SPLICE.match(script, k)
if spliced:
buf.append(OUTER_VALUE)
reopens = script.startswith("'", k)
if reopens:
i = k + 1
continue
ends = k == len(script) or script[k] in WORD_BREAK
if not ends:
raise CannotTell(f"a {m.group(1)} -c '...' script is glued to {script[k:k + 12]!r}")
break
found.append((m.group(1), "".join(buf)))
return found


def check(path: pathlib.Path) -> tuple[list[str], str | None]:
global _INNER_SEEN
import yaml # inside: a missing PyYAML is "cannot tell", not a crash
try:
wf = yaml.safe_load(path.read_text())
Expand All @@ -186,13 +243,31 @@ def check(path: pathlib.Path) -> tuple[list[str], str | None]:
shells = shell_of(step, job, wf)
except CannotTell as e:
return [], f"{path.name} :: {jn} :: step {k}: {e}"
where = f"{path.relative_to(ROOT)} :: {jn} :: {step.get('name') or f'step {k}'}"
outer_parses = bool(shells)
for sh in shells:
r = subprocess.run([sh, "-n"], input=str(step["run"]), capture_output=True, text=True)
if r.returncode:
err = " ".join(r.stderr.split())[:240]
bad.append(f"{path.relative_to(ROOT)} :: {jn} :: {step.get('name') or f'step {k}'}"
f" [{sh} -n]\n {err}")
bad.append(f"{where} [{sh} -n]\n {err}")
outer_parses = False
break
# Only a script whose own quoting is sound can be read for the strings
# it hands to bash -c; a broken one was reported above.
if not outer_parses:
continue
try:
inner = inner_scripts(str(step["run"]))
except CannotTell as e:
return [], f"{path.name} :: {jn} :: step {k}: {e}"
_INNER_SEEN += len(inner)
for n, (ish, text) in enumerate(inner, 1):
r = subprocess.run([ish, "-n"], input=text, capture_output=True, text=True)
inner_bad = r.returncode != 0
if inner_bad:
err = " ".join(r.stderr.split())[:240]
bad.append(f"{where} [{ish} -n on {ish} -c script {n}, lines from its opening quote]"
f"\n {err}")
return bad, None


Expand All @@ -206,14 +281,17 @@ def main() -> int:
if not shutil.which(sh):
print(f"cannot tell: no {sh} on PATH")
return 2
global _INNER_SEEN
_INNER_SEEN = 0
files = sorted(list(WORKFLOWS.glob("*.yml")) + list(WORKFLOWS.glob("*.yaml")))
bad, unread = [], []
for f in files:
b, err = check(f)
bad += b
if err:
unread.append(err)
print(f"workflows read: {len(files) - len(unread)} of {len(files)}; steps that do not parse: {len(bad)}")
print(f"workflows read: {len(files) - len(unread)} of {len(files)}; bash -c '...' scripts read out"
f" of them: {_INNER_SEEN}; steps and scripts that do not parse: {len(bad)}")
for b in bad:
print(" " + b)
for u in unread:
Expand Down
29 changes: 29 additions & 0 deletions tools/test_check_run_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,35 @@ def git_show(rev_path: str) -> str | None:
("'\\'' inside single quotes is a quote", 0, H +
" steps:\n - run: |\n bash -c 'grep -oE '\\''A.+'\\'' x'\n"),
("an unclosed if", 1, H + " steps:\n - run: |\n if true; then\n echo x\n"),
# Inside a single-quoted -c script: one string to the outer bash -n.
("an unclosed if inside bash -c '...'", 1, H +
" steps:\n - run: |\n docker run img bash -c '\n"
" if true; then\n make\n '\n"),
("a clean bash -c '...' script", 0, H +
" steps:\n - run: |\n docker run img bash -c '\n"
" if true; then\n make\n fi\n '\n"),
("'\"$X\"' splices an outer value as one word", 0, H +
" steps:\n - run: |\n docker run img bash -c '\n"
" git checkout '\"$REV\"' && make /w/'\"$PART\"'.bin\n '\n"),
("an error after a splice is still seen", 1, H +
" steps:\n - run: |\n docker run img bash -c '\n"
" git checkout '\"$REV\"'\n if true; then make\n '\n"),
("a bare '$X' splice is one word", 0, H +
" steps:\n - run: |\n bash -c 'echo '$X' '${Y}' done'\n"),
("sh -c '...' is checked with sh", 1, H +
" steps:\n - run: |\n docker run img sh -c 'cat <(echo x)'\n"),
("bash -lc '...' is checked", 1, H +
" steps:\n - run: |\n bash -lc 'if true; then echo x'\n"),
("bash -euo pipefail -c '...' is checked", 1, H +
" steps:\n - run: |\n bash -euo pipefail -c 'if true; then echo x'\n"),
("two scripts in one step: the second is checked too", 1, H +
" steps:\n - run: |\n bash -c 'echo ok'\n bash -c 'if true; then echo x'\n"),
("a -c script glued to a word = cannot tell", 2, H +
" steps:\n - run: |\n bash -c 'echo x'y\n"),
("zsh -c '...' is not read", 0, H +
" steps:\n - run: |\n zsh -c 'if true; then'\n"),
("a double-quoted bash -c \"...\" is not read", 0, H +
" steps:\n - run: |\n bash -c \"if true; then\"\n"),
("${{ }} is left as written", 0, H +
" steps:\n - run: echo \"${{ github.sha }}\"\n"),
# `cat <(echo x)` parses in bash and not in sh (dash, or bash in POSIX
Expand Down
Loading