A security linter with a bad false-positive rate does not make a codebase
safer. It teaches everyone on the team a reflex — add # noqa, move on — and
that reflex is applied uniformly, to the true positives as well. You end up
worse than with no linter, because now the true positive is annotated with a
comment that says a human looked at it.
So the question is never “should we enable the security rules”. It is which rules earn a hard failure, which earn a warning, and which are a code-review prompt that should not be automated at all.
Ruff’s S rules are a port of bandit. Here they are, sorted by that judgement
rather than by rule number.
Tier 1: hard failure, genuinely almost always a defect
S110 / S112 — try: ... except: pass and except: continue.
try:
result = parse(payload)
except Exception:
pass
This is not error handling; it is error deletion. Whatever went wrong is now
unobservable — no log line, no metric, no traceback — and the code continues
with result unbound or stale. Almost every instance is either a TODO that
was never done or a specific exception someone could not be bothered to name.
The legitimate case exists (a best-effort cleanup, a cache write that may fail)
and it has an idiomatic spelling that says so:
with contextlib.suppress(FileNotFoundError):. That is one specific exception,
named, in a construct whose whole meaning is “this failure is expected and
ignorable”. If you cannot name the exception, you have not decided that it is
ignorable.
S324 — insecure hash function. hashlib.md5(...) or sha1(...) used for
anything security-relevant. The fix is sha256. The genuine false positive is
non-security use — a cache key, a content-addressed filename, a dedupe hash —
and Python gives you a way to say so explicitly:
digest = hashlib.md5(key.encode(), usedforsecurity=False).hexdigest()
That keyword exists precisely so this rule can distinguish the two cases, and it
also makes the code work on a FIPS-mode build where MD5 is otherwise disabled.
Prefer it over # noqa.
S105–S107 — hardcoded passwords. High signal in application code. The
false positives are test fixtures and constants named *_TOKEN that hold a
header name, both of which are easy to scope away.
DTZ (the whole family) — naive datetimes. Not an S rule, and it belongs
in this tier anyway, because a naive datetime crossing a boundary is a
data-corruption bug rather than a style issue. datetime.now() gives you a
naive datetime in whatever timezone the machine happens to be in, which
differs between your laptop, CI and production. datetime.utcnow() is worse: it
returns a naive datetime that represents UTC, so .timestamp() on it
reinterprets it as local time. Under TZ=America/Los_Angeles that is a silent
28,800-second error in stored data.
DTZ requires an explicit tz=. There is no meaningful false-positive rate,
and the bugs it prevents are the kind you find months later in a report.
Tier 2: real, but needs per-occurrence review
S603 / S607 — subprocess call, and partial executable path.
S603 fires on essentially every subprocess.run, including the correct
list-form call with check=True that this track spends a whole problem
teaching. It cannot tell a safe invocation from an unsafe one, because the
difference is whether an argument came from untrusted input — which is a
data-flow question a linter of this class does not answer.
S607 fires on ["git", "status"] because git is resolved through PATH,
which an attacker who controls the environment could redirect. That is a real
attack in a setuid or CI-runner context and irrelevant in a container you built.
Both are worth having as a warning that prompts a review comment, and
actively harmful as a merge blocker: teams respond by adding a blanket
# noqa: S603 template, at which point the rule catches nothing.
What is worth a hard failure is the thing these rules cannot see:
shell=True with anything interpolated. S602/S604/S605/S606 cover the
shell variants and have a much better ratio.
S301 — pickle. Almost always right, and there is a narrow legitimate use
(a local cache your process wrote a moment ago). Review each one, and note that
the review question is not “is this pickle safe today” but “can anyone ever
influence these bytes”.
S311 — random for cryptographic purposes. The rule cannot tell a
password reset token from a dice roll in a game. But when it is right it is very
right, so treat it as a review prompt:
token = secrets.token_urlsafe(32) # session ids, reset tokens, API keys
value = random.randint(1, 6) # simulations, sampling, tests
random is a Mersenne Twister. Observing a few hundred outputs is enough to
recover its internal state and predict every subsequent one, which is fine for a
Monte Carlo simulation and catastrophic for a password reset link.
Tier 3: correct in one context, wrong in another
S101 — use of assert.
This is the rule that most needs a per-directory setting, because it is simply correct in library code and wrong in tests.
In library code: python -O strips every assert statement. So this
def transfer(account: Account, amount: Decimal) -> None:
assert amount > 0, "amount must be positive"
is not validation. It is validation that vanishes in exactly the deployment
where you were trying to save cycles. Anything you need to hold at runtime is an
if ... raise ValueError.
In tests: assert is how pytest works, and its rewriting is the reason failure
messages are readable. Blocking it is nonsense.
Configure it, do not argue about it:
[tool.ruff.lint]
select = ["E", "F", "S", "DTZ", "UP", "ANN"]
[tool.ruff.lint.per-file-ignores]
"tests/**" = ["S101"]
💡A reviewer flags if token == expected_token: in an authentication path and asks for hmac.compare_digest. Is that pedantry?
click to reveal
No, and the reason is a timing side channel that is easier to exploit than most people assume.
== on str or bytes short-circuits at the first differing byte. Comparing
"aaaa..." against the real token returns after one byte; comparing
"baaa..." when the token starts with b returns after two. The difference is
nanoseconds, but an attacker gets to average over millions of requests, and
averaging is exactly what defeats noise. You recover the secret one byte at a
time — linear work instead of exponential.
hmac.compare_digest(token, expected) compares in time that depends only on the
length of the inputs, not their contents. It is one import and one function
call, and it applies to any comparison where one side is a secret: API keys,
session tokens, HMAC signatures, password hashes (though for passwords you
should be using a KDF whose verify function already does this).
Two footnotes. It does not hide the length, so do not rely on it for that.
And no linter reliably catches this — S105 looks for hardcoded secrets, not
for insecure comparisons of them — which is precisely why it belongs on a
review checklist rather than in a rule set.
The configuration that actually works
Three tiers, three mechanisms:
[tool.ruff.lint]
select = ["E", "F", "B", "S", "DTZ", "UP", "ANN"]
ignore = [
"S603", # subprocess call — reviewed per call site, see CONTRIBUTING.md
"S607", # partial executable path — we control the image
]
[tool.ruff.lint.per-file-ignores]
"tests/**" = ["S101", "S105", "S311"]
"scripts/**" = ["S603", "S607"]-
Tier 1 blocks the merge. Nobody argues, because the false-positive rate is
near zero and each one has an explicit escape hatch (
usedforsecurity=False,contextlib.suppress). - Tier 2 is disabled globally with a comment saying where the review happens. A disabled rule with a documented reason is honest. A rule that fires forty times per pull request and is suppressed forty times is not.
- Tier 3 is scoped per directory, because the right answer genuinely differs by context.
And the residue — constant-time comparison, secrets versus random where the
linter cannot tell, whether these particular bytes crossed a trust boundary —
goes on a written review checklist. That is not a failure of tooling. Some
questions require knowing where the data came from, and no rule engine of this
class knows that.