Duo VR - Incorrect handling of detected FP

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Bug description

An MR is created for a detected FP vulnerability.
The diff of the MR shows a whole class removed and replaced by a text No fix needed as this is a false positive. The code already follows Django's secure practices for SQL generation.

How to reproduce

Run Vulnerability Resolution on etv/eyeballvul-django-635d53a/-/security/vulnerabilities/8445568

Notice that it doesn't always reproduce.
It depends on the LLM answer, which is not deterministic.

Unfortunately, the logs have already expired, so I cannot give the exact LLM response that triggered that bug. So it might not be easy to reproduce.

Actual behavior

@@ -13,87 +13,7 @@ __all__ = [
 ]
 
 
-class Aggregate(Func):
-    template = '%(function)s(%(distinct)s%(expressions)s)'
-    contains_aggregate = True
-    name = None
-    filter_template = '%s FILTER (WHERE %%(filter)s)'
-    window_compatible = True
-    allow_distinct = False
-
-    def __init__(self, *expressions, distinct=False, filter=None, **extra):
-        if distinct and not self.allow_distinct:
-            raise TypeError(""%s does not allow distinct."" % self.__class__.__name__)
-        self.distinct = distinct
-        self.filter = filter
-        super().__init__(*expressions, **extra)
-
-    def get_source_fields(self):
-        # Don't return the filter expression since it's not a source field.
-        return [e._output_field_or_none for e in super().get_source_expressions()]
-
-    def get_source_expressions(self):
-        source_expressions = super().get_source_expressions()
-        if self.filter:
-            return source_expressions + [self.filter]
-        return source_expressions
-
-    def set_source_expressions(self, exprs):
-        self.filter = self.filter and exprs.pop()
-        return super().set_source_expressions(exprs)
-
-    def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):
-        # Aggregates are not allowed in UPDATE queries, so ignore for_save
-        c = super().resolve_expression(query, allow_joins, reuse, summarize)
-        c.filter = c.filter and c.filter.resolve_expression(query, allow_joins, reuse, summarize)
-        if not summarize:
-            # Call Aggregate.get_source_expressions() to avoid
-            # returning self.filter and including that in this loop.
-            expressions = super(Aggregate, c).get_source_expressions()
-            for index, expr in enumerate(expressions):
-                if expr.contains_aggregate:
-                    before_resolved = self.get_source_expressions()[index]
-                    name = before_resolved.name if hasattr(before_resolved, 'name') else repr(before_resolved)
-                    raise FieldError(""Cannot compute %s('%s'): '%s' is an aggregate"" % (c.name, name, name))
-        return c
-
-    @property
-    def default_alias(self):
-        expressions = self.get_source_expressions()
-        if len(expressions) == 1 and hasattr(expressions[0], 'name'):
-            return '%s__%s' % (expressions[0].name, self.name.lower())
-        raise TypeError(""Complex expressions require an alias"")
-
-    def get_group_by_cols(self):
-        return []
-
-    def as_sql(self, compiler, connection, **extra_context):
-        extra_context['distinct'] = 'DISTINCT ' if self.distinct else ''
-        if self.filter:
-            if connection.features.supports_aggregate_filter_clause:
-                filter_sql, filter_params = self.filter.as_sql(compiler, connection)
-                template = self.filter_template % extra_context.get('template', self.template)
-                sql, params = super().as_sql(
-                    compiler, connection, template=template, filter=filter_sql,
-                    **extra_context
-                )
-                return sql, params + filter_params
-            else:
-                copy = self.copy()
-                copy.filter = None
-                source_expressions = copy.get_source_expressions()
-                condition = When(self.filter, then=source_expressions[0])
-                copy.set_source_expressions([Case(condition)] + source_expressions[1:])
-                return super(Aggregate, copy).as_sql(compiler, connection, **extra_context)
-        return super().as_sql(compiler, connection, **extra_context)
-
-    def _get_repr_options(self):
-        options = super()._get_repr_options()
-        if self.distinct:
-            options['distinct'] = self.distinct
-        if self.filter:
-            options['filter'] = self.filter
-        return options
+No fix needed as this is a false positive. The code already follows Django's secure practices for SQL generation.
 
 
 class Avg(FixDurationInputMixin, NumericOutputFieldMixin, Aggregate):

Expected behavior

image

/cc @nmccorrison

Edited by 🤖 GitLab Bot 🤖