From 7cdd1e1c78ed010770f74cbeeb555ab7dea7ba44 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Tue, 21 Jul 2026 19:14:58 +0200 Subject: [PATCH] fix(docs): bind user guidance to visible routes --- src/govoplan_docs/backend/api/v1/routes.py | 45 ++++++++++++++++++---- src/govoplan_docs/backend/manifest.py | 6 +++ tests/test_docs_context.py | 16 +++++++- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/govoplan_docs/backend/api/v1/routes.py b/src/govoplan_docs/backend/api/v1/routes.py index 1019817..c9228b9 100644 --- a/src/govoplan_docs/backend/api/v1/routes.py +++ b/src/govoplan_docs/backend/api/v1/routes.py @@ -43,16 +43,17 @@ def docs_context( registry = _registry(request) resolved_locale = _preferred_locale(request, locale) route_items = _route_items(registry.manifests(), principal) - visible_routes = [item for item in route_items if item["visible"]] + visible_route_items = [item for item in route_items if item["visible"]] documentation_layers = _documentation_layers(request, registry, principal, documentation_type=documentation_type, locale=resolved_locale) if documentation_type == "admin": + visible_routes = visible_route_items modules = [_module_payload(manifest, technical=True) for manifest in registry.manifests()] permissions = [_permission_payload(permission, principal) for permission in registry.permissions()] available_routes = [item for item in route_items if not item["visible"]] optional_modules = _optional_module_evidence(registry.manifests()) else: visible_module_ids = { - *(str(item["module_id"]) for item in visible_routes), + *(str(item["module_id"]) for item in visible_route_items), *(str(topic["source_module_id"]) for topic in _all_documentation_topics(documentation_layers)), } modules = [ @@ -61,7 +62,7 @@ def docs_context( if manifest.id in visible_module_ids ] permissions = [] - visible_routes = [_user_route_payload(item) for item in visible_routes] + visible_routes = _user_route_payloads(visible_route_items) available_routes = [] optional_modules = [] documentation_count = sum(len(layer) for layer in documentation_layers.values()) @@ -166,6 +167,18 @@ def _user_route_payload(item: Mapping[str, Any]) -> dict[str, Any]: } +def _user_route_payloads(items: list[dict[str, Any]]) -> list[dict[str, Any]]: + result: list[dict[str, Any]] = [] + seen: set[tuple[str, str]] = set() + for item in items: + key = (str(item["module_id"]), str(item["path"])) + if key in seen: + continue + seen.add(key) + result.append(_user_route_payload(item)) + return result + + def _permission_payload(permission: PermissionDefinition, principal: ApiPrincipal) -> dict[str, Any]: return { "scope": permission.scope, @@ -305,6 +318,14 @@ def _classify_documentation( ) -> dict[str, list[dict[str, Any]]]: layers: dict[str, list[dict[str, Any]]] = {"always": [], "configured": [], "available": [], "evidence": []} installed = {manifest.id for manifest in registry.manifests()} + visible_runtime_paths = frozenset([ + "/settings", # Authenticated shell route, not contributed by a module manifest. + *( + str(item["path"]) + for item in _route_items(registry.manifests(), principal) + if item["visible"] + ), + ]) if documentation_type == "user" else frozenset() topics = _collect_documentation_topics(registry, principal, settings=settings, session=session, documentation_type=documentation_type, locale=locale) for source_module_id, topic in sorted(topics, key=lambda item: (item[1].order, item[0], item[1].id)): if not _topic_matches_documentation_type(topic, documentation_type): @@ -324,6 +345,7 @@ def _classify_documentation( blockers=blockers, locale=locale, documentation_type=documentation_type, + visible_runtime_paths=visible_runtime_paths, )) return layers @@ -539,6 +561,7 @@ def _documentation_topic_payload( blockers: dict[str, list[str]], locale: str, documentation_type: DocumentationType, + visible_runtime_paths: frozenset[str], ) -> dict[str, Any]: module_id = topic.source_module_id or source_module_id translation_locale, translation = _translation_for_locale(topic, locale) @@ -598,7 +621,7 @@ def _documentation_topic_payload( "links": [ _documentation_link_payload(link) for link in topic.links - if _user_link_allowed(link) + if _user_link_allowed(link, visible_runtime_paths=visible_runtime_paths) ], "related_modules": [], "unlocks": list(topic.unlocks), @@ -680,16 +703,22 @@ def _bounded_string_list(value: object, *, maximum_items: int, maximum_length: i ] -def _user_link_allowed(link: DocumentationLink) -> bool: +def _user_link_allowed(link: DocumentationLink, *, visible_runtime_paths: frozenset[str]) -> bool: href = link.href.strip() if link.kind == "runtime": if not href.startswith("/") or href.startswith("//") or "\\" in href: return False - parsed = urlsplit(href) - return not parsed.scheme and not parsed.netloc + try: + parsed = urlsplit(href) + except ValueError: + return False + return not parsed.scheme and not parsed.netloc and parsed.path in visible_runtime_paths if link.kind != "public": return False - parsed = urlsplit(href) + try: + parsed = urlsplit(href) + except ValueError: + return False return parsed.scheme == "https" and bool(parsed.netloc) and not parsed.username and not parsed.password diff --git a/src/govoplan_docs/backend/manifest.py b/src/govoplan_docs/backend/manifest.py index 7e0f42e..91da8a0 100644 --- a/src/govoplan_docs/backend/manifest.py +++ b/src/govoplan_docs/backend/manifest.py @@ -2,6 +2,7 @@ from __future__ import annotations from govoplan_core.core.access import CAPABILITY_AUTH_PERMISSION_EVALUATOR, CAPABILITY_AUTH_PRINCIPAL_RESOLVER from govoplan_core.core.modules import ( + DocumentationCondition, DocumentationLink, DocumentationTopic, FrontendModule, @@ -168,6 +169,11 @@ manifest = ModuleManifest( audience=("tenant_admin", "access_admin", "operator", "user"), related_modules=("organizations", "identity", "idm", "access"), order=21, + conditions=( + DocumentationCondition( + required_modules=("organizations", "identity", "idm", "access"), + ), + ), links=( DocumentationLink(label="Organizations", href="/organizations", kind="runtime"), DocumentationLink(label="IDM assignments", href="/idm", kind="runtime"), diff --git a/tests/test_docs_context.py b/tests/test_docs_context.py index 0823550..c0636ca 100644 --- a/tests/test_docs_context.py +++ b/tests/test_docs_context.py @@ -8,7 +8,7 @@ from unittest.mock import patch from fastapi import HTTPException from govoplan_access.backend.manifest import get_manifest as get_access_manifest -from govoplan_core.core.modules import DocumentationCondition, DocumentationLink, DocumentationTopic, ModuleManifest +from govoplan_core.core.modules import DocumentationCondition, DocumentationLink, DocumentationTopic, ModuleManifest, NavItem from govoplan_core.core.registry import PlatformRegistry from govoplan_tenancy.backend.manifest import get_manifest as get_tenancy_manifest from govoplan_docs.backend.api.v1.routes import ( @@ -92,6 +92,7 @@ class DocsContextTests(unittest.TestCase): id="example", name="Example", version="9.9.9", + nav_items=(NavItem(path="/example", label="Example", required_any=("docs:documentation:read",)),), documentation=( DocumentationTopic( id="example.workflow.safe", @@ -103,7 +104,10 @@ class DocsContextTests(unittest.TestCase): DocumentationLink(label="Open task", href="/example", kind="runtime"), DocumentationLink(label="Unsafe runtime", href="javascript:alert(1)", kind="runtime"), DocumentationLink(label="Backslash runtime", href="/\\evil.invalid", kind="runtime"), + DocumentationLink(label="Invisible runtime", href="/hidden", kind="runtime"), + DocumentationLink(label="Shell settings", href="/settings?section=example", kind="runtime"), DocumentationLink(label="Public help", href="https://example.invalid/help", kind="public"), + DocumentationLink(label="Malformed public", href="https://[", kind="public"), DocumentationLink(label="Repository", href="example/docs/SECRET.md", kind="repository"), DocumentationLink(label="API", href="/api/v1/example", kind="api"), ), @@ -137,7 +141,10 @@ class DocsContextTests(unittest.TestCase): ) topic = layers["configured"][0] - self.assertEqual([link["href"] for link in topic["links"]], ["/example", "https://example.invalid/help"]) + self.assertEqual( + [link["href"] for link in topic["links"]], + ["/example", "/settings?section=example", "https://example.invalid/help"], + ) self.assertEqual(topic["conditions"], []) self.assertEqual(topic["configuration_keys"], []) self.assertEqual(topic["blockers"], {"modules": [], "capabilities": [], "scopes": []}) @@ -221,6 +228,11 @@ class DocsContextTests(unittest.TestCase): self.assertEqual(payload["layers"]["configured"]["permissions"], []) self.assertEqual(payload["layers"]["available"]["routes"], []) self.assertEqual(payload["layers"]["evidence"]["sources"], []) + self.assertEqual( + [(item["module_id"], item["path"]) for item in payload["layers"]["configured"]["routes"]], + [("docs", "/docs")], + ) + self.assertEqual(payload["summary"]["visible_route_count"], 1) def test_topic_anchor_is_stable(self) -> None: self.assertEqual(