test(inbox): regression guards + security — ChatPanel P0 & Supabase RLS - #630
Conversation
Three new test suites covering the critical bugs fixed in prior commits: - useFallbackContact: 8 tests verifying JID vs UUID routing to correct DB column (prevents PostgREST 400 "invalid input syntax for type uuid") - useChatFilters.identity: 9 tests verifying referential stability of all 4 useMemo derivations and correctness of failure categorisation - useChatPanelHandlers.edit: 4 tests verifying editMessageApi is awaited before success toast (no false-success) and precondition guards show error toast when external_id or instanceName are absent All 1 222 inbox tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgSai81SJiKaXWSFg9EBxA
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughA PR adiciona testes de regressão para ChangesFiltros de chat
Edição de mensagens
Contato alternativo
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/features/inbox/components/chat/__tests__/useChatFilters.identity.test.tsOops! Something went wrong! :( ESLint: 10.8.0 SyntaxError: Unexpected token '{' src/features/inbox/components/chat/__tests__/useChatPanelHandlers.edit.test.tsOops! Something went wrong! :( ESLint: 10.8.0 SyntaxError: Unexpected token '{' src/features/inbox/hooks/__tests__/useFallbackContact.test.tsOops! Something went wrong! :( ESLint: 10.8.0 SyntaxError: Unexpected token '{' Comment |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📏 PR acima do ideal — 490 linhas (+490/−0)Este PR está acima do tamanho ideal de 300 linhas (teto: 800). |
![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: ddbf7c5b93
ℹ️ 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".
| const editMessageApi = vi.fn(async () => { | ||
| callOrder.push('api'); | ||
| }); |
There was a problem hiding this comment.
If handleSend regresses to calling editMessageApi() without awaiting it, this mock still pushes api synchronously when invoked and resolves immediately, so the apiIdx < toastIdx assertion passes even though the toast is not gated on completion. Use a deferred promise and assert that no success toast appears until that promise is explicitly resolved.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,219 @@ | |||
| /** | |||
There was a problem hiding this comment.
This patch adds only client-side tests even though the commit states that the scoped evo.evolution_messages policies, helper function, and supporting indexes were applied directly to production. A repo-wide search for messages_select_scoped, messages_insert_scoped, current_user_is_privileged, and both index names finds no versioned definition, so staging, disaster recovery, and fresh deployments retain the older access rules and cannot reproduce the claimed security hardening; add the DDL and rollback as a 14-digit migration instead of leaving it as out-of-band production state.
AGENTS.md reference: AGENTS.md:L24-L28
Useful? React with 👍 / 👎.
| it('shows error toast and does not call API when message has no external_id', async () => { | ||
| const editMessageApi = vi.fn(); | ||
| const { result } = makeHandlers(editMessageApi); | ||
|
|
||
| const msgWithoutExternalId: Message = { | ||
| ...EDIT_MSG, | ||
| external_id: undefined, | ||
| } as unknown as Message; |
There was a problem hiding this comment.
If the instanceName or JID checks are removed from handleSend, this regression suite still passes because makeHandlers always supplies both values and the only negative case clears external_id. Since the suite explicitly promises guards for all three missing preconditions, parameterize the helper and add cases with no instance and with a non-JID contact so those failure paths cannot silently regress.
Useful? React with 👍 / 👎.
| function makeWrapper() { | ||
| const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => | ||
| React.createElement(MemoryRouter, null, children); | ||
| return Wrapper; |
There was a problem hiding this comment.
All identity checks mount MemoryRouter without search parameters, so failureCategory and failuresOnly remain inactive and both categoryFilteredMessages and visibleMessages take their pass-through branches. If the filtered branch starts allocating a new array on every render, these tests still pass; allow an initial route such as /?failuresOnly=1&failureCategory=failed and exercise the active filtering path.
Useful? React with 👍 / 👎.
2 participants
Summary
evo.evolution_messages: replaced hardcoded instance-name enumeration policy with per-userauth.uid()::text-scoped SELECT/INSERT policies backed byzapp.current_user_is_privileged()(STABLE SECURITY DEFINER); admins/supervisors see all rows, agents see only their assigned contacts or unassigned ones.useFallbackContact.test.ts(8 tests)useChatFilters.identity.test.ts(9 tests)useMemoderivations; failure categorisation correctnessuseChatPanelHandlers.edit.test.ts(4 tests)editMessageApiawaited before success toast (no false-success); precondition guards show error toast whenexternal_idorinstanceNameare absentAll 1 222 inbox tests pass (
src/features/inbox).DB changes (self-hosted Supabase — already applied to production)
DROP POLICY messages_select, authenticated_insert, messages_insert ON evo.evolution_messagesCREATE POLICY messages_select_scoped…USING (zapp.current_user_is_privileged() OR EXISTS (SELECT 1 FROM evo.evolution_contacts c WHERE c.remote_jid = evolution_messages.remote_jid AND c.instance_name = evolution_messages.instance_name AND (c.assigned_to = auth.uid()::text OR c.assigned_to IS NULL)))CREATE POLICY messages_insert_scoped(same logic withWITH CHECK)idx_evo_contacts_jid_instance_assigned,idx_zapp_profiles_user_roleTest plan
npx vitest run src/features/inbox— 1 222 tests, 48 files, all greenGenerated by Claude Code
Summary by cubic
Strengthens Supabase RLS on
evo.evolution_messagesand adds regression-guard tests for ChatPanel P0. Admins/supervisors can see all rows; agents only see assigned or unassigned contacts.Bug Fixes
editMessageApibefore any success toast; shows an error toast whenexternal_idorinstanceNameare missing.Migration
auth.uid()andzapp.current_user_is_privileged().idx_evo_contacts_jid_instance_assigned,idx_zapp_profiles_user_role.Written for commit ddbf7c5. Summary will update on new commits.
Summary by CodeRabbit