347 erro de permisionamento! - #348
Merged
GRHInvDev merged 2 commits intoMar 10, 2026
Merged
Conversation
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
Contributor
📝 WalkthroughWalkthroughThe PR introduces testing infrastructure via Vitest, adds UTC-based date utilities for birthday comparisons, and simplifies form access control logic by removing private-form and sector-based permission checks while retaining creator, owner, and sudo-based authorization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/lib/date-utils.ts (1)
1-10: Add JSDoc documentation and explicit return types.Per coding guidelines, public utility functions should have JSDoc comments with
@paramand@returnstags, and explicit return types.📝 Proposed documentation and typing improvements
+/**+ * Compares two dates to check if they share the same UTC month and day.+ * `@param` a - First date to compare+ * `@param` b - Second date to compare+ * `@returns` true if both dates have the same UTC month and day+ * `@example`+ * isSameUtcMonthDay(new Date('2024-01-15'), new Date('2025-01-15')) // true+ */-export function isSameUtcMonthDay(a: Date, b: Date): boolean {+export function isSameUtcMonthDay(a: Date, b: Date): boolean { return a.getUTCMonth() === b.getUTCMonth() && a.getUTCDate() === b.getUTCDate() } +/**+ * Extracts the UTC month and day from a date object.+ * `@param` date - The date to extract from+ * `@returns` Object containing UTC month (0-11) and day (1-31)+ */ export function getUtcMonthDay(date: Date): { month: number; day: number } { return { month: date.getUTCMonth(), day: date.getUTCDate(), } }As per coding guidelines: "Document functions and components with JSDoc comments including
@paramand@returns" and "Use JSDoc comments for all public functions and hooks with@param,@returns, and@exampletags."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/date-utils.ts` around lines 1 - 10, Add JSDoc comments to both exported functions isSameUtcMonthDay and getUtcMonthDay: include `@param` tags describing the Date parameters, an `@returns` tag explaining the boolean result for isSameUtcMonthDay and the { month:number; day:number } shape for getUtcMonthDay, and add a brief `@example` showing usage; keep the explicit return types (boolean and { month: number; day: number }) on the function signatures and ensure the comments sit immediately above each function declaration.src/lib/access-control-server.ts (1)
144-156: Consider usingcanEditFormto avoid logic duplication.The permission logic here duplicates
canEditFormfrom@/lib/access-control. This creates a maintenance risk if the permission rules change. Consider importing and usingcanEditFormdirectly.♻️ Proposed refactor to use centralized permission check
import "server-only"; import { redirect } from "next/navigation"; import { api } from "@/trpc/server"; import { hasAccessToAdminRoute } from "@/const/admin-routes"; +import { canEditForm } from "@/lib/access-control"; // ... existing code ... export async function checkFormEditAccess(formId: string) { const db_user = await api.user.me(); if (!db_user?.role_config) { redirect("/dashboard"); } if (db_user.role_config.isTotem) { redirect("/dashboard"); } const form = await api.form.getById(formId); if (!form || !db_user.id) { redirect("/forms"); } - const canEdit =- form.userId === db_user.id ||- form.ownerIds?.includes(db_user.id) ||- db_user.role_config.sudo ||- db_user.role_config.can_create_form;+ const canEdit = canEditForm(+ db_user.role_config,+ db_user.id,+ formId,+ form,+ db_user.setor+ ); if (!canEdit) { redirect("/forms"); } return db_user; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/access-control-server.ts` around lines 144 - 156, Replace the duplicated permission logic with the centralized canEditForm function: import canEditForm from '@/lib/access-control' and use it to compute permission (e.g., const canEdit = canEditForm({ form, user: db_user })); then keep the same redirect behavior when !form or !db_user.id and when !canEdit so behavior is unchanged; update or remove the existing manual checks (form.userId, form.ownerIds, db_user.role_config) in access-control-server.ts to rely on canEditForm instead.vitest.config.ts (1)
5-8: Consider jsdom environment for React component tests.The
environment: "node"setting works for server-side tests like access-control, but.tsxtests that render React components will require jsdom. If component tests are added later, you may need to configure per-file environments.💡 Example for future component tests
// For per-file environment configuration, add at the top of component test files:// `@vitest-environment` jsdom// Or configure in vitest.config.ts with environmentMatchGlobs: test: {environment: "node",environmentMatchGlobs: [["src/components/**/*.test.tsx","jsdom"],],// ...}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.ts` around lines 5 - 8, The test config currently sets environment: "node" which will break React .tsx component tests; update the Vitest test config block (the test object that contains environment) to add per-file environment rules by adding environmentMatchGlobs mapping (key: environmentMatchGlobs) that maps your component test glob(s) like "src/components/**/*.test.tsx" to "jsdom", or alternatively document adding a // `@vitest-environment` jsdom header in individual component test files so .tsx tests run under jsdom while keeping the default environment as node.src/lib/__tests__/access-control.forms-edit.test.ts (1)
2-2: Use the@/alias for this internal import.
../access-controlbreaks the repo’s import convention and is more brittle than the configured alias.As per coding guidelines, "Always use
@/aliases for internal imports instead of relative paths".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/__tests__/access-control.forms-edit.test.ts` at line 2, Replace the relative import "../access-control" with the project alias import using "@/access-control" in the test file so it follows the repo convention; locate the import line that imports canEditForm and change it to use the "@/..." alias (i.e., import { canEditForm } from "@/access-control") so the test uses the configured internal path alias.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/__tests__/access-control.forms-edit.test.ts`:
- Around line 111-144: The test and permission logic incorrectly treat
role_config.can_create_form as permission to edit any form; update the
canEditForm implementation so can_create_form does NOT grant edit-any
rights—only sudo or explicit ownership (userId === form.userId or form.ownerIds
includes userId) or explicit edit permissions should allow editing—and then
change the failing tests in access-control.forms-edit.test.ts to assert false
for a non-owner/non-sudo user with only can_create_form true; keep true
expectations only for sudo or actual owner scenarios and adjust references to
canEditForm, role_config.can_create_form, sudo, userId, form.userId, and
form.ownerIds accordingly.
---
Nitpick comments:
In `@src/lib/__tests__/access-control.forms-edit.test.ts`:
- Line 2: Replace the relative import "../access-control" with the project alias
import using "@/access-control" in the test file so it follows the repo
convention; locate the import line that imports canEditForm and change it to use
the "@/..." alias (i.e., import { canEditForm } from "@/access-control") so the
test uses the configured internal path alias.
In `@src/lib/access-control-server.ts`:
- Around line 144-156: Replace the duplicated permission logic with the
centralized canEditForm function: import canEditForm from '@/lib/access-control'
and use it to compute permission (e.g., const canEdit = canEditForm({ form,
user: db_user })); then keep the same redirect behavior when !form or
!db_user.id and when !canEdit so behavior is unchanged; update or remove the
existing manual checks (form.userId, form.ownerIds, db_user.role_config) in
access-control-server.ts to rely on canEditForm instead.
In `@src/lib/date-utils.ts`:
- Around line 1-10: Add JSDoc comments to both exported functions
isSameUtcMonthDay and getUtcMonthDay: include `@param` tags describing the Date
parameters, an `@returns` tag explaining the boolean result for isSameUtcMonthDay
and the { month:number; day:number } shape for getUtcMonthDay, and add a brief
`@example` showing usage; keep the explicit return types (boolean and { month:
number; day: number }) on the function signatures and ensure the comments sit
immediately above each function declaration.
In `@vitest.config.ts`:
- Around line 5-8: The test config currently sets environment: "node" which will
break React .tsx component tests; update the Vitest test config block (the test
object that contains environment) to add per-file environment rules by adding
environmentMatchGlobs mapping (key: environmentMatchGlobs) that maps your
component test glob(s) like "src/components/**/*.test.tsx" to "jsdom", or
alternatively document adding a // `@vitest-environment` jsdom header in
individual component test files so .tsx tests run under jsdom while keeping the
default environment as node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5d8c30e-237a-4587-87b3-be9c464ce26b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.jsonsrc/components/birthday/birthday-confetti-wrapper.tsxsrc/components/birthday/birthday-confetti.tsxsrc/lib/__tests__/access-control.forms-edit.test.tssrc/lib/access-control-server.tssrc/lib/access-control.tssrc/lib/date-utils.tssrc/server/api/routers/forms.tsvitest.config.ts
Comment on lines
+111
to
+144
| describe("sudo e can_create_form podem editar qualquer formulário", () => { | ||
| it("retorna true quando role_config.sudo é true", () => { | ||
| const result = canEditForm( | ||
| { ...baseRoleConfig, sudo: true }, | ||
| otherUserId, | ||
| formId, | ||
| { | ||
| userId: creatorId, | ||
| ownerIds: [ownerId], | ||
| isPrivate: true, | ||
| allowedUsers: [], | ||
| allowedSectors: [], | ||
| }, | ||
| null | ||
| ); | ||
| expect(result).toBe(true); | ||
| }); | ||
| it("retorna true quando role_config.can_create_form é true", () => { | ||
| const result = canEditForm( | ||
| { ...baseRoleConfig, can_create_form: true }, | ||
| otherUserId, | ||
| formId, | ||
| { | ||
| userId: creatorId, | ||
| ownerIds: [], | ||
| isPrivate: false, | ||
| allowedUsers: [], | ||
| allowedSectors: [], | ||
| }, | ||
| null | ||
| ); | ||
| expect(result).toBe(true); | ||
| }); |
Contributor
There was a problem hiding this comment.
Don’t lock in can_create_form as edit-any-form permission.
This test suite codifies the privilege escalation the PR is trying to close. If can_create_form alone returns true here, any user with form-creation rights can still modify unrelated — including private — forms they do not own.
🔐 Suggested test update
- describe("sudo e can_create_form podem editar qualquer formulário", () => {+ describe("apenas sudo pode editar qualquer formulário", () => {
it("retorna true quando role_config.sudo é true", () => {
const result = canEditForm(
{ ...baseRoleConfig, sudo: true },
otherUserId,
formId,
@@
);
expect(result).toBe(true);
});
- it("retorna true quando role_config.can_create_form é true", () => {+ it("retorna false quando role_config.can_create_form é true, mas o usuário não é owner/criador", () => {
const result = canEditForm(
{ ...baseRoleConfig, can_create_form: true },
otherUserId,
formId,
@@
null
);
- expect(result).toBe(true);+ expect(result).toBe(false);
});
});As per coding guidelines, "Implement permission checks in API procedures, throwing FORBIDDEN errors when users lack sudo privileges or attempt to modify resources they don't own".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/__tests__/access-control.forms-edit.test.ts` around lines 111 - 144,
The test and permission logic incorrectly treat role_config.can_create_form as
permission to edit any form; update the canEditForm implementation so
can_create_form does NOT grant edit-any rights—only sudo or explicit ownership
(userId === form.userId or form.ownerIds includes userId) or explicit edit
permissions should allow editing—and then change the failing tests in
access-control.forms-edit.test.ts to assert false for a non-owner/non-sudo user
with only can_create_form true; keep true expectations only for sudo or actual
owner scenarios and adjust references to canEditForm,
role_config.can_create_form, sudo, userId, form.userId, and form.ownerIds
accordingly.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Foi identificado hoje mais cedo um erro de permissão!
Um dos usuários conseguiu alterar um dos formulários sem a devida permissão!
O usuário Roger alterou o formulário de TI, inserindo acidentalmente no formulário o próprio nome!
Isso pode afetar formulários privados!
Summary by CodeRabbit
Bug Fixes
Improvements
Tests