From af38046cb6edb07a72ef8f0e083a6704a309baf1 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 10 Aug 2026 18:11:10 +0000 Subject: [PATCH] chore(lint): ban the object.__setattr__ frozen-instance bypass Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- ruff-strict.toml | 5 ++ scripts/check_type_discipline.py | 55 ++++++++++++++++++- scripts/type_discipline_gate.py | 4 +- .../test_check_type_discipline.py | 40 +++++++++++++- type-discipline-budget.json | 3 + 5 files changed, 104 insertions(+), 3 deletions(-) diff --git a/ruff-strict.toml b/ruff-strict.toml index d58885fe848..dea6d5ac80c 100644 --- a/ruff-strict.toml +++ b/ruff-strict.toml @@ -31,3 +31,8 @@ max-args = 5 "typing_extensions.TypeGuard".msg = "Same as typing.TypeGuard." "typing.TypeIs".msg = "Unverified narrowing (the body is trusted). Parse into a concrete type instead." "typing_extensions.TypeIs".msg = "Same as typing.TypeIs." +# Frozen-instance bypass. TID251 only resolves imported qualified names, so it sees the +# `builtins.` form and never the bare `object.__setattr__` builtin, which is LIT010 in +# scripts/check_type_discipline.py. +"builtins.object.__setattr__".msg = "Writes through a frozen dataclass' guard. Build the final value and construct once, or dataclasses.replace()." +"builtins.object.__delattr__".msg = "Same as builtins.object.__setattr__." diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 43a4cb66484..7edc8cbe322 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -45,6 +45,14 @@ LIT009 `# type: ignore` in any shape (bare, with codes, with a reason). pyrightconfig.json sets enableTypeIgnoreComments to false, so basedpyright never honors it: the comment is inert dead syntax that suppresses nothing. Use `# pyright: ignore[ruleName] # ` instead. +LIT010 `object.__setattr__(...)` / `object.__delattr__(...)` call. Both write straight + through a frozen dataclass' guard, so the immutability the type promises is not + real and any holder of the "frozen" value can be mutated out from under. Build + the final value and construct once, or produce a new instance with + dataclasses.replace(). ruff-strict.toml bans the qualified `builtins.` form via + TID251, but banned-api only resolves imported names and never sees the bare + `object` builtin, which is the form that actually gets written; this flags that. + Suppress with `# setattr-ok: ` on the call's first line. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -95,6 +103,7 @@ MUTABLE_CONSTRUCTORS = frozenset(( QUALIFIED_CONSTRUCTORS = MUTABLE_CONSTRUCTORS - frozenset(("dict", "list", "set")) FREEZING_WRAPPERS = frozenset(("tuple", "frozenset", "MappingProxyType")) UNSAFE_GUARDS = frozenset(("TypeGuard", "TypeIs")) +FROZEN_BYPASS_DUNDERS = frozenset(("__setattr__", "__delattr__")) MIN_REASON_LEN = 3 NOQA_RE = re.compile( @@ -111,6 +120,7 @@ MUTABLE_OK_RE = re.compile(r"#\s*mutable-ok(?::\s*(?P.*))?") CAST_OK_RE = re.compile(r"#\s*cast-ok(?::\s*(?P.*))?") GUARD_OK_RE = re.compile(r"#\s*guard-ok(?::\s*(?P.*))?") KWARGS_OK_RE = re.compile(r"#\s*kwargs-ok(?::\s*(?P.*))?") +SETATTR_OK_RE = re.compile(r"#\s*setattr-ok(?::\s*(?P.*))?") # Suppression tokens that must each carry a reason (LIT005). OK_SUPPRESSIONS: tuple[tuple[str, re.Pattern[str]], ...] = ( @@ -118,6 +128,7 @@ OK_SUPPRESSIONS: tuple[tuple[str, re.Pattern[str]], ...] = ( ("cast-ok", CAST_OK_RE), ("guard-ok", GUARD_OK_RE), ("kwargs-ok", KWARGS_OK_RE), + ("setattr-ok", SETATTR_OK_RE), ) @@ -139,6 +150,7 @@ class Comments: cast_ok_lines: frozenset[int] guard_ok_lines: frozenset[int] kwargs_ok_lines: frozenset[int] + setattr_ok_lines: frozenset[int] # --------------------------------------------------------------------------- # @@ -194,7 +206,7 @@ def scan_comments(path: Path, source: str) -> tuple[Comments, tuple[Violation, . # tokenize raises TokenError (EOF mid-construct) or a SyntaxError subclass # (IndentationError / TabError) on malformed source; defer to ast.parse below, # which re-raises and is reported as LIT000 rather than crashing the run. - return Comments(frozenset(), frozenset(), frozenset(), frozenset()), () + return Comments(frozenset(), frozenset(), frozenset(), frozenset(), frozenset()), () def _lines_with(regex: re.Pattern[str]) -> frozenset[int]: return frozenset(line for line, text in comment_toks if _valid_ok(regex, text)) @@ -205,6 +217,7 @@ def scan_comments(path: Path, source: str) -> tuple[Comments, tuple[Violation, . cast_ok_lines=_lines_with(CAST_OK_RE), guard_ok_lines=_lines_with(GUARD_OK_RE), kwargs_ok_lines=_lines_with(KWARGS_OK_RE), + setattr_ok_lines=_lines_with(SETATTR_OK_RE), ), tuple(v for line, text in comment_toks for v in _comment_violations(path, line, text)), ) @@ -355,6 +368,45 @@ def iter_guard_violations(path: Path, tree: ast.AST, comments: Comments) -> Iter ) +# --------------------------------------------------------------------------- # +# Frozen-instance bypass (LIT010) +# --------------------------------------------------------------------------- # + + +def _frozen_bypass_dunder(node: ast.Call) -> str | None: + """The dunder name when this call is `object.__setattr__`/`__delattr__`, else None. + + Matched on the `object.` receiver rather than the attribute alone, so a class' + own `self.__setattr__(...)` or a `super().__setattr__(...)` cooperative call is + left alone; only the builtin that steps around the owner's guard is flagged. + """ + func = node.func + if not isinstance(func, ast.Attribute) or func.attr not in FROZEN_BYPASS_DUNDERS: + return None + receiver = func.value + is_object = (isinstance(receiver, ast.Name) and receiver.id == "object") or ( + isinstance(receiver, ast.Attribute) + and receiver.attr == "object" + and isinstance(receiver.value, ast.Name) + and receiver.value.id == "builtins" + ) + return func.attr if is_object else None + + +def iter_frozen_bypass_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]: + for node in ast.walk(tree): + if not isinstance(node, ast.Call) or node.lineno in comments.setattr_ok_lines: + continue + dunder = _frozen_bypass_dunder(node) + if dunder is not None: + yield Violation( + path, node.lineno, "LIT010", + f"`object.{dunder}` writes through a frozen dataclass' guard, so the " + f"immutability the type promises is not real; build the final value and " + f"construct once, or dataclasses.replace() (suppress: `# setattr-ok: `)", + ) + + # --------------------------------------------------------------------------- # # Mutable-collection construction (LIT002) # --------------------------------------------------------------------------- # @@ -478,6 +530,7 @@ def check_file(path: Path) -> tuple[Violation, ...]: *iter_annotation_violations(path, tree, comments), *iter_cast_violations(path, tree, comments), *iter_guard_violations(path, tree, comments), + *iter_frozen_bypass_violations(path, tree, comments), *iter_construction_violations(path, tree, comments), ) diff --git a/scripts/type_discipline_gate.py b/scripts/type_discipline_gate.py index bd8d16553b1..e138da0fc6d 100644 --- a/scripts/type_discipline_gate.py +++ b/scripts/type_discipline_gate.py @@ -14,7 +14,9 @@ without codes or reason), LIT006 (cast), LIT008 (`**kwargs`), and LIT009 (inert `# type: ignore`, dead syntax while enableTypeIgnoreComments is false) carry limits at or above their current count to ratchet down; LIT005 (`*-ok` suppression without a reason) is frozen at limit 0 so any net-new reasonless -suppression trips the gate; and LIT007 (TypeGuard/TypeIs) is a hard zero. +suppression trips the gate; LIT007 (TypeGuard/TypeIs) is a hard zero; and LIT010 +(`object.__setattr__`/`__delattr__` frozen-instance bypass) is capped at the +handful that exist today. ``--update`` ratchets a limit down by the violations this branch fixed relative to its branch point (the merge-base). """ diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 53d672fc4a8..29e52c25082 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -237,6 +237,44 @@ def test_typed_args_is_clean_but_kwargs_ok_suppresses(tmp_path): ) +# --------------------------------------------------------------------------- # +# Frozen-instance bypass (LIT010) — only the `object` receiver counts +# --------------------------------------------------------------------------- # + + +def test_object_setattr_and_delattr_are_flagged(tmp_path): + assert "LIT010" in _codes(tmp_path, "def f(o: object) -> None:\n object.__setattr__(o, 'a', 1)\n") + assert "LIT010" in _codes(tmp_path, "def f(o: object) -> None:\n object.__delattr__(o, 'a')\n") + qualified = "import builtins\ndef f(o: object) -> None:\n builtins.object.__setattr__(o, 'a', 1)\n" + assert "LIT010" in _codes(tmp_path, qualified) + + +def test_cooperative_setattr_is_not_flagged(tmp_path): + # An owner writing through its own __setattr__ (directly or via super()) is not a + # bypass; only the builtin that steps around the owner's guard is. + src = ( + "class X:\n" + " def __setattr__(self, name: str, value: int) -> None:\n" + " super().__setattr__(name, value)\n" + " self.__setattr__(name, value)\n" + ) + assert "LIT010" not in _codes(tmp_path, src) + + +def test_setattr_ok_with_reason_suppresses(tmp_path): + src = ( + "def f(o: object) -> None:\n" + " object.__setattr__(o, 'a', 1) # setattr-ok: rehydrating a frozen row from the DB\n" + ) + assert "LIT010" not in _codes(tmp_path, src) + + +def test_setattr_ok_without_reason_is_lit005(tmp_path): + codes = _codes(tmp_path, "def f(o: object) -> None:\n object.__setattr__(o, 'a', 1) # setattr-ok\n") + assert "LIT005" in codes + assert "LIT010" in codes + + # --------------------------------------------------------------------------- # # Budget integrity: every emittable LIT rule (bar the LIT000 read/parse error) is gated # --------------------------------------------------------------------------- # @@ -244,4 +282,4 @@ def test_typed_args_is_clean_but_kwargs_ok_suppresses(tmp_path): def test_budget_covers_exactly_the_checker_rules(): budget = json.loads((_REPO_ROOT / "type-discipline-budget.json").read_text()) - assert set(budget) == {f"LIT00{n}" for n in range(1, 10)} + assert set(budget) == {f"LIT{n:03d}" for n in range(1, 11)} diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 9976be98522..a61f03fa939 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -25,5 +25,8 @@ }, "LIT009": { "limit": 2460 + }, + "LIT010": { + "limit": 5 } }