Studio: harden install-script gate against PR self-allowlist

Three findings against the install-script allowlist landed by the
HTML/SVG preview PR:

1. Allowlist matched on package name alone, so adding 'esbuild'
   silently approved every future esbuild postinstall version. Pin
   each entry to name@version and reject bare names.

2. The script defaulted the allowlist path to the head checkout's
   .install-script-allowlist, so the same PR that introduced a new
   postinstall dep could allowlist it in the same diff. Source the
   allowlist from the BASE ref instead; any head-only entry fails
   the gate.

3. The security-audit workflow only extracted the BASE package-lock,
   leaving the allowlist defaulted to the PR checkout. Update the
   workflow to also extract the BASE allowlist and pass it through
   --base-allowlist.

The existing esbuild entry is now pinned to esbuild@0.21.5 so the
gate refuses any future esbuild version that has not been
re-eyeballed.
This commit is contained in:
Daniel Han 2026-05-24 16:04:31 +00:00
commit 55e4d90d9e
3 changed files with 90 additions and 26 deletions

View file

@ -236,29 +236,41 @@ def diff_new_install_scripts(base_lock: dict, head_lock: dict) -> list[Finding]:
def _load_allowlist(path: Path) -> set[str]:
"""Read a file of newline-separated package names to skip.
"""Read a file of newline-separated ``name@version`` entries to skip.
Purely opt-in: an entry on its own line whitelists every version
of that package against the new-install-script gate. Lines
starting with ``#`` are comments; blank lines are ignored. The
intent is to triage well-known, eyeballed dev-only deps (vitest's
esbuild, sharp's libvips, etc.) without weakening the gate for
the long tail. Missing or unreadable file means empty allowlist.
Each entry MUST be pinned to an exact version (``esbuild@0.21.5``,
``@scope/pkg@1.2.3``). Bare names are rejected so allowlisting
``esbuild`` cannot silently approve a later malicious
``esbuild@99.0.0`` published by a compromised maintainer -- every
new version requires its own review. Lines starting with ``#`` are
comments; blank lines are ignored. Missing file = empty allowlist.
"""
if not path.exists():
return set()
out: set[str] = set()
try:
for raw in path.read_text(encoding = "utf-8").splitlines():
line = raw.strip()
if not line or line.startswith("#"):
continue
out.add(line)
text = path.read_text(encoding = "utf-8")
except OSError:
return set()
for raw in text.splitlines():
line = raw.strip()
if not line or line.startswith("#"):
continue
name, sep, version = line.rpartition("@")
if not sep or not name or not version:
raise ValueError(
f"{path}: allowlist entry {line!r} must be pinned to an "
"exact version (e.g. 'esbuild@0.21.5'). Bare names are "
"rejected so we cannot silently approve a later release.",
)
out.add(line.lower())
return out
def _finding_allowlist_key(finding: Finding) -> str:
return f"{finding.name}@{finding.version}".lower()
def main(argv: list[str] | None = None) -> int:
parser = argparse.ArgumentParser(
description = (
@ -280,10 +292,20 @@ def main(argv: list[str] | None = None) -> int:
"--allowlist",
default = None,
help = (
"Path to a newline-separated allowlist of package names "
"Path to the HEAD newline-separated 'name@version' allowlist "
"to skip. Defaults to '<head dir>/.install-script-allowlist'."
),
)
parser.add_argument(
"--base-allowlist",
default = None,
help = (
"Path to the TRUSTED BASE allowlist. Defaults to "
"'<base dir>/.install-script-allowlist'. Entries that exist "
"only on HEAD fail the gate so a PR cannot allowlist its "
"own new postinstall dependency."
),
)
args = parser.parse_args(argv)
try:
@ -293,21 +315,54 @@ def main(argv: list[str] | None = None) -> int:
print(f"[install-script-diff] ERROR: {exc}", file = sys.stderr)
return 2
allowlist_path = (
head_allowlist_path = (
Path(args.allowlist)
if args.allowlist
else Path(args.head).parent / ".install-script-allowlist"
)
allowlist = _load_allowlist(allowlist_path)
base_allowlist_path = (
Path(args.base_allowlist)
if args.base_allowlist
else Path(args.base).parent / ".install-script-allowlist"
)
try:
head_allowlist = _load_allowlist(head_allowlist_path)
base_allowlist = _load_allowlist(base_allowlist_path)
except ValueError as exc:
print(f"[install-script-diff] ERROR: {exc}", file = sys.stderr)
return 2
# Refuse a PR that adds new allowlist entries on its own head branch.
# Allowlist deltas must land in a separate, trusted commit on base
# first; otherwise the same PR could approve its own postinstall.
added_head_only = sorted(head_allowlist - base_allowlist)
if added_head_only:
print(
"[install-script-diff] FAIL: install-script allowlist entries "
"must already exist on the base branch; do not let a PR "
"allowlist its own new postinstall dependency.",
file = sys.stderr,
)
for entry in added_head_only:
print(f" head-only allowlist entry: {entry}", file = sys.stderr)
return 1
# Only the trusted base allowlist participates in the skip set.
allowlist = base_allowlist
findings = diff_new_install_scripts(base_lock, head_lock)
if allowlist:
skipped = [f for f in findings if f.name in allowlist]
findings = [f for f in findings if f.name not in allowlist]
skipped = [
f for f in findings if _finding_allowlist_key(f) in allowlist
]
findings = [
f for f in findings if _finding_allowlist_key(f) not in allowlist
]
for f in skipped:
print(
f"[install-script-diff] SKIP {f.name}@{f.version} "
f"(allowlisted via {allowlist_path.name})",
f"[install-script-diff] SKIP {_finding_allowlist_key(f)} "
"(allowlisted via trusted base allowlist)",
flush = True,
)
if not findings: