fix: post-Phase-0 corrections — data-dir bind mounts, reserved subdomains, list_active()
Unit Tests / test (push) Successful in 11m31s
Unit Tests / test (push) Successful in 11m31s
Three related fixes discovered during review of Phase 0 and Phase 1 manifests:
1. validate_rendered_compose(): add allowed_data_dir param. After ${PIC_DATA_DIR}
substitution, compose templates produce absolute paths; without this the
validator would reject every service install. ServiceComposer.write_compose()
now passes its resolved data_dir so only the designated data directory is
exempt — /etc, /proc, docker.sock etc. still blocked.
2. _RESERVED_SUBDOMAINS: remove service-level subdomains (mail, calendar, files,
webdav, webmail). The reserved list should protect PIC infrastructure endpoints
(api, webui, admin) — not service subdomains that official store services
(calendar, files, webmail) must be allowed to claim. Aligns with the
existing _RESERVED_SUBS in service_registry.py.
3. ServiceRegistry.list_active(): new method returning only installed store
services (no builtins). This is the forward-looking API that Phase 2 will
make the primary read path once builtins are deleted. Adding it now unblocks
the QA agent's test_optional_services_feature.py which was already testing
the expected Phase 2 behaviour.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -164,13 +164,15 @@ class TestValidateManifest(unittest.TestCase):
|
||||
ok, errs = validate_manifest(m)
|
||||
self.assertTrue(ok)
|
||||
|
||||
def test_subdomain_calendar_rejected(self):
|
||||
def test_subdomain_calendar_passes(self):
|
||||
# calendar/files/webmail/etc are service subdomains, not infrastructure;
|
||||
# official PIC store services need to claim them.
|
||||
ok, errs = validate_manifest(_minimal_manifest(subdomain='calendar'))
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(ok, errs)
|
||||
|
||||
def test_subdomain_files_rejected(self):
|
||||
def test_subdomain_files_passes(self):
|
||||
ok, errs = validate_manifest(_minimal_manifest(subdomain='files'))
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(ok, errs)
|
||||
|
||||
# ── extra_subdomains ─────────────────────────────────────────────────
|
||||
|
||||
@@ -489,6 +491,56 @@ class TestValidateRenderedCompose(unittest.TestCase):
|
||||
ok, errs = validate_rendered_compose(yaml_text)
|
||||
self.assertTrue(ok)
|
||||
|
||||
def test_absolute_volume_under_allowed_data_dir_passes(self):
|
||||
# After ${PIC_DATA_DIR} substitution, compose templates produce absolute
|
||||
# paths like /data/services/email/mail:/var/mail. These must be allowed
|
||||
# when allowed_data_dir is set to the same prefix.
|
||||
yaml_text = (
|
||||
'services:\n'
|
||||
' mail:\n'
|
||||
' image: svc-email:latest\n'
|
||||
' command: ["postfix"]\n'
|
||||
' volumes:\n'
|
||||
' - /data/services/email/mail:/var/mail\n'
|
||||
' - /data/services/email/config:/tmp/config\n'
|
||||
'networks:\n'
|
||||
' cell-network:\n'
|
||||
' external: true\n'
|
||||
)
|
||||
ok, errs = validate_rendered_compose(yaml_text, allowed_data_dir='/data')
|
||||
self.assertTrue(ok, errs)
|
||||
|
||||
def test_absolute_volume_outside_allowed_data_dir_still_rejected(self):
|
||||
yaml_text = (
|
||||
'services:\n'
|
||||
' app:\n'
|
||||
' image: nginx\n'
|
||||
' volumes:\n'
|
||||
' - /etc/passwd:/etc/passwd\n'
|
||||
'networks:\n'
|
||||
' cell-network:\n'
|
||||
' external: true\n'
|
||||
)
|
||||
ok, errs = validate_rendered_compose(yaml_text, allowed_data_dir='/data')
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(any('bind mount' in e for e in errs))
|
||||
|
||||
def test_absolute_volume_rejected_when_no_allowed_data_dir(self):
|
||||
yaml_text = (
|
||||
'services:\n'
|
||||
' app:\n'
|
||||
' image: nginx\n'
|
||||
' volumes:\n'
|
||||
' - /data/services/email/mail:/var/mail\n'
|
||||
'networks:\n'
|
||||
' cell-network:\n'
|
||||
' external: true\n'
|
||||
)
|
||||
# Without allowed_data_dir, even /data paths are rejected
|
||||
ok, errs = validate_rendered_compose(yaml_text)
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(any('bind mount' in e for e in errs))
|
||||
|
||||
# ── cap_add ──────────────────────────────────────────────────────────
|
||||
|
||||
def test_compose_cap_add_sys_admin_rejected(self):
|
||||
|
||||
Reference in New Issue
Block a user