python bridge: rewrite packages serving code

Rework the packages loading code.  The main changes:

 - we don't attempt to do checksums anymore.  This was taking a large
   amount of time when loading the bridge and we never implemented it
   properly anyway (since we weren't sending the `.checksum` fields in
   the manifests)

 - instead, we do scan the list of files in the package one time (at
   first use, not load) and build our map of URL paths to filenames at
   this point.  This allows us to retain our efficient lookup code, but
   requires us to do the actual file loading at the time of the request.
   Because we're no longer storing the contents of the files in memory,
   this is a substantial runtime memory usage reduction.

 - drop all of the extra code we had for walking the paths in a
   particular order.  We just do (r)globs now.

 - move all of the code for deciding which packages to load to a
   separate class.  We now load all of the manifests and evaluate them
   (requirements, conditionals, priorities) and only the packages that
   we actually intend to serve are then scanned.  The new structure also
   makes it easier for cockpit-beiboot to do its thing.

 - the packages loading code now uses the actual cockpit version number
   for requires comparisons instead of the previously hardcoded '300'.
   Add a somewhat arbitrary number to our `_versions.py` file instead of
   the current value of `0`.  This gets automatically filled in for
   release versions, but if we want to run things in tree, we need
   something valid here.

 - clean up the relationship between `cockpit.packages` and
   `cockpit.channels.packages`.  Previously, `cockpit.packages` would
   access properties on the channel object and call methods on it to
   serve the content back.  Now the channel requests the data from
   `cockpit.packages` (which merely returns the result).

 - enabled by the above: full typing throughout, and mypy is happy.  We
   have one tiny little thorn in that the packages channel is not
   strictly capable of knowing that the router to which it's connected
   has a `packages` attribute, but that's nothing that we weren't doing
   already.  Add a comment to draw attention to it.

 - to the extent possible, we try to keep the state of the packages
   channel away from the packages code proper.  This led to an overhaul
   of our `Content-Security-Policy` not to include the origin URLs in
   the policy output.  This is redundant anyway, since that's what
   "'self'" is for.  We do need to do one hack for websockets though,
   until we can convince ourselves about browser support for the
   standard.  This hack is lifted to the channel level.  Adjust tests
   accordingly.

 - with some small changes to our pyproject.toml, the two rewritten
   files (`packages.py` and `channels/packages.py`) are now also passing
   pylint, but we don't enable that yet, since everything else is
   broken.

pyproject.toml

@@ -15,10 +15,12 @@ max-line-length = 118
 disable = [
   "C0114",  # Missing module docstring
   "C0115",  # Missing class docstring
+  "C0116",  # Missing function or method docstring
   "R0902",  # Too many instance attributes
   "R0903",  # Too few public methods
   "R0913",  # Too many arguments
   "R1705",  # Unnecessary "else" after "return"
+  "W0120",  # Else clause on loop without a break statement
   "W1113",  # Keyword argument before variable positional arguments  (PEP-570 is Python 3.8)
 ]
 
@@ -58,6 +60,7 @@ ignore = [
     "E731",  # Do not assign a `lambda` expression, use a `def`
     "PT011", # `pytest.raises(OSError)` is too broad
     "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar`
+    "TCH001", # Move application import into a type-checking block
 ]
 line-length = 118
 src = []

src/client/cockpit-beiboot

@@ -21,7 +21,6 @@ import argparse
 import asyncio
 import base64
 import importlib.resources
-import json
 import logging
 import os
 import shlex
@@ -36,7 +35,7 @@ from cockpit.beipack import BridgeBeibootHelper
 from cockpit.bridge import setup_logging
 from cockpit.channel import ChannelRoutingRule
 from cockpit.channels import PackagesChannel
-from cockpit.packages import Package, Packages
+from cockpit.packages import Packages, PackagesLoader
 from cockpit.peer import Peer
 from cockpit.protocol import CockpitProblem
 from cockpit.router import Router, RoutingRule
@@ -73,41 +72,37 @@ def ensure_ferny_askpass() -> Path:
     return dest_path
 
 
