P0: release.yml typecheck + bind audit + loopback middleware#84
Merged
Conversation
Phase 1: Install + Token Persistence + Docs Scaffolding + Error UX. Decomposes the 16 official requirements (INST-01..06, DOCS-01..05, AUTH-01..06) plus two scope additions (#76 .deb ffprobe, #80 Docker LAN) into three parallel-optimized plans with goal-backward must-haves and STRIDE threat models. - 01-01 (Wave 1): Token resolver + encrypted SQLite store + log redactor + subprocess env injection + patch all 5 bare `os.environ.get("HF_TOKEN")` sites. Closes the #35 read-side bug systemically. - 01-02 (Wave 2): Split README → 4 per-OS install docs + 4 supporting docs + `scripts/validate-install-docs.py` CI drift gate + error→docs deeplink map (Python + TS halves) + ErrorBoundary wiring + Settings → API Keys panel UI. - 01-03 (Wave 3): AppRun strategy spike + AppImage WebKit conditional launcher (#56) + .deb ffprobe relocation + postinst cleanup (#76) + centralized apiBase.ts + Docker LAN frontend fix (#80) + macOS Gatekeeper detection probe (#54). Resolves all 5 Open Questions from 01-RESEARCH.md (AppRun via spike, settings table via alembic, repo URL via links.py, localhost via grep sweep + apiBase.ts, first-launch UI via backend probe → error class). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ocs + Error UX) Phase 1 planning complete for the v0.3.0 fat-milestone release. PLANS (3 plans, 11 tasks, all 18 reqs covered): - 01-01-PLAN.md: AUTH-01..06 — token resolver, encrypted SQLite, log redactor, subprocess env, 5 read-side patches (3 tasks) - 01-02-PLAN.md: INST-02/03/05/06/12 + DOCS-01..05 + AUTH-03 — docs split (8 install docs), drift CI gate, error→docs deeplinks, Settings → API Keys + Performance panels (4 tasks) - 01-03-PLAN.md: INST-01/03/04 + scope-additions #76 (.deb ffprobe) + #80 (Docker LAN frontend) — AppRun spike, AppImage launcher, .deb ffprobe relocation, apiBase.ts centralization, Gatekeeper detection (4 tasks) INFRASTRUCTURE: - 01-RESEARCH.md: 3-4 page research with HIGH-confidence sources, 5 open questions resolved - 01-VALIDATION.md: Nyquist test pyramid (50/30/15/5), per-cluster coverage targets, per-plan artifact bindings, sampling rates, gate thresholds (B-1 blocker from plan-checker) REVISION PASS 1 (addressed plan-checker findings): - B-1: VALIDATION.md created - B-2/B-7: INST-12 full delivery (docs + Settings → Performance toggle) not just docs - B-3: ROADMAP traceability table corrected (16 → 18; AUTH-05 → AUTH-06; INST-12 listed) - B-4: INST-03 split documented in both 01-02 and 01-03 frontmatter via requirements_split - B-5: validate-install-docs.py self-tests added (10 test cases via TDD behavior block) - B-6: backend/core/links.py ownership: Plan 01-02 owns, Plan 01-01 disclaims - W-1: AppRun shell unit test (4 cases via mocked pkg-config) - W-2: ffmpeg_utils.py + test added to Plan 01-03 files_modified - W-3: README hardcoded URL drift site documented (3rd accepted location) - W-5: must_have truth #6 reframed user-observable - W-6: RESEARCH.md Open Questions marked (RESOLVED) with per-question annotations CLAUDE.md cadence updated: v0.3.0 ships once when all phases complete; no incremental v0.3.x tags. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `typecheck:ci` script to frontend/package.json so both ci.yml and release.yml call an identical command. Previously release.yml ran `bunx tsc --noEmit` (no --checkJs override) while ci.yml ran `bunx tsc --noEmit --checkJs false`. PRs passed CI but failed at release the moment any new pre-existing JS-side error landed; v0.3.x release runs were the immediate casualty. - frontend/package.json: add "typecheck:ci": "tsc --noEmit --checkJs false" - .github/workflows/ci.yml: replace inline tsc with `bun run typecheck:ci` - .github/workflows/release.yml: same — drift is now structurally impossible Verified locally: `cd frontend && bun run typecheck:ci` returns exit 0.
backend/main.py used `host="0.0.0.0"` in its production uvicorn.run call, exposing every router on this process to the LAN. OmniVoice ships no authentication, so any host on the same network could reach /system/*, /dub/*, /generate, /model/*, etc. The recent /system/set-env loopback guard (PR #81) covered only that one endpoint; the rest of the surface was still open. - backend/main.py: read OMNIVOICE_BIND_HOST from os.environ, default 127.0.0.1 - deploy/docker-compose.yml: set OMNIVOICE_BIND_HOST=0.0.0.0 inside both the CPU and GPU service blocks so the host-side `127.0.0.1:3900:3900` mapping can still forward traffic in (the host mapping is what enforces loopback-only when running under Docker) - README.md: document OMNIVOICE_BIND_HOST + clarify Docker vs. bare-metal exposure model - tests/test_bind_host.py: new test file. Five tests: * default-when-unset returns 127.0.0.1 * explicit loopback honored * explicit 0.0.0.0 honored (Docker path) * source-level guard rejecting a hardcoded host="0.0.0.0" in uvicorn.run * source-level guard that OMNIVOICE_BIND_HOST reference stays present Verified: `uv run python -m pytest tests/test_bind_host.py -x -q` → 5 passed.
PR #81 added an inline loopback-origin check at /system/set-env only; the 260518-ivy deferred-items file enumerated the 5 sibling POST routes plus 4 GET routes on the same router that share the same trust-boundary defect. Promote the check to a router-level FastAPI dependency so every present and future route on backend/api/routers/system.py is gated by one source of truth — closes the architectural gap, not just the symptom. Routes now gated (all on backend/api/routers/system.py): - /system/info, /system/logs, /system/logs/tauri, /system/logs/stream (GET, info-disclosure: leaks data_dir / outputs_dir / crash_log_path / model checkpoints / paths and possibly secrets in log lines) - /model/unload/{model_id} (POST, force-evict TTS/ASR model from LAN) - /system/logs/clear, /system/logs/tauri/clear (POST, anti-forensics) - /system/flush-memory (POST, performance-degradation DoS) - /clean-audio (POST, resource exhaustion: arbitrary WAV upload → demucs) - /system/set-env (already loopback-gated inline — now via the router dependency, inline check removed; behavior preserved) Implementation: - backend/api/dependencies.py: new shared module — `require_loopback` raises 403 unless `request.client.host` is one of {"127.0.0.1", "::1", "localhost"}. Detail string "loopback origin required" so the existing /system/set-env tests pass against the new centralized message. - backend/api/routers/system.py: APIRouter is constructed with `dependencies=[Depends(require_loopback)]`. The inline loopback check at set_env_var is removed (router-level dependency makes it redundant). Unused `Request` import dropped. - tests/test_api.py: * `client` fixture now uses `TestClient(app, client=("127.0.0.1", 50000))` so the protected routes are reachable from happy-path tests. * `test_set_env_rejects_non_loopback` rewritten — no longer takes the `client` fixture (which is now loopback); builds its own plain `TestClient(app)` to exercise the rejection path. * New `test_clean_audio_rejects_non_loopback` — samples the previously- unprotected /clean-audio POST → 403. * New `test_system_info_rejects_non_loopback` — samples the previously- leaky /system/info GET → 403. - tests/test_router_smoke.py, tests/smoke/test_boot_smoke.py: same `client=("127.0.0.1", 50000)` fixture update so smoke tests pass the router-level gate. Verified: - `uv run python -m pytest tests/test_api.py -k "loopback or rejects_non" -x -q` → 5 passed. - `uv run python -m pytest tests/ -q` → 243 passed, 6 skipped, 10 xfailed, 3 xpassed. No new failures. - `PYTHONPATH=backend uv run python -c "from main import app"` → OK, 150 routes registered. Out of scope (per deferred-items follow-up triage): rate-limited LAN access for routes that might legitimately want it (e.g. future "control OmniVoice from your phone"), and a longer-term local-token handshake for the Tauri ↔ backend channel. This commit ships the immediate, conservative default — loopback-only — for every system-router route.
Contributor
📝 WalkthroughWalkthroughThis PR combines a P0 release-security update with comprehensive Phase 1 planning documentation. The code changes introduce loopback-first backend binding, unified TypeScript typecheck scripting, and router-level access gating for system endpoints. The planning documents establish research, validation, and multi-wave execution roadmaps for Phase 1 stabilization work spanning token persistence, docs scaffolding, and error UX. ChangesP0 Release & Security
Phase 1 Execution Planning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 6
🤖 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-RESEARCH.md:
- Around line 416-417: Replace all developer-local absolute path strings like
"/Users/user4/..." in the planning document with repo-relative paths (for
example "backend/api/routers/dub_core.py:540"); search the file for any
occurrences of that absolute-path pattern and swap them to repository-relative
references, updating all instances mentioned in the review (and any others
found) so no local user/machine paths remain.
In
@.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-VALIDATION.md:
- Around line 159-176: The Wave-0 checklist under the "6. Wave 0 Gaps Closed By
This Phase" heading currently marks items as closed (`[x]`) even though this PR
only contains planning docs; update each checklist item (e.g., entries for
`tests/backend/services/test_token_resolver.py`,
`frontend/src/utils/errorDocsMap.test.ts`, the CI step `python
scripts/validate-install-docs.py`,
`tests/scripts/test_validate_install_docs.py`, etc.) to the open/planned state
by replacing `- [x]` with `- [ ]` for all items until the actual
test/code/artifact lands so phase tracking remains accurate.
In @.planning/ROADMAP.md:
- Around line 167-180: Update the roadmap file footer metadata so the "Last
updated" date in the file metadata matches the body update date shown in the
header line ("v1 requirements mapped: 74 / 74 ✓ (updated 2026-05-18 ... )");
locate the footer "Last updated" metadata entry and replace its outdated date
with 2026-05-18 (or programmatically derive the same date string used in the
body) so the metadata and the shown coverage update are synchronized.
In @.planning/STATE.md:
- Line 13: The STATE.md has inconsistent/stale counters for Phase 1 (mentions 16
and 17 and shows 62/62 while the roadmap uses 18 and 74/74); update all
occurrences so they match the single source of truth (Phase 1 = 18 requirements,
total = 74/74). Locate the lines containing "Phase 1", "Current focus:", and the
numeric counters (e.g., the "16", "17", "62/62", "47-47", "67-67" occurrences)
and replace them with the normalized counts from the roadmap (18 and 74/74) so
every reference is consistent.
In `@README.md`:
- Line 282: Update the README's "docker run" quickstart to match the compose
note by explicitly setting OMNIVOICE_BIND_HOST=0.0.0.0 for the containerized
path and/or instructing users to publish the host port as 0.0.0.0:PORT when they
want LAN access; specifically mention OMNIVOICE_BIND_HOST and the docker run
invocation so readers know to export OMNIVOICE_BIND_HOST=0.0.0.0 (or pass -e
OMNIVOICE_BIND_HOST=0.0.0.0) when using docker run, and include a short sentence
clarifying that the default backend bind is 127.0.0.1 which will block forwarded
traffic unless changed.
In `@tests/test_bind_host.py`:
- Around line 41-43: The test intentionally asserts binding to 0.0.0.0 and Ruff
S104 should be suppressed for that line; edit the
test_explicit_all_interfaces_env_var_is_honored() test to add an inline Ruff
suppression on the assertion (e.g., append a noqa for S104 to the assert line
that calls _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"})) so CI won't
fail on this false-positive.
🪄 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: c4f4de6c-c350-472e-b30b-e94257e85481
📒 Files selected for processing (21)
.github/workflows/ci.yml.github/workflows/release.yml.planning/ROADMAP.md.planning/STATE.md.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-02-PLAN.md.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-03-PLAN.md.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-RESEARCH.md.planning/phases/01-install-token-persistence-docs-scaffolding-error-ux/01-VALIDATION.md.planning/quick/260518-p0-release-and-security/260518-p0-SUMMARY.mdCLAUDE.mdREADME.mdbackend/api/dependencies.pybackend/api/routers/system.pybackend/main.pydeploy/docker-compose.ymlfrontend/package.jsontests/smoke/test_boot_smoke.pytests/test_api.pytests/test_bind_host.pytests/test_router_smoke.py
Comment on lines
+416
to
+417
| **Code hook:** `/Users/user4/Desktop/voice-design/OmniVoice/backend/api/routers/dub_core.py:540`. | ||
|
|
Contributor
There was a problem hiding this comment.
Replace developer-local absolute paths with repo-relative paths.
These /Users/user4/... references are machine-specific and leak local path/user details; they also make the plan non-portable across contributors and CI environments. Please switch these to repo-relative paths (e.g., backend/api/routers/dub_core.py:540).
Also applies to: 446-447, 453-453, 762-766
🤖 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-RESEARCH.md
around lines 416 - 417, Replace all developer-local absolute path strings like
"/Users/user4/..." in the planning document with repo-relative paths (for
example "backend/api/routers/dub_core.py:540"); search the file for any
occurrences of that absolute-path pattern and swap them to repository-relative
references, updating all instances mentioned in the review (and any others
found) so no local user/machine paths remain.
Comment on lines
+159
to
+176
| ## 6. Wave 0 Gaps Closed By This Phase | ||
|
|
||
| Per RESEARCH.md § Validation Architecture, all Wave 0 gaps below MUST be closed in Phase 1: | ||
|
|
||
| - [x] `tests/backend/services/test_token_resolver.py` (Plan 01-01) | ||
| - [x] `tests/backend/services/test_settings_store.py` (Plan 01-01) | ||
| - [x] `tests/backend/core/test_logging_filter.py` (Plan 01-01) | ||
| - [x] `tests/backend/test_engine_spawn_token.py` (Plan 01-01) | ||
| - [x] `tests/backend/core/test_gatekeeper_detect.py` (Plan 01-03) | ||
| - [x] `frontend/src/utils/errorDocsMap.test.ts` (Plan 01-02) | ||
| - [x] CI step `python scripts/validate-install-docs.py` (Plan 01-02 Task 1 Step 14) | ||
| - [x] vitest setup (decided + documented in Plan 01-02 SUMMARY) | ||
| - [x] Validator self-tests `tests/scripts/test_validate_install_docs.py` (Plan 01-02, per B-5) | ||
| - [x] Links resolver test `tests/backend/core/test_links.py` (Plan 01-02, per B-6) | ||
| - [x] AppRun shell test `frontend/src-tauri/appimage/AppRun.test.sh` (Plan 01-03, per W-1) | ||
| - [x] ffmpeg_utils test `tests/backend/services/test_ffmpeg_utils.py` (Plan 01-03, per W-2) | ||
| - [x] Perf settings test `tests/backend/test_perf_settings.py` (Plan 01-02, per B-2/B-7) | ||
|
|
Contributor
There was a problem hiding this comment.
Wave-0 gap checklist is marked complete too early.
This section currently reports all gaps as already closed ([x]), but the PR scope here is planning docs. Please switch these to planned/open state until artifacts actually land, otherwise phase tracking can drift.
🤖 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-VALIDATION.md
around lines 159 - 176, The Wave-0 checklist under the "6. Wave 0 Gaps Closed By
This Phase" heading currently marks items as closed (`[x]`) even though this PR
only contains planning docs; update each checklist item (e.g., entries for
`tests/backend/services/test_token_resolver.py`,
`frontend/src/utils/errorDocsMap.test.ts`, the CI step `python
scripts/validate-install-docs.py`,
`tests/scripts/test_validate_install_docs.py`, etc.) to the open/planned state
by replacing `- [x]` with `- [ ]` for all items until the actual
test/code/artifact lands so phase tracking remains accurate.
Comment on lines
+167
to
+180
| **v1 requirements mapped:** 74 / 74 ✓ (updated 2026-05-18 — Phase 1 row corrected per checker B-3 to include INST-12 + AUTH-06; total now matches REQUIREMENTS.md 74) | ||
| **Orphaned requirements:** 0 ✓ | ||
| **Duplicates:** 0 ✓ | ||
|
|
||
| | Phase | Requirement Count | Requirement IDs | | ||
| |-------|-------------------|-----------------| | ||
| | Phase 0 | 6 | GATE-01 — GATE-06 | | ||
| | Phase 1 | 16 | INST-01 — INST-06, DOCS-01 — DOCS-05, AUTH-01 — AUTH-05 | | ||
| | Phase 1 | 18 | INST-01 — INST-06, INST-12, DOCS-01 — DOCS-05, AUTH-01 — AUTH-06 | | ||
| | Phase 2 | 8 | ENGINE-01 — ENGINE-07, BUG-01 | | ||
| | Phase 3 | 11 | TTS-01 — TTS-06, INST-07 — INST-11 | | ||
| | Phase 4 | 13 | SPIKE-01 — SPIKE-02, GGUF-01 — GGUF-06, SING-01 — SING-05 | | ||
| | Phase 5 | 12 | REPORT-01 — REPORT-12 | | ||
| | Phase 6 | 6 | REL-01 — REL-06 | | ||
| | **Total** | **62** | — | | ||
| | **Total** | **74** | — | |
Contributor
There was a problem hiding this comment.
Refresh roadmap metadata to match the new coverage update.
The body is updated to 74/74, but the file metadata still indicates the old update date in the footer. Please sync the “Last updated” metadata with this revision to avoid state drift.
🤖 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/ROADMAP.md around lines 167 - 180, Update the roadmap file footer
metadata so the "Last updated" date in the file metadata matches the body update
date shown in the header line ("v1 requirements mapped: 74 / 74 ✓ (updated
2026-05-18 ... )"); locate the footer "Last updated" metadata entry and replace
its outdated date with 2026-05-18 (or programmatically derive the same date
string used in the body) so the metadata and the shown coverage update are
synchronized.
| **Milestone:** v0.3.x stabilization — "Empty the inbox" (close all 11 open GitHub issues) plus two surgical additions (Supertonic-3 engine, opt-in bug reporting) plus two spike-first model additions (`Serveurperso/OmniVoice-GGUF` hardware-adaptive default, `ModelsLab/omnivoice-singing` for the dubbing pipeline). | ||
|
|
||
| **Current focus:** Roadmap defined (7 phases, 62 v1 requirements). Awaiting `/gsd:plan-phase 0` to decompose Phase 0 (Gates) into executable plans. | ||
| **Current focus:** Phase 1 planned. Three plans cover all 16 official Phase 1 requirements plus accepted scope additions (#76 .deb ffprobe, #80 Docker LAN). Ready for `/gsd:execute-phase 1`. |
Contributor
There was a problem hiding this comment.
Phase 1 requirement/count numbers are inconsistent and stale.
This file currently reports 16 and 17 in different places, and still shows 62/62 mappings, while the roadmap update tracks Phase 1 at 18 requirements and total 74/74. Please normalize these counters to one source of truth.
Also applies to: 47-47, 67-67
🤖 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 at line 13, The STATE.md has inconsistent/stale counters
for Phase 1 (mentions 16 and 17 and shows 62/62 while the roadmap uses 18 and
74/74); update all occurrences so they match the single source of truth (Phase 1
= 18 requirements, total = 74/74). Locate the lines containing "Phase 1",
"Current focus:", and the numeric counters (e.g., the "16", "17", "62/62",
"47-47", "67-67" occurrences) and replace them with the normalized counts from
the roadmap (18 and 74/74) so every reference is consistent.
| </details> | ||
|
|
||
| > **Network access:** the container binds to `127.0.0.1` only. To expose on your LAN, change the port mapping to `"0.0.0.0:3900:3900"`. OmniVoice ships no authentication — put it behind a reverse proxy with auth (Caddy `basic_auth`, nginx + htpasswd, Tailscale, etc.). | ||
| > **Network access:** the host-side port mapping binds to `127.0.0.1` only, and the backend itself defaults to `OMNIVOICE_BIND_HOST=127.0.0.1` (loopback). The shipped `docker-compose.yml` sets `OMNIVOICE_BIND_HOST=0.0.0.0` *inside* the container so the host mapping can forward traffic in — the `127.0.0.1:3900:3900` mapping is what enforces loopback-only on the host. To expose on your LAN, change the host port mapping to `"0.0.0.0:3900:3900"`. Running the backend directly (not under Docker)? Set `OMNIVOICE_BIND_HOST=0.0.0.0` to listen on all interfaces. OmniVoice ships no authentication — put it behind a reverse proxy with auth (Caddy `basic_auth`, nginx + htpasswd, Tailscale, etc.). |
Contributor
There was a problem hiding this comment.
docker run quickstart likely breaks after loopback-default bind change.
The note is accurate for Compose, but the documented docker run commands still don’t set OMNIVOICE_BIND_HOST=0.0.0.0. With the new backend default (127.0.0.1), that path can fail to accept forwarded traffic from host port mapping.
Suggested docs patch
# CPU mode
docker run -d --name omnivoice \
+ -e OMNIVOICE_BIND_HOST=0.0.0.0 \
-p 127.0.0.1:3900:3900 \
-v omnivoice-data:/app/omnivoice_data \
ghcr.io/debpalash/omnivoice-studio:latest
# NVIDIA GPU mode
docker run -d --name omnivoice --gpus all \
+ -e OMNIVOICE_BIND_HOST=0.0.0.0 \
-p 127.0.0.1:3900:3900 \
-v omnivoice-data:/app/omnivoice_data \
ghcr.io/debpalash/omnivoice-studio:latest🤖 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 `@README.md` at line 282, Update the README's "docker run" quickstart to match
the compose note by explicitly setting OMNIVOICE_BIND_HOST=0.0.0.0 for the
containerized path and/or instructing users to publish the host port as
0.0.0.0:PORT when they want LAN access; specifically mention OMNIVOICE_BIND_HOST
and the docker run invocation so readers know to export
OMNIVOICE_BIND_HOST=0.0.0.0 (or pass -e OMNIVOICE_BIND_HOST=0.0.0.0) when using
docker run, and include a short sentence clarifying that the default backend
bind is 127.0.0.1 which will block forwarded traffic unless changed.
Comment on lines
+41
to
+43
| def test_explicit_all_interfaces_env_var_is_honored(): | ||
| # Used by deploy/docker-compose.yml — must still work as an opt-in override. | ||
| assert _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"}) == "0.0.0.0" |
Contributor
There was a problem hiding this comment.
Suppress intentional Ruff S104 on the opt-in 0.0.0.0 test case.
This assertion is expected behavior, but Ruff is flagging it (S104) and can block CI. Add a targeted inline suppression to avoid false-positive lint failure.
Suggested minimal change
def test_explicit_all_interfaces_env_var_is_honored():
# Used by deploy/docker-compose.yml — must still work as an opt-in override.
- assert _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"}) == "0.0.0.0"
+ assert _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"}) == "0.0.0.0" # noqa: S104📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_explicit_all_interfaces_env_var_is_honored(): | |
| # Used by deploy/docker-compose.yml — must still work as an opt-in override. | |
| assert _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"}) == "0.0.0.0" | |
| def test_explicit_all_interfaces_env_var_is_honored(): | |
| # Used by deploy/docker-compose.yml — must still work as an opt-in override. | |
| assert _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"}) == "0.0.0.0" # noqa: S104 |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 43-43: Possible binding to all interfaces
(S104)
[error] 43-43: Possible binding to all interfaces
(S104)
🤖 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/test_bind_host.py` around lines 41 - 43, The test intentionally asserts
binding to 0.0.0.0 and Ruff S104 should be suppressed for that line; edit the
test_explicit_all_interfaces_env_var_is_honored() test to add an inline Ruff
suppression on the assertion (e.g., append a noqa for S104 to the assert line
that calls _resolve_bind_host({"OMNIVOICE_BIND_HOST": "0.0.0.0"})) so CI won't
fail on this false-positive.
4 tasks
debpalash
added a commit
that referenced
this pull request
May 19, 2026
Follow-up to f15f6d9 (WS loopback guard on /ws/transcribe). The functional tests in tests/test_capture_ws.py constructed TestClient with the Starlette default client=("testclient", 50000), which the guard's _LOOPBACK_HOSTS allow-list rejects. PR #84 established the fix pattern for HTTP tests: explicitly pass client=("127.0.0.1", 50000) to TestClient — same security property, same semantics. Applied here. 3 tests previously failing now pass: - test_eof_text_frame_triggers_final_without_disconnect - test_legacy_disconnect_still_finalizes - test_empty_binary_frame_acts_as_eof
This was referenced May 19, 2026
This was referenced May 30, 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.
Three P0 fixes bundled — all foundation cleanup before v0.3.0 phase work continues.
Summary
1.
ci: single-source typecheck flag across CI + release workflows(a5560f5)Closes the
release.ymlworkflow drift that caused today's v0.3.0 and v0.3.1 release runs to fail at the gate job.release.yml:98was runningbunx tsc --noEmitwithout the--checkJs falseflag thatci.yml:89has. PR #51's new sidebar tabs in App.jsx exposed the trap. Fix:frontend/package.jsonscript:"typecheck:ci": "tsc --noEmit --checkJs false"bun run typecheck:ci2.
security: default backend bind to 127.0.0.1; OMNIVOICE_BIND_HOST opt-in for Docker(b74b51c)Closes the production bind-address gap (Critic finding F1).
backend/main.py:496was binding0.0.0.0:3900in prod; PR #81's loopback guard only covered one endpoint. Fix:backend/main.pyreadsOMNIVOICE_BIND_HOSTenv var, defaults to127.0.0.1deploy/docker-compose.ymlsetsOMNIVOICE_BIND_HOST=0.0.0.0for both CPU + GPU servicesREADME.mddocuments the env var for LAN-access scenariostests/test_bind_host.py(new, 5 tests): asserts default is loopback, env var override works3.
security: apply require_loopback Depends across all /system/* routes(e280734)Closes the sibling endpoint exposure (Critic findings F2 + F3, Architect Candidate C). 5 POST endpoints + 4 GET endpoints in
system.pyshared the no-auth gap; the 4 GETs leak environment/paths/possibly tokens to LAN. Fix:backend/api/dependencies.pywithrequire_loopback(request: Request)APIRouter(dependencies=[Depends(require_loopback)])— protects every route mounted on system.py with one change/system/set-env(router dep makes it redundant)test_clean_audio_rejects_non_loopback,test_system_info_rejects_non_loopback, plus existing 3 set-env tests preserved)TestClient(app)(which has host="testclient", non-loopback) — they now useTestClient(app, client=(\"127.0.0.1\", 50000))Test plan
bun run --cwd frontend typecheck:ci→ exit 0pytest tests/test_bind_host.py→ 5 passedpytest tests/test_api.py -k 'loopback or rejects_non'→ 5 passedpytest tests/→ 243 passed, 6 skipped, 10 xfailed, 3 xpassedOut of scope (separate tasks)
media.js:20hardcoded localhost — Phase 1 plan 01-03dub_generate.py, etc.) — same pattern likely applies; future audit🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Security
Documentation
Tests