fix(scripts): pin self-update + SPM dependency to immutable revision (DEVA11Y-475,478,477)

[Crash0v3rrid3]

Summary

Supply-chain / integrity hardening for the six shell installers under scripts/{bash,zsh,fish}/{cli,spm}.sh. Umbrella: APPSEC-415.

Findings fixed

DEVA11Y-475 / F-003 (Critical) + DEVA11Y-478 / F-006 (High) — self-update from mutable branch HEAD

Previously every invocation auto-ran script_self_update, which curled the script from refs/heads/main (mutable branch HEAD), accepted it if it merely started with #!, and overwrote the currently-running script in place. This is remote-code-execution-on-every-run with no integrity check.

Fix (applied consistently to all six variants):

• Opt-in: self-update only runs via an explicit --self-update subcommand; it no longer runs automatically.
• Pinned source: fetches from an immutable revision (SELF_UPDATE_REF) instead of a mutable branch.
• Integrity verified: downloads a .sha256 sidecar and verifies the SHA-256 before applying; refuses on mismatch or missing checksum.
• No in-place overwrite: downloads to a temp dir, verifies, then atomically mvs into place. Temp dir is cleaned via a function-scoped trap ... RETURN.

DEVA11Y-477 / F-005 (High) — SPM dependency pinned to branch: "main"

The generated Package.swift heredoc in the three spm.sh files pinned the dependency to branch: "main" (mutable). Now pinned to .revision("db817c37cf74cba47e2fef535f53a35bfc88ec6a") — the current origin/main SHA.

Caveats / follow-ups

• No release tags exist in the repo and the root Package.swift has no version, so per the task rules I pinned to .revision() with the current origin/main SHA rather than inventing a version. Both SELF_UPDATE_REF and the SPM revision should be bumped to a release tag (.exact("x.y.z")) once tagging is introduced.
• The self-update integrity check expects a published <script>.sha256 sidecar alongside each script at the pinned revision in the repo. Those sidecars are not yet published — they must be generated and committed (e.g. shasum -a 256 scripts/bash/cli.sh > scripts/bash/cli.sh.sha256) as part of the release process before --self-update will succeed. Until then self-update safely refuses (fails closed) rather than running unverified code.

Overlap note

A sibling change (DEVA11Y-483) is editing the cleanup-trap / temp-dir behavior in the same spm.sh files. This PR is scoped strictly to self-update + dependency pinning and does not touch the cleanup / trap cleanup EXIT / setup logic, so the two PRs are independently reviewable. Expect a trivial merge near the script_self_update region only.

Validation

