Skip to content

Commit 8ad59bb

Browse files
committed
fix(security): honour cancel during hash verify and failed apply
Failed downloads no longer report success: the settings worker re-raises so the dialog can show the error. Cancel aborts SHA256 hashing and skips install/extract. The installer now refuses symlink zip members, Whisper pin refresh tolerates a missing Content-Length, and Portuguese VOSK keeps the published Facebook zip because vosk-model-pt-0.4 404s on Alphacephei. Co-authored-by: jatinkrmalik <jatinkrmalik@gmail.com>
1 parent e94389f commit 8ad59bb

9 files changed

Lines changed: 180 additions & 16 deletions

install.sh

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3408,6 +3408,27 @@ verify_zip_members_safe() {
34083408
return 1
34093409
fi
34103410

3411+
# Match the runtime extractor: refuse symlink members (unzip -Z1 only lists
3412+
# names, so a link to /etc/passwd would otherwise pass the path check).
3413+
if ! command -v python3 >/dev/null 2>&1; then
3414+
print_error "python3 is required to inspect archive members"
3415+
return 1
3416+
fi
3417+
python3 -c '
3418+
import sys, zipfile
3419+
with zipfile.ZipFile(sys.argv[1]) as archive:
3420+
for member in archive.infolist():
3421+
if (member.external_attr >> 16) & 0xF000 == 0xA000:
3422+
sys.exit(2)
3423+
' "$zip_file" || {
3424+
status=$?
3425+
if [ "$status" -eq 2 ]; then
3426+
print_error "Archive $(basename "$zip_file") contains a symlink; refusing to extract"
3427+
return 1
3428+
fi
3429+
print_error "Could not inspect archive $(basename "$zip_file")"
3430+
return 1
3431+
}
34113432
return 0
34123433
}
34133434

scripts/update_model_hashes.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,25 @@ def collect_whisper() -> Dict[str, dict]:
9999
logger.info("HEAD %s", url)
100100
response = requests.head(url, timeout=HTTP_TIMEOUT, allow_redirects=True)
101101
response.raise_for_status()
102+
content_length = response.headers.get("content-length")
103+
if not content_length:
104+
logger.warning(
105+
"HEAD for Whisper %s omitted Content-Length; leaving any existing pin untouched",
106+
size,
107+
)
108+
continue
109+
try:
110+
size_bytes = int(content_length)
111+
except ValueError:
112+
logger.warning(
113+
"HEAD for Whisper %s had invalid Content-Length %r; leaving existing pin",
114+
size,
115+
content_length,
116+
)
117+
continue
102118
digests[f"{size}.pt"] = {
103119
"sha256": sha256,
104-
"size": int(response.headers["content-length"]),
120+
"size": size_bytes,
105121
}
106122
logger.info("Pinned %d Whisper models", len(digests))
107123
return digests

src/vocalinux/speech_recognition/recognition_manager.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,6 +1889,14 @@ def _report_download_status(self, status: str, fraction: float = 0.0) -> None:
18891889
if self._download_progress_callback:
18901890
self._download_progress_callback(fraction, 0.0, status)
18911891

