Skip to content

Commit f2cc7a7

Browse files
committed
verify-action-build: scan JS/TS sources for unverified downloads
The binary-download check previously only covered Dockerfiles, action.yml run-blocks and referenced shell/python scripts, and was gated behind `if not is_js_action:` so it never fired for JS actions at all. But JS actions commonly shell out to fetch pre-built binaries from their own TypeScript sources (e.g. `@actions/tool-cache`'s `tc.downloadTool`, which does NOT verify checksums). This commit: - Moves the binary-download check out of the non-JS branch in `verification.py`, so it runs for every action type. - Adds JS/TS download and verification patterns in `security.py` (`tc.downloadTool`, `downloadTool`, bare `fetch("https://…")`, `http(s).get/request`, `axios.*`, `new HttpClient`, `node-fetch`; verification via `crypto.createHash`, WebCrypto `subtle.digest`, sigstore/cosign, `verify*`/`computeHash` helpers). - Discovers JS/TS source files via the GitHub trees API, scanning the repo root plus conventional source dirs (`src/`, `lib/`, `source/`, `sources/`, `scripts/`) and explicitly excluding `dist/`, `build/`, `out/`, `node_modules/`, `coverage/`, tests, examples and docs to keep noise down. - Applies the "verification must appear in the same file as the download" rule consistently with the existing shell-side check. Confirmed against posit-dev/setup-air (PR #724): the scanner now flags `src/download/download-version.ts` for calling `tc.downloadTool` with no adjacent checksum/signature step, and the overall verification exits 1. Generated-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1aa04bd commit f2cc7a7

2 files changed

Lines changed: 183 additions & 29 deletions

File tree

utils/verify_action_build/security.py

Lines changed: 152 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
"""Security analysis checks for GitHub Actions."""
2020

2121
import json
22+
import os
2223
import re
2324

25+
import requests
26+
2427
from .console import console, link
2528
from .github_client import GitHubClient
2629
from .action_ref import (
@@ -724,11 +727,121 @@ def analyze_action_metadata(
724727
]
725728

726729

730+
# Patterns indicating a JS/TS download of a remote artifact. Most JS actions
731+
# that fetch binaries go through @actions/tool-cache's downloadTool (which
732+
# does NOT verify checksums), or via node's http/https, fetch, axios, or
733+
# @actions/http-client. Each of these should have a companion hash/signature
734+
# check in the same file to count as verified.
735+
_JS_DOWNLOAD_PATTERNS = [
736+
re.compile(r"\btc\.downloadTool\s*\("),
737+
re.compile(r"(?<![a-zA-Z_.])downloadTool\s*\("),
738+
re.compile(r"\bfetch\s*\([^)]*['\"`]https?://"),
739+
re.compile(r"\bhttps?\.(?:get|request)\s*\("),
740+
re.compile(r"\baxios(?:\.(?:get|post|request))?\s*\("),
741+
re.compile(r"\bnew\s+HttpClient\s*\("),
742+
re.compile(r"\brequire\(\s*['\"`]node-fetch['\"`]"),
743+
]
744+
745+
# Verification patterns in JS/TS source: node crypto, WebCrypto, or common
746+
# sigstore/cosign / custom "verify" helper names.
747+
_JS_VERIFICATION_PATTERNS = [
748+
re.compile(r"\bcrypto\.createHash\s*\("),
749+
re.compile(r"\bcrypto\.subtle\.digest\b"),
750+
re.compile(r"\bsubtle\.verify\s*\("),
751+
re.compile(r"\b@noble/hashes\b"),
752+
re.compile(r"\bsigstore\b", re.IGNORECASE),
753+
re.compile(r"\bcosign\b", re.IGNORECASE),
754+
re.compile(r"\bverifySignature\b"),
755+
re.compile(r"\bverifyChecksum\b"),
756+
re.compile(r"\bcomputeHash\b"),
757+
]
758+
759+
_JS_SOURCE_EXTENSIONS = (".ts", ".js", ".mjs", ".cjs")
760+
_JS_SCAN_DIR_PREFIXES = ("src/", "lib/", "source/", "sources/", "scripts/")
761+
_JS_EXCLUDE_DIR_PREFIXES = (
762+
"dist/", "build/", "out/", "node_modules/", "coverage/",
763+
"__tests__/", "test/", "tests/", "examples/", "example/",
764+
"docs/", ".github/",
765+
)
766+
767+
727768
def _line_is_pkg_manager(line: str) -> bool:
728769
lower = line.lower()
729770
return any(marker in lower for marker in _PKG_MANAGER_MARKERS)
730771

731772

773+
def _find_binary_downloads_js(content: str) -> list[tuple[int, str]]:
774+
"""Find lines in JS/TS source that fetch remote artifacts.
775+
776+
Flags calls to ``tc.downloadTool`` / ``downloadTool``, bare ``fetch`` to
777+
an http(s) URL, node's ``http(s).get`` / ``.request``, ``axios.*``, and
778+
``new HttpClient()``. Skips comment-only lines.
779+
"""
780+
findings: list[tuple[int, str]] = []
781+
for i, line in enumerate(content.splitlines(), 1):
782+
stripped = line.strip()
783+
if not stripped or stripped.startswith("//") or stripped.startswith("*"):
784+
continue
785+
if any(p.search(line) for p in _JS_DOWNLOAD_PATTERNS):
786+
findings.append((i, stripped[:120]))
787+
return findings
788+
789+
790+
def _list_repo_files(org: str, repo: str, commit_hash: str) -> list[str]:
791+
"""List every blob path in the repo at ``commit_hash`` via the trees API.
792+
793+
Returns an empty list on error, auth failure, or truncated results (the
794+
caller should treat "no files discovered" as best-effort, not canonical).
795+
"""
796+
url = f"https://api.github.com/repos/{org}/{repo}/git/trees/{commit_hash}?recursive=1"
797+
headers = {"Accept": "application/vnd.github+json"}
798+
token = os.environ.get("GITHUB_TOKEN")
799+
if token:
800+
headers["Authorization"] = f"Bearer {token}"
801+
try:
802+
resp = requests.get(url, timeout=15, headers=headers)
803+
if not resp.ok:
804+
return []
805+
data = resp.json()
806+
if data.get("truncated"):
807+
return []
808+
return [t["path"] for t in data.get("tree", []) if t.get("type") == "blob"]
809+
except requests.RequestException:
810+
return []
811+
812+
813+
def _discover_js_source_files(
814+
org: str, repo: str, commit_hash: str, sub_path: str,
815+
) -> list[tuple[str, str]]:
816+
"""Return ``(path, content)`` for JS/TS source files worth scanning.
817+
818+
Includes files at the repo root and under conventional source dirs
819+
(``src/``, ``lib/``, …). Excludes compiled output, vendored modules,
820+
test/example dirs, and generated docs. For monorepo sub-actions the
821+
``sub_path`` acts as a prefix filter.
822+
"""
823+
files: list[tuple[str, str]] = []
824+
all_paths = _list_repo_files(org, repo, commit_hash)
825+
if not all_paths:
826+
return files
827+
828+
prefix = f"{sub_path.rstrip('/')}/" if sub_path else ""
829+
for path in all_paths:
830+
if prefix and not path.startswith(prefix):
831+
continue
832+
rel = path[len(prefix):] if prefix else path
833+
if not rel.endswith(_JS_SOURCE_EXTENSIONS):
834+
continue
835+
if any(rel.startswith(d) for d in _JS_EXCLUDE_DIR_PREFIXES):
836+
continue
837+
if "/" in rel and not any(rel.startswith(d) for d in _JS_SCAN_DIR_PREFIXES):
838+
continue
839+
content = fetch_file_from_github(org, repo, commit_hash, path)
840+
if content is not None:
841+
files.append((rel, content))
842+
return files
843+
844+
732845
def _find_binary_downloads(content: str) -> list[tuple[int, str]]:
733846
"""Find lines that download binaries or scripts over HTTP(S).
734847
@@ -857,7 +970,12 @@ def analyze_binary_downloads(
857970
if content is not None:
858971
files_to_scan.append((script_path, content))
859972

860-
if not files_to_scan:
973+
# JS/TS source files: shell-pattern downloads (curl/wget) are rare here
974+
# but JS actions commonly fetch binaries via @actions/tool-cache etc.,
975+
# so discover those separately and scan with JS-specific patterns.
976+
js_files_to_scan = _discover_js_source_files(org, repo, commit_hash, sub_path)
977+
978+
if not files_to_scan and not js_files_to_scan:
861979
return warnings, failures
862980

863981
console.print()
@@ -896,6 +1014,39 @@ def analyze_binary_downloads(
8961014
f"{path} line {line_num}: unverified download: {snippet[:80]}"
8971015
)
8981016

1017+
for path, content in js_files_to_scan:
1018+
downloads = _find_binary_downloads_js(content)
1019+
if not downloads:
1020+
continue
1021+
any_downloads = True
1022+
has_verify = any(p.search(content) for p in _JS_VERIFICATION_PATTERNS)
1023+
if has_verify:
1024+
console.print(
1025+
f" [green]✓[/green] {path}: {len(downloads)} JS download(s), "
1026+
f"verification present in file"
1027+
)
1028+
for line_num, snippet in downloads[:3]:
1029+
console.print(f" [dim]line {line_num}:[/dim] [dim]{snippet}[/dim]")
1030+
if len(downloads) > 3:
1031+
console.print(f" [dim]... and {len(downloads) - 3} more[/dim]")
1032+
for line_num, snippet in downloads:
1033+
warnings.append(
1034+
f"{path} line {line_num}: JS download present (review coverage): {snippet[:80]}"
1035+
)
1036+
else:
1037+
console.print(
1038+
f" [red]✗[/red] {path}: {len(downloads)} unverified JS download(s) "
1039+
f"[red bold](no checksum/signature check in file)[/red bold]"
1040+
)
1041+
for line_num, snippet in downloads[:5]:
1042+
console.print(f" [dim]line {line_num}:[/dim] [red]{snippet}[/red]")
1043+
if len(downloads) > 5:
1044+
console.print(f" [dim]... and {len(downloads) - 5} more[/dim]")
1045+
for line_num, snippet in downloads:
1046+
failures.append(
1047+
f"{path} line {line_num}: unverified JS download: {snippet[:80]}"
1048+
)
1049+
8991050
if not any_downloads:
9001051
console.print(" [green]✓[/green] No binary downloads detected")
9011052

utils/verify_action_build/verification.py

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,37 @@ def verify_single_action(
192192
checks_performed.append(("Action type detection", "info", action_type))
193193

194194
is_js_action = action_type.startswith("node") or action_type in ("unknown",)
195+
196+
# Binary-download verification runs for every action type. JS actions
197+
# can and do shell out to fetch pre-built binaries (e.g. platform-
198+
# specific CLIs) from the action's own `run:` blocks or Dockerfile,
199+
# and we want to flag those when they lack a checksum/signature step.
200+
if check_binary_downloads:
201+
bd_warnings, bd_failures = analyze_binary_downloads_recursive(
202+
org, repo, commit_hash, sub_path,
203+
)
204+
binary_download_failures.extend(bd_failures)
205+
if bd_failures:
206+
checks_performed.append((
207+
"Binary download verification", "fail",
208+
f"{len(bd_failures)} unverified download(s)",
209+
))
210+
elif bd_warnings:
211+
checks_performed.append((
212+
"Binary download verification", "warn",
213+
f"{len(bd_warnings)} download(s); verification present",
214+
))
215+
else:
216+
checks_performed.append((
217+
"Binary download verification", "pass",
218+
"no downloads or all verified",
219+
))
220+
else:
221+
checks_performed.append((
222+
"Binary download verification", "skip",
223+
"disabled via --no-binary-download-check",
224+
))
225+
195226
if not is_js_action:
196227
console.print()
197228
console.print(
@@ -273,34 +304,6 @@ def verify_single_action(
273304
f"{len(repo_warnings)} warning(s)" if repo_warnings else "ok",
274305
))
275306

276-
if check_binary_downloads:
277-
bd_warnings, bd_failures = analyze_binary_downloads_recursive(
278-
org, repo, commit_hash, sub_path,
279-
)
280-
non_js_warnings.extend(bd_warnings)
281-
non_js_warnings.extend(bd_failures)
282-
binary_download_failures.extend(bd_failures)
283-
if bd_failures:
284-
checks_performed.append((
285-
"Binary download verification", "fail",
286-
f"{len(bd_failures)} unverified download(s)",
287-
))
288-
elif bd_warnings:
289-
checks_performed.append((
290-
"Binary download verification", "warn",
291-
f"{len(bd_warnings)} download(s); verification present",
292-
))
293-
else:
294-
checks_performed.append((
295-
"Binary download verification", "pass",
296-
"no downloads or all verified",
297-
))
298-
else:
299-
checks_performed.append((
300-
"Binary download verification", "skip",
301-
"disabled via --no-binary-download-check",
302-
))
303-
304307
if non_js_warnings:
305308
console.print()
306309
console.print(

0 commit comments

Comments
 (0)