fix: clone/serialize double-free crash across every FFI handle class, plus a leak in the fix

Every handle-owning class (KeyPair, Session, StreamHandle, PendingCall,
Event, CallResponse, StreamOpenInfo, StreamReply, UcanPayload,
StreamItem) held a private raw cgo.Handle int with no __clone() guard.
PHP's clone shallow-copies private properties before running
__clone(), so `clone $keyPair` produces a second object holding the
identical handle; whichever of the two destructs second calls the Go
free function on an already-deleted runtime/cgo.Handle, which panics --
and an unrecovered panic inside any cgo-exported function is fatal to
the ENTIRE host PHP process, not a catchable exception. Reproduced
before fixing (KeyPair, no network needed):

    $copy = clone $original; unset($original); unset($copy);
    // panic: runtime/cgo: misuse of an invalid Handle -- SIGABRT, exit 134

A naive `__clone() { throw ...; }` with no other change still crashed:
PHP has already completed the shallow property copy before __clone()
runs, so the doomed clone (never assigned to a variable, since the
throw aborts the assignment) still holds a live handle copy, and ITS
OWN __destruct() fires normally when that temporary is discarded,
stealing the original's handle. Fixed by nulling the clone's own
handle copy before throwing, on all 10 classes.

Fable's review of that fix found the identical bug has a second door:
serialize()/unserialize() copies $handle by value too, and reaches
every consumer's private cache/queue/$_SESSION with no clone() call
anywhere in sight. Fixed with __serialize()/__unserialize() guards on
the same 10 classes, throwing LogicException (aligned with __clone(),
which was inconsistently RuntimeException -- nothing here is in
production yet, so the one-word cleanup across all 10 files is free).

