Skip to content

Commit 501d243

Browse files
authored
Add a runtime shape guard for the duplicate-role-map check (#414)
The existing test walks the AST for ast.Dict literals, so a copy of _ROLE_PRIORITY written as a list of pairs, a dict comprehension, dict(zip(...)), an enum.IntEnum, or a runtime-assembled dict never produces an ast.Dict node and passes unnoticed. Add a second guard that imports every module under api, core, services, tasks, integrations, notifications, and schemas and judges the value each module-level name resolves to, not the statement that built it. core.security._ROLE_PRIORITY is exempted by object identity so a second object holding the same values still fails. Both guards are kept, and each is verified to work independently: five synthetic copies (one per evasion shape) are confirmed to slip past the AST guard and be caught by the runtime one, and a plain dict literal is confirmed to still be caught by the AST guard on its own. Closes #387
1 parent 56a5eb5 commit 501d243

2 files changed

Lines changed: 380 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,22 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.
297297
the vulnerability drawer is changed here; the rest are unaffected and
298298
tracked separately.
299299

300+
- **The duplicate-role-map guard only recognised dictionary literals.** The
301+
test protecting `core.security._ROLE_PRIORITY` from growing a second copy
302+
walked the AST for `ast.Dict` nodes, so a copy written as a list of
303+
`(role, grade)` pairs, a dict comprehension, `dict(zip(...))`, an
304+
`enum.IntEnum`, or a dict assembled by a helper function at runtime was
305+
invisible to it: none of those produce an `ast.Dict` node, only the value a
306+
reader would call the same shape once the module finished importing. A
307+
second guard now imports every module under `api`, `core`, `services`,
308+
`tasks`, `integrations`, `notifications`, and `schemas`, and judges the
309+
value each module-level name resolves to rather than the statement that
310+
built it. The two guards are kept side by side rather than one replacing
311+
the other, and each is verified independently: five synthetic copies, one
312+
per evasion shape above, are confirmed to slip past the AST guard and be
313+
caught by the runtime one, and a plain dict literal is confirmed to still
314+
be caught by the AST guard on its own.
315+
300316
- **The nightly vendored-spec run failed on a column the product had
301317
intentionally dropped.** The plaintext-to-ciphertext migration for webhook
302318
secrets (0084-0086) removed `projects.webhook_secret` once its encrypted
Lines changed: 364 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
# Copyright 2026 TRUSCA contributors
3+
"""Privilege order is written down once, no matter how the copy is built.
4+
5+
``test_role_priority_has_one_home.py`` already guards ``_ROLE_PRIORITY``
6+
having exactly one home, but it does that by walking the AST for
7+
``ast.Dict`` literals. That is a guard on syntax, and syntax is one of many
8+
ways to produce a dict: ``{"viewer": 1, "developer": 2}`` and
9+
``dict(zip(("viewer", "developer"), (1, 2)))`` are two spellings of the same
10+
object, and only the first one is a literal the AST guard can see. A copy
11+
built with a dict comprehension, assembled at runtime by a helper function,
12+
or expressed as ``list[tuple[str, int]]`` or an ``enum.IntEnum`` never
13+
becomes an ``ast.Dict`` node, so the existing guard is silent about all of
14+
them (issue #387).
15+
16+
This file adds a second guard that judges the *value* every consumer module
17+
ends up holding after import, not the expression that produced it. A
18+
duplicate is a duplicate because of what it contains (two or more role
19+
names, all belonging to the same closed set), regardless of the statement
20+
that built it. The two guards are kept independent on purpose (CLAUDE.md
21+
hardening rule #8): this file does not import anything from
22+
``test_role_priority_has_one_home.py``, and its own tests (below) confirm
23+
each guard catches what the other misses rather than assuming it from the
24+
fact that both are green.
25+
26+
``core.security._ROLE_PRIORITY`` itself is exempted by object identity, not
27+
by module path or variable name: a second dict living inside
28+
``core/security.py`` under a different name, holding the same values, is
29+
still a copy and still fails. Identity is also why a module that legitimately
30+
imports and re-exports the real object (``from core.security import
31+
_ROLE_PRIORITY``) is not flagged: it holds a reference to the one object,
32+
not a value that merely looks the same.
33+
"""
34+
35+
from __future__ import annotations
36+
37+
import enum
38+
import importlib
39+
import importlib.util
40+
import pkgutil
41+
import textwrap
42+
from collections.abc import Iterator, Mapping, Sequence
43+
from pathlib import Path
44+
from types import ModuleType
45+
46+
import pytest
47+
48+
BACKEND_ROOT = Path(__file__).resolve().parents[2]
49+
50+
SEARCH_DIRS = ("api", "core", "services", "tasks", "integrations", "notifications", "schemas")
51+
52+
#: Mirrors the AST guard's threshold and rationale: one name is a lookup, not
53+
#: a ranking. Kept as its own constant in this file rather than imported from
54+
#: the AST guard module, so the two guards stay independent implementations
55+
#: of the same rule instead of one guard calling into the other's internals.
56+
_MIN_GRADES_TO_COUNT = 2
57+
58+
#: Modules that must be skipped when walking SEARCH_DIRS, each with a reason.
59+
#: A 2026-09-07 sweep imported all 348 modules under SEARCH_DIRS with no
60+
#: DATABASE_URL, REDIS_URL, or SECRET_KEY set at all (every accessor in
61+
#: core.config reads os.getenv() lazily per CLAUDE.md core rule #11, so
62+
#: nothing connects at import time). If a future module grows an
63+
#: import-time side effect that needs a live dependency, add it here with a
64+
#: reason rather than letting an ImportError take the whole guard down
65+
#: silently (the guard would stop partway through SEARCH_DIRS and every
66+
#: module after the failure would go unchecked, which looks identical to a
67+
#: passing run).
68+
SKIPPED_MODULES: dict[str, str] = {}
69+
70+
71+
def _role_names() -> frozenset[str]:
72+
from models.auth import ROLE_VALUES
73+
74+
return frozenset(ROLE_VALUES)
75+
76+
77+
def _is_role_keyed_mapping(value: object, roles: frozenset[str]) -> bool:
78+
"""True for any ``Mapping`` (dict literal, comprehension, ``dict(zip(...))``,
79+
``dict(...)`` call, ``MappingProxyType``, ...) whose string keys are two or
80+
more role names."""
81+
if not isinstance(value, Mapping):
82+
return False
83+
keys = list(value.keys())
84+
if not all(isinstance(key, str) for key in keys):
85+
return False
86+
if len(keys) < _MIN_GRADES_TO_COUNT:
87+
return False
88+
return set(keys) <= roles
89+
90+
91+
def _is_role_keyed_pair_sequence(value: object, roles: frozenset[str]) -> bool:
92+
"""True for a list/tuple of 2-element pairs whose first elements are two
93+
or more role names: the shape a dict takes before someone calls
94+
``dict(...)`` on it, or chooses not to."""
95+
if isinstance(value, str | bytes) or not isinstance(value, Sequence):
96+
return False
97+
firsts: list[str] = []
98+
for item in value:
99+
if isinstance(item, str | bytes) or not isinstance(item, Sequence):
100+
return False
101+
if len(item) != 2:
102+
return False
103+
first = item[0]
104+
if not isinstance(first, str):
105+
return False
106+
firsts.append(first)
107+
if len(firsts) < _MIN_GRADES_TO_COUNT:
108+
return False
109+
return set(firsts) <= roles
110+
111+
112+
def _is_role_keyed_enum(value: object, roles: frozenset[str]) -> bool:
113+
"""True for an ``enum.Enum``/``enum.IntEnum`` whose member names are two
114+
or more role names: an ordering encoded in member *values* rather than
115+
in a mapping at all."""
116+
if not (isinstance(value, type) and issubclass(value, enum.Enum)):
117+
return False
118+
names = [member.name for member in value]
119+
if len(names) < _MIN_GRADES_TO_COUNT:
120+
return False
121+
return set(names) <= roles
122+
123+
124+
def find_role_shaped_values(
125+
module: ModuleType, roles: frozenset[str], *, canonical: object
126+
) -> list[str]:
127+
"""Names of module-level attributes whose *value* looks like a role
128+
priority table, judged by shape rather than by how it was written.
129+
130+
``canonical`` is excluded by identity (``is``), not by name or module
131+
path: any other object, even one holding identical values, is a copy.
132+
"""
133+
offenders: list[str] = []
134+
for name, value in vars(module).items():
135+
if value is canonical:
136+
continue
137+
if (
138+
_is_role_keyed_mapping(value, roles)
139+
or _is_role_keyed_pair_sequence(value, roles)
140+
or _is_role_keyed_enum(value, roles)
141+
):
142+
offenders.append(name)
143+
return offenders
144+
145+
146+
def _iter_consumer_modules() -> Iterator[tuple[str, ModuleType]]:
147+
"""Import every module under SEARCH_DIRS, top package included.
148+
149+
``pkgutil.walk_packages`` only yields submodules, not the package named
150+
by ``prefix`` itself, so the top-level package (e.g. plain ``core``,
151+
whose ``__init__.py`` could in principle hold a copy too) is imported
152+
separately.
153+
"""
154+
for directory in SEARCH_DIRS:
155+
top = importlib.import_module(directory)
156+
if directory not in SKIPPED_MODULES:
157+
yield directory, top
158+
if not hasattr(top, "__path__"):
159+
continue
160+
for modinfo in pkgutil.walk_packages(top.__path__, prefix=directory + "."):
161+
if modinfo.name in SKIPPED_MODULES:
162+
continue
163+
module = importlib.import_module(modinfo.name)
164+
yield modinfo.name, module
165+
166+
167+
def test_every_skip_states_a_reason() -> None:
168+
"""An allow-list without reasons becomes a place to silence this guard."""
169+
for module_name, reason in SKIPPED_MODULES.items():
170+
assert len(reason.split()) >= 4, (
171+
f"the reason for skipping {module_name} is too short to be a reason"
172+
)
173+
174+
175+
def test_the_guard_knows_the_role_names() -> None:
176+
roles = _role_names()
177+
assert "viewer" in roles and "super_admin" in roles, roles
178+
179+
180+
def test_role_priority_has_no_lookalike_anywhere_else() -> None:
181+
from core.security import _ROLE_PRIORITY
182+
183+
roles = _role_names()
184+
offenders: list[str] = []
185+
for module_name, module in _iter_consumer_modules():
186+
for attr_name in find_role_shaped_values(module, roles, canonical=_ROLE_PRIORITY):
187+
offenders.append(f"{module_name}.{attr_name}")
188+
189+
assert not offenders, (
190+
"a value shaped like the role-priority table exists outside "
191+
"core.security, regardless of the statement that built it (dict "
192+
"literal, comprehension, dict(zip(...)), a list of pairs, an "
193+
"IntEnum, or a function assembling one at runtime): "
194+
+ ", ".join(offenders)
195+
+ ". Import core.security.highest_role, or _ROLE_PRIORITY itself, "
196+
"instead of reconstructing the order."
197+
)
198+
199+
200+
def test_a_second_object_with_the_same_values_still_fails() -> None:
201+
"""Identity exemption, not value equality: proven by breaking it.
202+
203+
A copy that is correct on the day it is written is exactly what this
204+
guard exists to catch, so a dict holding the *same* values as
205+
``_ROLE_PRIORITY`` but living at a different address must still be
206+
flagged. If this passed the guard, the exemption would be doing
207+
``==`` under the hood and every future copy would slip through as long
208+
as nobody let the numbers drift on day one.
209+
"""
210+
from core.security import _ROLE_PRIORITY
211+
212+
roles = _role_names()
213+
identical_copy = dict(_ROLE_PRIORITY)
214+
assert identical_copy == _ROLE_PRIORITY
215+
assert identical_copy is not _ROLE_PRIORITY
216+
217+
fake_module = ModuleType("role_priority_lookalike_test_module")
218+
fake_module._SECOND_MAP = identical_copy # type: ignore[attr-defined]
219+
220+
offenders = find_role_shaped_values(fake_module, roles, canonical=_ROLE_PRIORITY)
221+
assert offenders == ["_SECOND_MAP"], (
222+
"a dict with the same values as _ROLE_PRIORITY, but a different "
223+
"object, was not flagged; the identity check has degraded into an "
224+
"equality check"
225+
)
226+
227+
228+
# ---------------------------------------------------------------------------
229+
# Self-test: five ways to build a role-priority table that are not a dict
230+
# literal, each proven to trip the guard (CLAUDE.md hardening rule #7: a
231+
# guard that always passes tells you nothing about what it protects).
232+
# ---------------------------------------------------------------------------
233+
234+
_EVASION_MODULE_SOURCES: dict[str, str] = {
235+
"tuple_list": textwrap.dedent(
236+
"""
237+
# A list of (role, grade) pairs, which never becomes a dict at all, so an
238+
# ast.Dict-based guard has nothing to find here.
239+
_ROLE_ORDER_ITEMS = [
240+
("viewer", 1),
241+
("developer", 2),
242+
("team_admin", 3),
243+
("super_admin", 4),
244+
]
245+
"""
246+
),
247+
"dict_comprehension": textwrap.dedent(
248+
"""
249+
# A comprehension, not a dict literal: ast.walk finds a
250+
# ast.DictComp node here, not an ast.Dict.
251+
_grades = (("viewer", 1), ("developer", 2), ("team_admin", 3), ("super_admin", 4))
252+
_ROLE_ORDER = {name: grade for name, grade in _grades}
253+
"""
254+
),
255+
"dict_zip_call": textwrap.dedent(
256+
"""
257+
# dict(zip(...)) is a function call; the AST guard only recognises
258+
# ast.Dict nodes, so a call expression is invisible to it.
259+
_ROLE_NAMES = ("viewer", "developer", "team_admin", "super_admin")
260+
_ROLE_GRADES = (1, 2, 3, 4)
261+
_ROLE_ORDER = dict(zip(_ROLE_NAMES, _ROLE_GRADES, strict=True))
262+
"""
263+
),
264+
"int_enum": textwrap.dedent(
265+
"""
266+
# An ordering encoded in enum member values, not in any dict at all.
267+
import enum
268+
269+
270+
class RoleOrder(enum.IntEnum):
271+
viewer = 1
272+
developer = 2
273+
team_admin = 3
274+
super_admin = 4
275+
"""
276+
),
277+
"runtime_assembly_function": textwrap.dedent(
278+
"""
279+
# Built by a function at import time. The module-level name resolves
280+
# to a plain dict once the module finishes executing, same as a
281+
# literal would, but no ast.Dict node in this source spells it out.
282+
def _build_role_order():
283+
order = {}
284+
order["viewer"] = 1
285+
order["developer"] = 2
286+
order["team_admin"] = 3
287+
order["super_admin"] = 4
288+
return order
289+
290+
291+
_ROLE_ORDER = _build_role_order()
292+
"""
293+
),
294+
}
295+
296+
297+
def _load_module_from_source(name: str, source: str, tmp_path: Path) -> ModuleType:
298+
module_file = tmp_path / f"{name}.py"
299+
module_file.write_text(source, encoding="utf-8")
300+
spec = importlib.util.spec_from_file_location(name, module_file)
301+
assert spec is not None and spec.loader is not None
302+
module = importlib.util.module_from_spec(spec)
303+
spec.loader.exec_module(module)
304+
return module
305+
306+
307+
@pytest.mark.parametrize("shape_name", sorted(_EVASION_MODULE_SOURCES))
308+
def test_runtime_guard_catches_every_evasion_shape(shape_name: str, tmp_path: Path) -> None:
309+
"""Each shape below is exactly the kind of thing #387 reported: it holds
310+
the same judgement as ``_ROLE_PRIORITY`` and would pass the AST guard
311+
clean (proven separately below), yet must fail this one."""
312+
roles = _role_names()
313+
module = _load_module_from_source(
314+
f"role_priority_evasion_{shape_name}", _EVASION_MODULE_SOURCES[shape_name], tmp_path
315+
)
316+
offenders = find_role_shaped_values(module, roles, canonical=object())
317+
assert offenders, (
318+
f"the {shape_name} evasion shape was not caught by the runtime "
319+
"shape guard; issue #387 exists because exactly this kind of shape "
320+
"slipped past the AST-only guard"
321+
)
322+
323+
324+
@pytest.mark.parametrize("shape_name", sorted(_EVASION_MODULE_SOURCES))
325+
def test_ast_guard_alone_misses_every_evasion_shape(shape_name: str, tmp_path: Path) -> None:
326+
"""The other half of hardening rule #8: confirm the AST guard is really
327+
blind to these, rather than assuming it from issue #387's description.
328+
If this test ever fails, the AST guard is no longer the reason this
329+
runtime guard needs to exist for that shape."""
330+
import ast
331+
332+
from tests.unit.test_role_priority_has_one_home import _dict_literals_keyed_by_role
333+
334+
roles = _role_names()
335+
module_file = tmp_path / f"role_priority_evasion_{shape_name}.py"
336+
module_file.write_text(_EVASION_MODULE_SOURCES[shape_name], encoding="utf-8")
337+
tree = ast.parse(module_file.read_text(encoding="utf-8"))
338+
assert _dict_literals_keyed_by_role(tree, roles) == [], (
339+
f"the {shape_name} evasion shape was caught by the AST guard after "
340+
"all; either it grew an actual dict literal keyed by role names, or "
341+
"the AST guard changed to look for more than ast.Dict"
342+
)
343+
344+
345+
def test_runtime_guard_alone_is_not_needed_to_catch_a_plain_dict_literal() -> None:
346+
"""The AST guard's own reason to exist, checked from this file so the
347+
two tests do not silently depend on each other: a plain dict literal
348+
keyed by role names is caught by the AST guard with no runtime import
349+
required at all."""
350+
import ast
351+
352+
from tests.unit.test_role_priority_has_one_home import _dict_literals_keyed_by_role
353+
354+
roles = _role_names()
355+
source = textwrap.dedent(
356+
"""
357+
_ROLE_ORDER = {"viewer": 1, "developer": 2, "team_admin": 3, "super_admin": 4}
358+
"""
359+
)
360+
tree = ast.parse(source)
361+
assert _dict_literals_keyed_by_role(tree, roles), (
362+
"the AST guard no longer recognises a plain dict literal keyed by "
363+
"role names, which is its own reason to exist"
364+
)

0 commit comments

Comments
 (0)