Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions .github/workflows/validate-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions scripts/docker_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions scripts/tests/test_docker_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,21 @@ 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
path.parent.mkdir(parents=True, exist_ok=True)
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)
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

def invoke(self, selected: set[str], runner: FakeRunner) -> tuple[int, str]:
output = io.StringIO()
with contextlib.redirect_stdout(output):
Expand Down Expand Up @@ -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"))

Comment thread
coderabbitai[bot] marked this conversation as resolved.
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)
Expand Down