diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index 7be9e98e..0a1ded2b 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -115,10 +115,10 @@ jobs: mapfile -d '' -t candidates < <(git ls-files -z) for file in "${candidates[@]}"; do base="${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 + # Never follows a tracked symlink: -h short-circuits before -f's own dereferencing stat. + if [[ "$base" != *.* ]] && [ ! -h "$file" ] && [ -f "$file" ]; then + # `head` reads a no-trailing-newline file cleanly and still fails on a genuine read error. + first_line="$(head -n 1 -- "$file")" if is_shell_shebang "$first_line"; then scripts+=("$file") fi diff --git a/scripts/docker_lint.py b/scripts/docker_lint.py index 738b44d2..f1f9d1fa 100755 --- a/scripts/docker_lint.py +++ b/scripts/docker_lint.py @@ -194,9 +194,15 @@ def shell_shebang_interpreter(line: str) -> str | None: def has_shell_shebang(root: Path, relative_path: str) -> bool: - """Report whether a tracked file's shebang directly names bash or sh.""" + """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. + """ + path = root / relative_path try: - with (root / relative_path).open("rb") as handle: + if path.is_symlink(): + return False + with path.open("rb") as handle: first_line = handle.readline(256) except OSError: return False diff --git a/scripts/tests/test_docker_lint.py b/scripts/tests/test_docker_lint.py index a0fa1174..065d020b 100755 --- a/scripts/tests/test_docker_lint.py +++ b/scripts/tests/test_docker_lint.py @@ -44,6 +44,8 @@ class DockerLintCase(unittest.TestCase): def setUp(self) -> None: self.root = Path(self.enterContext(tempfile.TemporaryDirectory())) subprocess.run(["git", "init", "-q", str(self.root)], check=True) + # A separate temp dir, not self.root's own parent, which is the shared system temp root. + self.outside = Path(self.enterContext(tempfile.TemporaryDirectory())) def track(self, name: str, body: str = "content\n") -> None: path = self.root / name @@ -51,6 +53,12 @@ def track(self, name: str, body: str = "content\n") -> None: path.write_text(body, encoding="utf-8") subprocess.run(["git", "-C", str(self.root), "add", "--", name], check=True) + def track_symlink(self, name: str, target: Path) -> None: + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.symlink_to(target) + subprocess.run(["git", "-C", str(self.root), "add", "--", name], check=True) + def invoke(self, selected: set[str], runner: FakeRunner) -> tuple[int, str]: output = io.StringIO() with contextlib.redirect_stdout(output): @@ -157,6 +165,21 @@ def test_extensionless_shebang_script_merges_with_glob_matched_scripts(self) -> docker_lint.tracked_files(self.root, linter), ) + def test_extensionless_symlink_is_never_followed(self) -> None: + secret = self.outside / "secret" + secret.write_text("#!/usr/bin/env bash\n", encoding="utf-8") + self.track_symlink("ops/evil-symlink", secret) + linter = next(linter for linter in docker_lint.LINTERS if linter.name == "shellcheck") + with mock.patch.object(Path, "open", side_effect=AssertionError("symlink target opened")): + self.assertEqual([], docker_lint.tracked_files(self.root, linter)) + + def test_has_shell_shebang_reports_false_for_a_symlink_without_reading_it(self) -> None: + secret = self.outside / "secret" + secret.write_text("#!/usr/bin/env bash\n", encoding="utf-8") + self.track_symlink("ops/evil-symlink", secret) + 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_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)