As defense in depth (whatever the PHP-side guards don't catch),
cabi/main.go gained a safeDeleteHandle helper wrapping every _free
export's Delete() in recover() -- same principle as this morning's
macula-go pool.go fix (a double-free must cost nothing, never the
whole process). That recover() itself introduced a real leak Fable
caught: macula_session_close's single top-level recover() meant a
panic from validating identityHandle skipped the trailing
Delete(sessionHandle) entirely, leaking the session (and its live QUIC
connection) forever, silently, on exactly the path the recover() was
added for. Fixed by deferring safeDeleteHandle FIRST (runs LAST per
Go's LIFO defer order, so it fires regardless of where a panic
happened above it), with early returns replacing the nested ifs.
Verified with a standalone Go probe reproducing the exact control-flow
shape of both the old (leaks) and new (doesn't leak) code across every
handle-validity combination.

Added regression coverage in tests/KeyPairTest.php (clone throws and
leaves the original usable; serialize()/unserialize() of a live
instance and of a hand-crafted blob both throw). Full suite (24 tests)
green, composer validate clean, gofmt/vet/build clean, and every live
primitive re-verified against station-de-frankfurt.macula.io (session
close is exercised by all of them): handshake, call, pubsub, content,
UCAN, RPC provider, stream provider.

Fable-reviewed in two rounds (initial finding + a follow-up pass
verifying both fixes' completeness); second round returned zero
required items.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Khkcfi5pTAy2uA2ErjL57Q

Author: rgfaber

cabi/content.go

@@ -88,5 +88,5 @@ func macula_bytes_handle_read(bytesHandle C.uintptr_t, out *C.uchar) {
 
 //export macula_bytes_handle_free
 func macula_bytes_handle_free(bytesHandle C.uintptr_t) {
-	cgo.Handle(bytesHandle).Delete()
+	safeDeleteHandle(cgo.Handle(bytesHandle))
 }

cabi/main.go

@@ -110,6 +110,28 @@ func macula_free_string(s *C.char) {
 	C.free(unsafe.Pointer(s))
 }
 
+// safeDeleteHandle deletes h, recovering a panic instead of letting it
+// propagate. cgo.Handle.Delete (like .Value) panics on an
+// already-deleted or otherwise invalid handle, and an unrecovered
+// panic inside any cgo-exported function is fatal to the WHOLE host
+// process -- Go's panic unwinding doesn't stop at the C caller, it
+// terminates the process outright, taking down every other in-flight
+// request this same PHP process happens to be serving, not just the
+// one holding the bad handle.
+//
+// The PHP side is the primary guard (every wrapper class nulls its own
+// handle after freeing and rejects further use via handleOrFail(),
+// and __clone() on every one of them explicitly nulls the clone's copy
+// before throwing -- see e.g. KeyPair::__clone()). This is defense in
+// depth for whatever that tracking doesn't catch: a double-free must
+// cost nothing, never the whole process, same principle as
+// macula-go's own pool.deliverOne recovering a subscriber handler's
+// panic so one bad callback doesn't kill every other subscriber.
+func safeDeleteHandle(h cgo.Handle) {
+	defer func() { recover() }()
+	h.Delete()
+}
+
 //export macula_identity_generate
 func macula_identity_generate(errOut **C.char) C.uintptr_t {
 	id, err := identity.Generate()
@@ -156,7 +178,7 @@ func macula_identity_private_bytes(identityHandle C.uintptr_t, out32 *C.uchar) C
 
 //export macula_identity_free
 func macula_identity_free(identityHandle C.uintptr_t) {
-	cgo.Handle(identityHandle).Delete()
+	safeDeleteHandle(cgo.Handle(identityHandle))
 }
 
 //export macula_connect
@@ -272,14 +294,32 @@ func macula_session_station_node_id(sessionHandle C.uintptr_t, out32 *C.uchar) C
 
 //export macula_session_close
 func macula_session_close(sessionHandle C.uintptr_t, identityHandle C.uintptr_t) {
+	// safeDeleteHandle is deferred FIRST (so it runs LAST -- defers are
+	// LIFO) and unconditionally, before sessionHandle has even been
+	// validated: if the recover() below fires partway through (e.g. an
+	// invalid identityHandle), execution would otherwise return without
+	// ever reaching a trailing Delete() call, leaking the session
+	// handle (and its still-open QUIC connection) forever. Ordering
+	// this defer outermost means the handle gets deleted regardless of
+	// where -- or whether -- a panic happened above it; safeDeleteHandle
+	// has its own recover() too, so this is safe even if sessionHandle
+	// itself turns out to be invalid.
+	defer safeDeleteHandle(cgo.Handle(sessionHandle))
+	// Recovers a panic from EITHER .Value() call below (an invalid
+	// sessionHandle or identityHandle) -- see safeDeleteHandle's doc for
+	// why an unrecovered panic here is fatal to the whole process, not
+	// just this call.
+	defer func() { recover() }()
+
 	session, ok := cgo.Handle(sessionHandle).Value().(*connection.Session)
-	if ok {
-		id, idOk := cgo.Handle(identityHandle).Value().(identity.KeyPair)
-		if idOk {
-			_ = session.Close("normal", nil, id)
-		}
+	if !ok {
+		return
+	}
+	id, idOk := cgo.Handle(identityHandle).Value().(identity.KeyPair)
+	if !idOk {
+		return
 	}
-	cgo.Handle(sessionHandle).Delete()
+	_ = session.Close("normal", nil, id)
 }
 
 func main() {} // required by -buildmode=c-shared, never actually run

cabi/pubsub.go

@@ -183,5 +183,5 @@ func macula_event_payload_bytes(eventHandle C.uintptr_t, out *C.uchar) {
 
 //export macula_event_free
 func macula_event_free(eventHandle C.uintptr_t) {
-	cgo.Handle(eventHandle).Delete()
+	safeDeleteHandle(cgo.Handle(eventHandle))
 }

cabi/rpc.go

@@ -130,5 +130,5 @@ func macula_response_error_detail(responseHandle C.uintptr_t) *C.char {
 
 //export macula_response_free
 func macula_response_free(responseHandle C.uintptr_t) {
-	cgo.Handle(responseHandle).Delete()
+	safeDeleteHandle(cgo.Handle(responseHandle))
 }

cabi/serve.go

@@ -181,5 +181,5 @@ func macula_pending_call_reply_error(pendingHandle C.uintptr_t, detail *C.char,
 
 //export macula_pending_call_free
 func macula_pending_call_free(pendingHandle C.uintptr_t) {
-	cgo.Handle(pendingHandle).Delete()
+	safeDeleteHandle(cgo.Handle(pendingHandle))
 }

cabi/stream.go

@@ -140,7 +140,7 @@ func macula_stream_open_info_caller(infoHandle C.uintptr_t, out32 *C.uchar) {
 
 //export macula_stream_open_info_free
 func macula_stream_open_info_free(infoHandle C.uintptr_t) {
-	cgo.Handle(infoHandle).Delete()
+	safeDeleteHandle(cgo.Handle(infoHandle))
 }
 
 //export macula_stream_send_data
@@ -280,7 +280,7 @@ func macula_stream_item_body_bytes(itemHandle C.uintptr_t, out *C.uchar) {
 
 //export macula_stream_item_free
 func macula_stream_item_free(itemHandle C.uintptr_t) {
-	cgo.Handle(itemHandle).Delete()
+	safeDeleteHandle(cgo.Handle(itemHandle))
 }
 
 //export macula_stream_await_reply
@@ -341,7 +341,7 @@ func macula_stream_reply_responded_by(replyHandle C.uintptr_t, out32 *C.uchar) {
 
 //export macula_stream_reply_free
 func macula_stream_reply_free(replyHandle C.uintptr_t) {
-	cgo.Handle(replyHandle).Delete()
+	safeDeleteHandle(cgo.Handle(replyHandle))
 }
 
 //export macula_stream_abort
@@ -359,5 +359,5 @@ func macula_stream_abort(streamHandle C.uintptr_t, code *C.char, message *C.char
 
 //export macula_stream_free
 func macula_stream_free(streamHandle C.uintptr_t) {
-	cgo.Handle(streamHandle).Delete()
+	safeDeleteHandle(cgo.Handle(streamHandle))
 }

cabi/ucan.go

@@ -169,7 +169,7 @@ func macula_ucan_payload_proofs_json(payloadHandle C.uintptr_t) *C.char {
 
 //export macula_ucan_payload_free
 func macula_ucan_payload_free(payloadHandle C.uintptr_t) {
-	cgo.Handle(payloadHandle).Delete()
+	safeDeleteHandle(cgo.Handle(payloadHandle))
 }
 
 // The minted token is plain []byte -- read it back with the existing

src/CallResponse.php

@@ -19,6 +19,42 @@ public function __construct(int $handle)
         $this->handle = $handle;
     }
 
+    /**
+     * PHP shallow-copies $handle into the clone before this method
+     * runs, so nulling it here (not just throwing) is required: without
+     * it, the clone's own __destruct() would free the ORIGINAL's handle
+     * the moment this throw unwinds and the never-assigned clone is
+     * discarded, and the eventual double free() on an already-deleted
+     * cgo.Handle panics on the Go side, which is fatal to the whole
+     * process if unrecovered. This PHP-side guard remains the primary
+     * defense -- it fails fast with a catchable exception instead of
+     * relying on cabi/main.go's safeDeleteHandle recover() at all.
+     */
+    public function __clone(): never
+    {
+        $this->handle = null;
+        throw new \LogicException('CallResponse cannot be cloned -- each instance owns a unique FFI handle');
+    }
+
+    /**
+     * serialize()/unserialize() is a second door to the same bug
+     * clone() guards against -- it copies $handle by value, and
+     * unserialize() would hand back a second live object holding that
+     * same raw handle, reachable via ordinary PHP ($_SESSION, an
+     * object cache, a queue payload), no reflection needed. The handle
+     * isn't meaningful across requests/processes anyway.
+     */
+    public function __serialize(): never
+    {
+        throw new \LogicException('CallResponse cannot be serialized -- each instance owns a unique FFI handle');
+    }
+
+    /** @param array<mixed> $data */
+    public function __unserialize(array $data): never
+    {
+        throw new \LogicException('CallResponse cannot be unserialized -- each instance owns a unique FFI handle');
+    }
+
     public function isError(): bool
     {
         return Binding::get()->macula_response_is_error($this->handleOrFail()) !== 0;

src/Event.php

@@ -18,6 +18,42 @@ public function __construct(int $handle)
         $this->handle = $handle;
     }
 
+    /**
+     * PHP shallow-copies $handle into the clone before this method
+     * runs, so nulling it here (not just throwing) is required: without
+     * it, the clone's own __destruct() would free the ORIGINAL's handle
+     * the moment this throw unwinds and the never-assigned clone is
+     * discarded, and the eventual double free() on an already-deleted
+     * cgo.Handle panics on the Go side, which is fatal to the whole
+     * process if unrecovered. This PHP-side guard remains the primary
+     * defense -- it fails fast with a catchable exception instead of
+     * relying on cabi/main.go's safeDeleteHandle recover() at all.
+     */
+    public function __clone(): never
+    {
+        $this->handle = null;
+        throw new \LogicException('Event cannot be cloned -- each instance owns a unique FFI handle');
+    }
+
+    /**
+     * serialize()/unserialize() is a second door to the same bug
+     * clone() guards against -- it copies $handle by value, and
+     * unserialize() would hand back a second live object holding that
+     * same raw handle, reachable via ordinary PHP ($_SESSION, an
+     * object cache, a queue payload), no reflection needed. The handle
+     * isn't meaningful across requests/processes anyway.
+     */
+    public function __serialize(): never
+    {
+        throw new \LogicException('Event cannot be serialized -- each instance owns a unique FFI handle');
+    }
+
+    /** @param array<mixed> $data */
+    public function __unserialize(array $data): never
+    {
+        throw new \LogicException('Event cannot be unserialized -- each instance owns a unique FFI handle');
+    }
+
     public function topic(): string
     {
         $ffi = Binding::get();

src/KeyPair.php

@@ -19,6 +19,49 @@ private function __construct(int $handle)
         $this->handle = $handle;
     }
 
+    /**
+     * PHP shallow-copies $handle into the clone before running this
+     * method, so the clone already holds a live copy of the same raw
+     * handle at this point -- if left alone, its own __destruct() would
+     * free the ORIGINAL's handle out from under it the moment this
+     * throw unwinds and the clone (never assigned to a variable) is
+     * discarded. Null it first so that free() is a no-op on the clone,
+     * THEN throw -- the exception must not depend on which line runs
+     * first, since either order without both steps still crashes on a
+     * build without the cabi-side recover() (safeDeleteHandle in
+     * cabi/main.go): a double free() on an already-deleted cgo.Handle
+     * panics on the Go side, which is fatal to the whole process if
+     * unrecovered. This PHP-side guard remains the primary defense --
+     * it fails fast with a catchable exception instead of relying on
+     * that recover() at all.
+     */
+    public function __clone(): never
+    {
+        $this->handle = null;
+        throw new \LogicException('KeyPair cannot be cloned -- each instance owns a unique FFI handle');
+    }
+
+    /**
+     * serialize()/unserialize() is a second door to the exact same bug
+     * clone() has: it copies $handle by value into the serialized
+     * representation, and unserialize() would hand back a second live
+     * object holding that same raw handle -- reachable via ordinary PHP
+     * ($_SESSION, an object cache, a queue payload), no reflection
+     * needed. Block both directions; the handle isn't meaningful across
+     * requests/processes anyway. Persist the identity by value instead:
+     * fromSeedBytes(privateBytes()) reconstructs an equivalent KeyPair.
+     */
+    public function __serialize(): never
+    {
+        throw new \LogicException('KeyPair cannot be serialized -- persist privateBytes() and rebuild with fromSeedBytes()');
+    }
+
+    /** @param array<mixed> $data */
+    public function __unserialize(array $data): never
+    {
+        throw new \LogicException('KeyPair cannot be unserialized -- persist privateBytes() and rebuild with fromSeedBytes()');
+    }
+
     public static function generate(): self
     {
         $ffi = Binding::get();