Skip to content

fix(security): remove insecure self-update from cli.sh scripts [DEVA11Y-475]#16

Open
sunny-se wants to merge 2 commits into
mainfrom
fix/DEVA11Y-475-remove-cli-self-update
Open

fix(security): remove insecure self-update from cli.sh scripts [DEVA11Y-475]#16
sunny-se wants to merge 2 commits into
mainfrom
fix/DEVA11Y-475-remove-cli-self-update

Conversation

@sunny-se
Copy link
Copy Markdown
Collaborator

@sunny-se sunny-se commented May 26, 2026

Summary

  • Removes script_self_update() function and its invocation from all 3 CLI scripts
  • scripts/bash/cli.sh — removed function (lines 81-88) and call (line 95)
  • scripts/zsh/cli.sh — removed function (lines 92-98) and call (line 106)
  • scripts/fish/cli.sh — removed function (lines 93-99) and call (line 107)

Why: script_self_update() fetched the script from a mutable branch head (refs/heads/main) via curl with no integrity verification (CWE-494). The only guard was a ^#! regex — trivially bypassed by any attacker who controls the response. This ran unconditionally before any subcommand, making it a silent supply-chain vector.

Users should update scripts via git pull or their package manager.

Verification

grep -r "script_self_update" scripts/*/cli.sh
# returns nothing

Jira

DEVA11Y-475 — F-003

🤖 Generated with Claude Code

F-003 / DEVA11Y-475 — script_self_update() fetched the script from
a mutable branch head with no integrity verification (CWE-494).
The ^#! regex check is trivially bypassed. Remove self-update
entirely; users should update via git pull or package manager.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sunny-se sunny-se requested a review from a team as a code owner May 26, 2026 08:59
@sunny-se
Copy link
Copy Markdown
Collaborator Author

Suggestion: Keep self-update with HTTPS + SHA-256 integrity check instead of removing it.

Problem with full removal

The script_self_update() function is the only update mechanism for users who installed via the curl-based Xcode Build Phase path (per README). Removing it means those users run stale scripts forever — no way to receive security fixes, rule updates, or bug fixes without manually re-curling.

Proposed alternative

Keep self-update but harden it:

  1. HTTPS-only (already covered by PR fix(security): use HTTPS for binary download in shell scripts [DEVA11Y-474] #13)
  2. SHA-256 checksum verification before overwriting
script_self_update() {
  local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/bash/cli.sh"
  local checksum_url="${remote_url}.sha256"

  local updated_script
  updated_script=$(curl -sfSL "$remote_url") || return 0
  local expected_hash
  expected_hash=$(curl -sfSL "$checksum_url") || return 0

  local actual_hash
  actual_hash=$(echo "$updated_script" | shasum -a 256 | cut -d' ' -f1)

  if [[ "$actual_hash" == "$expected_hash" ]] && [[ $updated_script =~ ^#! ]]; then
    echo "$updated_script" > "$SCRIPT_PATH"
  fi
}

This requires publishing .sha256 sidecar files alongside each script in the repo (one-time addition to release process).

Why this is sufficient

Action

Ship .sha256 sidecar files for each script variant (bash/fish/zsh × cli.sh/spm.sh). Update this PR to use the hardened self-update instead of removing it. Closes F-003 while preserving the update channel for Xcode Build Phase users.

Same approach should apply to PR #18 (DEVA11Y-478 — spm.sh self-update).

Instead of removing script_self_update() entirely, restore it with
hardening: HTTPS-only fetch (pairs with PR #13) + SHA-256 checksum
verification against a .sha256 sidecar file. Silently skips update
if checksum fetch fails or hash mismatch — never overwrites with
unverified content.

Requires publishing .sha256 sidecar files alongside each script.

DEVA11Y-475

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant