fix(harness): bound requests by one read deadline and the head, not by call slots

Delta security review of #20 at b2cceaa:

- D2: a call takes its concurrency slot only once its whole request has
  arrived, and reading one request (head and body) has one wall-clock
  deadline instead of a timeout per read, so trickling peers no longer hold
  every slot. Requests being read are bounded by memory (256 MiB of heads
  and bodies), refused 503 before any of them is read; at the connection
  bound the connection idle the longest is closed instead of the newcomer.
- D5: the request line and headers are bounded at 32 KiB together (431).
- D1: a header line the parser could not read is refused, rather than
  ending the parsed headers early.
- D3: a handler keeps its slot until it returns, past a 504, and the drain
  at a signal waits for it.
- D4: the harness's own concurrency default is named as such, and the docs
  say how it relates to the runtime's installation concurrency.
- D6: idle connections are kept 120 s, longer than Go's 90 s pool, and
  advertised with Keep-Alive.
- D7/D8: the smuggling test covers 404, GET with a body and 503; tests wait
  on conditions the harness makes observable instead of sleeping.
- Nits: Expect: 100-continue gets the refusal instead of 100 Continue;
  HandlerFailure.code is read-only; the core-bounds test compares numbers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: antoinetoussaint-byte

docs/protocol.md

@@ -48,19 +48,58 @@ owner's body would be a field the owner never described.
 
 The body is read only by a `Content-Length` written in ASCII digits, at most
 `max-input-bytes`. A missing length is refused `411`; a `Transfer-Encoding`, a
-repeated length, a sign or any other spelling is refused `400`, unread; a body
-that ends short of its length is refused `400`. **Every answer but a `200`
+repeated length, a sign or any other spelling is refused `400`, unread; so is a
+request carrying a header line the parser could not read, which would otherwise
+end the parsed headers early and hide every header after it. A body that ends
+short of its length is refused `400`. A request asking `Expect: 100-continue` is
+told the refusal instead of being invited to upload. **Every answer but a `200`
 closes the connection** (`Connection: close`), so bytes a refused call did not
 consume are never read as the next call — under whatever Work Context those
 bytes name.
 
