peter
e9c60f72cd
Capture's routes (downloads, files, public watch links) and the token list no longer build SQL, call the session or set a model's fields: reads are downloads/queries, writes the unit's service (service, folders, links, quota), and each write flushes for the handler to commit. Whose a row is goes through userscope (owned_by, owned_row), which owned_or_404 now uses too. The fence holds Capture below its routes as well: it reads only Catalog's and Plex's tables, writes no other unit's, filters owners through userscope and commits only in its jobs and the upload ingest. A test per service use case, among them that the tray lists only the caller's files.
508 lines
20 KiB
Python
508 lines
20 KiB
Python
"""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. A route may NAME a unit's model as a type (under
|
|
`TYPE_CHECKING`), never use one as a value.
|
|
|
|
Below the HTTP layer, a fenced unit's own code keeps to its edges (`FENCED`, `READS`): see the
|
|
second half of this file.
|
|
"""
|
|
|
|
import ast
|
|
from pathlib import Path
|
|
|
|
import app.tables # noqa: F401 — every unit's tables, for the ownership check
|
|
from app.db 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,
|
|
"feed.py": 36,
|
|
"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,
|
|
"runs.py": 1,
|
|
"saved_views.py": 17,
|
|
"scheduler.py": 3,
|
|
"search.py": 46,
|
|
"sync.py": 38,
|
|
"tags.py": 10,
|
|
}
|
|
|
|
# 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 _type_only_imports(tree: ast.AST) -> set[ast.AST]:
|
|
"""The imports under `if TYPE_CHECKING:` — they name a type for an annotation and are never
|
|
loaded at run time, so a model imported there cannot be queried or written."""
|
|
out: set[ast.AST] = set()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.If):
|
|
test = node.test
|
|
name = test.id if isinstance(test, ast.Name) else getattr(test, "attr", None)
|
|
if name == "TYPE_CHECKING":
|
|
for child in node.body:
|
|
out |= set(ast.walk(child))
|
|
return out
|
|
|
|
|
|
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()
|
|
typing_only = _type_only_imports(tree)
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.ImportFrom) and node not in typing_only:
|
|
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,
|
|
"from typing import TYPE_CHECKING\nif TYPE_CHECKING:\n from app.downloads.models import Job": 0,
|
|
"import typing\nif typing.TYPE_CHECKING:\n from app.models import DownloadJob": 0,
|
|
"if TYPE_CHECKING:\n pass\nfrom app.downloads.models import DownloadJob": 1,
|
|
}
|
|
|
|
|
|
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"
|
|
]
|
|
|
|
|
|
# --- inside a unit: its package --------------------------------------------------------------
|
|
# A unit whose sprint has landed is fenced beyond its HTTP layer too. Its own code reads another
|
|
# unit's tables only along a declared edge (the feed joins Video with VideoState in one query; a
|
|
# boundary that split it would cost the speed the app is for), and writes another unit's tables
|
|
# never — that goes through the owner's service. Whose a row is, it says through `userscope`, and
|
|
# it commits only where a job owns its session.
|
|
|
|
# Each unit's code outside the HTTP layer: its package, and the entry point that runs its jobs.
|
|
UNIT_PACKAGES = {"capture": ("downloads/", "worker.py")}
|
|
# The units fenced so far; the rest join with their sprint.
|
|
FENCED = {"capture"}
|
|
# Whose tables a unit's own code may READ. Core code reads a fenced unit's tables through its queries.
|
|
READS = {"capture": {"catalog", "plex"}}
|
|
# The columns that say whose a row is.
|
|
OWNER_COLUMNS = {"user_id", "to_user_id", "from_user_id", "created_by"}
|
|
# Where a fenced unit's code may commit: the jobs that own their session (the worker and what it
|
|
# runs, the GC, the backfill — none of them inside a request), and the one write that must commit
|
|
# where it stands: an upload's rows and the file it placed on disk land together or not at all, so
|
|
# a failed commit can remove the file (`upload.ingest_upload`). Everything else flushes; the handler
|
|
# commits.
|
|
UNIT_COMMITS = {
|
|
"capture": {
|
|
"worker.py",
|
|
"downloads/backfill.py",
|
|
"downloads/gc.py",
|
|
"downloads/jobdb.py",
|
|
"downloads/pipeline.py",
|
|
"downloads/upload.py:ingest_upload",
|
|
}
|
|
}
|
|
SQL_WRITES = {"insert", "update", "delete", "pg_insert"}
|
|
|
|
|
|
def _package_unit(rel: str) -> str | None:
|
|
for unit, prefixes in UNIT_PACKAGES.items():
|
|
if any(rel == p or (p.endswith("/") and rel.startswith(p)) for p in prefixes):
|
|
return unit
|
|
return None
|
|
|
|
|
|
def _app_files():
|
|
for path in sorted(APP.rglob("*.py")):
|
|
rel = path.relative_to(APP).as_posix()
|
|
if not rel.startswith("routes/"):
|
|
yield rel, path.read_text(encoding="utf-8")
|
|
|
|
|
|
def _imported_models(tree: ast.AST) -> dict[str, str]:
|
|
"""The unit models a module imports at run time (a `TYPE_CHECKING` import names a type only),
|
|
by the name it binds them to."""
|
|
typing_only = _type_only_imports(tree)
|
|
out = {}
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.ImportFrom) and node not in typing_only:
|
|
module = node.module or ""
|
|
if module.startswith("app.") and "models" in module:
|
|
for alias in node.names:
|
|
if alias.name in UNIT_MODEL_NAMES:
|
|
out[alias.asname or alias.name] = alias.name
|
|
return out
|
|
|
|
|
|
def _enclosing_functions(tree: ast.AST) -> dict[ast.AST, str]:
|
|
out = {}
|
|
for fn in ast.walk(tree):
|
|
if isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)):
|
|
for node in ast.walk(fn):
|
|
out.setdefault(node, fn.name)
|
|
return out
|
|
|
|
|
|
def package_violations(rel: str, source: str) -> list[str]:
|
|
"""What a module outside the HTTP layer does against a fenced unit's boundary, as `line: what`."""
|
|
tree = ast.parse(source)
|
|
mine = _package_unit(rel)
|
|
found = []
|
|
models = _imported_models(tree)
|
|
for name in models.values():
|
|
theirs = UNIT_MODEL_NAMES[name]
|
|
if theirs == mine or (mine not in FENCED and theirs not in FENCED):
|
|
continue
|
|
if theirs not in READS.get(mine or "", set()):
|
|
found.append(f"0: {mine or 'core'} imports {theirs}'s {name}")
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Name):
|
|
continue
|
|
written = None
|
|
if node.func.id in models:
|
|
written = models[node.func.id]
|
|
elif node.func.id in SQL_WRITES and node.args and isinstance(node.args[0], ast.Name):
|
|
written = models.get(node.args[0].id)
|
|
if written is None:
|
|
continue
|
|
theirs = UNIT_MODEL_NAMES[written]
|
|
if theirs != mine and (mine in FENCED or theirs in FENCED):
|
|
found.append(f"{node.lineno}: {mine or 'core'} writes {theirs}'s {written}")
|
|
if mine in FENCED:
|
|
found += _owner_filters(tree)
|
|
functions = _enclosing_functions(tree)
|
|
allowed = UNIT_COMMITS.get(mine, set())
|
|
for node in ast.walk(tree):
|
|
if (
|
|
isinstance(node, ast.Call)
|
|
and isinstance(node.func, ast.Attribute)
|
|
and node.func.attr == "commit"
|
|
and rel not in allowed
|
|
and f"{rel}:{functions.get(node)}" not in allowed
|
|
):
|
|
found.append(f"{node.lineno}: commits in {functions.get(node)}")
|
|
return sorted(found, key=lambda v: int(v.split(":")[0]))
|
|
|
|
|
|
def _owner_filters(tree: ast.AST) -> list[str]:
|
|
"""A comparison on a table's owner column written inline (`Model.user_id == …`), not through
|
|
`userscope.owned_by`."""
|
|
found = []
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Compare):
|
|
for side in (node.left, *node.comparators):
|
|
if (
|
|
isinstance(side, ast.Attribute)
|
|
and isinstance(side.value, ast.Name)
|
|
and side.value.id in set(UNIT_MODEL_NAMES) | CORE_MODELS
|
|
and side.attr in OWNER_COLUMNS
|
|
):
|
|
found.append(f"{node.lineno}: filters {side.value.id}.{side.attr} inline")
|
|
return found
|
|
|
|
|
|
def test_a_fenced_units_code_keeps_to_its_edges():
|
|
hits = {rel: found for rel, src in _app_files() if (found := package_violations(rel, src))}
|
|
assert hits == {}, (
|
|
"read another unit's tables only along READS, write them only through their service, say"
|
|
" whose a row is through userscope, commit only where UNIT_COMMITS says"
|
|
)
|
|
|
|
|
|
def test_a_fenced_units_routes_filter_owners_through_userscope():
|
|
hits = {
|
|
rel: found
|
|
for rel, src in _http_files()
|
|
if _unit_of(rel) in FENCED and (found := _owner_filters(ast.parse(src)))
|
|
}
|
|
assert hits == {}
|
|
|
|
|
|
def test_every_unit_package_exists():
|
|
for prefixes in UNIT_PACKAGES.values():
|
|
for p in prefixes:
|
|
assert (APP / p).exists(), p
|
|
for commits in UNIT_COMMITS.values():
|
|
for entry in commits:
|
|
assert (APP / entry.split(":")[0]).exists(), entry
|
|
|
|
|
|
PACKAGE_CASES = {
|
|
# (module path, source): violations
|
|
("downloads/x.py", "from app.models import Video"): 0, # capture reads catalog
|
|
("downloads/x.py", "from app.models import Playlist"): 1, # no capture→reader edge
|
|
("downloads/x.py", "from app.models import Video\nVideo(id=1)"): 1, # a write across
|
|
("downloads/x.py", "from app.models import Video\nupdate(Video)"): 1,
|
|
("downloads/x.py", "from app.downloads.models import DownloadJob\nDownloadJob()"): 0,
|
|
("search.py", "from app.downloads.models import DownloadJob"): 1, # core reads capture
|
|
("search.py", "from app.models import Video\nVideo()"): 0, # neither side fenced yet
|
|
("downloads/x.py", "q = DownloadJob.user_id == 1"): 1,
|
|
("downloads/x.py", "q = owned_by(DownloadJob.user_id, 1)"): 0,
|
|
("downloads/x.py", "def f(db):\n db.commit()"): 1,
|
|
("downloads/gc.py", "def f(db):\n db.commit()"): 0,
|
|
("downloads/upload.py", "def ingest_upload(db):\n db.commit()"): 0,
|
|
("downloads/upload.py", "def other(db):\n db.commit()"): 1,
|
|
(
|
|
"downloads/x.py",
|
|
"from typing import TYPE_CHECKING\nif TYPE_CHECKING:\n from app.models import Playlist",
|
|
): 0,
|
|
}
|
|
|
|
|
|
def test_the_package_detector_sees_each_kind():
|
|
for (rel, source), expected in PACKAGE_CASES.items():
|
|
assert len(package_violations(rel, source)) == expected, (rel, source)
|