Apply CodeRabbit review: scope CI token, bound docker subprocesses, validate output cap

- Scope LOCALSTACK_AUTH_TOKEN to the integration-test step so checkout/setup
  steps never receive the secret (least privilege).
- Bound the container startup (docker run + readiness) with a single
  startup_timeout deadline in __aenter__, and bound the docker stop during
  cleanup, so an unresponsive Docker daemon raises LocalStackError instead of
  hanging __aenter__/__aexit__.
- Reject non-positive max_output_chars at construction; text[-0:] would
  otherwise return the full, untruncated output.

.github/workflows/main.yml

@@ -160,10 +160,9 @@ jobs:
     # satisfies zizmor's secrets-outside-env audit (same pattern as the release job).
     environment: localstack-integration
     env:
-      # Since LocalStack 2026.03.0 the single image requires an auth token to
-      # start, so the live tests need it. LOCALSTACK_REQUIRE_AUTH_TOKEN makes a
-      # missing token fail the job instead of silently skipping the container tests.
-      LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_AUTH_TOKEN }}
+      # LOCALSTACK_REQUIRE_AUTH_TOKEN makes a missing token fail the job instead
+      # of silently skipping the container tests. The token itself is scoped to
+      # the test step below so checkout/setup steps never see the secret.
       LOCALSTACK_REQUIRE_AUTH_TOKEN: '1'
       LOCALSTACK_AWS_CLI: aws
       LOCALSTACK_IMAGE: localstack/localstack
@@ -188,6 +187,9 @@ jobs:
       - run: aws --version
 
       - run: make integration-localstack
+        env:
+          # Since LocalStack 2026.03.0 the single image requires an auth token to start.
+          LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_AUTH_TOKEN }}
 
   coverage:
     needs: [test]

pydantic_ai_harness/localstack/_container.py

@@ -85,10 +85,18 @@ class LocalStackContainer:
         return self._container_id
 
     async def __aenter__(self) -> Self:
-        """Start the container and wait for it to become ready."""
-        self._container_id = await self._start()
+        """Start the container and wait for it to become ready.
+
+        `startup_timeout` bounds the whole sequence, so an unresponsive Docker
+        daemon that hangs `docker run` fails here rather than blocking forever.
+        """
         try:
-            await self._wait_until_ready()
+            with anyio.fail_after(self._startup_timeout):
+                self._container_id = await self._start()
+                await self._wait_until_ready()
+        except TimeoutError as e:
+            await self._stop()
+            raise LocalStackError(f'LocalStack did not become ready within {self._startup_timeout}s.') from e
         except BaseException:
             await self._stop()
             raise
@@ -162,15 +170,11 @@ class LocalStackContainer:
         return result.stdout.decode('utf-8', errors='replace').strip()
 
     async def _wait_until_ready(self) -> None:
-        """Poll the health endpoint until LocalStack responds or the timeout elapses."""
+        """Poll the health endpoint until LocalStack responds; `__aenter__` bounds the wait."""
         url = self._readiness_url
-        try:
-            with anyio.fail_after(self._startup_timeout):
-                async with httpx.AsyncClient() as client:
-                    while not await self._is_ready(client, url):
-                        await anyio.sleep(self._poll_interval)
-        except TimeoutError as e:
-            raise LocalStackError(f'LocalStack did not become ready within {self._startup_timeout}s.') from e
+        async with httpx.AsyncClient() as client:
+            while not await self._is_ready(client, url):
+                await anyio.sleep(self._poll_interval)
 
     async def _is_ready(self, client: httpx.AsyncClient, url: str) -> bool:
         """Return True once the health endpoint answers with HTTP 200."""
@@ -188,6 +192,7 @@ class LocalStackContainer:
         self._container_id = None
         with anyio.CancelScope(shield=True):
             try:
-                await anyio.run_process([self._docker_path, 'stop', container_id], check=False)
-            except FileNotFoundError:
+                with anyio.fail_after(self._startup_timeout):
+                    await anyio.run_process([self._docker_path, 'stop', container_id], check=False)
+            except (FileNotFoundError, TimeoutError):
                 pass

pydantic_ai_harness/localstack/_toolset.py

@@ -77,6 +77,8 @@ class LocalStackToolset(FunctionToolset[AgentDepsT]):
         super().__init__()
         if allowed_services and denied_services:
             raise ValueError('Specify allowed_services or denied_services, not both.')
+        if max_output_chars <= 0:
+            raise ValueError('max_output_chars must be a positive integer.')
 
         self._endpoint_url = endpoint_url
         self._region = region

tests/localstack/test_ci_workflow.py

@@ -29,6 +29,16 @@ def test_localstack_ci_authenticates_with_an_auth_token() -> None:
 
     # The single image requires a token since LocalStack 2026.03.0; the deprecated
     # LOCALSTACK_ACKNOWLEDGE_ACCOUNT_REQUIREMENT bypass expired and must not return.
