From d66174ed16071cff5c630be2cf9b1040a1106716 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 18 Oct 2026 23:01:12 +0200 Subject: [PATCH] test(arch): a unit's HTTP layer does no database work beyond its pin The fence is switched on unit by unit: each route file's database work (SQL built, session data calls, model attributes set, unit tables imported) is pinned at today's count and may only shrink. A core route may not reach into a unit's tables. Every model has an owner. --- backend/tests/test_unit_fences.py | 306 ++++++++++++++++++++++++++++++ docs/gates.md | 1 + 2 files changed, 307 insertions(+) create mode 100644 backend/tests/test_unit_fences.py diff --git a/backend/tests/test_unit_fences.py b/backend/tests/test_unit_fences.py new file mode 100644 index 00000000..f933560d --- /dev/null +++ b/backend/tests/test_unit_fences.py @@ -0,0 +1,306 @@ +"""The HTTP layer of a unit does no database work of its own, held in code rather than prose. + +A route validates, calls its unit's `queries` (reads) or `service` (writes), serializes and commits. +What it may not do is build SQL, call the session's data methods, change a model's attributes or +import a unit's tables: when a route did those, ownership, the commit and the conditional `WHERE` +lived in a different place in each handler, and a fix in one never reached its siblings. + +The rule is switched on unit by unit. `ALLOWED` pins what each HTTP file still does today, and the +pin is exact: a count that rises fails (new database work in a route), and one that falls fails +until its entry is lowered (the fence only tightens, as `baseline.json` does for basedpyright). An +empty `ALLOWED` is the end state — a file listed there is a debt, not an exemption. + +A core route (identity, admin, ops) is not a unit's HTTP layer and may use the session; it may +still not reach into a unit's tables. +""" + +import ast +from pathlib import Path + +from app.models import Base + +APP = Path(__file__).parent.parent / "app" +ROUTES = APP / "routes" + +# Each unit's tables. A new model is placed here (or in CORE_MODELS) by the test below, so a table +# never joins the schema without an owner. +UNIT_MODELS = { + "capture": { + "MediaAsset", + "DownloadProfile", + "FileFolder", + "DownloadJob", + "DownloadShare", + "DownloadQuota", + "DownloadLink", + "DownloadCredential", + "LibraryState", + }, + "reader": { + "VideoState", + "Playlist", + "PlaylistItem", + "SavedView", + "ComfortPin", + "Tag", + "ChannelTag", + }, + "catalog": { + "Channel", + "Subscription", + "Video", + "SearchFind", + "ExploredChannel", + "BlockedChannel", + }, + "plex": { + "PlexLibrary", + "PlexShow", + "PlexSeason", + "PlexItem", + "PlexPerson", + "PlexCollection", + "PlexState", + "PlexLink", + "PlexPlaylist", + "PlexPlaylistItem", + }, +} +CORE_MODELS = { + "User", + "OAuthToken", + "AuthToken", + "Invite", + "ApiQuotaUsage", + "QuotaEvent", + "AuditLog", + "LogEvent", + "DebugSession", + "DebugScreenshot", + "AppState", + "JobRun", + "SystemConfig", + "Notification", + "ApiToken", + "ApiTokenGrant", +} + +# Which unit each HTTP file belongs to (path relative to app/routes; a folder covers its files). +# Anything not listed is a core route. +UNIT_ROUTES = { + "downloads.py": "capture", + "files.py": "capture", + "public.py": "capture", + "feed.py": "reader", + "playlists.py": "reader", + "saved_views.py": "reader", + "discover.py": "reader", + "tags.py": "reader", + "channels.py": "catalog", + "search.py": "catalog", + "sync.py": "catalog", + "plex/": "plex", +} + +# What each HTTP file still does, by file: the debt a unit's sprint pays off. Exact counts. A core +# route's count is only the unit tables it imports; it is paid by the sprint of the unit it reaches. +ALLOWED = { + "channels.py": 83, + "discover.py": 7, + "downloads.py": 88, + "feed.py": 36, + "files.py": 58, + "playlists.py": 72, + "plex/_shared.py": 1, + "plex/admin.py": 12, + "plex/browse.py": 93, + "plex/collections.py": 33, + "plex/detail.py": 29, + "plex/discover.py": 44, + "plex/people.py": 20, + "plex/playback.py": 42, + "plex/playlists.py": 80, + "public.py": 12, + "runs.py": 1, + "saved_views.py": 17, + "scheduler.py": 3, + "search.py": 46, + "sync.py": 38, + "tags.py": 10, + "tokens.py": 1, +} + +# The session methods that read or write data. `commit` and `rollback` stay with the handler. +SESSION_DATA = { + "add", + "add_all", + "delete", + "execute", + "get", + "merge", + "flush", + "refresh", + "query", + "scalar", + "scalars", + "expunge", +} +# What a route may name from SQLAlchemy: the session's type, for its `db: Session` parameter, and +# the errors its commit raises (the commit is the handler's, so is catching what it throws). +SQLALCHEMY_ALLOWED = {"Session"} +SQLALCHEMY_ALLOWED_MODULES = {"sqlalchemy.exc"} +# Objects a route owns and may set attributes on; anything else it writes to is a model. +OWN_OBJECTS = {"self", "cls", "request", "response"} +UNIT_MODEL_NAMES = {name: unit for unit, names in UNIT_MODELS.items() for name in names} + + +def _unit_of(rel: str) -> str | None: + for key, unit in UNIT_ROUTES.items(): + if rel == key or (key.endswith("/") and rel.startswith(key)): + return unit + return None + + +def _session_names(tree: ast.AST) -> set[str]: + """The names a module binds to a database session: a parameter typed `Session`, or a name + assigned (or `with`-bound) from `SessionLocal()`.""" + names = set() + for node in ast.walk(tree): + if isinstance(node, ast.arg) and isinstance(node.annotation, ast.Name): + if node.annotation.id == "Session": + names.add(node.arg) + made = None + if isinstance(node, ast.Assign) and len(node.targets) == 1: + made, target = node.value, node.targets[0] + elif isinstance(node, ast.withitem): + made, target = node.context_expr, node.optional_vars + if ( + isinstance(made, ast.Call) + and isinstance(made.func, ast.Name) + and made.func.id == "SessionLocal" + and isinstance(target, ast.Name) + ): + names.add(target.id) + return names + + +def _root(node: ast.expr) -> ast.expr: + while isinstance(node, (ast.Attribute, ast.Subscript)): + node = node.value + return node + + +def violations(source: str, unit: str | None) -> list[str]: + """Each piece of database work in an HTTP file, as `line: what`. A core route (`unit` None) is + held only to the last rule: no unit's tables.""" + tree = ast.parse(source) + found = [] + sql_names = set() + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + module = node.module or "" + for alias in node.names: + bound = alias.asname or alias.name + if ( + module.split(".")[0] == "sqlalchemy" + and module not in SQLALCHEMY_ALLOWED_MODULES + and alias.name not in SQLALCHEMY_ALLOWED + ): + sql_names.add(bound) + if ( + alias.name in UNIT_MODEL_NAMES + and module.startswith("app.") + and "models" in module + ): + found.append( + f"{node.lineno}: imports {UNIT_MODEL_NAMES[alias.name]}'s {alias.name}" + ) + elif isinstance(node, ast.Import): + for alias in node.names: + if alias.name.split(".")[0] == "sqlalchemy": + sql_names.add((alias.asname or alias.name).split(".")[0]) + if unit is None: + return found + sessions = _session_names(tree) + for node in ast.walk(tree): + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load) and node.id in sql_names: + found.append(f"{node.lineno}: uses sqlalchemy's {node.id}") + if isinstance(node, ast.Call): + f = node.func + if isinstance(f, ast.Attribute) and isinstance(f.value, ast.Name): + if f.value.id in sessions and f.attr in SESSION_DATA: + found.append(f"{node.lineno}: {f.value.id}.{f.attr}()") + targets = [] + if isinstance(node, ast.Assign): + targets = node.targets + elif isinstance(node, (ast.AugAssign, ast.AnnAssign)): + targets = [node.target] + for target in targets: + if isinstance(target, ast.Attribute): + base = _root(target) + if not (isinstance(base, ast.Name) and base.id in OWN_OBJECTS): + found.append(f"{node.lineno}: sets .{target.attr}") + return sorted(found, key=lambda v: int(v.split(":")[0])) + + +def _http_files(): + for path in sorted(ROUTES.rglob("*.py")): + rel = path.relative_to(ROUTES).as_posix() + yield rel, path.read_text(encoding="utf-8") + + +def test_every_table_has_an_owner(): + tables = {mapper.class_.__name__ for mapper in Base.registry.mappers} + owned = set(UNIT_MODEL_NAMES) | CORE_MODELS + assert tables - owned == set(), "a model with no unit: place it in UNIT_MODELS or CORE_MODELS" + assert owned - tables == set(), "UNIT_MODELS/CORE_MODELS name a model that no longer exists" + assert not set(UNIT_MODEL_NAMES) & CORE_MODELS + + +def test_every_listed_route_exists(): + rels = [rel for rel, _ in _http_files()] + for key in UNIT_ROUTES: + assert any(rel == key or (key.endswith("/") and rel.startswith(key)) for rel in rels), key + assert set(ALLOWED) <= set(rels), set(ALLOWED) - set(rels) + + +def test_no_http_file_does_database_work_beyond_its_pin(): + drift = {} + for rel, src in _http_files(): + found = violations(src, _unit_of(rel)) + pinned = ALLOWED.get(rel, 0) + if len(found) != pinned: + drift[rel] = (pinned, len(found), found if len(found) > pinned else "lower its pin") + assert drift == {}, ( + "an HTTP file's database work moved off its pin (pinned, now, what): move new work into" + " the unit's queries/service; when a count fell, lower ALLOWED to it" + ) + + +# The detector itself: each kind of database work it must see, and what it must leave alone. +DETECTOR_CASES = { + "from sqlalchemy import select\nq = select(1)": 1, + "import sqlalchemy as sa\nsa.select(1).where(sa.true())": 2, + "from sqlalchemy import func\nn = func.count()": 1, + "from sqlalchemy.orm import Session\ndef f(db: Session): db.commit(); db.rollback()": 0, + "from sqlalchemy.orm import Session\ndef f(db: Session): db.add(x)": 1, + "def f(conn: Session): conn.execute(q)": 1, + "from app.db import SessionLocal\nwith SessionLocal() as s:\n s.get(X, 1)": 1, + "from app.db import SessionLocal\ns = SessionLocal()\ns.scalars(q)": 1, + "def f(job): job.status = 'x'": 1, + "def f(job): job.meta['k'] = 1": 0, + "def f(job): job.parts.count += 1": 1, + "def f(self, request, response): self.x = 1; request.state.y = 2; response.status_code = 3": 0, + "from app.models import DownloadJob, User": 1, + "from app.capture.models import DownloadJob as J": 1, + "def f(cache): cache.get('k')": 0, + "from sqlalchemy.exc import IntegrityError\ntry:\n pass\nexcept IntegrityError:\n pass": 0, +} + + +def test_the_detector_sees_each_kind_of_database_work(): + for source, expected in DETECTOR_CASES.items(): + assert len(violations(source, "capture")) == expected, source + assert violations("from app.models import Video\nfrom sqlalchemy import select", None) == [ + "1: imports catalog's Video" + ] diff --git a/docs/gates.md b/docs/gates.md index 75537e98..14276a61 100644 --- a/docs/gates.md +++ b/docs/gates.md @@ -113,6 +113,7 @@ project checks needs a row. | CVE gate | HIGH/CRITICAL with a fix available only | `--ignore-unfixed` + `--severity HIGH,CRITICAL`: a finding with no fix cannot be acted on by an upgrade; it shows in the scan output. A fixable one fails the publish, and the only way past it is `.trivyignore.yaml`. | 0 open (E32 S1) | | npm audit | `--omit=dev --audit-level=high` | What ships to the browser, at high or above. The dev toolchain's advisories were burned down by hand in E32 S1 (18 → 0). | 0 | | knip | `entry` — one file added | Not a narrowing: knip's Vite plugin reads entries from index.html only, so the shared link's page (`watch.html` → `src/watchMain.tsx`) is named here, or the lane reports that whole entry unused. It adds files to the lane, never takes any out. | 1 entry | +| unit fence | the database work the HTTP layer still does, pinned | `backend/tests/test_unit_fences.py`: a unit's route file may not build SQL, call the session's data methods, set a model's attributes or import a unit's tables; a core route may not import a unit's tables. Switched on unit by unit: `ALLOWED` pins each file's count exactly, so a rise fails and a fall fails until the pin is lowered — the list may only shrink. End state: empty, and the pin mechanism goes. | 826 in 23 files (E42 S0) | | React Compiler | the functions it skips, pinned | `components/compilerCoverage.contract.test.ts` — the list may only shrink. End state: only VirtualGrid (TanStack Virtual, an IncompatibleLibrary) remains. | 93 (E32 S1) → 29 (E32 S3) → 1, VirtualGrid (E32 S4) | ## Accepted findings