From b21717120cf1a47a77245e8ac963a9e24a69198a Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Sun, 24 May 2026 17:35:45 +0000 Subject: [PATCH] Studio: bootstrap install-script allowlist on PR that introduces it The previous commit tightened the install-script gate so a PR cannot add new allowlist entries on its own head branch -- they must already exist on the base ref. That created a chicken-and-egg problem for the very PR that introduces studio/frontend/.install-script-allowlist: base does not have the file yet, so every head entry is "new" and the gate refuses to land. Resolve by making the head-only rejection conditional on the base ref actually having the allowlist file. The workflow signals "missing on base" by removing the temp file after a failed git show (previously it was always truncated to empty, which the checker could not distinguish from an empty-by-intent base). The checker treats a missing base allowlist path as bootstrap mode and accepts the head allowlist as-is for that single run. Once the file exists on base, every subsequent PR is back under the strict head-only rejection rule. --- .github/workflows/security-audit.yml | 13 ++++--- scripts/check_new_install_scripts.py | 54 ++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml index 8cfa78c743..f3f03c9c5d 100644 --- a/.github/workflows/security-audit.yml +++ b/.github/workflows/security-audit.yml @@ -1105,10 +1105,15 @@ jobs: > /tmp/base-package-lock.json # Pull the TRUSTED allowlist from the base ref so a PR cannot # allowlist its own new postinstall dependency in the same diff - # the checker scans. Missing file is OK (empty allowlist). - git show "$BASE_SHA:studio/frontend/.install-script-allowlist" \ - > /tmp/base-install-script-allowlist 2>/dev/null \ - || : > /tmp/base-install-script-allowlist + # the checker scans. If the file does NOT exist on base, REMOVE + # the temp file -- the checker treats a missing base allowlist + # as bootstrap mode (the PR that introduces the file is allowed + # to populate it; once it lands, subsequent PRs must respect + # the head-only rejection rule). + if ! git show "$BASE_SHA:studio/frontend/.install-script-allowlist" \ + > /tmp/base-install-script-allowlist 2>/dev/null; then + rm -f /tmp/base-install-script-allowlist + fi - name: Diff for newly-added install-script deps if: github.event_name == 'pull_request' diff --git a/scripts/check_new_install_scripts.py b/scripts/check_new_install_scripts.py index 80aed26c74..a536c9bbe1 100644 --- a/scripts/check_new_install_scripts.py +++ b/scripts/check_new_install_scripts.py @@ -328,28 +328,52 @@ def main(argv: list[str] | None = None) -> int: 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: + # Bootstrap: if the BASE ref has no allowlist file at all, this is + # the PR that creates it. There is no prior allowlist to diff + # against, and refusing every head entry here would make the gate + # unlandable. The workflow signals "missing on base" by NOT writing + # the temp file (rm -f after a failed ``git show``), which is what + # we detect here. Once the file exists on base, future PRs must + # land allowlist deltas there first. + if not base_allowlist_path.exists(): 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, + f"[install-script-diff] bootstrap: {base_allowlist_path} " + "missing on base; accepting head allowlist as-is for this run.", + flush = True, ) - for entry in added_head_only: - print(f" head-only allowlist entry: {entry}", file = sys.stderr) - return 1 + allowlist = head_allowlist + else: + try: + base_allowlist = _load_allowlist(base_allowlist_path) + except ValueError as exc: + print(f"[install-script-diff] ERROR: {exc}", file = sys.stderr) + return 2 - # Only the trusted base allowlist participates in the skip set. - allowlist = base_allowlist + # 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 dependency. + 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: