From d3da858372b13dd054b2f0de48f8b9a89e3c3a54 Mon Sep 17 00:00:00 2001 From: sam Date: Sat, 24 Oct 2026 15:54:05 +0800 Subject: [PATCH] fix(e2b): accept optional observation domains safely --- services/core/tools/e2b-provider/README.md | 2 +- .../e2b-provider/observation_domain_test.py | 118 ++++++++++++++++++ services/core/tools/e2b-provider/provider.py | 14 ++- .../core/tools/e2b-provider/suspension.py | 2 +- .../tools/e2b-provider/suspension_test.py | 43 +++++++ 5 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 services/core/tools/e2b-provider/observation_domain_test.py diff --git a/services/core/tools/e2b-provider/README.md b/services/core/tools/e2b-provider/README.md index bee9d23c2..4863fc9b2 100644 --- a/services/core/tools/e2b-provider/README.md +++ b/services/core/tools/e2b-provider/README.md @@ -50,7 +50,7 @@ An unknown Create is never repeated. A Create whose connection material was lost ## Inspection and cleanup -Inspection uses SDK metadata and ID reads only. Inspection never uses SDK `connect`, which is reserved for the explicit managed Resume operation. The version-pinned constructor that restores a client from saved connection material is confined to [`sdk.py`](sdk.py) and covered by a no-connect, no-create test. Resource drift fails inspection but still permits ownership-based cleanup. +Inspection accepts an omitted `SandboxInfo.sandbox_domain`, as allowed by the pinned SDK; an explicit domain must match the configured domain or its subdomains. Create and Resume connection objects and clients restored from saved connection material must always have a matching domain before envd access. Ownership, template and resource checks apply regardless of the observation domain. Inspection uses SDK metadata and ID reads only. Inspection never uses SDK `connect`, which is reserved for the explicit managed Resume operation. The version-pinned constructor that restores a client from saved connection material is confined to [`sdk.py`](sdk.py) and covered by a no-connect, no-create test. Resource drift fails inspection but still permits ownership-based cleanup. `CreateSettled` proves that the original Create and bootstrap can no longer mutate; it is independent of `BootstrapComplete`. An empty lookup never settles an unknown Create. Kill destroys every matching sandbox, confirms that none remains and only then records a settled tombstone; it returns `State=absent` with `CreateSettled`. A settled rejected Create with no sandbox IDs proves absence without a cloud request, so GetInfo and Kill still succeed when the key is invalid. Ordinary missing compute has no such proof. diff --git a/services/core/tools/e2b-provider/observation_domain_test.py b/services/core/tools/e2b-provider/observation_domain_test.py new file mode 100644 index 000000000..644e35e30 --- /dev/null +++ b/services/core/tools/e2b-provider/observation_domain_test.py @@ -0,0 +1,118 @@ +"""Optional SDK observation metadata never relaxes connection validation.""" +from pathlib import Path +from types import SimpleNamespace +import unittest + +import provider_test + + +class ObservationDomainTest(unittest.TestCase): + setUp = provider_test.ProviderTest.setUp + call = provider_test.ProviderTest.call + record = provider_test.ProviderTest.record + + def observed(self, **changes): + fields = dict(sandbox_id=self.cloud.sandbox_id, sandbox_domain=None, + metadata=self.cloud.metadata, template_id=self.cloud.template_id, + cpu_count=2, memory_mb=2048, state=self.cloud.state) + fields.update(changes) + return SimpleNamespace(**fields) + + def test_create_and_inspect_missing_observation_domain_use_saved_connection(self): + self.config['Resources'] = {'cpus': 2, 'memory_mib': 2048} + self.api.get_info.side_effect = lambda *a, **k: self.observed() + self.assertEqual(self.call('create')['ErrorCode'], '') + self.assertTrue(self.call('inspect')['Info']['BootstrapComplete']) + self.assertEqual(self.call('renew')['ErrorCode'], '') + self.assertEqual(self.record()['connection']['sandbox_domain'], 'e2b.app') + self.api.connect.assert_not_called() + self.api.create.assert_called_once() + + def test_paused_inspection_without_domain_does_not_wake(self): + self.api.get_info.side_effect = lambda *a, **k: self.observed() + self.assertEqual(self.call('create')['ErrorCode'], '') + self.cloud.state = 'paused' + result = self.call('inspect') + self.assertEqual(result['ErrorCode'], '') + self.assertEqual(result['Info']['State'], 'paused') + self.api.connect.assert_not_called() + + def test_unknown_create_list_without_domain_never_recreates_or_attaches(self): + self.api.create.side_effect = TimeoutError('unknown create') + self.assertEqual(self.call('create')['ErrorCode'], 'unconfirmed') + self.api.list.return_value = SimpleNamespace( + has_next=True, next_items=lambda **kwargs: [self.observed()]) + # One page exhausts the SDK paginator. + def page(**kwargs): + self.api.list.return_value.has_next = False + return [self.observed()] + self.api.list.return_value.next_items = page + result = self.call('inspect') + self.assertEqual(result['ErrorCode'], '') + self.assertFalse(result['Info']['BootstrapComplete']) + self.api.create.assert_called_once() + self.api.connect.assert_not_called() + self.cloud.files.write.assert_not_called() + + def test_missing_domain_does_not_bypass_identity_or_resource_checks(self): + self.config['Resources'] = {'cpus': 2, 'memory_mib': 2048} + for changes, error in (({'metadata': {}}, 'ownership'), + ({'template_id': 'other'}, 'invalid'), + ({'cpu_count': 4}, 'invalid'), + ({'memory_mb': 8192}, 'invalid')): + with self.subTest(changes=changes): + for path in Path(self.temporary.name).glob('*.json'): + path.unlink() + self.api.get_info.return_value = self.observed(**changes) + self.assertEqual(self.call('create')['ErrorCode'], error) + self.cloud.files.write.assert_not_called() + self.api.connect.assert_not_called() + + def test_explicit_invalid_observation_domain_rejected_before_credentials(self): + for domain in ('foreign.example', 'e2b.app.foreign.example', '', 3): + with self.subTest(domain=domain): + for path in Path(self.temporary.name).glob('*.json'): + path.unlink() + self.api.get_info.return_value = self.observed(sandbox_domain=domain) + self.assertEqual(self.call('create')['ErrorCode'], 'ownership') + self.cloud.files.write.assert_not_called() + self.api.connect.assert_not_called() + + def test_rejected_connection_cannot_reenter_through_missing_observation_domain(self): + for domain in ('foreign.example', None): + with self.subTest(domain=domain): + for path in Path(self.temporary.name).glob('*.json'): + path.unlink() + self.cloud.sandbox_domain = domain + self.api.get_info.return_value = self.observed() + self.assertEqual(self.call('create')['ErrorCode'], 'ownership') + self.assertEqual(self.call('inspect')['ErrorCode'], 'ownership') + self.cloud.files.read.assert_not_called() + self.cloud.files.write.assert_not_called() + self.api.connect.assert_not_called() + + def test_rejected_observation_cannot_resume_bootstrap_on_later_missing_domain(self): + self.api.get_info.return_value = self.observed(sandbox_domain='foreign.example') + self.assertEqual(self.call('create')['ErrorCode'], 'ownership') + self.assertEqual(self.record()['status'], 'configuration_rejected') + self.api.get_info.return_value = self.observed() + result = self.call('inspect') + self.assertEqual(result['ErrorCode'], '') + self.assertFalse(result['Info']['BootstrapComplete']) + self.assertTrue(result['Info']['CreateSettled']) + self.assertEqual(self.record()['status'], 'configuration_rejected') + self.cloud.files.read.assert_not_called() + self.cloud.files.write.assert_not_called() + self.api.connect.assert_not_called() + + def test_missing_actual_connection_domain_is_still_rejected(self): + self.api.get_info.return_value = self.observed() + self.cloud.sandbox_domain = None + self.assertEqual(self.call('create')['ErrorCode'], 'ownership') + self.api.get_info.assert_not_called() + self.cloud.files.write.assert_not_called() + self.api.connect.assert_not_called() + + +if __name__ == '__main__': + unittest.main() diff --git a/services/core/tools/e2b-provider/provider.py b/services/core/tools/e2b-provider/provider.py index 5c985cce4..ba6c4d127 100644 --- a/services/core/tools/e2b-provider/provider.py +++ b/services/core/tools/e2b-provider/provider.py @@ -190,15 +190,18 @@ def owns(self, cloud): raise Failure('ownership') return cloud - def check_domain(self, cloud): + def check_domain(self, sandbox_domain): domain = self.options()['domain'] - sandbox_domain = cloud.sandbox_domain if not isinstance(sandbox_domain, str) or not ( sandbox_domain == domain or sandbox_domain.endswith('.' + domain)): raise Failure('ownership') def qualified(self, cloud): - self.check_domain(cloud) + # SandboxInfo omits the optional domain on the official API. It is an + # observation, not connection material; Create and saved clients still + # require a qualified domain before any envd access. + if cloud.sandbox_domain is not None: + self.check_domain(cloud.sandbox_domain) template = self.config['Template'].split(':', 1)[0] if cloud.template_id not in (template, self.config['Template']): raise Failure('invalid') @@ -236,6 +239,7 @@ def client(self, cloud): material = (self.receipt.data or {}).get('connection') if not material or material['sandbox_id'] != cloud.sandbox_id: raise Failure('unconfirmed') + self.check_domain(material['sandbox_domain']) return restore(material, self.options()) def inspect(self): @@ -252,7 +256,7 @@ def inspect(self): self.qualified(cloud) record = self.receipt.data or {} if (cloud.state == 'running' and not record.get('bootstrap_complete') and record.get('connection') - and record.get('status') not in ('bootstrap_failed', 'killed')): + and record.get('status') not in ('configuration_rejected', 'bootstrap_failed', 'killed')): try: receipt = json.loads(self.client(cloud).files.read('/root/.oac/e2b/managed-ready.json', user='root', request_timeout=self.remaining())) @@ -292,7 +296,7 @@ def create(self): compute={'Generation': 0, 'Name': self.reference['AllocationID'], 'ID': cloud.sandbox_id, 'RestoredFrom': None}) # A create response must not steer envd traffic to an unrelated host. - self.check_domain(cloud) + self.check_domain(cloud.sandbox_domain) # SDK Create returns connection material, but no metadata or resources. # Read its exact ID before writing credentials, even when Core adopts the # template's resources and does not supply explicit limits. diff --git a/services/core/tools/e2b-provider/suspension.py b/services/core/tools/e2b-provider/suspension.py index 453fb9ac6..531b6e2c8 100644 --- a/services/core/tools/e2b-provider/suspension.py +++ b/services/core/tools/e2b-provider/suspension.py @@ -181,7 +181,7 @@ def resume(self): on_resume='restore', **self.p.options()) if connected.sandbox_id != target['ID']: raise Failure('ownership') - self.p.check_domain(connected) + self.p.check_domain(connected.sandbox_domain) entry = dict(entry, connected=True) self.p.receipt.save(suspension=entry, connection=self.material(connected)) else: diff --git a/services/core/tools/e2b-provider/suspension_test.py b/services/core/tools/e2b-provider/suspension_test.py index de1d6c5e0..f67083d37 100644 --- a/services/core/tools/e2b-provider/suspension_test.py +++ b/services/core/tools/e2b-provider/suspension_test.py @@ -1,5 +1,6 @@ """Native lifecycle effects and durable recovery are tested through the helper.""" import unittest +from types import SimpleNamespace from uuid import uuid4 from provider import Provider @@ -35,6 +36,48 @@ def resume_request(self, retained): return {'Reference': self.reference, 'OperationID': str(uuid4()), 'Target': target, 'Retained': retained, 'ReconcileOnly': False} + def missing_domain_observations(self): + self.api.get_info.side_effect = lambda *a, **k: SimpleNamespace( + sandbox_id=self.cloud.sandbox_id, sandbox_domain=None, + metadata=self.cloud.metadata, template_id=self.cloud.template_id, + state=self.cloud.state, cpu_count=2, memory_mb=2048) + + def test_missing_observation_domain_preserves_explicit_pause_resume(self): + self.missing_domain_observations() + current = self.prepare() + _, paused = self.pause(current) + self.assertEqual(paused['ErrorCode'], '') + self.assertTrue(paused['State']['ResourcesReleased']) + self.assertEqual(self.invoke('compute_info', Compute=current)['State']['Status'], 'paused') + self.api.connect.assert_not_called() + request = self.resume_request(paused['State']['Retained']) + resumed = self.invoke('resume', Resume=request) + self.assertEqual(resumed['ErrorCode'], '') + self.assertEqual(resumed['State']['Compute']['ID'], current['ID']) + self.assertEqual(resumed['State']['Compute']['Generation'], current['Generation'] + 1) + self.assertEqual(self.record()['connection']['sandbox_domain'], 'e2b.app') + self.api.connect.assert_called_once() + self.assertEqual(self.api.connect.call_args.kwargs['on_resume'], 'restore') + + def invalid_resume_connection_domain(self, domain): + self.missing_domain_observations() + current = self.prepare() + _, paused = self.pause(current) + request = self.resume_request(paused['State']['Retained']) + self.cloud.sandbox_domain = domain + self.assertEqual(self.invoke('resume', Resume=request)['ErrorCode'], 'ownership') + self.assertEqual(self.record()['connection']['sandbox_domain'], 'e2b.app') + self.assertFalse(self.record()['suspension']['connected']) + self.api.connect.assert_called_once() + self.assertEqual(self.invoke('resume', Resume=dict(request, ReconcileOnly=True))['ErrorCode'], 'unconfirmed') + self.api.connect.assert_called_once() + + def test_missing_resume_connection_domain_is_rejected(self): + self.invalid_resume_connection_domain(None) + + def test_foreign_resume_connection_domain_is_rejected(self): + self.invalid_resume_connection_domain('foreign.example') + def test_resume_rejects_external_workspace_before_sdk(self): from state import Failure with self.assertRaises(Failure) as raised: