From 86a4949751b148e530670af9940d5c85fa026169 Mon Sep 17 00:00:00 2001 From: Evandro Fonseca Junior Date: Tue, 20 Oct 2026 00:49:37 -0300 Subject: [PATCH] =?UTF-8?q?wip:=20snapshot=20das=20altera=C3=A7=C3=B5es=20?= =?UTF-8?q?n=C3=A3o=20commitadas=20(2026-10-20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Origem: D:/Documents/Github/HealthTracking Arquivos omitidos (segredos, bancos de dados, logs, >50 MB): 1 --- .pi/settings.json | 27 ++++ code-review.md | 288 ++++++++++++++++++++++++++++++++++++++++ scripts/debug_mcp.py | 42 ++++++ scripts/fix_mcp_json.py | 64 +++++++++ 4 files changed, 421 insertions(+) create mode 100644 .pi/settings.json create mode 100644 code-review.md create mode 100644 scripts/debug_mcp.py create mode 100644 scripts/fix_mcp_json.py diff --git a/.pi/settings.json b/.pi/settings.json new file mode 100644 index 0000000..4280ad2 --- /dev/null +++ b/.pi/settings.json @@ -0,0 +1,27 @@ +{ + "hud": { + "mode": "footer", + "position": "bottom-center", + "shortcut": "ctrl+shift+h", + "switchShortcut": "ctrl+.", + "minimizeShortcut": "ctrl+h", + "autoCompactWhileStreaming": true, + "startupNotification": true, + "usageDisplay": "metered", + "contextIndicator": "icon", + "expandedWidth": 42, + "compactWidth": 26, + "minTerminalWidth": 90, + "margin": { + "top": 1, + "right": 1, + "bottom": 1 + }, + "visibility": { + "context": true, + "project": true, + "worktrees": true, + "mcps": true + } + } +} diff --git a/code-review.md b/code-review.md new file mode 100644 index 0000000..fa06ccf --- /dev/null +++ b/code-review.md @@ -0,0 +1,288 @@ +# Code Review — whisper-mcp + +**Escopo:** `C:\Users\Evandro\.pi\agent\mcp-servers\whisper\` +**Arquivos:** `server.py` (1127), `parakeet_bootstrap.py` (332), `tests/test_parakeet_bootstrap.py` (237), `cli/whisper` (113) +**Lint/Build:** `py_compile` OK · `pytest 26 passed` · sem `git` no diretório + +--- + +## CRITICAL + +### C1 · SSRF bypass via HTTP redirect — `server.py:832` + +```python +with urllib.request.urlopen(url, timeout=TIMEOUT) as resp: +``` + +`urllib.request.urlopen` segue `30x` por padrão. Atacante serve URL pública que retorna `302 → http://127.0.0.1:8000/...` e a checagem `_is_private_host` (linha 382) é contornada. `_validate_url` só olha a URL inicial, não a cadeia de redirects. +**Fix:** montar `OpenerDirector` com `HTTPRedirectHandler` que revalida cada destino (ou `no-redirect`), ou extrair IP canônico antes de cada hop. + +### C2 · SSRF via DNS rebinding (TOCTOU) — `server.py:333-388`, `:832` + +`_is_private_host` resolve o host **uma vez** (linha 354) e libera. Entre o check e `urlopen`, o atacante controla TTL baixo e devolve IP público no primeiro resolve e `127.0.0.1` no segundo (dentro do mesmo `urlopen`). +**Fix:** resolver o IP antes do request, **abrir socket manualmente** conectando ao IP literal e usar `Host:` header — ou usar `requests` + `requests-ossrf` / `httpx` com hook de validação por hop. Mínimo: revalidar o IP em cada redirect. + +### C3 · `sh -c` com pipe para URL externa, sem verificação — `parakeet_bootstrap.py:124` + +```python +_run(["sh", "-c", "curl -fsSL https://get.docker.com | sh"], timeout=600) +``` + +- Não usa `shell=False` (mesmo com lista, passa pelo `sh -c`). +- Não valida TLS pinning / assinatura / checksum. +- Executa como root via `sudo systemctl start docker` (linha 127). +- Se o DNS do `get.docker.com` for sequestrado → RCE. + **Fix:** baixar o script primeiro para arquivo, comparar SHA-256 contra valor conhecido, só então executar. Ou usar pacotes nativos do gerenciador de pacotes da distro. + +### C4 · Dead code corrompe arquivo do usuário — `server.py:996` + +```python +out.write_bytes(pcm) # raw PCM, will be wrapped below +# Write proper WAV +wav_path = _write_wav_to(pcm, sample_rate, out) +``` + +A primeira escrita é PCD cru (não-WAV); a segunda sobrescreve com WAV válido — porém: + +- Se `_write_wav_to` falhar, o arquivo fica como PCD cru no disco do usuário (corrompido). +- Janela de race em que terceiros leriam o arquivo. + **Fix:** remover a linha 996 inteira. `_write_wav_to` já cria o arquivo. + +### C5 · FD leak em `tempfile.mkstemp` — `server.py:499`, `:932` + +```python +out_path = Path(tempfile.mkstemp(suffix=".wav", prefix="whisper-vad-")[1]) +path = Path(tempfile.mkstemp(suffix=".wav", prefix="whisper-rec-")[1]) +``` + +`mkstemp` retorna `(fd, path)`. O `[1]` descarta o `fd`, que vaza (até o GC fechar). Em loops de gravação contínua pode esgotar FDs. +**Fix:** usar `tempfile.NamedTemporaryFile(delete=False, suffix=".wav")` ou `os.close(fd)` antes de descartar. + +--- + +## IMPORTANT + +### I1 · Cobertura de testes = 0% em `server.py` (1127 linhas) + +Os 26 testes cobrem **apenas** `parakeet_bootstrap.py`. Nenhum teste para: `health`, `transcribe_file`, `transcribe_url`, `record_audio`, `_vad_trim_wav`, `_is_private_host`, `_validate_url`, `LRUCache`, `Metrics`, `WhisperError`, `_post_multipart`. O caminho de maior valor (transcrição + SSRF + cache) está intocado. +**Fix:** mínimo — testes para `_is_private_host` (loopback, IPv6 mapped, IPv4-mapped IPv6, getaddrinfo com múltiplos IPs), `_validate_url` (file://, ftp://, redirect), `_cache_key`, `LRUCache.put` (eviction), `Metrics.snapshot`. Testes de integração marcados como `slow` e gated. + +### I2 · `transcribe_url` não usa cache — `server.py:807-879` + +O docstring do módulo (linha 21-24) promete "cache LRU local ... evita re-transcrever mesmo áudio". URL baixada é re-transcrita toda vez — `cache_hits`/`cache_misses` não são chamados no path de URL. Comportamento documentado ≠ comportamento real. +**Fix:** hashear `data_bytes` (ou do `tmp_path`) e gravar entrada de cache no fim do path de URL, com `source`=URL. + +### I3 · Default port documentado diverge do código — `server.py:35` vs `:92` + +```python +# docstring linha 35: +WHISPER_BASE_URL (default ``http://localhost:8000``) +# código linha 92: +BASE_URL = os.environ.get("WHISPER_BASE_URL", "http://localhost:5092").rstrip("/") +``` + +8000 (Whisper) vs 5092 (Parakeet) — duas fontes da verdade dessincronizadas. README (a refazer) também pode estar errado. +**Fix:** padronizar; de preferência carregar o default de um único constante e fazer o docstring apontar para ele. + +### I4 · `logging.basicConfig` no import do módulo — `server.py:83` + +```python +logging.basicConfig(level=os.environ.get("WHISPER_LOG_LEVEL", "INFO"), ...) +``` + +Quem importa `server` (incluindo `cli/whisper`, testes, ferramentas) tem o root logger reconfigurado. O `basicConfig` do chamador (ex.: `parakeet_bootstrap.main` linha 312) é silenciosamente ignorado. Anti-pattern bem conhecido. +**Fix:** remover `basicConfig` daqui; deixar o chamador (entry point) configurar, ou usar `logger.setLevel()` local. + +### I5 · Health infla métrica `requests_success` antes de checar — `server.py:635` + +```python +def health() -> dict[str, Any]: + METRICS.record_request("health", success=True) # otimismo prematuro + payload = {} + for path in ("/v1/models", "/health"): + try: ...; except: continue + if not payload: + METRICS.record_request("health", success=False) # conta 2 requests! +``` + +Incrementa `requests_total` duas vezes (uma com `success=True` e outra com `False`) em falha. Comparado a `transcribe_file` que faz o oposto. Inconsistência. +**Fix:** não chamar `record_request` na entrada; decidir uma vez no fim. + +### I6 · Duplicação `_write_wav` / `_write_wav_to` — `server.py:930` e `:1033` + +Quase idênticos; diferença real é só o destino do `wave.open`. `_write_wav_to` ainda hardcoda `nchannels=1` (linha 1037) enquanto `_write_wav` aceita o param (linha 935) — `record_audio` não propaga `channels`. +**Fix:** uma única função `_write_wav(pcm, sr, channels, dest: Path | None = None)`. + +### I7 · Three `except Exception` sem narrowing — `server.py:764`, `:872`, `:984` + +Engole `KeyboardInterrupt`, `SystemExit`, `MemoryError`. `# noqa: BLE001` é aceito mas mascara bugs reais. Em `record_audio` linha 984, o broad catch esconde erros de configuração do PyAudio. +**Fix:** capturar `OSError`, `wave.Error`, `pyaudio.PyAudioError` (após import) e deixar `KeyboardInterrupt` subir. + +### I8 · `sudo systemctl ...` silencioso — `parakeet_bootstrap.py:127-128` + +```python +_run(["sudo", "systemctl", "start", "docker"], check=False, timeout=30) +_run(["sudo", "enable", "docker"], check=False, timeout=30) +``` + +Se sudo pedir senha e o usuário não conseguir, falha silenciosa — próximo `_docker_daemon_running()` lança `RuntimeError` sem contexto. Idem para `winget install` (linha 107) com timeout 600s sem feedback. +**Fix:** checar `returncode` e emitir mensagem acionável (ex.: "senha sudo necessária; rode manualmente"). + +### I9 · `_is_private_host` usa só primeira resposta do `getaddrinfo` indiretamente — `server.py:354` + +```python +infos = socket.getaddrinfo(host, None) +for info in infos: + ip_str = info[4][0] +``` + +Percorre todos os IPs do `getaddrinfo` (OK), mas se a chamada for lenta ela acontece em todo `transcribe_url`. Pode cachear por `host` (com TTL curto) — performance, não correção. +**Fix:** opcional, `@functools.lru_cache(maxsize=1024)` com TTL via `time.monotonic`. Apenas nice-to-have. + +### I10 · `parakeet_bootstrap` mistura `Optional`/`List` com PEP 604 — `parakeet_bootstrap.py:20` + +`from __future__ import annotations` está presente, então `Optional[str]` é coerente com `str | None` em server.py. Não é bug, mas inconsistência estilística. +**Fix:** decidir um estilo. `list[str]`, `str | None` é o padrão moderno. + +### I11 · `cache_key` lê o arquivo inteiro do disco para cada request — `server.py:624` + +```python +def _cache_key(...): data = file_path.read_bytes() +``` + +Para um WAV de 25 MB a cada chamada, mesmo se o cache for hit. Aceitável para hit (mas é miss porque hit é verificado **antes** — então ok), porém no caminho de miss é relido e ainda reenviado em `_post_multipart` (linha 538). O `_post_multipart` poderia receber `data` cacheado em memória em vez de re-ler do disco. +**Fix:** opcional — passar bytes em vez de path. + +### I12 · TOCTOU em `transcribe_file` — `server.py:700-716` + +```python +file_path = Path(path).expanduser().resolve() +if not file_path.is_file(): ... +# mais tarde: +data = file_path.read_bytes() +``` + +Janela de race entre `is_file()` e `read_bytes()`. Impacto baixo (input do operador), mas é padrão. Também: `Path.resolve()` em Windows pode quebrar em symlinks de rede. +**Fix:** opcional; abrir e validar com `try: open(...)` direto. + +### I13 · Limite hardcoded de 600s em `_record_audio_pyaudio` — `server.py:901` + +Magic number. `record_audio` (linha 949) aceita `seconds` mas passa a um validador de teto de 600. Inconsistente com o resto do módulo que prefere env-vars. +**Fix:** `WHISPER_RECORD_MAX_SECONDS` env var. + +### I14 · Magic numbers no VAD — `server.py:428, :454, :472, :482` + +`0.030` (janela 30ms), `32767.0` (peak int16), `30` (ms hardcoded para janela) aparecem em vários lugares. O valor `30` é duplicado nas linhas 472, 475, 482 (são ms-por-janela). Refatorar para `WINDOW_MS = 30` constante. +**Fix:** extrair constantes. + +### I15 · Subprocess `check=True` em comandos idempotentes — `parakeet_bootstrap.py:97, :166, :187, :195` + +Para `docker inspect` (existência), falha com `CalledProcessError` é informação, não erro. O código captura certo, mas usar `subprocess.CompletedProcess` com `check=False` e checar `returncode` é mais barato e idiomático. +**Fix:** cosmético. + +--- + +## MINOR + +### M1 · Comentários-mortos imports — `parakeet_bootstrap.py:12, :19` + +```python +import logging +# (os removido) +import platform +# (Path removido) +``` + +**Fix:** deletar. + +### M2 · `pass # OK` — `parakeet_bootstrap.py:273` + +```python +if _is_windows() or _linux_distro() is not None or platform.system() == "Linux": + pass # OK +``` + +Lógica tautológica: as três condições equivalem a `platform.system() in {"Windows","Linux"}` — a chamada de `_linux_distro()` é gratuita e a última condição é estritamente mais ampla. Refatorar. + +### M3 · `_emit` ramifica `if cb: ... if not cb: ...` — `parakeet_bootstrap.py:46-54` + +Pode ser linear: `if cb: try call; else: log`. Reduz duas branches. + +### M4 · `LRUCache.put` log "cache evicted" não remove arquivo — `server.py:306` + +Atualmente só remove do índice; se evoluir para cachear áudio no disco, o log vira mentira. Documentar ou remover até a evolução. + +### M5 · `_validate_url` retorna `dict | None` — `server.py:375` + +Caller precisa de `if err := _validate_url(...)`. Mais idiomático levantar `WhisperError` ou usar `Result`. Cosmético. + +### M6 · `record_audio` sem `transcribe_file`-style error handler unificado — `server.py:948-1030` + +Três tools com três padrões diferentes de captura de métrica + error wrapping (linhas 697, 814, 969). Considerar decorator `@track_metrics("tool_name")` que envolve a chamada. +**Fix:** refactor — único `with metrics.track("tool"):` helper. + +### M7 · `health` expõe `default_model` e `base_url` mesmo em falha — `server.py:648-654` + +Em ok=False não há razão para esconder, mas considere suprimir `models: []` vazio (linha 671) — fica confuso. + +### M8 · `clamp` ausente em `sample_rate` — `server.py:954, :980` + +`record_audio(sample_rate=0)` passa direto para `pyaudio.open(rate=0)` que retorna erro não-amigável. Validar `sample_rate > 0` e ≤ 48000. + +### M9 · `cli/whisper` sem shebang funcional cross-platform — `cli/whisper:1` + +`#!/usr/bin/env python3` não funciona com o launcher `.exe`/`.bat` em Windows. Já que o README vai ser refeito, mencionar uso via `python cli/whisper`. + +### M10 · Teste `test_linux_distro_*` mocka `builtins.open` globalmente — `tests/test_parakeet_bootstrap.py:38-51` + +Em Python 3.12+ `pathlib.Path` usa `io.open` que pode não ser capturado por `builtins.open` dependendo do monkeypatch. Hoje funciona, mas é frágil. Use `mock.patch("builtins.open", ...)` com `create=True` ou `mock.patch.object(pb, "open", ...)`. + +### M11 · Falta teste de `_wait_healthy` (timeout) — `tests/test_parakeet_bootstrap.py` + +A função existe (linha 224) mas não tem teste para o caso de timeout — exatamente o cenário de maior risco de produção. + +### M12 · Cache não tem método `close()` / cleanup de temp — `server.py:254` + +Se processo for SIGKILL, `index.json` pode ficar corrompido. Adicionar `try/except` no write (já tem) e validar JSON no load (já tem). OK, mas considerar `fcntl.flock` no índice para multi-processo. + +### M13 · Sem `__all__` em nenhum módulo + +Não é bug, mas o módulo `server` exporta ~15 símbolos públicos. Adicionar `__all__` ajuda tooling e leitores. + +### M14 · Tipos: `dict` sem parametrização — `server.py:636`, `:213` + +`payload: dict = {}` em vez de `dict[str, Any]`. Idem `result: dict = {}`. `snapshot() -> dict[str, Any]` está ok. Cosmético. + +### M15 · `_emit` engole exception do callback sem logar — `parakeet_bootstrap.py:50` + +```python +except Exception: # pragma: no cover + pass +``` + +Pelo menos um `logger.debug` aqui para diagnóstico. Está silencioso. + +--- + +## Veredito + +**NÃO pronto para próxima fase** sem corrigir C1–C5 (1-2 horas de trabalho total). A base de código é sólida e idiomática, mas tem um punhado de buratos críticos de segurança (SSRF redirect + DNS rebinding, RCE via curl|sh) e um bug de corrupção de arquivo que pode perder dados do usuário. + +**Prioridades de correção:** + +1. **C1 + C2** (SSRF) — mesma PR; o `_validate_url` precisa virar um guard revalidado por hop. +2. **C3** (curl|sh) — substituir por download → checksum → exec, ou apenas instruções manuais. +3. **C4** (dead write) — one-liner, remover linha 996. +4. **C5** (FD leak) — `os.close(fd)` ou trocar API. +5. **I1** (testes do server) — bloquear próxima fase sem pelo menos testes de `_is_private_host` e `_post_multipart` mockado. + +**Não-bloqueantes (I2–I15):** podem entrar como follow-ups em PRs separados, idealmente junto com I6 (DRY) e I5 (métricas). + +**Boas práticas que vale destacar:** + +- Thread-safety em `Metrics` e `LRUCache` está correta (lock por seção crítica). +- Retry com backoff exponencial no 5xx (linha 590). +- `TemporaryDirectory` com cleanup automático em `transcribe_url` (linha 829). +- Idempotência real do `parakeet_bootstrap` (testada e correta). +- Tipagem progressiva (`from __future__ import annotations`, `dict[str, Any]`). +- Tratamento de `urllib.error` separado para `HTTPError` vs `URLError`. diff --git a/scripts/debug_mcp.py b/scripts/debug_mcp.py new file mode 100644 index 0000000..ea4568a --- /dev/null +++ b/scripts/debug_mcp.py @@ -0,0 +1,42 @@ +"""Debug mcp.json by counting braces""" +import sys + +with open('C:/Users/Evandro/.pi/agent/mcp.json', 'rb') as f: + raw = f.read().decode('utf-8') + +# Count braces not in strings +depth = 0 +in_string = False +escape = False +for i, c in enumerate(raw): + if escape: + escape = False + continue + if c == '\\': + escape = True + continue + if c == '"': + in_string = not in_string + continue + if in_string: + continue + if c == '{': + depth += 1 + elif c == '}': + depth -= 1 + +print(f'Final depth: {depth}') +# Find chrome-devtools and check if its block is properly closed +idx_cd = raw.find('"chrome-devtools"') +print(f'chrome-devtools at: {idx_cd}') +# Show the block from chrome-devtools +end = raw.find('"id": "speaches"', 0) +if end == -1: + end = len(raw) +# Find where the whisper block ENDS +idx_w = raw.find('"whisper"') +print(f'whisper at: {idx_w}') +# Show lines around line 140 +lines = raw.split('\n') +for i in range(max(0, 135), min(len(lines), len(lines))): + print(f'{i+1}: {lines[i]}') diff --git a/scripts/fix_mcp_json.py b/scripts/fix_mcp_json.py new file mode 100644 index 0000000..1c31343 --- /dev/null +++ b/scripts/fix_mcp_json.py @@ -0,0 +1,64 @@ +"""Just rewrite the mcp.json cleanly with the new whisper path""" +import json +import os +import sys + +path = 'C:/Users/Evandro/.pi/agent/mcp.json' +with open(path, 'r', encoding='utf-8') as f: + raw = f.read() + +# Find the whisper block +start_marker = '"whisper": {' +start_idx = raw.find(start_marker) +if start_idx == -1: + print("Not found") + sys.exit(1) + +# Find the matching closing brace +depth = 0 +i = start_idx + len(start_marker) - 1 +in_string = False +escape = False +while i < len(raw): + c = raw[i] + if escape: + escape = False + i += 1 + continue + if c == chr(92): # backslash + escape = True + i += 1 + continue + if c == '"': + in_string = not in_string + i += 1 + continue + if in_string: + i += 1 + continue + if c == '{': + depth += 1 + elif c == '}': + depth -= 1 + if depth == 0: + end_idx = i + 1 + break + i += 1 +else: + print("No matching close brace found") + sys.exit(1) + +print(f"start={start_idx}, end={end_idx}") +new_whisper = '"whisper": {\n "command": "C:\\\\Users\\\\Evandro\\\\AppData\\\\Local\\\\Programs\\\\Python\\\\Python312\\\\python.exe",\n "args": [\n "D:/Documents/GitHub/whisper-mcp/server.py"\n ],\n "description": "Whisper transcription via local Speaches/Whisper (faster-whisper-large-v3-turbo). v2.0 with SSRF guard, size limit, retry, structured errors.",\n "timeout": 180,\n "lifecycle": "lazy",\n "idleTimeout": 10,\n "directTools": true,\n "environment": {\n "WHISPER_BASE_URL": "http://localhost:8000",\n "WHISPER_MODEL": "deepdml/faster-whisper-large-v3-turbo-ct2",\n "WHISPER_TIMEOUT": "120",\n "WHISPER_MAX_BYTES": "26214400"\n }\n }' +new_raw = raw[:start_idx] + new_whisper + raw[end_idx:] +try: + parsed = json.loads(new_raw) + print("JSON valid") + with open(path, 'w', encoding='utf-8') as f: + f.write(new_raw) + print("Saved") +except json.JSONDecodeError as e: + print(f"Still invalid: {e}") + start = max(0, e.pos - 100) + end = min(len(new_raw), e.pos + 100) + print(f"Context: {new_raw[start:end]!r}")