diff --git a/loopx/extensions/lark/bot_scopes.py b/loopx/extensions/lark/bot_scopes.py index 1ab9d8d9f3..1f82d929ce 100644 --- a/loopx/extensions/lark/bot_scopes.py +++ b/loopx/extensions/lark/bot_scopes.py @@ -9,9 +9,8 @@ from __future__ import annotations -import re +from .identity_shapes import LARK_APP_ID_PATTERN as APP_ID_PATTERN -APP_ID_PATTERN = re.compile(r"cli_[A-Za-z0-9_-]+") _OFFICIAL_SCOPE_APPLY_HOSTS = ("open.feishu.cn", "open.larkoffice.com") # 核心：发消息、建群/查群/群成员管理 —— goal-channel、reviewer、kanban 必需 diff --git a/loopx/extensions/lark/goal_channel_delivery_contract.py b/loopx/extensions/lark/goal_channel_delivery_contract.py index ea12b3e820..90c2b90b50 100644 --- a/loopx/extensions/lark/goal_channel_delivery_contract.py +++ b/loopx/extensions/lark/goal_channel_delivery_contract.py @@ -6,10 +6,9 @@ from collections.abc import Callable, Mapping from typing import Any -from .identity_shapes import LARK_CHAT_ID_PATTERN +from .identity_shapes import LARK_APP_ID_PATTERN, LARK_CHAT_ID_PATTERN _GOAL_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,159}$") -_LARK_APP_ID_RE = re.compile(r"^cli_[A-Za-z0-9_-]+$") _LARK_PROFILE_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,99}$") @@ -45,7 +44,7 @@ def goal_channel_delivery_route( or not LARK_CHAT_ID_PATTERN.fullmatch(chat_id) or not _LARK_PROFILE_RE.fullmatch(sender_profile) or sender_profile.lower() == "default" - or not _LARK_APP_ID_RE.fullmatch(bot_app_id) + or not LARK_APP_ID_PATTERN.fullmatch(bot_app_id) or not bot_display_name or not cli_bin ): diff --git a/loopx/extensions/lark/goal_channel_transport.py b/loopx/extensions/lark/goal_channel_transport.py index e09f8a68ba..39ea408d94 100644 --- a/loopx/extensions/lark/goal_channel_transport.py +++ b/loopx/extensions/lark/goal_channel_transport.py @@ -10,13 +10,13 @@ # Re-exported for the existing callers of this module; the shapes themselves are # decided once, in ``identity_shapes``. from .identity_shapes import ( # noqa: F401 + LARK_APP_ID_PATTERN as APP_ID_PATTERN, LARK_CHAT_ID_SEARCH as CHAT_ID_PATTERN, LARK_MESSAGE_ID_SEARCH as MESSAGE_ID_PATTERN, LARK_OPEN_ID_SEARCH as OPEN_ID_PATTERN, ) from .presentation.kanban import CommandRunner -APP_ID_PATTERN = re.compile(r"cli_[A-Za-z0-9_-]+") SAFE_PROFILE_PATTERN = re.compile(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,99}") REQUIRED_GOAL_TOPIC_SCOPES = ("im:message", "im:message:readonly") REQUIRED_BOT_GROUP_HISTORY_SCOPES = ( diff --git a/loopx/extensions/lark/identity_shapes.py b/loopx/extensions/lark/identity_shapes.py index 8aa2eb8632..34b8e16e9b 100644 --- a/loopx/extensions/lark/identity_shapes.py +++ b/loopx/extensions/lark/identity_shapes.py @@ -14,6 +14,9 @@ ``goal_channel_contracts`` and ``goal_channel_notification`` ``search()`` for an id inside payload text, where ``^`` and ``$`` would change the answer -- so both spellings stay distinct and one module decides both. + +The fifth shape is the application id, ``cli_``-prefixed. It has only the +whole-value spelling because every caller applies ``fullmatch``. """ from __future__ import annotations @@ -26,6 +29,14 @@ LARK_MESSAGE_ID_PATTERN = re.compile(r"^om_[A-Za-z0-9_-]+$") LARK_CHAT_ID_PATTERN = re.compile(r"^oc_[A-Za-z0-9_-]+$") LARK_OPEN_ID_PATTERN = re.compile(r"^ou_[A-Za-z0-9_-]+$") +# The application a bot belongs to. Four sites decided this themselves -- the +# transport hub, ``bot_scopes``, ``event_collector_runtime`` and +# ``goal_channel_delivery_contract``, the last one with its own anchors on the +# same body -- and eight more modules reach the decision by importing the +# transport hub's name, so a fix to the body had three possible homes. +# Every caller applies ``fullmatch``, which is why only the whole-value spelling +# is stated here. +LARK_APP_ID_PATTERN = re.compile(r"^cli_[A-Za-z0-9_-]+$") LARK_MESSAGE_ID_SEARCH = re.compile(r"om_[A-Za-z0-9_-]+") LARK_CHAT_ID_SEARCH = re.compile(r"oc_[A-Za-z0-9_-]+") diff --git a/tests/architecture/test_lark_identity_shape_owner.py b/tests/architecture/test_lark_identity_shape_owner.py index acea4b8a76..dfe96c195e 100644 --- a/tests/architecture/test_lark_identity_shape_owner.py +++ b/tests/architecture/test_lark_identity_shape_owner.py @@ -11,7 +11,7 @@ makes the decision is still an offender. 3. Are the two *uses* of a shape kept apart? A whole-value check and a search for an id inside larger text are different questions, so the owner states - both spellings: four anchored patterns and three unanchored ones. Either + both spellings: five anchored patterns and three unanchored ones. Either spelling anywhere else is an offender, and so is converting a declared site without retiring its declaration. """ @@ -20,6 +20,7 @@ import ast import pathlib +import re from types import ModuleType import pytest @@ -50,6 +51,7 @@ "message_id": r"om_[A-Za-z0-9_-]+", "chat_id": r"oc_[A-Za-z0-9_-]+", "operator_id": r"ou_[A-Za-z0-9_-]+", + "app_id": r"cli_[A-Za-z0-9_-]+", } # A module has to import the regex machinery before it can decide a shape, and # both spellings this scan accepts (``re.X(...)`` and a name from @@ -58,13 +60,16 @@ # single body, which is one of the probe cases below. PRESCREEN_TOKENS = ("import re", "from re import") # Only these three bodies have a search spelling. An event id is never looked -# for inside larger text. +# for inside larger text, and neither is an application id: every ``cli_`` site +# this slice gathered applies ``fullmatch``, so the whole-value spelling alone is +# the complete contract there. SEARCH_IDENTIFIERS = {"chat_id", "message_id", "operator_id"} ANCHORED_EXPORTS = { "event_id": "LARK_EVENT_ID_PATTERN", "message_id": "LARK_MESSAGE_ID_PATTERN", "chat_id": "LARK_CHAT_ID_PATTERN", "operator_id": "LARK_OPEN_ID_PATTERN", + "app_id": "LARK_APP_ID_PATTERN", } SEARCH_EXPORTS = { "message_id": "LARK_MESSAGE_ID_SEARCH", @@ -98,13 +103,19 @@ "message_id", "operator_id", "event_id", + # The application id is gated on its field name, not on the ``cli_`` prefix: + # ``cli_`` also occurs in this package as a binary name in data (``lark-cli``, + # ``cli_bin``), which drags in regexes built from URLs and markdown headings + # and turns the declaration layer into the noise its comment warns about. + "app_id", ) DECLARED_INDIVIDUAL_SITES: dict[str, int] = { - # ``re.fullmatch(r"[A-Za-z0-9._:-]{1,240}", ...)`` against an event id. The - # in-flight goal-channel claim work restructures this file, so the site is - # declared here instead of racing that branch. - "loopx/extensions/lark/event_collector_runtime.py": 1, + # ``re.fullmatch(r"[A-Za-z0-9._:-]{1,240}", ...)`` against an event id, plus + # this file's own ``cli_`` compile. The in-flight goal-channel claim work + # restructures this file, so both sites are declared here instead of racing + # that branch; converting either one deletes its share of the count. + "loopx/extensions/lark/event_collector_runtime.py": 2, } SHAPE_CONSUMERS: dict[str, tuple[ModuleType, str]] = { @@ -134,7 +145,7 @@ INBOX_HUB: (event_inbox, ("CHAT_ID_PATTERN", "MESSAGE_ID_PATTERN")), TRANSPORT_HUB: ( goal_channel_transport, - ("CHAT_ID_PATTERN", "MESSAGE_ID_PATTERN", "OPEN_ID_PATTERN"), + ("APP_ID_PATTERN", "CHAT_ID_PATTERN", "MESSAGE_ID_PATTERN", "OPEN_ID_PATTERN"), ), } _RE_MODULE = "re" @@ -497,6 +508,9 @@ def test_owner_states_each_shape_as_the_recorded_whole_value_body() -> None: assert identity_shapes.LARK_OPEN_ID_PATTERN.pattern == ( "^" + WHOLE_VALUE_BODIES["operator_id"] + "$" ) + assert identity_shapes.LARK_APP_ID_PATTERN.pattern == ( + "^" + WHOLE_VALUE_BODIES["app_id"] + "$" + ) def test_the_owner_is_the_only_module_that_defines_any_of_them() -> None: @@ -575,6 +589,66 @@ def test_inbox_callers_still_receive_one_object_through_the_chain() -> None: assert module.MESSAGE_ID_PATTERN is identity_shapes.LARK_MESSAGE_ID_PATTERN, name +# The application id reaches most of its callers through the transport hub, so the +# identity assertion is the wiring proof that none of them kept a private copy. +APP_ID_CALLERS = ( + "bot_scopes", + "goal_channel_setup", + "goal_channel_runtime", + "goal_channel_blocked_notice", + "goal_channel_targets", + "goal_topic_connections", + "private_conversations", + "event_inbox", +) + + +@pytest.mark.parametrize("name", sorted(APP_ID_CALLERS)) +def test_every_app_id_caller_holds_the_owners_object(name: str) -> None: + # Identity is the wiring proof: an alias import hands on the owner's object, + # while a module that recompiled the body would hold a distinct one even though + # the pattern text matched. + module = __import__(f"loopx.extensions.lark.{name}", fromlist=["APP_ID_PATTERN"]) + assert module.APP_ID_PATTERN is identity_shapes.LARK_APP_ID_PATTERN, name + + +def test_the_delivery_contract_holds_the_owners_object_too() -> None: + # It had its own anchored compile of the same body rather than a hub import. + assert ( + goal_channel_delivery_contract.LARK_APP_ID_PATTERN + is identity_shapes.LARK_APP_ID_PATTERN + ) + + +@pytest.mark.parametrize( + "value", + [ + "cli_ok1", + "cli_ok1\n", + "\ncli_ok1", + "cli_a\nb", + "", + "cli_", + "cli_\u00e9", + "cli_a b", + "xcli_a", + "cli_a-b_1.C", + "cli_a" + "z" * 200, + ], +) +def test_the_app_id_answer_is_the_same_anchored_or_not(value: str) -> None: + """Why gathering these sites cannot change a product answer. + + All three defining sites applied ``fullmatch`` to an unanchored body, and the + fourth applied an anchored one; ``re.fullmatch`` already requires the whole + string, so both spellings accept and reject exactly the same values. + """ + + anchored = bool(identity_shapes.LARK_APP_ID_PATTERN.fullmatch(value)) + unanchored = bool(re.fullmatch(r"cli_[A-Za-z0-9_-]+", value)) + assert anchored == unanchored, value + + def test_callback_validators_do_not_restate_the_identity_table() -> None: for module in (goal_channel_operation, team_plan_confirmation): tree = parse_module(module) @@ -755,6 +829,14 @@ def test_anchored_and_unanchored_fullmatch_agree_on_every_tricky_value( "operator id restated under an unrelated name", 'import re\n\nOPERATOR = re.compile(r"^ou_[A-Za-z0-9_-]+$")\n', ), + ( + "app id restated unanchored, the spelling three sites used", + 'import re\n\nAPP = re.compile(r"cli_[A-Za-z0-9_-]+")\n', + ), + ( + "app id restated anchored, the spelling the fourth site used", + 'import re\n\nAPP = re.compile(r"^cli_[A-Za-z0-9_-]+$")\n', + ), ( "event id inside a function body, not at module level", 'import re\n\n\ndef pick(value):\n' @@ -771,14 +853,14 @@ def test_anchored_and_unanchored_fullmatch_agree_on_every_tricky_value( "a chat id embedded in a larger route grammar", 'import re\n\nROUTE = re.compile(r"^route:oc_[A-Za-z0-9_-]+:v1$")\n', ), - ( - "an app id, which this slice does not own", - 'import re\n\nAPP = re.compile(r"^cli_[A-Za-z0-9_-]+$")\n', - ), ( "a pattern built from runtime data, which cannot be folded", 'import re\n\nCHAT = re.compile(prefix + "[A-Za-z0-9_-]+")\n', ), + ( + "a sender profile shape, which is a different decision", + 'import re\n\nPROFILE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,99}$")\n', + ), ( "a module constant rebound locally before use, which is not trusted", 'import re\n\nCHAT_ID = "^oc_[A-Za-z0-9_-]+$"\n\n\ndef pick(CHAT_ID):\n' @@ -818,7 +900,7 @@ def test_declared_site_count_is_enforced_in_both_directions() -> None: assert offender_rows(rows) == [] retired = [item for item in rows if item["file"] != declared_file] assert offender_rows(retired) == [ - f"{declared_file} declares 1 individual shape site(s), found 0" + f"{declared_file} declares 2 individual shape site(s), found 0" ] @@ -831,6 +913,15 @@ def test_a_converted_declared_site_is_not_silently_reintroduced() -> None: "identifier": "event_id", "anchored": False, }, + # The declared budget is two sites here (the event-id match plus this file's + # own ``cli_`` compile), so a fixture that clears the budget has to carry + # both; dropping either one is the failure this test is about. + { + "file": declared_file, + "line": 33, + "identifier": "app_id", + "anchored": False, + }, { "file": "loopx/extensions/lark/other_module.py", "line": 9,