CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 18s
CI & Build / TypeScript typecheck (push) Successful in 20s
CI & Build / Python tests (push) Successful in 42s
CI & Build / Build & push image (push) Successful in 27s
Milestone #254 step 1 (#2286). A design system becomes a record Scribe holds rather than prose in a rulebook: a named set of tokens with an OPTIONAL parent, so a family system carries the house style and an app system carries only what it changes. Answering "what does this app alter?" is then `list its tokens` — nothing to compute. `parent_id` is the whole model. It replaces both an `always_on` flag (a family system is one with no parent) and a subscription join table (a project points at ONE system; the chain supplies the rest) — less schema than the rulebook shape it mirrors. Two decisions the task left open, settled here: - **Token values are JSONB keyed by mode**, not `value_light`/`value_dark` columns. The deciding argument was not flexibility, it was ambiguity: in a child system an unset mode means "inherit", in a root it means "not mode-dependent", and as columns both are NULL and the resolver cannot tell them apart. As a map, resolution is `{**parent, **child}` at every level with no special case for roots. Against it: queryability — but nothing filters tokens by value in SQL, so that buys a query no caller makes. - **`group_name` is free text, no CHECK enum.** Groupings are each design system's own vocabulary; a whitelist would bake one install's kit into the schema. No CHECK is introduced anywhere, so rule #36 does not fire. The cascade lives in `services/design_cascade.py` as pure functions over a `{id: parent_id}` map, importing nothing — which is what lets both the service and `access.py` use it without a cycle, and lets a test state a whole hierarchy in one literal. Cycles are refused on WRITE by walking up from the proposed parent (the cheap direction), and survived on READ by a visited-set, because a loop from a direct DB edit must truncate rather than hang. ACL (rule #78) is deliberately asymmetric: owning a system grants write, reaching one through a project you can see grants READ ONLY. An editor on a shared project must not be able to rewrite the family system every other project in that family resolves through. Also renames `services/design_system.py` -> `design_rulebook_import.py`. It is the #251 prose extractor, whose role is already scheduled to become a one-shot importer (#2288), and leaving it one character away from the new `design_systems.py` was a trap for every later session. Rule #115 throughout: nothing seeds a system or implies a default. An install with zero design systems is ordinary, not degraded.
172 lines
6.5 KiB
Python
172 lines
6.5 KiB
Python
"""The two ACL predicates behind list queries.
|
|
|
|
`get_note_permission` answers "may I read THIS note?" one row at a time, which a
|
|
list query can't use. These express the same resolution as set membership. They
|
|
are pure SQL builders — group membership is a subquery rather than a fetched
|
|
list, so they need no session and callers' unit tests need not know they exist.
|
|
|
|
Two scopes, deliberately different (decision note 2094):
|
|
readable_* — everything the ACL permits, for explicit acts (a typed search, a
|
|
fetch by id).
|
|
browsable_* — owner + project access only, for passive surfaces (browse lists,
|
|
facet counts, the process→skill manifest).
|
|
|
|
Assertions compile each clause to SQL and inspect its shape.
|
|
"""
|
|
import pytest
|
|
|
|
from scribe.services.access import (
|
|
browsable_notes_clause,
|
|
notes_visibility_clause,
|
|
readable_notes_clause,
|
|
)
|
|
|
|
|
|
def _sql(clause) -> str:
|
|
return str(clause.compile(compile_kwargs={"literal_binds": True}))
|
|
|
|
|
|
def _read(user_id: int = 7) -> str:
|
|
return _sql(readable_notes_clause(user_id))
|
|
|
|
|
|
def _browse(user_id: int = 7) -> str:
|
|
return _sql(browsable_notes_clause(user_id))
|
|
|
|
|
|
# --- read scope --------------------------------------------------------------
|
|
|
|
def test_read_scope_covers_ownership_and_every_share_path():
|
|
sql = _read()
|
|
assert "notes.user_id = 7" in sql # 1. ownership
|
|
assert "note_shares" in sql # 2/3. direct or group note share
|
|
assert "notes.project_id IN" in sql # 4. inherited from a shared project
|
|
assert "project_shares" in sql
|
|
|
|
|
|
def test_read_scope_resolves_group_membership_in_sql():
|
|
"""Group ids are a subquery, not a pre-fetched list — that's what keeps this
|
|
a pure function with no session of its own."""
|
|
sql = _read()
|
|
assert "group_memberships.user_id = 7" in sql
|
|
# Both the note-level and project-level share lookups consult it. Count FROM
|
|
# clauses rather than bare occurrences — each rendered subquery names the
|
|
# table in SELECT, FROM and WHERE — and compare loosely, since the assertion
|
|
# is about the arms existing, not about SQLAlchemy's formatting.
|
|
assert sql.count("FROM group_memberships") >= 2
|
|
assert _browse().count("FROM group_memberships") >= 1
|
|
|
|
|
|
def test_read_scope_is_never_the_whole_table():
|
|
"""Guard against the predicate degrading to always-true, which would expose
|
|
every user's notes to every other user."""
|
|
sql = _read().lower()
|
|
assert " true" not in sql
|
|
assert "1 = 1" not in sql
|
|
|
|
|
|
# --- browse scope: the trust boundary ---------------------------------------
|
|
|
|
def test_browse_scope_excludes_direct_note_shares():
|
|
"""The whole point of the narrower scope. If `note_shares` leaks in here, a
|
|
record someone shared one-to-one with the operator lands in their own browse
|
|
list, facet counts and skill manifest as though they had recorded it."""
|
|
assert "note_shares" not in _browse()
|
|
|
|
|
|
def test_browse_scope_keeps_ownership_and_project_access():
|
|
sql = _browse()
|
|
assert "notes.user_id = 7" in sql # your own records
|
|
assert "project_shares" in sql # a project shared with you
|
|
assert "projects" in sql # a project you own
|
|
|
|
|
|
def test_browse_scope_is_strictly_narrower_than_read_scope():
|
|
"""Browse must never surface something read scope wouldn't also allow, or a
|
|
list could show a record the caller cannot then open."""
|
|
browse, read = _browse(), _read()
|
|
assert "note_shares" in read and "note_shares" not in browse
|
|
for arm in ("notes.user_id = 7", "project_shares"):
|
|
assert arm in browse and arm in read
|
|
|
|
|
|
def test_browse_scope_is_never_the_whole_table():
|
|
sql = _browse().lower()
|
|
assert " true" not in sql
|
|
assert "1 = 1" not in sql
|
|
|
|
|
|
# --- the scope resolver ------------------------------------------------------
|
|
|
|
def test_own_scope_is_ownership_alone():
|
|
"""The near-duplicate gate depends on this: its verdict must not turn on
|
|
another person's records, or it would refuse a write and point the caller at
|
|
something they may not be able to edit."""
|
|
sql = _sql(notes_visibility_clause(7, "own"))
|
|
assert sql == "notes.user_id = 7"
|
|
|
|
|
|
def test_scope_resolver_maps_to_the_right_clauses():
|
|
assert _sql(notes_visibility_clause(7, "browse")) == _browse()
|
|
assert _sql(notes_visibility_clause(7, "read")) == _read()
|
|
|
|
|
|
def test_scope_defaults_to_the_narrowest():
|
|
"""A caller that forgets to choose must be wrong in the safe direction."""
|
|
assert _sql(notes_visibility_clause(7)) == _sql(notes_visibility_clause(7, "own"))
|
|
|
|
|
|
def test_unknown_scope_is_rejected_loudly():
|
|
"""Silently falling back would turn a typo into a data-exposure bug."""
|
|
with pytest.raises(ValueError):
|
|
notes_visibility_clause(7, "everything")
|
|
|
|
|
|
@pytest.mark.parametrize("clause_fn", [readable_notes_clause, browsable_notes_clause])
|
|
def test_clauses_are_pure_builders(clause_fn):
|
|
"""Synchronous and side-effect free — no coroutine, no session of their own.
|
|
|
|
This is the property that keeps them usable: when they opened their own
|
|
session, every unrelated service test had to know they existed and stub them,
|
|
and four test modules broke the moment a service started calling one."""
|
|
import inspect
|
|
assert not inspect.iscoroutinefunction(clause_fn)
|
|
assert _sql(clause_fn(7)) # builds without touching a database
|
|
|
|
|
|
# --- design systems ----------------------------------------------------------
|
|
#
|
|
# A design system is reachable two ways: you own it, or you can see a project
|
|
# that inherits from it. The gap between what those two grant is the invariant
|
|
# worth guarding — see get_design_system_permission.
|
|
|
|
@pytest.mark.asyncio
|
|
@pytest.mark.parametrize(
|
|
"permission, readable, writable",
|
|
[
|
|
("owner", True, True),
|
|
# Project-derived. Being an EDITOR on a shared project must not confer
|
|
# the right to rewrite the family system that project inherits from —
|
|
# that would let one project's collaborator restyle every other project
|
|
# in the family.
|
|
("viewer", True, False),
|
|
(None, False, False),
|
|
],
|
|
)
|
|
async def test_reaching_a_design_system_via_a_project_reads_but_never_writes(
|
|
permission, readable, writable
|
|
):
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
from scribe.services.access import (
|
|
can_read_design_system,
|
|
can_write_design_system,
|
|
)
|
|
|
|
with patch(
|
|
"scribe.services.access.get_design_system_permission",
|
|
AsyncMock(return_value=permission),
|
|
):
|
|
assert await can_read_design_system(1, 3) is readable
|
|
assert await can_write_design_system(1, 3) is writable
|