From c4e85242b0e0c868490927061cc7103880872ea6 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 23 Aug 2026 10:09:22 -0700 Subject: [PATCH 1/2] Propagate has_shell_shebang's Read Failures Instead of Swallowing Them Fixes one real finding from coderabbitai on PR #952 (declines the other; see below). ## Propagate shell-file read failures `has_shell_shebang` caught every `OSError` from `path.open()`/ `readline()` and returned `False`, the same value it returns for a file that legitimately isn't a shell script. Unlike the CI bash side (where `read` fails at true EOF even after filling the variable), Python's `readline()` never raises for EOF, an empty read is just `b''` with no exception, so every `OSError` this caught was a genuine failure (permission denied, the file vanishing between `git ls-files` and the read, disk I/O). Swallowing it meant a tracked file this couldn't open silently dropped out of the lint target list, and `lint()` could report success having never actually checked it. - `scripts/docker_lint.py`: `has_shell_shebang` now raises `CommandFailed` on a genuine read `OSError`, matching the pattern `ls_files` already uses for its own I/O failures. The deliberate `False` cases (a symlink, invalid UTF-8) are unchanged. - `scripts/tests/test_docker_lint.py`: added `test_has_shell_shebang_raises_rather_than_swallowing_a_read_failure`, confirming a mocked `PermissionError` surfaces as `CommandFailed` instead of a silent `False`. ## Declined: reject symlinks in every shell-discovery path The `*.sh`-glob-matched branch (`ls_files(root, linter.patterns)`) never reads file content on the host at all, before or after this chain's own symlink fix (#955): it only builds a path list and passes it to `docker run ... -- files`. Confirmed empirically that a symlink processed *inside* the container cannot escape to the host filesystem regardless of target: `docker run -v "$PWD":/mnt alpine sh -c 'cat /mnt/link-to-etc-shadow'` reads the container's own `/etc/shadow` (byte-identical to reading it directly), and a symlink to a real host tmp file that exists on the host but not in the container's own filesystem tree fails with "No such file or directory" (i.e., the container's own root, not the host's, is what a bind-mounted symlink resolves against). The host-side read this chain actually guards against is specific to `extensionless_shell_scripts`' shebang peek, which already rejects symlinks (#955); the glob-matched branch has no equivalent host-side read to guard. ## Verified Full test suite (798 tests), ruff, mypy, `repo_gate.py`, `prose_lint.py --diff origin/develop`, and the complete `docker_lint.py` run (all 7 linters) all pass clean. --- scripts/docker_lint.py | 12 ++++++++---- scripts/tests/test_docker_lint.py | 8 ++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/scripts/docker_lint.py b/scripts/docker_lint.py index f1f9d1fa..f1452bfc 100755 --- a/scripts/docker_lint.py +++ b/scripts/docker_lint.py @@ -197,15 +197,19 @@ def has_shell_shebang(root: Path, relative_path: str) -> bool: """Report whether a tracked file's shebang directly names bash or sh. Never follows a tracked symlink: `is_symlink()` uses `lstat`, keeping the target unreached. + A read failure raises `CommandFailed` instead of reading as `False`. + That keeps a tracked file this cannot open from silently dropping out of the target list. """ path = root / relative_path + if path.is_symlink(): + return False try: - if path.is_symlink(): - return False with path.open("rb") as handle: first_line = handle.readline(256) - except OSError: - return False + except OSError as error: + raise CommandFailed( + f"target discovery failed: could not read {relative_path}: {error}" + ) from error try: text = first_line.decode("utf-8").rstrip("\n") except UnicodeDecodeError: diff --git a/scripts/tests/test_docker_lint.py b/scripts/tests/test_docker_lint.py index 065d020b..12d2fc7a 100755 --- a/scripts/tests/test_docker_lint.py +++ b/scripts/tests/test_docker_lint.py @@ -180,6 +180,14 @@ def test_has_shell_shebang_reports_false_for_a_symlink_without_reading_it(self) with mock.patch.object(Path, "open", side_effect=AssertionError("symlink target opened")): self.assertFalse(docker_lint.has_shell_shebang(self.root, "ops/evil-symlink")) + def test_has_shell_shebang_raises_rather_than_swallowing_a_read_failure(self) -> None: + self.track("ops/unreadable") + with ( + mock.patch.object(Path, "open", side_effect=PermissionError("denied")), + self.assertRaisesRegex(docker_lint.CommandFailed, "could not read"), + ): + docker_lint.has_shell_shebang(self.root, "ops/unreadable") + def test_extensionless_untracked_shebang_script_is_not_picked_up(self) -> None: path = self.root / "ops" / "vps-backup-pull" path.parent.mkdir(parents=True, exist_ok=True) From 48486beae9f40c67b870075c97a34cfebe6e67f9 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 23 Aug 2026 10:13:13 -0700 Subject: [PATCH 2/2] Say the Docstring Returns False, Not "Reads As" It Fixes a Copilot nitpick on PR #956: the function returns False, it doesn't "read" anything as False. Wording only, no behavior change. --- scripts/docker_lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/docker_lint.py b/scripts/docker_lint.py index f1452bfc..88ef902e 100755 --- a/scripts/docker_lint.py +++ b/scripts/docker_lint.py @@ -197,7 +197,7 @@ def has_shell_shebang(root: Path, relative_path: str) -> bool: """Report whether a tracked file's shebang directly names bash or sh. Never follows a tracked symlink: `is_symlink()` uses `lstat`, keeping the target unreached. - A read failure raises `CommandFailed` instead of reading as `False`. + A read failure raises `CommandFailed` instead of returning `False`. That keeps a tracked file this cannot open from silently dropping out of the target list. """ path = root / relative_path