Skip to content

[flake8-bandit] Allow suspicious imports in TYPE_CHECKING blocks (S401-S415)#23441

Merged
ntBre merged 2 commits intoastral-sh:mainfrom
gcomneno:fix/s408-type-checking
Feb 24, 2026
Merged

[flake8-bandit] Allow suspicious imports in TYPE_CHECKING blocks (S401-S415)#23441
ntBre merged 2 commits intoastral-sh:mainfrom
gcomneno:fix/s408-type-checking

Conversation

@gcomneno
Copy link
Contributor

Fix false positives for S408/S409 when xml.dom.minidom or xml.dom.pulldom are imported inside if TYPE_CHECKING: blocks.

Imports inside TYPE_CHECKING are not executed at runtime, so they should not trigger these Bandit-based security rules.

Adds a dedicated fixture and snapshot test for the TYPE_CHECKING case.

Refs #14901

}

// Imports inside `if TYPE_CHECKING:` are not executed at runtime.
let is_type_checking_block = checker.semantic().in_type_checking_block();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would make sense to return right here for all of these rules. The same argument about TYPE_CHECKING seems to apply to all of the rules that only check imports, and this would avoid all the separate checks and continues.

@ntBre ntBre added the rule Implementing or modifying a lint rule label Feb 23, 2026
@gcomneno
Copy link
Contributor Author

Thanks! I agree — since this check is import-only, it makes sense to early-return for TYPE_CHECKING blocks at the top level. I'll refactor accordingly and remove the per-rule continues.

@gcomneno gcomneno force-pushed the fix/s408-type-checking branch from 5ba556b to a22f0b2 Compare February 24, 2026 08:24
@gcomneno
Copy link
Contributor Author

Heads-up: the CI workflow is ending immediately with conclusion=action_required on this PR from a fork. Could a maintainer approve/enable the workflow run (or rerun CI) on their side? The local tests (cargo test -p ruff_linter flake8_bandit::) are passing.

@gcomneno gcomneno force-pushed the fix/s408-type-checking branch from 54e7e55 to a22f0b2 Compare February 24, 2026 08:29
@ntBre
Copy link
Contributor

ntBre commented Feb 24, 2026

Heads-up: the CI workflow is ending immediately with conclusion=action_required on this PR from a fork. Could a maintainer approve/enable the workflow run (or rerun CI) on their side? The local tests (cargo test -p ruff_linter flake8_bandit::) are passing.

Thanks, this is an intentional setting for first-time contributors. We'll notice and run CI, so you don't have to give us a heads-up.

@ntBre ntBre changed the title flake8-bandit: skip S408/S409 inside TYPE_CHECKING blocks [flake8-bandit] Allow suspicious imports in TYPE_CHECKING blocks (S401-S415) Feb 24, 2026
@astral-sh-bot
Copy link

astral-sh-bot bot commented Feb 24, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -1 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)

apache/airflow (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL

- airflow-core/src/airflow/cli/hot_reload.py:32:12: S404 `subprocess` module is possibly insecure

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
S404 1 0 1 0 0

@ntBre ntBre added the preview Related to preview mode features label Feb 24, 2026
@ntBre ntBre merged commit b2d856f into astral-sh:main Feb 24, 2026
67 of 109 checks passed
@gcomneno gcomneno deleted the fix/s408-type-checking branch February 24, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants