diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index ac8a8dd8..7be9e98e 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -79,16 +79,34 @@ jobs: fi [[ "$interpreter" == "env" ]] || return 1 local -a args=("${tokens[@]:1}") - while [ "${#args[@]}" -gt 0 ] && [[ "${args[0]}" == -* ]] && [[ "${args[0]}" != "--" ]]; do - if [[ "${args[0]}" == "-S" ]]; then + local token + while [ "${#args[@]}" -gt 0 ]; do + token="${args[0]}" + if [[ "$token" == "--" ]]; then args=("${args[@]:1}") break fi - args=("${args[@]:1}") + if [[ "$token" == "-S" || "$token" == "--split-string" ]]; then + args=("${args[@]:1}") + break + fi + if [[ "$token" == -* ]]; then + case "$token" in + -u | --unset | -C | --chdir) + args=("${args[@]:2}") + ;; + *) + args=("${args[@]:1}") + ;; + esac + continue + fi + if [[ "$token" =~ ^[A-Za-z_][A-Za-z0-9_]*= ]]; then + args=("${args[@]:1}") + continue + fi + break done - if [ "${#args[@]}" -gt 0 ] && [[ "${args[0]}" == "--" ]]; then - args=("${args[@]:1}") - fi [ "${#args[@]}" -gt 0 ] || return 1 local cmd="${args[0]##*/}" [[ "$cmd" == "bash" || "$cmd" == "sh" ]] @@ -97,9 +115,13 @@ jobs: mapfile -d '' -t candidates < <(git ls-files -z) for file in "${candidates[@]}"; do base="${file##*/}" - if [[ "$base" != *.* ]] && [ -f "$file" ] && IFS= read -r first_line < "$file" \ - && is_shell_shebang "$first_line"; then - scripts+=("$file") + if [[ "$base" != *.* ]] && [ -f "$file" ]; then + # `read` fails at EOF even when it fills first_line, so the check reads the content regardless. + first_line="" + IFS= read -r first_line < "$file" || true + if is_shell_shebang "$first_line"; then + scripts+=("$file") + fi fi done if [ "${#scripts[@]}" -gt 0 ]; then diff --git a/scripts/docker_lint.py b/scripts/docker_lint.py index 0f76a932..738b44d2 100755 --- a/scripts/docker_lint.py +++ b/scripts/docker_lint.py @@ -138,11 +138,26 @@ def ls_files( return [os.fsdecode(entry) for entry in result.stdout.split(b"\0") if entry] +# The env options below take a separate operand token, never mistaken for the command. +ENV_OPERAND_FLAGS = {"-u", "--unset", "-C", "--chdir"} + + +def _is_env_assignment(token: str) -> bool: + """Report whether token is a `NAME=VALUE` env-style assignment.""" + name, separator, _ = token.partition("=") + return ( + bool(separator) + and bool(name) + and (name[0].isalpha() or name[0] == "_") + and all(char.isalnum() or char == "_" for char in name) + ) + + def shell_shebang_interpreter(line: str) -> str | None: """Return the shebang's direct interpreter, bash or sh, or None otherwise. Tokenizes rather than substring-matches, so a plain-argument `bash` is not the interpreter. - An `env` shebang walks past its own flags to the command it selects. + An `env` shebang walks past its own flags and `NAME=VALUE` assignments to find the command. """ if not line.startswith("#!"): return None @@ -158,13 +173,21 @@ def shell_shebang_interpreter(line: str) -> str | None: if interpreter != "env": return None args = tokens[1:] - while args and args[0].startswith("-") and args[0] != "--": - if args[0] == "-S": + while args: + token = args[0] + if token == "--": + args = args[1:] + break + if token in {"-S", "--split-string"}: args = args[1:] break - args = args[1:] - if args and args[0] == "--": - args = args[1:] + if token.startswith("-"): + args = args[2:] if token in ENV_OPERAND_FLAGS else args[1:] + continue + if _is_env_assignment(token): + args = args[1:] + continue + break if args and args[0].rsplit("/", 1)[-1] in {"bash", "sh"}: return args[0].rsplit("/", 1)[-1] return None diff --git a/scripts/tests/test_docker_lint.py b/scripts/tests/test_docker_lint.py index 7b3d8d1b..a0fa1174 100755 --- a/scripts/tests/test_docker_lint.py +++ b/scripts/tests/test_docker_lint.py @@ -186,6 +186,24 @@ def test_extensionless_script_naming_bash_only_as_an_argument_is_excluded(self) linter = next(linter for linter in docker_lint.LINTERS if linter.name == "shellcheck") self.assertEqual([], docker_lint.tracked_files(self.root, linter)) + def test_extensionless_shebang_script_with_no_trailing_newline_is_picked_up(self) -> None: + self.track("ops/vps-backup-pull", "#!/usr/bin/env bash") + linter = next(linter for linter in docker_lint.LINTERS if linter.name == "shellcheck") + self.assertEqual(["ops/vps-backup-pull"], docker_lint.tracked_files(self.root, linter)) + + def test_shell_shebang_interpreter_walks_past_env_grammar(self) -> None: + cases = { + "#!/usr/bin/env FOO=1 bash": "bash", + "#!/usr/bin/env -u bash python": None, + "#!/usr/bin/env -i FOO=1 bash": "bash", + "#!/usr/bin/env --unset=FOO bash": "bash", + "#!/usr/bin/env -C /tmp bash": "bash", + "#!/usr/bin/env FOO=1 BAR=2 sh": "sh", + } + for line, expected in cases.items(): + with self.subTest(line=line): + self.assertEqual(expected, docker_lint.shell_shebang_interpreter(line)) + def test_cspell_literal_marker_precedes_option_shaped_filename(self) -> None: linter = next(linter for linter in docker_lint.LINTERS if linter.name == "cspell") command = docker_lint.container_command(