Walk env's Full Argument Grammar, Fix read's EOF Quirk #953
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Comment on lines
+151
to
+152
There was a problem hiding this comment. 2. Unicode env assignment mismatch scripts/docker_lint.py treats non-ASCII letters/digits as valid env-var names via str.isalpha/isalnum, but the CI workflow’s is_shell_shebang only recognizes ASCII [A-Za-z0-9_]. This can cause the same shebang to be classified as a shell script locally but not in CI (or vice versa), despite the workflow comment stating the two implementations must stay in sync. Agent Prompt
|
||
| ) | ||
|
|
||
|
|
||
| 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 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: | ||
| token = args[0] | ||
| if token == "--": | ||
| args = args[1:] | ||
| break | ||
| if token in {"-S", "--split-string"}: | ||
| args = args[1:] | ||
| break | ||
| 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 | ||
|
Comment on lines
+176
to
+190
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: printf '%s\n' 'Repository guidance and diff scope:'
find .. -name AGENTS.md -print
printf '%s\n' 'Changed-file summary:'
git diff --stat
printf '%s\n' 'Changed files:'
git diff --name-onlyRepository: ptr727/ProjectTemplate Length of output: 250 🏁 Script executed: cat -n ../git/AGENTS.md
printf '%s\n' 'Code-review skill candidates:'
find .github -path '*/skills/*' -type f -maxdepth 5 -print 2>/dev/null
printf '%s\n' 'Repository metadata:'
git remote -v
git status --shortRepository: ptr727/ProjectTemplate Length of output: 22160 🏁 Script executed: set -eu
hub_checkout="$(mktemp -d)"
git clone --depth 1 --branch main https://github.com/ptr727/ProjectTemplate.git "$hub_checkout" >/tmp/projecttemplate-clone.log
printf 'hub_checkout=%s\n' "$hub_checkout"
printf '%s\n' 'Hub files:'
for f in \
"$hub_checkout/.github/skills/code-review/SKILL.md" \
"$hub_checkout/.github/skills/python-codestyle/SKILL.md" \
"$hub_checkout/.github/skills/shell-codestyle/SKILL.md" \
"$hub_checkout/.github/skills/workflow-ci-contract/SKILL.md" \
"$hub_checkout/.github/skills/comment-and-doc-style/SKILL.md"
do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
doneRepository: ptr727/ProjectTemplate Length of output: 45828 🏁 Script executed: set -eu
printf '%s\n' 'Relevant files:'
git ls-files -- scripts/docker_lint.py scripts/tests/test_docker_lint.py .github/workflows/validate-task.yml OPERATIONS.md WORKFLOW.md GOVERNANCE.md pyproject.toml
printf '%s\n' 'Line counts:'
wc -l scripts/docker_lint.py scripts/tests/test_docker_lint.py .github/workflows/validate-task.yml 2>/dev/null
printf '%s\n' 'Python outline:'
ast-grep outline scripts/docker_lint.py
printf '%s\n' 'Test outline:'
ast-grep outline scripts/tests/test_docker_lint.py
printf '%s\n' 'Relevant symbols and references:'
rg -n --glob '!AGENTS.md' 'shell_shebang_interpreter|ENV_OPERAND_FLAGS|split-string|/usr/bin/env|validate-task' scripts .github
printf '%s\n' 'Documented verification headings:'
for f in OPERATIONS.md WORKFLOW.md GOVERNANCE.md; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
grep -n '^## ' "$f" | head -80
fi
doneRepository: ptr727/ProjectTemplate Length of output: 10031 🏁 Script executed: set -eu
printf '%s\n' 'scripts/docker_lint.py:'
sed -n '130,215p' scripts/docker_lint.py
printf '%s\n' 'scripts/tests/test_docker_lint.py:'
sed -n '155,215p' scripts/tests/test_docker_lint.py
printf '%s\n' '.github/workflows/validate-task.yml:'
sed -n '55,125p' .github/workflows/validate-task.yml
printf '%s\n' 'OPERATIONS.md local verification:'
sed -n '1,65p' OPERATIONS.mdRepository: ptr727/ProjectTemplate Length of output: 16736 🏁 Script executed: set -eu
python3 - <<'PY'
import shlex
import subprocess
def is_assignment(token: str) -> bool:
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 current_python_parser(line: str) -> str | None:
tokens = shlex.split(line[2:]) if line.startswith("#!") else []
if not tokens:
return None
interpreter = tokens[0].rsplit("/", 1)[-1]
if interpreter in {"bash", "sh"}:
return interpreter
if interpreter != "env":
return None
args = tokens[1:]
while args:
token = args[0]
if token == "--":
args = args[1:]
break
if token in {"-S", "--split-string"}:
args = args[1:]
break
if token.startswith("-"):
args = args[2:] if token in {"-u", "--unset", "-C", "--chdir"} else args[1:]
continue
if is_assignment(token):
args = args[1:]
continue
break
return args[0].rsplit("/", 1)[-1] if args and args[0].rsplit("/", 1)[-1] in {"bash", "sh"} else None
cases = [
"#!/usr/bin/env -- FOO=1 bash",
"#!/usr/bin/env -S FOO=1 bash",
"#!/usr/bin/env FOO=1 bash",
]
for case in cases:
print(f"python current: {case} -> {current_python_parser(case)!r}")
for env_args in (
["--", "FOO=1", "bash", "-c", 'printf "%s" "$FOO"'],
["-S", "FOO=1", "bash", "-c", 'printf "%s" "$FOO"'],
):
result = subprocess.run(["env", *env_args], check=True, text=True, capture_output=True)
print(f"system env: {' '.join(env_args[:3])} -> {result.stdout!r}")
PY
bash <<'BASH'
set -Eeuo pipefail
is_shell_shebang_current() {
local line="$1"
[[ "$line" == '#!'* ]] || return 1
local -a tokens
read -ra tokens <<< "${line#\#!}"
[ "${`#tokens`[@]}" -gt 0 ] || return 1
local interpreter="${tokens[0]##*/}"
if [[ "$interpreter" == "bash" || "$interpreter" == "sh" ]]; then
return 0
fi
[[ "$interpreter" == "env" ]] || return 1
local -a args=("${tokens[@]:1}")
local token
while [ "${`#args`[@]}" -gt 0 ]; do
token="${args[0]}"
if [[ "$token" == "--" ]]; then
args=("${args[@]:1}")
break
fi
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
[ "${`#args`[@]}" -gt 0 ] || return 1
local cmd="${args[0]##*/}"
[[ "$cmd" == "bash" || "$cmd" == "sh" ]]
}
for line in \
'#!/usr/bin/env -- FOO=1 bash' \
'#!/usr/bin/env -S FOO=1 bash' \
'#!/usr/bin/env FOO=1 bash'
do
if is_shell_shebang_current "$line"; then result=true; else result=false; fi
printf 'bash current: %s -> %s\n' "$line" "$result"
done
BASHRepository: ptr727/ProjectTemplate Length of output: 302 🏁 Script executed: set -eu
python3 - <<'PY'
import shlex
import subprocess
def is_assignment(token):
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 current_python_parser(line):
tokens = shlex.split(line[2:]) if line.startswith("#!") else []
if not tokens:
return None
interpreter = tokens[0].rsplit("/", 1)[-1]
if interpreter in {"bash", "sh"}:
return interpreter
if interpreter != "env":
return None
args = tokens[1:]
while args:
token = args[0]
if token == "--":
args = args[1:]
break
if token in {"-S", "--split-string"}:
args = args[1:]
break
if token.startswith("-"):
args = args[2:] if token in {"-u", "--unset", "-C", "--chdir"} else args[1:]
continue
if is_assignment(token):
args = args[1:]
continue
break
return args[0].rsplit("/", 1)[-1] if args and args[0].rsplit("/", 1)[-1] in {"bash", "sh"} else None
for case in (
"#!/usr/bin/env -- FOO=1 bash",
"#!/usr/bin/env -S FOO=1 bash",
"#!/usr/bin/env FOO=1 bash",
):
print("python current:", case, "->", repr(current_python_parser(case)))
for env_args in (
["--", "FOO=1", "bash", "-c", 'printf "%s" "$FOO"'],
["-S", "FOO=1", "bash", "-c", 'printf "%s" "$FOO"'],
):
result = subprocess.run(["env"] + env_args, check=True, text=True, capture_output=True)
print("system env:", " ".join(env_args[:3]), "->", repr(result.stdout))
PY
bash <<'BASH'
set -Eeuo pipefail
is_shell_shebang_current() {
local line="$1"
[[ "$line" == '#!'* ]] || return 1
local -a tokens
read -ra tokens <<< "${line#\#!}"
[ "${`#tokens`[@]}" -gt 0 ] || return 1
local interpreter="${tokens[0]##*/}"
if [[ "$interpreter" == "bash" || "$interpreter" == "sh" ]]; then
return 0
fi
[[ "$interpreter" == "env" ]] || return 1
local -a args=("${tokens[@]:1}"
)
local token
while [ "${`#args`[@]}" -gt 0 ]; do
token="${args[0]}"
if [[ "$token" == "--" ]]; then
args=("${args[@]:1}")
break
fi
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
[ "${`#args`[@]}" -gt 0 ] || return 1
local cmd="${args[0]##*/}"
[[ "$cmd" == "bash" || "$cmd" == "sh" ]]
}
for line in \
'#!/usr/bin/env -- FOO=1 bash' \
'#!/usr/bin/env -S FOO=1 bash' \
'#!/usr/bin/env FOO=1 bash'
do
if is_shell_shebang_current "$line"; then result=true; else result=false; fi
printf 'bash current: %s -> %s\n' "$line" "$result"
done
BASHRepository: ptr727/ProjectTemplate Length of output: 536 🌐 Web query:
💡 Result: The behavior you observe—such as a process hanging when using a shebang like #!/usr/bin/env FOO=1 bash—is caused by a fundamental limitation in how operating system kernels handle the shebang (#!) line, rather than a bug in the env command [1][2][3]. Kernel Semantics and the Shebang Line When a script is executed, the kernel (specifically the binfmt_script handler on Linux) reads the first line of the file [4][5]. It performs minimal parsing: it identifies the interpreter path and treats everything else on that line (up to the newline) as a single, optional argument [2][6][7]. The kernel does not perform shell-like splitting, quote interpretation, or environment variable expansion [2]. If your shebang is: #!/usr/bin/env FOO=1 bash The kernel executes /usr/bin/env, passing the entire string "FOO=1 bash" as a single argument [1][3]. Consequently, env is invoked effectively as: env "FOO=1 bash" /path/to/your/script Because "FOO=1 bash" is treated as the command to run, env searches for a program literally named "FOO=1 bash" in your PATH [8]. When it fails to find it, or if it treats it as an environment setting for a subsequent command that is never properly resolved, the process often hangs or fails [1][3]. In many cases, it triggers an infinite loop where the script re-executes itself via env, causing the system to continuously attempt to restart the process [1][3]. The Solution: env -S To support multiple arguments and environment variables in shebang lines on modern systems, GNU env introduced the -S (--split-string) option [1][9][10]. This option instructs env to perform its own, more sophisticated splitting on the argument that follows it [1][11]. To correctly use your example, you should write the shebang as: #!/usr/bin/env -S FOO=1 bash With -S, env receives the single argument "FOO=1 bash" from the kernel, then manually splits it into "FOO=1" and "bash" [1][11]. It then correctly executes bash with FOO=1 in its environment, and bash subsequently executes your script [1][10]. Note that when using -S in a shebang line, you should generally avoid extra quotes that you might otherwise use on a command line, as they would be treated as literal characters [12]. Top Results: [1][2][3][4][11] Citations:
🏁 Script executed: set -eu
python3 - <<'PY'
import subprocess
cases = [
["-- FOO=1 bash", "/bin/true"],
["-S FOO=1 bash -c 'printf \"%s\" \"$FOO\"'"],
]
for args in cases:
result = subprocess.run(
["env"] + args,
check=False,
text=True,
capture_output=True,
timeout=5,
)
print("argv after env:", args)
print("exit:", result.returncode)
print("stdout:", repr(result.stdout))
print("stderr:", repr(result.stderr))
PYRepository: ptr727/ProjectTemplate Length of output: 412 🌐 Web query:
💡 Result: GNU
#!/usr/bin/env -S perl -T -w
Citations: Continue parsing For Do not add the
📍 Affects 3 files
🤖 Prompt for AI Agents |
||
| if args and args[0].rsplit("/", 1)[-1] in {"bash", "sh"}: | ||
| return args[0].rsplit("/", 1)[-1] | ||
| return None | ||
|
|
||
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 |
|---|---|---|
|
|
@@ -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( | ||
|
|
||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Pr title not title case
📘 Rule violation⚙ Maintainability