-
Notifications
You must be signed in to change notification settings - Fork 3
[v6 PROD RELEASE] - dev -> master #12
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
Conversation
fix(PM-2539): Added timeout to prisma service in member service
| jobs: | ||
| trivy-scan: | ||
| name: Use Trivy | ||
| runs-on: ubuntu-24.04 |
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.
[maintainability]
Consider using a stable version of the runner, such as ubuntu-latest, instead of ubuntu-24.04 to ensure compatibility and support. This can help avoid potential issues with specific versions that may not be maintained.
| ignore-unfixed: true | ||
| format: "sarif" | ||
| output: "trivy-results.sarif" | ||
| severity: "CRITICAL,HIGH,UNKNOWN" |
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.
[❗❗ correctness]
The severity level UNKNOWN is not a standard severity level in Trivy. Consider removing it to avoid potential misconfigurations or errors in the scan results.
| USERFLOW: process.env.USERFLOW_PRIVATE_KEY | ||
| } | ||
| }, | ||
| MEMBER_SERVICE_PRISMA_TIMEOUT: process.env.MEMBER_SERVICE_PRISMA_TIMEOUT ? parseInt(process.env.MEMBER_SERVICE_PRISMA_TIMEOUT, 10) : 10000, |
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.
[correctness]
Using parseInt without a radix can lead to unexpected behavior if the environment variable is not a valid number. Ensure that process.env.MEMBER_SERVICE_PRISMA_TIMEOUT is always a valid integer string or handle potential NaN values appropriately.
|
|
||
| const clientOptions = { | ||
| transactionOptions: { | ||
| timeout: config.MEMBER_SERVICE_PRISMA_TIMEOUT, |
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.
[correctness]
Ensure that config.MEMBER_SERVICE_PRISMA_TIMEOUT is properly validated and set to a sensible default to avoid potential issues with transaction timeouts.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
| throw new errors.UnauthorizedError('Authentication token is required to query users by email') | ||
| } | ||
| if (!helper.hasSearchByEmailRole(currentUser)) { | ||
| if (!currentUser.isMachine && !helper.hasSearchByEmailRole(currentUser)) { |
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.
[❗❗ security]
The added condition !currentUser.isMachine changes the logic to allow machine users to bypass the hasSearchByEmailRole check. Ensure that this change is intentional and that machine users should indeed be allowed to query users by email without this role.
| const canBypassStatusRestriction = currentUser && (currentUser.isMachine || helper.hasAdminRole(currentUser)) | ||
| const prismaFilter = prismaHelper.buildSearchMemberFilter(query, { | ||
| restrictStatus: !canBypassStatusRestriction | ||
| restrictStatus: !(canBypassStatusRestriction || isExplicitMemberLookup) |
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.
[❗❗ security]
The logic for isExplicitMemberLookup allows bypassing status restrictions if any of the specified fields are present in the query. Ensure that this behavior is intended and does not inadvertently expose sensitive member data without proper authorization.
No description provided.