Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions loopx/extensions/lark/bot_scopes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 必需
Expand Down
5 changes: 2 additions & 3 deletions loopx/extensions/lark/goal_channel_delivery_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}$")


Expand Down Expand Up @@ -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
):
Expand Down
2 changes: 1 addition & 1 deletion loopx/extensions/lark/goal_channel_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
11 changes: 11 additions & 0 deletions loopx/extensions/lark/identity_shapes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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_-]+")
Expand Down
115 changes: 103 additions & 12 deletions tests/architecture/test_lark_identity_shape_owner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand All @@ -20,6 +20,7 @@

import ast
import pathlib
import re
from types import ModuleType

import pytest
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -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]] = {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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"
]


Expand All @@ -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,
Expand Down