Conversation
When checking m2m scopes - one scope match is enough
fix(PM-2539): added timeout for prisma client
|
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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| jobs: | ||
| trivy-scan: | ||
| name: Use Trivy | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
[maintainability]
Consider using ubuntu-latest instead of ubuntu-24.04 to ensure the workflow benefits from the latest security patches and updates. This can help avoid potential issues with outdated dependencies or vulnerabilities.
| output: "trivy-results.sarif" | ||
| severity: "CRITICAL,HIGH,UNKNOWN" | ||
| scanners: vuln,secret,misconfig,license | ||
| github-pat: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
[❗❗ security]
Ensure that the GITHUB_TOKEN secret is configured with the necessary permissions for this workflow. If additional permissions are required beyond the default, they should be explicitly set in the permissions section.
| .filter(Boolean); | ||
|
|
||
| const scopeOk = fallbackScopes.every((s) => scopes.includes(s)); | ||
| const scopeOk = fallbackScopes.some((s) => scopes.includes(s)); |
There was a problem hiding this comment.
[❗❗ correctness]
The change from every to some alters the logic to allow access if any fallback scope matches, rather than requiring all to match. Ensure this change aligns with the intended authorization logic, as it could potentially broaden access permissions.
| .filter(Boolean); | ||
|
|
||
| const ok = required.every((s) => scopes.includes(s)); | ||
| const ok = required.some((s) => scopes.includes(s)); |
There was a problem hiding this comment.
[❗❗ correctness]
Changing from every to some alters the logic from requiring all scopes to match to requiring only one. Ensure this change aligns with the intended authorization logic, as it could inadvertently grant access with insufficient permissions.
| this.client = new PrismaClient({ datasources: { db: { url } } }); | ||
| this.client = new PrismaClient({ | ||
| transactionOptions: { | ||
| timeout: process.env.BA_SERVICE_PRISMA_TIMEOUT |
There was a problem hiding this comment.
[correctness]
The parseInt function is used without specifying a radix, which defaults to 10 in this context. However, it's a good practice to always specify the radix explicitly to avoid potential issues in other environments. Consider using parseInt(process.env.BA_SERVICE_PRISMA_TIMEOUT, 10).
| this.client = new PrismaClient({ datasources: { db: { url } } }); | ||
| this.client = new PrismaClient({ | ||
| transactionOptions: { | ||
| timeout: process.env.BA_SERVICE_PRISMA_TIMEOUT |
There was a problem hiding this comment.
[❗❗ security]
The environment variable BA_SERVICE_PRISMA_TIMEOUT is parsed as an integer and used as a timeout value. Ensure that this environment variable is always set to a valid integer to prevent runtime errors. Consider adding validation logic to handle invalid or missing values more gracefully.
| constructor() { | ||
| super({ | ||
| transactionOptions: { | ||
| timeout: process.env.BA_SERVICE_PRISMA_TIMEOUT |
There was a problem hiding this comment.
[❗❗ correctness]
Consider validating the environment variable process.env.BA_SERVICE_PRISMA_TIMEOUT before using it. If it is not a valid number, parseInt will return NaN, which could lead to unexpected behavior. You might want to add a check to ensure it is a valid number and handle the case where it is not.
5 participants
No description provided.