-    assert '      LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_AUTH_TOKEN }}' in lines
     assert "      LOCALSTACK_REQUIRE_AUTH_TOKEN: '1'" in lines
     assert not any('LOCALSTACK_ACKNOWLEDGE_ACCOUNT_REQUIREMENT' in line for line in lines)
+
+
+def test_localstack_ci_scopes_the_auth_token_to_the_test_step() -> None:
+    lines = _workflow_lines()
+
+    # The token is scoped to the integration-test step, not the job-level env, so
+    # the checkout and setup steps never receive the secret.
+    run_index = lines.index('      - run: make integration-localstack')
+    step_block = lines[run_index : run_index + 4]
+    assert '          LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_AUTH_TOKEN }}' in step_block
+    assert '      LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_AUTH_TOKEN }}' not in lines

tests/localstack/test_container.py

@@ -68,6 +68,14 @@ def _failing_docker_stub(tmp_path: Path) -> str:
     return str(stub)
 
 
+def _hanging_docker_stub(tmp_path: Path) -> str:
+    """A fake `docker` CLI that never returns, standing in for an unresponsive daemon."""
+    stub = tmp_path / 'docker-hang'
+    stub.write_text('#!/bin/sh\nsleep 30\n')
+    stub.chmod(stub.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
+    return str(stub)
+
+
 class TestProperties:
     def test_endpoint_url(self, tmp_path: Path) -> None:
         docker, _ = _docker_stub(tmp_path)
@@ -235,10 +243,10 @@ class TestLifecycle:
     async def test_readiness_timeout_stops_container(self, tmp_path: Path) -> None:
         docker, log = _docker_stub(tmp_path)
         port = unused_tcp_port()
-        with pytest.raises(LocalStackError, match='did not become ready within 0.05s'):
-            async with LocalStackContainer(
-                docker_path=docker, host_port=port, startup_timeout=0.05, poll_interval=0.01
-            ):
+        # startup_timeout must clear `docker run` (the stub subprocess) so the deadline
+        # is spent on readiness polling, which never succeeds against the unused port.
+        with pytest.raises(LocalStackError, match='did not become ready within 0.3s'):
+            async with LocalStackContainer(docker_path=docker, host_port=port, startup_timeout=0.3, poll_interval=0.01):
                 pass  # pragma: no cover
         assert 'stop container-abc123' in log.read_text()
 
@@ -252,6 +260,17 @@ class TestLifecycle:
             async with LocalStackContainer(docker_path=_failing_docker_stub(tmp_path)):
                 pass  # pragma: no cover
 
+    async def test_start_timeout_when_docker_hangs(self, tmp_path: Path) -> None:
+        with pytest.raises(LocalStackError, match='did not become ready within 0.05s'):
+            async with LocalStackContainer(docker_path=_hanging_docker_stub(tmp_path), startup_timeout=0.05):
+                pass  # pragma: no cover
+
+    async def test_stop_timeout_is_swallowed_when_docker_hangs(self, tmp_path: Path) -> None:
+        container = LocalStackContainer(docker_path=_hanging_docker_stub(tmp_path), startup_timeout=0.05)
+        container._container_id = 'stuck'
+        await container.__aexit__(None, None, None)
+        assert container.container_id is None
+
     async def test_exit_without_start_is_safe(self, tmp_path: Path) -> None:
         docker, log = _docker_stub(tmp_path)
         await LocalStackContainer(docker_path=docker).__aexit__(None, None, None)

tests/localstack/test_localstack.py

@@ -71,6 +71,10 @@ class TestConstruction:
         with pytest.raises(ValueError, match='Specify allowed_services or denied_services, not both.'):
             _toolset(allowed_services=['s3'], denied_services=['dynamodb'])
 
+    def test_non_positive_max_output_chars_rejected(self) -> None:
+        with pytest.raises(ValueError, match='max_output_chars must be a positive integer.'):
+            _toolset(max_output_chars=0)
+
 
 class TestAwsCli:
     async def test_injects_endpoint_region_and_command(self, tmp_path: Path) -> None:
@@ -333,12 +337,12 @@ class TestContainerManagement:
 
     async def test_managed_defaults_to_edge_port_when_endpoint_has_no_port(self, tmp_path: Path) -> None:
         docker, log = _docker_stub(tmp_path)
-        with pytest.raises(LocalStackError, match='did not become ready within 0.05s'):
+        with pytest.raises(LocalStackError, match='did not become ready within 0.3s'):
             async with _toolset(
                 endpoint_url='http://localhost',
                 manage_container=True,
                 docker_path=docker,
-                startup_timeout=0.05,
+                startup_timeout=0.3,
             ):
                 pass  # pragma: no cover
         log_text = log.read_text()