feat(security+ai): goal-oriented agent + hardening de segurança (audit 2026-04-17) - #37
Conversation
…adas em audit - SEC-001: sanitizar parâmetro `q` nas 4 rotas da Public API (contacts, deals, companies, boards) usando sanitizePostgrestValue() para prevenir PostgREST filter injection - SEC-002: sanitizar queries em lib/ai/tools.ts (stageName, search terms, contact query, effectiveStageName) — mesma vulnerabilidade nas ferramentas do AI - SEC-003: rate-limiter.ts reescrito para usar ai_conversation_log como source of truth em vez de Map em memória (correto para Vercel serverless multi-instance) - SEC-004: Z-API webhook — default-deny quando nenhum channelSecret configurado; usar UUID interno do insert (não ID externo do Z-API) para AI processing; detecção de duplicata via SQLSTATE 23505 (estável) em vez de string matching - SEC-005: Resend webhook — implementar verificação HMAC-SHA256 Svix completa com validação de timestamp (previne replay attacks de até 5 minutos) - SEC-006: Evolution webhook — adicionar audit logging em messaging_webhook_events com stable event IDs para deduplicação - SEC-007: agent.service.ts — input-filter.ts (sanitização de prompt injection), output-validator.ts (validação de output do LLM), structured-logger.ts (logs JSON) integrados ao pipeline de processamento Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ilidade Performance: - MessageInput: EmojiPicker migrado para next/dynamic (lazy) — remove ~1.2MB do bundle inicial - KanbanBoard/useBoardsController: drag handlers (handleDragStart/Over/Drop) e handleKeyDown memoizados com useCallback para evitar re-renders desnecessários - useContactsQuery/useDuplicateContactsQuery: invalidações pontuais substituem invalidateQueries(deals.all) que fazia refetch do Kanban inteiro - layout.tsx: removido 'use client' desnecessário (Script funciona em Server Components) - lib/query: refetchOnWindowFocus desabilitado globalmente (Realtime cobre as entidades) Acessibilidade (WCAG 2.2 A): - ContactFormModal: labels associados via htmlFor/id em todos os campos de formulário - DealDetailModal: aria-label em botões de ação (fechar, editar) sem texto visível - KanbanBoard: role="list" e role="listitem" na grade Kanban + aria-label descritivo Infraestrutura: - GitHub Actions: CI (lint+typecheck+tests em PRs) + build (main push) + release (tags semver) - .commitlintrc.json: conventional commits enforcement no CI - CHANGELOG.md + docs/release-engineering.md: processo de release documentado - package.json: scripts release:prepare, release:draft, release:tag Observabilidade: - app/api/health/route.ts: health check GET/HEAD (Supabase + AI provider) - docs/observability/: guia de monitoramento e implementação Banco de dados: - migration 20260409120000: pg_cron jobs para alertas HITL pendentes >24h - migration 20260409130000: tabela ai_pending_evaluations para decoupling de stage eval - vercel.json: cron /api/cron/stage-evaluations (cada minuto) + maxDuration 60s Testes: - test/briefingApi.test.ts: 12 testes para API de briefing - test/hitlApi.test.ts: testes para HITL resolve/list/count - test/messagingAiProcess.test.ts: 14 testes para pipeline de AI - test/publicApi.contacts.test.ts + publicApi.deals.test.ts: 39 testes de API pública Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tput-validator - Evolution webhook: comparação de API key migrada para HMAC-SHA256 timing-safe (previne timing oracle attack em `providedKey !== webhookSecret`) - eslint.config.mjs: removido plugin react-hooks duplicado (causava "Cannot redefine plugin" no CI); adicionados ignores para worktrees e lame.min.js; desabilitadas rules que geravam falsos positivos (refs, no-img-element, alt-text para ícones Lucide); removidos 5 eslint-disable comments stale - lint: zero errors, zero warnings (unblocks CI gate --max-warnings 0) - test/aiInputFilter.test.ts: 23 testes para sanitizeIncomingMessage (EN/PT-BR, multi-padrão, preservação de contexto) - test/aiOutputValidator.test.ts: 19 testes para validateAIOutput (empty, length limit, system prompt leakage, PII detection, estrutura do retorno) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sanitize: - sanitizePostgrestValue() agora preserva '.' — dentro de %value% num ilike o ponto é literal e não permite injeção; remover quebrava buscas por email (test/tools.multiTenant.test.ts agora passa) ESLint: - Adicionados .claude/worktrees/** e .dmux/worktrees/** ao ignore (evita linting de worktrees de agentes); adicionado public/lame.min.js - Desabilitadas rules que geravam falsos positivos: no-img-element (ícones Lucide chamados Image), jsx-a11y/alt-text (mesmo motivo), refs (padrão pré-existente) - Removidos eslint-disable comments stale após desabilitar regras globalmente - lint agora passa com zero errors e zero warnings Testes: - vitest.config.ts: excluídos .claude/** e .dmux/** (worktrees de agentes não são código fonte e duplicavam execução de testes) - lib/query/__tests__/cache-integrity.test.ts: snapshot atualizado após invalidações mudarem de queryKeys.deals.all para DEALS_VIEW_KEY pontual - app/api/ai/hitl/route.ts: usa new URL(request.url).searchParams em vez de request.nextUrl.searchParams para testabilidade fora do runtime Next.js - test/aiInputFilter.test.ts + test/aiOutputValidator.test.ts: 42 testes novos Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gurança ## Goal-Oriented Agent (feature/004) - BoardAIConfigModal e BoardAgentsSection: UI para configurar objetivos por board - BoardAIConfig API routes (generate-goal, generate-persona, board-config CRUD) - Migration 20260409140000_board_ai_config: tabela board_ai_configs - lib/ai/messaging/ e lib/ai/utils/: módulos de suporte ao agente - generate-prompts-schema.ts: clamp de arrays e fix AI SDK v6 (result.output) - Testes: goalOrientedAgent.test.ts, agentGoalStage.test.ts ## Audit Fixes (validação 2026-04-17) - SECURITY_PREAMBLE exportado de agent.service.ts (antes era const privado) - sanitizeIncomingMessage() adicionado em todos os entry points AI: actions/route.ts (chatWithCRM, chatWithBoardAgent, refineBoardWithAI, parseNaturalLanguageAction, rewriteMessageDraft, generateBoardStructure) generate-goal/route.ts, generate-prompts.service.ts - ai_conversation_log: logging adicionado em analyze/route.ts, generate-goal/route.ts, generate-prompts.service.ts e todos os casos de actions/route.ts via helper logAIAction() - ALLOWED_GOOGLE_MODELS whitelist em lib/ai/config.ts (fallback para default) - import 'server-only' em staticAdminClient.ts (service role key nunca ao client) - CLAUDE.md: atualizado com novos padrões críticos (AI SDK v6, Realtime, etc.) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds goal-oriented, board-scoped AI agent features with security hardening (sanitization, output validation, rate limits, circuit-breaker), RAG integration, structured observability, HITL queueing and cron processing, board AI CRUD + persona/goal generation APIs, DB migrations, webhook hardening/dedupe, CI/release automation, extensive tests and UI components for board agent configuration. Changes
Sequence Diagram(s)sequenceDiagram
actor User as Lead/User
participant Conv as Conversation
participant Agent as AI Agent Service
participant Rate as RateLimiter(DB)
participant CB as CircuitBreaker(DB)
participant RAG as FileSearch/RAG
participant LLM as Google GenAI
participant Validator as OutputValidator
participant Sender as MessageSender
participant Eval as HITL Queue (DB)
User->>Conv: inbound message
Conv->>Agent: processIncomingMessage(conversationId, text)
Agent->>Rate: checkConversationRateLimit(conversationId)
Rate-->>Agent: allowed/remaining
alt denied
Agent-->>Conv: return skipped (rate_limited)
else allowed
Agent->>CB: getCircuitBreakerState(conversationId)
CB-->>Agent: isOpen?
alt open
Agent-->>Conv: return skipped (circuit_open)
else closed
Agent->>Agent: sanitizeIncomingMessage(text)
alt agent_mode == observe
Agent->>Agent: logAIAction(observe)
Agent-->>Conv: return decision (observe)
else respond
alt knowledge_store configured
Agent->>RAG: generateWithFileSearch(...)
RAG->>LLM: generate (with files)
LLM-->>RAG: response
RAG-->>Agent: text
else
Agent->>LLM: generateWithFailover(...)
LLM-->>Agent: text
end
Agent->>Validator: validateAIOutput(text, context)
Validator-->>Agent: safe? / issues
alt safe
Agent->>Sender: sendAIResponse(text)
Sender-->>Conv: message sent
Agent->>Eval: enqueue ai_pending_evaluations (async)
Agent->>CB: resetCircuitBreaker(conversationId)
else unsafe
Agent->>Sender: sendAIResponse(fallback)
Sender-->>Conv: fallback sent
Agent->>CB: incrementCircuitBreakerError(conversationId)
end
Agent-->>Conv: return AgentDecision{latency_ms,...}
end
end
end
sequenceDiagram
participant Webhook as External Provider
participant Auth as TimingSafeAuth
participant Dedup as Dedup/Audit Table
participant Handler as EventHandler
Webhook->>Auth: verify signature (timing-safe)
Auth-->>Webhook: valid?
alt invalid
Webhook-->>Webhook: return 401
else valid
Auth->>Dedup: generateStableEventId(payload)
Dedup-->>Auth: external_event_id
Auth->>Dedup: INSERT processed=false (external_event_id)
Dedup-->>Auth: insert result / duplicate error
alt duplicate
Auth-->>Webhook: return 200 { duplicate: true }
else not duplicate
Auth->>Handler: process event
Handler-->>Dedup: UPDATE processed=true (+error if failed)
Handler-->>Webhook: return 200
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de56b632fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return json({ error: 'Unauthorized' }, 401); | ||
| } | ||
|
|
||
| const supabase = await createClient(); |
There was a problem hiding this comment.
This cron handler authenticates only with CRON_SECRET but creates a regular SSR client (createClient()), so Supabase requests run as anon when no user cookie is present. In this same commit, ai_pending_evaluations is protected by RLS and has no anon update policy, so the first .update({ status: 'processing' }) claim step will fail and the queue will never be drained, leaving stage advancement/HITL evaluations stuck in pending.
Useful? React with 👍 / 👎.
| organization_id: orgId, | ||
| ai_response: (result.text || '').slice(0, 1000), | ||
| tokens_used: result.usage?.totalTokens ?? 0, | ||
| model_used: modelId, | ||
| action_taken: action, |
There was a problem hiding this comment.
These inserts cannot succeed against the current ai_conversation_log schema/policies: the payload omits required fields like conversation_id, uses action_taken values outside the table check constraint, and runs via a user-scoped client (insert policy is service-role only). Because errors are only logged asynchronously, every call silently drops the intended audit record, so the new “log all AI actions” hardening is effectively non-functional.
Useful? React with 👍 / 👎.
| CREATE POLICY "board_ai_config: admin write" | ||
| ON board_ai_config FOR ALL | ||
| USING (organization_id = get_user_org_id()); |
There was a problem hiding this comment.
The write policy named "admin write" does not check admin role at all; it allows any org member matching organization_id = get_user_org_id() to perform FOR ALL operations directly through Supabase. That bypasses the API route’s profile.role === 'admin' guard and lets non-admin users change agent mode/persona/goals for the whole board.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/ai/tools.ts (1)
690-702:⚠️ Potential issue | 🟠 MajorInconsistent sanitization — several PostgREST interpolations still use raw user/model input.
The PR applies
sanitizePostgrestValue()inresolveStageIdForBoard,searchDeals,searchContacts, andlistDealsByStage, but the same injection vector is still open in several other tools that interpolate LLM-provided strings into.or()/.ilike()clauses:
- Line 695 —
moveDeal:`name.ilike.%${stageName}%,label.ilike.%${stageName}%`(rawstageName).- Line 761 —
createDeal:.ilike('name', contactName)(rawcontactName).- Line 876 —
markDealAsWon:`%${dealTitle}%`inilike('title', ...)(rawdealTitle).- Line 928 —
markDealAsWon:`name.ilike.%${wonStageNameFromContext}%,label.ilike.%${wonStageNameFromContext}%`(raw fromcontext.wonStage).A
,or)in any of those values can break out of the filter expression and inject new conditions (the exact vector the SEC fix is meant to close). Organization isolation still holds via the.eq('organization_id', …)guard, but within-tenant filter manipulation (e.g., widening a match, OR-ing unrelated predicates) is still possible.Proposed fix
- .or(`name.ilike.%${stageName}%,label.ilike.%${stageName}%`); + .or(`name.ilike.%${sanitizePostgrestValue(stageName)}%,label.ilike.%${sanitizePostgrestValue(stageName)}%`);- .ilike('name', contactName) + .ilike('name', sanitizePostgrestValue(contactName))- query = query.ilike('title', `%${dealTitle}%`); + query = query.ilike('title', `%${sanitizePostgrestValue(dealTitle)}%`);- .or(`name.ilike.%${wonStageNameFromContext}%,label.ilike.%${wonStageNameFromContext}%`) + .or(`name.ilike.%${sanitizePostgrestValue(wonStageNameFromContext)}%,label.ilike.%${sanitizePostgrestValue(wonStageNameFromContext)}%`)As per coding guidelines: "Use
sanitizePostgrestValue()andsanitizeUrl()fromlib/utils/sanitize.tsfor data sanitization".Also applies to: 760-778, 870-880, 920-934
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/tools.ts` around lines 690 - 702, Multiple PostgREST filter interpolations in moveDeal, createDeal, and markDealAsWon still interpolate raw LLM/user strings into .or() and .ilike() expressions; fix by passing those variables through sanitizePostgrestValue() (and sanitizeUrl() where URL fragments are used) before using them in template literals or .ilike() calls — e.g., sanitize stageName/ wonStageNameFromContext/contactName/dealTitle and then build the .or(`name.ilike.%${sanitized}%,label.ilike.%${sanitized}%`) or .ilike('field', sanitized) expressions so no unescaped commas/parentheses can break the PostgREST filter; update the usages in moveDeal, createDeal, and both markDealAsWon occurrences to call sanitizePostgrestValue() on the interpolated variables.supabase/functions/messaging-webhook-resend/index.ts (1)
322-325:⚠️ Potential issue | 🟡 MinorUse SQLSTATE
23505instead of substring-matching onerror.messagefor duplicate detection.
eventInsertErr?.message?.toLowerCase().includes("duplicate")is locale/version-sensitive and will silently stop working if PostgREST or Postgres ever tweaks the error text. PostgREST/@supabase/supabase-jsexposeserror.code— unique-violation is SQLSTATE23505. The PR description already states "webhook deduplication via SQLSTATE", so this call site should align.♻️ Proposed fix
- if (eventInsertErr?.message?.toLowerCase().includes("duplicate")) { + if (eventInsertErr?.code === "23505") { console.log(`[Webhook/Resend] Duplicate event ignored: ${externalEventId}`); return json(200, { ok: true, duplicate: true, event_id: externalEventId }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 322 - 325, Replace the fragile substring check on the DB error message with a SQLSTATE check: in the block handling eventInsertErr (the variable named eventInsertErr used alongside externalEventId and the json(...) return), change the condition to detect duplicate key violations by checking eventInsertErr.code === "23505" (SQLSTATE for unique_violation) instead of eventInsertErr?.message?.toLowerCase().includes("duplicate"); keep the same log and json response behavior when a 23505 error is detected.
🟡 Minor comments (24)
test/briefingApi.test.ts-133-141 (1)
133-141:⚠️ Potential issue | 🟡 MinorEmpty-string
dealIdmay not actually reach the route handler.
callGet('')builds the URLhttp://localhost/api/ai/briefing/and passes{ dealId: '' }inparams. In production, Next.js routing would never match this path to[dealId]at all (it would 404 from the router), so this test is really exercising your handler's validation branch with a synthetic input that can't occur in practice. That's fine as a defensive unit test, but worth a brief comment noting it's a guard against direct handler invocation rather than a real route scenario — otherwise a future reader may "fix" the validator thinking it's dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/briefingApi.test.ts` around lines 133 - 141, This test uses callGet('') to pass an empty-string dealId which wouldn't match Next.js dynamic route in production, so add a short clarifying comment above the it('retorna 400 quando dealId está vazio', ...) test explaining that this case is a defensive unit test for direct handler invocation (validator guard) rather than a real routed request; reference callGet and the empty '' dealId in the comment so future readers understand why the synthetic input is intentional and should not be removed.lib/ai/agent/rate-limiter.ts-29-53 (1)
29-53:⚠️ Potential issue | 🟡 MinorTOCTOU race across concurrent invocations.
Since this is a pure check (no atomic increment), two concurrent requests on the same
conversation_idcan both readcallsInWindow < maxand both proceed — the actual floor on calls-per-minute ismax + (concurrent_requests - 1). For a per-conversation AI spend limiter this is usually acceptable, but it's worth being explicit about in the JSDoc and, if stricter enforcement is needed, consider a SQL function that atomically counts-and-inserts, or a per-org/token-bucket in a Redis-like store.Also,
ai_conversation_loglikely lives under RLS and — per repo guidelines for service-role queries — filtering additionally byorganization_idtightens tenant isolation and also lets you add a useful composite index(organization_id, conversation_id, created_at)for this hot path.As per coding guidelines: "All database queries must filter by
organization_idin addition to RLS, especially when using service role client for AI/tools".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/rate-limiter.ts` around lines 29 - 53, checkConversationRateLimit currently does a non-atomic read of ai_conversation_log leading to a TOCTOU race (concurrent requests can exceed the per-minute limit); update the function signature (checkConversationRateLimit) to accept organization_id, add .eq('organization_id', organizationId) to the Supabase query and document the TOCTOU caveat in the JSDoc; for stricter enforcement either implement an atomic DB side procedure (count-and-insert SQL function) or use a Redis token-bucket per conversation/org, and add/ensure a composite index on (organization_id, conversation_id, created_at) to optimize this hot path.supabase/functions/messaging-webhook-resend/index.ts-135-147 (1)
135-147:⚠️ Potential issue | 🟡 MinorRemove the dead code path —
crypto.subtle.timingSafeEqualdoes not exist in Deno.The condition checking for
crypto.subtle.timingSafeEqualwill never be true. Deno's WebCrypto implementation does not expose this method, so the branch is unreachable dead code. The fallback XOR-based constant-time comparison is correct and will always execute. Remove the unnecessary if statement and type casting gymnastics; keep only the XOR loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 135 - 147, The timingSafeEqual function contains a dead branch checking for crypto.subtle.timingSafeEqual which doesn't exist in Deno; remove that entire if block and the associated type casts so the function only performs the constant-time XOR comparison (the existing diff/XOR loop) and returns diff === 0. Update the function body in timingSafeEqual to eliminate the unreachable crypto.subtle branch and keep the fallback loop as the sole comparison implementation.lib/ai/agent/input-filter.ts-108-114 (1)
108-114:⚠️ Potential issue | 🟡 MinorOnly the first occurrence of each injection pattern is neutralized.
None of the regexes in
INJECTION_PATTERNShave thegflag, sosanitized.replace(pattern, …)only wraps the first match. A message like"ignore previous instructions. also ignore previous instructions."is sanitized to"[ignore previous instructions]. also ignore previous instructions."— the second occurrence reaches the LLM unbracketed and can still be interpreted as an instruction. ThematchedPatternslog also won't reflect that multiple attempts were present.Also, doing
.test()followed by.replace()scans the string twice per pattern. A single.replacewith a counter is cleaner.🛡️ Proposed fix
-const INJECTION_PATTERNS: Array<[RegExp, string]> = [ - // --- Direct instruction override (EN) --- - [/ignore\s+(all\s+)?(previous|prior|above|earlier)\s+(instructions?|prompts?|rules?|directives?)/iu, 'ignore_instructions_en'], +const INJECTION_PATTERNS: Array<[RegExp, string]> = [ + // --- Direct instruction override (EN) --- + [/ignore\s+(all\s+)?(previous|prior|above|earlier)\s+(instructions?|prompts?|rules?|directives?)/giu, 'ignore_instructions_en'],(apply
giuto every pattern), and simplify the loop:- for (const [pattern, label] of INJECTION_PATTERNS) { - if (pattern.test(sanitized)) { - matchedPatterns.push(label); - // Neutralize: wrap matched content in brackets so LLM reads it as quoted text - sanitized = sanitized.replace(pattern, (match) => `[${match}]`); - } - } + for (const [pattern, label] of INJECTION_PATTERNS) { + let matched = false; + sanitized = sanitized.replace(pattern, (m) => { + matched = true; + return `[${m}]`; + }); + if (matched) matchedPatterns.push(label); + }Note: once patterns carry the
gflag, avoid callingpattern.test()on them beforereplace, sinceRegExp.prototype.testwith/gadvanceslastIndexbetween calls and the regex objects are module-scoped (shared across invocations).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/input-filter.ts` around lines 108 - 114, INJECTION_PATTERNS loop currently calls pattern.test() then sanitized.replace(), which only neutralizes the first occurrence (no g flag) and double-scans the string; change to a single replace pass per pattern: ensure each pattern is applied with the global flag (e.g., giu or create a new RegExp(pattern.source, 'giu') if patterns are shared), remove the test() call, and perform sanitized = sanitized.replace(patternWithG, (match) => { matchedPatterns.push(label); return `[${match}]`; }) so every match is wrapped and matchedPatterns records each occurrence; also avoid reusing module-scoped RegExp objects with /g by constructing fresh RegExp instances before replace.docs/observability/MONITORING_GUIDE.md-366-383 (1)
366-383:⚠️ Potential issue | 🟡 MinorDatadog snippet uses the browser SDK for server-side logging.
@datadog/browser-logsis explicitly for browser environments (relies onwindow, attaches to page lifecycle, uses client-side intake keys). For the AI agent service running server-side (Vercel Functions / Edge), use@datadog/datadog-api-client(logs API) ordd-tracewith the Node agent. Readers following this snippet on the server will hit runtime errors at import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/observability/MONITORING_GUIDE.md` around lines 366 - 383, The snippet imports and uses the browser SDK (`@datadog/browser-logs`) which will fail server-side; replace usage of the browser package and client-side calls (import { datadog } from '@datadog/browser-logs', datadog.setUser, datadog.logger.info) with a server-appropriate Datadog client (e.g., use `@datadog/datadog-api-client` or dd-trace), condition on process.env.DATADOG_KEY as before, and send the structured event via the server logs API or tracer API (mapping event, tokens_used, latency_ms into the API request) so the code runs in Vercel Functions/Edge without referencing window or browser-only APIs.lib/ai/config.ts-16-24 (1)
16-24:⚠️ Potential issue | 🟡 MinorRemove deprecated preview models from ALLOWED_GOOGLE_MODELS whitelist.
gemini-2.5-pro-preview-03-25andgemini-2.5-flash-preview-04-17are no longer available. Both were shut down by Google—the former on December 2, 2025, and the latter on July 15, 2025. Any organization configuration using these IDs will silently fall back togemini-2.0-flash(line 56) with no log or warning, masking the misconfiguration. Remove both preview IDs from the whitelist and keep only the stable GA models:gemini-2.0-flash,gemini-2.0-flash-lite,gemini-1.5-pro,gemini-1.5-flash,gemini-1.5-flash-8b.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/config.ts` around lines 16 - 24, ALLOWED_GOOGLE_MODELS currently contains two deprecated preview model IDs that must be removed; edit the ALLOWED_GOOGLE_MODELS Set in lib/ai/config.ts (symbol: ALLOWED_GOOGLE_MODELS) and delete 'gemini-2.5-pro-preview-03-25' and 'gemini-2.5-flash-preview-04-17', leaving only the stable GA entries: 'gemini-2.0-flash', 'gemini-2.0-flash-lite', 'gemini-1.5-pro', 'gemini-1.5-flash', and 'gemini-1.5-flash-8b'..github/workflows/preview.yml-20-50 (1)
20-50:⚠️ Potential issue | 🟡 MinorAddress fork PR failures and consider supply-chain pinning for the third-party action.
Three operational concerns with this workflow:
Fork PRs will fail silently.
pull_requestdoes not expose repo secrets to workflows triggered from forks, so the deploy step fails whenVERCEL_TOKEN,VERCEL_ORG_ID, andVERCEL_PROJECT_IDare empty. If the project accepts external contributors, gate the job withif: github.event.pull_request.head.repo.full_name == github.repositoryto avoid a red ❌ check on every external PR.Third-party action pinned to a floating tag.
amondnet/vercel-action@v25can be re-tagged by the publisher. GitHub's guidance is to pin to a commit SHA with the version annotated in a comment for maintainability.Potential duplicate PR comments. Vercel's GitHub app may post a comment independently of this action. Test the first fork PR to confirm whether duplicate comments or missing URLs appear with the current
--no-wait+github-comment: trueconfiguration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/preview.yml around lines 20 - 50, The workflow exposes three issues: fork PRs fail due to missing secrets, the third‑party action is pinned to a floating tag, and the deploy may create duplicate comments; fix by gating the deploy job (job name deploy-preview) with a conditional such as checking github.event.pull_request.head.repo.full_name == github.repository so fork PRs skip the deploy step, change the action pin from amondnet/vercel-action@v25 to a specific commit SHA (keep the tag in a comment for readability), and review the vercel-args/github-comment settings (e.g., --no-wait plus github-comment: true) to avoid duplicate comments from Vercel’s app—adjust or disable github-comment if the Vercel app already posts links.lib/ai/agent/generate-prompts.service.ts-145-154 (1)
145-154:⚠️ Potential issue | 🟡 MinorFire-and-forget log insert can be dropped in serverless runtimes.
In a Vercel/serverless Route Handler, the function instance may suspend or terminate as soon as the response is returned, so this un-awaited
insert(...).then(...)can silently lose log writes under load. Since this log is what the PR repositions as the source of truth for auditing and rate limiting, dropping entries is worth avoiding.Either
awaitthe insert, or schedule it withafter()fromnext/serverso it runs after the response is flushed but is still tracked by the runtime:♻️ Proposed fix
- void supabase.from('ai_conversation_log').insert({ - organization_id: organizationId, - ai_response: '', - tokens_used: result.usage?.totalTokens ?? 0, - model_used: aiConfig.model, - action_taken: 'generate_stage_prompts', - context_snapshot: { boardId, stageCount: stages.length }, - }).then(({ error }: { error: unknown }) => { - if (error) console.error('[AI] log failed:', error); - }); + const { error: logError } = await supabase.from('ai_conversation_log').insert({ + organization_id: organizationId, + ai_response: '', + tokens_used: result.usage?.totalTokens ?? 0, + model_used: aiConfig.model, + action_taken: 'generate_stage_prompts', + context_snapshot: { boardId, stageCount: stages.length }, + }); + if (logError) console.error('[AI] log failed:', logError);Same pattern applies to
app/api/ai/board-config/generate-goal/route.ts(lines 81-90).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/generate-prompts.service.ts` around lines 145 - 154, The fire-and-forget supabase.from('ai_conversation_log').insert(...).then(...) call can be dropped in serverless runtimes; change it to be awaited (await supabase.from('ai_conversation_log').insert(...)) or schedule it with next/server's after() so the runtime tracks it after the response is flushed, and propagate/handle errors via processLogger or console.error; apply the same change for the analogous insert call in the generate-goal route (the insert block around the ai conversation log in app/api/ai/board-config/generate-goal/route.ts).app/api/ai/board-config/generate-goal/route.ts-36-56 (1)
36-56:⚠️ Potential issue | 🟡 MinorUnknown
categoryfalls through as raw user input into the prompt.
CATEGORY_LABELS[category] ?? categorymeans any category string the client sends (e.g.,category: "\n\nIGNORE ABOVE. Now do X...") is interpolated directly into the LLM prompt when it doesn't match a whitelisted key. Given that the rest of this PR hardens LLM inputs viasanitizeIncomingMessage, this is a small but real prompt-injection gap. Prefer rejecting unknown categories, or at minimum sanitizing:🛡️ Proposed fix
- const { businessContext, category } = body; - if (!businessContext?.trim() || !category) { + const { businessContext, category } = body; + if (!businessContext?.trim() || !category) { return NextResponse.json({ error: 'businessContext and category are required' }, { status: 400 }); } + if (!(category in CATEGORY_LABELS)) { + return NextResponse.json({ error: 'Invalid category' }, { status: 400 }); + } @@ - const categoryLabel = CATEGORY_LABELS[category] ?? category; + const categoryLabel = CATEGORY_LABELS[category];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/ai/board-config/generate-goal/route.ts` around lines 36 - 56, The code allows unrecognized category values to be interpolated into LLM prompts via CATEGORY_LABELS[category] ?? category; validate the incoming category against the known whitelist and reject unknown values (return 400) instead of falling back to raw input, or at minimum run the value through sanitizeIncomingMessage before using it in the prompt; update the logic around the category variable/CATEGORY_LABELS lookup in route.ts to either (a) check Object.hasOwn(CATEGORY_LABELS, category) and return a 400 with an error message, or (b) call sanitizeIncomingMessage(category) and use that sanitized value when constructing the prompt so raw client input cannot perform prompt injection.lib/ai/agent/output-validator.ts-86-92 (1)
86-92:⚠️ Potential issue | 🟡 MinorPotential false positives on
deal.valuewith 3-digit numbers.Values like
100,250,500are extremely common in sales copy (e.g., "desconto de 100 reais"), so a 3-digit floor will flag many legitimate responses and swap them with the generic fallback — defeating the point of a tailored answer. Consider raising the threshold (e.g.,>= 4digits) or requiring the value to appear with a currency prefix/suffix before flagging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/output-validator.ts` around lines 86 - 92, The current leak check in output-validator.ts flags any numeric deal.value with length >= 3; change the guard on context.deal.value to require length >= 4 (e.g., valueStr.length >= 4) to avoid common 3-digit false positives, or, even better, add an extra condition that the response contains a currency marker around the number (use a regex test on response for currency symbols/words like /(?:R\$|\$|€|£|\breais\b)/i) before pushing into leaks; update the logic that references context.deal.value, valueStr, response, and the leaks.push(`deal_value:...`) (and keep the redaction format deal_value:{firstTwo}***) accordingly..github/workflows/release.yml-83-83 (1)
83-83:⚠️ Potential issue | 🟡 MinorBump
softprops/action-gh-releasebeyondv1.actionlint flags
v1as too old for current GitHub Actions runners. Pin to a recent major (e.g.,v2) or, better, a commit SHA for supply-chain hardening.- uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml at line 83, Update the GitHub Actions step that currently uses softprops/action-gh-release@v1 to a supported reference: replace the tag "v1" with a newer major tag such as "v2" or, preferably, a commit SHA for supply-chain hardening (the action reference is the uses: softprops/action-gh-release entry); ensure the workflow still passes by running a quick CI test after updating the reference.lib/ai/messaging/circuit-breaker.ts-31-64 (1)
31-64:⚠️ Potential issue | 🟡 MinorNon-atomic read-modify-write on
consecutive_ai_errors.Under concurrent failures on the same conversation, two handlers can both read
nand both writen+1, losing increments and delaying the trip. Prefer an atomic RPC (e.g.,increment_ai_errors(conversation_id)usingUPDATE … SET consecutive_ai_errors = consecutive_ai_errors + 1 RETURNING consecutive_ai_errors) so the threshold check uses the authoritative new value.♻️ Sketch
- const { data } = await supabase - .from('messaging_conversations') - .select('consecutive_ai_errors') - .eq('id', conversationId) - .maybeSingle(); - - const newCount = (data?.consecutive_ai_errors ?? 0) + 1; - - await supabase - .from('messaging_conversations') - .update({ consecutive_ai_errors: newCount }) - .eq('id', conversationId); + const { data: newCount } = await supabase.rpc('increment_conversation_ai_errors', { + p_conversation_id: conversationId, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/messaging/circuit-breaker.ts` around lines 31 - 64, The current incrementCircuitBreakerError function does a non-atomic read-modify-write causing lost increments under concurrency; change it to perform the increment atomically in the database (use supabase.from('messaging_conversations').update({ consecutive_ai_errors: 'consecutive_ai_errors + 1' } or better, create an RPC like increment_ai_errors(conversation_id) that executes UPDATE ... SET consecutive_ai_errors = consecutive_ai_errors + 1 RETURNING consecutive_ai_errors) and use the returned authoritative consecutive_ai_errors value as newCount for the threshold check; then, based on that returned newCount and contactId, update contacts.ai_paused and emit the log using the newCount (replace the separate select/update pair in incrementCircuitBreakerError with the single atomic update/RPC and use its returned value).features/settings/components/ai/AIAgentConfigSection.tsx-47-55 (1)
47-55:⚠️ Potential issue | 🟡 Minor
useEffectdep list is missingupdateConfig/provisionStages— will trip the zero-warnings lint gate.
react-hooks/exhaustive-depsflags this. Even if the mutation objects happen to be stable at runtime, the inline comment isn't enough to silence ESLint. Either add an explicit// eslint-disable-next-line react-hooks/exhaustive-depswith the justification, or capture themutateAsyncfunctions in a ref.As per coding guidelines: "Enforce zero warnings in linting with
npm run lint".♻️ Proposed fix
- useEffect(() => { + useEffect(() => { if (hasProvisioned.current) return; if (!config || config.ai_config_mode) return; hasProvisioned.current = true; updateConfig.mutateAsync({ ai_config_mode: 'zero_config' }) .then(() => provisionStages.mutateAsync()) .catch((e: unknown) => console.error('[AIAgentConfig] Auto-provision failed:', e)); - }, [config]); // deps: only config matters — mutations are stable refs + // eslint-disable-next-line react-hooks/exhaustive-deps -- mutations from react-query are stable; re-running would re-provision. + }, [config]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/settings/components/ai/AIAgentConfigSection.tsx` around lines 47 - 55, The useEffect depends on updateConfig.mutateAsync and provisionStages.mutateAsync but doesn't include them in the deps, causing react-hooks/exhaustive-deps lint warnings; fix by either (A) capturing the mutateAsync functions into refs (e.g., const updateConfigRef = useRef(updateConfig.mutateAsync) and const provisionStagesRef = useRef(provisionStages.mutateAsync) and use those refs inside useEffect), or (B) explicitly add a single-line eslint disable above the useEffect (// eslint-disable-next-line react-hooks/exhaustive-deps) with a short justification comment referencing hasProvisioned and that the mutation objects are stable; update the effect to call the ref functions (or keep current calls if using the eslint disable) and ensure hasProvisioned.current handling remains unchanged.app/api/health/route.ts-98-100 (1)
98-100:⚠️ Potential issue | 🟡 MinorGET vs HEAD disagree on the "AI key configured" boundary.
GET rejects
aiApiKey.length < 20(so exactly 20 passes). HEAD requiresaiApiKey.length > 20(so exactly 20 fails). A real key landing at length 20 would cause a flapping health signal whereGET /api/healthishealthybutHEAD /api/healthis503, confusing monitors/load balancers that use HEAD probes.♻️ Proposed fix — extract a single predicate
- const aiConfigured = !!(aiApiKey && aiApiKey.length > 20); + const aiConfigured = !!(aiApiKey && aiApiKey.length >= 20);Also applies to: 157-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/health/route.ts` around lines 98 - 100, The GET and HEAD handlers use inconsistent predicates for aiApiKey length (one checks aiApiKey.length < 20, the other aiApiKey.length > 20), causing flapping at length 20; extract a single predicate function (e.g., isAiKeyConfigured(aiApiKey): boolean) that returns aiApiKey?.length >= 20 and use that function in both the GET health path (replace the aiApiKey.length < 20 check) and the HEAD path (replace the aiApiKey.length > 20 check) so the boundary is identical across handlers (also update the two spots where those comparisons currently appear).app/api/cron/stage-evaluations/route.ts-129-141 (1)
129-141:⚠️ Potential issue | 🟡 Minor
status: 'completed'with a populatedlast_erroris semantically contradictory.When stage AI is disabled/removed, the row is marked
completedyet itslast_errorrecords "Stage AI not enabled at evaluation time". Downstream dashboards filteringstatus='completed' AND last_error IS NULLwill miscount, and audit queries will surface "errors" on successful-terminal rows. Consider either leavinglast_errornull (this is a normal terminal state) or introducing askippedstatus to match theskippedcounter already used in the response body.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/cron/stage-evaluations/route.ts` around lines 129 - 141, The current update in route.ts marks rows with status: 'completed' while setting last_error to "Stage AI not enabled at evaluation time", which is contradictory; change the update in the block that checks if (!stageConfig) to either (A) set status to 'skipped' (matching the skipped counter) and set last_error to null, or (B) keep status 'completed' but clear last_error (set to null) — update the supabase.from('ai_pending_evaluations').update(...) call accordingly so that last_error is null for normal terminal skips and optionally use status: 'skipped' to align with the skipped counter and downstream filters.docs/release-engineering.md-70-70 (1)
70-70:⚠️ Potential issue | 🟡 MinorAdd language identifiers to these fenced code blocks (markdownlint MD040).
Lines 70, 133, 160, and 222 open with bare
```. Usetext(ordiff/bashwhere applicable) to satisfy MD040 and give the renderer proper semantics.Also applies to: 133-133, 160-160, 222-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/release-engineering.md` at line 70, Several fenced code blocks open with bare triple backticks (```); add a language identifier to each opening fence to satisfy markdownlint MD040 and improve rendering. Replace the bare fences for the blocks starting at the noted locations with appropriate identifiers such as ```text for plain snippets, ```bash for shell commands, or ```diff for patch-like ranges; ensure the closing fences remain ``` so each block is consistently fenced. Locate occurrences of the bare ``` tokens and update them to include the chosen language token.app/api/ai/board-config/generate-persona/route.ts-23-28 (1)
23-28:⚠️ Potential issue | 🟡 MinorUnhandled JSON parse error.
await request.json()throws on malformed payloads and the exception escapes the handler (notry/catch), producing an uncontrolled 500. Wrap intry/catchand return400with a stable error body, same as other board-config routes should do.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/ai/board-config/generate-persona/route.ts` around lines 23 - 28, Wrap the JSON parsing step (the await request.json() call in route.ts) in a try/catch so malformed payloads don't bubble up; on parse failure return NextResponse.json({ error: 'Invalid JSON payload' }, { status: 400 }) (matching the other board-config route error shape), then proceed to destructure businessContext/agentGoal/websiteUrl and keep the existing businessContext validation. Ensure the catch only handles JSON parse errors and preserves behavior for valid requests.test/aiOutputValidator.test.ts-199-207 (1)
199-207:⚠️ Potential issue | 🟡 MinorNit: comment disagrees with the fixture value.
The comment says
deal.value = 150 (2 dígitos < 3)but the fixture setsvalue: 15. Either align the comment to the actual value (15, one/two digits) or fix the fixture to match the documented "150" case — whichever matches the validator's real threshold.💚 Proposed comment fix
- // deal.value = 150 (2 dígitos < 3) não deve triggar + // deal.value = 15 (abaixo do mínimo de dígitos exigido) não deve triggar🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/aiOutputValidator.test.ts` around lines 199 - 207, The test comment and fixture disagree: the comment claims deal.value = 150 but the test sets value: 15; update either the comment or the fixture to be consistent with the intended threshold. Locate the test case that calls validateAIOutput in this spec (the it block with description 'não rejeita número muito curto como PII (evita falso positivo)'), then either change the inline comment to reference 15 (or “2 dígitos”) to match the current fixture, or change the fixture value to 150 if the intent was to test the 3-digit boundary; ensure EMPTY_CONTEXT, the deal object, and the assertion remain appropriate for the chosen value.app/api/cron/stage-evaluations/route.ts-39-44 (1)
39-44:⚠️ Potential issue | 🟡 MinorUse timing-safe comparison for
CRON_SECRET.The PR summary notes the audit standardized on timing-safe HMAC checks for sensitive tokens. A plain
!==leaks comparison time. Usecrypto.timingSafeEqualon equal-length buffers (guarding against length mismatch first).♻️ Proposed fix
-import { createClient } from '@/lib/supabase/server'; +import { createClient } from '@/lib/supabase/server'; +import { timingSafeEqual } from 'node:crypto'; ... - if (!cronSecret || authHeader !== `Bearer ${cronSecret}`) { + const expected = cronSecret ? `Bearer ${cronSecret}` : ''; + const provided = authHeader ?? ''; + const equal = + !!cronSecret && + provided.length === expected.length && + timingSafeEqual(Buffer.from(provided), Buffer.from(expected)); + if (!equal) { return json({ error: 'Unauthorized' }, 401); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/cron/stage-evaluations/route.ts` around lines 39 - 44, Replace the plain string comparison of `authHeader` and `cronSecret` with a timing-safe comparison: ensure both values exist, strip the "Bearer " prefix from `authHeader`, convert both strings to Buffers, check lengths match, and use `crypto.timingSafeEqual` to compare the buffers; if lengths differ or the timing-safe check fails, return the same unauthorized response. Update the code around the existing `authHeader` and `cronSecret` variables in `route.ts` to perform these steps and preserve the existing 401/json({ error: 'Unauthorized' }) behavior when validation fails.features/settings/components/ai/BoardAIConfigModal.tsx-883-887 (1)
883-887:⚠️ Potential issue | 🟡 MinorTernary used for side effects triggers
no-unused-expressions.
next.has(id) ? next.delete(id) : next.add(id);is flagged by ESLint'sno-unused-expressions; the coding guidelines require zero lint warnings across**/*.{ts,tsx}. Prefer anif/else(or the booleannext.delete(id) || next.add(id)idiom).As per coding guidelines: "ESLint must have zero warnings across all TypeScript and TSX files".
🛠️ Proposed fix
- onToggleStage={(id) => setEnabledStages((prev) => { - const next = new Set(prev); - next.has(id) ? next.delete(id) : next.add(id); - return next; - })} + onToggleStage={(id) => setEnabledStages((prev) => { + const next = new Set(prev); + if (next.has(id)) next.delete(id); + else next.add(id); + return next; + })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/settings/components/ai/BoardAIConfigModal.tsx` around lines 883 - 887, The ternary in the onToggleStage handler used for side effects causes a no-unused-expressions lint warning; update the callback passed to setEnabledStages (the onToggleStage handler in BoardAIConfigModal) to perform the mutation with an explicit if/else (or use the boolean short-circuit idiom) instead of a ternary: create next as a new Set(prev), check next.has(id) and call next.delete(id) in the if branch else call next.add(id), then return next; keep the rest of the setEnabledStages logic the same.features/settings/components/ai/BoardAgentsSection.tsx-69-94 (1)
69-94:⚠️ Potential issue | 🟡 MinorSilent failures in
handleModeChangemay desync UI from server.The
DELETEpath updates local state tonullregardless ofres.ok, and thePUTpath silently swallows non-ok responses. If either request fails (5xx, network), the pills will show a state that doesn't match the backend, and the user gets no feedback. Consider checkingres.okon both branches and surfacing a toast on failure (the same patternhandleSaveuses further down).🛠️ Proposed fix
if (mode === 'off') { - await fetch(`/api/ai/board-config/${boardId}`, { method: 'DELETE' }); - setBoardConfigs((prev) => ({ ...prev, [boardId]: null })); + const res = await fetch(`/api/ai/board-config/${boardId}`, { method: 'DELETE' }); + if (!res.ok) throw new Error('Falha ao desligar o agente'); + setBoardConfigs((prev) => ({ ...prev, [boardId]: null })); } else { const res = await fetch(`/api/ai/board-config/${boardId}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ agent_mode: mode }), }); - if (res.ok) { - const { config } = await res.json() as { config: BoardAIConfig }; - setBoardConfigs((prev) => ({ ...prev, [boardId]: config })); - } + if (!res.ok) throw new Error('Falha ao salvar o modo do agente'); + const { config } = await res.json() as { config: BoardAIConfig }; + setBoardConfigs((prev) => ({ ...prev, [boardId]: config })); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/settings/components/ai/BoardAgentsSection.tsx` around lines 69 - 94, handleModeChange currently updates local state without checking the HTTP response, causing silent failures and UI desync; update the function to check response.ok for both the DELETE and PUT branches (do not call setBoardConfigs unless res.ok), catch network/errors from fetch, and on any non-ok or caught error show the same toast/error UI used by handleSave (and avoid mutating state on failure); keep the existing setLoadingBoards/finally behavior so loading is always cleared.lib/ai/agent/agent.service.ts-701-704 (1)
701-704:⚠️ Potential issue | 🟡 MinorWrong identifiers passed as sanitize context.
org_idis being set tocontext.organization.name(a display name, not a UUID) andconversation_idis set tocontext.deal?.id(deal id, not conversation id). Downstream logging insidesanitizeIncomingMessagewill attribute injection attempts to the wrong org/conversation, which will make forensic queries misleading.The
conversationIdis already a parameter ofprocessIncomingMessage, andorganizationIdis too — pass those through instead.🛠️ Proposed fix
At
generateResponse, thread the IDs throughGenerateResponseParams:interface GenerateResponseParams { context: LeadContext; stageConfig: StageAIConfig; incomingMessage: string; aiConfig: OrgAIConfig; boardAIConfig: BoardAIConfig | null; + organizationId: string; + conversationId: string; }Then:
- const sanitized = sanitizeIncomingMessage(incomingMessage, { - org_id: context.organization.name, - conversation_id: context.deal?.id, - }); + const sanitized = sanitizeIncomingMessage(incomingMessage, { + org_id: params.organizationId, + conversation_id: params.conversationId, + });And pass them from
processIncomingMessageat thegenerateResponse({...})call site.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 701 - 704, sanitizeIncomingMessage is being called with the wrong identifiers (org name and deal id); update the call site so it receives the actual organizationId and conversationId parameters instead. Thread organizationId and conversationId through GenerateResponseParams and ensure generateResponse accepts/forwards them, then change the sanitizeIncomingMessage invocation in generateResponse to use organizationId and conversationId (not context.organization.name or context.deal?.id), and update the processIncomingMessage call-site to pass those IDs into generateResponse.supabase/migrations/20260409120000_hitl_pending_alerts.sql-132-139 (1)
132-139:⚠️ Potential issue | 🟡 Minor
affected_dealscounts across runs and across orgs, not this invocation.This post-loop aggregation scans
deal_activitiesglobally for anyhitl_alertwithin the last 6 hours, which:
- double-counts alerts from the previous cron run (since the job runs every 6 hours, boundaries overlap), and
- ignores org scoping so organizations are mixed in the total.
Tracking deal IDs inside the loop gives the accurate count of what this invocation actually touched.
♻️ Proposed fix
DECLARE v_alert_count BIGINT := 0; v_affected_deals BIGINT := 0; v_pending_advance RECORD; + v_deal_ids UUID[] := ARRAY[]::UUID[]; BEGIN @@ v_alert_count := v_alert_count + 1; + v_deal_ids := array_append(v_deal_ids, v_pending_advance.deal_id); END LOOP; - -- Contar deals únicos afetados - SELECT COUNT(DISTINCT deal_id) INTO v_affected_deals - FROM public.deal_activities - WHERE - type = 'hitl_alert' - AND created_at >= (NOW() - INTERVAL '6 hours'); + -- Contar deals únicos afetados nesta execução + SELECT COUNT(DISTINCT x) INTO v_affected_deals + FROM unnest(v_deal_ids) AS x;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260409120000_hitl_pending_alerts.sql` around lines 132 - 139, The current global COUNT(DISTINCT deal_id) into v_affected_deals queries public.deal_activities for the last 6 hours and therefore double-counts across overlapping runs and mixes organizations; instead, collect affected deal_ids during the loop that processes hitl_alert entries (e.g., push each processed deal_id into a local array or insert into a temporary table like temp_hitl_deals tied to this invocation), ensure you scope inserts by org_id consistent with how alerts are processed, then compute v_affected_deals as COUNT(DISTINCT ...) over that per-invocation collection before RETURN QUERY SELECT v_alert_count, v_affected_deals; this guarantees the count reflects only deals touched by this run and by the correct org.supabase/migrations/20260409120000_hitl_pending_alerts.sql-232-235 (1)
232-235:⚠️ Potential issue | 🟡 MinorAge buckets overlap at exact boundaries.
BETWEENis inclusive at both ends, so a row withcreated_at = NOW() - 12his counted in bothage_6_12handage_12_24h; a row atNOW() - 6his missing fromage_0_6h; a row atNOW() - 24his missing fromage_gt_24h. Use half-open ranges for consistent bucketing.♻️ Proposed fix
- COUNT(CASE WHEN created_at > (NOW() - INTERVAL '6 hours') THEN 1 END) as age_0_6h, - COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '12 hours') AND (NOW() - INTERVAL '6 hours') THEN 1 END) as age_6_12h, - COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '24 hours') AND (NOW() - INTERVAL '12 hours') THEN 1 END) as age_12_24h, - COUNT(CASE WHEN created_at < (NOW() - INTERVAL '24 hours') THEN 1 END) as age_gt_24h, + COUNT(*) FILTER (WHERE created_at >= (NOW() - INTERVAL '6 hours')) as age_0_6h, + COUNT(*) FILTER (WHERE created_at >= (NOW() - INTERVAL '12 hours') AND created_at < (NOW() - INTERVAL '6 hours')) as age_6_12h, + COUNT(*) FILTER (WHERE created_at >= (NOW() - INTERVAL '24 hours') AND created_at < (NOW() - INTERVAL '12 hours')) as age_12_24h, + COUNT(*) FILTER (WHERE created_at < (NOW() - INTERVAL '24 hours')) as age_gt_24h,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260409120000_hitl_pending_alerts.sql` around lines 232 - 235, The age buckets overlap or leave gaps because BETWEEN is inclusive; update the COUNT(CASE WHEN ...) conditions for aliases age_0_6h, age_6_12h, age_12_24h, age_gt_24h to use half-open ranges: make age_0_6h include created_at >= (NOW() - INTERVAL '6 hours'), make age_6_12h use created_at >= (NOW() - INTERVAL '12 hours') AND created_at < (NOW() - INTERVAL '6 hours'), make age_12_24h use created_at >= (NOW() - INTERVAL '24 hours') AND created_at < (NOW() - INTERVAL '12 hours'), and keep age_gt_24h as created_at < (NOW() - INTERVAL '24 hours') so no timestamp falls into two buckets or none.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff1270ef-66e8-4647-9356-f8762f83dee6
⛔ Files ignored due to path filters (2)
lib/query/__tests__/__snapshots__/cache-integrity.test.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (82)
.commitlintrc.json.github/workflows/ci.yml.github/workflows/preview.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdRELEASE-SETUP.mdRELEASE.mdapp/(protected)/layout.tsxapp/api/ai/actions/route.tsapp/api/ai/board-config/[boardId]/route.tsapp/api/ai/board-config/generate-goal/route.tsapp/api/ai/board-config/generate-persona/route.tsapp/api/ai/hitl/route.tsapp/api/ai/tasks/deals/analyze/route.tsapp/api/cron/stage-evaluations/route.tsapp/api/health/route.tsapp/api/public/v1/boards/route.tsapp/api/public/v1/companies/route.tsapp/api/public/v1/contacts/route.tsapp/api/public/v1/deals/route.tsdocs/audit-report.mddocs/observability/IMPLEMENTATION_SUMMARY.mddocs/observability/MONITORING_GUIDE.mddocs/release-engineering.mddocs/superpowers/specs/2026-04-09-goal-oriented-agent-design.mdeslint.config.mjsfeatures/boards/components/Kanban/KanbanBoard.tsxfeatures/boards/components/Modals/CreateDealModal.tsxfeatures/boards/components/Modals/CreateDealModalV2.tsxfeatures/boards/components/Modals/DealDetailModal.test.tsxfeatures/boards/components/Modals/DealDetailModal.tsxfeatures/boards/hooks/useBoardsController.tsfeatures/contacts/components/ContactFormModal.tsxfeatures/messaging/components/MessageInput.tsxfeatures/settings/SettingsPage.rbac.test.tsxfeatures/settings/components/StageAIConfig.tsxfeatures/settings/components/ai/AIAgentConfigSection.tsxfeatures/settings/components/ai/BoardAIConfigModal.tsxfeatures/settings/components/ai/BoardAgentsSection.tsxfeatures/settings/components/ai/TelegramNotificationSettings.tsxlib/ai/agent/agent.service.tslib/ai/agent/generate-prompts-schema.tslib/ai/agent/generate-prompts.service.tslib/ai/agent/input-filter.tslib/ai/agent/output-validator.tslib/ai/agent/rate-limiter.tslib/ai/agent/structured-logger.tslib/ai/agent/types.tslib/ai/config.tslib/ai/messaging/board-config.tslib/ai/messaging/circuit-breaker.tslib/ai/messaging/file-search.tslib/ai/messaging/persona-generator.tslib/ai/messaging/types.tslib/ai/tools.tslib/ai/utils/web-scraper.tslib/query/hooks/useContactsQuery.tslib/query/hooks/useDuplicateContactsQuery.tslib/query/index.tsxlib/supabase/staticAdminClient.tslib/utils/sanitize.tspackage.jsonsupabase/functions/messaging-webhook-evolution/index.tssupabase/functions/messaging-webhook-resend/index.tssupabase/functions/messaging-webhook-zapi/index.tssupabase/migrations/20260409120000_hitl_pending_alerts.sqlsupabase/migrations/20260409130000_ai_pending_evaluations.sqlsupabase/migrations/20260409140000_board_ai_config.sqltest/agentGoalStage.test.tstest/aiInputFilter.test.tstest/aiOutputValidator.test.tstest/briefingApi.test.tstest/goalOrientedAgent.test.tstest/hitlApi.test.tstest/messagingAiProcess.test.tstest/publicApi.contacts.test.tstest/publicApi.deals.test.tstest/stories/US-001-abrir-deal-no-boards.test.tsxtest/stories/US-AI-007-stage-advancement.test.tsxvercel.jsonvitest.config.ts
💤 Files with no reviewable changes (3)
- features/boards/components/Modals/CreateDealModalV2.tsx
- features/settings/components/ai/TelegramNotificationSettings.tsx
- features/boards/components/Modals/CreateDealModal.tsx
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| LATEST=$(gh release list --limit 1 --json tagName --jq -r '.[] | .tagName' 2>/dev/null || echo "") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
gh CLI --jq flag syntax for extracting raw values from --json output
💡 Result:
The syntax for the gh CLI --jq flag to extract raw values from --json output is: gh --json <field1,field2,...> --jq '' Key points: - --json must be used first with a comma-separated list of fields (e.g., number,title,author). Run gh --json without fields to list available fields. - --jq follows, taking a string argument in standard jq query syntax (quoted with single quotes for shell safety). - The --jq expression processes the JSON array/object from --json and outputs matching raw values, without quotes for strings when possible. - No external jq tool is needed; gh has built-in support. - Output is pretty-printed in terminals. Examples: 1. Extract author logins as raw strings: gh pr list --json author --jq '.[].author.login' Output: monalisa codercat cli-maintainer 2. Complex filtering (issues with labels, first 3): gh issue list --json number,title,labels --jq 'map(select((.labels | length) > 0)) | map(.labels = (.labels | map(.name))) | .[:3]' For gh api, --jq works similarly on the API response: gh api repos/{owner}/{repo}/issues --jq '.[].title' To get raw values without formatting/quoting, use jq expressions like .field or .[].field that output primitives directly.
Citations:
- 1: https://cli.github.com/manual/gh_help_formatting
- 2: https://cli.github.com/manual/gh_api
- 3: https://cli.github.com/manual/gh_help_reference
gh --jq -r '…' is malformed — the step always falls back to git describe.
gh release list --jq takes a single jq expression as its argument. The -r flag does not exist for gh's built-in jq—it is a standalone jq tool flag. As written, gh receives --jq -r (treating -r as the jq expression) and .[] | .tagName as extraneous positional arguments, which fails. Because of 2>/dev/null || echo "", LATEST is always empty and the fallback git describe --tags --abbrev=0 is the only path actually used—so GitHub Releases is never consulted.
🔧 Proposed fix
- LATEST=$(gh release list --limit 1 --json tagName --jq -r '.[] | .tagName' 2>/dev/null || echo "")
+ LATEST=$(gh release list --limit 1 --json tagName --jq '.[0].tagName' 2>/dev/null || echo "")📝 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.
| LATEST=$(gh release list --limit 1 --json tagName --jq -r '.[] | .tagName' 2>/dev/null || echo "") | |
| LATEST=$(gh release list --limit 1 --json tagName --jq '.[0].tagName' 2>/dev/null || echo "") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 55, The gh CLI invocation incorrectly
supplies -r as a separate argument; update the LATEST assignment so the jq
expression is passed as a single argument to gh (remove the standalone -r), e.g.
call gh release list --limit 1 --json tagName --jq '.[] | .tagName' (or select
the first element with --jq '.[0].tagName') and/or pipe the output to a real jq
-r if you need raw output, ensuring you don't pass extraneous positional args;
adjust the LATEST assignment around the variable name LATEST and the gh release
list command accordingly so the command returns the release tag instead of
always falling back to git describe.
| const { data, error } = await supabase | ||
| .from('board_ai_config') | ||
| .select('*') | ||
| .eq('board_id', boardId) | ||
| .maybeSingle(); | ||
|
|
||
| if (error) { | ||
| console.error('[BoardConfig] GET failed:', error); | ||
| return NextResponse.json({ error: 'Failed to fetch config' }, { status: 500 }); | ||
| } | ||
|
|
||
| return NextResponse.json({ config: data }); | ||
| } |
There was a problem hiding this comment.
GET is missing the organization_id filter.
Only board_id is used in the .eq(...). If RLS on board_ai_config is ever misconfigured or rolled back, any authenticated user could fetch configs across tenants — including sensitive fields like persona_prompt and knowledge_store_id. Even with RLS in place, the project standard requires defense-in-depth.
As per coding guidelines: "All database queries must filter by organization_id in addition to RLS, especially when using service role client for AI/tools" and "All database queries must filter by organization_id to ensure multi-tenant security".
🛡️ Proposed fix
const { data, error } = await supabase
.from('board_ai_config')
.select('*')
.eq('board_id', boardId)
+ .eq('organization_id', profile.organization_id)
.maybeSingle();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/`[boardId]/route.ts around lines 38 - 50, The query
on board_ai_config only filters by board_id and must also filter by
organization_id to enforce tenant isolation; retrieve the current organization
id (e.g., from the authenticated session/request context or the same place you
get boardId) and add an additional filter .eq('organization_id', organizationId)
to the supabase query (the chain on
supabase.from('board_ai_config').select('*').eq('board_id',
boardId).maybeSingle()) so the query becomes constrained by both board_id and
organization_id before calling maybeSingle(); ensure the organizationId variable
is validated/present and include it in the query to satisfy defense-in-depth
requirements.
| const body = await request.json() as { businessContext?: string; agentGoal?: string; websiteUrl?: string }; | ||
| const { businessContext, agentGoal, websiteUrl } = body; | ||
|
|
||
| if (!businessContext?.trim()) { | ||
| return NextResponse.json({ error: 'businessContext is required' }, { status: 400 }); | ||
| } | ||
|
|
||
| // Scrape website content if URL provided (via r.jina.ai → Markdown) | ||
| let scrapedWebContent: string | undefined; | ||
| if (websiteUrl?.startsWith('http')) { | ||
| const scraped = await scrapeUrl(websiteUrl); | ||
| if (scraped) { | ||
| scrapedWebContent = [ | ||
| scraped.title ? `Título do site: ${scraped.title}` : '', | ||
| `Conteúdo do site (${scraped.source === 'jina' ? 'renderizado com JS' : 'HTML estático'}):\n${scraped.markdown}`, | ||
| ].filter(Boolean).join('\n\n'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Apply sanitizeIncomingMessage and logAIAction here too for consistency with the rest of the audit.
The PR audit applies sanitizeIncomingMessage to user input before LLM use and centralizes audit via logAIAction in actions/route.ts, generate-goal/route.ts, generate-prompts.service.ts, and analyze/route.ts — but this sibling route (generate-persona) feeds user-controlled businessContext / agentGoal straight into generatePersonaPrompt (LLM) without either. That leaves a prompt-injection / audit gap in the same surface area the audit intended to close.
🛡️ Proposed fix
- const body = await request.json() as { businessContext?: string; agentGoal?: string; websiteUrl?: string };
- const { businessContext, agentGoal, websiteUrl } = body;
+ const body = await request.json() as { businessContext?: string; agentGoal?: string; websiteUrl?: string };
+ const { websiteUrl } = body;
+ const businessContext = body.businessContext
+ ? sanitizeIncomingMessage(body.businessContext).text
+ : undefined;
+ const agentGoal = body.agentGoal
+ ? sanitizeIncomingMessage(body.agentGoal).text
+ : undefined;Then call logAIAction on both success and failure paths, mirroring generate-goal/route.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/generate-persona/route.ts` around lines 23 - 40,
Sanitize incoming user fields and record the AI action: run
sanitizeIncomingMessage on businessContext and agentGoal (from the body parsed
in route.ts) before passing them into generatePersonaPrompt, and ensure you call
logAIAction on both success and error paths (similar to generate-goal/route.ts)
including context like sanitized inputs, websiteUrl/scrapedWebContent, and any
error details; keep scrapeUrl/scrapedWebContent usage the same but log the
scraped result context in the audit entry.
| if (websiteUrl?.startsWith('http')) { | ||
| const scraped = await scrapeUrl(websiteUrl); | ||
| if (scraped) { | ||
| scrapedWebContent = [ | ||
| scraped.title ? `Título do site: ${scraped.title}` : '', | ||
| `Conteúdo do site (${scraped.source === 'jina' ? 'renderizado com JS' : 'HTML estático'}):\n${scraped.markdown}`, | ||
| ].filter(Boolean).join('\n\n'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect scrapeUrl to confirm whether it always proxies via r.jina.ai or can
# fall back to a direct fetch of the user-supplied URL.
fd -t f 'web-scraper' lib/ai/utils
fd -t f 'web-scraper' | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}"'
# Also look for existing URL allow/deny helpers we should reuse.
rg -nP '\b(sanitizeUrl|isSafeUrl|isPrivateAddress|ssrf)\b' -C2Repository: thaleslaray/nossocrm
Length of output: 10975
🏁 Script executed:
cat -n lib/utils/sanitize.ts | head -60Repository: thaleslaray/nossocrm
Length of output: 2010
🏁 Script executed:
rg -n "private|ssrf|169\.254|localhost|RFC1918|192\.|172\.|10\." lib/ai lib/utils lib/query 2>/dev/null | head -20Repository: thaleslaray/nossocrm
Length of output: 232
Validate websiteUrl against SSRF before handing to scrapeUrl.
The startsWith('http') check permits http://localhost/, http://169.254.169.254/..., RFC1918 ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), and non-TLS targets. The scrapeUrl function falls back to a direct fetch(url) if the Jina proxy fails, creating a trivial SSRF vector against internal services or Vercel's runtime metadata endpoint.
Add hostname validation (blocklist private IPs, localhost, metadata endpoints, or use an allowlist) before passing to scrapeUrl.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/generate-persona/route.ts` around lines 32 - 40, The
route currently accepts websiteUrl and calls scrapeUrl when
websiteUrl?.startsWith('http')—add SSRF protection by parsing websiteUrl with
the URL constructor and validating the hostname before calling scrapeUrl: reject
localhost, 127.0.0.1, [::1], link-local and RFC1918 ranges (10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16), the 169.254.0.0/16 metadata range, and any known
cloud metadata hostnames; prefer enforcing https and/or an allowlist of domains
if applicable; perform this check where websiteUrl is used to set
scrapedWebContent so scrapeUrl is only invoked with validated, non-private
targets.
| // If duplicate (already processed), return early with success | ||
| if (eventInsertErr?.message?.toLowerCase().includes("duplicate")) { | ||
| console.log(`[Evolution] Duplicate event ignored: ${externalEventId}`); | ||
| return json(200, { ok: true, duplicate: true, event_id: externalEventId }); | ||
| } | ||
|
|
||
| if (eventInsertErr) { | ||
| // Log but don't fail — audit logging is best-effort | ||
| console.error("[Evolution] Error logging webhook event:", eventInsertErr); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the table has a unique constraint / index on (channel_id, external_event_id)
rg -nP -C2 'messaging_webhook_events' --type sql
rg -nP -C3 'CREATE\s+(UNIQUE\s+)?INDEX.*messaging_webhook_events|UNIQUE\s*\(.*external_event_id' --type sqlRepository: thaleslaray/nossocrm
Length of output: 6445
Use SQLSTATE "23505" to detect duplicate webhook events instead of substring matching.
The includes("duplicate") check is fragile — it depends on Postgres locale/wording, can match unrelated errors ("duplicate key in metadata"), and silently breaks if message phrasing changes. PostgREST returns code: "23505" for unique-constraint violations. The database already has the required UNIQUE(channel_id, external_event_id) constraint on messaging_webhook_events, so use that stable error code.
🔒 Proposed fix
- // If duplicate (already processed), return early with success
- if (eventInsertErr?.message?.toLowerCase().includes("duplicate")) {
+ // If duplicate (already processed), return early with success.
+ // 23505 = unique_violation (requires UNIQUE(channel_id, external_event_id) on messaging_webhook_events).
+ if ((eventInsertErr as { code?: string } | null)?.code === "23505") {
console.log(`[Evolution] Duplicate event ignored: ${externalEventId}`);
return json(200, { ok: true, duplicate: true, event_id: externalEventId });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/messaging-webhook-evolution/index.ts` around lines 438 -
447, The duplicate-detection logic currently checks
eventInsertErr.message.includes("duplicate") which is fragile; change it to
detect Postgres unique-violation via SQLSTATE "23505" on the error object
(eventInsertErr.code or eventInsertErr?.code) when deciding to return early as a
duplicate for messaging_webhook_events; update the branch that logs/returns
duplicate to use eventInsertErr.code === "23505" (falling back to the existing
substring check only if code is absent) and keep the existing console/error
handling for other cases, referencing eventInsertErr, externalEventId and the
UNIQUE(channel_id, external_event_id) constraint.
| } else { | ||
| // Default-deny: reject unauthenticated requests when no secret is configured | ||
| return json(401, { error: "Webhook secret não configurado para este canal" }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for a migration or script that backfills messaging_channels.credentials.webhookSecret
fd -t f -e sql 'messaging_channel|webhook.*secret' supabase/migrations 2>/dev/null
rg -nP -C3 'credentials.*webhookSecret|webhookSecret.*credentials' -g '!**/node_modules/**'Repository: thaleslaray/nossocrm
Length of output: 1638
🏁 Script executed:
# Check the migration file content
cat supabase/migrations/20260205200000_fix_messaging_channels_unique.sql
# Search for feature flags related to webhook/secret
rg -n 'DEFAULT_DENY|webhookSecret.*flag|feature.*flag.*webhook' --type-list
rg -n 'DEFAULT_DENY|webhookSecret.*flag|feature.*flag.*webhook'
# Check the full context of lines 340-350 in the zapi webhook handler
sed -n '330,360p' supabase/functions/messaging-webhook-zapi/index.tsRepository: thaleslaray/nossocrm
Length of output: 1991
🏁 Script executed:
# Search for feature flags related to webhook or default-deny behavior
rg -n 'instanceFlags|DEFAULT_DENY|webhook.*flag|feature.*webhook' supabase/functions/messaging-webhook-zapi/
# Look for any configuration or feature flag setup
fd -t f -e ts 'flag|config' supabase/ | head -20
# Check if there's a .env or constants file with webhook configuration
rg -n 'WEBHOOK_SECRET|WEBHOOK_DEFAULT'
# Search for any rollout or deprecation documentation
fd -t f \( -name '*.md' -o -name '*.txt' \) | xargs rg -l 'webhook.*secret|rollout|migration' 2>/dev/null | head -10Repository: thaleslaray/nossocrm
Length of output: 1113
🏁 Script executed:
# Search all migrations for webhookSecret backfill
rg -l 'webhookSecret' supabase/migrations/
# Check all SQL files for any backfill logic
rg 'UPDATE messaging_channels.*webhookSecret|webhookSecret.*UPDATE' supabase/migrations/
# List all migrations in order to see if any were missed
ls -la supabase/migrations/ | grep -E '\.sql$'Repository: thaleslaray/nossocrm
Length of output: 4233
Default-deny webhook secret validation is a breaking change — existing channels without configured webhookSecret will immediately start rejecting Z-API callbacks on deploy.
The code now enforces a hard 401 response when webhookSecret is null/empty (lines 347-350), but there is no corresponding migration or feature flag to backfill secrets on existing channels or to gate this behavior. Any messaging_channels row created before secret enforcement was enforced in the UI will start rejecting legitimate callbacks as soon as the Edge Function deploys.
Ensure the rollout includes one of:
- A data migration + re-registration: backfill
webhookSecretfor all active Z-API channels and re-register updated webhook URLs with Z-API; or - A feature flag (e.g., via
instanceFlags) to allow ops to gate the default-deny until the backfill completes.
Without this, you risk silent, org-scoped message loss until each affected customer manually re-saves their channel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/messaging-webhook-zapi/index.ts` around lines 347 - 350,
The new default-deny behavior returns a 401 when webhookSecret is missing (see
the branch that returns json(401, { error: "Webhook secret não configurado para
este canal" })) which will break existing channels; instead implement a rollout
guard: add a feature flag check (e.g., instanceFlags.allowWebhookEnforcement) or
a migration gate before enforcing the 401, and if the flag is false or not set,
continue to accept requests for channels with null/empty webhookSecret (preserve
the current permissive path) while you run a data migration to backfill
messaging_channels.webhookSecret and re-register Z-API webhooks; update the
authorization branch that currently returns the 401 to consult the new
flag/function (e.g., isWebhookEnforcementEnabled) or migration-complete marker
so enforcement only activates after backfill.
| DROP VIEW IF EXISTS public.vw_hitl_pending_by_age CASCADE; | ||
|
|
||
| CREATE VIEW public.vw_hitl_pending_by_age AS | ||
| SELECT | ||
| organization_id, | ||
| COUNT(*) as pending_count, | ||
| COUNT(CASE WHEN created_at > (NOW() - INTERVAL '6 hours') THEN 1 END) as age_0_6h, | ||
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '12 hours') AND (NOW() - INTERVAL '6 hours') THEN 1 END) as age_6_12h, | ||
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '24 hours') AND (NOW() - INTERVAL '12 hours') THEN 1 END) as age_12_24h, | ||
| COUNT(CASE WHEN created_at < (NOW() - INTERVAL '24 hours') THEN 1 END) as age_gt_24h, | ||
| MAX(created_at) as oldest_pending_at | ||
| FROM public.ai_pending_stage_advances | ||
| WHERE status = 'pending' | ||
| GROUP BY organization_id; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether ai_pending_stage_advances has a SELECT policy the view can rely on
rg -nP --type=sql -C3 'ai_pending_stage_advances' supabase/migrations | rg -nP -C3 '(POLICY|ENABLE ROW LEVEL)'Repository: thaleslaray/nossocrm
Length of output: 2517
Set security_invoker=true on the view to enforce RLS.
In PostgreSQL 15+, views without security_invoker=true execute with the view owner's privileges, bypassing RLS policies on the underlying table. Although ai_pending_stage_advances correctly has RLS policies configured, this view defeats those protections—any authenticated user can query aggregated counts across all organizations.
Modify the view creation to enforce the user's RLS:
Proposed fix
-CREATE VIEW public.vw_hitl_pending_by_age AS
+CREATE VIEW public.vw_hitl_pending_by_age
+WITH (security_invoker = true) AS
SELECT
organization_id,📝 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.
| DROP VIEW IF EXISTS public.vw_hitl_pending_by_age CASCADE; | |
| CREATE VIEW public.vw_hitl_pending_by_age AS | |
| SELECT | |
| organization_id, | |
| COUNT(*) as pending_count, | |
| COUNT(CASE WHEN created_at > (NOW() - INTERVAL '6 hours') THEN 1 END) as age_0_6h, | |
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '12 hours') AND (NOW() - INTERVAL '6 hours') THEN 1 END) as age_6_12h, | |
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '24 hours') AND (NOW() - INTERVAL '12 hours') THEN 1 END) as age_12_24h, | |
| COUNT(CASE WHEN created_at < (NOW() - INTERVAL '24 hours') THEN 1 END) as age_gt_24h, | |
| MAX(created_at) as oldest_pending_at | |
| FROM public.ai_pending_stage_advances | |
| WHERE status = 'pending' | |
| GROUP BY organization_id; | |
| DROP VIEW IF EXISTS public.vw_hitl_pending_by_age CASCADE; | |
| CREATE VIEW public.vw_hitl_pending_by_age | |
| WITH (security_invoker = true) AS | |
| SELECT | |
| organization_id, | |
| COUNT(*) as pending_count, | |
| COUNT(CASE WHEN created_at > (NOW() - INTERVAL '6 hours') THEN 1 END) as age_0_6h, | |
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '12 hours') AND (NOW() - INTERVAL '6 hours') THEN 1 END) as age_6_12h, | |
| COUNT(CASE WHEN created_at BETWEEN (NOW() - INTERVAL '24 hours') AND (NOW() - INTERVAL '12 hours') THEN 1 END) as age_12_24h, | |
| COUNT(CASE WHEN created_at < (NOW() - INTERVAL '24 hours') THEN 1 END) as age_gt_24h, | |
| MAX(created_at) as oldest_pending_at | |
| FROM public.ai_pending_stage_advances | |
| WHERE status = 'pending' | |
| GROUP BY organization_id; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260409120000_hitl_pending_alerts.sql` around lines 226
- 239, The view vw_hitl_pending_by_age currently executes with the view owner's
privileges and bypasses RLS on ai_pending_stage_advances; update the CREATE VIEW
statement to enforce the caller's rights by adding the security_invoker option
(use WITH (security_invoker = true) or equivalent) so queries run under the
invoking user's permissions and RLS policies on ai_pending_stage_advances are
applied.
| CREATE POLICY "board_ai_config: admin write" | ||
| ON board_ai_config FOR ALL | ||
| USING (organization_id = get_user_org_id()); |
There was a problem hiding this comment.
RLS policy missing WITH CHECK on FOR ALL.
The FOR ALL policy only specifies USING, which applies to SELECT/UPDATE/DELETE row visibility but is not enforced on INSERT and does not validate new row values on UPDATE. Without WITH CHECK, an authenticated user can insert a board_ai_config row with any organization_id (including another tenant's), or update a row to change its organization_id. This breaks tenant isolation.
Additionally, the policy is named "admin write" but does not actually restrict writes to admins — consider either enforcing the admin role check or renaming.
🛡️ Proposed fix
-CREATE POLICY "board_ai_config: admin write"
- ON board_ai_config FOR ALL
- USING (organization_id = get_user_org_id());
+CREATE POLICY "board_ai_config: org members write"
+ ON board_ai_config FOR ALL
+ USING (organization_id = get_user_org_id())
+ WITH CHECK (organization_id = get_user_org_id());As per coding guidelines: "RLS (Row Level Security) policies in Supabase must be correctly configured to enforce tenant boundaries".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20260409140000_board_ai_config.sql` around lines 51 - 53,
The RLS policy "board_ai_config: admin write" on table board_ai_config is
missing a WITH CHECK clause and does not enforce admin-only writes; update the
policy to include WITH CHECK (organization_id = get_user_org_id()) so
INSERT/UPDATE new rows are validated, and either add an admin role requirement
to both USING and WITH CHECK (e.g., include is_admin() or equivalent) to truly
restrict writes to admins, or rename the policy to reflect that it only enforces
tenant ownership if you choose not to require admin role.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ai/agent/agent.service.ts (1)
323-329:⚠️ Potential issue | 🟠 MajorAdd
organization_idfilter tostage_ai_configquery for multi-tenant security.The query at lines 324–329 filters only by
stage_idandenabled, but should also include.eq('organization_id', organizationId)since this function uses a service role client (which bypasses RLS). Thestage_ai_configtable has anorganization_idcolumn and it is available in the function context. Per coding guidelines, all database queries must filter byorganization_idwhen using service role client.Current query
const { data: stageConfig } = await supabase .from('stage_ai_config') .select('*') .eq('stage_id', deal.stage_id) .eq('enabled', true) .single();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 323 - 329, The query against stage_ai_config (the block that selects into stageConfig) currently filters only by deal.stage_id and enabled, but because this uses the service-role supabase client you must also filter by organization_id to enforce multi-tenant security; update the supabase.from('stage_ai_config').select(...).eq('stage_id', deal.stage_id).eq('enabled', true) chain to also include .eq('organization_id', organizationId) (use the existing organizationId from the function context) so the query returns only rows for that organization.
🧹 Nitpick comments (2)
supabase/functions/messaging-webhook-resend/index.ts (1)
193-210: Minor: parsed signature header could be tightened.Two small robustness notes on the Svix header parser — neither is a security issue given the HMAC check, but worth a look:
svixSignature.split(" ")will produce empty tokens if the header has double spaces / leading-trailing whitespace; they're filtered by theparts[0] !== "v1"check, but.trim().split(/\s+/)is cleaner.- Comparing base64 via
encoder.encode(expectedB64)works because base64 is ASCII, but comparing the raw signature bytes (base64-decodeprovidedB64once, then compare againstsignatureBytes) avoids the encode round-trip and makes intent clearer.Non-blocking; flagging for future cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 193 - 210, Trim and split the Svix header more robustly and compare decoded signature bytes directly: replace svixSignature.split(" ") with svixSignature.trim().split(/\s+/) when iterating providedSignatures, and instead of encoder.encode(providedB64) decode providedB64 from base64 to a Uint8Array (once) and compare that against signatureBytes (or expectedB64 decoded to signatureBytes) using timingSafeEqual; update the loop around providedB64/expectedB64, encoder, and timingSafeEqual to use the decoded byte arrays for a clearer, stronger comparison.lib/ai/agent/agent.service.ts (1)
669-690: Reasonable decoupling via queue.Moving stage evaluation to
ai_pending_evaluations(consumed by the cron) avoids the second LLM call racing the Vercel function timeout. The non-fatal error path is correct since the user-visible response has already been sent.One small suggestion: include
stage_id: deal.stage_idin the enqueued row if the worker needs it, so the worker doesn't have to re-fetch the deal to know the pre-message stage (guards against mid-flight stage transitions).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 669 - 690, Include the pre-message stage id when enqueuing the pending evaluation so the worker can use it without re-fetching the deal: in the block that inserts into supabase.from('ai_pending_evaluations') (the insert call inside the if (config.advancement_criteria... ) branch), add stage_id: deal.stage_id to the inserted object along with organization_id, conversation_id, deal_id, message_id, and message_text so the cron consumer has the pre-message stage available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/ai/board-config/generate-goal/route.ts`:
- Around line 91-93: The current promise chain using .then(({ error }) => ...)
in route.ts can produce an unhandled rejection if the Supabase HTTP call itself
rejects; update the call that currently ends with .then(({ error }: { error:
unknown }) => { if (error) console.error('[AI] log failed:', error); }); to
handle rejections by adding a .catch(...) that logs the thrown error, or
refactor to use await inside an async function wrapped in try/catch (reference
the same call site in route.ts where the Supabase log is invoked). Ensure any
added catch logs the error with context like '[AI] log failed (network):' to
avoid swallowing network/timeout failures.
- Around line 84-93: The ai_conversation_log insert using
supabase.from('ai_conversation_log').insert(...) will always fail because
action_taken is set to 'generate_goal' (not allowed by the CHECK constraint) and
conversation_id is not provided though declared NOT NULL; remove this mismatched
insert from generateGoal flow (route handler using
supabase.from('ai_conversation_log').insert) and either omit logging here or
instead write to an appropriate audit log table; if you must keep centralized
logs, create/route to a separate table that accepts admin/config events (or
update the insert to include a valid conversation_id and an allowed action_taken
value) — but the minimal fix is to drop the ai_conversation_log insert in this
route.
In `@app/api/health/route.ts`:
- Around line 84-89: The health route currently returns raw err.message into
components.database and components.ai_provider on failures, which can leak
sensitive DB/AI provider details; instead modify the catch blocks that set
components.database and components.ai_provider so they only return a generic
status (e.g., { status: 'error' } or include a short non-sensitive code) to the
public response, and send the full error to your server-side logger
(console.error or the app's logger) for debugging; update both the catch
handling that assigns components.database and the one that assigns
components.ai_provider to follow this pattern.
- Line 100: The GET and HEAD handlers use inconsistent AI key length checks: the
GET handler checks aiApiKey.length < 20 while the HEAD handler uses
aiApiKey.length > 20, causing different health results for a 20-char key; update
the HEAD handler to use the same condition as GET (or better: extract a single
constant like MIN_AI_KEY_LENGTH and use aiApiKey.length < MIN_AI_KEY_LENGTH in
both handlers) so both the GET and HEAD health checks consult the same threshold
for aiApiKey.
In `@lib/ai/agent/agent.service.ts`:
- Around line 733-737: The sanitizer is being passed the wrong metadata: change
the call to sanitizeIncomingMessage so org_id is the organizationId UUID (not
context.organization.name) and conversation_id is the actual conversationId (not
context.deal?.id); thread organizationId and conversationId from
processIncomingMessage through generateResponse (or read from the wider context
type) and update the call site where generateResponse is invoked so
sanitizeIncomingMessage receives { org_id: organizationId, conversation_id:
conversationId } instead of the current fields. Ensure the unique identifiers
referenced are organizationId, conversationId, sanitizeIncomingMessage,
processIncomingMessage, and generateResponse so reviewers can locate and verify
the fix.
In `@supabase/functions/messaging-webhook-resend/index.ts`:
- Around line 119-128: The decodeSvixSecret function can throw when atob(raw)
gets malformed input and that propagates through verifySvixSignature to produce
an unhandled 500; update decodeSvixSecret to validate/guard the base64 decode
(wrap the atob/raw→Uint8Array logic in try/catch and throw a controlled error)
and wrap the call site of verifySvixSignature (the handler around
verifySvixSignature) in a try/catch that maps any decode/verification errors to
a 401 response (use the webhookSecret variable and verifySvixSignature function
name to find the call), ensuring malformed or misconfigured secrets return 401
instead of letting the Deno.serve handler fail with 500.
- Around line 135-147: The timingSafeEqual function contains a dead branch
checking crypto.subtle.timingSafeEqual (which Deno's WebCrypto does not
provide); remove that branch from timingSafeEqual and keep the constant-time XOR
fallback, or if you want the native fast path import the real helper from
"jsr:`@std/crypto/timing-safe-equal`" and call that instead; update the function
body in timingSafeEqual to either directly perform the XOR comparison or
delegate to the imported timing-safe-equal implementation and drop the incorrect
feature-detection/comment.
---
Outside diff comments:
In `@lib/ai/agent/agent.service.ts`:
- Around line 323-329: The query against stage_ai_config (the block that selects
into stageConfig) currently filters only by deal.stage_id and enabled, but
because this uses the service-role supabase client you must also filter by
organization_id to enforce multi-tenant security; update the
supabase.from('stage_ai_config').select(...).eq('stage_id',
deal.stage_id).eq('enabled', true) chain to also include .eq('organization_id',
organizationId) (use the existing organizationId from the function context) so
the query returns only rows for that organization.
---
Nitpick comments:
In `@lib/ai/agent/agent.service.ts`:
- Around line 669-690: Include the pre-message stage id when enqueuing the
pending evaluation so the worker can use it without re-fetching the deal: in the
block that inserts into supabase.from('ai_pending_evaluations') (the insert call
inside the if (config.advancement_criteria... ) branch), add stage_id:
deal.stage_id to the inserted object along with organization_id,
conversation_id, deal_id, message_id, and message_text so the cron consumer has
the pre-message stage available.
In `@supabase/functions/messaging-webhook-resend/index.ts`:
- Around line 193-210: Trim and split the Svix header more robustly and compare
decoded signature bytes directly: replace svixSignature.split(" ") with
svixSignature.trim().split(/\s+/) when iterating providedSignatures, and instead
of encoder.encode(providedB64) decode providedB64 from base64 to a Uint8Array
(once) and compare that against signatureBytes (or expectedB64 decoded to
signatureBytes) using timingSafeEqual; update the loop around
providedB64/expectedB64, encoder, and timingSafeEqual to use the decoded byte
arrays for a clearer, stronger comparison.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c59e336-8e27-4e46-b9cb-19b7360f15c0
📒 Files selected for processing (9)
app/api/ai/board-config/[boardId]/route.tsapp/api/ai/board-config/generate-goal/route.tsapp/api/ai/board-config/generate-persona/route.tsapp/api/health/route.tslib/ai/agent/agent.service.tslib/ai/agent/input-filter.tslib/ai/utils/web-scraper.tssupabase/functions/messaging-webhook-resend/index.tssupabase/migrations/20260417160000_fix_board_ai_config_rls.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/ai/board-config/generate-persona/route.ts
- lib/ai/utils/web-scraper.ts
| void supabase.from('ai_conversation_log').insert({ | ||
| organization_id: profile.organization_id, | ||
| ai_response: text.slice(0, 1000), | ||
| tokens_used: usage?.totalTokens ?? 0, | ||
| model_used: aiConfig.model, | ||
| action_taken: 'generate_goal', | ||
| context_snapshot: {}, | ||
| }).then(({ error }: { error: unknown }) => { | ||
| if (error) console.error('[AI] log failed:', error); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm schema constraints for ai_conversation_log.
rg -nP --type=sql -C2 'ai_conversation_log|action_taken\s+TEXT\s+CHECK' supabase/migrationsRepository: thaleslaray/nossocrm
Length of output: 5273
🏁 Script executed:
#!/bin/bash
# Read the full April 9 migration to see what was changed
cat supabase/migrations/20260409100000_fix_ai_conversation_log_action_taken.sqlRepository: thaleslaray/nossocrm
Length of output: 1060
🏁 Script executed:
#!/bin/bash
# Also read the initial creation to see the original schema and if conversation_id is NOT NULL
grep -A 30 "CREATE TABLE ai_conversation_log" supabase/migrations/20260206200000_create_stage_ai_config.sqlRepository: thaleslaray/nossocrm
Length of output: 1172
Critical: this ai_conversation_log insert will fail on every call.
Two schema violations will make every insert error out (silently, since this is void + .then):
action_taken: 'generate_goal'violates theCHECKconstraint on the column. The constraint allows only'responded' | 'advanced_stage' | 'handoff' | 'skipped' | 'stage_evaluation'—'generate_goal'is not among these values.conversation_idis missing, but the column is declaredUUID NOT NULL REFERENCES messaging_conversations(id). There is no conversation in this flow (it's admin-invoked config generation), so there's no valid FK to supply.
Net effect: the PR-advertised observability ("Centralized logging to ai_conversation_log for generateText paths") silently does nothing for this route, and console.error('[AI] log failed:', error) will fire on every request.
Options:
- Drop this insert (logging a config-generation event in a conversation log is a conceptual mismatch anyway), or
- Route it to a dedicated audit log table, or
- Relax both constraints in a migration (add
'generate_goal'to the CHECK, makeconversation_idnullable) — but that weakens the schema's invariants for the real conversation path.
🛡️ Minimal fix — drop the mismatched insert
- void supabase.from('ai_conversation_log').insert({
- organization_id: profile.organization_id,
- ai_response: text.slice(0, 1000),
- tokens_used: usage?.totalTokens ?? 0,
- model_used: aiConfig.model,
- action_taken: 'generate_goal',
- context_snapshot: {},
- }).then(({ error }: { error: unknown }) => {
- if (error) console.error('[AI] log failed:', error);
- });
+ // TODO: log to a dedicated ai_admin_audit table (ai_conversation_log is
+ // conversation-scoped and its CHECK/NOT NULL constraints disallow this path).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/generate-goal/route.ts` around lines 84 - 93, The
ai_conversation_log insert using
supabase.from('ai_conversation_log').insert(...) will always fail because
action_taken is set to 'generate_goal' (not allowed by the CHECK constraint) and
conversation_id is not provided though declared NOT NULL; remove this mismatched
insert from generateGoal flow (route handler using
supabase.from('ai_conversation_log').insert) and either omit logging here or
instead write to an appropriate audit log table; if you must keep centralized
logs, create/route to a separate table that accepts admin/config events (or
update the insert to include a valid conversation_id and an allowed action_taken
value) — but the minimal fix is to drop the ai_conversation_log insert in this
route.
| }).then(({ error }: { error: unknown }) => { | ||
| if (error) console.error('[AI] log failed:', error); | ||
| }); |
There was a problem hiding this comment.
Minor: add .catch to avoid unhandled rejection on network failure.
.then(({ error }) => ...) only handles the resolved-with-error case. If the Supabase HTTP call itself rejects (network blip, timeout), the promise rejects unhandled (since void discards the reference). Chain a .catch or prefer await wrapped in a try/catch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/generate-goal/route.ts` around lines 91 - 93, The
current promise chain using .then(({ error }) => ...) in route.ts can produce an
unhandled rejection if the Supabase HTTP call itself rejects; update the call
that currently ends with .then(({ error }: { error: unknown }) => { if (error)
console.error('[AI] log failed:', error); }); to handle rejections by adding a
.catch(...) that logs the thrown error, or refactor to use await inside an async
function wrapped in try/catch (reference the same call site in route.ts where
the Supabase log is invoked). Ensure any added catch logs the error with context
like '[AI] log failed (network):' to avoid swallowing network/timeout failures.
| } catch (err) { | ||
| components.database = { | ||
| status: 'error', | ||
| error: err instanceof Error ? err.message : 'Unknown database error', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Avoid echoing raw error messages from a public, unauthenticated endpoint.
/api/health has no auth gate, and components.database.error / components.ai_provider.error forward raw err.message (including Supabase/Postgres error strings) to any caller. That can leak schema names, RLS hints, or configuration details. Consider returning a generic status ('error' without the message, or a short code) in the public response and logging the full error server-side.
Also applies to: 108-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/health/route.ts` around lines 84 - 89, The health route currently
returns raw err.message into components.database and components.ai_provider on
failures, which can leak sensitive DB/AI provider details; instead modify the
catch blocks that set components.database and components.ai_provider so they
only return a generic status (e.g., { status: 'error' } or include a short
non-sensitive code) to the public response, and send the full error to your
server-side logger (console.error or the app's logger) for debugging; update
both the catch handling that assigns components.database and the one that
assigns components.ai_provider to follow this pattern.
| } | ||
|
|
||
| // Validate API key format (basic check) | ||
| if (aiApiKey.length < 20) { |
There was a problem hiding this comment.
Inconsistent AI key length threshold between GET and HEAD.
GET treats the key as invalid when aiApiKey.length < 20 (length 20 passes), while HEAD requires aiApiKey.length > 20 (length 20 fails). The same configuration will therefore report healthy on GET but 503 on HEAD, which will confuse uptime probes that use HEAD.
🛡️ Proposed fix
- const aiConfigured = !!(aiApiKey && aiApiKey.length > 20);
+ const aiConfigured = !!(aiApiKey && aiApiKey.length >= 20);Also applies to: 162-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/health/route.ts` at line 100, The GET and HEAD handlers use
inconsistent AI key length checks: the GET handler checks aiApiKey.length < 20
while the HEAD handler uses aiApiKey.length > 20, causing different health
results for a 20-char key; update the HEAD handler to use the same condition as
GET (or better: extract a single constant like MIN_AI_KEY_LENGTH and use
aiApiKey.length < MIN_AI_KEY_LENGTH in both handlers) so both the GET and HEAD
health checks consult the same threshold for aiApiKey.
| // Sanitize incoming message to neutralize prompt injection attempts | ||
| const sanitized = sanitizeIncomingMessage(incomingMessage, { | ||
| org_id: context.organization.name, | ||
| conversation_id: context.deal?.id, | ||
| }); |
There was a problem hiding this comment.
Sanitizer metadata has wrong semantics — hurts audit traceability.
org_id: context.organization.name— this logs the org name, not the UUID. The structured-logger's typed field isorg_id; mixing names and IDs makes injection-incident triage harder (and across orgs with identical/similar names, ambiguous).conversation_id: context.deal?.id— this is the deal id, not the conversation id. The calling function already has the trueconversationIdin scope viaProcessMessageParams.
Thread organizationId and conversationId from processIncomingMessage down into generateResponse (or read them off a wider context type) and pass them here.
🛠️ Proposed fix sketch
interface GenerateResponseParams {
context: LeadContext;
stageConfig: StageAIConfig;
incomingMessage: string;
aiConfig: OrgAIConfig;
boardAIConfig: BoardAIConfig | null;
+ organizationId: string;
+ conversationId: string;
}
…
- const sanitized = sanitizeIncomingMessage(incomingMessage, {
- org_id: context.organization.name,
- conversation_id: context.deal?.id,
- });
+ const sanitized = sanitizeIncomingMessage(incomingMessage, {
+ org_id: organizationId,
+ conversation_id: conversationId,
+ });And pass them at the call site around L525.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ai/agent/agent.service.ts` around lines 733 - 737, The sanitizer is being
passed the wrong metadata: change the call to sanitizeIncomingMessage so org_id
is the organizationId UUID (not context.organization.name) and conversation_id
is the actual conversationId (not context.deal?.id); thread organizationId and
conversationId from processIncomingMessage through generateResponse (or read
from the wider context type) and update the call site where generateResponse is
invoked so sanitizeIncomingMessage receives { org_id: organizationId,
conversation_id: conversationId } instead of the current fields. Ensure the
unique identifiers referenced are organizationId, conversationId,
sanitizeIncomingMessage, processIncomingMessage, and generateResponse so
reviewers can locate and verify the fix.
| function decodeSvixSecret(secret: string): Uint8Array { | ||
| const raw = secret.startsWith("whsec_") ? secret.slice(6) : secret; | ||
| // Deno has atob built-in; convert base64 → Uint8Array | ||
| const binaryStr = atob(raw); | ||
| const bytes = new Uint8Array(binaryStr.length); | ||
| for (let i = 0; i < binaryStr.length; i++) { | ||
| bytes[i] = binaryStr.charCodeAt(i); | ||
| } | ||
| return bytes; | ||
| } |
There was a problem hiding this comment.
atob in decodeSvixSecret can throw on a malformed secret and surface as an unhandled 500.
verifySvixSignature is not wrapped in try/catch at its call site (line 282). If webhookSecret was stored without the expected whsec_<base64> format (e.g. partial migration, hand-edited credential), atob(raw) throws InvalidCharacterError and the Deno.serve handler rejects with 500 — Svix/Resend will then retry indefinitely against a channel that can never authenticate. This is a misconfiguration path, but webhook auth errors should deterministically return 401 so the source stops retrying.
🛡️ Proposed fix
- const isValid = await verifySvixSignature(rawBody, { svixId, svixTimestamp, svixSignature }, webhookSecret);
- if (!isValid) {
+ let isValid = false;
+ try {
+ isValid = await verifySvixSignature(rawBody, { svixId, svixTimestamp, svixSignature }, webhookSecret);
+ } catch (e) {
+ console.warn(`[Webhook/Resend] Signature verification threw for channel ${channelId}:`, e);
+ }
+ if (!isValid) {
console.warn(`[Webhook/Resend] Invalid Svix signature for channel ${channelId}`);
return json(401, { error: "Assinatura Svix inválida" });
}Also applies to: 282-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 119 - 128,
The decodeSvixSecret function can throw when atob(raw) gets malformed input and
that propagates through verifySvixSignature to produce an unhandled 500; update
decodeSvixSecret to validate/guard the base64 decode (wrap the
atob/raw→Uint8Array logic in try/catch and throw a controlled error) and wrap
the call site of verifySvixSignature (the handler around verifySvixSignature) in
a try/catch that maps any decode/verification errors to a 401 response (use the
webhookSecret variable and verifySvixSignature function name to find the call),
ensuring malformed or misconfigured secrets return 401 instead of letting the
Deno.serve handler fail with 500.
| async function timingSafeEqual(a: Uint8Array, b: Uint8Array): Promise<boolean> { | ||
| if (a.length !== b.length) return false; | ||
| // Deno exposes crypto.subtle.timingSafeEqual since 1.38 | ||
| if (typeof (crypto.subtle as Record<string, unknown>).timingSafeEqual === "function") { | ||
| return (crypto.subtle as unknown as { timingSafeEqual: (a: BufferSource, b: BufferSource) => boolean }).timingSafeEqual(a, b); | ||
| } | ||
| // Fallback: constant-time XOR comparison | ||
| let diff = 0; | ||
| for (let i = 0; i < a.length; i++) { | ||
| diff |= a[i] ^ b[i]; | ||
| } | ||
| return diff === 0; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is crypto.subtle.timingSafeEqual part of Deno's runtime global WebCrypto (not @std/crypto) in 2026?
💡 Result:
No, crypto.subtle.timingSafeEqual is not part of Deno's runtime global WebCrypto (crypto.subtle). Deno's Web Crypto API implements the standard SubtleCrypto interface, which does not include timingSafeEqual as a method. Official Deno documentation for SubtleCrypto lists standard methods like generateKey, digest, encrypt/decrypt, sign/verify, etc., but no timingSafeEqual. The function timingSafeEqual is available in Deno's standard library module @std/crypto (not the runtime global), as a standalone function for constant-time comparisons. It was temporarily provided as an extension on std/crypto.subtle but deprecated and removed from std because it's not part of the Web Crypto standard. Node.js compatibility provides crypto.timingSafeEqual (not on subtle), but the question specifies the runtime global WebCrypto, which remains standard WebCrypto without this extension even in 2026.
Citations:
- 1: https://docs.deno.com/api/web/~/SubtleCrypto
- 2: https://docs.deno.com/api/web/crypto
- 3: https://doc.deno.land/deno/dom/~/SubtleCrypto
- 4: timingSafeEqual is provided from 2 places denoland/std#3541
- 5: https://jsr.io/@std/crypto/doc/~/timingSafeEqual
Remove the dead crypto.subtle.timingSafeEqual branch — this method is not part of Deno's runtime global WebCrypto.
Deno's standard WebCrypto API does not expose timingSafeEqual on crypto.subtle. The feature-detection check will always fail, making the fast path unreachable; only the fallback XOR comparison runs. The comment claiming "Deno 1.38+" support is incorrect. The fallback implementation is correct and constant-time, so either remove the dead branch or import timingSafeEqual from jsr:@std/crypto/timing-safe-equal`` if the faster path is desired.
Suggested cleanup
-async function timingSafeEqual(a: Uint8Array, b: Uint8Array): Promise<boolean> {
- if (a.length !== b.length) return false;
- // Deno exposes crypto.subtle.timingSafeEqual since 1.38
- if (typeof (crypto.subtle as Record<string, unknown>).timingSafeEqual === "function") {
- return (crypto.subtle as unknown as { timingSafeEqual: (a: BufferSource, b: BufferSource) => boolean }).timingSafeEqual(a, b);
- }
- // Fallback: constant-time XOR comparison
- let diff = 0;
- for (let i = 0; i < a.length; i++) {
- diff |= a[i] ^ b[i];
- }
- return diff === 0;
-}
+function timingSafeEqual(a: Uint8Array, b: Uint8Array): boolean {
+ if (a.length !== b.length) return false;
+ let diff = 0;
+ for (let i = 0; i < a.length; i++) diff |= a[i] ^ b[i];
+ return diff === 0;
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 135 - 147,
The timingSafeEqual function contains a dead branch checking
crypto.subtle.timingSafeEqual (which Deno's WebCrypto does not provide); remove
that branch from timingSafeEqual and keep the constant-time XOR fallback, or if
you want the native fast path import the real helper from
"jsr:`@std/crypto/timing-safe-equal`" and call that instead; update the function
body in timingSafeEqual to either directly perform the XOR comparison or
delegate to the imported timing-safe-equal implementation and drop the incorrect
feature-detection/comment.
7 vulnerabilidades corrigidas via ultrareview: - input-filter: flag `g` em 26 regex + reset lastIndex (regex stateful) - web-scraper: SSRF — validateScrapableUrl() exige https, bloqueia IPs privados RFC1918/loopback/link-local - board-config PUT: verifica board ownership antes do upsert (cross-tenant poisoning) - board_ai_config RLS: migration com subquery EXISTS para ownership - generate-goal/persona: role check admin, sanitizeIncomingMessage, SECURITY_PREAMBLE, ai_conversation_log - agent.service: logAIInteraction em sendResult failure + incrementCircuitBreaker em LLM error - webhook-resend: default-deny sem webhookSecret (era default-allow) - health: PUBLISHABLE_KEY || ANON_KEY fallback em GET e HEAD Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cbe66f6 to
a9d4833
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ai/agent/agent.service.ts (1)
1260-1271:⚠️ Potential issue | 🟠 MajorAdd
organizationIdparameter and verify conversation scope ingetConversationHistory.
getConversationHistoryqueries messages byconversationIdalone without explicitorganizationIdfiltering. Sincemessaging_messageshas noorganization_idcolumn, verify the conversation belongs to the target organization first, then fetch its messages.🛡️ Proposed API direction
export async function getConversationHistory( supabase: SupabaseClient, + organizationId: string, conversationId: string, limit: number = 10 ): Promise<Array<{ role: 'user' | 'assistant'; content: string }>> { + // Verify conversation belongs to organizationId + const { data: conversation } = await supabase + .from('messaging_conversations') + .select('id') + .eq('id', conversationId) + .eq('organization_id', organizationId) + .maybeSingle(); + + if (!conversation) { + return []; + } + const { data: messages } = await supabase .from('messaging_messages') .select('direction, content, created_at') .eq('conversation_id', conversationId)Update the call site in
app/api/cron/stage-evaluations/route.tsto passorganization_id:- getConversationHistory(supabase, conversation_id), + getConversationHistory(supabase, organization_id, conversation_id),Per coding guidelines: "All database queries must filter by
organization_idfor multi-tenant security." Per learnings: "Service role queries must always includeorganization_idfilter."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 1260 - 1271, Add an organizationId parameter to getConversationHistory and enforce tenant scoping: first query the conversations table (e.g., messaging_conversations) by conversationId and organization_id to verify the conversation belongs to the given organization, throw/return early if not found, then fetch messages from messaging_messages as before; update the getConversationHistory function signature and its call site in app/api/cron/stage-evaluations/route.ts to pass organization_id and ensure all Supabase service-role queries include the organizationId filter.
♻️ Duplicate comments (5)
supabase/functions/messaging-webhook-resend/index.ts (1)
119-128:⚠️ Potential issue | 🟡 Minor
atobon a malformedwhsec_secret still throws and surfaces as HTTP 500.
decodeSvixSecretinvokesatob(raw)without guarding against non-base64 input, and the call site at lines 282–286 does not wrapverifySvixSignatureintry/catch. A misconfigured credential (e.g. stored without proper base64 padding, or missing thewhsec_prefix with non-base64 chars) will raiseInvalidCharacterError, propagate out of theDeno.servehandler, and cause Svix/Resend to retry indefinitely. Webhook auth errors should deterministically return 401.🛡️ Proposed fix
function decodeSvixSecret(secret: string): Uint8Array { const raw = secret.startsWith("whsec_") ? secret.slice(6) : secret; - // Deno has atob built-in; convert base64 → Uint8Array - const binaryStr = atob(raw); + let binaryStr: string; + try { + binaryStr = atob(raw); + } catch { + throw new Error("Invalid Svix secret: not valid base64"); + } const bytes = new Uint8Array(binaryStr.length); for (let i = 0; i < binaryStr.length; i++) { bytes[i] = binaryStr.charCodeAt(i); } return bytes; }And at the call site (around line 282):
- const isValid = await verifySvixSignature(rawBody, { svixId, svixTimestamp, svixSignature }, webhookSecret); - if (!isValid) { + let isValid = false; + try { + isValid = await verifySvixSignature(rawBody, { svixId, svixTimestamp, svixSignature }, webhookSecret); + } catch (e) { + console.warn(`[Webhook/Resend] Signature verification threw for channel ${channelId}:`, e); + } + if (!isValid) { console.warn(`[Webhook/Resend] Invalid Svix signature for channel ${channelId}`); return json(401, { error: "Assinatura Svix inválida" }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/messaging-webhook-resend/index.ts` around lines 119 - 128, decodeSvixSecret currently calls atob(raw) without validating base64 which can throw InvalidCharacterError and bubble up to the Deno.serve handler; update decodeSvixSecret to validate/try to decode safely (catch errors from atob or detect non-base64 characters/padding) and return a failure indicator (e.g., null or throw a custom error), and wrap the call site that invokes verifySvixSignature inside a try/catch in the Deno.serve webhook handler so any decoding/verification errors result in an immediate 401 response instead of propagating a 500; specifically modify decodeSvixSecret and the place that calls verifySvixSignature to handle invalid secrets deterministically and log the error while returning 401.lib/ai/agent/agent.service.ts (2)
711-737:⚠️ Potential issue | 🟡 MinorThread real IDs into sanitizer metadata.
This is still using
context.organization.nameasorg_idandcontext.deal?.idasconversation_id, which makes input-filter audit logs ambiguous. AddorganizationIdandconversationIdtoGenerateResponseParamsand pass those UUIDs tosanitizeIncomingMessage.🛠️ Proposed metadata fix
interface GenerateResponseParams { context: LeadContext; stageConfig: StageAIConfig; incomingMessage: string; aiConfig: OrgAIConfig; boardAIConfig: BoardAIConfig | null; + organizationId: string; + conversationId: string; } async function generateResponse(params: GenerateResponseParams): Promise<AgentDecision> { - const { context, stageConfig, incomingMessage, aiConfig, boardAIConfig } = params; + const { context, stageConfig, incomingMessage, aiConfig, boardAIConfig, organizationId, conversationId } = params; @@ const sanitized = sanitizeIncomingMessage(incomingMessage, { - org_id: context.organization.name, - conversation_id: context.deal?.id, + org_id: organizationId, + conversation_id: conversationId, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 711 - 737, The sanitizer is being fed ambiguous human-readable IDs (context.organization.name and context.deal?.id); update the GenerateResponseParams type to include organizationId (UUID) and conversationId (UUID), update generateResponse to accept those new fields, and pass organizationId and conversationId into sanitizeIncomingMessage instead of context.organization.name and context.deal?.id; ensure any callers of generateResponse (and the type) are updated to supply the UUIDs so the sanitizer metadata is unambiguous.
277-297:⚠️ Potential issue | 🟠 MajorAdd tenant filters to the deal and board AI config lookups.
This unresolved issue still applies: the deal fetch uses only
dealId, andgetBoardAIConfigis called with onlyboardId. Ensure both paths enforceorganization_idexplicitly, including updatinggetBoardAIConfigto accept and filter byorganizationId.🛡️ Proposed call-site direction
const { data: deal } = await supabase .from('deals') .select('id, stage_id, board_id') .eq('id', dealId) + .eq('organization_id', organizationId) .single(); @@ - boardAIConfig = await getBoardAIConfig(supabase, deal.board_id); + boardAIConfig = await getBoardAIConfig(supabase, organizationId, deal.board_id);Based on learnings, “for service role queries, ensure
organization_idfiltering is always present.” As per coding guidelines, “Service role queries in AI tools must always includeorganization_idfilter for tenant isolation.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 277 - 297, The deal lookup and board AI config retrieval lack tenant isolation; update the supabase query that fetches the deal (the .from('deals').select(...).eq('id', dealId).single() call) to also filter by organization_id (eq('organization_id', organizationId)) and propagate organizationId into getBoardAIConfig by changing its signature to accept organizationId and making it apply the same organization_id filter when querying board_ai_config; update the call site here (where boardAIConfig = await getBoardAIConfig(supabase, deal.board_id)) to pass the deal's/handler's organizationId and ensure BoardAIConfig remains typed as BoardAIConfig | null.app/api/ai/board-config/generate-goal/route.ts (2)
84-93:⚠️ Potential issue | 🔴 CriticalThis conversation-log insert is still schema-incompatible.
This remains unresolved:
action_taken: 'generate_goal'does not match the knownai_conversation_logaction constraint, and this admin/config route has noconversation_idto satisfy the conversation-scoped log row. Route this to an admin audit table or remove this insert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/ai/board-config/generate-goal/route.ts` around lines 84 - 93, The insert into ai_conversation_log (supabase.from('ai_conversation_log').insert({...}) in route.ts) is schema-incompatible because action_taken 'generate_goal' isn't an allowed enum and this admin/config route lacks a conversation_id; fix by removing this insert or replacing it with an insert into an admin/audit table (e.g., admin_audit_log) that matches your schema, or if you must log to ai_conversation_log, set action_taken to a valid enum value and provide a conversation_id (or null if the schema allows) and ensure context_snapshot matches the column type; update the supabase.from call and payload keys accordingly to match the target table schema.
63-99:⚠️ Potential issue | 🟠 MajorUse AI SDK v6 structured output instead of manual JSON parsing.
The current implementation requests JSON and manually parses free-form text with fence stripping (lines 95–97). This violates the coding guideline and is inconsistent with 15+ established examples throughout the codebase. Use
generateTextwithoutput: Output.object({ schema })and returnresult.output.🛠️ Proposed structured-output refactor
import { NextRequest, NextResponse } from 'next/server'; import { createClient } from '@/lib/supabase/server'; import { getOrgAIConfig, SECURITY_PREAMBLE } from '@/lib/ai/agent/agent.service'; import { sanitizeIncomingMessage } from '@/lib/ai/agent/input-filter'; import { getModel } from '@/lib/ai/config'; -import { generateText } from 'ai'; +import { generateText, Output } from 'ai'; +import { z } from 'zod'; export const maxDuration = 20; +const GoalSuggestionSchema = z.object({ + whatToDo: z.string().min(1), + whatNotToDo: z.string().min(1), +}); + const CATEGORY_LABELS: Record<string, string> = { qualificacao: 'Qualificação de Leads', agendamento: 'Agendamento', @@ -60,7 +68,8 @@ const { text: safeContext } = sanitizeIncomingMessage(businessContext, { org_id: profile.organization_id }); try { - const { text, usage } = await generateText({ + const result = await generateText({ model, system: SECURITY_PREAMBLE, prompt: `Você é um especialista em configurar agentes de IA para vendas. @@ -80,18 +89,13 @@ Seja específico ao negócio. Mencione elementos reais do contexto (tipo de cliente, produto, serviço). Retorne APENAS o JSON, sem markdown ou explicações.`, + output: Output.object({ schema: GoalSuggestionSchema }), }); void supabase.from('ai_conversation_log').insert({ organization_id: profile.organization_id, - ai_response: text.slice(0, 1000), - tokens_used: usage?.totalTokens ?? 0, + ai_response: JSON.stringify(result.output).slice(0, 1000), + tokens_used: result.usage?.totalTokens ?? 0, model_used: aiConfig.model, action_taken: 'generate_goal', context_snapshot: {}, }).then(({ error }: { error: unknown }) => { if (error) console.error('[AI] log failed:', error); }); - // Parse JSON response - const cleaned = text.trim().replace(/^```json\s*/i, '').replace(/```\s*$/, '').trim(); - const result = JSON.parse(cleaned) as { whatToDo: string; whatNotToDo: string }; - - return NextResponse.json(result); + return NextResponse.json(result.output);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/ai/board-config/generate-goal/route.ts` around lines 63 - 99, Replace the manual free-form JSON parsing after generateText with the SDK's structured output feature: modify the generateText call (in this file's generate-goal route) to pass output: Output.object({ whatToDo: z.string(), whatNotToDo: z.string() }) (or equivalent schema object) so the SDK returns a parsed result object, remove the fence-stripping and JSON.parse logic, and return NextResponse.json(result.output) instead; keep the existing logging to supabase.from('ai_conversation_log') and SECURITY_PREAMBLE/model usage unchanged.
🧹 Nitpick comments (2)
lib/ai/agent/input-filter.ts (1)
11-11: Use the@/alias for this import.This new module should follow the repository-wide alias convention.
♻️ Proposed import cleanup
-import { logStructured } from './structured-logger'; +import { logStructured } from '@/lib/ai/agent/structured-logger';As per coding guidelines, “All imports must use
@/alias instead of relative paths.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/input-filter.ts` at line 11, Replace the relative import of logStructured from './structured-logger' in input-filter.ts with the repository alias import that starts with '@/'; specifically update the import specifier that provides the logStructured symbol so it uses the '@/...' alias path matching the module's location (e.g. the aliased path to the structured-logger module) while keeping the imported name logStructured unchanged.lib/ai/agent/agent.service.ts (1)
14-46: Convert the new local imports to@/aliases.The newly added AI helper imports should follow the repository import convention.
♻️ Proposed import cleanup
-import { checkConversationRateLimit } from './rate-limiter'; +import { checkConversationRateLimit } from '@/lib/ai/agent/rate-limiter'; @@ -import { +import { logStructured, logAIError, logAIResponse, logRateLimit, logTokenBudgetExceeded, logHandoff, logAIInitError, -} from './structured-logger'; -import { sanitizeIncomingMessage } from './input-filter'; -import { validateAIOutput } from './output-validator'; +} from '@/lib/ai/agent/structured-logger'; +import { sanitizeIncomingMessage } from '@/lib/ai/agent/input-filter'; +import { validateAIOutput } from '@/lib/ai/agent/output-validator';As per coding guidelines, “All imports must use
@/alias instead of relative paths.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ai/agent/agent.service.ts` around lines 14 - 46, The new AI helper imports use relative paths; update their module specifiers to the repository alias (e.g., "@/...") so they follow the import convention. Specifically change the imports that bring in checkConversationRateLimit, checkTokenBudget, buildLeadContext, formatContextForPrompt, buildConversationalPromptFromPatterns, the LearnedPattern type, the StageAIConfig/LeadContext/AgentDecision/AgentProcessResult types, the structured-logger functions (logStructured, logAIError, etc.), sanitizeIncomingMessage, validateAIOutput, and extractAndUpdateBANT to use "@/..." aliases that map to the appropriate lib/ai or extraction modules in the codebase. Ensure only the specifiers change (not the imported symbols) so the rest of agent.service.ts continues to reference the same function/type names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/ai/board-config/`[boardId]/route.ts:
- Around line 73-84: Wrap the request.json() call in a try/catch and return a
400 on parse error, then validate that the parsed body is a plain object (typeof
body === 'object' && body !== null && !Array.isArray(body)) and return 400 if
not; after that safely copy allowedFields into payload by checking ownership
with Object.prototype.hasOwnProperty.call(body, field) (or equivalent) instead
of using `field in body` so arrays/primitives/null won't be accepted and
malformed JSON won't crash the route handling for
boardId/profile.organization_id processing.
In `@app/api/ai/board-config/generate-goal/route.ts`:
- Around line 59-60: categoryLabel currently falls back to the raw category
value which allows an attacker to inject arbitrary prompt text; change the logic
that computes categoryLabel (using CATEGORY_LABELS and the variable
categoryLabel) to reject unknown categories rather than interpolating them —
validate that category exists as a key in CATEGORY_LABELS and throw or return a
400 error if not, or else map to a fixed safe default token (e.g., "Unknown")
before including it in the prompt; do the same validation where categoryLabel is
used later (lines around the block between the current compute and lines 66-71)
and avoid relying on sanitizeIncomingMessage alone for category values.
In `@app/api/health/route.ts`:
- Around line 68-78: The health probe is querying the tenant-protected
organizations table with the public/anon client (the supabase
.from('organizations') .select(...).maybeSingle() call) which RLS will block;
replace this with a dedicated health mechanism: either call a lightweight RPC or
read a small permissively-policied table designed for anon health checks, or
switch to the server/service-role client reading a cookie-less permissive table,
and stop probing tenant data. Also avoid .maybeSingle() here (use
.select(...).limit(1) or a non-throwing RPC) so the probe doesn't throw on "no
rows"; update the code paths that reference data, error, dbLatency and
dbStartTime accordingly to use the new health endpoint/table instead of
organizations.
In `@lib/ai/agent/agent.service.ts`:
- Around line 299-321: The circuit-breaker helpers are missing tenant scoping:
thread the organizationId from the call site into getCircuitBreakerState,
incrementCircuitBreakerError, and resetCircuitBreaker, and update those helper
implementations to include organization_id filters on any
messaging_conversations/contacts queries and updates (and any writes to the
circuit-breaker table) so all reads/updates are scoped by the tenant; ensure the
call signatures and call sites (e.g., where getCircuitBreakerState is invoked)
pass organizationId and that the helper functions use it in WHERE clauses for
service-role DB queries.
- Around line 210-214: The DB-backed rate limiter is currently scoped only by
conversationId; update the call in the handler to pass organizationId into
checkConversationRateLimit (e.g., change checkConversationRateLimit(supabase,
conversationId) to checkConversationRateLimit(supabase, conversationId,
organizationId)) and then modify the helper implementation to include an
additional filter .eq('organization_id', organizationId) alongside
.eq('conversation_id', conversationId) so all service-role queries are always
tenant-scoped; keep parameter and call-site names consistent with the existing
checkConversationRateLimit function and update any callers/tests accordingly.
---
Outside diff comments:
In `@lib/ai/agent/agent.service.ts`:
- Around line 1260-1271: Add an organizationId parameter to
getConversationHistory and enforce tenant scoping: first query the conversations
table (e.g., messaging_conversations) by conversationId and organization_id to
verify the conversation belongs to the given organization, throw/return early if
not found, then fetch messages from messaging_messages as before; update the
getConversationHistory function signature and its call site in
app/api/cron/stage-evaluations/route.ts to pass organization_id and ensure all
Supabase service-role queries include the organizationId filter.
---
Duplicate comments:
In `@app/api/ai/board-config/generate-goal/route.ts`:
- Around line 84-93: The insert into ai_conversation_log
(supabase.from('ai_conversation_log').insert({...}) in route.ts) is
schema-incompatible because action_taken 'generate_goal' isn't an allowed enum
and this admin/config route lacks a conversation_id; fix by removing this insert
or replacing it with an insert into an admin/audit table (e.g., admin_audit_log)
that matches your schema, or if you must log to ai_conversation_log, set
action_taken to a valid enum value and provide a conversation_id (or null if the
schema allows) and ensure context_snapshot matches the column type; update the
supabase.from call and payload keys accordingly to match the target table
schema.
- Around line 63-99: Replace the manual free-form JSON parsing after
generateText with the SDK's structured output feature: modify the generateText
call (in this file's generate-goal route) to pass output: Output.object({
whatToDo: z.string(), whatNotToDo: z.string() }) (or equivalent schema object)
so the SDK returns a parsed result object, remove the fence-stripping and
JSON.parse logic, and return NextResponse.json(result.output) instead; keep the
existing logging to supabase.from('ai_conversation_log') and
SECURITY_PREAMBLE/model usage unchanged.
In `@lib/ai/agent/agent.service.ts`:
- Around line 711-737: The sanitizer is being fed ambiguous human-readable IDs
(context.organization.name and context.deal?.id); update the
GenerateResponseParams type to include organizationId (UUID) and conversationId
(UUID), update generateResponse to accept those new fields, and pass
organizationId and conversationId into sanitizeIncomingMessage instead of
context.organization.name and context.deal?.id; ensure any callers of
generateResponse (and the type) are updated to supply the UUIDs so the sanitizer
metadata is unambiguous.
- Around line 277-297: The deal lookup and board AI config retrieval lack tenant
isolation; update the supabase query that fetches the deal (the
.from('deals').select(...).eq('id', dealId).single() call) to also filter by
organization_id (eq('organization_id', organizationId)) and propagate
organizationId into getBoardAIConfig by changing its signature to accept
organizationId and making it apply the same organization_id filter when querying
board_ai_config; update the call site here (where boardAIConfig = await
getBoardAIConfig(supabase, deal.board_id)) to pass the deal's/handler's
organizationId and ensure BoardAIConfig remains typed as BoardAIConfig | null.
In `@supabase/functions/messaging-webhook-resend/index.ts`:
- Around line 119-128: decodeSvixSecret currently calls atob(raw) without
validating base64 which can throw InvalidCharacterError and bubble up to the
Deno.serve handler; update decodeSvixSecret to validate/try to decode safely
(catch errors from atob or detect non-base64 characters/padding) and return a
failure indicator (e.g., null or throw a custom error), and wrap the call site
that invokes verifySvixSignature inside a try/catch in the Deno.serve webhook
handler so any decoding/verification errors result in an immediate 401 response
instead of propagating a 500; specifically modify decodeSvixSecret and the place
that calls verifySvixSignature to handle invalid secrets deterministically and
log the error while returning 401.
---
Nitpick comments:
In `@lib/ai/agent/agent.service.ts`:
- Around line 14-46: The new AI helper imports use relative paths; update their
module specifiers to the repository alias (e.g., "@/...") so they follow the
import convention. Specifically change the imports that bring in
checkConversationRateLimit, checkTokenBudget, buildLeadContext,
formatContextForPrompt, buildConversationalPromptFromPatterns, the
LearnedPattern type, the
StageAIConfig/LeadContext/AgentDecision/AgentProcessResult types, the
structured-logger functions (logStructured, logAIError, etc.),
sanitizeIncomingMessage, validateAIOutput, and extractAndUpdateBANT to use
"@/..." aliases that map to the appropriate lib/ai or extraction modules in the
codebase. Ensure only the specifiers change (not the imported symbols) so the
rest of agent.service.ts continues to reference the same function/type names.
In `@lib/ai/agent/input-filter.ts`:
- Line 11: Replace the relative import of logStructured from
'./structured-logger' in input-filter.ts with the repository alias import that
starts with '@/'; specifically update the import specifier that provides the
logStructured symbol so it uses the '@/...' alias path matching the module's
location (e.g. the aliased path to the structured-logger module) while keeping
the imported name logStructured unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86e1c1fc-09d8-44cc-bf58-d23393dd213e
📒 Files selected for processing (10)
.github/workflows/ci.ymlapp/api/ai/board-config/[boardId]/route.tsapp/api/ai/board-config/generate-goal/route.tsapp/api/ai/board-config/generate-persona/route.tsapp/api/health/route.tslib/ai/agent/agent.service.tslib/ai/agent/input-filter.tslib/ai/utils/web-scraper.tssupabase/functions/messaging-webhook-resend/index.tssupabase/migrations/20260417160000_fix_board_ai_config_rls.sql
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/ci.yml
- lib/ai/utils/web-scraper.ts
- app/api/ai/board-config/generate-persona/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- supabase/migrations/20260417160000_fix_board_ai_config_rls.sql
| const body = await request.json() as Record<string, unknown>; | ||
| const allowedFields = [ | ||
| 'agent_name', 'business_context', 'agent_goal', 'persona_prompt', | ||
| 'knowledge_store_id', 'knowledge_store_name', | ||
| 'agent_mode', 'circuit_breaker_threshold', | ||
| 'hitl_threshold', 'hitl_min_confidence', 'hitl_expiration_hours', | ||
| 'handoff_keywords', 'max_messages_before_handoff', 'response_delay_seconds', | ||
| ]; | ||
| const payload: Record<string, unknown> = { board_id: boardId, organization_id: profile.organization_id }; | ||
| for (const field of allowedFields) { | ||
| if (field in body) payload[field] = body[field]; | ||
| } |
There was a problem hiding this comment.
Unsafe JSON parse and in on potentially non-object body.
await request.json() throws on malformed JSON and propagates as a 500. Also, if the body is null, an array, or a primitive, field in body at line 83 will throw (for null/primitive) or silently accept numeric-index "fields" (for arrays). Both should yield a 400.
🛡️ Proposed fix
- const body = await request.json() as Record<string, unknown>;
+ let body: Record<string, unknown>;
+ try {
+ const raw = await request.json();
+ if (!raw || typeof raw !== 'object' || Array.isArray(raw)) {
+ return NextResponse.json({ error: 'Invalid body' }, { status: 400 });
+ }
+ body = raw as Record<string, unknown>;
+ } catch {
+ return NextResponse.json({ error: 'Invalid JSON' }, { status: 400 });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/`[boardId]/route.ts around lines 73 - 84, Wrap the
request.json() call in a try/catch and return a 400 on parse error, then
validate that the parsed body is a plain object (typeof body === 'object' &&
body !== null && !Array.isArray(body)) and return 400 if not; after that safely
copy allowedFields into payload by checking ownership with
Object.prototype.hasOwnProperty.call(body, field) (or equivalent) instead of
using `field in body` so arrays/primitives/null won't be accepted and malformed
JSON won't crash the route handling for boardId/profile.organization_id
processing.
| const categoryLabel = CATEGORY_LABELS[category] ?? category; | ||
| const { text: safeContext } = sanitizeIncomingMessage(businessContext, { org_id: profile.organization_id }); |
There was a problem hiding this comment.
Reject unknown categories instead of interpolating them into the prompt.
categoryLabel falls back to raw category, so a direct API caller can inject arbitrary prompt text via category. Restrict this to known keys or sanitize the fallback before using it in the prompt.
🛡️ Proposed validation
- const categoryLabel = CATEGORY_LABELS[category] ?? category;
+ const categoryLabel = CATEGORY_LABELS[category];
+ if (!categoryLabel) {
+ return NextResponse.json({ error: 'Invalid category' }, { status: 400 });
+ }Also applies to: 66-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/ai/board-config/generate-goal/route.ts` around lines 59 - 60,
categoryLabel currently falls back to the raw category value which allows an
attacker to inject arbitrary prompt text; change the logic that computes
categoryLabel (using CATEGORY_LABELS and the variable categoryLabel) to reject
unknown categories rather than interpolating them — validate that category
exists as a key in CATEGORY_LABELS and throw or return a 400 error if not, or
else map to a fixed safe default token (e.g., "Unknown") before including it in
the prompt; do the same validation where categoryLabel is used later (lines
around the block between the current compute and lines 66-71) and avoid relying
on sanitizeIncomingMessage alone for category values.
| const { data, error } = await supabase | ||
| .from('organizations') | ||
| .select('id') | ||
| .limit(1) | ||
| .maybeSingle(); | ||
|
|
||
| const dbLatency = Date.now() - dbStartTime; | ||
|
|
||
| if (error) { | ||
| throw new Error(error.message); | ||
| } |
There was a problem hiding this comment.
RLS will likely make this probe always report error.
The query uses the anon key against organizations, which is gated by RLS requiring an authenticated user with a matching organization_id. With no session, the query will either return data: null (silently OK) or return an RLS error, making the "database OK" signal unreliable as a real health indicator. Consider either (a) a dedicated lightweight RPC/table readable by anon for health, or (b) use the server client with a cookie-less read on a table with a permissive policy. A pure select('id').limit(1) without .maybeSingle() would also avoid throwing "no rows" style errors.
As per coding guidelines: "All database queries must filter by organization_id in addition to RLS, especially for service role client usage" — a health ping should use a dedicated mechanism rather than probing tenant data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/health/route.ts` around lines 68 - 78, The health probe is querying
the tenant-protected organizations table with the public/anon client (the
supabase .from('organizations') .select(...).maybeSingle() call) which RLS will
block; replace this with a dedicated health mechanism: either call a lightweight
RPC or read a small permissively-policied table designed for anon health checks,
or switch to the server/service-role client reading a cookie-less permissive
table, and stop probing tenant data. Also avoid .maybeSingle() here (use
.select(...).limit(1) or a non-throwing RPC) so the probe doesn't throw on "no
rows"; update the code paths that reference data, error, dbLatency and
dbStartTime accordingly to use the new health endpoint/table instead of
organizations.
| // 0a. Rate limit check (per-conversation) — uses DB so it's safe across serverless instances | ||
| const rateCheck = await checkConversationRateLimit(supabase, conversationId); | ||
| if (!rateCheck.allowed) { | ||
| console.warn('[AIAgent] Rate limited for conversation:', conversationId); | ||
| logRateLimit(organizationId, conversationId, 0); |
There was a problem hiding this comment.
Scope the DB-backed rate limiter by organizationId.
checkConversationRateLimit only receives conversationId, and the referenced helper filters only by conversation_id. Since this service can run with privileged Supabase access, pass organizationId through and add .eq('organization_id', organizationId) in the helper query.
🛡️ Proposed call-site direction
- const rateCheck = await checkConversationRateLimit(supabase, conversationId);
+ const rateCheck = await checkConversationRateLimit(supabase, organizationId, conversationId);Based on learnings, “for service role queries, ensure organization_id filtering is always present.” As per coding guidelines, “All database queries must filter by organization_id to ensure multi-tenant security.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ai/agent/agent.service.ts` around lines 210 - 214, The DB-backed rate
limiter is currently scoped only by conversationId; update the call in the
handler to pass organizationId into checkConversationRateLimit (e.g., change
checkConversationRateLimit(supabase, conversationId) to
checkConversationRateLimit(supabase, conversationId, organizationId)) and then
modify the helper implementation to include an additional filter
.eq('organization_id', organizationId) alongside .eq('conversation_id',
conversationId) so all service-role queries are always tenant-scoped; keep
parameter and call-site names consistent with the existing
checkConversationRateLimit function and update any callers/tests accordingly.
| // 2c. Circuit breaker: verificar erros consecutivos | ||
| if (boardAIConfig) { | ||
| const cb = await getCircuitBreakerState( | ||
| supabase, | ||
| conversationId, | ||
| boardAIConfig.circuit_breaker_threshold, | ||
| ); | ||
| if (cb.isOpen) { | ||
| console.warn( | ||
| '[AIAgent] Circuit breaker OPEN for conversation %s (%d/%d errors)', | ||
| conversationId, | ||
| cb.consecutiveErrors, | ||
| cb.threshold, | ||
| ); | ||
| return { | ||
| success: true, | ||
| decision: { | ||
| action: 'skipped', | ||
| reason: `Circuit breaker aberto (${cb.consecutiveErrors} erros consecutivos)`, | ||
| }, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Pass organizationId through the circuit-breaker helpers.
The circuit-breaker helpers currently operate by conversationId/contactId only, and the provided helper snippets show no organization_id filters on messaging_conversations or contacts. Thread organizationId into getCircuitBreakerState, incrementCircuitBreakerError, and resetCircuitBreaker, then filter every read/update by tenant.
🛡️ Proposed call-site direction
const cb = await getCircuitBreakerState(
supabase,
+ organizationId,
conversationId,
boardAIConfig.circuit_breaker_threshold,
);
@@
await incrementCircuitBreakerError(
supabase,
+ organizationId,
conversationId,
conversation?.contact_id ?? null,
boardAIConfig.circuit_breaker_threshold,
);
@@
- await resetCircuitBreaker(supabase, conversationId);
+ await resetCircuitBreaker(supabase, organizationId, conversationId);Based on learnings, “for service role queries, ensure organization_id filtering is always present.” As per coding guidelines, “All database queries must filter by organization_id in addition to RLS, especially for service role client usage.”
Also applies to: 533-543, 603-631
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ai/agent/agent.service.ts` around lines 299 - 321, The circuit-breaker
helpers are missing tenant scoping: thread the organizationId from the call site
into getCircuitBreakerState, incrementCircuitBreakerError, and
resetCircuitBreaker, and update those helper implementations to include
organization_id filters on any messaging_conversations/contacts queries and
updates (and any writes to the circuit-breaker table) so all reads/updates are
scoped by the tenant; ensure the call signatures and call sites (e.g., where
getCircuitBreakerState is invoked) pass organizationId and that the helper
functions use it in WHERE clauses for service-role DB queries.
1 participant
Summary
BoardAIConfigModal,BoardAgentsSection, API routes de board-config, migrationboard_ai_configs, móduloslib/ai/messaging/elib/ai/utils/Security Fixes (todos confirmados como 🔴 Crítico)
export const SECURITY_PREAMBLE— propagado para todos os entry pointsagent.service.tssanitizeIncomingMessage()em inputs de usuário antes do LLMactions/route.ts,generate-goal/route.ts,generate-prompts.service.tsai_conversation_logem todos osgenerateTextvia helperlogAIActionactions/route.ts,analyze/route.ts,generate-goal/route.ts,generate-prompts.service.tsALLOWED_GOOGLE_MODELSwhitelist — fallback para default se modelId inválidolib/ai/config.tsimport 'server-only'— service role key nunca bundle no clientlib/supabase/staticAdminClient.tsItens reclassificados (não implementados por validação)
z.string().uuid()— deprecated mas funcional em Zod v4, sem urgência.allinvalidation — não é anti-pattern oficial (docs v5)Test plan
pnpm typecheck— zero errospnpm lint— zero warningspnpm test:run— 339 testes passam (1 falha pré-existente:tools.multiTenantpor conectividade Supabase offline)ai_conversation_logapós chamar qualquer endpoint AI🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security & Safety
Infrastructure
Documentation
Accessibility
Tests