Files
FabledScribe/tests/test_services_access_visibility.py
bvandeusen f80401d58e
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
fix(knowledge): the browse vocabulary catches up three kinds, and a snippet's mirror survives the generic door (#3128 recs 2-6)
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.
2026-08-28 12:06:43 -04:00

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