Skip to content

fix(django): let django-no-csrf-token span realistic form bodies - #4025

Open
Eljees wants to merge 2 commits into
semgrep:developfrom
Eljees:agent/csrf-token-ellipsis-span
Open

fix(django): let django-no-csrf-token span realistic form bodies#4025
Eljees wants to merge 2 commits into
semgrep:developfrom
Eljees:agent/csrf-token-ellipsis-span

Conversation

@Eljees

@Eljees Eljees commented Jul 28, 2026

Copy link
Copy Markdown

Fixes #3816

Problem

In generic mode an ellipsis matches at most 10 newlines (generic_ellipsis_max_span, default 10). A real form body is longer than that, which breaks the rule in two directions.

Measured on the reporter's templates plus two variants, with the rule as it is on develop versus with the option raised:

template today with this change
13-line form with {% csrf_token %} (nested {% if %}, multi-line element) 1 finding — the reported false positive 0
11-line form with {% csrf_token %} (same, collapsed to one line) 0 0
10-line form without a token 1 1
43-line form without a token 0 — silently missed 1

So the same limit that hides the {% csrf_token %} exemption on a slightly larger form also stops the rule from matching longer forms at all. The false negative is the more dangerous half, and it is invisible to users.

Change

One options: block on the rule raising generic_ellipsis_max_span to 50, so both the main pattern and the pattern-not-inside exemptions reach through a normal form body.

50 is a judgement call — large enough for realistic templates, still bounded. Happy to move it if you'd prefer a different ceiling.

Tests

django-no-csrf-token.html gains the reporter's template as an ok case and a longer token-less form as a ruleid case (the latter fails without this change).

semgrep --test --config python/django/security/django-no-csrf-token.yaml python/django/security/django-no-csrf-token.html
1/1: ✓ All tests passed

In generic mode an ellipsis matches at most 10 newlines by default,
which is smaller than a real form body. Two things follow from that:
the csrf_token exemption stops matching once the body grows, so a form
that does contain {% csrf_token %} is reported, and a longer form is not
matched at all, so a form that is genuinely missing the token is
silently skipped.

Raise generic_ellipsis_max_span for this rule so both the exemption and
the rule itself reach through a normal form.

Signed-off-by: Eljees <3.14hell@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba0345790e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread python/django/security/django-no-csrf-token.yaml
@Eljees

Eljees commented Jul 28, 2026

Copy link
Copy Markdown
Author

Thanks — I checked the cross-form concern before changing anything, and I can't reproduce it. The pattern-not-inside still has to match a whole <form ...> ... </form> region, so a {% csrf_token %} in a neighbouring form doesn't satisfy it.

Three layouts, all with the patched rule (generic_ellipsis_max_span: 50):

layout finding
unprotected form, then protected form right after line 1 — unprotected form still reported
protected form first, then unprotected form line 6 — unprotected form still reported
unprotected multi-line form (nested {% if %}), then protected form line 2 — unprotected form still reported

So raising the span doesn't let the exemption leak across form boundaries. Happy to add one of these as a regression test in this PR if you'd like it pinned down.

Signed-off-by: Eljees <3.14hell@gmail.com>
@Eljees

Eljees commented Aug 10, 2026

Copy link
Copy Markdown
Author

Ping — open since 28 July, no review yet. Checks are green on c704428.

In generic mode an ellipsis spans at most 10 newlines, so a 13-line form that does contain {% csrf_token %} is reported as unprotected. The before/after table on the reporter's templates is in the description: the false positive goes away and the genuinely unprotected forms are still reported.

I also checked the obvious risk — that raising the span lets the exemption leak from one form into a neighbouring one. Three layouts are in the thread; the unprotected form is still reported in all three. Happy to adjust the span or the approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

django-no-csrf-token: false positive when form body contains nested {% if %} with a multi-line element

1 participant