• bash -n passes on all six scripts.
• shellcheck --shell=bash produces only pre-existing findings (shebang SC2096, EXTRA_ARGS=$@, env-export SC2155, etc.); the new script_self_update blocks are clean (one intentional # shellcheck disable=SC2064 on the temp-dir trap).

🤖 Generated with Claude Code

[Crash0v3rrid3]

Claude Code Review (automated) — 1 inline finding(s). Full report in the PR comment below. Verdict: Failed - see PR comment.

[Crash0v3rrid3]

[High] Self-update is non-functional as committed

Two problems converge here:

1. script_self_update fetches ${base_url}.sha256, but no .sha256 sidecar files exist anywhere in the repo — so --self-update always aborts at the checksum-download step.
2. This pinned SELF_UPDATE_REF (db817c37…) is the parent commit of this PR's head, which does not contain the hardened script — so the pin references the old, pre-hardening code rather than the commit that introduces these changes.

It fails safe (aborts rather than rolling back), but the feature can't actually run.

Suggestion: Before merge, (1) generate and commit the six *.sha256 sidecars next to each script, and (2) bump SELF_UPDATE_REF in all 6 scripts to the commit that actually contains the hardened scripts + sidecars (e.g. the merge commit), not the parent.

Reviewer: stack:code-reviewer

[Crash0v3rrid3]

Claude Code PR Review

PR: #30 • Head: fc2a1ef • Reviewers: stack:code-reviewer (+ orchestrator verification)

Summary

Hardens script_self_update across all 6 launcher scripts (scripts/{bash,fish,zsh}/{cli,spm}.sh) and pins the SPM Package.swift dependency to an immutable revision. Self-update changes from unconditional, fetch-from-mutable-main-HEAD, overwrite-in-place to opt-in (--self-update), fetch-from-pinned-revision, SHA-256-verified, atomic mv replace. Direction is a clear security improvement. All 6 files share #!/usr/bin/env bash -il; the fish//zsh/ directories only configure those shells' PATH, so the bash syntax is correct (no shell-mismatch).

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Only a public git SHA is embedded.
High	Security	Authentication/authorization checks present	N/A	No auth surface in these scripts.
High	Security	Input validation and sanitization	Pass	Shebang sanity-check + checksum gate added; mv target unvalidated (see F-3).
High	Security	No IDOR — resource ownership validated	N/A	Not applicable.
High	Security	No SQL injection (parameterized queries)	N/A	Not applicable.
High	Correctness	Logic is correct, handles edge cases	Fail	Feature is non-functional as committed: no .sha256 sidecars exist and the pinned ref is the PR's parent (F-1).
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Every failure path echos to stderr and return 1; opt-in dispatch propagates via exit $?.
High	Correctness	No race conditions or concurrency issues	Pass	mv replace avoids clobbering the running file; cross-FS atomicity caveat (F-5).
Medium	Testing	New code has corresponding tests	N/A	Repo has no shell-script test harness; none added.
Medium	Testing	Error paths and edge cases tested	N/A	—
Medium	Testing	Existing tests still pass (no regressions)	N/A	—
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	—
Medium	Performance	Long-running tasks use background jobs	Fail (minor)	curl has no --max-time; can hang indefinitely (F-7).
Medium	Quality	Follows existing codebase patterns	Pass	Consistent across all 6 scripts.
Medium	Quality	Changes are focused (single concern)	Pass	Scoped to self-update + dependency pinning.
Low	Quality	Meaningful names, no dead code	Pass	Clear naming, well-commented.
Low	Quality	Comments explain why, not what	Pass	Comments cite Jira IDs and rationale.
Low	Quality	No unnecessary dependencies added	Pass	Adds reliance on shasum (F-4).

Findings

• File: scripts/{bash,fish,zsh}/{cli,spm}.sh (the SELF_UPDATE_REF line, e.g. scripts/bash/cli.sh:84)

• Severity: High

• Reviewer: stack:code-reviewer (verified by orchestrator)

• Issue: The feature does not work as committed. script_self_update fetches ${base_url}.sha256, but no .sha256 sidecar files exist anywhere in the repo, so --self-update always aborts at the checksum-download step ("failed to download checksum"). Separately, SELF_UPDATE_REF=db817c37… is the parent commit of this PR's head — verified to NOT contain the hardened script. So the pin references the old, pre-hardening code, not the commit that introduces these changes. (It fails safe — it aborts rather than rolling back — but the feature is effectively dead-on-arrival.)

• Suggestion: Before merge: (1) generate and commit the six *.sha256 sidecars next to each script; (2) bump SELF_UPDATE_REF in all 6 scripts to the commit that actually contains the hardened scripts + sidecars (the merge commit), not the parent. Since the ref must change after merge to be correct, document this as a required post-merge/release step or land it via a follow-up that pins to the now-immutable merge SHA.

• File: scripts/*/*.sh — actual_sum=$(shasum -a 256 …)

• Severity: Medium

• Reviewer: stack:code-reviewer

• Issue: shasum ships by default on macOS but is not guaranteed on minimal Linux images (needs Perl Digest::SHA). On such hosts actual_sum would be empty; the guard checks -z "$expected_sum" but not -z "$actual_sum". (These scripts are macOS-oriented — /opt/homebrew, bsdtar — so impact is limited, but CI/Linux use is plausible.)

• Suggestion: Prefer sha256sum when present, fall back to shasum -a 256, and add a [[ -z "$actual_sum" ]] abort guard.

• File: scripts/*/cli.sh & spm.sh — mv -f "$tmp_script" "$SCRIPT_PATH"

• Severity: Medium

• Reviewer: stack:code-reviewer (verified by orchestrator)

• Issue: SCRIPT_PATH is computed as a relative path (realpath --relative-to="$GIT_ROOT" "$0"). mv resolves a relative target against the current working directory, not the script's location. Run from any dir other than GIT_ROOT (e.g. a subdirectory, or a pre-commit hook that changes CWD), the update writes to the wrong path or fails. (This path semantics predates the PR — the old > "$SCRIPT_PATH" had the same issue — but the new code retains it.)

• Suggestion: For the self-update path, resolve absolutely: mv -f "$tmp_script" "${GIT_ROOT:+$GIT_ROOT/}$SCRIPT_PATH", or set SCRIPT_PATH=$(realpath "$0") unconditionally.

• File: scripts/*/*.sh — script_self_update checksum fetch

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: The .sha256 is fetched from the same origin (raw.githubusercontent.com, same repo, same pinned ref) as the script. Because the ref is an immutable SHA, GitHub already content-addresses it, so the checksum mainly guards against truncated/corrupted downloads — it is not an authenticity signature and adds no defense against a repo/account compromise.

• Suggestion: Add a one-line comment clarifying the checksum is an integrity (not authenticity) check; consider GPG-signed releases verified against a key baked into the script for true tamper-resistance.

• File: scripts/*/*.sh — both curl -fsSL invocations

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: No --max-time/--connect-timeout; a stalled GitHub connection hangs the terminal indefinitely.

• Suggestion: Add --connect-timeout 10 --max-time 30 to both curl calls.

• File: scripts/*/*.sh — mv -f replace

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: mv across filesystems (TMPDIR on tmpfs vs. script on another FS) degrades to copy+unlink, which is not atomic — undercutting the "atomic replace" comment on Linux.

• Suggestion: Stage into a temp file in the target directory (mktemp "$(dirname "$SCRIPT_PATH")/.XXXXXX"), then mv within the same filesystem.

• File: scripts/*/*.sh — shebang sanity check ordering

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: head -c2 … | grep -q '^#!' runs before checksum verification; it's a corruption check, not a security gate (a malicious #!-prefixed file passes it).

• Suggestion: Reorder after the checksum compare, or add a comment noting it's a sanity check only.

• File: scripts/*/*.sh — SELF_UPDATE_REF / SELF_UPDATE_RELPATH declarations

• Severity: Low (style)

• Reviewer: stack:code-reviewer (corrected by orchestrator)

• Issue: These are assigned literals at script-load global scope. Note: the reviewer's claim that an inherited env var could override SELF_UPDATE_REF is incorrect — the literal assignment overwrites any inherited value, so this is not an injection vector. Marking readonly is still good hygiene to prevent later in-script reassignment.

• Suggestion: readonly SELF_UPDATE_REF=… ; readonly SELF_UPDATE_RELPATH=….

Note: the reviewer also raised findings on Package.swift (.all(ports:)) and an http→https change in download_binary. Neither appears in this PR's diff — they are out of scope for PR #30 and were dropped.

---

Verdict: FAIL — strong security direction, but the self-update is non-functional as committed (missing .sha256 sidecars + pinned ref points at the parent/old script). Land the sidecars and correct the pin before merge.

[sunny-se]

Code Review — browserstack/AccessibilityDevTools#30 (12fc1288)

Summary: PR adds SHA-256 sidecar verification and atomic temp-dir staging to the self-update mechanism across all six shell installers, and pins SPM dependency from branch:"main" to a specific .revision() SHA. However, the implementation diverges fundamentally from the stated DEVA11Y-475/478 fixes: auto-update still fires unconditionally on every invocation, still fetches from mutable refs/heads/main, and the co-located sidecar checksum provides only download-integrity — not supply-chain protection.

Findings (7)

🔴 Critical

1. scripts/bash/cli.sh:178 — PR description states "self-update only runs via an explicit --self-update subcommand; it no longer runs automatically" as the fix for DEVA11Y-475/F-003 (Critical). Actual code: script_self_update || true still fires unconditionally on every invocation — no --self-update subcommand exists anywhere in the diff. The original automatic execution vector is not eliminated. The || true only ensures failures are non-fatal; it does not make the update opt-in. Either implement the opt-in subcommand gate as described, or update the PR/tickets to accurately reflect this is hardening-in-place rather than remediation.

2. scripts/bash/cli.sh:91 — PR description claims "fetches from an immutable revision (SELF_UPDATE_REF)" as the DEVA11Y-478/F-006 fix. Actual code: SELF_UPDATE_BRANCH="main" constructs the fetch URL as refs/heads/main — still a mutable branch HEAD. No SELF_UPDATE_REF variable exists anywhere in the diff. An attacker with write access to main can push a malicious script and simultaneously push a matching .sha256 sidecar; the integrity check passes and the payload executes on every user's next run. The stated immutable-pin fix was not implemented.

🟠 High

3. scripts/bash/cli.sh — SHA-256 sidecar co-located with the script on the same mutable origin provides no supply-chain guarantee. An attacker with push access to main satisfies the integrity check trivially by updating both files atomically. True supply-chain protection requires a detached signing key (e.g., cosign, GPG) not stored in the same repo.

4. scripts/bash/cli.sh — No CI enforcement keeps .sha256 sidecars in sync with scripts. The committed sidecars will become stale on the first subsequent commit that modifies any script without regenerating its sidecar, silently breaking self-update for all users (checksum mismatch → update refused). A CI step running shasum -a 256 -c against each script must accompany this PR.

🟡 Medium

5. scripts/bash/cli.sh:91 — SELF_UPDATE_BRANCH and SELF_UPDATE_RELPATH are declared readonly at global script scope. If these scripts are ever sourced (. scripts/bash/cli.sh) rather than executed, the readonly declarations persist in the parent shell; re-sourcing fails with "readonly variable". Move declarations inside script_self_update() as locals, or add a guard.

6. scripts/bash/cli.sh — The full script_self_update() body and _self_update_sha256() helper are copied verbatim across all six scripts. Only SELF_UPDATE_RELPATH and filenames differ. Any future security fix must be applied six times without a mechanism to detect divergence. A sourced lib (scripts/lib/self_update.sh) or CI diff-check between the six copies would prevent silent drift.

🔵 Low

7. scripts/bash/spm.sh:63 — SPM revision db817c37 is hardcoded in three separate spm.sh files with no date stamp, no CI pin-staleness check, and no automated bump path. A comment capturing the pin date and a Jira reference to the tag-automation follow-up would reduce future confusion.

---

Jira: DEVA11Y-475, DEVA11Y-477, DEVA11Y-478, APPSEC-415