Skip to content

[flake8-bandit] Fix misleading docstring for mako-templates (S702)#26432

Merged
ntBre merged 1 commit into
astral-sh:mainfrom
nikolauspschuetz:docs/s702-mako-example
Jul 7, 2026
Merged

[flake8-bandit] Fix misleading docstring for mako-templates (S702)#26432
ntBre merged 1 commit into
astral-sh:mainfrom
nikolauspschuetz:docs/s702-mako-example

Conversation

@nikolauspschuetz

@nikolauspschuetz nikolauspschuetz commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

The mako-templates (S702) rule docstring is misleading in two ways.

1. The "Use instead" example demonstrates nothing.

# Example (flagged)
Template("hello")

# Use instead (current)
Template("hello |h")

Template("hello |h") escapes nothing: mako's |h filter applies to a
variable expression (${ data |h }), so "hello |h" is just a literal
string. It also contradicts the rule's own prose just above it:

For example, to HTML escape the variable data, use ${ data |h }.

2. The rationale lists n as a sanitizing filter.

The "Why is this bad?" text said variables should be "sanitized via the n,
h or x flags"
. Mako's n filter disables default filtering, so it
cannot sanitize — listing it alongside the escaping filters is misleading.

Change

Docstring-only; no behavior change.

  • Rework both examples to show the escaping contrast on a variable, with inline
    comments matching the convention in sibling bandit rules (e.g.
    unsafe_markup_use's # XSS / # Safe):

    # Example (bad)
    Template("${ data }")      # Unescaped: vulnerable to XSS.
    
    # Use instead
    Template("${ data |h }")   # HTML-escaped with the `h` filter.
  • Drop n from the list of sanitizing filters in the rationale; keep the
    accurate h (HTML) and x (XML) escaping filters.

Test plan

  • cargo dev generate-docs regenerates docs/rules/mako-templates.md cleanly.
  • scripts/check_docs_formatted.py passes — both snippets are already
    ruff format-clean and parse without error.

@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre June 27, 2026 18:01
The "Use instead" example was Template("hello |h"), which demonstrates
nothing: mako's |h filter applies to a variable expression (${ data |h }),
so "hello |h" is just a literal string. It contradicted the rule's own
prose ("to HTML escape the variable data, use ${ data |h }").

- Rework both examples to show the escaping contrast on a variable, with
  inline comments matching the convention in sibling rules (e.g.
  unsafe_markup_use's `# XSS` / `# Safe`):
    bad:  Template("${ data }")      # Unescaped: vulnerable to XSS.
    good: Template("${ data |h }")   # HTML-escaped with the `h` filter.
- Drop `n` from the list of "sanitizing" filters in the rationale: mako's
  `n` filter disables default filtering, so it cannot sanitize. Keep the
  accurate `h` (HTML) and `x` (XML) escaping filters.
@nikolauspschuetz
nikolauspschuetz force-pushed the docs/s702-mako-example branch from 884792c to fe1758b Compare June 27, 2026 18:48
@nikolauspschuetz nikolauspschuetz changed the title [flake8-bandit] Fix misleading docstring examples for mako-templates (S702) [flake8-bandit] Fix misleading docstring for mako-templates (S702) Jun 27, 2026
@ntBre ntBre added the documentation Improvements or additions to documentation label Jul 7, 2026

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! This does seem like an improvement to me.

@ntBre ntBre changed the title [flake8-bandit] Fix misleading docstring for mako-templates (S702) [flake8-bandit] Fix misleading docstring for mako-templates (S702) Jul 7, 2026
@ntBre
ntBre enabled auto-merge (squash) July 7, 2026 15:30
@ntBre
ntBre merged commit 4b24ed1 into astral-sh:main Jul 7, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants