Palash DebnathandClaude Opus 4.8
1cfda2f44e
* feat(settings): configurable models directory (#64) Let users pick where model weights download (the HuggingFace / Torch cache) instead of being pinned to ~/.cache/huggingface — useful when the system drive is small or slow. Backend: - core/user_env.py: durable per-user env file (~/.config/omnivoice/env) helper with upsert/unset that preserves other keys and writes 0600. main.py already loads this at startup before importing torch/HF, so the value takes effect on the next launch. Path resolves at call time via an OMNIVOICE_ENV_FILE override so it's robust to module re-import in tests. - settings.py: GET/PUT /api/settings/storage/models-dir — validates the dir is writable (mkdir + write-probe → 400 if not), persists the choice, and writes OMNIVOICE_CACHE_DIR to the durable env. Empty path clears → reverts to default. Returns restart_required since an in-use cache can't be safely moved mid-process. Loopback-gated like the other settings. Frontend: - StoragePanel: Models tab panel to view/set/reset the directory, shows effective vs configured vs default + a restart note. Cross-platform default parity preserved (default cache path is the HF default on every OS); local-first (no network); backward-compatible (absent setting → existing behavior). No version bump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#64): harden models-dir input + clear CodeQL hygiene flags - settings.py: reject control/NUL chars in the path with a 400 before any filesystem call (an embedded NUL otherwise raised ValueError → 500). Also serves as the explicit input-validation barrier for the user-chosen path (loopback-gated same-user local file picker — no cross-privilege boundary). - test_user_env.py: use `with open(...)` so the file is closed and the assert has no side effects. - user_env.py: comment the best-effort chmod except clause. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(#64): single source of truth for models dir + review fixes Address CodeRabbit + Greptile review on PR #149: - P1 (both bots): the settings_store copy of the models dir was only ever read by this GET endpoint, so it was a redundant cache that could diverge from the durable env file (the value main.py actually reads). Drop it — the per-user env file (OMNIVOICE_CACHE_DIR) is now the single source of truth: PUT writes it, GET reads it back. No divergence possible. - XDG-aware default (CodeRabbit): _default_models_dir now honors XDG_CACHE_HOME, matching huggingface_hub's real default on Linux. - Atomic 0600 write (Greptile, security): user_env writes via an os.open opener that creates the file 0600 from the start — no world-readable window before chmod for a file that can hold HF_TOKEN. - _read_lines only swallows FileNotFoundError; other OSErrors propagate so an upsert can't silently drop existing keys on a transient read failure. - Guard makedirs("") when the env path is a bare filename (no parent). - Best-effort write-probe cleanup in a finally; raise ... from e. - a11y: label the models-dir input via aria-labelledby/aria-describedby. - OS-neutral unwritable-dir test (mock makedirs) instead of Unix-only /dev/null path semantics. 12 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
85 lines
3.2 KiB
Python
85 lines
3.2 KiB
Python
"""#64 — the configurable models-dir settings endpoints (validate + persist +
|
|
write the durable env that main.py reads at startup).
|
|
|
|
Single source of truth: the durable per-user env file (``OMNIVOICE_CACHE_DIR``).
|
|
``main.py`` reads it at launch; the GET endpoint reads it back. There is no
|
|
second store to diverge from.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
|
|
import fastapi
|
|
import pytest
|
|
|
|
from core import user_env
|
|
from api.routers import settings as s
|
|
|
|
|
|
@pytest.fixture
|
|
def env(tmp_path, monkeypatch):
|
|
# Resolve the durable env file via a process-global override so it survives
|
|
# module re-import: some tests/backend/* tests stub `core.*` in sys.modules,
|
|
# which can give the endpoint's `core.user_env` and this test's a *different*
|
|
# module object — a setattr monkeypatch wouldn't reach the endpoint's copy.
|
|
envfile = str(tmp_path / "env")
|
|
monkeypatch.setenv("OMNIVOICE_ENV_FILE", envfile)
|
|
return envfile
|
|
|
|
|
|
def test_set_persists_and_writes_durable_env(env, tmp_path):
|
|
target = str(tmp_path / "models")
|
|
res = s.set_models_dir(s._ModelsDirBody(path=target))
|
|
abs_target = os.path.abspath(target)
|
|
assert res["configured"] == abs_target
|
|
assert res["restart_required"] is True
|
|
# main.py reads this on next launch; GET reads it back — single source:
|
|
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR") == abs_target
|
|
assert s.get_models_dir()["configured"] == abs_target
|
|
assert os.path.isdir(target)
|
|
|
|
|
|
def test_rejects_unwritable_dir(env, monkeypatch, tmp_path):
|
|
# OS-neutral: force the mkdir to fail rather than relying on Unix-only
|
|
# /dev/null path semantics (cross-platform parity).
|
|
def boom(*a, **k):
|
|
raise OSError("read-only filesystem")
|
|
|
|
monkeypatch.setattr(os, "makedirs", boom)
|
|
with pytest.raises(fastapi.HTTPException) as ei:
|
|
s.set_models_dir(s._ModelsDirBody(path=str(tmp_path / "ro")))
|
|
assert ei.value.status_code == 400
|
|
|
|
|
|
def test_rejects_path_with_null_byte(env):
|
|
# An embedded NUL would otherwise blow up os.makedirs with a ValueError
|
|
# (→ 500). Validate up front and return a clean 400 instead.
|
|
with pytest.raises(fastapi.HTTPException) as ei:
|
|
s.set_models_dir(s._ModelsDirBody(path="/tmp/mo\x00dels"))
|
|
assert ei.value.status_code == 400
|
|
|
|
|
|
def test_clear_reverts_to_default(env):
|
|
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/old")
|
|
res = s.set_models_dir(s._ModelsDirBody(path=""))
|
|
assert res["configured"] is None
|
|
assert res["restart_required"] is True
|
|
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR") is None
|
|
assert s.get_models_dir()["configured"] is None
|
|
|
|
|
|
def test_get_shape(env):
|
|
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/configured")
|
|
res = s.get_models_dir()
|
|
assert res["configured"] == "/configured"
|
|
assert "effective" in res and "default" in res
|
|
|
|
|
|
def test_default_is_xdg_aware(env, monkeypatch, tmp_path):
|
|
# huggingface_hub's default cache root honors XDG_CACHE_HOME on Linux.
|
|
monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "xdg"))
|
|
for var in ("HF_HUB_CACHE", "HUGGINGFACE_HUB_CACHE", "HF_HOME"):
|
|
monkeypatch.delenv(var, raising=False)
|
|
default = s.get_models_dir()["default"]
|
|
assert default == str(tmp_path / "xdg" / "huggingface")
|