-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
🐛 fix(github): make inbound/outbound assignment sync case-insensitive #101984
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
| external_actors = ExternalActor.objects.filter( | ||
| provider=EXTERNAL_PROVIDERS_REVERSE[ExternalProviderEnum(integration.provider)].value, | ||
| external_name=external_user_name, | ||
| external_name__iexact=external_user_name, |
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.
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.
we also don't have any logic that prevents repeat external_names i believe, something we should enforce.
| external_actors = ExternalActor.objects.filter( | ||
| provider=EXTERNAL_PROVIDERS_REVERSE[ExternalProviderEnum(integration.provider)].value, | ||
| external_name=external_user_name, | ||
| external_name__iexact=external_user_name, | ||
| integration_id=integration.id, | ||
| user_id__isnull=False, | ||
| ).values_list("user_id", flat=True) |
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.
High severity vulnerability may affect your project—review required:
Line 140 lists a dependency (django) with a known High severity vulnerability.
ℹ️ Why this matters
Affected versions of Django are vulnerable to Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). SQL injection in Django's ORM column aliases: when using QuerySet.annotate(), QuerySet.alias(), QuerySet.aggregate(), or QuerySet.extra() with dictionary expansion (**kwargs), the dictionary keys are used unescaped as SQL column aliases. On MySQL and MariaDB backends, an attacker who can influence those keys (for example, by passing a crafted dict of annotations) can inject arbitrary SQL into the generated query.
To resolve this comment:
Check if you are using Django with MySQL or MariaDB.
- If you're affected, upgrade this dependency to at least version 5.2.7 at uv.lock.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues: |
Github usernames are case-insensitive and users currently can configure their usernames with any captialization they see fit.
To correctly attribute usernames to users, we need to do a case-insensitive search when looking for
ExternalActorand send Github a case-insensitive username back as well. The second point is probably not needed but good to do.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.