-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add support for csrf #2604
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
bugbot run |
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.
Pull Request Overview
This PR adds CSRF protection support to the YdbEmbeddedAPI by introducing a configurable token getter function. The changes enable CSRF tokens to be set across all API service instances when a token is provided.
- Adds
csrfTokenGetter
parameter to the YdbEmbeddedAPI constructor - Implements token distribution logic to set CSRF tokens on all API service instances
Comments suppressed due to low confidence (1)
src/services/api/index.ts:33
- [nitpick] The parameter name 'csrfTokenGetter' could be more descriptive. Consider renaming to 'getCsrfToken' or 'csrfTokenProvider' to better indicate its purpose as a function that provides/retrieves the token.
csrfTokenGetter = () => undefined,
src/services/api/index.ts
Outdated
this.auth.setCSRFToken(token); | ||
this.meta?.setCSRFToken(token); | ||
this.codeAssist?.setCSRFToken(token); | ||
this.operation.setCSRFToken(token); | ||
this.pdisk.setCSRFToken(token); | ||
this.scheme.setCSRFToken(token); | ||
this.storage.setCSRFToken(token); | ||
this.streaming.setCSRFToken(token); | ||
this.tablets.setCSRFToken(token); | ||
this.vdisk.setCSRFToken(token); | ||
this.viewer.setCSRFToken(token); |
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.
The repetitive manual token setting for each API service creates maintainability issues. Consider iterating over the API services or creating a helper method to reduce code duplication.
this.auth.setCSRFToken(token); | |
this.meta?.setCSRFToken(token); | |
this.codeAssist?.setCSRFToken(token); | |
this.operation.setCSRFToken(token); | |
this.pdisk.setCSRFToken(token); | |
this.scheme.setCSRFToken(token); | |
this.storage.setCSRFToken(token); | |
this.streaming.setCSRFToken(token); | |
this.tablets.setCSRFToken(token); | |
this.vdisk.setCSRFToken(token); | |
this.viewer.setCSRFToken(token); | |
const services = [ | |
this.auth, | |
this.meta, | |
this.codeAssist, | |
this.operation, | |
this.pdisk, | |
this.scheme, | |
this.storage, | |
this.streaming, | |
this.tablets, | |
this.vdisk, | |
this.viewer, | |
]; | |
services.forEach(service => service?.setCSRFToken(token)); |
Copilot uses AI. Check for mistakes.
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.
dont like
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.
✅ Bugbot reviewed your changes and found no bugs!
Bugbot free trial expires on July 29, 2025
Learn more in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: ✅
Current: 85.21 MB | Main: 85.21 MB
Diff: +1.73 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information