mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[F090] drop auditor from write_roles on main-pm-board / board-private
The auditor is a silent, read-only observer on every channel, but the channel catalog (roboco/foundation/policy/communications.py) listed it in write_roles for main-pm-board and board-private 'for parity' with the legacy CHANNEL_ACCESS table, while the actual silent-observer rule was enforced only at the say/dm guard (content_actions._NO_COMMS_ROLES) and PermissionService.can_write_channel's auditor short-circuit. That left the catalog-only enforcement path — the HTTP messaging route (messages.py send_message -> validate_channel_access) — authorizing an auditor write that both the say/dm guard and PermissionService would have blocked. A reader of the catalog also believed the auditor could post to those channels, which is false. Fix: remove Role.AUDITOR from write_roles on both channels (main-pm + board remain writers; ceo remains a writer on board-private). The auditor stays in read_roles, so its silent read is unchanged. silent_roles is left empty (matches the announcements precedent: auditor reads via read_roles, not the silent bucket) — the DB seed and silent_observers field are untouched. Logical-regression check: the auditor's read access on both channels is byte-for-byte preserved (still in read_roles, so validate_channel_access read returns True via the direct list); the legitimate writers (main-pm, product-owner, head-marketing, ceo) are untouched; CHANNEL_ACCESS is derived from the spec so the foundation/seed drift tests self-adjust; PermissionService.can_write_channel already short-circuited auditor to False everywhere, so no behavior change there; AUDITOR_SILENT_ACCESS is unchanged (auditor not added to silent_roles -> no DB silent_observers change -> no group-access behavior change); the say/dm _NO_COMMS_ROLES guard is unchanged. Tests: 3 new in test_channel_access.py — auditor write on main-pm-board/board-private now raises ChannelAccessDeniedError (RED before: returned True), auditor read still True, main-pm/ceo still write.
This commit is contained in:
@@ -219,9 +219,14 @@ CHANNELS: dict[str, ChannelSpec] = {
|
||||
silent_roles=_AUDITOR_ONLY,
|
||||
),
|
||||
# -- Management channels --------------------------------------------------
|
||||
# Legacy CHANNEL_ACCESS lists auditor as both read AND write here. We
|
||||
# preserve that behaviour for parity; the runtime guard that downgrades
|
||||
# auditor to silent lives in services.
|
||||
# The auditor is a silent, read-only observer on every channel — its
|
||||
# read membership is declared in read_roles, but it must NOT appear in
|
||||
# write_roles. Previously the catalog listed the auditor as a writer
|
||||
# here "for parity" with the legacy CHANNEL_ACCESS table, while the
|
||||
# actual silent-observer rule was enforced only at the say/dm guard.
|
||||
# That left the catalog-only enforcement path (the HTTP messaging route
|
||||
# -> validate_channel_access) authorizing an auditor write that the
|
||||
# guard would have blocked. The catalog now matches the enforced rule.
|
||||
"main-pm-board": ChannelSpec(
|
||||
slug="main-pm-board",
|
||||
description="Main PM and Board communication",
|
||||
@@ -229,9 +234,7 @@ CHANNELS: dict[str, ChannelSpec] = {
|
||||
read_roles=frozenset(
|
||||
{Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING, Role.AUDITOR}
|
||||
),
|
||||
write_roles=frozenset(
|
||||
{Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING, Role.AUDITOR}
|
||||
),
|
||||
write_roles=frozenset({Role.MAIN_PM, Role.PRODUCT_OWNER, Role.HEAD_MARKETING}),
|
||||
silent_roles=frozenset(),
|
||||
),
|
||||
"board-private": ChannelSpec(
|
||||
@@ -247,9 +250,7 @@ CHANNELS: dict[str, ChannelSpec] = {
|
||||
Role.MAIN_PM,
|
||||
}
|
||||
),
|
||||
write_roles=frozenset(
|
||||
{Role.PRODUCT_OWNER, Role.HEAD_MARKETING, Role.AUDITOR, Role.CEO}
|
||||
),
|
||||
write_roles=frozenset({Role.PRODUCT_OWNER, Role.HEAD_MARKETING, Role.CEO}),
|
||||
silent_roles=frozenset(),
|
||||
),
|
||||
# -- Special / broadcast channels -----------------------------------------
|
||||
|
||||
@@ -75,3 +75,35 @@ def test_validate_channel_access_silent_observer_can_read() -> None:
|
||||
"""Line 80: silent observers can read."""
|
||||
# backend-cell has 'auditor' as silent observer.
|
||||
assert validate_channel_access("auditor", "backend-cell", "read") is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Auditor is a silent, read-only observer — the catalog must not grant it
|
||||
# write_roles on any channel. main-pm-board / board-private used to list the
|
||||
# auditor in write_roles (legacy parity), which let the catalog-only
|
||||
# enforcement path (the HTTP messaging route -> validate_channel_access)
|
||||
# authorize an auditor write that the say/dm guard would have blocked.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("channel", ["main-pm-board", "board-private"])
|
||||
def test_validate_channel_access_auditor_cannot_write_management_channels(
|
||||
channel: str,
|
||||
) -> None:
|
||||
"""Auditor must not be in write_roles for any channel — silent observer."""
|
||||
with pytest.raises(ChannelAccessDeniedError):
|
||||
validate_channel_access("auditor", channel, "write")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("channel", ["main-pm-board", "board-private"])
|
||||
def test_validate_channel_access_auditor_can_still_read_management_channels(
|
||||
channel: str,
|
||||
) -> None:
|
||||
"""Removing write access must not regress the auditor's silent read."""
|
||||
assert validate_channel_access("auditor", channel, "read") is True
|
||||
|
||||
|
||||
def test_validate_channel_access_main_pm_still_writes_main_pm_board() -> None:
|
||||
"""The legitimate writers (main-pm / board / ceo) are untouched."""
|
||||
assert validate_channel_access("main-pm", "main-pm-board", "write") is True
|
||||
assert validate_channel_access("ceo", "board-private", "write") is True
|
||||
|
||||
Reference in New Issue
Block a user