-The harness bounds what a peer can hold. A read or write that stalls for
-`IO_TIMEOUT_SECONDS` (10 s) ends the connection, answering `408` when it was a
-body that stalled; the same timeout releases an idle kept-alive connection.
-Simultaneous calls are bounded by the declaration's `concurrency`
-(`DEFAULT_CONCURRENCY`, 32, when it declares 0, the facility default); a call
-over the bound is refused `503` before the handler runs. Open connections are
-bounded at twice that, and one over it is closed unread.
+### What a peer can hold
+
+Every bound below is in `codefly_runnable/protocol.py`, and each is the harness's
+own: none is a value a placement resolved.
+
+| Bound | Value | At the bound |
+| --- | --- | --- |
+| reading one request, head and body together (`REQUEST_READ_SECONDS`) | 10 s wall clock, from its first byte | `408` once the body has begun; a head that has not arrived is dropped unanswered |
+| the request line and headers (`MAX_HEAD_BYTES`) | 32 KiB | `431` |
+| requests being read at once (`READ_MEMORY_BYTES` ÷ (32 KiB + `max-input-bytes`), at most 1,024) | 256 MiB of heads and bodies: 248 readers at the default 1 MiB input | `503`, before any of the request is read |
+| calls whose handler is running (the declared `concurrency`) | `HARNESS_CONCURRENCY`, 32, when the declaration states 0 | `503` |
+| open connections (readers + concurrency + `IDLE_CONNECTIONS`, 64) | — | the connection idle the longest is closed to make room; a new one is never refused |
+| an idle kept-alive connection (`IDLE_SECONDS`) | 120 s, advertised as `Keep-Alive: timeout=120` | closed |
+| one write of an answer (`WRITE_STALL_SECONDS`) | 10 s | the connection is dropped |
+
+The read deadline is one instant per request, not a bound on each read: a peer
+sending a byte every few seconds is cut when the request's time is up, however
+regularly it sends. A call takes its concurrency slot only once its whole
+request has arrived, so a request still arriving holds no slot, and a slow peer
+costs a reader — bounded by memory, released at the deadline — rather than a
+call. A slot is held until the handler really returns, not until the call is
+answered: a call answered `504` at its deadline keeps its slot while its handler
+runs on, so the declared concurrency bounds author code running.
+
+**Concurrency is not yet one number.** Core's declaration says `0` means the
+facility default and defines none, so the harness admits 32 when the declaration
+is silent. The durable-work runtime decides how many calls it dispatches from
+its own installation material (1 to 200), not from the declaration. When the
+runtime dispatches more than the harness admits, each excess call answers `503`:
+an unproven outcome the caller recovers by its recovery policy. Until the
+placement passes the number it resolved, declare `concurrency` and install with
+the same number. A declaration of `0` is not refused: core's own scaffold
+declares no concurrency, so refusing it would refuse every scaffolded runnable.
+
+The idle timeout is longer than the idle timeout of the pool the durable-work
+runtime calls through (Go's `net/http`, 90 s by default), so the caller retires
+an idle connection before the harness closes it. The other order makes a call
+written onto a connection the harness had just closed come back as a reset — an
+unproven outcome for a call that never ran. A proxy pooling connections to the
+harness needs an idle timeout under 120 s for the same reason; Envoy's default
+of one hour is not. Under the connection bound, the harness closing an idle
+connection to make room has the same cost, which is why that bound sits beside
+every reader and every running call rather than in place of them.
 
 | Route | Answers |
 | --- | --- |
@@ -117,8 +156,8 @@ as proof would stop looking for a receipt that exists.
 | raised anything else | `500`, no failure code | unproven |
 | was still running at the deadline | `504`, no failure code | unproven |
 | returned an answer the contract refuses | `500`, no failure code | unproven |
-| was never reached (payload, identity, effect id, framing) | `400`/`408`/`411`, no failure code | unproven |
-| was never reached (over the concurrency, or stopping) | `503`, no failure code | unproven |
+| was never reached (payload, identity, effect id, framing) | `400`/`408`/`411`/`431`, no failure code | unproven |
+| was never reached (over a bound, or stopping) | `503`, no failure code | unproven |
 
 The statuses are for humans and proxies: core classifies on the failure-code
 header and the answer document, never on a status class.
@@ -136,17 +175,19 @@ representable instant, is refused before the handler runs.
 
 The handler runs on its own thread, because Python cannot raise into one. When
 the budget passes the harness answers and the handler is still running; what it
-would have returned is no longer that call's outcome.
+would have returned is no longer that call's outcome. It keeps its concurrency
+slot until it returns.
 
 ## Signals and logs
 
 A signal stops the harness taking new calls — on new connections and on ones
 already open, which answer `503` with `Connection: close` — and gives the calls
-in flight `SHUTDOWN_GRACE_SECONDS` (30 s) to finish. Cutting them off would turn
-every one of them into an unproven effect at once. The harness exits `0` as
-soon as the last call in flight has answered; an idle connection holds nothing.
-If the grace ends with calls still running, they are cut off unanswered and the
-harness exits `1`, saying how many effects it left unproven.
+in flight `SHUTDOWN_GRACE_SECONDS` (30 s) to finish — including a handler whose
+call was already answered at its deadline. Cutting them off would turn every one
+of them into an unproven effect at once. The harness exits `0` as soon as the
+last handler has returned; an idle connection holds nothing. If the grace ends
+with handlers still running, they are cut off and the harness exits `1`, saying
+how many effects it left unproven.
 
 `stdout` and `stderr` carry diagnostics only, never completion data: a handler
 that printed its output would be indistinguishable from a library that printed a

pkg/contract/contract.go

@@ -40,8 +40,9 @@ type Generated struct {
 	// a call carrying no deadline header is bounded by the author's own
 	// statement of how long the operation may run.
 	TimeoutNanoseconds int64 `json:"timeout-nanoseconds"`
-	// Concurrency is the declared bound on simultaneous calls, which the
-	// harness enforces before a handler runs; 0 is the facility default.
+	// Concurrency is the declared bound on handlers running at once, which the
+	// harness enforces before a handler runs. Core leaves 0 to the facility;
+	// the harness then applies its own bound (see docs/protocol.md).
 	Concurrency uint32 `json:"concurrency"`
 }
 

pkg/contract/contract_test.go

@@ -3,6 +3,8 @@ package contract_test
 import (
 	"fmt"
 	"io/fs"
+	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -77,8 +79,12 @@ func TestTheHeaderBoundsAreCoresBounds(t *testing.T) {
 		if limit == 0 {
 			t.Fatalf("core declares no max_len on %s", field.FullName())
 		}
-		if declaration := fmt.Sprintf("%s = %d", bound.literal, limit); !strings.Contains(protocol, declaration) {
-			t.Errorf("the harness does not declare %s, core's bound on %s", declaration, field.FullName())
+		declared := regexp.MustCompile(`(?m)^`+bound.literal+` = ([0-9]+)$`).FindAllStringSubmatch(protocol, -1)
+		if len(declared) != 1 {
+			t.Fatalf("the harness declares %s %d times, not once", bound.literal, len(declared))
+		}
+		if value, err := strconv.ParseUint(declared[0][1], 10, 64); err != nil || value != limit {
+			t.Errorf("the harness declares %s = %s; core bounds %s at %d", bound.literal, declared[0][1], field.FullName(), limit)
 		}
 	}
 }

pkg/harness/codefly_runnable/protocol.py

@@ -45,14 +45,40 @@
 # caller as no assertion at all while the handler believed it had asserted.
 FAILURE_CODE_PATTERN = re.compile(r"[a-zA-Z0-9][a-zA-Z0-9_.-]{0,63}")
 
-# How long one read or write on a call's connection may stall, and how long an
-# idle kept-alive connection is held. Without it a peer that stops sending holds
-# a thread for as long as it likes.
-IO_TIMEOUT_SECONDS = 10.0
-
-# Simultaneous calls when the declaration's concurrency is 0, the facility
-# default. A call over the bound is refused before the handler runs.
-DEFAULT_CONCURRENCY = 32
+# How long reading one request may take, its head and its body together. It is
+# one wall-clock deadline rather than a bound on each read, which a peer sending
+# a byte just inside it would hold open forever.
+REQUEST_READ_SECONDS = 10.0
+
+# How long one write of an answer may stall before the connection is dropped.
+WRITE_STALL_SECONDS = 10.0
+
+# How long a kept-alive connection may sit idle between calls. It is longer than
+# the idle timeout of the pool the durable-work runtime calls through (Go's
+# net/http, 90 s by default), so the caller retires an idle connection before
+# the harness closes it: a call written onto a connection the harness had just
+# closed would come back as a reset, an uncertain outcome for a call that never
+# ran. Answers advertise it in a Keep-Alive header for clients that read one.
+IDLE_SECONDS = 120.0
+
+# The request line and every header together. The per-call facts need under
+# 9 KiB at core's bounds; the stdlib alone would admit 100 lines of 64 KiB.
+MAX_HEAD_BYTES = 32 * 1024
+
+# Memory for requests being read, head and body. It sizes how many requests are
+# read at once: a request over that count is refused before anything of it is
+# read, and a call holds no slot while its request is still arriving.
+READ_MEMORY_BYTES = 256 * 1024 * 1024
+MAX_READERS = 1024
+
+# Idle connections kept beside every request being read and every call
+# running. Past them, a new connection closes the one idle the longest.
+IDLE_CONNECTIONS = 64
+
+# The harness's own bound on simultaneous calls when the declaration states
+# none (concurrency 0, which core leaves to the facility). It is this harness's
+# number, not one any placement resolved; see docs/protocol.md.
+HARNESS_CONCURRENCY = 32
 
 # How long a call in flight has to finish after a signal. A handler interrupted
 # mid-effect leaves it unproven, so the process waits rather than making every
@@ -79,9 +105,14 @@ def __init__(self, code: str, message: str) -> None:
             raise ValueError(f"failure code must match {FAILURE_CODE_PATTERN.pattern}")
         if not isinstance(message, str):
             raise ValueError("failure message must be a string")
-        self.code = code
+        self._code = code
         super().__init__(message)
 
+    @property
+    def code(self) -> str:
+        """The validated code. Read-only: what was checked is what is sent."""
+        return self._code
+
 
 @dataclass(frozen=True)
 class RunnableIdentity: