CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 1m6s
CI & Build / Build & push image (push) Successful in 33s
Spike #3128 found the storage sound and the retrieval vocabulary frozen before `issue` shipped (0065). Five things, in the order they had to land. **The mirror (rec 5, the data-integrity one).** `notes.data` is DERIVED from a snippet's body, but only `update_snippet` knew that. `update_note` is a hasattr loop with no snippet awareness, and both doors reach it — so PATCH /api/notes/<snippet_id> {body} rewrote the body and left the mirror behind. `snippet_fields` PREFERS the mirror, so the row went on reporting its old repo/path/symbol to the location reverse lookup and to prior-art recall while displaying its new body: surfaced with full authority, and wrong. `snippets.recompose_data` rebuilds it from the body, carrying `verification` and `provenance` (neither is in the body to parse). An explicit `data` still wins, so every snippet-service write is untouched. **One facet table (rec 3), before adding any facet.** The type predicate was written three times — SQL, Python over semantic candidates, and a ternary computing the `is_task` pre-filter — and agreed only by luck. Adding `issue` to the SQL arm alone would have set the pre-filter to is_task=False, handed the Python arm a candidate set with no tasks in it, and returned an empty semantic half for the Issues facet forever with nothing red. `_FACETS` now generates all three. The Python arm also regains the `status IS NULL` half its SQL twin always had. **Issue and spike become facets (rec 2).** 435 issues — 17% of every task — were filterable nowhere on the human surface, while retired `plan` (90 rows) had a chip of its own. `_VALID_TYPES` was a hand-kept copy and is now derived. `plan` stays a valid facet for its legacy rows; it loses its chip. **Snippets stop being half-present in the feed (rec 4).** All 90 were in the All list, in no count, wearing an empty badge, and opening in the note editor. Counts now group by task_kind — every kind for the same two round-trips, which is why `issue` had no number — and total includes snippets, so the All chip matches the list it labels. Snippet cards route to /snippets/:id. **The prose that excused it (rec 6).** `snippet_fields` and the `data` column both still said pre-0070 rows were "never backfilled". True when 0070 landed, false since `backfill_snippet_data` shipped, and it read as licence for a stale mirror. Tests: the pre-filter can never exclude a row its own facet accepts (the regression, parameterised over every facet); both dialects select exactly their own rows; an unknown facet matches nothing; the mirror follows a body or title write, carries the verdict, and yields to an explicit `data`. `compiled_sql` moves to tests/helpers rather than becoming a third copy. Write-up: note #3161.
171 lines
6.5 KiB
Python
171 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,
|
|
)
|
|
from tests.helpers import compiled_sql
|
|
|
|
_sql = compiled_sql
|
|
|
|
|
|
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
|