-
Notifications
You must be signed in to change notification settings - Fork 62
Perf improvements #1196
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
Perf improvements #1196
Conversation
Codecov Report❌ Patch coverage is |
| """Store the token locally.""" | ||
| self.save_token(token) | ||
| """Store the token locally and fetch organization info.""" | ||
| self.save_token_with_org_info(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.
Security issue: Token is stored without validation
authenticate_token no longer verifies that the supplied token is active, so revoked or invalid tokens will be persisted.
| self.save_token_with_org_info(token) | |
| # Validate token before saving | |
| identity = RestAPI(token).identify() | |
| if not identity or identity.auth_context.status != "active": | |
| raise AuthError("API token is invalid or inactive.") | |
| # Save once validation succeeds | |
| self.save_token_with_org_info(token) |
| return None | ||
| return None | ||
|
|
||
| def get_org_name(self) -> str | None: |
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.
Logic bug: get_current_token falls back to uncached get_token() but ignores cache invalidation
If an exception occurs, the global cache remains stale and may serve invalid data on subsequent calls.
| def get_org_name(self) -> str | None: | |
| except Exception: | |
| _token_cache = None | |
| _cache_mtime = None | |
| # Fall back to uncached read | |
| token_manager = TokenManager() | |
| return token_manager.get_token() |
|
Found 2 issues. Please review my inline comments above. |
|
🎉 This PR is included in version 0.56.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Motivation
Content
Testing
Please check the following before marking your PR as ready for review