From 278555ddf396ad6c0a6b08fd9dd9f0d8c120fcad Mon Sep 17 00:00:00 2001 From: psychologister <230141834+psychologister@users.noreply.github.com> Date: Thu, 12 Nov 2026 11:11:10 +0800 Subject: [PATCH] control_plane/projects: single-source the safe project-id grammar repository_identity._SAFE_PROJECT_ID is the canonical stable-identifier grammar (^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$); resolve_project_identity uses it to validate the loopx: fallback when a checkout has no origin remote. The projects registry restated the identical pattern as its own _SAFE_ID and used it in _identifier to reject unsafe project/goal ids. The registry already imports normalize_repository_identity from repository_identity, so borrowing one more owner object adds no import edge and cannot cycle. _SAFE_ID is retired and _identifier now fullmatches against the owner; the sole remaining use of the re module in this file was that definition, so its import is dropped too. presentation/static_site.py keeps a same-named _SAFE_ID with a genuinely different grammar (no colon, 63-character cap) and is left untouched -- it is a neighbour, not a copy. The by-name semantic inventory cannot see this: both bindings are private _-names it does not catalogue, and they carried different names, so a name-keyed match would not pair them even if it looked. The new guard keys on the compiled pattern value -- a whole-tree census of re.compile sites, identity for the consumer, retirement, the distinct static-site neighbour, and behavioural parity through registry._identifier over the owner's grammar (including the 256/257 length boundary) plus the resolve_project_identity fallback. Refs #4447. Signed-off-by: psychologister <230141834+psychologister@users.noreply.github.com> --- loopx/control_plane/projects/registry.py | 9 +- .../test_repository_identity_safe_id_owner.py | 187 ++++++++++++++++++ 2 files changed, 192 insertions(+), 4 deletions(-) create mode 100644 tests/architecture/test_repository_identity_safe_id_owner.py diff --git a/loopx/control_plane/projects/registry.py b/loopx/control_plane/projects/registry.py index c52a8c05b2..45411b0e27 100644 --- a/loopx/control_plane/projects/registry.py +++ b/loopx/control_plane/projects/registry.py @@ -1,7 +1,6 @@ from __future__ import annotations import hashlib -import re from collections.abc import Callable from pathlib import Path from typing import Any @@ -31,7 +30,10 @@ register_fresh_source_session_project, resolve_source_session_project, ) -from ...repository_identity import normalize_repository_identity +from ...repository_identity import ( + _SAFE_PROJECT_ID, + normalize_repository_identity, +) from .contract import validate_project_record_bindings from .registration_state import ( registration_state_matches, @@ -45,12 +47,11 @@ ) PROJECT_KINDS = ("work", "personal") -_SAFE_ID = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$") def _identifier(value: str, *, field: str) -> str: compact = str(value or "").strip() - if not _SAFE_ID.fullmatch(compact): + if not _SAFE_PROJECT_ID.fullmatch(compact): raise ValueError(f"{field} must be a stable identifier") return compact diff --git a/tests/architecture/test_repository_identity_safe_id_owner.py b/tests/architecture/test_repository_identity_safe_id_owner.py new file mode 100644 index 0000000000..f37b145452 --- /dev/null +++ b/tests/architecture/test_repository_identity_safe_id_owner.py @@ -0,0 +1,187 @@ +"""Guard one owner for the safe project-id grammar (#4447). + +``_SAFE_PROJECT_ID`` in ``repository_identity.py`` is the compiled grammar for a +stable project identifier (``^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$``). The project +registry restated the *same* compiled regex under a different name (``_SAFE_ID``) +to validate its identifiers. ``registry.py`` *already imports* +``normalize_repository_identity`` from ``repository_identity`` -- a stdlib-only +leaf module -- so borrowing the owner's compiled pattern adds no new import edge +and cannot introduce a cycle. The restated regex is retired and the registry's +``_identifier`` gate now references the single owner. + +The by-name semantic inventory cannot see this: both bindings are private +``_``-names (the inventory only catalogues ``UPPER_NAME`` carriers) and they differ +by name. This guard keys on the **compiled pattern literal** instead, so a renamed +restatement is caught, while the genuinely different neighbour -- the stricter +``static_site._SAFE_ID`` grammar (no colon, 63 chars) -- is never swept. + +Two layers: a whole-tree compiled-pattern census (only the owner compiles it), and +behavioural parity through the registry's own ``_identifier`` gate plus the owner's +``resolve_project_identity`` fallback, so a migration that silently changed which +identifiers are accepted fails here. +""" + +from __future__ import annotations + +import ast +import pathlib + +import pytest + +from loopx import repository_identity as owner +from loopx.control_plane.projects import registry as registry_mod +from loopx.presentation import static_site as static_site_mod + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] +PACKAGE_ROOT = REPO_ROOT / "loopx" +OWNER_PATH = "loopx/repository_identity.py" +REGISTRY_PATH = "loopx/control_plane/projects/registry.py" + +PATTERN = r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$" +STATIC_SITE_PATTERN = r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$" + + +def _compiled_pattern_sites(source: str, pattern: str) -> list[int]: + """Every ``re.compile()`` call in the source whose pattern matches. + + Keys on the pattern literal, so a restatement under any name (or with extra + flags) is still one site; a bare pattern string that is never compiled is not + a compiled-regex restatement and is left alone. + """ + + tree = ast.parse(source) + sites: list[int] = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + fname = getattr(node.func, "id", None) or getattr(node.func, "attr", None) + if fname != "compile": + continue + if ( + node.args + and isinstance(node.args[0], ast.Constant) + and node.args[0].value == pattern + ): + sites.append(node.lineno) + return sites + + +@pytest.fixture(scope="module") +def tree_offenders() -> dict[str, list[int]]: + offenders: dict[str, list[int]] = {} + for path in sorted(PACKAGE_ROOT.rglob("*.py")): + relative = path.relative_to(REPO_ROOT).as_posix() + if relative == OWNER_PATH: + continue + sites = _compiled_pattern_sites(path.read_text(encoding="utf-8"), PATTERN) + if sites: + offenders[relative] = sites + return offenders + + +def test_only_the_owner_compiles_the_pattern(tree_offenders) -> None: + assert tree_offenders == {}, ( + f"a restatement of the safe project-id grammar remains: {tree_offenders}" + ) + + +def test_the_owner_compiles_the_recorded_pattern_once() -> None: + assert owner._SAFE_PROJECT_ID.pattern == PATTERN + source = (REPO_ROOT / OWNER_PATH).read_text(encoding="utf-8") + assert len(_compiled_pattern_sites(source, PATTERN)) == 1 + + +def test_registry_borrows_the_owner_object() -> None: + assert registry_mod._SAFE_PROJECT_ID is owner._SAFE_PROJECT_ID + + +def test_registry_retired_its_restated_regex() -> None: + source = (REPO_ROOT / REGISTRY_PATH).read_text(encoding="utf-8") + assert _compiled_pattern_sites(source, PATTERN) == [] + assert "_SAFE_PROJECT_ID" in source + assert "_SAFE_ID" not in source + + +def test_the_static_site_id_is_a_different_grammar() -> None: + # The site-id grammar is stricter (no colon, 63 chars); it stays its own + # owner in static_site.py and is deliberately left untouched. + assert static_site_mod._SAFE_ID.pattern == STATIC_SITE_PATTERN + assert STATIC_SITE_PATTERN != PATTERN + + +# --- behavioural parity through both gates that share the owner ------------------------- + +CANDIDATES = ( + "abc", + "A1", + "a.b", + "a:b", + "a-b", + "a_b", + "a1.b:c-d_e", + "1", + "a" * 256, + "a" * 257, + "1" + "a" * 255, + "", + ".x", + "-x", + ":x", + "_x", + "a/b", + "a b", + "a@b", + " abc ", + "héllo", +) + + +@pytest.mark.parametrize("value", CANDIDATES) +def test_registry_identifier_accepts_exactly_the_owner_grammar(value: str) -> None: + compact = str(value or "").strip() + if owner._SAFE_PROJECT_ID.fullmatch(compact) is not None: + assert registry_mod._identifier(value, field="project_id") == compact + else: + with pytest.raises(ValueError): + registry_mod._identifier(value, field="project_id") + + +def test_owner_project_id_fallback_shares_the_grammar() -> None: + # An unusable remote forces the loopx project-id fallback, which the owner + # validates with the very regex the registry now borrows. + resolved = owner.resolve_project_identity( + "/nonexistent-checkout", + loopx_project_id="valid.id:1", + remote_url="::invalid::", + ) + assert resolved == "loopx:valid.id:1" + with pytest.raises(ValueError): + owner.resolve_project_identity( + "/nonexistent-checkout", + loopx_project_id="bad/id", + remote_url="::invalid::", + ) + + +# --- the census bites every restatement form and leaves the others ---------------------- + +POISON_CORPUS = ( + ("re.compile of the pattern", 'X = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$")\n', 1), + ("bare compile from re", 'from re import compile\nX = compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$")\n', 1), + ("nested in a function", 'def f():\n return re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$")\n', 1), + ("flags do not hide the restated pattern", 'X = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$", re.IGNORECASE)\n', 1), + ("the static-site grammar is different", 'X = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$")\n', 0), + ("a shorter max length is different", 'X = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$")\n', 0), + ("a bare pattern string is not a compiled regex", 'X = r"^[A-Za-z0-9][A-Za-z0-9._:-]{0,255}$"\n', 0), + ("importing the owner is not a restatement", "from ...repository_identity import _SAFE_PROJECT_ID\n", 0), + ("alias rebind is not a restatement", "_SAFE_ID = _SAFE_PROJECT_ID\n", 0), +) + + +@pytest.mark.parametrize( + "label,source,expected", POISON_CORPUS, ids=[case[0] for case in POISON_CORPUS] +) +def test_the_census_names_every_form_and_leaves_the_others( + label: str, source: str, expected: int +) -> None: + assert len(_compiled_pattern_sites(source, PATTERN)) == expected, label \ No newline at end of file