feat: network sharing (PIN-gated LAN + QR) & Tailscale remote access (#125)#159
Merged
Conversation
Same-state LAN sharing via a second in-process uvicorn listener on a dedicated share port (no restart, model/jobs preserved), PIN-gated for non-loopback clients, with QR + all-LAN-addresses panel. Tailscale serve for private remote access. Supersedes the raw 0.0.0.0 default-flip in #125. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security review of #157 confirmed the /system router is already loopback-gated via Depends(require_loopback) (non-spoofable request.client.host). The network control endpoints inherit it and /system/set-env is auto-protected from the LAN listener — no new guard needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… fields Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The no-hardcoded-CJK guard walked the filesystem, so local untracked vendored experiments (research/voice-pro etc. with JP issue templates) caused false local failures while CI (committed files) passed. Scan via git ls-files so local-only and CI behavior match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ring on Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…R URL Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…py/open Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ale) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ver binds Defensive guard (spec §7): if the second uvicorn server doesn't reach 'started' (e.g. the share port was taken in the race after the free-port probe), cancel the task, reset state, and raise — so the API surfaces the failure and the UI stays Local rather than reporting a dead 'Network' state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds network sharing and Tailscale remote access. A second in-process uvicorn server can be enabled on 0.0.0.0 with a generated 6-digit PIN and auto-detected LAN addresses; a loopback-only middleware gates non-loopback access when a PIN is set. Tailscale CLI integration provides identity-gated remote HTTPS access. The frontend captures PIN from URL, gates the app on 401, and provides LAN toggle and Tailscale controls in the footer and Settings. ChangesNetwork Sharing and Remote Access Feature
Sequence DiagramssequenceDiagram
participant RemoteDevice
participant Browser
participant NetworkToggle as NetworkToggle
participant APIClient as API Client
participant Middleware as PIN Middleware
participant Backend as Backend Service
participant NetworkShare as network_share Service
RemoteDevice->>Browser: Open QR URL (with ?pin=XXXXXX)
Browser->>APIClient: Load app
APIClient->>APIClient: Extract ?pin from URL
APIClient->>APIClient: Store pin in sessionStorage
Browser->>APIClient: GET /system/info (from 192.168.x.x)
APIClient->>Middleware: Attach X-OmniVoice-Pin header
Middleware->>Middleware: Verify PIN (constant-time)
Middleware->>Backend: Allow request
Backend->>Browser: Response with data
Note over APIClient,NetworkShare: Local user enables sharing
NetworkToggle->>APIClient: POST /system/network/enable
APIClient->>Backend: Request from localhost 127.0.0.1
Backend->>NetworkShare: enable(app)
NetworkShare->>NetworkShare: Find free port, generate PIN, enumerate LAN IPs
NetworkShare->>NetworkShare: Start second uvicorn server binding 0.0.0.0
NetworkShare->>Backend: Update app.state.network_share
Backend->>APIClient: Return state (enabled, port, PIN, IPs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for p in range(base, base + tries): | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| try: | ||
| s.bind(("0.0.0.0", p)) |
| return _state | ||
| port = _find_free_port(BACKEND_PORT + 1) | ||
| pin = _gen_pin() | ||
| config = uvicorn.Config(app, host="0.0.0.0", port=port, log_level="warning") |
|
|
||
| @router.post("/system/tailscale/enable") | ||
| async def tailscale_enable(): | ||
| return _tailscale.serve_enable() |
| server.should_exit = True | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=2) | ||
| except Exception: |
| await asyncio.wait_for(_task, timeout=2) | ||
| except Exception: | ||
| pass | ||
| _task = None |
| if _task is not None: | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=5) | ||
| except Exception: |
| await asyncio.wait_for(_task, timeout=5) | ||
| except Exception: | ||
| pass | ||
| _server = _task = None |
| self_ = data.get("Self") or {} | ||
| out["magic_dns_name"] = (self_.get("DNSName") or "").rstrip(".") | ||
| out["tailnet_ips"] = self_.get("TailscaleIPs") or [] | ||
| except Exception: |
| return {"ok": True} | ||
| try: | ||
| subprocess.run([cli, "serve", "reset"], capture_output=True, text=True, timeout=20) | ||
| except Exception: |
| continue | ||
| yield _REPO / n | ||
| return | ||
| except Exception: |
Contributor
|
| Filename | Overview |
|---|---|
| backend/services/network_share.py | New service that starts a second in-process uvicorn server sharing the same FastAPI app; includes PIN generation, free-port probing with TOCTOU-safe failure handling, and graceful disable. |
| backend/main.py | Adds NetworkAccessMiddleware (PIN gate) and wires network share state into app.state on lifespan startup. Middleware correctly uses constant-time compare and exempts loopback + SPA shell paths. |
| frontend/src/api/client.ts | Adds PIN capture from QR URL params into sessionStorage and attaches the header on every apiFetch. Hardcoded port 3900 makes LAN API calls miss the share port (noted in a prior review, still unresolved). |
| backend/services/tailscale.py | Thin CLI wrapper with graceful degradation. serve_disable uses tailscale serve reset which clears all serve configs, not just OmniVoice's. |
| backend/api/routers/system.py | New control endpoints added to the existing router that applies require_loopback at the router level, so all new endpoints are loopback-gated automatically. |
| frontend/src/components/RemoteAuthGate.jsx | PIN entry gate triggered by ov:pin-required event; correct and simple. |
| frontend/src/components/settings/SharingPanel.jsx | Combines NetworkToggle for LAN sharing with a Tailscale section; QR generation is cancel-safe. |
| frontend/src/components/NetworkToggle.jsx | Footer/panel toggle for LAN sharing. Each instance maintains independent, unsynchronized state. |
Sequence Diagram
sequenceDiagram
participant Host as Host Browser (loopback)
participant LAN as LAN Device Browser
participant Share as Share Server (0.0.0.0:3901)
participant Middleware as NetworkAccessMiddleware
participant API as FastAPI App
Host->>API: POST /system/network/enable
API-->>Host: "{enabled, share_port, pin, lan_addresses}"
Note over Host: Displays QR → http://lan-ip:3901/?pin=XXXXXX
LAN->>Share: "GET /?pin=XXXXXX"
Note over LAN: sessionStorage.setItem('ov_pin', 'XXXXXX')
LAN->>Share: GET /api/voices + X-OmniVoice-Pin header
Share->>Middleware: dispatch(request)
Middleware->>Middleware: compare_digest(supplied, pin) ✓
Middleware->>API: call_next(request)
API-->>LAN: 200 + set ov_pin cookie
Host->>API: POST /system/network/disable
API-->>Host: "{enabled: false}"
Reviews (2): Last reviewed commit: "fix(network): apiFetch leaves opts untou..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return JSONResponse({"detail": "PIN required"}, status_code=401) | ||
| response = await call_next(request) | ||
| if request.cookies.get("ov_pin") != pin: | ||
| response.set_cookie("ov_pin", pin, samesite="lax") |
Contributor
There was a problem hiding this comment.
httponly=True
The ov_pin cookie is set without httponly=True, making the PIN readable from document.cookie by any JavaScript running on the page. The middleware reads it server-side; the frontend reads its copy from sessionStorage. Setting httponly=True prevents client-side script from accessing the cookie without affecting any existing flow.
Suggested change
| response.set_cookie("ov_pin", pin, samesite="lax") | |
| response.set_cookie("ov_pin", pin, samesite="lax", httponly=True) |
Comment on lines
+66
to
+94
| async def enable(app) -> ShareState: | ||
| global _server, _task, _state | ||
| if _state.enabled: | ||
| return _state | ||
| port = _find_free_port(BACKEND_PORT + 1) | ||
| pin = _gen_pin() | ||
| config = uvicorn.Config(app, host="0.0.0.0", port=port, log_level="warning") | ||
| server = uvicorn.Server(config) | ||
| server.install_signal_handlers = lambda: None # never hijack signals in-process | ||
| _task = asyncio.create_task(server.serve()) | ||
| for _ in range(100): # ~5s for the socket to bind | ||
| if getattr(server, "started", False): | ||
| break | ||
| await asyncio.sleep(0.05) | ||
| if not getattr(server, "started", False): | ||
| # Bind failed (e.g. the port was taken in the race after the | ||
| # free-port probe). Tear down and stay Local — never report enabled | ||
| # with a listener that isn't actually up (spec §7). | ||
| server.should_exit = True | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=2) | ||
| except Exception: | ||
| pass | ||
| _task = None | ||
| raise RuntimeError("share listener failed to start") | ||
| _server = server | ||
| _state = ShareState(True, port, pin, lan_ipv4_addresses()) | ||
| app.state.network_share = _state | ||
| return _state |
Contributor
There was a problem hiding this comment.
enable() calls
enable() checks _state.enabled before proceeding, but there is no lock. Two simultaneous POST requests to /system/network/enable (both loopback, both arriving before the first server finishes starting) will both see enabled == False, both start a uvicorn.Server task, and then the second assignment to _server/_state will orphan the first server's task, leaving a zombie listener bound to a port until the process exits. Adding an asyncio.Lock around the body of enable() (and disable()) would prevent this.
… tsc CI runs 'tsc --noEmit --checkJs false', which type-checks .ts files; Node's 'global' isn't typed there (TS2304). vitest (esbuild) tolerated it locally. Use globalThis (standard, typed) + cast the mock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The unconditional headers merge changed the request shape for callers with no headers (e.g. FormData posts), breaking the legacy 'apiPost passes FormData without Content-Type override' node test. Only spread opts + inject X-OmniVoice-Pin when a PIN is actually present; otherwise pass opts through unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 14
🧹 Nitpick comments (1)
docs/sharing.md (1)
43-43: ⚡ Quick winClarify the relationship between server deployments and the toggle.
The phrase "the in-app toggle... is unaffected by these flows" is ambiguous. Based on the design spec (line 79), the toggle is hidden in server deployments where
OMNIVOICE_BIND_HOST=0.0.0.0.Consider rephrasing for clarity:
📝 Suggested clarification
-Server deployments (docker, `OMNIVOICE_BIND_HOST=0.0.0.0`) manage their own networking; the in-app toggle is for the desktop app and is unaffected by these flows. +Server deployments (docker, `OMNIVOICE_BIND_HOST=0.0.0.0`) manage their own networking independently; the in-app toggle is designed for the desktop app and is hidden in server deployments.🤖 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 `@docs/sharing.md` at line 43, Summary: The sentence is ambiguous about how server deployments interact with the in-app toggle; it should explicitly state that the toggle is hidden for server deployments when OMNIVOICE_BIND_HOST=0.0.0.0. Update the line to say that server deployments (e.g., Docker with OMNIVOICE_BIND_HOST=0.0.0.0) manage networking themselves and therefore the in-app "share on local network" toggle is not shown/available in those deployments; replace the ambiguous phrase "is unaffected by these flows" with a clear statement that the toggle is hidden/disabled when OMNIVOICE_BIND_HOST=0.0.0.0 so readers understand the condition.
🤖 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 `@backend/main.py`:
- Around line 487-515: NetworkAccessMiddleware only protects HTTP; add the same
PIN/loopback checks to the WebSocket handlers so /ws/events and /ws/tts enforce
the gate. Extract the PIN-check logic from NetworkAccessMiddleware.dispatch into
a reusable helper (e.g., validate_network_access(request_or_ws, app_state))
that: reads app.state.network_share.pin, allows _LOOPBACK_CLIENTS, reads
supplied pin from headers/query/cookies, uses secrets.compare_digest, and
returns True/False (or raises). Call that helper at the start of the connection
handlers in backend/api/routers/events.py and backend/api/routers/tts_stream.py
and if it fails, immediately reject/close the WebSocket (WebSocket.close with an
appropriate code/reason) so non-loopback clients cannot connect when a PIN is
set; keep /ws/transcribe behavior consistent by either calling the same helper
or updating its inline guard to enforce the PIN.
In `@backend/services/network_share.py`:
- Around line 97-108: The disable function currently clears _state and
app.state.network_share regardless of whether the background server/task
actually stopped; change disable so it only resets _server, _task and sets
_state = ShareState() and app.state.network_share after confirming the server
task has completed successfully (i.e., after await asyncio.wait_for(_task,
timeout=5) returns), and on timeout/exception leave _server/_task/_state
untouched (or re-raise/log) so the UI still reflects that sharing may be active;
update references in disable to only mutate _server, _task and _state after
successful shutdown and handle the exception branch without clearing
app.state.network_share.
- Around line 66-108: The enable/disable functions mutate globals (_state,
_server, _task) without synchronization, causing races; add a module-level
asyncio.Lock (e.g. _lock = asyncio.Lock()) and wrap the critical sections in
both enable and disable with "async with _lock:" so the check (if
_state.enabled), the server start/teardown, and assignments to
_server/_task/_state and app.state.network_share are serialized; ensure the lock
is acquired before reading/modifying these globals and released via context
manager even on exceptions, and import asyncio if needed.
In `@backend/services/tailscale.py`:
- Around line 50-58: The serve_disable function currently swallows all
subprocess errors; update it to surface failures by running subprocess.run with
check=True (or inspect CompletedProcess.returncode) when calling
subprocess.run([cli, "serve", "reset"], ...) and catch
subprocess.CalledProcessError and subprocess.TimeoutExpired (and any other
Exception) to return a failure result to the caller instead of always {"ok":
True}; keep the early short-circuit when _cli() returns falsy, but in error
cases return a structured failure such as {"ok": False, "error": str(e)} that
includes the exception message so the caller/UI can detect and display the
failure (refer to serve_disable, _cli, and the subprocess.run invocation).
In `@frontend/src/api/client.ts`:
- Around line 13-21: After grabbing and persisting the QR PIN (the block that
calls new URL(window.location.href).searchParams.get('pin') and
sessionStorage.setItem('ov_pin', p)), remove the pin from the visible URL by
creating a copy of the current URL without the 'pin' search param and calling
history.replaceState(null, document.title, cleanedUrl); keep the existing
try/catch behavior so any errors during URL parsing or replaceState are
swallowed, and ensure this runs only when typeof window !== 'undefined' and a
pin was actually found.
- Around line 50-53: The current code assumes opts.headers is a plain object and
spreads it into headers, which will drop headers when callers pass a Headers
instance or string[][]; change the logic in the client fetch block to normalize
headers by constructing a new Headers from opts.headers (e.g., const headers =
new Headers(opts.headers || {})), then set the pin header via
headers.set('X-OmniVoice-Pin', pin) when pin exists, and pass that Headers
instance into fetch(apiUrl(path), { ...opts, headers }) so all header forms
(Headers, string[][], plain object) are preserved; update any type assertions
around opts.headers as needed.
In `@frontend/src/App.jsx`:
- Line 825: The RemoteAuthGate currently wraps only the main app shell and so is
bypassed by early-return branches; move the <RemoteAuthGate> wrapper to a higher
level so it encloses the entire render/return logic that checks setupChecked,
setupNeeded and bootstrapStage (i.e. place RemoteAuthGate above the early-return
conditions in App.jsx so that those branches render inside RemoteAuthGate);
update both occurrences (around line ranges ~825 and ~1147) to ensure PIN gating
runs before any early returns.
In `@frontend/src/components/NetworkToggle.jsx`:
- Around line 35-36: The hardcoded English strings in NetworkToggle.jsx (the
window.confirm prompt and other visible copy across the sharing flow lines
noted) must be replaced with i18n keys via the t(...) function: update the
window.confirm call and all labels/buttons/messages referenced in this component
(e.g., the prompt string, button labels and status text around setBusy and the
sharing UI) to use t('networkSharing.prompt'), t('networkSharing.allow'), etc.,
and add corresponding entries in the locales/*.json files; ensure you import the
i18n hook/function used elsewhere (e.g., const { t } = useI18n() or import { t }
from 'i18n') and use those keys everywhere the component currently uses
hardcoded English.
- Line 48: The copy function currently calls navigator.clipboard.writeText(text)
without awaiting it, so denied/failed writes still show a success toast; update
the copy function to check navigator.clipboard exists, await
navigator.clipboard.writeText(text) inside a try/catch, call
toast.success('Copied') only after the await succeeds, and call toast.error(...)
in the catch (or when clipboard is unavailable) to surface failures; adjust the
function referenced as copy in NetworkToggle.jsx accordingly.
- Around line 69-76: The current copy/open actions build and use url which
includes the sensitive st.pin query parameter and thus leaks the PIN into
browser history; update NetworkToggle.jsx so that the URL used by copy(url) and
openExternal(url) does NOT include ?pin=st.pin (keep using ip and
st.share_port), and instead keep the PIN only for the QR or other deliberate
share mechanism if needed; specifically change the value passed to the onClick
handlers (the url variable used with copy(...) and openExternal(...)) to a
PIN-less share URL while leaving any QR-generation code to reference the full
URL with st.pin if intended.
In `@frontend/src/components/RemoteAuthGate.jsx`:
- Around line 32-36: Replace hardcoded UI strings in RemoteAuthGate.jsx with
i18n keys: import and use the translation hook (e.g., const { t } =
useTranslation()) and swap "Enter access PIN", the paragraph text, the label
text ("Access PIN") and the button text ("Connect") for
t('remoteAuth.enterPin'), t('remoteAuth.sharedInstance'),
t('remoteAuth.accessPinLabel'), t('remoteAuth.connect') (or similar keys you
choose), then add those keys and translations to the locales/*.json files so the
component localizes correctly.
In `@frontend/src/components/settings/SharingPanel.jsx`:
- Around line 79-85: The SharingPanel component contains hardcoded user-facing
strings (titles, button labels, aria-labels, status text, and toast messages
such as the toast.success/toast.error lines and the catch error message) that
must be routed through the i18n layer; replace all literals in SharingPanel
(component name) with t('sharingPanel.<descriptive_key>') calls (e.g., keys for
"Sharing & Remote Access", "Checking for Tailscale…", "Install Tailscale",
success/failure toast messages, button/aria labels, and any status text between
lines ~96–225), and add corresponding entries to the locales/*.json files (e.g.,
locales/en.json) with those keys and English values before committing so every
user-facing string is translatable.
In `@tests/test_network_middleware.py`:
- Around line 41-43: The test test_spa_shell_served_without_pin currently
requests /health which may be public; change it to request a client-side SPA
route (e.g. "/") using TestClient(_app_with_pin(), client=("10.0.0.5", 1)) and
assert you receive the SPA shell: check response.status_code == 200,
response.headers["content-type"] contains "text/html", and the response.text
includes a known shell marker (for example a root mount element like "<div
id=\"root\">" or another string your app's shell emits). This ensures the SPA
fallback is exercised when PIN protection is enabled.
In `@tests/test_no_hardcoded_cjk.py`:
- Around line 98-111: The test helper currently swallows all exceptions (except
Exception: pass) which can hide real bugs; narrow it to subprocess-related
failures by replacing the broad except with an exception tuple that only catches
subprocess failures (e.g. except (subprocess.SubprocessError,
FileNotFoundError): pass) around the subprocess.run call in the block that uses
subprocess.run, names, _REPO and _SKIP_EXT so only expected git/subprocess
errors are ignored and other exceptions propagate.
---
Nitpick comments:
In `@docs/sharing.md`:
- Line 43: Summary: The sentence is ambiguous about how server deployments
interact with the in-app toggle; it should explicitly state that the toggle is
hidden for server deployments when OMNIVOICE_BIND_HOST=0.0.0.0. Update the line
to say that server deployments (e.g., Docker with OMNIVOICE_BIND_HOST=0.0.0.0)
manage networking themselves and therefore the in-app "share on local network"
toggle is not shown/available in those deployments; replace the ambiguous phrase
"is unaffected by these flows" with a clear statement that the toggle is
hidden/disabled when OMNIVOICE_BIND_HOST=0.0.0.0 so readers understand the
condition.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b4e63ab8-856a-4211-8734-5dd5c6d8452e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!**/*.lock,!**/bun.lock
📒 Files selected for processing (26)
backend/api/routers/system.pybackend/api/schemas.pybackend/main.pybackend/services/network_share.pybackend/services/tailscale.pydocs/sharing.mddocs/superpowers/specs/2026-05-30-network-sharing-design.mdfrontend/package.jsonfrontend/src/App.jsxfrontend/src/api/client.test.tsfrontend/src/api/client.tsfrontend/src/components/LogsFooter.jsxfrontend/src/components/NetworkToggle.cssfrontend/src/components/NetworkToggle.jsxfrontend/src/components/NetworkToggle.test.jsxfrontend/src/components/RemoteAuthGate.jsxfrontend/src/components/RemoteAuthGate.test.jsxfrontend/src/components/settings/SharingPanel.cssfrontend/src/components/settings/SharingPanel.jsxfrontend/src/components/settings/SharingPanel.test.jsxfrontend/src/i18n/locales/en.jsonfrontend/src/pages/Settings.jsxtests/test_network_middleware.pytests/test_network_share.pytests/test_no_hardcoded_cjk.pytests/test_tailscale_service.py
Comment on lines
+487
to
+515
| class NetworkAccessMiddleware(BaseHTTPMiddleware): | ||
| """When a share PIN is set, require it for non-loopback clients on API | ||
| routes. Inert when no PIN (default + docker deploys). Loopback (incl. | ||
| Tailscale-proxied) always bypasses; the SPA shell is always served so the | ||
| PIN gate UI can load.""" | ||
|
|
||
| async def dispatch(self, request, call_next): | ||
| ns = getattr(request.app.state, "network_share", None) | ||
| pin = getattr(ns, "pin", None) if ns else None | ||
| if not pin: | ||
| return await call_next(request) | ||
| client = request.client.host if request.client else None | ||
| if client in _LOOPBACK_CLIENTS: | ||
| return await call_next(request) | ||
| path = request.url.path | ||
| if path in _SHELL_PATHS or path.startswith("/assets/") or path.startswith("/favicon"): | ||
| return await call_next(request) | ||
| supplied = ( | ||
| request.headers.get("x-omnivoice-pin") | ||
| or request.query_params.get("pin") | ||
| or request.cookies.get("ov_pin") | ||
| or "" | ||
| ) | ||
| if not secrets.compare_digest(supplied, pin): | ||
| return JSONResponse({"detail": "PIN required"}, status_code=401) | ||
| response = await call_next(request) | ||
| if request.cookies.get("ov_pin") != pin: | ||
| response.set_cookie("ov_pin", pin, samesite="lax") | ||
| return response |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WebSocket endpoint declarations ==="
rg -n -C2 --glob 'backend/**/*.py' '@.*websocket|WebSocket\('
echo
echo "=== WebSocket-related routers mounted from backend/main.py ==="
rg -n -C2 'include_router\(.*capture_ws|include_router\(.*websocket|capture_ws' backend/main.py backend/api/routers
echo
echo "=== Existing PIN / loopback checks inside websocket code ==="
rg -n -C3 --glob 'backend/**/*.py' 'x-omnivoice-pin|ov_pin|compare_digest|require_loopback|query_params\.get\("pin"\)'Repository: debpalash/OmniVoice-Studio
Length of output: 11191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== capture_ws websocket loopback/PIN enforcement ==="
sed -n '1,220p' backend/api/routers/capture_ws.py | nl -ba | sed -n '1,220p' | sed -n '1,220p'
echo
echo "=== events websocket router definition (dependencies/guards) ==="
sed -n '1,220p' backend/api/routers/events.py | nl -ba | sed -n '1,220p'
echo
echo "=== tts_stream websocket router definition (dependencies/guards) ==="
sed -n '1,260p' backend/api/routers/tts_stream.py | nl -ba | sed -n '1,260p'
echo
echo "=== Any PIN/loopback checks in websocket handlers (broader search) ==="
rg -n --glob 'backend/api/routers/*.py' 'ws_|websocket\(|require_loopback|x-omnivoice-pin|ov_pin|compare_digest|loopback' backend/api/routers | head -n 200Repository: debpalash/OmniVoice-Studio
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== capture_ws.py (top + route + guard region) ==="
sed -n '1,220p' backend/api/routers/capture_ws.py
echo
echo "=== events.py (top + websocket route) ==="
sed -n '1,220p' backend/api/routers/events.py
echo
echo "=== tts_stream.py (top + websocket route) ==="
sed -n '1,260p' backend/api/routers/tts_stream.py
echo
echo "=== WebSocket handlers: look for loopback/PIN enforcement tokens ==="
rg -n --glob 'backend/api/routers/*.py' 'ws_|websocket\(|require_loopback|x-omnivoice-pin|ov_pin|compare_digest|pin|loopback' backend/api/routers/*.pyRepository: debpalash/OmniVoice-Studio
Length of output: 25403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== events.py: check for loopback/PIN enforcement terms ==="
rg -n 'require_loopback|_LOOPBACK_HOSTS|loopback|x-omnivoice-pin|ov_pin|compare_digest|pin' backend/api/routers/events.py || true
echo
echo "=== tts_stream.py: check for loopback/PIN enforcement terms ==="
rg -n 'require_loopback|_LOOPBACK_HOSTS|loopback|x-omnivoice-pin|ov_pin|compare_digest|pin' backend/api/routers/tts_stream.py || true
echo
echo "=== events.py / tts_stream.py: router declarations (dependencies/prefix) ==="
rg -n 'router\s*=\s*APIRouter' backend/api/routers/events.py backend/api/routers/tts_stream.pyRepository: debpalash/OmniVoice-Studio
Length of output: 686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ws_events: does it check websocket.client.host or pin headers/cookies? ==="
rg -n 'websocket\.client\.host|client\.host|x-omnivoice-pin|ov_pin|pin' backend/api/routers/events.py || true
echo
echo "=== ws_tts: does it check websocket.client.host or pin headers/cookies? ==="
rg -n 'websocket\.client\.host|client\.host|x-omnivoice-pin|ov_pin|pin' backend/api/routers/tts_stream.py || trueRepository: debpalash/OmniVoice-Studio
Length of output: 527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Locate NetworkAccessMiddleware registration in backend/main.py ==="
rg -n 'NetworkAccessMiddleware' backend/main.py
rg -n 'add_middleware|middleware\(' backend/main.py | head -n 50
echo
echo "=== Show surrounding lines where middleware is registered ==="
# Print a window around each match to capture context.
rg -n 'NetworkAccessMiddleware' backend/main.py | cut -d: -f1 | while read -r line; do
start=$((line-20))
end=$((line+20))
sed -n "${start},${end}p" backend/main.py | nl -ba -w2 -s':'
echo "----"
doneRepository: debpalash/OmniVoice-Studio
Length of output: 441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== backend/main.py around app.add_middleware(NetworkAccessMiddleware) occurrences ==="
for start in 470 510 520 530 540; do
end=$((start+60))
echo
echo "---- lines ${start}-${end} ----"
sed -n "${start},${end}p" backend/main.py
doneRepository: debpalash/OmniVoice-Studio
Length of output: 11855
PIN gate bypasses WebSockets (/ws/events and /ws/tts)
NetworkAccessMiddleware is BaseHTTPMiddleware, so it only guards HTTP—WebSocket endpoints skip it. /ws/events (backend/api/routers/events.py) and /ws/tts (backend/api/routers/tts_stream.py) accept connections without any PIN/loopback validation, so non-loopback clients can reach them even when a share PIN is set. (/ws/transcribe does have an inline loopback-only guard, but it doesn’t enforce the PIN.)
🤖 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 487 - 515, NetworkAccessMiddleware only
protects HTTP; add the same PIN/loopback checks to the WebSocket handlers so
/ws/events and /ws/tts enforce the gate. Extract the PIN-check logic from
NetworkAccessMiddleware.dispatch into a reusable helper (e.g.,
validate_network_access(request_or_ws, app_state)) that: reads
app.state.network_share.pin, allows _LOOPBACK_CLIENTS, reads supplied pin from
headers/query/cookies, uses secrets.compare_digest, and returns True/False (or
raises). Call that helper at the start of the connection handlers in
backend/api/routers/events.py and backend/api/routers/tts_stream.py and if it
fails, immediately reject/close the WebSocket (WebSocket.close with an
appropriate code/reason) so non-loopback clients cannot connect when a PIN is
set; keep /ws/transcribe behavior consistent by either calling the same helper
or updating its inline guard to enforce the PIN.
Comment on lines
+66
to
+108
| async def enable(app) -> ShareState: | ||
| global _server, _task, _state | ||
| if _state.enabled: | ||
| return _state | ||
| port = _find_free_port(BACKEND_PORT + 1) | ||
| pin = _gen_pin() | ||
| config = uvicorn.Config(app, host="0.0.0.0", port=port, log_level="warning") | ||
| server = uvicorn.Server(config) | ||
| server.install_signal_handlers = lambda: None # never hijack signals in-process | ||
| _task = asyncio.create_task(server.serve()) | ||
| for _ in range(100): # ~5s for the socket to bind | ||
| if getattr(server, "started", False): | ||
| break | ||
| await asyncio.sleep(0.05) | ||
| if not getattr(server, "started", False): | ||
| # Bind failed (e.g. the port was taken in the race after the | ||
| # free-port probe). Tear down and stay Local — never report enabled | ||
| # with a listener that isn't actually up (spec §7). | ||
| server.should_exit = True | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=2) | ||
| except Exception: | ||
| pass | ||
| _task = None | ||
| raise RuntimeError("share listener failed to start") | ||
| _server = server | ||
| _state = ShareState(True, port, pin, lan_ipv4_addresses()) | ||
| app.state.network_share = _state | ||
| return _state | ||
|
|
||
|
|
||
| async def disable(app) -> ShareState: | ||
| global _server, _task, _state | ||
| if _server is not None: | ||
| _server.should_exit = True | ||
| if _task is not None: | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=5) | ||
| except Exception: | ||
| pass | ||
| _server = _task = None | ||
| _state = ShareState() | ||
| app.state.network_share = _state |
Contributor
There was a problem hiding this comment.
Serialize listener lifecycle changes.
_state, _server, and _task are mutated without a lock. Two overlapping /system/network/enable or enable/disable requests can both pass the enabled check, start different listeners, and then overwrite the only handle that can stop the orphaned server.
Suggested fix
_state = ShareState()
_server: Optional["uvicorn.Server"] = None
_task: Optional["asyncio.Task"] = None
+_state_lock = asyncio.Lock()
...
async def enable(app) -> ShareState:
global _server, _task, _state
- if _state.enabled:
- return _state
- port = _find_free_port(BACKEND_PORT + 1)
- pin = _gen_pin()
- config = uvicorn.Config(app, host="0.0.0.0", port=port, log_level="warning")
- server = uvicorn.Server(config)
- server.install_signal_handlers = lambda: None # never hijack signals in-process
- _task = asyncio.create_task(server.serve())
- for _ in range(100): # ~5s for the socket to bind
- if getattr(server, "started", False):
- break
- await asyncio.sleep(0.05)
- if not getattr(server, "started", False):
- server.should_exit = True
- try:
- await asyncio.wait_for(_task, timeout=2)
- except Exception:
- pass
- _task = None
- raise RuntimeError("share listener failed to start")
- _server = server
- _state = ShareState(True, port, pin, lan_ipv4_addresses())
- app.state.network_share = _state
- return _state
+ async with _state_lock:
+ if _state.enabled:
+ return _state
+ port = _find_free_port(BACKEND_PORT + 1)
+ pin = _gen_pin()
+ config = uvicorn.Config(app, host="0.0.0.0", port=port, log_level="warning")
+ server = uvicorn.Server(config)
+ server.install_signal_handlers = lambda: None
+ _task = asyncio.create_task(server.serve())
+ for _ in range(100):
+ if getattr(server, "started", False):
+ break
+ await asyncio.sleep(0.05)
+ if not getattr(server, "started", False):
+ server.should_exit = True
+ try:
+ await asyncio.wait_for(_task, timeout=2)
+ except Exception:
+ pass
+ _task = None
+ raise RuntimeError("share listener failed to start")
+ _server = server
+ _state = ShareState(True, port, pin, lan_ipv4_addresses())
+ app.state.network_share = _state
+ return _state🧰 Tools
🪛 GitHub Check: Bandit
[warning] 72-72:
Possible binding to all interfaces.
🪛 GitHub Check: CodeQL
[notice] 87-87: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
[notice] 89-89: Unused global variable
The global variable '_task' is not used.
[notice] 104-104: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
[notice] 106-106: Unused global variable
The global variable '_server' is not used.
🪛 Ruff (0.15.14)
[error] 72-72: Possible binding to all interfaces
(S104)
[error] 87-88: try-except-pass detected, consider logging the exception
(S110)
[warning] 87-87: Do not catch blind exception: Exception
(BLE001)
[error] 104-105: try-except-pass detected, consider logging the exception
(S110)
[warning] 104-104: 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/services/network_share.py` around lines 66 - 108, The enable/disable
functions mutate globals (_state, _server, _task) without synchronization,
causing races; add a module-level asyncio.Lock (e.g. _lock = asyncio.Lock()) and
wrap the critical sections in both enable and disable with "async with _lock:"
so the check (if _state.enabled), the server start/teardown, and assignments to
_server/_task/_state and app.state.network_share are serialized; ensure the lock
is acquired before reading/modifying these globals and released via context
manager even on exceptions, and import asyncio if needed.
Comment on lines
+97
to
+108
| async def disable(app) -> ShareState: | ||
| global _server, _task, _state | ||
| if _server is not None: | ||
| _server.should_exit = True | ||
| if _task is not None: | ||
| try: | ||
| await asyncio.wait_for(_task, timeout=5) | ||
| except Exception: | ||
| pass | ||
| _server = _task = None | ||
| _state = ShareState() | ||
| app.state.network_share = _state |
Contributor
There was a problem hiding this comment.
Don't clear the share state before shutdown is confirmed.
If await asyncio.wait_for(_task, timeout=5) times out or raises, this still resets _state and app.state.network_share to disabled. That can leave the 0.0.0.0 listener alive while the UI reports that sharing is off.
Suggested fix
async def disable(app) -> ShareState:
global _server, _task, _state
if _server is not None:
_server.should_exit = True
if _task is not None:
try:
await asyncio.wait_for(_task, timeout=5)
- except Exception:
- pass
+ except Exception as exc:
+ raise RuntimeError("share listener failed to stop cleanly") from exc
_server = _task = None
_state = ShareState()
app.state.network_share = _state
return _state🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 104-104: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
[notice] 106-106: Unused global variable
The global variable '_server' is not used.
🪛 Ruff (0.15.14)
[error] 104-105: try-except-pass detected, consider logging the exception
(S110)
[warning] 104-104: 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/services/network_share.py` around lines 97 - 108, The disable
function currently clears _state and app.state.network_share regardless of
whether the background server/task actually stopped; change disable so it only
resets _server, _task and sets _state = ShareState() and app.state.network_share
after confirming the server task has completed successfully (i.e., after await
asyncio.wait_for(_task, timeout=5) returns), and on timeout/exception leave
_server/_task/_state untouched (or re-raise/log) so the UI still reflects that
sharing may be active; update references in disable to only mutate _server,
_task and _state after successful shutdown and handle the exception branch
without clearing app.state.network_share.
Comment on lines
+50
to
+58
| def serve_disable() -> dict: | ||
| cli = _cli() | ||
| if not cli: | ||
| return {"ok": True} | ||
| try: | ||
| subprocess.run([cli, "serve", "reset"], capture_output=True, text=True, timeout=20) | ||
| except Exception: | ||
| pass | ||
| return {"ok": True} |
Contributor
There was a problem hiding this comment.
Surface tailscale serve reset failures to the caller.
serve_disable() always returns {"ok": True} even when the subprocess errors or times out. The settings UI will report remote access as disabled while tailscale serve may still be publishing the backend.
Suggested fix
def serve_disable() -> dict:
cli = _cli()
if not cli:
return {"ok": True}
try:
- subprocess.run([cli, "serve", "reset"], capture_output=True, text=True, timeout=20)
- except Exception:
- pass
- return {"ok": True}
+ r = subprocess.run([cli, "serve", "reset"], capture_output=True, text=True, timeout=20)
+ if r.returncode != 0:
+ return {"ok": False, "error": (r.stderr or r.stdout or "tailscale serve reset failed").strip()}
+ return {"ok": True}
+ except Exception as e:
+ return {"ok": False, "error": str(e)}🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 56-56: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
🪛 Ruff (0.15.14)
[error] 55-55: subprocess call: check for execution of untrusted input
(S603)
[error] 56-57: try-except-pass detected, consider logging the exception
(S110)
[warning] 56-56: 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/services/tailscale.py` around lines 50 - 58, The serve_disable
function currently swallows all subprocess errors; update it to surface failures
by running subprocess.run with check=True (or inspect
CompletedProcess.returncode) when calling subprocess.run([cli, "serve",
"reset"], ...) and catch subprocess.CalledProcessError and
subprocess.TimeoutExpired (and any other Exception) to return a failure result
to the caller instead of always {"ok": True}; keep the early short-circuit when
_cli() returns falsy, but in error cases return a structured failure such as
{"ok": False, "error": str(e)} that includes the exception message so the
caller/UI can detect and display the failure (refer to serve_disable, _cli, and
the subprocess.run invocation).
Comment on lines
+13
to
+21
| // Capture a QR-supplied PIN once on load. When LAN sharing is on, the host's | ||
| // QR code links to `http://<lan-ip>:<port>/?pin=<pin>`; stash it in | ||
| // sessionStorage so apiFetch attaches it to every request automatically. | ||
| if (typeof window !== 'undefined') { | ||
| try { | ||
| const p = new URL(window.location.href).searchParams.get('pin'); | ||
| if (p) sessionStorage.setItem('ov_pin', p); | ||
| } catch { /* noop */ } | ||
| } |
Contributor
There was a problem hiding this comment.
Strip the pin query parameter after storing it.
Right now the QR PIN stays in the address bar and browser history after bootstrap, which turns the shared URL into a longer-lived bearer link than intended. Persist it, then immediately replaceState the URL without pin.
Suggested fix
if (typeof window !== 'undefined') {
try {
- const p = new URL(window.location.href).searchParams.get('pin');
- if (p) sessionStorage.setItem('ov_pin', p);
+ const url = new URL(window.location.href);
+ const p = url.searchParams.get('pin');
+ if (p) {
+ sessionStorage.setItem('ov_pin', p);
+ url.searchParams.delete('pin');
+ window.history.replaceState(window.history.state, '', `${url.pathname}${url.search}${url.hash}`);
+ }
} catch { /* noop */ }
}📝 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
| // Capture a QR-supplied PIN once on load. When LAN sharing is on, the host's | |
| // QR code links to `http://<lan-ip>:<port>/?pin=<pin>`; stash it in | |
| // sessionStorage so apiFetch attaches it to every request automatically. | |
| if (typeof window !== 'undefined') { | |
| try { | |
| const p = new URL(window.location.href).searchParams.get('pin'); | |
| if (p) sessionStorage.setItem('ov_pin', p); | |
| } catch { /* noop */ } | |
| } | |
| // Capture a QR-supplied PIN once on load. When LAN sharing is on, the host's | |
| // QR code links to `http://<lan-ip>:<port>/?pin=<pin>`; stash it in | |
| // sessionStorage so apiFetch attaches it to every request automatically. | |
| if (typeof window !== 'undefined') { | |
| try { | |
| const url = new URL(window.location.href); | |
| const p = url.searchParams.get('pin'); | |
| if (p) { | |
| sessionStorage.setItem('ov_pin', p); | |
| url.searchParams.delete('pin'); | |
| window.history.replaceState(window.history.state, '', `${url.pathname}${url.search}${url.hash}`); | |
| } | |
| } catch { /* noop */ } | |
| } |
🤖 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 `@frontend/src/api/client.ts` around lines 13 - 21, After grabbing and
persisting the QR PIN (the block that calls new
URL(window.location.href).searchParams.get('pin') and
sessionStorage.setItem('ov_pin', p)), remove the pin from the visible URL by
creating a copy of the current URL without the 'pin' search param and calling
history.replaceState(null, document.title, cleanedUrl); keep the existing
try/catch behavior so any errors during URL parsing or replaceState are
swallowed, and ensure this runs only when typeof window !== 'undefined' and a
pin was actually found.
Comment on lines
+69
to
+76
| const url = `http://${ip}:${st.share_port}/?pin=${st.pin}`; | ||
| return ( | ||
| <div key={ip} className="net-toggle__row"> | ||
| <div className="net-toggle__row-main"> | ||
| <code className="net-toggle__addr">{ip}:{st.share_port}</code> | ||
| <div className="net-toggle__row-actions"> | ||
| <button type="button" className="net-toggle__iconbtn" onClick={() => copy(url)} aria-label={`Copy ${ip}`} title="Copy link"><Copy size={12} /></button> | ||
| <button type="button" className="net-toggle__iconbtn" onClick={() => openExternal(url)} aria-label={`Open ${ip}`} title="Open in browser"><ExternalLink size={12} /></button> |
Contributor
There was a problem hiding this comment.
Don't expose the PIN in the copied/opened LAN URL.
Using ?pin=... here writes the access secret into browser history and any other URL surface the external browser persists. The QR code may be an intentional tradeoff, but the copy/open actions do not need to bypass the PIN prompt this way.
🔒 Safer split
- const url = `http://${ip}:${st.share_port}/?pin=${st.pin}`;
+ const url = `http://${ip}:${st.share_port}/`;
return (
...
- <button type="button" className="net-toggle__iconbtn" onClick={() => copy(url)} aria-label={`Copy ${ip}`} title="Copy link"><Copy size={12} /></button>
- <button type="button" className="net-toggle__iconbtn" onClick={() => openExternal(url)} aria-label={`Open ${ip}`} title="Open in browser"><ExternalLink size={12} /></button>
+ <button type="button" className="net-toggle__iconbtn" onClick={() => copy(url)} aria-label={`Copy ${ip}`} title="Copy link"><Copy size={12} /></button>
+ <button type="button" className="net-toggle__iconbtn" onClick={() => openExternal(url)} aria-label={`Open ${ip}`} title="Open in browser"><ExternalLink size={12} /></button>🤖 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 `@frontend/src/components/NetworkToggle.jsx` around lines 69 - 76, The current
copy/open actions build and use url which includes the sensitive st.pin query
parameter and thus leaks the PIN into browser history; update NetworkToggle.jsx
so that the URL used by copy(url) and openExternal(url) does NOT include
?pin=st.pin (keep using ip and st.share_port), and instead keep the PIN only for
the QR or other deliberate share mechanism if needed; specifically change the
value passed to the onClick handlers (the url variable used with copy(...) and
openExternal(...)) to a PIN-less share URL while leaving any QR-generation code
to reference the full URL with st.pin if intended.
Comment on lines
+32
to
+36
| <h2>Enter access PIN</h2> | ||
| <p>This OmniVoice instance is shared on the network. Enter the PIN shown on the host.</p> | ||
| <label htmlFor="ov-pin">Access PIN</label> | ||
| <input id="ov-pin" inputMode="numeric" value={pin} onChange={(e) => setPin(e.target.value)} autoFocus /> | ||
| <button type="submit">Connect</button> |
Contributor
There was a problem hiding this comment.
Route the gate copy through i18n.
These new labels/messages are hardcoded in JSX, so the remote-auth flow will not localize with the rest of the app.
♻️ Minimal fix pattern
import { useEffect, useState } from 'react';
+import { useTranslation } from 'react-i18next';
export default function RemoteAuthGate({ children, forceGate = false }) {
+ const { t } = useTranslation();
const [gated, setGated] = useState(forceGate);
const [pin, setPin] = useState('');
...
- <h2>Enter access PIN</h2>
- <p>This OmniVoice instance is shared on the network. Enter the PIN shown on the host.</p>
- <label htmlFor="ov-pin">Access PIN</label>
+ <h2>{t('sharing.remoteAuth.title')}</h2>
+ <p>{t('sharing.remoteAuth.description')}</p>
+ <label htmlFor="ov-pin">{t('sharing.remoteAuth.label')}</label>
...
- <button type="submit">Connect</button>
+ <button type="submit">{t('sharing.remoteAuth.connect')}</button>As per coding guidelines, frontend/src/**/*.{vue,ts,tsx,js,jsx}: "All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files".
🤖 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 `@frontend/src/components/RemoteAuthGate.jsx` around lines 32 - 36, Replace
hardcoded UI strings in RemoteAuthGate.jsx with i18n keys: import and use the
translation hook (e.g., const { t } = useTranslation()) and swap "Enter access
PIN", the paragraph text, the label text ("Access PIN") and the button text
("Connect") for t('remoteAuth.enterPin'), t('remoteAuth.sharedInstance'),
t('remoteAuth.accessPinLabel'), t('remoteAuth.connect') (or similar keys you
choose), then add those keys and translations to the locales/*.json files so the
component localizes correctly.
Comment on lines
+79
to
+85
| toast.success('Tailscale serve enabled'); | ||
| await refresh(); | ||
| } else { | ||
| toast.error(r?.error || 'Could not enable Tailscale'); | ||
| } | ||
| } catch (e) { | ||
| toast.error(`Could not enable Tailscale: ${e.message}`); |
Contributor
There was a problem hiding this comment.
Move the new Sharing UI copy into i18n keys.
This panel introduces a lot of hardcoded user-facing text (Sharing & Remote Access, Checking for Tailscale…, Install Tailscale, toast messages, button labels, aria labels, etc.). Please route the new strings through t('...') and add them to locales/*.json before merging.
As per coding guidelines, frontend/src/**/*.{vue,ts,tsx,js,jsx}: All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files, never hardcode non-English text.
Also applies to: 96-103, 117-225
🤖 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 `@frontend/src/components/settings/SharingPanel.jsx` around lines 79 - 85, The
SharingPanel component contains hardcoded user-facing strings (titles, button
labels, aria-labels, status text, and toast messages such as the
toast.success/toast.error lines and the catch error message) that must be routed
through the i18n layer; replace all literals in SharingPanel (component name)
with t('sharingPanel.<descriptive_key>') calls (e.g., keys for "Sharing & Remote
Access", "Checking for Tailscale…", "Install Tailscale", success/failure toast
messages, button/aria labels, and any status text between lines ~96–225), and
add corresponding entries to the locales/*.json files (e.g., locales/en.json)
with those keys and English values before committing so every user-facing string
is translatable.
Comment on lines
+41
to
+43
| def test_spa_shell_served_without_pin(): | ||
| c = TestClient(_app_with_pin(), client=("10.0.0.5", 1)) | ||
| assert c.get("/health").status_code == 200 |
Contributor
There was a problem hiding this comment.
Exercise an actual SPA route here.
/health can stay public for reasons unrelated to the shell fallback, so this test can pass even if browser routes are still PIN-gated. Hit / (or another client-side route) and assert the shell response instead.
🤖 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_network_middleware.py` around lines 41 - 43, The test
test_spa_shell_served_without_pin currently requests /health which may be
public; change it to request a client-side SPA route (e.g. "/") using
TestClient(_app_with_pin(), client=("10.0.0.5", 1)) and assert you receive the
SPA shell: check response.status_code == 200, response.headers["content-type"]
contains "text/html", and the response.text includes a known shell marker (for
example a root mount element like "<div id=\"root\">" or another string your
app's shell emits). This ensures the SPA fallback is exercised when PIN
protection is enabled.
Comment on lines
+98
to
+111
| try: | ||
| out = subprocess.run( | ||
| ["git", "ls-files", "-z"], | ||
| cwd=str(_REPO), capture_output=True, text=True, timeout=30, check=True, | ||
| ).stdout | ||
| names = [n for n in out.split("\0") if n] | ||
| if names: | ||
| for n in names: | ||
| if os.path.splitext(n)[1].lower() in _SKIP_EXT: | ||
| continue | ||
| yield _REPO / n | ||
| return | ||
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
Narrow the fallback exception to subprocess failures only.
Catching all exceptions and silently passing can hide unrelated bugs in this test helper. Restrict this to expected subprocess failures so genuine errors still surface.
Proposed fix
- except Exception:
- pass
+ except (subprocess.SubprocessError, OSError):
+ # Not a git checkout / git unavailable / command failed: use filesystem fallback.
+ pass📝 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
| try: | |
| out = subprocess.run( | |
| ["git", "ls-files", "-z"], | |
| cwd=str(_REPO), capture_output=True, text=True, timeout=30, check=True, | |
| ).stdout | |
| names = [n for n in out.split("\0") if n] | |
| if names: | |
| for n in names: | |
| if os.path.splitext(n)[1].lower() in _SKIP_EXT: | |
| continue | |
| yield _REPO / n | |
| return | |
| except Exception: | |
| pass | |
| try: | |
| out = subprocess.run( | |
| ["git", "ls-files", "-z"], | |
| cwd=str(_REPO), capture_output=True, text=True, timeout=30, check=True, | |
| ).stdout | |
| names = [n for n in out.split("\0") if n] | |
| if names: | |
| for n in names: | |
| if os.path.splitext(n)[1].lower() in _SKIP_EXT: | |
| continue | |
| yield _REPO / n | |
| return | |
| except (subprocess.SubprocessError, OSError): | |
| # Not a git checkout / git unavailable / command failed: use filesystem fallback. | |
| pass |
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 110-110: Empty except
'except' clause does nothing but pass and there is no explanatory comment.
🪛 Ruff (0.15.14)
[error] 100-100: Starting a process with a partial executable path
(S607)
[error] 110-111: try-except-pass detected, consider logging the exception
(S110)
[warning] 110-110: 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 `@tests/test_no_hardcoded_cjk.py` around lines 98 - 111, The test helper
currently swallows all exceptions (except Exception: pass) which can hide real
bugs; narrow it to subprocess-related failures by replacing the broad except
with an exception tuple that only catches subprocess failures (e.g. except
(subprocess.SubprocessError, FileNotFoundError): pass) around the subprocess.run
call in the block that uses subprocess.run, names, _REPO and _SKIP_EXT so only
expected git/subprocess errors are ignored and other exceptions propagate.
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
2 participants
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.
Lets a user access the same running OmniVoice instance from their other machines — without restarting the backend or dropping the loaded model / in-flight jobs, and with loopback-only as the default on every launch. Supersedes the raw
0.0.0.0default-flip from #125 with an explicit, user-driven feature.What it does
uvicorn.Serveron0.0.0.0:<P+1>serving the same FastAPIapp(no restart, same model/jobs/SSE). The panel lists every LAN address with copy / open / QR and a 6-digit PIN. Disabling stops the listener → the0.0.0.0socket is genuinely closed.tailscale serveto publish the loopback backend over your tailnet at an HTTPS*.ts.netURL (TLS + identity by Tailscale, no open ports, no PIN). Graceful when the CLI is absent.Security model
0.0.0.0until you explicitly enable sharing; no persisted auto-exposure.0.0.0.0deploys and the default path are unaffected (no regression). When active: loopback (incl. Tailscale-proxied) bypasses; the SPA shell +/healthare served; all other non-loopback requests need the PIN (X-OmniVoice-Pinheader /?pin=/ov_pincookie, constant-time compare); invalid/absent → 401./system/*stays loopback-gated via the existingrequire_loopbackdependency (non-spoofablerequest.client.host) — a PIN'd LAN client can never reach the control endpoints orset-env. Verified in a dedicated security review.Tests
test_network_share.py,test_network_middleware.py,test_tailscale_service.py(+ the CJK guard) — all green. Frontend (Vitest): client PIN injection,RemoteAuthGate,NetworkToggle,SharingPanel— all green. Build passes; no hardcoded CJK.Notes / follow-ups
App.jsx; moving it to the outermost provider would also cover a remote device that types a bare URL (no?pin=) during first-run setup — a narrow edge, noted for a follow-up.Design spec:
docs/superpowers/specs/2026-05-30-network-sharing-design.md· Plan:docs/superpowers/plans/2026-05-30-network-sharing.md· Guide:docs/sharing.md🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security / Behavior
Documentation