test(acl): drive the wire tests from core's conformance vectors

The end-to-end tests asserted reply[7] == PERM_ACL_ADMIN, which follows
the constant wherever it goes — the one thing these tests exist to stop.
They now assert the literal bytes from openhop_core's acl_conformance
table, so both repos are pinned to the same numbers rather than to each
other's vocabulary. Checked out against 2a55dd6 with these tests copied
in, all seven fail.

Also cover the fail-closed import guard, which was the only untested
path protecting against a mixed-version install, plus its happy path so
a guard that always fires cannot pass unnoticed.
This commit is contained in:
agessaman committed 2026-08-21 18:56:23 -07:00
1 parent 7a2fb9b2df
commit f91d2e9504
2 files changed
+120 -46

No files matched your search

+61 -5
View File
@@ -13,6 +13,8 @@ from pathlib import Path
import pytest
import yaml
from openhop_core.protocol.acl_conformance import ROLE_MASK, ROLE_VALUES
from repeater.handler_helpers.acl import (
ACL,
PERM_ACL_ADMIN,
@@ -41,11 +43,12 @@ class _Id:
def test_role_constants_match_firmware_clientacl_h():
assert PERM_ACL_ROLE_MASK == 3
assert PERM_ACL_GUEST == 0
assert PERM_ACL_READ_ONLY == 1
assert PERM_ACL_READ_WRITE == 2
assert PERM_ACL_ADMIN == 3
"""Literal pin against the firmware header, via the shared vector table."""
assert PERM_ACL_ROLE_MASK == ROLE_MASK == 0x03
assert PERM_ACL_GUEST == ROLE_VALUES["guest"] == 0x00
assert PERM_ACL_READ_ONLY == ROLE_VALUES["read_only"] == 0x01
assert PERM_ACL_READ_WRITE == ROLE_VALUES["read_write"] == 0x02
assert PERM_ACL_ADMIN == ROLE_VALUES["admin"] == 0x03
# Every role must be distinct; GUEST and READ_WRITE used to collide on 0x01.
roles = {PERM_ACL_GUEST, PERM_ACL_READ_ONLY, PERM_ACL_READ_WRITE, PERM_ACL_ADMIN}
assert len(roles) == 4
@@ -222,3 +225,56 @@ def test_every_role_name_is_allowed_by_the_openapi_schema():
role_name(PERM_ACL_ADMIN),
}
assert emitted <= set(enum), f"roles missing from the OpenAPI enum: {emitted - set(enum)}"
def test_acl_refuses_to_import_against_a_core_without_the_role_constants(monkeypatch):
"""The fail-closed guard must actually fire, with a message that helps.
A core lacking these symbols still builds the login reply's admin byte
from ``permissions & 0x02``, which would announce a room server's
READ_WRITE clients as admins. Refusing to import is the safe failure, and
it is worth a test because nothing else exercises that path.
"""
import importlib
import sys
import types
import openhop_core.protocol.constants as real_constants
stub = types.ModuleType("openhop_core.protocol.constants")
for name in dir(real_constants):
if not name.startswith("__"):
setattr(stub, name, getattr(real_constants, name))
for missing in (
"PERM_ACL_ROLE_MASK",
"PERM_ACL_GUEST",
"PERM_ACL_READ_ONLY",
"PERM_ACL_READ_WRITE",
"PERM_ACL_ADMIN",
"acl_is_admin",
"acl_role",
):
delattr(stub, missing)
monkeypatch.setitem(sys.modules, "openhop_core.protocol.constants", stub)
monkeypatch.delitem(sys.modules, "repeater.handler_helpers.acl")
with pytest.raises(ImportError) as exc:
importlib.import_module("repeater.handler_helpers.acl")
message = str(exc.value)
assert "openhop_core is too old" in message
assert "fix/login-perms" in message
def test_acl_imports_normally_against_the_installed_core():
"""Companion to the guard test: the happy path must still work.
Guards that always fire are worse than no guard, and the test above
manipulates sys.modules.
"""
import importlib
module = importlib.import_module("repeater.handler_helpers.acl")
assert module.PERM_ACL_ADMIN == 0x03
assert module.is_admin_permissions(0x03) is True
+59 -41
View File
@@ -23,18 +23,25 @@ from openhop_core.protocol.constants import (
ROUTE_TYPE_FLOOD,
)
from repeater.handler_helpers.acl import (
ACL,
PERM_ACL_ADMIN,
PERM_ACL_GUEST,
PERM_ACL_READ_WRITE,
)
from openhop_core.protocol.acl_conformance import OUTBOUND
from repeater.handler_helpers.acl import ACL
ROOM_CFG = {
"type": "room_server",
"settings": {"admin_password": "room-admin", "guest_password": "room-guest"},
}
# How each conformance vector's (server_type, credential) is produced here.
# The ACL is built with these passwords; "" exercises allow_read_only.
CREDENTIALS = {
("repeater", "admin_password"): ("rpt-admin", None),
("repeater", "guest_password"): ("rpt-guest", None),
("repeater", "blank_read_only"): ("", None),
("room_server", "admin_password"): ("room-admin", ROOM_CFG),
("room_server", "guest_password"): ("room-guest", ROOM_CFG),
}
def _build_login_packet(
client: LocalIdentity, server: LocalIdentity, password: str, *, room: bool = False
@@ -101,46 +108,57 @@ async def _login_reply(acl: ACL, password: str, *, room_config: dict = None) ->
@pytest.mark.asyncio
async def test_repeater_admin_login_is_admin_on_the_wire():
"""The bug from #388: byte 7 must decode to ADMIN, not READ_WRITE."""
@pytest.mark.parametrize(
"server_type, credential, admin_code, permissions",
OUTBOUND,
ids=[f"{srv}-{cred}" for srv, cred, _, _ in OUTBOUND],
)
async def test_outbound_conformance_vectors(server_type, credential, admin_code, permissions):
"""The real ACL plus the real core handler must emit the literal bytes.
This is the only place both halves of the fix meet, and the expectations
are literals from openhop_core.protocol.acl_conformance rather than
PERM_ACL_* — a symbolic assertion would follow the constants if they ever
drift away from the mesh again, which is exactly how #388 stayed hidden.
"""
password, room_config = CREDENTIALS[(server_type, credential)]
acl = ACL(
admin_password="rpt-admin",
guest_password="rpt-guest",
allow_read_only=True,
)
reply = await _login_reply(acl, password, room_config=room_config)
assert reply[6] == admin_code
assert reply[7] == permissions
@pytest.mark.asyncio
async def test_admin_is_role_three_not_the_0x02_bit():
"""The single assertion that would have caught #388.
A stock client computes (perms & 3) == 3. Our admin used to be 0x02, which
that expression reads as READ_WRITE.
"""
acl = ACL(admin_password="rpt-admin", guest_password="rpt-guest")
reply = await _login_reply(acl, "rpt-admin")
assert reply[6] == 1 # legacy is_admin flag
assert reply[7] == PERM_ACL_ADMIN # ACL byte modern clients read
assert reply[7] & 0x03 == 3 # what a stock client computes
assert reply[7] != PERM_ACL_READ_WRITE # the value we used to send
assert reply[7] & 0x03 == 3
assert reply[7] != 0x02
@pytest.mark.asyncio
async def test_repeater_guest_login_is_guest_on_the_wire():
acl = ACL(admin_password="rpt-admin", guest_password="rpt-guest")
reply = await _login_reply(acl, "rpt-guest")
async def test_repeater_and_room_guests_get_different_roles():
"""The split is observable on the wire, not just in our own vocabulary.
assert reply[6] == 0
assert reply[7] == PERM_ACL_GUEST
GUEST and READ_WRITE were both 0x01 before, so these two logins were
indistinguishable to every client.
"""
rpt = await _login_reply(
ACL(admin_password="rpt-admin", guest_password="rpt-guest"), "rpt-guest"
)
room = await _login_reply(ACL(), "room-guest", room_config=ROOM_CFG)
@pytest.mark.asyncio
async def test_room_server_guest_login_is_read_write_on_the_wire():
"""Room guests post messages, so they must decode as READ_WRITE, not admin."""
reply = await _login_reply(ACL(), "room-guest", room_config=ROOM_CFG)
assert reply[6] == 0 # read-write is not admin
assert reply[7] == PERM_ACL_READ_WRITE
@pytest.mark.asyncio
async def test_room_server_admin_login_is_admin_on_the_wire():
reply = await _login_reply(ACL(), "room-admin", room_config=ROOM_CFG)
assert reply[6] == 1
assert reply[7] == PERM_ACL_ADMIN
@pytest.mark.asyncio
async def test_blank_password_read_only_login_is_guest_on_the_wire():
reply = await _login_reply(ACL(allow_read_only=True), "")
assert reply[6] == 0
assert reply[7] == PERM_ACL_GUEST
assert rpt[7] == 0x00
assert room[7] == 0x02
assert rpt[7] != room[7]