-
Notifications
You must be signed in to change notification settings - Fork 226
[comp] Production Deploy #1940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[comp] Production Deploy #1940
Conversation
* fix(api): update response and docs for policy & risks endpoints * fix(api): add API key auth support with userId parameter for comments API * fix(api): add API key auth support with userId parameter for transfer-ownership and task/attachments endpoints * fix(api): move userId to request body for comment deletion endpoint * fix(api): don't expose sensitive fields of user model from /risks endpoint * fix(api): update error description of transfer-ownership api --------- Co-authored-by: chasprowebdev <chasgarciaprowebdev@gmail.com> Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryAdds JumpCloud employee sync end-to-end, implements OAuth token revocation and connection auth teardown, and updates APIs/DTOs to support API key userId flows with related UI and docs changes.
Written by Cursor Bugbot for commit 74a444e. This will update automatically on new commits. Configure here. |
|
|
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Risk detail leaks full assignee user fields
RisksService.findById now uses include: { assignee: { include: { user: true }}}, which returns all scalar User fields (e.g., emailVerified, emailPreferences, isPlatformAdmin) in the API response. This expands exposed data compared to the list endpoint’s limited select and can leak sensitive/internal user attributes.
Graphite Automations"Auto-assign PRs to Author" took an action on this PR • (12/17/25)1 reviewer was added to this PR based on Mariano Fuentes's automation. |
| @ApiResponse({ | ||
| status: 200, | ||
| description: 'Comment deleted successfully', | ||
| content: { | ||
| 'application/json': { | ||
| schema: { | ||
| type: 'object', | ||
| properties: { | ||
| success: { type: 'boolean', example: true }, | ||
| deletedCommentId: { type: 'string', example: 'cmt_abc123def456' }, | ||
| message: { | ||
| type: 'string', | ||
| example: 'Comment deleted successfully', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| @ApiResponse({ | ||
| status: 401, | ||
| description: 'Unauthorized - Invalid authentication', | ||
| content: { | ||
| 'application/json': { | ||
| schema: { | ||
| type: 'object', | ||
| properties: { message: { type: 'string', example: 'Unauthorized' } }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| @ApiResponse({ | ||
| status: 404, | ||
| description: 'Comment not found', | ||
| content: { | ||
| 'application/json': { | ||
| schema: { | ||
| type: 'object', | ||
| properties: { | ||
| message: { | ||
| type: 'string', | ||
| example: 'Comment with ID cmt_abc123def456 not found', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| async deleteComment( | ||
| @OrganizationId() organizationId: string, | ||
| @AuthContext() authContext: AuthContextType, | ||
| @Param('commentId') commentId: string, | ||
| @Body() deleteDto: DeleteCommentDto, | ||
| ): Promise<{ success: boolean; deletedCommentId: string; message: string }> { | ||
| if (!authContext.userId) { | ||
| throw new BadRequestException('User ID is required'); | ||
| // For API key auth, userId must be provided in the request body | ||
| // For JWT auth, userId comes from the authenticated session | ||
| let userId: string; | ||
| if (authContext.isApiKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DELETE endpoint now requires a request body, which is non-standard for DELETE requests. The OpenAPI spec marks requestBody as required: true, but the DeleteCommentDto has userId as optional. This creates ambiguity:
- For JWT auth users, they must send an empty body
{}even though userId is ignored - Many HTTP clients, proxies, and tools have poor support for DELETE with body
- Some infrastructure may strip bodies from DELETE requests
Impact: JWT authentication users may get errors if their HTTP client doesn't send a body, or if infrastructure strips the body.
Fix: Consider one of these approaches:
// Option 1: Make body truly optional
@Body() deleteDto?: DeleteCommentDto,
// Then handle undefined case
// Option 2: Use query parameter for API key auth
@Query('userId') userId?: string,
// Option 3: Use custom header| @ApiResponse({ | |
| status: 200, | |
| description: 'Comment deleted successfully', | |
| }) | |
| @Delete(':commentId') | |
| async deleteComment( | |
| @OrganizationId() organizationId: string, | |
| @AuthContext() authContext: AuthContextType, | |
| @Param('commentId') commentId: string, | |
| @Query('userId') userIdParam?: string, | |
| ): Promise<{ success: boolean; deletedCommentId: string; message: string }> { | |
| // For API key auth, userId must be provided in the query parameter | |
| // For JWT auth, userId comes from the authenticated session | |
| let userId: string; | |
| if (authContext.isApiKey) { |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
* feat(ai): enhance policy assistant prompts for user guidance * chore(db): update package.json exports to include types for modules * fix(app): change overflow style in PolicyDetails component * fix(app): enable AI assistant by default in PolicyDetails component --------- Co-authored-by: Daniel Fu <itsnotaka@gmail.com> Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
| ...(authContext.userEmail && { email: authContext.userEmail }), | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: API key can transfer ownership via userId
transferOwnership no longer enforces session-only auth; under API key auth it trusts a body-provided userId and passes it as currentUserId. Any API key holder who knows an owner’s userId can trigger an ownership transfer, effectively bypassing the prior “session authentication required” gate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: PDF view clipped by hidden overflow
The left pane wrapper switched from overflow-y-auto to overflow-hidden, but the PDF/content inside (e.g., PdfViewer’s fixed h-[800px] iframe) can exceed the h-[60vh] container. This causes policy content/PDF to be cut off with no way to scroll, making parts of the document unreachable.
apps/app/src/app/(app)/[orgId]/policies/[policyId]/editor/components/PolicyDetails.tsx#L185-L252
* feat(jumpcloud): add integration for syncing employees from JumpCloud * fix(jumpcloud): added rippling back as a valid provider * fix(jumpcloud): updated help text * fix(sync): normalize email to lowercase for consistent database operations * chore(deps): update package versions and bun.lock for consistency
| name: gwUser.name.fullName || gwUser.primaryEmail.split('@')[0], | ||
| email: normalizedEmail, | ||
| name: gwUser.name.fullName || normalizedEmail.split('@')[0], | ||
| emailVerified: true, // Google Workspace users are verified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Email normalization can create duplicate users
Google Workspace and JumpCloud sync now lowercases emails for findUnique and create, but existing User.email values may have been stored with original casing. Because the DB @@unique([email]) is case-sensitive, a previously-created mixed-case email won’t match the new lowercase lookup and a second user record can be created for the same email.
Additional Locations (1)
|
🎉 This PR is included in version 1.72.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.