-def get_xdg_data_dirs():
-    try:
-        yield os.environ['XDG_DATA_HOME']
-    except KeyError:
-        yield os.path.expanduser('~/.local/share')
-
-    try:
-        yield from os.environ['XDG_DATA_DIRS'].split(':')
-    except KeyError:
-        yield from ('/usr/local/share', '/usr/share')
-
-
 def get_interesting_files() -> Iterable[str]:
-    for datadir in get_xdg_data_dirs():
-        logger.debug("Scanning for manifest files under %s", datadir)
-        for file in Path(datadir).glob('cockpit/*/manifest.json'):
-            logger.debug("Considering file %s", file)
-            try:
-                manifest = json.loads(file.read_text())
-            except json.JSONDecodeError as exc:
-                logger.error("%s: %s", file, exc)
-                continue
-            if not isinstance(manifest, dict):
-                logger.error("%s: json document isn't an object", file)
-                continue
-
-            try:
-                conditions = manifest['conditions']
-            except KeyError:
-                continue
-
-            for condition in conditions:
-                for key, value in condition.items():
-                    if key in ['path-exists', 'path-not-exists']:
-                        yield value
+    for candidate in PackagesLoader.load_candidates():
+        try:
+            conditions = candidate.manifest['conditions']
+        except KeyError:
+            continue
+
+        assert isinstance(conditions, list)
+        for condition in conditions:
+            assert isinstance(condition, dict)
+            for key, value in condition.items():
+                if key in ['path-exists', 'path-not-exists']:
+                    yield value
+
+
+class ProxyPackagesLoader(PackagesLoader):
+    file_status: Dict[str, bool]
+
+    def check_condition(self, condition: str, value: object) -> bool:
+        assert isinstance(value, str)
+        assert value in self.file_status
+
+        if condition == 'path-exists':
+            return self.file_status[value]
+        elif condition == 'path-not-exists':
+            return not self.file_status[value]
+        else:
+            raise KeyError
+
+    def __init__(self, file_status: Dict[str, bool]):
+        self.file_status = file_status
 
 
 BEIBOOT_GADGETS = {
@@ -176,13 +171,7 @@ class AuthorizeResponder(ferny.InteractionResponder):
 
         if command == 'cockpit.report-exists':
             file_status, = args
-            # Constructed to throw KeyError if an unexpected file is requested
-            # HACK:  ...please don't look at this code...
-            Package.CONDITIONS = {
-                'path-exists': lambda filename: file_status[filename],
-                'path-not-exists': lambda filename: not file_status[filename],
-            }
-            self.router.packages = Packages()  # type: ignore[attr-defined]
+            self.router.packages = Packages(loader=ProxyPackagesLoader(file_status))
             self.router.routing_rules.insert(0, ChannelRoutingRule(self.router, [PackagesChannel]))
 
 

src/cockpit/_version.py

@@ -1,2 +1,2 @@
 # This file is only in git.  It gets replaced by `make dist`.
-__version__ = '0'
+__version__ = '295'

src/cockpit/bridge.py

@@ -121,17 +121,15 @@ def do_init(self, message: Dict[str, object]) -> None:
             self.superuser_rule.init(superuser)
 
     def do_send_init(self) -> None:
+        init_args = {
+            'capabilities': {'explicit-superuser': True},
+            'os-release': self.get_os_release(),
+        }
+
         if self.packages is not None:
-            packages_info = {
-                'checksum': self.packages.checksum,
-                'packages': {p: None for p in self.packages.packages}
-            }
-        else:
-            packages_info = {}
+            init_args['packages'] = {p: None for p in self.packages.packages}
 
-        self.write_control(command='init', version=1, **packages_info,
-                           os_release=self.get_os_release(),
-                           capabilities={'explicit-superuser': True})
+        self.write_control(command='init', version=1, **init_args)
 
     # PackagesListener interface
     def packages_loaded(self):

src/cockpit/channels/packages.py

@@ -16,9 +16,11 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 import logging
+from typing import Dict, Optional
 
 from .. import data
 from ..channel import Channel
+from ..packages import Packages
 
 logger = logging.getLogger(__name__)
 
@@ -27,50 +29,81 @@ class PackagesChannel(Channel):
     payload = 'http-stream1'
     restrictions = [("internal", "packages")]
 
-    headers = None
-    protocol = None
-    host = None
-    origin = None
-    out_headers = None
-    options = None
-    post = None
-
-    def push_header(self, key, value):
-        if self.out_headers is None:
-            self.out_headers = {}
-        self.out_headers[key] = value
-
-    def http_ok(self, content_type, extra_headers=None):
-        headers = {'Content-Type': content_type}
-        if self.out_headers is not None:
-            headers.update(self.out_headers)
-        if extra_headers is not None:
-            headers.update(extra_headers)
-        self.send_message(status=200, reason='OK', headers={k: v for k, v in headers.items() if v is not None})
-
-    def http_error(self, status, message):
+    # used to carry data forward from open to done
+    options: Optional[Dict[str, object]] = None
+
+    def http_error(self, status: int, message: str) -> None:
         template = data.read_cockpit_data_file('fail.html')
         self.send_message(status=status, reason='ERROR', headers={'Content-Type': 'text/html; charset=utf-8'})
         self.send_data(template.replace(b'@@message@@', message.encode('utf-8')))
 
-    def do_done(self):
-        assert not self.post
-        assert self.options['method'] == 'GET'
-        path = self.options['path']
+    def do_open(self, options: Dict[str, object]) -> None:
+        self.ready()
 
-        self.headers = self.options['headers']
-        self.protocol = self.headers['X-Forwarded-Proto']
-        self.host = self.headers['X-Forwarded-Host']
-        self.origin = f'{self.protocol}://{self.host}'
+        self.options = options
 
-        self.router.packages.serve_file(path, self)
-        self.done()
-        self.close()
+    def do_done(self) -> None:
+        packages: Packages = self.router.packages  # type: ignore[attr-defined]  # yes, this is evil
+        assert self.options is not None
+        options = self.options
 
-    def do_data(self, data):
-        self.post += data
+        try:
+            if options.get('method') != 'GET':
+                raise ValueError(f'Unsupported HTTP method {options["method"]}')
 
-    def do_open(self, options):
-        self.post = b''
-        self.options = options
-        self.ready()
+            path = options.get('path')
+            if not isinstance(path, str) or not path.startswith('/'):
+                raise ValueError(f'Unsupported HTTP method {options["method"]}')
+
+            headers = options.get('headers')
+            if not isinstance(headers, dict) or not all(isinstance(value, str) for value in headers.values()):
+                raise ValueError(f'Unsupported HTTP method {options["method"]}')
+
+            document = packages.load_path(path, headers)
+
+            # Note: we can't cache documents right now.  See
+            # https://github.com/cockpit-project/cockpit/issues/19071
+            # for future plans.
+            out_headers = {
+                'Cache-Control': 'no-cache, no-store',
+                'Content-Type': document.content_type,
+            }
+
+            if document.content_encoding is not None:
+                out_headers['Content-Encoding'] = document.content_encoding
+
+            if document.content_security_policy is not None:
+                policy = document.content_security_policy
+
+                # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/connect-src
+                #
+                #    Note: connect-src 'self' does not resolve to websocket
+                #    schemes in all browsers, more info in this issue.
+                #
+                # https://github.com/w3c/webappsec-csp/issues/7
+                if "connect-src 'self';" in policy:
+                    protocol = headers.get('X-Forwarded-Proto')
+                    host = headers.get('X-Forwarded-Host')
+                    if not isinstance(protocol, str) or not isinstance(host, str):
+                        raise ValueError('Invalid host or protocol header')
+
+                    websocket_scheme = "wss" if protocol == "https" else "ws"
+                    websocket_origin = f"{websocket_scheme}://{host}"
+                    policy = policy.replace("connect-src 'self';", f"connect-src {websocket_origin} 'self';")
+
+                out_headers['Content-Security-Policy'] = policy
+
+            self.send_message(status=200, reason='OK', headers=out_headers)
+            self.send_data(document.data)
+
+        except ValueError as exc:
+            self.http_error(400, str(exc))
+
+        except KeyError:
+            self.http_error(404, 'Not found')
+
+        except OSError as exc:
+            self.http_error(500, f'Internal error: {exc!s}')
+
+        self.done()
+        self.close()