1892+
def _raise_if_download_cancelled(self) -> None:
1893+
"""Abort the current download if the user has already hit Cancel."""
1894+
if self._download_cancelled:
1895+
raise RuntimeError("Download cancelled")
1896+
1897+
def _download_was_cancelled(self) -> bool:
1898+
return self._download_cancelled
1899+
18921900
def _interruptible_pause(self, seconds: float) -> None:
18931901
"""Sleep in short slices so Cancel can abort between UI stages.
18941902
@@ -1897,8 +1905,7 @@ def _interruptible_pause(self, seconds: float) -> None:
18971905
"""
18981906
deadline = time.time() + seconds
18991907
while True:
1900-
if self._download_cancelled:
1901-
raise RuntimeError("Download cancelled")
1908+
self._raise_if_download_cancelled()
19021909
remaining = deadline - time.time()
19031910
if remaining <= 0:
19041911
return
@@ -1929,12 +1936,15 @@ def _verify_download_with_status(self, filepath: str, model_type: str, filename:
19291936
out. Unpinned models (non-strict mode) get an honest "skipping" status
19301937
so the UI never claims a verification that did not happen.
19311938
"""
1939+
self._raise_if_download_cancelled()
19321940
watching = self._download_progress_callback is not None
19331941
pinned = get_pinned_digest(model_type, filename)
1942+
abort = self._download_was_cancelled
19341943

19351944
if pinned is None:
19361945
# verify_downloaded_model will warn (or raise in strict mode).
1937-
verify_downloaded_model(filepath, model_type, filename)
1946+
verify_downloaded_model(filepath, model_type, filename, should_abort=abort)
1947+
self._raise_if_download_cancelled()
19381948
self._report_download_status("No pinned digest — skipping hash check", 1.0)
19391949
if watching:
19401950
self._interruptible_pause(0.35)
@@ -1945,7 +1955,8 @@ def _verify_download_with_status(self, filepath: str, model_type: str, filename:
19451955
# we burn CPU hashing; large models take long enough on their own.
19461956
if watching:
19471957
self._interruptible_pause(0.3)
1948-
verify_downloaded_model(filepath, model_type, filename)
1958+
verify_downloaded_model(filepath, model_type, filename, should_abort=abort)
1959+
self._raise_if_download_cancelled()
19491960
self._report_download_status("Hash matches — integrity verified", 1.0)
19501961
if watching:
19511962
self._interruptible_pause(0.5)
@@ -2071,6 +2082,7 @@ def _download_whispercpp_model(self):
20712082
self._announce_secured_download("whispercpp", os.path.basename(model_path))
20722083
self._stream_model_download(url, temp_file)
20732084
self._verify_download_with_status(temp_file, "whispercpp", os.path.basename(model_path))
2085+
self._raise_if_download_cancelled()
20742086
os.rename(temp_file, model_path)
20752087
logger.info("whisper.cpp model downloaded successfully")
20762088

@@ -2249,6 +2261,7 @@ def _download_vosk_model(self):
22492261
logger.info(f"Download progress: {progress * 100:.1f}% - {status}")
22502262

22512263
self._verify_download_with_status(zip_path, "vosk", os.path.basename(zip_path))
2264+
self._raise_if_download_cancelled()
22522265

22532266
# Update status for extraction phase
22542267
if self._download_progress_callback:
@@ -2366,6 +2379,7 @@ def _download_whisper_model(self, cache_dir: str):
23662379
logger.info(f"Download progress: {progress * 100:.1f}% - {status}")
23672380

23682381
self._verify_download_with_status(temp_file, "whisper", f"{self.model_size}.pt")
2382+
self._raise_if_download_cancelled()
23692383

23702384
# Rename temp file to final
23712385
os.rename(temp_file, model_file)

src/vocalinux/ui/settings_dialog.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4967,7 +4967,7 @@ def check_cancelled():
49674967
cancel_check_id = GLib.timeout_add(100, check_cancelled)
49684968

49694969
try:
4970-
self._apply_settings_internal(settings)
4970+
self._apply_settings_internal(settings, show_errors=False)
49714971
GLib.idle_add(download_dialog.set_complete, True, "")
49724972
GLib.idle_add(self._populate_model_options)
49734973
finally:
@@ -5257,7 +5257,7 @@ def check_cancelled():
52575257
cancel_check_id = GLib.timeout_add(100, check_cancelled)
52585258

52595259
try:
5260-
self._apply_settings_internal(settings)
5260+
self._apply_settings_internal(settings, show_errors=False)
52615261
GLib.idle_add(download_dialog.set_complete, True, "")
52625262
finally:
52635263
GLib.source_remove(cancel_check_id)
@@ -5282,8 +5282,13 @@ def check_cancelled():
52825282

52835283
return self._apply_settings_internal(settings)
52845284

5285-
def _apply_settings_internal(self, settings: dict) -> bool:
5286-
"""Internal method to apply settings."""
5285+
def _apply_settings_internal(self, settings: dict, *, show_errors: bool = True) -> bool:
5286+
"""Internal method to apply settings.
5287+
5288+
When ``show_errors`` is False (the secured-download worker), failures
5289+
propagate so the download dialog can show them. GTK dialogs must not be
5290+
created from that background thread.
5291+
"""
52875292
try:
52885293
self._save_selected_settings(settings)
52895294

@@ -5298,6 +5303,8 @@ def _apply_settings_internal(self, settings: dict) -> bool:
52985303
return True
52995304
except Exception as e:
53005305
logger.error(f"Failed to apply settings: {e}", exc_info=True)
5306+
if not show_errors:
5307+
raise
53015308

53025309
if "whisper" in str(e).lower() and "no module named" in str(e).lower():
53035310
self._show_whisper_install_dialog()

src/vocalinux/utils/model_integrity.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import os
3131
import zipfile
3232
from pathlib import Path
33-
from typing import Optional
33+
from typing import Callable, Optional
3434
from urllib.parse import urlparse
3535

3636
logger = logging.getLogger(__name__)
@@ -102,11 +102,20 @@ def get_pinned_digest(model_type: str, filename: str) -> Optional[dict]:
102102
return None
103103

104104

105-
def sha256_file(filepath: str) -> str:
106-
"""Compute the SHA256 digest of a file."""
105+
def sha256_file(
106+
filepath: str,
107+
should_abort: Optional[Callable[[], bool]] = None,
108+
) -> str:
109+
"""Compute the SHA256 digest of a file.
110+
111+
``should_abort`` is checked between chunks so a UI Cancel during a large
112+
model hash can stop before the file is installed.
113+
"""
107114
digest = hashlib.sha256()
108115
with open(filepath, "rb") as handle:
109116
for chunk in iter(lambda: handle.read(_HASH_CHUNK_BYTES), b""):
117+
if should_abort is not None and should_abort():
118+
raise RuntimeError("Download cancelled")
110119
digest.update(chunk)
111120
return digest.hexdigest()
112121

@@ -116,6 +125,7 @@ def verify_downloaded_model(
116125
model_type: str,
117126
filename: str,
118127
strict: Optional[bool] = None,
128+
should_abort: Optional[Callable[[], bool]] = None,
119129
) -> None:
120130
"""
121131
Check a downloaded model against its pinned digest.
@@ -126,10 +136,13 @@ def verify_downloaded_model(
126136
filename: Registry key for the model, e.g. "ggml-tiny.bin".
127137
strict: Refuse unpinned models. Defaults to the value of
128138
VOCALINUX_STRICT_MODEL_VERIFICATION.
139+
should_abort: Optional callback checked between hash chunks. When it
140+
returns True, raises RuntimeError("Download cancelled").
129141
130142
Raises:
131143
ModelIntegrityError: If the size or digest does not match the pin, or if
132144
the model is unpinned and strict verification is enabled.
145+
RuntimeError: If ``should_abort`` returns True while hashing.
133146
"""
134147
if strict is None:
135148
strict = strict_mode_enabled()
@@ -154,7 +167,7 @@ def verify_downloaded_model(
154167
f"The download is incomplete or the file was replaced upstream."
155168
)
156169

157-
actual_sha256 = sha256_file(filepath)
170+
actual_sha256 = sha256_file(filepath, should_abort=should_abort)
158171
if actual_sha256 != pinned["sha256"]:
159172
raise ModelIntegrityError(
160173
f"SHA256 mismatch for {filename}: expected {pinned['sha256']}, "

src/vocalinux/utils/vosk_model_info.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,9 @@ def vosk_model_url(model_name: str) -> str:
251251
"ko": "vosk-model-small-ko-0.22",
252252
"fa": "vosk-model-fa-0.42",
253253
"pl": "vosk-model-small-pl-0.22",
254-
"pt": "vosk-model-pt-0.4",
254+
# Alphacephei never shipped vosk-model-pt-0.4 (404). The published
255+
# medium/large Portuguese archive is the Facebook-trained zip.
256+
"pt": "vosk-model-pt-fb-v0.1.1-20220516_2113",
255257
"ru": "vosk-model-ru-0.22",
256258
"es": "vosk-model-es-0.42",
257259
"sv": "vosk-model-small-sv-rhasspy-0.15",
@@ -279,7 +281,9 @@ def vosk_model_url(model_name: str) -> str:
279281
"ko": "vosk-model-small-ko-0.22",
280282
"fa": "vosk-model-fa-0.42",
281283
"pl": "vosk-model-small-pl-0.22",
282-
"pt": "vosk-model-pt-0.4",
284+
# Alphacephei never shipped vosk-model-pt-0.4 (404). The published
285+
# medium/large Portuguese archive is the Facebook-trained zip.
286+
"pt": "vosk-model-pt-fb-v0.1.1-20220516_2113",
283287
"ru": "vosk-model-ru-0.22",
284288
"es": "vosk-model-es-0.42",
285289
"sv": "vosk-model-small-sv-rhasspy-0.15",

tests/test_installer_model_verification.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,18 @@ def test_absolute_path_is_refused(self, tmp_path):
148148

149149
assert "FAIL" in result.stdout
150150
assert "unsafe paths" in result.stdout
151+
152+
def test_symlink_member_is_refused(self, tmp_path):
153+
archive = tmp_path / "link.zip"
154+
with zipfile.ZipFile(archive, "w") as zf:
155+
info = zipfile.ZipInfo("model/link")
156+
info.external_attr = (0xA000 | 0o777) << 16
157+
zf.writestr(info, "/etc/passwd")
158+
159+
result = _run(
160+
f'{_PRELUDE}\n{_source("verify_zip_members_safe")}\n'
161+
f'verify_zip_members_safe "{archive}" && echo PASS || echo FAIL'
162+
)
163+
164+
assert "FAIL" in result.stdout
165+
assert "symlink" in result.stdout

tests/test_model_integrity.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
strict_mode_enabled,
2121
verify_downloaded_model,
2222
)
23-
from vocalinux.utils.vosk_model_info import VOSK_MODEL_INFO
23+
from vocalinux.utils.vosk_model_info import VOSK_MODEL_INFO, vosk_model_url
2424
from vocalinux.utils.whisper_model_info import WHISPER_MODEL_URLS
2525
from vocalinux.utils.whispercpp_model_info import WHISPERCPP_MODEL_INFO
2626

@@ -74,6 +74,13 @@ def test_whisper_digests_match_the_digest_embedded_in_the_url(self):
7474
url_digest = url.strip("/").split("/")[-2]
7575
assert get_pinned_digest("whisper", f"{size}.pt")["sha256"] == url_digest
7676

77+
def test_portuguese_medium_model_is_the_published_archive(self):
78+
"""vosk-model-pt-0.4 404s; Alphacephei ships the Facebook-trained zip."""
79+
assert VOSK_MODEL_INFO["medium"]["languages"]["pt"] == (
80+
"vosk-model-pt-fb-v0.1.1-20220516_2113"
81+
)
82+
assert get_pinned_digest("vosk", "vosk-model-pt-fb-v0.1.1-20220516_2113.zip")
83+
7784
def test_entries_are_well_formed(self):
7885
registry = load_registry()
7986
for section in ("whispercpp", "whisper", "vosk"):
@@ -107,6 +114,19 @@ def test_matching_file_passes(self, pinned_file):
107114
def test_sha256_file_matches_hashlib(self, pinned_file):
108115
assert sha256_file(str(pinned_file)) == hashlib.sha256(pinned_file.read_bytes()).hexdigest()
109116

117+
def test_sha256_file_can_be_aborted(self, pinned_file):
118+
with pytest.raises(RuntimeError, match="cancelled"):
119+
sha256_file(str(pinned_file), should_abort=lambda: True)
120+
121+
def test_verify_can_be_aborted_while_hashing(self, pinned_file):
122+
with pytest.raises(RuntimeError, match="cancelled"):
123+
verify_downloaded_model(
124+
str(pinned_file),
125+
"whispercpp",
126+
"ggml-test.bin",
127+
should_abort=lambda: True,
128+
)
129+
110130
def test_tampered_contents_are_rejected(self, pinned_file):
111131
pinned_file.write_bytes(b"pretend model weight5") # same length, different bytes
112132
with pytest.raises(ModelIntegrityError, match="SHA256 mismatch"):
@@ -175,6 +195,9 @@ def test_every_shipped_model_url_is_trusted(self):
175195
ensure_trusted_model_url(info["url"])
176196
for url in WHISPER_MODEL_URLS.values():
177197
ensure_trusted_model_url(url)
198+
for tier in VOSK_MODEL_INFO.values():
199+
for name in tier["languages"].values():
200+
ensure_trusted_model_url(vosk_model_url(name))
178201

179202

180203
class TestSafeExtractZip:

tests/test_recognition_manager_downloads.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,33 @@ def test_interruptible_pause_honours_cancel(self):
446446
with pytest.raises(RuntimeError, match="cancelled"):
447447
manager._interruptible_pause(1.0)
448448

449+
def test_verify_honours_cancel_before_hashing(self, tmp_path):
450+
manager = _make_manager(engine="whisper_cpp")
451+
manager._download_cancelled = True
452+
model = tmp_path / "ggml-tiny.bin"
453+
model.write_bytes(b"weights")
454+
with pytest.raises(RuntimeError, match="cancelled"):
455+
manager._verify_download_with_status(str(model), "whispercpp", "ggml-tiny.bin")
456+
457+
def test_verify_honours_cancel_during_hash(self, tmp_path):
458+
manager = _make_manager(engine="whisper_cpp")
459+
model = tmp_path / "ggml-tiny.bin"
460+
model.write_bytes(b"weights")
461+
462+
def cancel_mid_hash(*_args, **_kwargs):
463+
manager._download_cancelled = True
464+
raise RuntimeError("Download cancelled")
465+
466+
with (
467+
patch("time.sleep"),
468+
patch(
469+
"vocalinux.speech_recognition.recognition_manager.verify_downloaded_model",
470+
side_effect=cancel_mid_hash,
471+
),
472+
):
473+
with pytest.raises(RuntimeError, match="cancelled"):
474+
manager._verify_download_with_status(str(model), "whispercpp", "ggml-tiny.bin")
475+
449476

450477
class TestDownloadIntegrityEnforcement:
451478
"""The download paths must refuse artifacts that fail an integrity check."""
@@ -533,6 +560,30 @@ def test_vosk_download_refuses_archive_that_escapes_models_dir(self, tmp_path):
533560
assert not (tmp_path / "pwned.txt").exists()
534561
assert list(models_dir.iterdir()) == [], "the archive must be cleaned up"
535562

563+
def test_whispercpp_cancel_after_verify_does_not_install(self, tmp_path):
564+
"""Cancel during SHA256 verify must not rename the temp file into place."""
565+
manager = _make_manager(engine="whisper_cpp")
566+
manager.model_size = "tiny"
567+
model_file = str(tmp_path / "ggml-tiny.bin")
568+
569+
def verify_then_cancel(*_args, **_kwargs):
570+
manager._download_cancelled = True
571+
572+
with patch.dict("sys.modules", {"requests": self._mock_requests([b"payload"])}):
573+
with patch(
574+
"vocalinux.speech_recognition.recognition_manager.get_model_path",
575+
return_value=model_file,
576+
):
577+
with patch(
578+
"vocalinux.speech_recognition.recognition_manager.verify_downloaded_model",
579+
side_effect=verify_then_cancel,
580+
):
581+
with pytest.raises(RuntimeError, match="cancelled"):
582+
manager._download_whispercpp_model()
583+
584+
assert not os.path.exists(model_file)
585+
assert not os.path.exists(model_file + ".tmp")
586+
536587

537588
class TestAudioReconnection:
538589
"""Test audio reconnection logic."""

0 commit comments

Comments
 (0)