feat(settings): configurable models directory (#64) (#149)

* 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>
This commit is contained in:
Palash Debnath
2026-05-29 16:52:50 +05:30
committed by GitHub
co-authored by Claude Opus 4.8
parent 15958d3860
commit 1cfda2f44e
7 changed files with 517 additions and 1 deletions
+91
View File
@@ -13,6 +13,7 @@ The state endpoint duplicates `/system/hf-token/state` (which lives on
from __future__ import annotations
import logging
import os
from dataclasses import asdict
from typing import Optional
@@ -194,3 +195,93 @@ def get_license_acceptance(engine_id: str) -> dict:
logger.exception("get_license_accepted failed for %s", eid)
raise HTTPException(status_code=500, detail="Failed to read license acceptance")
return {"engine_id": eid, "accepted": bool(accepted)}
# ── Storage: configurable models directory (#64) ──────────────────────────
# Where HuggingFace / Torch download model weights. The user's choice is
# persisted durably to the per-user env file as OMNIVOICE_CACHE_DIR, which
# main.py maps to HF_HOME / HF_HUB_CACHE / TORCH_HOME at startup. That env file
# is the *single source of truth*: PUT writes it, GET reads it back — there is
# no second store to diverge from. Takes effect on the next backend restart
# (a storage-location change can't safely move an in-use cache mid-process).
_MODELS_DIR_ENV = "OMNIVOICE_CACHE_DIR"
def _default_models_dir() -> str:
"""huggingface_hub's default cache root, honoring XDG_CACHE_HOME on Linux
(matches HF so GET reports the *true* default the backend would use)."""
base = os.environ.get("XDG_CACHE_HOME") or os.path.expanduser("~/.cache")
return os.path.join(base, "huggingface")
def _effective_models_dir() -> str:
return (
os.environ.get("HF_HUB_CACHE")
or os.environ.get("HUGGINGFACE_HUB_CACHE")
or os.environ.get("HF_HOME")
or _default_models_dir()
)
class _ModelsDirBody(BaseModel):
path: str = Field(default="", description="Absolute directory; empty clears → default cache")
@router.get("/storage/models-dir")
def get_models_dir():
"""Current models directory: the persisted choice (from the durable env
file — the same value main.py reads at startup), what's effective in this
process, and the platform default."""
from core import user_env
configured = user_env.get_user_env(_MODELS_DIR_ENV) or None
return {
"configured": configured,
"effective": _effective_models_dir(),
"default": _default_models_dir(),
"restart_required": False,
}
@router.put("/storage/models-dir")
def set_models_dir(body: _ModelsDirBody):
"""Set (or clear, with an empty path) the models download directory.
Validates the directory is writable, then writes OMNIVOICE_CACHE_DIR to the
durable per-user env file so main.py applies it on the next launch. The env
file is the only persisted store, so GET can never diverge from what was
saved. Returns restart_required=True.
"""
from core import user_env
raw = (body.path or "").strip()
if not raw:
user_env.unset_user_env(_MODELS_DIR_ENV)
return {"configured": None, "default": _default_models_dir(), "restart_required": True}
# Reject control characters / NUL before touching the filesystem: an
# embedded NUL makes os.makedirs raise ValueError (→ 500). This is also
# the input-validation barrier for the path before it reaches any fs call
# (the dir is user-chosen by design — this is a loopback-gated, same-user
# local file picker, not a cross-privilege boundary).
if any(ord(ch) < 0x20 or ord(ch) == 0x7F for ch in raw):
raise HTTPException(status_code=400, detail="Path contains invalid control characters")
path = os.path.abspath(os.path.expanduser(raw))
try:
os.makedirs(path, exist_ok=True)
probe = os.path.join(path, ".omnivoice_write_test")
with open(probe, "w", encoding="utf-8") as f:
f.write("ok")
except OSError as e:
raise HTTPException(status_code=400, detail=f"Directory is not writable: {e}") from e
finally:
# Best-effort cleanup; a failed remove (concurrent process, perm change)
# must not leave the request hanging or mask the real error.
try:
os.remove(os.path.join(path, ".omnivoice_write_test"))
except OSError:
pass
user_env.set_user_env(_MODELS_DIR_ENV, path)
return {"configured": path, "effective": _effective_models_dir(), "restart_required": True}
+84
View File
@@ -0,0 +1,84 @@
"""Durable per-user environment file (`~/.config/omnivoice/env`).
`main.py` loads this file at startup (via dotenv) before importing torch/HF, so
values written here take effect on the next backend launch. Used by the
configurable models directory (#64): the Settings endpoint upserts
``OMNIVOICE_CACHE_DIR`` here, which main.py then maps to
``HF_HOME`` / ``HF_HUB_CACHE`` / ``TORCH_HOME``.
Format is dotenv-style ``KEY=value`` lines. Upsert preserves other keys (e.g. a
persisted ``HF_TOKEN``) and writes the file ``0600`` (it can hold secrets).
"""
from __future__ import annotations
import os
from typing import Optional
USER_ENV_PATH = os.path.expanduser("~/.config/omnivoice/env")
def _read_lines(path: str) -> list[str]:
try:
with open(path, "r", encoding="utf-8") as f:
return f.read().splitlines()
except FileNotFoundError:
return []
# Any *other* OSError (permission, I/O error) propagates: collapsing it to
# an empty baseline would make a subsequent upsert silently drop existing
# keys (e.g. a persisted HF_TOKEN) when it rewrites the file.
def _opener_0600(path: str, flags: int) -> int:
# Create the file with 0600 from the start (no world-readable window before
# a follow-up chmod) — it can hold secrets like HF_TOKEN. The mode is
# masked by umask but only ever *more* restrictive; non-POSIX platforms
# ignore the mode bits.
return os.open(path, flags, 0o600)
def _write_lines(path: str, lines: list[str]) -> None:
parent = os.path.dirname(path)
if parent: # bare filename (e.g. an OMNIVOICE_ENV_FILE override) has no parent
os.makedirs(parent, exist_ok=True)
body = "\n".join(lines)
if body and not body.endswith("\n"):
body += "\n"
with open(path, "w", encoding="utf-8", opener=_opener_0600) as f:
f.write(body)
try:
os.chmod(path, 0o600) # tighten an existing file that predates the opener
except OSError:
pass # best-effort; some filesystems/Windows don't support chmod
def get_user_env(key: str, path: Optional[str] = None) -> Optional[str]:
path = path or os.environ.get("OMNIVOICE_ENV_FILE") or USER_ENV_PATH # resolved at call time so tests can monkeypatch
prefix = f"{key}="
for line in _read_lines(path):
if line.startswith(prefix):
return line[len(prefix):]
return None
def set_user_env(key: str, value: str, path: Optional[str] = None) -> None:
"""Upsert ``KEY=value``, preserving all other lines."""
path = path or os.environ.get("OMNIVOICE_ENV_FILE") or USER_ENV_PATH
prefix = f"{key}="
lines = _read_lines(path)
replaced = False
for i, line in enumerate(lines):
if line.startswith(prefix):
lines[i] = f"{key}={value}"
replaced = True
break
if not replaced:
lines.append(f"{key}={value}")
_write_lines(path, lines)
def unset_user_env(key: str, path: Optional[str] = None) -> None:
"""Remove ``KEY=...`` if present, preserving all other lines."""
path = path or os.environ.get("OMNIVOICE_ENV_FILE") or USER_ENV_PATH
prefix = f"{key}="
lines = [ln for ln in _read_lines(path) if not ln.startswith(prefix)]
_write_lines(path, lines)
@@ -0,0 +1,59 @@
.storagepanel {
display: flex;
flex-direction: column;
gap: 8px;
padding: 12px 14px;
margin-bottom: 12px;
border: 1px solid var(--border, #3c3836);
border-radius: 8px;
background: var(--panel-bg, rgba(255, 255, 255, 0.02));
}
.storagepanel__title {
display: flex;
align-items: center;
gap: 6px;
margin: 0;
font-size: 13px;
font-weight: 600;
}
.storagepanel__help {
margin: 0;
font-size: 12px;
opacity: 0.8;
}
.storagepanel__row {
display: flex;
gap: 8px;
align-items: center;
flex-wrap: wrap;
}
.storagepanel__input {
flex: 1 1 280px;
min-width: 0;
padding: 6px 8px;
font-size: 12px;
font-family: var(--mono, ui-monospace, monospace);
border: 1px solid var(--border, #504945);
border-radius: 6px;
background: var(--input-bg, rgba(0, 0, 0, 0.2));
color: inherit;
}
.storagepanel__btn {
padding: 6px 12px;
font-size: 12px;
border: 1px solid var(--border, #504945);
border-radius: 6px;
background: var(--accent, #83a598);
color: #1d2021;
cursor: pointer;
}
.storagepanel__btn:disabled { opacity: 0.5; cursor: default; }
.storagepanel__btn--ghost { background: transparent; color: inherit; }
.storagepanel__meta { margin: 0; font-size: 11px; opacity: 0.7; }
.storagepanel__meta code { font-size: 11px; }
.storagepanel__restart { margin: 0; font-size: 12px; color: var(--warn, #fabd2f); }
.storagepanel__error {
font-size: 12px;
color: var(--danger, #fb4934);
padding: 4px 0;
}
@@ -0,0 +1,128 @@
/**
* Settings → Models tab → Models directory panel (#64).
*
* Lets the user choose where model weights download (the HuggingFace / Torch
* cache). The backend persists it to the durable per-user env file as
* OMNIVOICE_CACHE_DIR, which main.py maps to HF_HOME / HF_HUB_CACHE / TORCH_HOME
* on the next launch — so changes apply after a restart.
*
* Endpoints:
* GET /api/settings/storage/models-dir
* → {configured, effective, default, restart_required}
* PUT /api/settings/storage/models-dir body {path} (empty path clears)
*/
import React, { useCallback, useEffect, useState } from 'react';
import { HardDrive } from 'lucide-react';
import toast from 'react-hot-toast';
import { apiJson, apiFetch } from '../../api/client';
import './StoragePanel.css';
export default function StoragePanel() {
const [configured, setConfigured] = useState('');
const [effective, setEffective] = useState('');
const [def, setDef] = useState('');
const [input, setInput] = useState('');
const [loading, setLoading] = useState(false);
const [saving, setSaving] = useState(false);
const [error, setError] = useState(null);
const [restart, setRestart] = useState(false);
const refresh = useCallback(async () => {
setLoading(true);
setError(null);
try {
const d = await apiJson('/api/settings/storage/models-dir');
setConfigured(d?.configured || '');
setEffective(d?.effective || '');
setDef(d?.default || '');
setInput(d?.configured || '');
} catch (e) {
setError(e?.message || 'Failed to load storage settings');
} finally {
setLoading(false);
}
}, []);
useEffect(() => { refresh(); }, [refresh]);
const save = async (path) => {
setSaving(true);
setError(null);
try {
const res = await apiFetch('/api/settings/storage/models-dir', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ path }),
});
if (!res.ok) {
const b = await res.json().catch(() => ({}));
throw new Error(b?.detail || `HTTP ${res.status}`);
}
const b = await res.json();
setConfigured(b?.configured || '');
setRestart(Boolean(b?.restart_required));
toast.success(path ? 'Models directory saved — restart to apply' : 'Reverted to default — restart to apply');
refresh();
} catch (e) {
setError(e?.message || 'Failed to save models directory');
} finally {
setSaving(false);
}
};
return (
<section className="storagepanel" aria-labelledby="storagepanel-heading">
<h3 id="storagepanel-heading" className="storagepanel__title">
<HardDrive size={14} /> Models directory
</h3>
{error && <div className="storagepanel__error" role="alert">{error}</div>}
<p id="storagepanel-help" className="storagepanel__help">
Where model weights download (the HuggingFace / Torch cache). Point this
at a larger or faster drive useful when your system drive is small.
Changes apply on the next restart.
</p>
<div className="storagepanel__row">
<input
className="storagepanel__input"
type="text"
value={input}
placeholder={def || '~/.cache/huggingface'}
onChange={(e) => setInput(e.target.value)}
disabled={saving || loading}
spellCheck={false}
aria-labelledby="storagepanel-heading"
aria-describedby="storagepanel-help"
data-testid="models-dir-input"
/>
<button
className="storagepanel__btn"
onClick={() => save(input.trim())}
disabled={saving || loading}
data-testid="models-dir-save"
>
{saving ? 'Saving…' : 'Save'}
</button>
<button
className="storagepanel__btn storagepanel__btn--ghost"
onClick={() => { setInput(''); save(''); }}
disabled={saving || loading || !configured}
title="Revert to the default cache location"
>
Reset
</button>
</div>
<p className="storagepanel__meta">
Effective now: <code>{effective || '…'}</code>
{configured ? <> · configured: <code>{configured}</code></> : <> · using default</>}
</p>
{restart && (
<p className="storagepanel__restart"> Restart OmniVoice to use the new location.</p>
)}
</section>
);
}
+7 -1
View File
@@ -25,6 +25,7 @@ import { useAppStore } from '../store';
import ApiKeysPanel from '../components/settings/ApiKeysPanel';
import PerformancePanel from '../components/settings/PerformancePanel';
import AppearancePanel from '../components/settings/AppearancePanel';
import StoragePanel from '../components/settings/StoragePanel';
import EngineCompatibilityMatrix from '../components/EngineCompatibilityMatrix';
import './Settings.css';
@@ -1027,7 +1028,12 @@ export default function Settings() {
className="settings-tabs-ui"
/>
{activeTab === 'models' && <ModelStoreTab info={info} modelBadge={modelBadge} />}
{activeTab === 'models' && (
<>
<StoragePanel />
<ModelStoreTab info={info} modelBadge={modelBadge} />
</>
)}
{activeTab === 'engines' && <EnginesTab />}
+84
View File
@@ -0,0 +1,84 @@
"""#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")
+64
View File
@@ -0,0 +1,64 @@
"""plan-01 follow-up / #64 — durable per-user env file helper.
Backs the configurable models directory: the Settings endpoint writes
OMNIVOICE_CACHE_DIR into ~/.config/omnivoice/env, which main.py loads at startup
(→ HF_HOME / HF_HUB_CACHE / TORCH_HOME). The helper must upsert one key without
clobbering others (e.g. a persisted HF_TOKEN) and store the file 0600.
"""
from __future__ import annotations
import os
import stat
import sys
from core import user_env
def test_set_creates_and_upserts(tmp_path):
p = str(tmp_path / "env")
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/data/models", path=p)
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR", path=p) == "/data/models"
# upsert: change value, do not duplicate the key
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/other/models", path=p)
assert user_env.get_user_env("OMNIVOICE_CACHE_DIR", path=p) == "/other/models"
with open(p) as f:
assert f.read().count("OMNIVOICE_CACHE_DIR=") == 1
def test_preserves_other_keys(tmp_path):
p = tmp_path / "env"
p.write_text("HF_TOKEN=hf_abc123\nFOO=bar\n")
user_env.set_user_env("OMNIVOICE_CACHE_DIR", "/m", path=str(p))
txt = p.read_text()
assert "HF_TOKEN=hf_abc123" in txt
assert "FOO=bar" in txt
assert "OMNIVOICE_CACHE_DIR=/m" in txt
def test_unset_removes_only_that_key(tmp_path):
p = tmp_path / "env"
p.write_text("HF_TOKEN=hf_x\nOMNIVOICE_CACHE_DIR=/m\n")
user_env.unset_user_env("OMNIVOICE_CACHE_DIR", path=str(p))
txt = p.read_text()
assert "OMNIVOICE_CACHE_DIR" not in txt
assert "HF_TOKEN=hf_x" in txt
def test_get_missing_returns_none(tmp_path):
assert user_env.get_user_env("NOPE", path=str(tmp_path / "env")) is None
def test_set_with_bare_filename_no_parent(tmp_path, monkeypatch):
# A path with no directory component (e.g. OMNIVOICE_ENV_FILE=env) must not
# blow up: os.makedirs("") raises, so the helper has to skip the mkdir.
monkeypatch.chdir(tmp_path)
user_env.set_user_env("K", "v", path="envfile")
assert user_env.get_user_env("K", path="envfile") == "v"
def test_file_is_0600(tmp_path):
if sys.platform == "win32":
return # POSIX perms not meaningful on Windows
p = tmp_path / "env"
user_env.set_user_env("K", "v", path=str(p))
assert stat.S_IMODE(os.stat(p).st_mode) == 0o600