From f7bb2cc96284bc3a7a44f359f8ff068c37f84d4d Mon Sep 17 00:00:00 2001 From: Dmitrii Iurco Date: Sat, 30 May 2026 03:09:41 -0400 Subject: [PATCH] fix: allow first-party store service subdomains and registry images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two manifest validation bugs blocked all store service installs: 1. service_store_manager.RESERVED_SUBDOMAINS included 'mail', which prevented the email service from using its required subdomain. Removed mail/calendar/files/webmail — they belong to official PIC store services and must be claimable by them. 2. manifest_validator required @sha256 digest pins on ALL images, including first-party git.pic.ngo/roof/* images that the PIC team builds and controls. service_store_manager._validate_manifest already only warned for first-party images; the secondary validator was stricter than intended, causing a hard reject on :latest tags. Aligned to warn-not-reject for first-party; malformed digests (when provided) are still a hard error. Co-Authored-By: Claude Sonnet 4.6 --- api/manifest_validator.py | 19 ++++++++++++++++--- api/service_store_manager.py | 4 +++- tests/test_manifest_validator.py | 25 ++++++++++++++----------- tests/test_service_store_manager.py | 12 ++++++------ 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/api/manifest_validator.py b/api/manifest_validator.py index c702a98..444825d 100644 --- a/api/manifest_validator.py +++ b/api/manifest_validator.py @@ -5,9 +5,12 @@ Both ServiceComposer and ServiceStoreManager import from here so validation logi lives in exactly one place and cannot be bypassed by taking either code path. """ +import logging import re import yaml +logger = logging.getLogger('picell') + _SUBDOMAIN_RE = re.compile(r'^[a-z][a-z0-9-]{0,30}$') _BACKEND_RE = re.compile(r'^[A-Za-z0-9._-]+:\d{1,5}$') _CAP_ALLOWLIST = frozenset({ @@ -73,13 +76,23 @@ def validate_manifest(manifest: dict) -> tuple: 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 must come from git.pic.ngo/roof/*; if a digest IS provided it must be + # valid; first-party images without a digest pin are allowed with a warning. image = manifest.get('image') if image is not None: - if not isinstance(image, str) or not _IMAGE_DIGEST_RE.match(image): + if not isinstance(image, str): + errors.append(f'image must be a string, got: {image!r}') + elif not image.startswith('git.pic.ngo/roof/'): errors.append( - f'image must match git.pic.ngo/roof/*@sha256:<64-hex>, got: {image!r}' + f'image must be from git.pic.ngo/roof/*, got: {image!r}' ) + elif '@sha256:' in image: + if not _IMAGE_DIGEST_RE.match(image): + errors.append( + f'image digest must match @sha256:<64-hex>, got: {image!r}' + ) + else: + logger.warning('manifest image %s has no digest pin', image) # container_name structural check cname = manifest.get('container_name') diff --git a/api/service_store_manager.py b/api/service_store_manager.py index e97a179..cc33cac 100644 --- a/api/service_store_manager.py +++ b/api/service_store_manager.py @@ -58,8 +58,10 @@ FORBIDDEN_MOUNTS = frozenset([ '/', '/etc', '/var', '/proc', '/sys', '/dev', '/app', '/run', '/boot', ]) RESERVED_SUBDOMAINS = frozenset([ - 'api', 'webui', 'admin', 'www', 'mail', 'ns1', 'ns2', + 'api', 'webui', 'admin', 'www', 'ns1', 'ns2', 'git', 'registry', 'install', + # mail, calendar, files, webmail are intentionally absent: + # they are claimed by official PIC store services. ]) ENV_VALUE_RE = re.compile(r'^[A-Za-z0-9._@:/+\-= ]*$') SUBDOMAIN_RE = re.compile(r'^[a-z][a-z0-9-]{0,30}$') diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index 352bb6e..811d599 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -162,12 +162,13 @@ class TestValidateManifest(unittest.TestCase): ) self.assertTrue(ok) - def test_image_tag_only_rejected(self): + def test_image_tag_only_first_party_allowed(self): + # First-party images without a digest pin are allowed (warning only). 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)) + self.assertTrue(ok) + self.assertEqual(errs, []) def test_image_wrong_registry_rejected(self): digest = 'a' * 64 @@ -190,11 +191,13 @@ class TestValidateManifest(unittest.TestCase): ) self.assertFalse(ok) - def test_image_no_tag_no_digest_rejected(self): + def test_image_no_tag_no_digest_first_party_allowed(self): + # No tag and no digest — Docker defaults to :latest; treated as tag-only, allowed. ok, errs = validate_manifest( _minimal_manifest(image='git.pic.ngo/roof/myapp') ) - self.assertFalse(ok) + self.assertTrue(ok) + self.assertEqual(errs, []) def test_image_absent_passes(self): m = _minimal_manifest() @@ -1261,12 +1264,12 @@ 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.""" + def test_validate_manifest_allows_image_tag_only_first_party(self): + """First-party images without a digest pin are allowed (warning only).""" 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) + self.assertTrue(ok) def test_validate_manifest_accepts_image_with_digest(self): from service_store_manager import ServiceStoreManager @@ -1309,7 +1312,8 @@ class TestInstallManifestValidation(unittest.TestCase): )) return ssm - def test_install_returns_error_when_image_tag_only(self): + def test_install_succeeds_when_image_tag_only_first_party(self): + # First-party images without a digest pin are allowed (warning, not error). manifest = { 'id': 'myapp', 'name': 'My App', @@ -1320,8 +1324,7 @@ class TestInstallManifestValidation(unittest.TestCase): } ssm = self._make_ssm(manifest) result = ssm.install('myapp') - self.assertFalse(result['ok']) - self.assertIn('errors', result) + self.assertTrue(result['ok']) def test_install_returns_error_when_kind_is_builtin(self): digest = 'c' * 64 diff --git a/tests/test_service_store_manager.py b/tests/test_service_store_manager.py index 5e44725..ea1554f 100644 --- a/tests/test_service_store_manager.py +++ b/tests/test_service_store_manager.py @@ -156,17 +156,17 @@ class TestValidateManifestImage(unittest.TestCase): self.assertTrue(ok) self.assertEqual(errs, []) - def test_image_tag_only_rejected(self): - # Digest pinning is required; tag-only images are rejected. + def test_image_tag_only_first_party_allowed(self): + # First-party images without a digest pin are allowed (warning only). m = _valid_manifest(image='git.pic.ngo/roof/something:1.2.3') ok, errs = ServiceStoreManager._validate_manifest(m) - self.assertFalse(ok) + self.assertTrue(ok) - def test_image_git_pic_ngo_roof_no_tag_rejected(self): - # No tag and no digest — rejected because digest pin is required. + def test_image_git_pic_ngo_roof_no_tag_allowed(self): + # No tag and no digest — Docker defaults to :latest; allowed for first-party. m = _valid_manifest(image='git.pic.ngo/roof/myservice') ok, errs = ServiceStoreManager._validate_manifest(m) - self.assertFalse(ok) + self.assertTrue(ok) def test_image_wrong_registry_rejected(self): m = _valid_manifest(image='ghcr.io/roof/myapp:latest')