From ee7f9847dc3f56f178e2ea90a56857da36955e29 Mon Sep 17 00:00:00 2001 From: Zhang Yue Date: Fri, 3 Apr 2026 16:46:15 +0800 Subject: [PATCH] fix(server): enforce renew expiresAt must be after current expiresAt Add the missing validation to both Docker and Kubernetes renew_expiration implementations. Return HTTP 400 with INVALID_EXPIRATION error code when the requested time is not strictly after the current expiration. --- server/opensandbox_server/services/docker.py | 11 ++++++- .../services/k8s/kubernetes_service.py | 9 ++++++ server/tests/k8s/test_kubernetes_service.py | 22 +++++++++++++- server/tests/test_docker_service.py | 29 +++++++++++++++++++ 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/server/opensandbox_server/services/docker.py b/server/opensandbox_server/services/docker.py index 89e3c980b..50e8ce8bc 100644 --- a/server/opensandbox_server/services/docker.py +++ b/server/opensandbox_server/services/docker.py @@ -1695,7 +1695,8 @@ def renew_expiration( "message": f"Sandbox {sandbox_id} does not have automatic expiration enabled.", }, ) - if self._get_tracked_expiration(sandbox_id, labels) is None: + current_expiration = self._get_tracked_expiration(sandbox_id, labels) + if current_expiration is None: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail={ @@ -1705,6 +1706,14 @@ def renew_expiration( ), }, ) + if new_expiration <= current_expiration: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "code": SandboxErrorCodes.INVALID_EXPIRATION, + "message": "New expiration time must be after the current expiresAt time.", + }, + ) # Persist the new timeout in memory; it will also be respected on restart via _restore_existing_sandboxes self._schedule_expiration(sandbox_id, new_expiration) diff --git a/server/opensandbox_server/services/k8s/kubernetes_service.py b/server/opensandbox_server/services/k8s/kubernetes_service.py index 2f0160aff..a53c03cf7 100644 --- a/server/opensandbox_server/services/k8s/kubernetes_service.py +++ b/server/opensandbox_server/services/k8s/kubernetes_service.py @@ -660,6 +660,15 @@ def renew_expiration( }, ) + if new_expiration <= current_expiration: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "code": SandboxErrorCodes.INVALID_EXPIRATION, + "message": "New expiration time must be after the current expiresAt time.", + }, + ) + # Update BatchSandbox spec.expireTime field self.workload_provider.update_expiration( sandbox_id=sandbox_id, diff --git a/server/tests/k8s/test_kubernetes_service.py b/server/tests/k8s/test_kubernetes_service.py index 7c4b1771f..72d7724fa 100644 --- a/server/tests/k8s/test_kubernetes_service.py +++ b/server/tests/k8s/test_kubernetes_service.py @@ -643,11 +643,12 @@ def test_renew_expiration_succeeds(self, k8s_service, mock_workload): Purpose: Verify that sandbox expiration can be successfully renewed """ + current_expiration = datetime.now(timezone.utc) + timedelta(hours=1) new_expiration = datetime.now(timezone.utc) + timedelta(hours=2) k8s_service.workload_provider.get_workload.return_value = mock_workload k8s_service.workload_provider.update_expiration.return_value = None - k8s_service.workload_provider.get_expiration.return_value = new_expiration + k8s_service.workload_provider.get_expiration.return_value = current_expiration from opensandbox_server.api.schema import RenewSandboxExpirationRequest request = RenewSandboxExpirationRequest(expires_at=new_expiration) @@ -694,3 +695,22 @@ def test_renew_returns_409_when_sandbox_has_no_expiration(self, k8s_service): assert exc_info.value.status_code == 409 assert "does not have automatic expiration" in exc_info.value.detail["message"] k8s_service.workload_provider.update_expiration.assert_not_called() + + def test_renew_rejects_time_before_current_expires_at(self, k8s_service): + """Renew is rejected when the new expiresAt is not after the current expiresAt (spec alignment).""" + current_expiration = datetime.now(timezone.utc) + timedelta(hours=2) + # Request a time that is in the future but earlier than the current expiration + earlier_time = datetime.now(timezone.utc) + timedelta(hours=1) + + k8s_service.workload_provider.get_workload.return_value = MagicMock() + k8s_service.workload_provider.get_expiration.return_value = current_expiration + + from opensandbox_server.api.schema import RenewSandboxExpirationRequest + request = RenewSandboxExpirationRequest(expires_at=earlier_time) + + with pytest.raises(HTTPException) as exc_info: + k8s_service.renew_expiration("test-sandbox-id", request) + + assert exc_info.value.status_code == 400 + assert "after the current expiresAt" in exc_info.value.detail["message"] + k8s_service.workload_provider.update_expiration.assert_not_called() diff --git a/server/tests/test_docker_service.py b/server/tests/test_docker_service.py index 1249dfb2e..8cc22b89c 100644 --- a/server/tests/test_docker_service.py +++ b/server/tests/test_docker_service.py @@ -827,6 +827,35 @@ def test_renew_expiration_rejects_manual_cleanup_sandbox(): assert exc_info.value.detail["message"] == "Sandbox manual-id does not have automatic expiration enabled." +def test_renew_expiration_rejects_time_before_current_expires_at(): + """Renew must be rejected when the new expiresAt is before the current expiresAt (spec alignment).""" + service = DockerSandboxService(config=_app_config()) + current_expiration = datetime.now(timezone.utc) + timedelta(hours=2) + # Request a time that is in the future but earlier than the current expiration + earlier_time = datetime.now(timezone.utc) + timedelta(hours=1) + + container = MagicMock() + container.attrs = { + "Config": { + "Labels": { + SANDBOX_ID_LABEL: "sbx-001", + SANDBOX_EXPIRES_AT_LABEL: current_expiration.isoformat(), + } + } + } + request = MagicMock(expires_at=earlier_time) + + with ( + patch.object(service, "_get_container_by_sandbox_id", return_value=container), + patch.object(service, "_get_tracked_expiration", return_value=current_expiration), + ): + with pytest.raises(HTTPException) as exc_info: + service.renew_expiration("sbx-001", request) + + assert exc_info.value.status_code == status.HTTP_400_BAD_REQUEST + assert "after the current expiresAt" in exc_info.value.detail["message"] + + @pytest.mark.asyncio @patch("opensandbox_server.services.docker.docker") async def test_create_sandbox_async_returns_provisioning(mock_docker):