mirror of
https://github.com/zulip/zulip.git
synced 2025-11-02 13:03:29 +00:00
7d0c12aa3a
Previously, #26419 addressed the majority of these calls, but did not prevent more from creeping in. Remove the one remaining callsite (after the cleanup from the previous commits), and ban any future use of the pattern.
318 lines
12 KiB
YAML
318 lines
12 KiB
YAML
# See https://semgrep.dev/docs/writing-rules/rule-syntax/ for documentation on YAML rule syntax
|
|
|
|
rules:
|
|
####################### PYTHON RULES #######################
|
|
- id: deprecated-render-usage
|
|
pattern: django.shortcuts.render_to_response(...)
|
|
message: "Use render() (from django.shortcuts) instead of render_to_response()"
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: dont-use-stream-objects-filter
|
|
pattern: Stream.objects.filter(...)
|
|
message: "Please use access_stream_by_*() to fetch Stream objects"
|
|
languages: [python]
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- zerver/views/
|
|
|
|
- id: time-machine-travel-specify-tick
|
|
patterns:
|
|
- pattern: time_machine.travel(...)
|
|
- pattern-not: time_machine.travel(..., tick=..., ...)
|
|
message: |
|
|
Specify tick kwarg value for time_machine.travel(). Most cases will want to use False.
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: limit-message-filter
|
|
patterns:
|
|
- pattern: Message.objects.filter(...)
|
|
- pattern-not: Message.objects.filter(..., realm=..., ...)
|
|
- pattern-not: Message.objects.filter(..., realm_id=..., ...)
|
|
- pattern-not: Message.objects.filter(..., realm_id__in=..., ...)
|
|
- pattern-not: Message.objects.filter(..., id=..., ...)
|
|
- pattern-not: Message.objects.filter(..., id__in=..., ...)
|
|
- pattern-not: Message.objects.filter(..., id__lt=..., ...)
|
|
- pattern-not: Message.objects.filter(..., id__gt=..., ...)
|
|
message: "Set either a realm limit or an id limit on Message queries"
|
|
languages: [python]
|
|
severity: ERROR
|
|
paths:
|
|
exclude:
|
|
- "**/migrations/"
|
|
|
|
- id: dont-use-empty-select_related
|
|
pattern-either:
|
|
- pattern: $X.objects. ... .select_related()
|
|
- pattern: $X.objects. ... .prefetch_related()
|
|
message: |
|
|
Do not use a bare '.select_related()' or '.prefetch_related()', which can join many more tables than expected. Specify the relations to follow explicitly.
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: dont-import-models-in-migrations
|
|
patterns:
|
|
- pattern-not: from zerver.lib.redis_utils import get_redis_client
|
|
- pattern-not: from zerver.lib.utils import generate_api_key
|
|
- pattern-not: from zerver.models.linkifiers import filter_pattern_validator
|
|
- pattern-not: from zerver.models.linkifiers import url_template_validator
|
|
- pattern-not: from zerver.models.streams import generate_email_token_for_stream
|
|
- pattern-not: from zerver.models.realms import generate_realm_uuid_owner_secret
|
|
- pattern-either:
|
|
- pattern: from zerver import $X
|
|
- pattern: from analytics import $X
|
|
- pattern: from confirmation import $X
|
|
message: "Don't import models or other code in migrations; see https://zulip.readthedocs.io/en/latest/subsystems/schema-migrations.html"
|
|
languages: [python]
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "**/migrations"
|
|
exclude:
|
|
- zerver/migrations/0032_verify_all_medium_avatar_images.py
|
|
- zerver/migrations/0104_fix_unreads.py
|
|
- zerver/migrations/0206_stream_rendered_description.py
|
|
- zerver/migrations/0209_user_profile_no_empty_password.py
|
|
- zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py
|
|
- zerver/migrations/0387_reupload_realmemoji_again.py
|
|
- zerver/migrations/0443_userpresence_new_table_schema.py
|
|
- zerver/migrations/0493_rename_userhotspot_to_onboardingstep.py
|
|
- pgroonga/migrations/0002_html_escape_subject.py
|
|
|
|
- id: html-format
|
|
languages: [python]
|
|
pattern-either:
|
|
- pattern: markupsafe.Markup(... .format(...))
|
|
- pattern: markupsafe.Markup(f"...")
|
|
- pattern: markupsafe.Markup(... + ...)
|
|
severity: ERROR
|
|
message: "Do not write an HTML injection vulnerability please"
|
|
|
|
- id: sql-format
|
|
languages: [python]
|
|
pattern-either:
|
|
- pattern: ... .execute("...".format(...))
|
|
- pattern: ... .execute(f"...")
|
|
- pattern: ... .execute(... + ...)
|
|
- pattern: psycopg2.sql.SQL(... .format(...))
|
|
- pattern: psycopg2.sql.SQL(f"...")
|
|
- pattern: psycopg2.sql.SQL(... + ...)
|
|
- pattern: django.db.migrations.RunSQL(..., "..." .format(...), ...)
|
|
- pattern: django.db.migrations.RunSQL(..., f"...", ...)
|
|
- pattern: django.db.migrations.RunSQL(..., ... + ..., ...)
|
|
- pattern: django.db.migrations.RunSQL(..., [..., "..." .format(...), ...], ...)
|
|
- pattern: django.db.migrations.RunSQL(..., [..., f"...", ...], ...)
|
|
- pattern: django.db.migrations.RunSQL(..., [..., ... + ..., ...], ...)
|
|
severity: ERROR
|
|
message: "Do not write a SQL injection vulnerability please"
|
|
|
|
- id: translated-format-lazy
|
|
languages: [python]
|
|
pattern: django.utils.translation.gettext_lazy(...).format(...)
|
|
severity: ERROR
|
|
message: "Immediately formatting a lazily translated string destroys its laziness"
|
|
|
|
- id: translated-positional-field
|
|
languages: [python]
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: django.utils.translation.gettext("$MESSAGE")
|
|
- pattern: django.utils.translation.pgettext($CONTEXT, "$MESSAGE")
|
|
- pattern: django.utils.translation.gettext_lazy("$MESSAGE")
|
|
- pattern: django.utils.translation.pgettext_lazy($CONTEXT, "$MESSAGE")
|
|
- metavariable-regex:
|
|
metavariable: $MESSAGE
|
|
regex: (^|.*[^{])(\{\{)*\{[:!}].*
|
|
severity: ERROR
|
|
message: "Prefer {named} fields over positional {} in translated strings"
|
|
|
|
- id: mutable-default-type
|
|
languages: [python]
|
|
pattern-either:
|
|
- pattern: |
|
|
def $F(..., $A: typing.List[...] = zerver.lib.request.REQ(..., default=[...], ...), ...) -> ...:
|
|
...
|
|
- pattern: |
|
|
def $F(..., $A: typing.Optional[typing.List[...]] = zerver.lib.request.REQ(..., default=[...], ...), ...) -> ...:
|
|
...
|
|
- pattern: |
|
|
def $F(..., $A: typing.Dict[...] = zerver.lib.request.REQ(..., default={}, ...), ...) -> ...:
|
|
...
|
|
- pattern: |
|
|
def $F(..., $A: typing.Optional[typing.Dict[...]] = zerver.lib.request.REQ(..., default={}, ...), ...) -> ...:
|
|
...
|
|
severity: ERROR
|
|
message: "Guard mutable default with read-only type (Sequence, Mapping, AbstractSet)"
|
|
|
|
- id: percent-formatting
|
|
languages: [python]
|
|
pattern-either:
|
|
- pattern: '"..." % ...'
|
|
- pattern: django.utils.translation.gettext(...) % ...
|
|
- pattern: django.utils.translation.pgettext(...) % ...
|
|
- pattern: django.utils.translation.gettext_lazy(...) % ...
|
|
- pattern: django.utils.translation.pgettext_lazy(...) % ...
|
|
severity: ERROR
|
|
message: "Prefer f-strings or .format for string formatting"
|
|
|
|
- id: change-user-is-active
|
|
languages: [python]
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: |
|
|
$X.is_active = ...
|
|
- pattern: |
|
|
setattr($X, 'is_active', ...)
|
|
- pattern-not-inside: |
|
|
def change_user_is_active(...):
|
|
...
|
|
message: "Use change_user_is_active to mutate user_profile.is_active"
|
|
severity: ERROR
|
|
paths:
|
|
exclude:
|
|
- zerver/migrations/0373_fix_deleteduser_dummies.py
|
|
|
|
- id: confirmation-object-get
|
|
languages: [python]
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: Confirmation.objects.get(...)
|
|
- pattern: Confirmation.objects.filter(..., confirmation_key=..., ...)
|
|
- pattern-not-inside: |
|
|
def get_object_from_key(...):
|
|
...
|
|
paths:
|
|
exclude:
|
|
- zerver/tests/
|
|
message: "Do not fetch a Confirmation object directly, use get_object_from_key instead"
|
|
severity: ERROR
|
|
|
|
- id: dont-make-batched-migration-atomic
|
|
patterns:
|
|
- pattern: |
|
|
class Migration(migrations.Migration):
|
|
...
|
|
- pattern-inside: |
|
|
...
|
|
BATCH_SIZE = ...
|
|
...
|
|
- pattern-not: |
|
|
class Migration(migrations.Migration):
|
|
atomic = False
|
|
paths:
|
|
include:
|
|
- "**/migrations"
|
|
message: 'A batched migration should not be atomic. Add "atomic = False" to the Migration class'
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: typed_endpoint_without_keyword_only_param
|
|
patterns:
|
|
- pattern: |
|
|
@typed_endpoint
|
|
def $F(...)-> ...:
|
|
...
|
|
- pattern-not-inside: |
|
|
@typed_endpoint
|
|
def $F(..., *, ...)-> ...:
|
|
...
|
|
message: |
|
|
@typed_endpoint should not be used without keyword-only parameters.
|
|
Make parameters to be parsed from the request as keyword-only,
|
|
or use @typed_endpoint_without_parameters instead.
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: dont-nest-annotated-types-with-param-config
|
|
patterns:
|
|
- pattern-not: |
|
|
def $F(..., invalid_param: typing.Optional[<... zerver.lib.typed_endpoint.ApiParamConfig(...) ...>], ...) -> ...:
|
|
...
|
|
- pattern-not: |
|
|
def $F(..., $A: typing_extensions.Annotated[<... zerver.lib.typed_endpoint.ApiParamConfig(...) ...>], ...) -> ...:
|
|
...
|
|
- pattern-not: |
|
|
def $F(..., $A: typing_extensions.Annotated[<... zerver.lib.typed_endpoint.ApiParamConfig(...) ...>] = ..., ...) -> ...:
|
|
...
|
|
- pattern-either:
|
|
- pattern: |
|
|
def $F(..., $A: $B[<... zerver.lib.typed_endpoint.ApiParamConfig(...) ...>], ...) -> ...:
|
|
...
|
|
- pattern: |
|
|
def $F(..., $A: $B[<... zerver.lib.typed_endpoint.ApiParamConfig(...) ...>] = ..., ...) -> ...:
|
|
...
|
|
message: |
|
|
Annotated types containing zerver.lib.typed_endpoint.ApiParamConfig should not be nested inside Optional. Use Annotated[Optional[...], zerver.lib.typed_endpoint.ApiParamConfig(...)] instead.
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: exists-instead-of-count
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: ... .count() == 0
|
|
- pattern: |
|
|
if not ... .count():
|
|
...
|
|
message: 'Use "not .exists()" instead; it is more efficient'
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: exists-instead-of-count-not-zero
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: ... .count() != 0
|
|
- pattern: ... .count() > 0
|
|
- pattern: ... .count() >= 1
|
|
- pattern: |
|
|
if ... .count():
|
|
...
|
|
message: 'Use ".exists()" instead; it is more efficient'
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: functools-partial
|
|
pattern: functools.partial
|
|
message: "Replace functools.partial with returns.curry.partial for type safety"
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: timedelta-positional-argument
|
|
patterns:
|
|
- pattern: timedelta(...)
|
|
- pattern-not: timedelta(0)
|
|
- pattern-not: timedelta(..., days=..., ...)
|
|
- pattern-not: timedelta(..., seconds=..., ...)
|
|
- pattern-not: timedelta(..., microseconds=..., ...)
|
|
- pattern-not: timedelta(..., milliseconds=..., ...)
|
|
- pattern-not: timedelta(..., minutes=..., ...)
|
|
- pattern-not: timedelta(..., hours=..., ...)
|
|
- pattern-not: timedelta(..., weeks=..., ...)
|
|
message: |
|
|
Specify timedelta with named arguments.
|
|
languages: [python]
|
|
severity: ERROR
|
|
|
|
- id: time-machine
|
|
languages: [python]
|
|
patterns:
|
|
- pattern-either:
|
|
- pattern: patch("$FUNCTION", return_value=$TIME)
|
|
- pattern: mock.patch("$FUNCTION", return_value=$TIME)
|
|
- metavariable-regex:
|
|
metavariable: $FUNCTION
|
|
regex: .*timezone_now
|
|
fix: time_machine.travel($TIME, tick=False)
|
|
severity: ERROR
|
|
message: "Use the time_machine package, rather than mocking timezone_now"
|
|
|
|
- id: urlparse
|
|
languages: [python]
|
|
pattern-either:
|
|
- pattern: urllib.parse.urlparse
|
|
- pattern: urllib.parse.urlunparse
|
|
- pattern: urllib.parse.ParseResult
|
|
severity: ERROR
|
|
message: "Use urlsplit rather than urlparse"
|