test(appsec): make server startup failures diagnosable and port reuse safe (#19674)

APPSEC-69623

All 13 quarantined tests here fail in `appsec_application_server`, not in their own bodies. The recorded error is the "Server failed to start" assertion, and the runs with a duration cluster at 22.0–22.6s against a ~13s startup budget.

**The failures are undebuggable.** The assertion interpolated `getattr(server_process, "stdout", None)` — `None` for a `multiprocessing.Process`, a stream rather than text for a `Popen` — so every one reports a literal `None`. It now reports what is *not* already in the captured output: exit code (`None` = too slow, non-zero = died), whether the port is still taken, and the command.

**Port 8050 is shared by 31 tests**, these suites run serially, and teardown is best effort (SIGTERM to the process group, `join(timeout=5)`, each step in `except/pass`). A gunicorn worker outliving that leaves the port taken and the next test can't bind. Now waits for the port both before starting and after tearing down.

**The suites never ran.** The first push was green with zero test events for any of the 13 tests: `tests/appsec/appsec_utils.py` matched 15 suites but none of the `appsec_integrations_*` ones that import it, because `@appsec` covers `ddtrace/appsec/*` source only and each suite lists just its own test directory. The second commit adds the file to the six suites whose tests import it (grep-verified: `flask_tests` 7 files, `fastapi_tests` 2, `django_tests` 1, plus `iast_packages` and `iast_tdd_propagation`). It now matches 21 suites; `suitespec-check` passes both gates.

Notes:
- The port wait tests whether the port can be **bound**, not whether it accepts connections. `connect()` reports a port free once a bound server's listen backlog fills — exactly the wedged state worth catching — and opens real connections to a live server. My first version did use `connect()` and wrongly reported "released" after 0.3s against a still-bound socket.
- Warns rather than raises when the port never frees; raising in teardown would turn one leak into a failure across every appsec server suite.
- No per-worker port offsets: these suites have no `-n` in the riotfile, so the collision is sequential, not concurrent.
- The newly triggered suites also hold quarantined tests this branch does not fix — the SCA reachability pair (fixed in #19671) and `test_django_insecure_cookie_secure` (an `index_aspect` bug). Deliberately left unkeyed rather than un-quarantined on a commit that would not fix them.

Blast radius: `appsec_application_server` backs all five server context managers, and the suitespec change widens what CI runs on such edits — both intended.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: christophe.papazian <christophe.papazian@datadoghq.com>

Author: christophe-papazian

tests/appsec/appsec_utils.py

@@ -22,6 +22,50 @@
 FILE_PATH = Path(__file__).resolve().parent
 
 
+def _port_is_available(port: int) -> bool:
+    """Whether a server could bind the port right now.
+
+    Binding is the question that matters, since it is what the next server does. Probing with
+    connect() instead reports a port as free once a bound server's listen backlog fills, and
+    opens real connections to a live server.
+    """
+    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
+        sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        try:
+            sock.bind(("0.0.0.0", int(port)))
+            return True
+        except OSError:
+            return False
+
+
+def _wait_for_port_release(port: int, timeout: float = 10.0) -> bool:
+    """Wait until the port can be bound again, returning False if it never can.
+
+    Server teardown is best effort and gunicorn workers can outlive it, so without this the
+    next test to use the same port fails to bind. 31 tests share port 8050.
+    """
+    deadline = time.monotonic() + timeout
+    while time.monotonic() < deadline:
+        if _port_is_available(port):
+            return True
+        time.sleep(0.1)
+    return _port_is_available(port)
+
+
+def _server_diagnostics(server_process, port: int, cmd: list) -> str:
+    """Facts that are not in the captured server output but explain most startup failures."""
+    if isinstance(server_process, multiprocessing.Process):
+        exit_code = server_process.exitcode
+    else:
+        exit_code = server_process.poll()
+    return (
+        f"port={port} port_still_bound={not _port_is_available(port)} pid={server_process.pid} "
+        f"exit_code={exit_code} (None means it was still running, so it was too slow rather "
+        f"than dead; a non-zero code with the port bound means another server still holds it)\n"
+        f"command={cmd}"
+    )
+
+
 @contextmanager
 def gunicorn_flask_server(
     use_ddtrace_cmd: bool = True,
@@ -335,6 +379,10 @@ def appsec_application_server(
         if preexec is not None:
             subprocess_kwargs["preexec_fn"] = preexec  # type: ignore[assignment]
 
+    # A previous test's server may still hold the port, which would make this one fail to bind.
+    if not _wait_for_port_release(port):
+        print(f"WARNING: port {port} was still bound when starting the server")
+
     if use_multiprocess:
         # Run the server command by replacing the child Python process with the target binary (exec),
         # ensuring signals/termination behave like the subprocess.Popen path.
@@ -374,17 +422,13 @@ def appsec_application_server(
                 print("Server started")
         except RetryError:
             raise AssertionError(
-                "Server failed to start, see stdout and stderr logs"
-                "\n=== Captured STDOUT ===\n%s=== End of captured STDOUT ==="
-                "\n=== Captured STDERR ===\n%s=== End of captured STDERR ==="
-                % (getattr(server_process, "stdout", None), getattr(server_process, "stderr", None))
+                "Server failed to start; its output is in the captured stdout/stderr above.\n"
+                + _server_diagnostics(server_process, port, cmd)
             )
         except Exception:
             raise AssertionError(
-                "Server FAILED, see stdout and stderr logs"
-                "\n=== Captured STDOUT ===\n%s=== End of captured STDOUT ==="
-                "\n=== Captured STDERR ===\n%s=== End of captured STDERR ==="
-                % (getattr(server_process, "stdout", None), getattr(server_process, "stderr", None))
+                "Server FAILED; its output is in the captured stdout/stderr above.\n"
+                + _server_diagnostics(server_process, port, cmd)
             )
 
         # If we run a Gunicorn application, we want to get the child's pid, see test_flask_remoteconfig.py
@@ -399,9 +443,8 @@ def appsec_application_server(
             pass
         except Exception:
             raise AssertionError(
-                "\n=== Captured STDOUT ===\n%s=== End of captured STDOUT ==="
-                "\n=== Captured STDERR ===\n%s=== End of captured STDERR ==="
-                % (getattr(server_process, "stdout", None), getattr(server_process, "stderr", None))
+                "Server shutdown request failed; its output is in the captured stdout/stderr above.\n"
+                + _server_diagnostics(server_process, port, cmd)
             )
     finally:
         try:
@@ -433,7 +476,9 @@ def appsec_application_server(
                     assert "Return value is tainted" in stderr_output
                     assert "Tainted arguments:" in stderr_output
         finally:
-            pass
+            # Do not hand the port to the next test while a worker still holds it.
+            if not _wait_for_port_release(port):
+                print(f"WARNING: port {port} still bound after server teardown")
 
 
 def _mp_target(_cmd: list[str], _env: dict) -> None:

tests/appsec/suitespec.yml

@@ -74,6 +74,7 @@ suites:
     paths:
       - '@appsec_iast'
       - tests/appsec/iast_packages/*
+      - tests/appsec/appsec_utils.py
     timeout: 50m
   iast_tdd_propagation:
     venvs_per_job: 1
@@ -84,6 +85,7 @@ suites:
       - '@appsec_iast'
       - '@remoteconfig'
       - tests/appsec/iast_tdd_propagation/*
+      - tests/appsec/appsec_utils.py
     retry: 2
     snapshot: true
   appsec_integrations_pygoat:
@@ -148,6 +150,7 @@ suites:
       - '@appsec_iast'
       - tests/appsec/integrations/flask_tests/test_iast_flask.py
       - tests/appsec/integrations/flask_tests/test_appsec_flask_telemetry.py
+      - tests/appsec/appsec_utils.py
     retry: 2
     # test_appsec_flask_telemetry.py asserts on payloads received by the test agent.
     snapshot: true
@@ -162,6 +165,7 @@ suites:
       - '@appsec_iast'
       - '@remoteconfig'
       - tests/appsec/integrations/flask_tests/*
+      - tests/appsec/appsec_utils.py
     retry: 2
     services:
       - testagent
@@ -176,6 +180,7 @@ suites:
       - '@appsec_iast'
       - '@remoteconfig'
       - tests/appsec/integrations/django_tests/*
+      - tests/appsec/appsec_utils.py
     retry: 2
     services:
       - testagent
@@ -190,6 +195,7 @@ suites:
       - '@appsec_iast'
       - '@remoteconfig'
       - tests/appsec/integrations/fastapi_tests/*
+      - tests/appsec/appsec_utils.py
     retry: 2
     services:
       - testagent