Dev agent: release builds open no port; 32-byte token minimum; port cannot be taken over (#233)

- The runtime starts the agent only when the variable is set and an installed
  payload root holds python_multiplatform_devagent.enable. The library is one
  artifact for debug and release, so the app's build decides: toolchain stages
  the marker into debug variants only. Without it the variable is ignored with
  a warning.
- The token must be at least 43 characters (secrets.token_urlsafe(32)), or the
  agent refuses to start.
- Windows sets SO_EXCLUSIVEADDRUSE. POSIX keeps SO_REUSEADDR only to rebind the
  launcher's port past TIME_WAIT; it does not let another socket bind the port
  while the agent listens (tested).
- stop() deadlocked on the reader lock while a connection was blocked in a
  read; it now shuts the socket down and the serving thread closes itself.

Author: b-re-w

docs/SPEC.md

@@ -136,18 +136,26 @@ fail, then implement.
   (resolution order; the simulator test task's `PYTHONHOME` still wins). The installed-app run on the
   simulator is a manual check, `docs/platforms/ios-app-bundle.md`.
 - **L-12** The dev agent (#233; toolchain `docs/design/live-connection.md`, option B): when the launcher sets
-  `PYTHON_MULTIPLATFORM_DEV_AGENT` to `port=<n>;token=<t>[;overlay=<dir>]` (a debug launch; a release build
-  never does), `Python3.initialize()` loads the embedded agent (`src/commonMain/python/
-  python_multiplatform_devagent.py`, generated into Kotlin by `generateDevAgentSource`) as the module
-  `python_multiplatform_devagent` and it listens on 127.0.0.1 only. A connection's first message must be
-  `hello` with the token. Then, one JSON object per line: `execute` runs code in `__main__`'s persistent
-  namespace, streams stdout and stderr, returns the last expression's value and reports errors with their
-  traceback; `complete` completes from that namespace; `push` writes `.py` files into a writable overlay first
-  on `sys.path` and re-imports the loaded ones, children first, through `typedpython.importgate` when the app
-  has it (N-7b), keeping the old module when a re-import is refused or fails. A malformed request is printed
+  `PYTHON_MULTIPLATFORM_DEV_AGENT` to `port=<n>;token=<t>[;overlay=<dir>]` **and** an installed payload root
+  holds the marker `python_multiplatform_devagent.enable`, `Python3.initialize()` loads the embedded agent
+  (`src/commonMain/python/python_multiplatform_devagent.py`, generated into Kotlin by `generateDevAgentSource`)
+  as the module `python_multiplatform_devagent` and it listens on 127.0.0.1 only. The marker is the build-time
+  switch: this library is one artifact for debug and release, so the app's build decides, and toolchain stages
+  the marker into debug variants only. Without it the variable is ignored with a warning and no port opens. The
+  token must be at least 43 characters (`secrets.token_urlsafe(32)`), or the agent refuses to start. The listening
+  socket cannot be bound by another socket while it listens: `SO_EXCLUSIVEADDRUSE` on Windows; on POSIX,
+  `SO_REUSEADDR` only so that a restarted app can rebind the launcher's port past TIME_WAIT. A connection's first
+  message must be `hello` with the token. Then, one JSON object per line: `execute` runs code in `__main__`'s
+  persistent namespace, streams stdout and stderr, returns the last expression's value and reports errors with
+  their traceback; `complete` completes from that namespace; `push` writes `.py` files into a writable overlay
+  first on `sys.path` and re-imports the loaded ones, children first, through `typedpython.importgate` when the
+  app has it (N-7b), keeping the old module when a re-import is refused or fails. A malformed request is printed
   and does not stop the app. `Status: partial`: desktop
-  (`PM/desktopTest/.../env/PythonDevAgentTest.kt`: the embedded agent answers over loopback, and a child JVM started with the variable has an agent on that port while one started without it has none; the protocol, `python-multiplatform/src/test/python/
-  test_devagent.py`, CI `Dev agent protocol tests`); nothing sets the variable on Android or iOS yet
+  (`PM/desktopTest/.../env/PythonDevAgentTest.kt`: the embedded agent answers over loopback; a child JVM with the
+  marker and the variable has an agent on that port, one without the variable has none, one without the marker
+  (a release build) opens no port with the variable set, and a short token opens no port; the protocol, token
+  length, port takeover and restart: `python-multiplatform/src/test/python/test_devagent.py`, CI `Dev agent
+  protocol tests`, macOS only so far); nothing sets the variable or stages the marker on Android or iOS yet
   (toolchain's M2, M3); no interrupt; `execute` runs on the agent's thread (no main-thread dispatch yet).
 
 ## 2. Low-level C API (downcall surface)

python-multiplatform/src/commonMain/kotlin/python/multiplatform/env/PythonDevAgent.kt

@@ -11,14 +11,22 @@ import python.multiplatform.ffi.pyErrorOrGeneric
  * interpreter, designed in toolchain's `docs/design/live-connection.md` (option B).
  *
  * [Python3.initialize] calls [startIfRequested]. It does nothing unless the launcher set
- * [ENVIRONMENT_VARIABLE] (`port=<n>;token=<t>[;overlay=<dir>]`), which a debug launch does and a release
- * build never does; then the agent's Python ([DEV_AGENT_SOURCE], embedded from
- * `src/commonMain/python/python_multiplatform_devagent.py`) is loaded as the module [MODULE] and listens on
- * 127.0.0.1 with that token. A malformed request is reported on stderr and does not stop the app.
+ * [ENVIRONMENT_VARIABLE] (`port=<n>;token=<t>[;overlay=<dir>]`) **and** the app was built for it: one of
+ * its installed payload roots ([PythonPayload.installed]) holds [ENABLE_MARKER]. Then the agent's Python
+ * ([DEV_AGENT_SOURCE], embedded from `src/commonMain/python/python_multiplatform_devagent.py`) is loaded as
+ * the module [MODULE] and listens on 127.0.0.1 with that token, which must be at least 32 random bytes
+ * (`secrets.token_urlsafe(32)`). A refused or malformed request is reported on stderr and does not stop the app.
+ *
+ * The marker is the build-time switch. This library is one artifact for an app's debug and release builds
+ * alike, so a constant in it cannot tell them apart; the app's build can, and toolchain stages the marker
+ * into a debug variant's payload only. Without it, a release build opens no port even if something starts
+ * its process with the variable set: "a release launch never sets it" is the launcher's promise, which this
+ * runtime does not rely on.
  */
 internal object PythonDevAgent {
     const val ENVIRONMENT_VARIABLE = "PYTHON_MULTIPLATFORM_DEV_AGENT"
     const val MODULE = "python_multiplatform_devagent"
+    const val ENABLE_MARKER = "python_multiplatform_devagent.enable"
 
     private const val PY_FILE_INPUT: Int = 257
     private var loaded = false
@@ -27,6 +35,13 @@ internal object PythonDevAgent {
     fun startIfRequested() {
         if (requested || readEnvVar(ENVIRONMENT_VARIABLE).isNullOrBlank()) return
         requested = true
+        if (enabledRoot() == null) {
+            println(
+                "WARNING: python-multiplatform dev agent not started: $ENVIRONMENT_VARIABLE is set, but no " +
+                    "payload root holds $ENABLE_MARKER, so this is not a build made for it (a release build).",
+            )
+            return
+        }
         try {
             load()
             Python3.exec("__import__(\"$MODULE\").start_from_env()")
@@ -35,6 +50,10 @@ internal object PythonDevAgent {
         }
     }
 
+    /** The installed payload root that holds [ENABLE_MARKER], if any. */
+    private fun enabledRoot(): String? =
+        PythonPayload.installed.firstOrNull { pathIsAccessible("$it/$ENABLE_MARKER") }
+
     /** Loads [DEV_AGENT_SOURCE] as the module [MODULE] (once). */
     fun load() {
         if (loaded) return

python-multiplatform/src/commonMain/python/python_multiplatform_devagent.py

@@ -4,8 +4,10 @@
 
 The runtime starts it from `Python3.initialize()` only when the launcher sets the environment variable
 `PYTHON_MULTIPLATFORM_DEV_AGENT` to `port=<n>;token=<t>[;overlay=<dir>]` (a debug launch does; a release
-build never does). It listens on 127.0.0.1 only. A connection's first message must be `hello` with the
-token; anything else closes the connection.
+build never does), and only in a build that staged the enabling marker (the runtime checks that; see
+`PythonDevAgent.kt`). It listens on 127.0.0.1 only. The token must be at least `MIN_TOKEN_LENGTH` characters
+(`secrets.token_urlsafe(32)`), or the agent refuses to start. A connection's first message must be `hello`
+with the token; anything else closes the connection.
 
 Messages are JSON objects, one per line, each carrying the request's `id`:
 
@@ -39,6 +41,9 @@
 PROTOCOL = 1
 ENVIRONMENT_VARIABLE = "PYTHON_MULTIPLATFORM_DEV_AGENT"
 LOOPBACK = "127.0.0.1"
+# len(secrets.token_urlsafe(32)): 32 random bytes. The token is all that stands between any local process
+# and code execution in the app, so a short one is refused rather than used.
+MIN_TOKEN_LENGTH = 43
 
 
 def parse_request(value):
@@ -53,8 +58,9 @@ def parse_request(value):
         token = fields["token"]
     except (KeyError, ValueError):
         raise ValueError(f"{ENVIRONMENT_VARIABLE} must be port=<n>;token=<t>[;overlay=<dir>], not {value!r}")
-    if not token or not 0 <= port <= 65535:
+    if not 0 <= port <= 65535:
         raise ValueError(f"{ENVIRONMENT_VARIABLE} must be port=<n>;token=<t>[;overlay=<dir>], not {value!r}")
+    _check_token(token)
     return {"port": port, "token": token, "overlay": fields.get("overlay") or None}
 
 
@@ -69,8 +75,17 @@ def start_from_env():
     return agent
 
 
+def _check_token(token):
+    if not isinstance(token, str) or len(token) < MIN_TOKEN_LENGTH:
+        raise ValueError(
+            f"the dev agent's token must be at least {MIN_TOKEN_LENGTH} characters "
+            "(secrets.token_urlsafe(32)); refusing to start"
+        )
+
+
 def start(port, token, overlay=None):
     """Start an agent on loopback; `port=0` picks a free one (the agent's `port` says which)."""
+    _check_token(token)
     agent = Agent(token, overlay or tempfile.mkdtemp(prefix="python-multiplatform-overlay-"))
     agent.listen(port)
     return agent
@@ -123,7 +138,16 @@ def _install_gate(self):
 
     def listen(self, port):
         self.server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-        self.server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        if hasattr(socket, "SO_EXCLUSIVEADDRUSE"):
+            # Windows: SO_REUSEADDR there lets another socket bind the port while this one listens, and
+            # without SO_EXCLUSIVEADDRUSE a same-user process can do so even when this one did not set it.
+            self.server.setsockopt(socket.SOL_SOCKET, socket.SO_EXCLUSIVEADDRUSE, 1)
+        else:
+            # POSIX: only so that a restarted app can bind the launcher's port while the last run's
+            # connections sit in TIME_WAIT. It does not let another socket bind 127.0.0.1:<port> while this
+            # one listens (Linux refuses a second listener; BSD and macOS need SO_REUSEPORT on both).
+            # test_another_socket_cannot_take_the_port_over checks that on each platform the tests run on.
+            self.server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         self.server.bind((LOOPBACK, port))
         self.server.listen()
         self.port = self.server.getsockname()[1]
@@ -135,7 +159,7 @@ def stop(self):
         except OSError:
             pass
         for connection in list(self.connections):
-            connection.close()
+            connection.shutdown()
 
     def _accept(self):
         while True:
@@ -161,6 +185,17 @@ def send(self, message):
         with self.send_lock:
             self.sock.sendall(data)
 
+    def shutdown(self):
+        """Wake the serving thread, which then closes the connection itself.
+
+        Closing the reader from another thread would wait on the buffered reader's lock, which the serving
+        thread holds while it blocks in a read: stop() hung there.
+        """
+        try:
+            self.sock.shutdown(socket.SHUT_RDWR)
+        except OSError:
+            pass
+
     def close(self):
         for closable in (self.reader, self.sock):
             try:

python-multiplatform/src/desktopTest/kotlin/python/multiplatform/env/PythonDevAgentTest.kt

@@ -21,14 +21,17 @@ import kotlin.test.assertTrue
  * over a real loopback socket. The protocol itself is `src/test/python/test_devagent.py`'s.
  */
 class PythonDevAgentTest {
+    /** As long as the agent's minimum (`secrets.token_urlsafe(32)`). */
+    private val token = "a".repeat(22) + "b".repeat(21)
+
     @Test
     fun `the embedded agent runs in this interpreter and answers over loopback`() =
         PythonTestFixture.withInterpreter {
             PythonDevAgent.load()
             val overlay = Files.createTempDirectory("pmp-devagent").toFile()
             Python3.exec(
                 "__import__('sys').modules['__main__'].__dict__['_pmp_devagent_test'] = " +
-                    "__import__('${PythonDevAgent.MODULE}').start(0, 'kt-token', '${overlay.absolutePath}')",
+                    "__import__('${PythonDevAgent.MODULE}').start(0, '$token', '${overlay.absolutePath}')",
             )
             val port = PythonTestFixture.eval("_pmp_devagent_test.port").toString().toInt()
             try {
@@ -38,7 +41,7 @@ class PythonDevAgentTest {
                     val out = socket.getOutputStream()
                     fun send(json: String) = out.write("$json\n".toByteArray(Charsets.UTF_8)).also { out.flush() }
 
-                    send("""{"id": 1, "op": "hello", "token": "kt-token"}""")
+                    send("""{"id": 1, "op": "hello", "token": "$token"}""")
                     val hello = reader.readLine()
                     assertTrue("\"type\": \"hello\"" in hello && "\"protocol\": 1" in hello, hello)
 
@@ -56,20 +59,21 @@ class PythonDevAgentTest {
         }
 
     /**
-     * The hook in `Python3.initialize()`: a process started with [PythonDevAgent.ENVIRONMENT_VARIABLE] has an
-     * agent on that port with that token; one started without it has nothing listening. Each runs in its own
-     * JVM ([DevAgentProbe]), since an environment variable cannot be set in this one.
+     * The hook in `Python3.initialize()`: a debug build (its payload carries [PythonDevAgent.ENABLE_MARKER])
+     * started with [PythonDevAgent.ENVIRONMENT_VARIABLE] has an agent on that port with that token; one started
+     * without it has nothing listening. Each runs in its own JVM ([DevAgentProbe]), since an environment
+     * variable cannot be set in this one.
      */
     @Test
     fun `a process the launcher asked for an agent answers on the given port, and one it did not, does not`() {
-        val requested = launchProbe(request = true)
+        val requested = launchProbe(request = true, debugBuild = true)
         try {
             requested.awaitLine("python-multiplatform dev agent: listening on 127.0.0.1:${requested.port}")
             Socket("127.0.0.1", requested.port).use { socket ->
                 socket.soTimeout = 10_000
                 val reader = BufferedReader(InputStreamReader(socket.getInputStream(), Charsets.UTF_8))
                 val out = socket.getOutputStream()
-                out.write("{\"id\": 1, \"op\": \"hello\", \"token\": \"launch-token\"}\n".toByteArray())
+                out.write("{\"id\": 1, \"op\": \"hello\", \"token\": \"$token\"}\n".toByteArray())
                 out.write("{\"id\": 2, \"op\": \"execute\", \"code\": \"6 * 7\"}\n".toByteArray())
                 out.flush()
                 assertTrue("\"type\": \"hello\"" in reader.readLine())
@@ -79,7 +83,7 @@ class PythonDevAgentTest {
             requested.process.destroyForcibly()
         }
 
-        val notRequested = launchProbe(request = false)
+        val notRequested = launchProbe(request = false, debugBuild = true)
         try {
             notRequested.awaitLine(DevAgentProbe.READY)
             assertFailsWith<ConnectException> { Socket("127.0.0.1", notRequested.port).close() }
@@ -88,6 +92,37 @@ class PythonDevAgentTest {
         }
     }
 
+    /**
+     * A release build (its payload has no [PythonDevAgent.ENABLE_MARKER]) opens no port even when its process
+     * is started with the variable: "a release launch never sets it" is the launcher's promise, not this
+     * runtime's guarantee, so the runtime does not rely on it.
+     */
+    @Test
+    fun `a release build opens no port even when the variable is set`() {
+        val release = launchProbe(request = true, debugBuild = false)
+        try {
+            release.awaitLine(DevAgentProbe.READY)
+            assertFailsWith<ConnectException> { Socket("127.0.0.1", release.port).close() }
+            assertTrue(release.log.readLines().none { "listening" in it }, release.log.readText())
+            assertTrue(release.log.readText().contains(PythonDevAgent.ENABLE_MARKER), release.log.readText())
+        } finally {
+            release.process.destroyForcibly()
+        }
+    }
+
+    /** A token shorter than the agent's minimum is refused before anything listens. */
+    @Test
+    fun `a debug build refuses a short token and opens no port`() {
+        val short = launchProbe(request = true, debugBuild = true, token = "short-token")
+        try {
+            short.awaitLine(DevAgentProbe.READY)
+            assertFailsWith<ConnectException> { Socket("127.0.0.1", short.port).close() }
+            assertTrue(short.log.readText().contains("refusing to start"), short.log.readText())
+        } finally {
+            short.process.destroyForcibly()
+        }
+    }
+
     private class Probe(val process: Process, val port: Int, val log: File) {
         fun awaitLine(line: String) {
             val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(120)
@@ -100,21 +135,28 @@ class PythonDevAgentTest {
         }
     }
 
-    private fun launchProbe(request: Boolean): Probe {
+    /**
+     * [debugBuild] stages a payload as toolchain does for a debug variant: a `python/` root on the class path
+     * with a module and the marker. A release variant's payload is the same without the marker.
+     */
+    private fun launchProbe(request: Boolean, debugBuild: Boolean, token: String = this.token): Probe {
         val port = ServerSocket(0).use { it.localPort }
         val workDir = Files.createTempDirectory("pmp-devagent-probe").toFile()
         val log = File(workDir, "probe.log")
+        val payload = File(workDir, "classes/${PythonPayload.PAYLOAD_ROOT}").apply { mkdirs() }
+        File(payload, "devagent_probe_app.py").writeText("VALUE = 1\n")
+        if (debugBuild) File(payload, PythonDevAgent.ENABLE_MARKER).writeText("")
         val command = listOf(
             File(System.getProperty("java.home"), "bin/java").path,
             "--enable-preview",
             "--enable-native-access=ALL-UNNAMED",
             "-cp",
-            System.getProperty("java.class.path"),
+            File(workDir, "classes").path + File.pathSeparator + System.getProperty("java.class.path"),
             DevAgentProbe::class.java.name,
         )
         val builder = ProcessBuilder(command).directory(workDir).redirectErrorStream(true).redirectOutput(log)
         builder.environment().remove(PythonDevAgent.ENVIRONMENT_VARIABLE)
-        if (request) builder.environment()[PythonDevAgent.ENVIRONMENT_VARIABLE] = "port=$port;token=launch-token"
+        if (request) builder.environment()[PythonDevAgent.ENVIRONMENT_VARIABLE] = "port=$port;token=$token"
         return Probe(builder.start(), port, log)
     }