diff --git a/api/manifest_validator.py b/api/manifest_validator.py index 29c2b0c..c702a98 100644 --- a/api/manifest_validator.py +++ b/api/manifest_validator.py @@ -37,6 +37,10 @@ _CONTAINER_NAME_RE = re.compile(r'^cell-[a-z0-9][a-z0-9-]{0,30}$') _ENV_VALUE_RE = re.compile(r'^[A-Za-z0-9._@:/+\-]{0,256}$') _HOOK_BINARY_RE = re.compile(r'^[a-z][a-z0-9_-]{0,31}$') _CAP_NAME_RE = re.compile(r'^[A-Z_]+$') +_ID_RE = re.compile(r'^[a-z][a-z0-9_-]{0,30}$') +_IMAGE_DIGEST_RE = re.compile( + r'^git\.pic\.ngo/roof/[a-zA-Z0-9._/-]+@sha256:[0-9a-f]{64}$' +) def validate_manifest(manifest: dict) -> tuple: @@ -49,11 +53,34 @@ def validate_manifest(manifest: dict) -> tuple: """ errors = [] + # schema_version must be 3 + schema_version = manifest.get('schema_version') + if schema_version is not None and schema_version != 3: + errors.append( + f'schema_version must be 3, got: {schema_version!r}' + ) + # kind must be "store" if present — reject builtins coming in over the wire kind = manifest.get('kind') if kind is not None and kind != 'store': errors.append(f'manifest kind must be "store", got: {kind!r}') + # id format check + manifest_id = manifest.get('id') + if manifest_id is not None: + if not isinstance(manifest_id, str) or not _ID_RE.match(manifest_id): + errors.append( + f'id must match ^[a-z][a-z0-9_-]{{0,30}}$, got: {manifest_id!r}' + ) + + # image must be digest-pinned from git.pic.ngo/roof/* + image = manifest.get('image') + if image is not None: + if not isinstance(image, str) or not _IMAGE_DIGEST_RE.match(image): + errors.append( + f'image must match git.pic.ngo/roof/*@sha256:<64-hex>, got: {image!r}' + ) + # container_name structural check cname = manifest.get('container_name') if cname is not None: @@ -156,6 +183,10 @@ def validate_rendered_compose(yaml_text: str, allowed_data_dir: str = None) -> t continue prefix = f'service {svc_name!r}' + cname = svc.get('container_name') + if cname is not None and cname in _RESERVED_CONTAINER_NAMES: + errors.append(f'{prefix}: container_name {cname!r} is reserved') + if svc.get('privileged') is True: errors.append(f'{prefix}: privileged: true is not allowed') diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index abf4c4d..352bb6e 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -88,6 +88,120 @@ class TestValidateManifest(unittest.TestCase): for e in errs: self.assertIsInstance(e, str) + # ── schema_version ─────────────────────────────────────────────────── + + def test_schema_version_3_passes(self): + ok, errs = validate_manifest(_minimal_manifest(schema_version=3)) + self.assertTrue(ok) + + def test_schema_version_2_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(schema_version=2)) + self.assertFalse(ok) + self.assertTrue(any('schema_version' in e for e in errs)) + + def test_schema_version_1_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(schema_version=1)) + self.assertFalse(ok) + + def test_schema_version_string_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(schema_version='3')) + self.assertFalse(ok) + self.assertTrue(any('schema_version' in e for e in errs)) + + def test_schema_version_absent_passes(self): + m = _minimal_manifest() + m.pop('schema_version', None) + ok, errs = validate_manifest(m) + self.assertTrue(ok) + + # ── id ─────────────────────────────────────────────────────────────── + + def test_id_valid_lowercase_passes(self): + ok, errs = validate_manifest(_minimal_manifest(id='myapp')) + self.assertTrue(ok) + + def test_id_with_hyphen_and_underscore_passes(self): + ok, errs = validate_manifest(_minimal_manifest(id='my-app_v2')) + self.assertTrue(ok) + + def test_id_starts_with_digit_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(id='1app')) + self.assertFalse(ok) + self.assertTrue(any('id' in e for e in errs)) + + def test_id_uppercase_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(id='MyApp')) + self.assertFalse(ok) + + def test_id_with_space_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(id='my app')) + self.assertFalse(ok) + + def test_id_too_long_rejected(self): + ok, errs = validate_manifest(_minimal_manifest(id='a' * 32)) + self.assertFalse(ok) + self.assertTrue(any('id' in e for e in errs)) + + def test_id_31_chars_passes(self): + # Pattern allows up to 31 chars total (1 + 30) + ok, errs = validate_manifest(_minimal_manifest(id='a' + 'b' * 30)) + self.assertTrue(ok) + + def test_id_absent_passes(self): + m = _minimal_manifest() + m.pop('id', None) + ok, errs = validate_manifest(m) + self.assertTrue(ok) + + # ── image ──────────────────────────────────────────────────────────── + + def test_image_with_digest_passes(self): + digest = 'a' * 64 + ok, errs = validate_manifest( + _minimal_manifest(image=f'git.pic.ngo/roof/myapp@sha256:{digest}') + ) + self.assertTrue(ok) + + def test_image_tag_only_rejected(self): + ok, errs = validate_manifest( + _minimal_manifest(image='git.pic.ngo/roof/myapp:latest') + ) + self.assertFalse(ok) + self.assertTrue(any('image' in e for e in errs)) + + def test_image_wrong_registry_rejected(self): + digest = 'a' * 64 + ok, errs = validate_manifest( + _minimal_manifest(image=f'docker.io/library/nginx@sha256:{digest}') + ) + self.assertFalse(ok) + + def test_image_digest_too_short_rejected(self): + ok, errs = validate_manifest( + _minimal_manifest(image='git.pic.ngo/roof/myapp@sha256:abc123') + ) + self.assertFalse(ok) + + def test_image_digest_with_uppercase_hex_rejected(self): + # sha256 digest must be lowercase hex + digest = 'A' * 64 + ok, errs = validate_manifest( + _minimal_manifest(image=f'git.pic.ngo/roof/myapp@sha256:{digest}') + ) + self.assertFalse(ok) + + def test_image_no_tag_no_digest_rejected(self): + ok, errs = validate_manifest( + _minimal_manifest(image='git.pic.ngo/roof/myapp') + ) + self.assertFalse(ok) + + def test_image_absent_passes(self): + m = _minimal_manifest() + m.pop('image', None) + ok, errs = validate_manifest(m) + self.assertTrue(ok) + # ── kind ───────────────────────────────────────────────────────────── def test_kind_builtin_rejected(self): @@ -747,6 +861,88 @@ class TestValidateRenderedCompose(unittest.TestCase): ok, errs = validate_rendered_compose(yaml_text) self.assertFalse(ok) + # ── container_name reserved in compose ────────────────────────────── + + def test_compose_container_name_cell_api_rejected(self): + yaml_text = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + ' container_name: cell-api\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + ok, errs = validate_rendered_compose(yaml_text) + self.assertFalse(ok) + self.assertTrue(any('reserved' in e for e in errs)) + + def test_compose_container_name_cell_caddy_rejected(self): + yaml_text = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + ' container_name: cell-caddy\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + ok, errs = validate_rendered_compose(yaml_text) + self.assertFalse(ok) + + def test_compose_container_name_cell_wireguard_rejected(self): + yaml_text = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + ' container_name: cell-wireguard\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + ok, errs = validate_rendered_compose(yaml_text) + self.assertFalse(ok) + + def test_compose_container_name_cell_coredns_rejected(self): + yaml_text = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + ' container_name: cell-coredns\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + ok, errs = validate_rendered_compose(yaml_text) + self.assertFalse(ok) + + def test_compose_container_name_non_reserved_passes(self): + yaml_text = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + ' container_name: cell-myapp\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + ok, errs = validate_rendered_compose(yaml_text) + self.assertTrue(ok) + + def test_compose_no_container_name_passes(self): + ok, errs = validate_rendered_compose(_valid_compose()) + self.assertTrue(ok) + # ── multi-service ───────────────────────────────────────────────────── def test_multiple_services_one_invalid_rejected(self): @@ -970,7 +1166,7 @@ class TestServiceStoreManagerSecurityIntegration(unittest.TestCase): 'name': 'My App', 'version': '1.0.0', 'author': 'Test Author', - 'image': 'git.pic.ngo/roof/myapp:latest', + 'image': 'git.pic.ngo/roof/myapp@sha256:' + 'a' * 64, 'container_name': 'cell-myapp', } m.update(overrides) @@ -1065,6 +1261,250 @@ class TestServiceStoreManagerSecurityIntegration(unittest.TestCase): ok, errs = ServiceStoreManager._validate_manifest(m) self.assertTrue(ok) + def test_validate_manifest_rejects_image_tag_only(self): + """Image without digest pin must be rejected even for git.pic.ngo/roof/* images.""" + from service_store_manager import ServiceStoreManager + m = self._valid_manifest(image='git.pic.ngo/roof/myapp:latest') + ok, errs = ServiceStoreManager._validate_manifest(m) + self.assertFalse(ok) + + def test_validate_manifest_accepts_image_with_digest(self): + from service_store_manager import ServiceStoreManager + digest = 'b' * 64 + m = self._valid_manifest(image=f'git.pic.ngo/roof/myapp@sha256:{digest}') + ok, errs = ServiceStoreManager._validate_manifest(m) + self.assertTrue(ok) + + +# --------------------------------------------------------------------------- +# TestInstallManifestValidation +# --------------------------------------------------------------------------- + +class TestInstallManifestValidation(unittest.TestCase): + """Tests that ServiceStoreManager.install() propagates manifest validation errors.""" + + def _make_ssm(self, manifest): + from service_store_manager import ServiceStoreManager + cm = MagicMock() + cm.get_installed_services.return_value = {} + composer = MagicMock() + composer._resolve_requires.return_value = None + composer.install.return_value = {'ok': True} + ssm = ServiceStoreManager( + config_manager=cm, + caddy_manager=MagicMock(), + container_manager=MagicMock(), + service_composer=composer, + ) + ssm._fetch_manifest = MagicMock(return_value=manifest) + ssm._fetch_template = MagicMock(return_value=( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + )) + return ssm + + def test_install_returns_error_when_image_tag_only(self): + manifest = { + 'id': 'myapp', + 'name': 'My App', + 'version': '1.0.0', + 'author': 'Test', + 'image': 'git.pic.ngo/roof/myapp:latest', + 'container_name': 'cell-myapp', + } + ssm = self._make_ssm(manifest) + result = ssm.install('myapp') + self.assertFalse(result['ok']) + self.assertIn('errors', result) + + def test_install_returns_error_when_kind_is_builtin(self): + digest = 'c' * 64 + manifest = { + 'id': 'myapp', + 'name': 'My App', + 'version': '1.0.0', + 'author': 'Test', + 'image': f'git.pic.ngo/roof/myapp@sha256:{digest}', + 'container_name': 'cell-myapp', + 'kind': 'builtin', + } + ssm = self._make_ssm(manifest) + result = ssm.install('myapp') + self.assertFalse(result['ok']) + + def test_install_returns_error_when_cap_add_sys_admin(self): + digest = 'd' * 64 + manifest = { + 'id': 'myapp', + 'name': 'My App', + 'version': '1.0.0', + 'author': 'Test', + 'image': f'git.pic.ngo/roof/myapp@sha256:{digest}', + 'container_name': 'cell-myapp', + 'cap_add': ['SYS_ADMIN'], + } + ssm = self._make_ssm(manifest) + result = ssm.install('myapp') + self.assertFalse(result['ok']) + + def test_install_succeeds_with_valid_manifest(self): + digest = 'e' * 64 + manifest = { + 'id': 'myapp', + 'name': 'My App', + 'version': '1.0.0', + 'author': 'Test', + 'image': f'git.pic.ngo/roof/myapp@sha256:{digest}', + 'container_name': 'cell-myapp', + } + ssm = self._make_ssm(manifest) + result = ssm.install('myapp') + self.assertTrue(result['ok']) + + +# --------------------------------------------------------------------------- +# TestWriteComposeValidation +# --------------------------------------------------------------------------- + +class TestWriteComposeValidation(unittest.TestCase): + """Tests that ServiceComposer.write_compose() rejects each specific violation.""" + + def _make_cm(self): + cm = MagicMock() + cm.get_identity.return_value = { + 'cell_name': 'testcell', + 'domain': 'cell.local', + } + cm.get_effective_domain.return_value = 'cell.local' + cm.configs = {} + return cm + + def _make_template(self, **service_extras): + """Build a compose YAML string with optional extra service fields.""" + extra_lines = '' + for key, val in service_extras.items(): + if isinstance(val, bool): + extra_lines += f' {key}: {"true" if val else "false"}\n' + elif isinstance(val, list): + extra_lines += f' {key}:\n' + for item in val: + extra_lines += f' - {item}\n' + else: + extra_lines += f' {key}: {val}\n' + return ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + + extra_lines + + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + + def test_write_compose_raises_on_network_mode_host(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(network_mode='host') + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_on_pid_host(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(pid='host') + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_on_ipc_host(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(ipc='host') + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_on_userns_mode_host(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(userns_mode='host') + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_on_reserved_container_name(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(container_name='cell-api') + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_on_cap_add_all(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = self._make_template(cap_add=['ALL']) + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + + def test_write_compose_raises_when_no_external_network(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: ["echo"]\n' + 'networks:\n' + ' internal:\n' + ' driver: bridge\n' + ) + with self.assertRaises(ValueError): + composer.write_compose('testsvc', manifest, template) + + def test_write_compose_raises_on_string_command(self): + from service_composer import ServiceComposer + with tempfile.TemporaryDirectory() as tmpdir: + composer = ServiceComposer(config_manager=self._make_cm(), data_dir=tmpdir) + manifest = {'id': 'test', 'kind': 'store', 'config_schema': {}} + template = ( + 'version: "3.8"\n' + 'services:\n' + ' app:\n' + ' image: nginx\n' + ' command: "echo hello && rm -rf /"\n' + 'networks:\n' + ' cell-network:\n' + ' external: true\n' + ) + with self.assertRaises(ValueError) as ctx: + composer.write_compose('testsvc', manifest, template) + self.assertIn('security validation', str(ctx.exception)) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_optional_services_feature.py b/tests/test_optional_services_feature.py index 1c76080..7c854a8 100644 --- a/tests/test_optional_services_feature.py +++ b/tests/test_optional_services_feature.py @@ -58,6 +58,9 @@ def _store_manifest(service_id, subdomain=None, backend=None): return m +_FIXTURE_DIGEST = 'a' * 64 + + def _ssm_manifest(service_id='myapp', **overrides): """Minimal manifest that passes ServiceStoreManager._validate_manifest.""" m = { @@ -65,7 +68,7 @@ def _ssm_manifest(service_id='myapp', **overrides): 'name': 'My App', 'version': '1.0.0', 'author': 'Test Author', - 'image': f'git.pic.ngo/roof/{service_id}:latest', + 'image': f'git.pic.ngo/roof/{service_id}@sha256:{_FIXTURE_DIGEST}', 'container_name': f'cell-{service_id}', } m.update(overrides) diff --git a/tests/test_service_store_manager.py b/tests/test_service_store_manager.py index 614c2dc..5e44725 100644 --- a/tests/test_service_store_manager.py +++ b/tests/test_service_store_manager.py @@ -58,6 +58,12 @@ def _make_manager(tmp_dir=None, installed=None, identity=None): return mgr +_VALID_IMAGE = ( + 'git.pic.ngo/roof/myapp@sha256:' + + 'a' * 64 +) + + def _valid_manifest(**overrides): """Return a minimal valid manifest, with optional field overrides.""" m = { @@ -65,7 +71,7 @@ def _valid_manifest(**overrides): 'name': 'My App', 'version': '1.0.0', 'author': 'Test Author', - 'image': 'git.pic.ngo/roof/myapp:latest', + 'image': _VALID_IMAGE, 'container_name': 'cell-myapp', } m.update(overrides) @@ -143,16 +149,24 @@ class TestValidateManifestImage(unittest.TestCase): self.assertFalse(ok) self.assertTrue(any('image must match' in e for e in errs)) - def test_image_matching_git_pic_ngo_roof_with_tag_passes(self): - m = _valid_manifest(image='git.pic.ngo/roof/something:1.2.3') + def test_image_matching_git_pic_ngo_roof_with_digest_passes(self): + digest = 'a' * 64 + m = _valid_manifest(image=f'git.pic.ngo/roof/something@sha256:{digest}') ok, errs = ServiceStoreManager._validate_manifest(m) self.assertTrue(ok) self.assertEqual(errs, []) - def test_image_git_pic_ngo_roof_no_tag_passes(self): + def test_image_tag_only_rejected(self): + # Digest pinning is required; tag-only images are rejected. + m = _valid_manifest(image='git.pic.ngo/roof/something:1.2.3') + ok, errs = ServiceStoreManager._validate_manifest(m) + self.assertFalse(ok) + + def test_image_git_pic_ngo_roof_no_tag_rejected(self): + # No tag and no digest — rejected because digest pin is required. m = _valid_manifest(image='git.pic.ngo/roof/myservice') ok, errs = ServiceStoreManager._validate_manifest(m) - self.assertTrue(ok) + self.assertFalse(ok) def test_image_wrong_registry_rejected(self): m = _valid_manifest(image='ghcr.io/roof/myapp:latest')