Phase 1 Wave 1: HF token persistence + redactor (closes #35) - #91
Merged
Conversation
…-01-01) Adds the SQLite-backed encrypted settings store that Phase 1 token resolver will read from. Closes the at-rest plaintext risk for HF tokens (T-01-01). - backend/services/settings_store.py: get_hf_token / set_hf_token / clear_hf_token using Fernet symmetric AEAD. Stored value column never contains the literal "hf_" substring. - backend/services/_secret_key.py: per-install Fernet key derived via scrypt(machine-id + 16-byte random salt). machine-id resolution covers macOS (ioreg IOPlatformUUID), Linux (/etc/machine-id and dbus fallback), Windows (HKLM Cryptography MachineGuid via winreg). Final fallback to hostname+user with a warn log. - backend/migrations/versions/0001_phase1_settings_table.py: alembic migration adding `settings(key, value, updated_at)`. Idempotent — checks for an existing table so fresh installs (where _BASE_SCHEMA already created it) and v0.2.7 upgrades both succeed. - backend/core/db.py: _BASE_SCHEMA grows the settings table for fresh installs; init_db() now runs `alembic upgrade head` after the CREATE. - backend/migrations/env.py: honours an externally-set sqlalchemy.url so tests can point alembic at a fixture DB; falls back to core.config DB_PATH for production. - pyproject.toml: cryptography>=41 added explicitly (RESEARCH.md Assumption A1 was checked at execute-time and proved false; the dep was not present transitively, so the install would fail without this). Tests (10 cases, all green): - Round-trip encryption + plaintext-leakage check (T-01-01 invariant) - Salt persistence across clear/set cycles - InvalidToken decrypt path returns None (Open Question #5 resolution) - Concurrent reads consistent under sqlite WAL - Alembic upgrade on a hand-built v0.2.7 fixture DB preserves all existing tables + seeded rows (CLAUDE.md backward-compat constraint) - Alembic downgrade -1 drops only the settings table Refs #35.
… patched Closes the #35 bug class (bare os.environ.get('HF_TOKEN') reads) by routing every backend HF-token consumer through one resolver, and mitigates T-01-02 (info disclosure via logs) by stripping `hf_[A-Za-z0-9]{30,}` substrings from every log record at the root logger. backend/services/token_resolver.py: - resolve(skip) — 3-source cascade (App → Env → HF-CLI), each source validated via huggingface_hub.whoami(); first valid wins. - on_401(active) — invalidate cache and re-resolve skipping the source that just 401'd (AUTH-06). - state() — three SourceState rows for the Settings UI: set, masked preview (hf_…<last 3>), whoami_user, whoami_ok. - save_app_token / clear_app_token — wraps settings_store + calls huggingface_hub.login(add_to_git_credential=False) per Pitfall #2. - 300-second whoami cache so repeated Settings-page renders don't hit the HF API. backend/core/logging_filter.py: - HFTokenRedactor(logging.Filter) — regex `hf_[A-Za-z0-9]{30,}` so real tokens are masked but `hf_hub` / `hf_token` literals survive. - install_redaction_filter() — idempotent attach to root + every handler. backend/main.py: install the redactor at startup, BEFORE the file handler is added. Re-installed after the file handler attaches so the handler-attached filter list includes it too. Read-side call sites patched (per Pitfall #1 — every HF token read must flow through token_resolver.resolve()): - backend/api/routers/dub_core.py:540 (the original #35 site) - backend/api/routers/system.py:38 (_has_hf_token notification) - backend/services/model_manager.py:480 (diarization pipeline auth) - backend/services/sonitranslate.py:143 (Popen env for SoniTranslate child) - backend/services/sonitranslate.py:217 (gradio_client predict call) New endpoint: - GET /system/hf-token/state — returns the 3-source cascade state with masked tokens for the Wave 2 Settings UI panel. Grep gate confirmed clean: zero `os.environ.get("HF_TOKEN")` reads remain outside token_resolver.py. Tests (17 new cases, all green): - tests/backend/services/test_token_resolver.py: priority cascade, 401 skip mid-resolve, on_401 fallback, state() shape, save+login invariant (add_to_git_credential=False), HUGGING_FACE_HUB_TOKEN alias acceptance. - tests/backend/core/test_logging_filter.py: msg + args redaction, multi-token redaction, non-string args pass-through, short-token literals preserved, install_redaction_filter idempotence. Refs #35.
…on (AUTH-03/04)
Backend half of the Wave 2 Settings → API Keys UI plus the AUTH-04
subprocess env-injection invariant.
backend/api/routers/settings.py:
- POST /api/settings/hf-token — body {token: str} → save_app_token
- DELETE /api/settings/hf-token — also_clear_hf_cli query → clear_app_token
- GET /api/settings/hf-token/state — same shape as token_resolver.state()
All three are gated by `Depends(require_loopback)` at the router level
(threat T-01-03 mitigation; non-loopback Host → 403).
backend/main.py: router mounted alongside existing API routers.
Subprocess env injection (AUTH-04, threat T-01-04 disposition=accept):
- backend/services/sonitranslate.py already updated in Task 2 to read
via token_resolver.resolve() and inject HF_TOKEN + YOUR_HF_TOKEN into
the SoniTranslate child env block.
- backend/services/gpu_sandbox.py: NOT patched — the GPU sandbox runs
in-process TTS generation that uses the parent's already-loaded HF
state. Adding env injection there is a no-op (parent and child share
state via multiprocessing.Pipe before any HF API call).
- backend/services/model_manager.py:480 (Task 2): resolves in-process,
no subprocess crosses here.
- backend/api/routers/exports.py: subprocess.Popen calls only spawn
`open` / `explorer` / `xdg-open` — file-manager launchers with no
HF needs. Skipped per Task 3 conservative-patching rule.
So the canonical AUTH-04 site for this milestone is sonitranslate.py.
Future SubprocessBackend work in Phase 2 will inherit the same pattern.
Tests (8 new cases, all green):
- tests/backend/test_engine_spawn_token.py
* POST /hf-token loopback → 200 + state.active == "app"
* POST /hf-token non-loopback → 403 ("loopback origin required")
* DELETE /hf-token clears settings_store + state.active == None
* GET /hf-token/state returns 3 source rows in priority order
* GET /hf-token/state non-loopback → 403
* env block contains HF_TOKEN + YOUR_HF_TOKEN when resolver returns one
* env block does NOT contain an injected empty HF_TOKEN when resolver
returns None
* source-level check that backend/services/sonitranslate.py still
reads via token_resolver.resolve() (regression guard against
silent reverts of the AUTH-04 wiring)
Full Wave 1 test suite: 35/35 green. Phase 0 smoke tests still green.
Refs #35.
Records execution outcome of the 3-task plan: 10 files created, 9 modified, 35 new test cases, 5 read sites patched, grep gate clean. Documents the two Rule-3/Rule-2 deviations applied (cryptography dep, env.py URL override), the subprocess-launcher inventory for Phase 2, and the known stray edit to the main repo's pyproject.toml that needs a one- line user action to revert. Updates STATE.md current-position table, progress bar, and open TODOs to point at Wave 2 (Plan 01-02) and Wave 3 (Plan 01-03) as the next steps.
Contributor
📝 WalkthroughWalkthroughThis PR implements Phase 1 Wave 1: encrypted HF token persistence with a multi-source resolution cascade, Settings API endpoints, logging redaction, and 35 new test cases validating encryption, cascade behavior, and endpoint security. ChangesHF Token Persistence and Settings API
Sequence Diagram(s)sequenceDiagram
participant SettingsUI as Settings UI
participant APIServer as /api/settings endpoint
participant Resolver as token_resolver
participant Store as settings_store
participant HFHub as huggingface_hub
participant HFCli as huggingface-cli
SettingsUI->>APIServer: POST /api/settings/hf-token {token}
APIServer->>Resolver: save_app_token(token)
Resolver->>Store: set_hf_token(token)
Store->>HFHub: Fernet encrypt & store in DB
Resolver->>HFHub: login(token, add_to_git_credential=False)
APIServer->>Resolver: state()
Resolver->>Store: get_hf_token() [app source]
Resolver->>HFHub: whoami(app_token) [validate]
Resolver->>HFHub: get_token() [hf-cli source]
Resolver->>HFHub: whoami(cli_token) [validate]
Resolver-->>APIServer: {active: 'app', sources: [...]}
APIServer-->>SettingsUI: HTTP 200 with state
SettingsUI->>APIServer: GET /api/settings/hf-token/state
APIServer->>Resolver: state()
Resolver-->>APIServer: cached state within 300s
APIServer-->>SettingsUI: HTTP 200 with sources + active
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/main.py (1)
372-375:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same HF-token redaction before writing crash logs.
This path writes the raw request URL and traceback straight to
CRASH_LOG_PATH, so tokens embedded in exception text or query params bypassHFTokenRedactorentirely and still persist on disk.🩹 Suggested direction
- f.write(f"Request: {request.url}\n") - f.write(traceback.format_exc()) + f.write(f"Request: {redact_hf_tokens(str(request.url))}\n") + f.write(redact_hf_tokens(traceback.format_exc()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 372 - 375, Crash logs currently write raw request.url and traceback into CRASH_LOG_PATH inside the with _crash_log_lock block, which can leak HF tokens; before writing, run both request.url and the traceback text through the HF token redactor (use the HFTokenRedactor.redact or equivalent redact method) so any tokens in query params or exception text are removed, then write the redacted_request_url and redacted_traceback to the file; ensure you reference HFTokenRedactor where imports/instances exist and keep the locking and file append behavior unchanged.backend/services/model_manager.py (1)
477-499:⚠️ Potential issue | 🟠 Major | ⚡ Quick winImplement 401 retry path per AUTH-06 requirement.
Per the plan (AUTH-06: "When the active token returns HTTP 401 mid-download, resolver auto-retries the next source"), this function should detect auth failures and fall back to lower-priority sources. Currently, if
Pipeline.from_pretrained()receives a cached token that was revoked mid-flow (within the 300s cache window), the error is caught generically and the function returns None without attemptingtoken_resolver.on_401(resolved.source).Wrap the
Pipeline.from_pretrained()call in a dedicated exception handler that detects 401 responses, callson_401(resolved.source)to invalidate the cache and get the next valid token, then retries once before falling back toNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/model_manager.py` around lines 477 - 499, The current diarization loader catches all exceptions and never triggers the AUTH-06 retry path; modify the block around Pipeline.from_pretrained() so that when calling token_resolver.resolve() you store resolved.token/resolved.source, call Pipeline.from_pretrained(...) in a try and specifically detect an HTTP 401 auth failure from that call (inspect the exception message/type), upon 401 call token_resolver.on_401(resolved.source) to invalidate the cached token, re-call token_resolver.resolve() to obtain a new resolved and retry Pipeline.from_pretrained(...) exactly once before giving up and returning None; keep existing behavior for non-401 exceptions and ensure _diar_pipeline, _lazy_torch(), get_best_device() usage remains unchanged.
🧹 Nitpick comments (3)
.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.md (1)
175-177: ⚡ Quick winAvoid machine-specific absolute paths in committed docs.
This section embeds a local user path (
/Users/user4/...). Prefer a neutral placeholder path to avoid leaking workstation-specific identifiers and to keep instructions portable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.md around lines 175 - 177, The doc embeds a machine-specific absolute path in the "Action item for the user" paragraph and related text (the stray pyproject.toml mention and checkout command) — replace that concrete path with a neutral placeholder (e.g. <MAIN_REPO_PATH> or <WORKTREE_ROOT>) and update the checkout instruction to read generically (git -C <MAIN_REPO_PATH> checkout -- pyproject.toml), ensure references to pyproject.toml and the untracked file .planning/phases/00-gates/VERIFICATION.md remain but without any user-specific directories, and confirm the paragraph clarifies which repo/worktree the user should run the command in using the placeholders.tests/backend/core/test_logging_filter.py (1)
30-35: ⚡ Quick winAdd a true
record.msgredaction case.Line 32 still routes the token through
%sargs, so this test exercises the same branch astest_redacts_args_tuple(). A preformatted message likef"...{VALID_TOKEN}..."is still untested, which leaves the directrecord.msgredaction path uncovered.Suggested change
def test_redacts_msg_substring(redactor_logger, caplog): with caplog.at_level(logging.INFO, logger=redactor_logger.name): - redactor_logger.info("download failed for %s while reading model", VALID_TOKEN) + redactor_logger.info(f"download failed for {VALID_TOKEN} while reading model") text = caplog.records[0].getMessage() assert VALID_TOKEN not in text assert "hf_***REDACTED***" in text🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/backend/core/test_logging_filter.py` around lines 30 - 35, The test currently passes the token via formatting args (exercising args redaction) so add or modify a call in test_redacts_msg_substring to log a preformatted message that contains VALID_TOKEN directly in record.msg (e.g. use an f-string or formatted literal) to exercise the record.msg redaction path; keep the same caplog capture (with caplog.at_level and logger=redactor_logger.name), read text = caplog.records[0].getMessage(), and assert VALID_TOKEN is not in text and that the redacted marker "hf_***REDACTED***" is present so the test verifies direct record.msg redaction.tests/backend/services/test_settings_store.py (1)
242-247: ⚡ Quick winAvoid POSIX-only root detection in
_run_alembic().Line 245 hard-codes
"/"as the filesystem root sentinel. On Windows,os.path.dirname("C:\\") == "C:\\", so ifalembic.iniis not found before the drive root, this loop never terminates.Suggested change
- here = os.path.abspath(os.path.dirname(__file__)) - root = here - while root and root != "/" and not os.path.isfile(os.path.join(root, "alembic.ini")): - root = os.path.dirname(root) - assert os.path.isfile(os.path.join(root, "alembic.ini")), "alembic.ini not found" - cfg = Config(os.path.join(root, "alembic.ini")) + from pathlib import Path + + here = Path(__file__).resolve().parent + root = next((p for p in (here, *here.parents) if (p / "alembic.ini").is_file()), None) + assert root is not None, "alembic.ini not found" + cfg = Config(str(root / "alembic.ini"))As per coding guidelines,
**/*.{py,ts,tsx,js,jsx}: Cross-platform support: Every fix must work on macOS (Apple Silicon + Intel), Windows (x64), and Linux (AppImage + deb) with no platform-only regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/backend/services/test_settings_store.py` around lines 242 - 247, The loop in _run_alembic() uses a POSIX-only sentinel (root != "/") so it can never terminate on Windows drives; change the loop termination to detect when we've reached the filesystem root in a cross-platform way (e.g., compute parent = os.path.dirname(root) and stop when parent == root, or use os.path.ismount(root)), so the walk that updates root from here will break when no further parent exists; update the while condition that currently references "/" to instead compare root to its parent (or use os.path.ismount) so the os.path.isfile(os.path.join(root, "alembic.ini")) check remains correct across platforms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.md:
- Around line 112-115: The fenced code block containing the grep command (grep
-RnE "os\.(environ|getenv).*HF_TOKEN" backend/ --include='*.py' | grep -v
token_resolver.py | grep -v '^[[:space:]]*#') needs a language tag to satisfy
markdownlint MD040; change the opening fence from ``` to ```bash so the snippet
is marked as a bash shell block.
In @.planning/STATE.md:
- Around line 3-4: Update the footer timestamp to match the "Last updated"
header by changing the footer date that currently reads "2026-05-18" to
"2026-05-20" so the "Last updated:" line and the footer are in sync; locate the
"Last updated:" heading and the footer timestamp in STATE.md and replace the
footer date string to match the header.
In `@backend/core/logging_filter.py`:
- Around line 32-57: The filter currently redacts only record.msg and
record.args but leaves exception text in record.exc_info (used by
Formatter.formatException), so capture and redact exceptions too: inside filter
(function filter) detect if record.exc_info is set, produce the exception string
via logging.Formatter().formatException(record.exc_info), run
_HF_TOKEN_RE.sub(REDACTED, that_string), then assign the redacted string to
record.exc_text and clear record.exc_info (or set to None) so formatters
(including the JSON formatter that calls Formatter.formatException) will use the
redacted exception text instead of leaking tokens.
In `@backend/services/_secret_key.py`:
- Around line 63-67: The subprocess call in get_machine_identifiers (the
check_output invocation in backend/services/_secret_key.py) should use the
absolute macOS binary path instead of just "ioreg" to avoid PATH stripping when
launched from Finder/Tauri; update the argument list to use "/usr/sbin/ioreg"
(and optionally check for existence and fall back to the bare "ioreg" if not
found) so the call reliably finds the binary on both Intel and Apple Silicon
macOS systems while leaving non-macOS behavior unchanged.
In `@backend/services/settings_store.py`:
- Around line 76-98: Update set_hf_token to persist the HF token using the
Hugging Face CLI API: call huggingface_hub.login(token=token,
add_to_git_credential=False) (import from huggingface_hub) as the primary
persistence step before/alongside the local DB write; retain the existing
encrypted DB storage (cipher = _fernet(), blob = cipher.encrypt(...)) for
app-specific needs but do not rely on it as the canonical credential store.
Ensure you handle and surface any exceptions from huggingface_hub.login (so
failures don’t silently skip persistence), and keep the existing
clear_hf_token() behavior for empty tokens.
In `@backend/services/token_resolver.py`:
- Around line 3-7: Change the resolution order so environment overrides take
precedence over stored app settings: in the token resolution function (look for
function names like resolve_hf_token / get_hf_token and references to
settings_store.get_hf_token()), check os.getenv('HF_TOKEN') or
os.getenv('HUGGING_FACE_HUB_TOKEN') first, then fall back to
settings_store.get_hf_token(), then huggingface_hub.get_token(); ensure any code
that writes/persists a token (e.g., save_hf_token / settings_store.set_hf_token)
does NOT persist a token when it came from HF_TOKEN (treat HF_TOKEN as
override-only), and update the function docstring/comments to state that
HF_TOKEN is override-only.
- Around line 217-248: save_app_token writes the token both to the encrypted
settings_store and via huggingface_hub.login(), but clear_app_token currently
only clears the app store and therefore leaves the HF CLI token live; fix by
adding provenance tracking so we only undo the HF-CLI side effect when the app
originally created it: update save_app_token to call
settings_store.set_hf_token(...) plus a provenance flag (e.g.,
settings_store.set_hf_token_source or set_hf_token({value:..., source:"app"})),
change clear_app_token to read that provenance and call huggingface_hub.logout()
(or remove the token via the hub API) only when the provenance indicates the app
wrote it, leaving externally-created tokens untouched, and update tests around
resolve()/hf-cli fallback to assert behavior against persisted state rather than
only mocking huggingface_hub.get_token().
In `@tests/backend/services/test_settings_store.py`:
- Around line 111-127: The test test_invalid_token_returns_none_with_warning
currently only asserts the return is None but doesn't verify the promised
warning; after calling settings_store.get_hf_token() add an assertion that
caplog captured a WARNING from the settings_store code (e.g., assert
any(r.levelname == "WARNING" and "hf_token" in r.getMessage() for r in
caplog.records)) so the test fails if the decryption warning is removed or not
emitted. Use the injected caplog and reference the settings_store.get_hf_token
call and caplog.records in the assertion.
---
Outside diff comments:
In `@backend/main.py`:
- Around line 372-375: Crash logs currently write raw request.url and traceback
into CRASH_LOG_PATH inside the with _crash_log_lock block, which can leak HF
tokens; before writing, run both request.url and the traceback text through the
HF token redactor (use the HFTokenRedactor.redact or equivalent redact method)
so any tokens in query params or exception text are removed, then write the
redacted_request_url and redacted_traceback to the file; ensure you reference
HFTokenRedactor where imports/instances exist and keep the locking and file
append behavior unchanged.
In `@backend/services/model_manager.py`:
- Around line 477-499: The current diarization loader catches all exceptions and
never triggers the AUTH-06 retry path; modify the block around
Pipeline.from_pretrained() so that when calling token_resolver.resolve() you
store resolved.token/resolved.source, call Pipeline.from_pretrained(...) in a
try and specifically detect an HTTP 401 auth failure from that call (inspect the
exception message/type), upon 401 call token_resolver.on_401(resolved.source) to
invalidate the cached token, re-call token_resolver.resolve() to obtain a new
resolved and retry Pipeline.from_pretrained(...) exactly once before giving up
and returning None; keep existing behavior for non-401 exceptions and ensure
_diar_pipeline, _lazy_torch(), get_best_device() usage remains unchanged.
---
Nitpick comments:
In
@.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.md:
- Around line 175-177: The doc embeds a machine-specific absolute path in the
"Action item for the user" paragraph and related text (the stray pyproject.toml
mention and checkout command) — replace that concrete path with a neutral
placeholder (e.g. <MAIN_REPO_PATH> or <WORKTREE_ROOT>) and update the checkout
instruction to read generically (git -C <MAIN_REPO_PATH> checkout --
pyproject.toml), ensure references to pyproject.toml and the untracked file
.planning/phases/00-gates/VERIFICATION.md remain but without any user-specific
directories, and confirm the paragraph clarifies which repo/worktree the user
should run the command in using the placeholders.
In `@tests/backend/core/test_logging_filter.py`:
- Around line 30-35: The test currently passes the token via formatting args
(exercising args redaction) so add or modify a call in
test_redacts_msg_substring to log a preformatted message that contains
VALID_TOKEN directly in record.msg (e.g. use an f-string or formatted literal)
to exercise the record.msg redaction path; keep the same caplog capture (with
caplog.at_level and logger=redactor_logger.name), read text =
caplog.records[0].getMessage(), and assert VALID_TOKEN is not in text and that
the redacted marker "hf_***REDACTED***" is present so the test verifies direct
record.msg redaction.
In `@tests/backend/services/test_settings_store.py`:
- Around line 242-247: The loop in _run_alembic() uses a POSIX-only sentinel
(root != "/") so it can never terminate on Windows drives; change the loop
termination to detect when we've reached the filesystem root in a cross-platform
way (e.g., compute parent = os.path.dirname(root) and stop when parent == root,
or use os.path.ismount(root)), so the walk that updates root from here will
break when no further parent exists; update the while condition that currently
references "/" to instead compare root to its parent (or use os.path.ismount) so
the os.path.isfile(os.path.join(root, "alembic.ini")) check remains correct
across platforms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cbaf773-bbb7-4c85-9272-6e4f214a78e9
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.planning/STATE.md.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.mdbackend/api/routers/dub_core.pybackend/api/routers/settings.pybackend/api/routers/system.pybackend/core/db.pybackend/core/logging_filter.pybackend/main.pybackend/migrations/env.pybackend/migrations/versions/0001_phase1_settings_table.pybackend/services/_secret_key.pybackend/services/model_manager.pybackend/services/settings_store.pybackend/services/sonitranslate.pybackend/services/token_resolver.pypyproject.tomltests/backend/__init__.pytests/backend/core/__init__.pytests/backend/core/test_logging_filter.pytests/backend/services/__init__.pytests/backend/services/test_settings_store.pytests/backend/services/test_token_resolver.pytests/backend/test_engine_spawn_token.py
Comment on lines
+112
to
+115
| ``` | ||
| grep -RnE "os\.(environ|getenv).*HF_TOKEN" backend/ --include='*.py' \ | ||
| | grep -v token_resolver.py | grep -v '^[[:space:]]*#' | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced code block.
The block starting at Line 112 is missing a language identifier, which triggers markdownlint MD040. Use ```bash for this shell snippet.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.md
around lines 112 - 115, The fenced code block containing the grep command (grep
-RnE "os\.(environ|getenv).*HF_TOKEN" backend/ --include='*.py' | grep -v
token_resolver.py | grep -v '^[[:space:]]*#') needs a language tag to satisfy
markdownlint MD040; change the opening fence from ``` to ```bash so the snippet
is marked as a bash shell block.
Comment on lines
+3
to
4
| **Last updated:** 2026-05-20 — Phase 1 Wave 1 (Plan 01-01) complete; ready for Wave 2 (Plan 01-02) | ||
|
|
Contributor
There was a problem hiding this comment.
Update the footer timestamp to match the new state date.
Line 3 says the file was updated on 2026-05-20, but the footer still states 2026-05-18 (Lines 116-117). Keep these in sync to avoid session handoff ambiguity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.planning/STATE.md around lines 3 - 4, Update the footer timestamp to match
the "Last updated" header by changing the footer date that currently reads
"2026-05-18" to "2026-05-20" so the "Last updated:" line and the footer are in
sync; locate the "Last updated:" heading and the footer timestamp in STATE.md
and replace the footer date string to match the header.
Comment on lines
+32
to
+57
| def filter(self, record: logging.LogRecord) -> bool: | ||
| # Rewrite the format string itself if it's a str. | ||
| try: | ||
| if isinstance(record.msg, str): | ||
| record.msg = _HF_TOKEN_RE.sub(REDACTED, record.msg) | ||
|
|
||
| # Rewrite per-argument so the formatted final message also | ||
| # comes out clean. logger.info("token=%s", tok). | ||
| if record.args: | ||
| if isinstance(record.args, tuple): | ||
| new_args = tuple( | ||
| _HF_TOKEN_RE.sub(REDACTED, a) if isinstance(a, str) else a | ||
| for a in record.args | ||
| ) | ||
| record.args = new_args | ||
| elif isinstance(record.args, dict): | ||
| record.args = { | ||
| k: (_HF_TOKEN_RE.sub(REDACTED, v) if isinstance(v, str) else v) | ||
| for k, v in record.args.items() | ||
| } | ||
| except Exception: | ||
| # Never let the filter break the log pipeline. If anything | ||
| # unexpected happens, just let the record pass through — | ||
| # erring on the side of the log being noisy, not silent. | ||
| pass | ||
| return True |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import io
import logging
import re
token = "hf_" + "A" * 30
pattern = re.compile(r"hf_[A-Za-z0-9]{30,}")
class MsgOnlyFilter(logging.Filter):
def filter(self, record):
if isinstance(record.msg, str):
record.msg = pattern.sub("hf_***REDACTED***", record.msg)
return True
buf = io.StringIO()
handler = logging.StreamHandler(buf)
logger = logging.getLogger("verify_exc_redaction")
logger.handlers[:] = [handler]
logger.setLevel(logging.INFO)
logger.propagate = False
logger.addFilter(MsgOnlyFilter())
try:
raise RuntimeError(f"token leaked from exception: {token}")
except RuntimeError:
logger.exception("boom")
print(buf.getvalue())
PYRepository: debpalash/OmniVoice-Studio
Length of output: 227
🏁 Script executed:
# First, let's examine the actual HFTokenRedactor.filter() implementation
cat -n backend/core/logging_filter.py | head -80Repository: debpalash/OmniVoice-Studio
Length of output: 3717
🏁 Script executed:
# Next, let's check the _JsonFormatter in backend/main.py
rg -A 30 "_JsonFormatter" backend/main.pyRepository: debpalash/OmniVoice-Studio
Length of output: 3131
🏁 Script executed:
# Let's also check if exc_info redaction happens anywhere else
rg "exc_info" backend/ -A 3Repository: debpalash/OmniVoice-Studio
Length of output: 549
Redact exception text too, not just msg/args.
This filter leaves record.exc_info untouched, so any token embedded in an exception message or traceback still reaches Formatter.formatException. That leak path is live for logger.exception(...), including the JSON formatter in backend/main.py which explicitly calls self.formatException(record.exc_info) without redaction. The module docstring claims to "mitigate threat T-01-02" but exceptions with tokens are not redacted—contradicting both the stated purpose and the coding guideline requiring bug reports to capture errors without leaking tokens.
🧰 Tools
🪛 Ruff (0.15.13)
[error] 52-56: try-except-pass detected, consider logging the exception
(S110)
[warning] 52-52: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/core/logging_filter.py` around lines 32 - 57, The filter currently
redacts only record.msg and record.args but leaves exception text in
record.exc_info (used by Formatter.formatException), so capture and redact
exceptions too: inside filter (function filter) detect if record.exc_info is
set, produce the exception string via
logging.Formatter().formatException(record.exc_info), run
_HF_TOKEN_RE.sub(REDACTED, that_string), then assign the redacted string to
record.exc_text and clear record.exc_info (or set to None) so formatters
(including the JSON formatter that calls Formatter.formatException) will use the
redacted exception text instead of leaking tokens.
Comment on lines
+63
to
+67
| out = subprocess.check_output( | ||
| ["ioreg", "-rd1", "-c", "IOPlatformExpertDevice"], | ||
| stderr=subprocess.DEVNULL, | ||
| timeout=5, | ||
| ).decode("utf-8", errors="replace") |
Contributor
There was a problem hiding this comment.
Use an absolute ioreg path on macOS.
Launching the app from Finder/Tauri often gives a stripped PATH that does not include /usr/sbin, so ["ioreg", ...] can fail on a normal macOS install and silently drop into the hostname/user fallback.
🩹 Suggested fix
if plat == "darwin":
+ ioreg = "/usr/sbin/ioreg"
out = subprocess.check_output(
- ["ioreg", "-rd1", "-c", "IOPlatformExpertDevice"],
+ [ioreg, "-rd1", "-c", "IOPlatformExpertDevice"],
stderr=subprocess.DEVNULL,
timeout=5,
).decode("utf-8", errors="replace")As per coding guidelines, "Cross-platform support: Every fix must work on macOS (Apple Silicon + Intel), Windows (x64), and Linux (AppImage + deb) with no platform-only regressions".
🧰 Tools
🪛 Ruff (0.15.13)
[error] 64-64: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/_secret_key.py` around lines 63 - 67, The subprocess call in
get_machine_identifiers (the check_output invocation in
backend/services/_secret_key.py) should use the absolute macOS binary path
instead of just "ioreg" to avoid PATH stripping when launched from Finder/Tauri;
update the argument list to use "/usr/sbin/ioreg" (and optionally check for
existence and fall back to the bare "ioreg" if not found) so the call reliably
finds the binary on both Intel and Apple Silicon macOS systems while leaving
non-macOS behavior unchanged.
Comment on lines
+76
to
+98
| def set_hf_token(token: str) -> None: | ||
| """Persist an encrypted HF token. The first call also writes the per-install | ||
| salt row inside the same transaction (atomic, no torn state).""" | ||
| if not token: | ||
| # Defense in depth — callers should pass a non-empty string. An empty | ||
| # token in the cascade is the same as "no token" and we should not | ||
| # write a row that round-trips to the empty string. | ||
| clear_hf_token() | ||
| return | ||
|
|
||
| from core.db import db_conn | ||
|
|
||
| # Derive the key first — this lazily generates the salt row on first | ||
| # write, inside its own transaction. Subsequent ops use the cached key. | ||
| cipher = _fernet() | ||
| blob = cipher.encrypt(token.encode("utf-8")).decode("ascii") | ||
|
|
||
| with db_conn() as conn: | ||
| conn.execute( | ||
| "INSERT OR REPLACE INTO settings(key, value, updated_at) " | ||
| "VALUES (?, ?, ?)", | ||
| (_TOKEN_KEY, blob, time.time()), | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the latest huggingface_hub documentation, what does huggingface_hub.login(token=..., add_to_git_credential=False)persist, and is$HF_HOME/token the canonical token store shared with the CLI?
💡 Result:
When you call huggingface_hub.login(token=..., add_to_git_credential=False), the provided token is persisted in the local cache on your machine [1][2]. Specifically, the token is saved to a file (typically located at ~/.cache/huggingface/token, though this path can be influenced by the HF_HOME environment variable) [3][4][5]. By setting add_to_git_credential=False, you explicitly prevent the library from attempting to store the token in your system's git credential helper [1][2]. Regarding your second question, yes, the file at $HF_HOME/token (or the default cache location if HF_HOME is unset) is the canonical token store shared between the huggingface_hub library and the Hugging Face CLI [3][5]. Both the library's login function and the huggingface-cli login command interact with this same file to save and retrieve the active authentication token [6][3][5]. When you switch tokens using the CLI or library, this canonical file is updated to reflect the currently active token [5].
Citations:
- 1: https://huggingface.co/docs/huggingface_hub/v0.32.0/en/package_reference/authentication
- 2: https://huggingface.co/docs/huggingface_hub/en/package_reference/authentication
- 3: https://github.com/huggingface/huggingface_hub/blob/9e46a06f/docs/source/en/guides/cli.md
- 4: Store user token in huggingface home folder huggingface/huggingface_hub#1232
- 5: Support multiple tokens locally huggingface/huggingface_hub#2549
- 6: huggingface-cli login doesn't work huggingface/huggingface_hub#2765
Use huggingface_hub.login() as the primary persistence mechanism for HF tokens.
Lines 76–98 write the user token only to SQLite settings, making the app DB the primary credential store. Per coding guidelines, HF token persistence must use huggingface_hub.login(token=..., add_to_git_credential=False) to persist to $HF_HOME/token, aligning with the CLI and hub ecosystem. Call this function in set_hf_token() to store credentials in the canonical location.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/settings_store.py` around lines 76 - 98, Update set_hf_token
to persist the HF token using the Hugging Face CLI API: call
huggingface_hub.login(token=token, add_to_git_credential=False) (import from
huggingface_hub) as the primary persistence step before/alongside the local DB
write; retain the existing encrypted DB storage (cipher = _fernet(), blob =
cipher.encrypt(...)) for app-specific needs but do not rely on it as the
canonical credential store. Ensure you handle and surface any exceptions from
huggingface_hub.login (so failures don’t silently skip persistence), and keep
the existing clear_hf_token() behavior for empty tokens.
Comment on lines
+3
to
+7
| Resolution priority (highest → lowest): | ||
|
|
||
| 1. app — `settings_store.get_hf_token()` (encrypted in SQLite) | ||
| 2. env — `HF_TOKEN` or the legacy `HUGGING_FACE_HUB_TOKEN` env var | ||
| 3. hf-cli — `huggingface_hub.get_token()` (canonical ~/.cache/huggingface/token) |
Contributor
There was a problem hiding this comment.
Make HF_TOKEN the actual override source.
Line 31 currently puts app ahead of env, so once a token is saved in Settings, HF_TOKEN can no longer override it. That breaks the repo contract for temporary/ops overrides and makes the docstring here impossible to describe as “override-only”.
♻️ Minimal fix
-_PRIORITY: tuple[Source, ...] = ("app", "env", "hf-cli")
+_PRIORITY: tuple[Source, ...] = ("env", "app", "hf-cli")As per coding guidelines, "HuggingFace token persistence: Document HF_TOKEN environment variable as an override-only path, not the primary mechanism for users to set their token".
Also applies to: 31-31, 151-167
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/token_resolver.py` around lines 3 - 7, Change the resolution
order so environment overrides take precedence over stored app settings: in the
token resolution function (look for function names like resolve_hf_token /
get_hf_token and references to settings_store.get_hf_token()), check
os.getenv('HF_TOKEN') or os.getenv('HUGGING_FACE_HUB_TOKEN') first, then fall
back to settings_store.get_hf_token(), then huggingface_hub.get_token(); ensure
any code that writes/persists a token (e.g., save_hf_token /
settings_store.set_hf_token) does NOT persist a token when it came from HF_TOKEN
(treat HF_TOKEN as override-only), and update the function docstring/comments to
state that HF_TOKEN is override-only.
Comment on lines
+217
to
+248
| def save_app_token(token: str) -> None: | ||
| """Persist token to the encrypted settings store AND populate the HF | ||
| canonical file via `huggingface_hub.login()`. Per Pitfall #2: | ||
| `add_to_git_credential=False` is non-negotiable — the alternative | ||
| silently writes the token to the user's global git credential helper, | ||
| which is leaks-galore for a desktop app.""" | ||
| if not token: | ||
| clear_app_token() | ||
| return | ||
| from services import settings_store | ||
| settings_store.set_hf_token(token) | ||
| try: | ||
| import huggingface_hub | ||
| huggingface_hub.login( | ||
| token=token, | ||
| add_to_git_credential=False, | ||
| new_session=False, | ||
| ) | ||
| except TypeError: | ||
| # Older huggingface_hub may not have new_session kwarg — retry | ||
| # without it. The add_to_git_credential=False kwarg is the | ||
| # invariant that matters; new_session is just a perf tweak. | ||
| try: | ||
| import huggingface_hub | ||
| huggingface_hub.login(token=token, add_to_git_credential=False) | ||
| except Exception: | ||
| logger.exception("huggingface_hub.login failed (non-fatal)") | ||
| except Exception: | ||
| # Hub login failure must not strand the user — the token is still | ||
| # in the encrypted store and the resolver will pick it up. | ||
| logger.exception("huggingface_hub.login failed (non-fatal)") | ||
| invalidate_cache() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the token_resolver.py file and examine its structure
fd -t f token_resolver.pyRepository: debpalash/OmniVoice-Studio
Length of output: 151
🏁 Script executed:
# Get an overview of the file and examine the functions mentioned in the review
git ls-files | grep -E "(token|resolver)" | head -20Repository: debpalash/OmniVoice-Studio
Length of output: 868
🏁 Script executed:
# Read the token_resolver.py file to see clear_app_token() and resolve() implementations
cat -n backend/services/token_resolver.py | head -300Repository: debpalash/OmniVoice-Studio
Length of output: 10873
🏁 Script executed:
# Search for tests related to token clearing and persistence
fd -t f -e py test | xargs rg -l "token|clear_app_token|save_app_token" | head -10Repository: debpalash/OmniVoice-Studio
Length of output: 315
🏁 Script executed:
# Read the test file for token_resolver
cat -n tests/backend/services/test_token_resolver.pyRepository: debpalash/OmniVoice-Studio
Length of output: 10731
🏁 Script executed:
# Search for all calls to clear_app_token() in the codebase
rg "clear_app_token" --type py -B 2 -A 2Repository: debpalash/OmniVoice-Studio
Length of output: 2719
🏁 Script executed:
# Search for calls to save_app_token() to understand the usage patterns
rg "save_app_token" --type py -B 2 -A 2Repository: debpalash/OmniVoice-Studio
Length of output: 3063
🏁 Script executed:
# Look at the settings router to understand how clear_app_token is called
cat -n backend/api/routers/settings.py | grep -A 20 "clear_app_token"Repository: debpalash/OmniVoice-Studio
Length of output: 556
🏁 Script executed:
# Check if there's any documentation or planning about the expected behavior
fd -t f PLAN.md RESEARCH.md VALIDATION.md | xargs grep -l "clear\|persist" 2>/dev/null | head -3Repository: debpalash/OmniVoice-Studio
Length of output: 229
🏁 Script executed:
# Look at the relevant planning files mentioned in the token_resolver.py comments
cat -n ".planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-RESEARCH.md" | grep -A 10 -B 5 "Pitfall.*2\|clear"Repository: debpalash/OmniVoice-Studio
Length of output: 11237
clear_app_token() does not undo the HF-CLI side effect from save_app_token() by default.
save_app_token() persists the token to both the encrypted app store and Hugging Face's canonical store via huggingface_hub.login(), but the default clear path only removes the app copy. Since resolve() includes "hf-cli" as a resolver source, the token remains discoverable immediately after a normal clear—the app will continue to authenticate requests using the same persisted token.
The issue is structural: changing the default to call logout() unconditionally risks deleting pre-existing external tokens set via huggingface-cli login, so a solution likely requires provenance tracking to distinguish app-written state from externally-managed tokens, not just flipping the default behavior. The current test suite does not catch this because it mocks huggingface_hub.get_token() rather than testing the resolver's cascade over real persisted state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/token_resolver.py` around lines 217 - 248, save_app_token
writes the token both to the encrypted settings_store and via
huggingface_hub.login(), but clear_app_token currently only clears the app store
and therefore leaves the HF CLI token live; fix by adding provenance tracking so
we only undo the HF-CLI side effect when the app originally created it: update
save_app_token to call settings_store.set_hf_token(...) plus a provenance flag
(e.g., settings_store.set_hf_token_source or set_hf_token({value:...,
source:"app"})), change clear_app_token to read that provenance and call
huggingface_hub.logout() (or remove the token via the hub API) only when the
provenance indicates the app wrote it, leaving externally-created tokens
untouched, and update tests around resolve()/hf-cli fallback to assert behavior
against persisted state rather than only mocking huggingface_hub.get_token().
Comment on lines
+111
to
+127
| def test_invalid_token_returns_none_with_warning(isolated_db, caplog): | ||
| """If the encrypted blob can't be decrypted (e.g. machine-id changed | ||
| because the user migrated omnivoice_data/ across machines), get_hf_token | ||
| must return None and the resolver falls through to env / HF-CLI naturally.""" | ||
| from services import settings_store | ||
|
|
||
| # Hand-inject garbage ciphertext so Fernet raises InvalidToken on decrypt. | ||
| with sqlite3.connect(str(isolated_db)) as conn: | ||
| conn.execute( | ||
| "INSERT OR REPLACE INTO settings(key, value, updated_at) VALUES (?, ?, ?)", | ||
| ("hf_token", "not-a-valid-fernet-blob", time.time()), | ||
| ) | ||
| conn.commit() | ||
|
|
||
| caplog.clear() | ||
| result = settings_store.get_hf_token() | ||
| assert result is None |
Contributor
There was a problem hiding this comment.
Assert the warning this test promises.
The test name/docstring say decryption failure should log a warning, and caplog is injected, but Lines 125-127 only check the return value. If that warning ever disappears, this test still passes.
Suggested change
caplog.clear()
- result = settings_store.get_hf_token()
+ with caplog.at_level("WARNING"):
+ result = settings_store.get_hf_token()
assert result is None
+ assert caplog.records🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/backend/services/test_settings_store.py` around lines 111 - 127, The
test test_invalid_token_returns_none_with_warning currently only asserts the
return is None but doesn't verify the promised warning; after calling
settings_store.get_hf_token() add an assertion that caplog captured a WARNING
from the settings_store code (e.g., assert any(r.levelname == "WARNING" and
"hf_token" in r.getMessage() for r in caplog.records)) so the test fails if the
decryption warning is removed or not emitted. Use the injected caplog and
reference the settings_store.get_hf_token call and caplog.records in the
assertion.
3 tasks
debpalash
added a commit
that referenced
this pull request
May 19, 2026
…92) Phase 0 was verified PASS against `main` (7/7 truths, 9/9 artifacts, 6/6 GATE requirements, 5/5 success criteria; live smoke `tests/smoke/` green in 1.73s). Add the verifier's report and reconcile the REQUIREMENTS tracker — GATE-01..06 and AUTH-01..06 now show Done now that PR #71 (Phase 0) and PR #91 (Phase 1 Wave 1) are both on `main`. AUTH-03 is split: backend endpoints landed in Wave 1, UI ships in Wave 2. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 1 of the Phase 1 stabilization milestone — the highest user-value piece that closes the #35 community complaint about HF token persistence. Closes the
dub_core.py:540bug class systemically by routing every backend HF-token read through one resolver.What this PR delivers
A user enters their HF token once, anywhere (Settings → API Keys, the
HF_TOKENenv var, orhuggingface-cli login), and OmniVoice finds it. The current bug (bareos.environ.get("HF_TOKEN")atdub_core.py:540and four other sites) shows "no HF_TOKEN" to users who only ranhuggingface-cli logineven though the library would happily read the canonical file.AUTH requirements covered
backend/services/token_resolver.py::resolve()backend/services/settings_store.py+_secret_key.py(Fernet + scrypt + per-install salt)backend/api/routers/settings.py(loopback-gated)backend/services/sonitranslate.py:148(env block containsHF_TOKEN+YOUR_HF_TOKEN)backend/core/logging_filter.py::HFTokenRedactor(regexhf_[A-Za-z0-9]{30,})backend/services/token_resolver.py::on_401()Read sites patched (5/5)
backend/api/routers/dub_core.py:540(the original OmniVoice Studio Setup failed #35 site — error message now references all 3 sources and surfacessource/usernamewhen token exists but pipeline still failed)backend/api/routers/system.py:38(_has_hf_tokennotification)backend/services/model_manager.py:480(diarization pipeline auth)backend/services/sonitranslate.py:143(SoniTranslate Popen env block)backend/services/sonitranslate.py:217(gradio client predict call)Grep gate (in the plan's verification command) returns 0 hits. Reads outside the resolver are gone.
Tests added (35 cases, all green)
tests/backend/services/test_token_resolver.py— 11 cases: priority cascade, env override, 401 mid-resolve skip,on_401fallback,state()shape,add_to_git_credential=Falseinvariant (Pitfall Fix README and make dev script cross-platform #2),HUGGING_FACE_HUB_TOKENalias acceptancetests/backend/services/test_settings_store.py— 10 cases: round-trip encryption, plaintext-leakage check (the T-01-01 invariant), salt persistence across clear/set,InvalidTokendecrypt fallback (Open Question One-Click Docker is not working properly #5), concurrent reads consistent, alembic upgrade on a hand-built v0.2.7 fixture DB preserves all existing tables + seeded rows, alembic downgrade -1 drops onlysettingstests/backend/core/test_logging_filter.py— 6 cases: msg + args redaction, multi-token redaction, non-string args pass-through, short-token literals preserved,install_redaction_filteridempotencetests/backend/test_engine_spawn_token.py— 8 cases: POST/DELETE/GET endpoints (loopback succeeds, non-loopback 403), env block containsHF_TOKENwhen resolver returns one, no empty-string injection when resolver returns None, source-level regression guard on the SoniTranslate launcherPhase 0
tests/smoke/still green (4/4).Notable decisions / deviations
cryptography>=41added directly — RESEARCH.md Assumption A1 (transitive arrival via pyannote-audio/huggingface_hub) was checked at execute-time and proved false. Without this PR'spyproject.tomleditimport cryptographyraisesModuleNotFoundError.env.pyhonors externally-setsqlalchemy.url— previously the file unconditionally overrode any caller-supplied URL withcore.config.DB_PATH, which means tests would silently run migrations against the developer's realomnivoice_data/. Now wrapped inif not config.get_main_option(...)._BASE_SCHEMAand alembic — fresh installs land the schema viaCREATE IF NOT EXISTS, v0.2.7 upgrades land it viaalembic upgrade head. The migration is idempotent (checkssqlite_masterbeforecreate_table) so both paths converge.multiprocessing.Pipe, parent already holds HF state. Patching there would be a no-op. Documented in SUMMARY.md so future readers don't second-guess.require_loopback— same dep shipped in commite1f08a6(quick task 260518-ivy). No new helper.Constraints honored (CLAUDE.md)
require_loopbackdepv0.3.xtag created — branch-level only, ready formainmerge per "Beta release cadence" in CLAUDE.mdAction item for the reviewer
There is a stray edit to
pyproject.tomlin the parent repo working tree (outside this worktree). The classifier denied me reverting it. To clean up:The worktree's commit
aa06754carries the same change cleanly, so the PR itself is complete; this is just local-state hygiene.How to verify
uv sync uv run pytest tests/backend/services/test_token_resolver.py \ tests/backend/services/test_settings_store.py \ tests/backend/core/test_logging_filter.py \ tests/backend/test_engine_spawn_token.py -v uv run pytest tests/smoke/ -q grep -RnE "os\.(environ|getenv).*HF_TOKEN" backend/ --include='*.py' \ | grep -v token_resolver.py | grep -v '^[[:space:]]*#' # (last grep must return zero lines)Manual sanity (after starting the backend):
Refs
.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-PLAN.md.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-01-SUMMARY.mdDo NOT auto-merge. Wave 2 (Plan 01-02) consumes this PR's resolver state endpoints.
Summary by CodeRabbit
Release Notes
New Features
Improvements