Remediação de segurança — 8 findings do scan v3.8.51 - #3
Merged
Merged
Conversation
#1) O endpoint de teste de webhook validava apenas a string do hostname inicial, usava `fetch` (segue redirect por padrão) e decidia a redação pelo hostname inicial — então um destino público podia redirecionar (302) para um serviço interno e vazar o corpo, e um hostname público que resolve para IP privado/metadata era conectado. Correção (novo `src/shared/network/hardenedWebhookFetch.ts`): - resolve A/AAAA e valida TODOS os IPs resolvidos (metadata/link-local sempre bloqueado; privado só sob opt-in) — classificação pelo IP resolvido, fechando o DNS rebinding; - fixa o IP validado via undici Agent connect.lookup (anti-TOCTOU); Host/SNI preservados; - nunca segue redirect (redirect: "manual"; 3xx vira diagnóstico bloqueado, sem corpo); - nunca devolve o corpo de destino privado; - timer manual + clearTimeout + agent.close() (sem handle/socket órfão). A rota passa allowPrivate=arePrivateProviderUrlsAllowed(), preservando o local-first. Testes: tests/unit/api/webhooks/webhook-test-ssrf-rebinding.test.ts (rebinding→metadata/ privado, literais, redirect nunca seguido com prova de request único, corpo privado retido). Verificação: node --test 18/18 (com a suíte SSRF existente, sem regressão); tsc 0 erros nos arquivos; eslint (com suppressions) exit 0. Evidência: docs/evidence/remediation/01-ssrf-webhook-test.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
O proxy /api/openapi/try aceitava qualquer /api/, todos os métodos mutáveis, fazia self-fetch same-origin e encaminhava o cookie de sessão implicitamente — então alcançava rotas host-sensitive sob a localidade/identidade do próprio servidor (loopback). Correção: - bloqueia destinos LOCAL_ONLY e ALWAYS_PROTECTED (isLocalOnlyPath/isAlwaysProtectedPath) em todos os métodos -> 403; - métodos mutáveis só nas superfícies de inferência/agent (/v1, /v1beta, /a2a, /.well-known); o /api/ de gestão é GET/HEAD por este proxy -> 405; - remove o encaminhamento implícito do cookie de sessão (credenciais só explícitas). Testes: tests/unit/api/openapi-try-confused-deputy.test.ts (LOCAL_ONLY/ALWAYS_PROTECTED 403, método mutável no /api/ 405 vs /v1 liberado, e prova via echo server de que o cookie não é encaminhado). Verificação: node --test 6/6; tsc 0 erros; eslint exit 0. Evidência: docs/evidence/remediation/05-openapi-try-confused-deputy.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
evaluateToolScopes já negava ferramenta sem definição e escopo ausente QUANDO enforcing, mas server.ts só ligava enforcement com OMNIROUTE_MCP_ENFORCE_SCOPES === "true" (default OFF): com a env ausente, uma chave mcp:connect (só transporte) alcançava qualquer ferramenta write:*. A doc canônica (ENVIRONMENT.md) já dizia `true`, contradizendo o código. Correção: - novo helper isMcpScopeEnforcementEnabled() (default ON; opt-out explícito false/0/no/off); - server.ts passa a usar o helper; - MCP-SERVER.md corrigido para `true` (default on), alinhando com ENVIRONMENT.md. Testes: tests/unit/mcp-scope-enforcement-default.test.ts (default ON, opt-out, mcp:connect negado para write:combos, tool_definition_missing, escopo válido, wildcard). Regressão: 70/70 nos testes MCP (mcp-connect-scope, notion, obsidian, pool, etc.) — sem quebra pelo flip. tsc 0 erros; eslint exit 0. Evidência: docs/evidence/remediation/04-mcp-scopes-default-on.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
webhooks.secret (chave HMAC whsec_) era gravada em texto puro e usada direto como chave HMAC — uma leitura do banco/backup permitia forjar eventos assinados. Correção: - createWebhook/updateWebhook cifram o segredo (AES-256-GCM enc:v1:) antes de persistir; - rowToWebhook decifra na leitura, então dispatcher e rota de teste seguem usando o plaintext para o HMAC — banco/backup passam a conter só ciphertext; chave ausente => secret=null => entrega sem assinatura (fail-safe, nunca com chave errada); - encryptExistingWebhookSecrets(): backfill idempotente e transacional das linhas legadas em plaintext, ligado no init do DB (core.ts) ao lado da migração de cifra legada. Testes: tests/unit/db-webhook-secret-encryption.test.ts (ciphertext no repouso, round-trip do leitor, backfill migra 1 e é idempotente, assinatura HMAC idêntica antes/depois). Regressão: 46/46 na suíte de webhook (dispatcher/delivery/ssrf/cli). tsc 0 erros; eslint exit 0. O "falhar sem chave" em perfil exposto é do finding #3 (encryptOrThrow), que generaliza a política; aqui a cifra é aplicada sempre que há STORAGE_ENCRYPTION_KEY (cenário de produção). Evidência: docs/evidence/remediation/08-webhook-secret-encryption.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…finding #2) O gate Tier-1 só rejeitava LOCAL_ONLY para peers que NÃO fossem loopback E NÃO fossem LAN privada. Um dispositivo de LAN pulava o gate inteiro e caía no fallback anônimo (requireLogin=false), alcançando rotas de instalar pacote/spawn/config do host sem credencial. Correção: removida a exceção !isPrivateLanRequest — todo chamador não-loopback (LAN incluída) passa pelo gate. O carve-out autenticado (manage/admin/mcp:connect ou sessão de dashboard) segue sendo o único caminho não-loopback para o subset bypassável (TRUSTED_LAN autenticado); rotas host-sensitive/spawn (/api/cli-tools/runtime/*) ficam estritamente loopback, mesmo com auth. Anti-spoofing preservado (localidade vem do peer real do socket, não do header host). Testes (peer LAN 192.168.1.50): spawn+requireLogin=false → 403; /api/mcp sem auth → 403; /api/mcp + manage key → allow; spawn + manage key → 403. 22/22 no arquivo (também corrige um EPERM de teardown pré-existente no Windows fechando o DB no after()); 93/93 de regressão authz. tsc 0 erros; eslint exit 0. Evidência: docs/evidence/remediation/02-local-only-loopback-only.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(finding #3) encrypt() faz passthrough (retorna plaintext) sem STORAGE_ENCRYPTION_KEY, e os writers checavam `if (!encrypted) throw` — que não pega o plaintext (não-vazio), gravando o segredo em texto puro achando que cifrou. Correção (encryption.ts): - encryptOrThrow(): ciphertext real enc:v1: OU lança EncryptionUnavailableError — nunca passthrough; - isStorageEncryptionRequired(): fail-closed em produção/exposto (NODE_ENV=production) ou opt-in explícito (OMNIROUTE_REQUIRE_STORAGE_ENCRYPTION); dev/test = passthrough; - encryptSensitive(): contrato dos writers (encryptOrThrow no perfil exigido, senão encrypt); - assertStorageEncryptionConfigured(): gate de startup ligado no init do DB (core.ts) antes de setDb — instância exposta sem chave recusa iniciar. No-op em dev/test. Writers convertidos: saveCloudAgentCredential, markCommandCodeAuthSessionReceived, services/apiKey. Testes: tests/unit/db-encrypt-or-throw.test.ts (lança sem chave; perfil exigido => encryptSensitive e gate lançam; dev => passthrough; cifra com chave). Regressão: 13/13 (webhook-secret + ssrf + init com o gate) e 9/9 (cloud-agent-credentials, db-command-code-auth, migration-071). tsc 0 erros; eslint exit 0. Follow-up honesto: demais writers passthrough (obsidian, radar, settings oidc, logExport, webhookDispatcher metadata, secrets.ts) devem adotar encryptSensitive; readiness-scan de colunas sensíveis idem. O gate de startup + writers de credencial de maior risco já fecham o caminho. Evidência: docs/evidence/remediation/03-encrypt-or-throw-fail-closed.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…h (finding #7) A coluna api_keys.key guardava a bearer em texto puro ao lado do key_hash — uma leitura do banco/backup entregava chaves ativas usáveis, apesar da validação já aceitar o hash. Abordagem (Opção B endurecida, não hash-only puro): cifrar no repouso + validar só por hash, preservando o reuso interno (pickApiKeyForInternalUse) sem redesenho da auto-auth nem rotação forçada. Opção A (hash-only + sem coluna key) fica como hardening futuro documentado. Correção: - validação hash-only: _stmtValidateKey/_stmtGetKeyMetadata -> WHERE key_hash = ? (removido OR key); call sites passam só o hash; - createApiKey/regenerateApiKey gravam encryptSensitive(key) (fail-closed em prod via #3); retorno mantém o plaintext (revelação única); - getApiKeys/getApiKeyById decifram key na leitura (reuso interno/máscara seguem com plaintext; chave ausente => null, nunca ciphertext exposto); - encryptExistingApiKeyPlaintext(): backfill idempotente/transacional das linhas legadas, ligado no init do DB. Hash inalterado => a chave segue validando. Testes: tests/unit/db-apikey-encryption-at-rest.test.ts (ciphertext no repouso, validação hash-only, decrypt na leitura, backfill idempotente, regenerate rotaciona). Regressão: 197 subtestes de lógica passam na suíte apiKeys/auth; os 2 arquivos "vermelhos" (lifecycle/regeneration) falham só no EPERM de teardown do Windows — idêntico no baseline (ambiental, passa no CI/Linux). tsc 0 erros; eslint 0. Evidência: docs/evidence/remediation/07-apikey-encryption-at-rest.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…is (finding #6) No modo Remote Server, a mesma janela (com preload privilegiado) carrega URL HTTP(S) arbitrária, e login:start devolvia result.credentials ao renderer — uma página remota chamava startLogin e recebia tokens/cookies de provedores. Correção verificável: - novo electron/lib/ipcOriginGuard.js (puro, testável): isPrivilegedSenderAllowed nega origem de rede http(s) NÃO-loopback (permite loopback/file/app/about — conteúdo local); isLoopbackHostname; isCrossOriginNavigation; - main.js login:start: rejeita sender não-local, valida providerId, e NUNCA retorna credentials (persistidas só no processo principal; retorno sanitizado com credentialsPersisted:boolean). Testes: tests/unit/electron-ipc-origin-guard.test.ts (guards puros + estáticos no main.js). 7/7; regressão electron-remote-server 25/25; node --check main.js OK; eslint exit 0. BLOCKED_BY_EXTERNAL (gate "testes Electron", precisa de build+runtime): janela/preload separados para o modo remoto, bloqueio de navegação (helper pronto), HTTPS obrigatório p/ não-loopback, sandbox:true, validação de sender.id. Documentado em docs/evidence/remediation/06-electron-remote-ipc.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
…tSensitive Completa o finding #3 convertendo os writers restantes que usavam encrypt() passthrough para encryptSensitive (fail-closed em perfil exposto/produção; passthrough em dev): - db/obsidian.ts (token + password), db/radar.ts (key), db/settings.ts (oidcClientSecret), logExport/secrets.ts, webhookDispatcher.ts (metadata). - db/secrets.ts: persistSecret/getPersistedSecret passam a cifrar/decifrar. ACHADO EXTRA — este store é usado pelo login:start do Electron para salvar credenciais de provedor e gravava em TEXTO PURO; agora cifra no repouso (legado plaintext segue lido via passthrough do decrypt). Testes: tests/unit/db-secrets-encryption.test.ts (ciphertext no repouso, round-trip, legado). Regressão: 207/207 nos testes unitários dos writers (db-secrets, db-settings, obsidian, log-export, cli-radar). tsc 0 erros nos arquivos; eslint exit 0. Evidência atualizada: docs/evidence/remediation/03-encrypt-or-throw-fail-closed.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adiciona scripts/smoke/authenticated-smoke.mjs (Node ESM puro, sem deps) para o smoke
autenticado do gate: GET /v1/models (auth + auth negativa, grátis) e POST /v1/messages e
/v1/responses (geração real, PAGOS — só com OMNIROUTE_SMOKE_ALLOW_PAID=1; senão SKIP).
Seguro por padrão: nenhuma chamada paga sem opt-in, nenhuma credencial é impressa, reachability
tratada graciosamente. Paths /v1/* confirmados pelo rewrite do next.config (/v1/:path* ->
/api/v1/:path*) e pelas rotas reais src/app/api/v1/{models,messages,responses}.
Execução real fica com o operador (instância de pé + credencial de teste) — BLOCKED_BY_EXTERNAL.
node --check OK; run sem servidor reporta reachability FAIL sem crash.
Evidência/uso: docs/evidence/remediation/09-authenticated-smoke-harness.md
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regressões introduzidas por esta remediação (agora corrigidas): - #4 (MCP scopes default-ON): os testes FUNCIONAIS de ferramentas MCP não fornecem escopos e passaram a ser negados. Desliga o enforcement no harness desses testes (não são testes do gate): env OMNIROUTE_MCP_ENFORCE_SCOPES=false em vitest.mcp.config.ts e na fixture mcp-public-error-boundaries. O comportamento default-ON segue coberto por tests/unit/mcp-scope-enforcement-default.test.ts (valores explícitos). - #6 (Electron): electron/lib/ipcOriginGuard.js agora está em electron/package.json build.files (senão o app empacotado crasharia com "Cannot find module"). - #3/smoke: novas envs (OMNIROUTE_REQUIRE_STORAGE_ENCRYPTION, OMNIROUTE_SMOKE_*) documentadas em .env.example e docs/reference/ENVIRONMENT.md (env-doc-sync bidirecional). - #5 (OpenAPI Try): a versão inicial era agressiva demais — bloquear método mutável no /api/ e remover o cookie de sessão quebrava a feature legítima (admin autenticado testando POST /api/*). Mantido apenas o núcleo do fix: bloquear destinos LOCAL_ONLY/ALWAYS_PROTECTED (fecha o confused-deputy). Métodos mutáveis e o cookie do próprio admin voltam a ser permitidos. Teste de confused-deputy ajustado; o teste existente openapi-try-route volta a passar. Verificação: 64/64 nas suítes tocadas (mcp-public-error-boundaries, mcp-scope-enforcement-default, openapi-try-route + confused-deputy, electron-packaging/main/ipc-origin-guard, 7793 env-doc, db-secrets); vitest MCP 54/54; tsc/eslint limpos nos arquivos. NOTA: os gates ainda vermelhos no PR (glm.ts TS2554 no api-typecheck, stream-handler redaction, contagem de migrations nos docs, fragmento changelog.d malformado) são PRÉ-EXISTENTES na base release/v3.8.51 — reproduzidos num worktree limpo de b345c7f SEM estas mudanças. Não são regressões desta remediação. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…emediation-v3.8.51
LMPrado-DZ23
pushed a commit
that referenced
this pull request
Sep 9, 2026
…ted evidence Records the final state after merging PR #4 (base hygiene + path sanitizer) and PR #3 (8 security findings) into release/v3.8.51 (31c6f44), with integrated validation evidence: 64/64 regression tests pass, open-sse typecheck clean, API-route baseline gate PASS (0 regressions), docs/changelog/env gates green. Honestly scopes out remaining work (authenticated smoke needs operator credential; Phase 2/3 and any release/publish need explicit authorization). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LMPrado-DZ23
pushed a commit
that referenced
this pull request
Sep 12, 2026
…nstead of committing secret-shaped literals The CI secret scan (check:secrets --ratchet, gitleaks generic-api-key) flagged the 64-hex STORAGE_ENCRYPTION_KEY literal the new check:standalone-boot gate passed to the production-profile boot. The gate still exercises readiness #3 (the profile refuses to boot without the three secrets) but now derives all three from crypto.randomBytes at run time, so nothing secret-shaped lives in the repository and the baseline of 0 findings holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Remediação de segurança — scan v3.8.51 (8 findings)
Corrige os 8 findings do scan de segurança (7 high + 1 medium), cada um por causa-raiz, com teste de regressão real,
tsc+eslintlimpos e commit isolado. Base: SHA exato do scanb345c7f.Vulnerabilidades corrigidas
hardenedWebhookFetch— resolve A/AAAA, valida todos os IPs resolvidos, fixa o IP (anti-rebinding via undici), não segue redirect, nunca devolve corpo privado7488e1cbf/v1, sem encaminhar cookie de sessão02f4c08famcp:connectnão alcançawrite:*)6eac2ed09007f6ab2a92e20e6d7encryptOrThrowfail-closed + gate de startup em perfil exposto (3 writers de credencial convertidos)4e2a14c9a99139d5c2login:start: rejeita sender remoto, validaproviderIde nunca retorna credentials ao renderer; guard de origem puro testávelabd765c19Evidência
tsc --noEmit(core + open-sse): 0 erros nos arquivos tocados.eslint(com suppressions): exit 0.node --check electron/main.js: OK.docs/evidence/remediation/0N-*.md+ baseline em00-baseline.md.Migrações (JS, idempotentes, no init do DB, não-destrutivas)
encryptExistingWebhookSecrets()(#8) eencryptExistingApiKeyPlaintext()(#7) — transacionais, no-op sem chave; a assinatura/validação são preservadas (hash inalterado). Plaintext recuperável viadecrypt()com a mesmaSTORAGE_ENCRYPTION_KEY.Pendências honestas (follow-up)
encrypt()passthrough (obsidian, radar, settings-oidc, logExport, webhookDispatcher-metadata, secrets.ts) devem adotarencryptSensitive; readiness-scan de colunas sensíveis.key) + rotear auto-auth interna pelo machine token (elimina o resíduo; exige rotação de chaves).sandbox:true,sender.id— exigem E2E Electron (build+runtime).Gate final (antes de release de produção)
Ainda dependem do operador/ambiente: smoke autenticado
/v1/messagese/v1/responses(precisa de credencial), E2E Electron do #6, e uma nova auditoria independente sem Critical/High.🤖 Generated with Claude Code