fix(ffi): harden AdNativeHandle against double-free + null reuse (todo 009)

Closes P1 todo 009 (Option 2: minimal hardening; Option 1 opaque-ID
table deferred as an ABI-breaking follow-up).

Three concrete changes:

1. ad_free_handle signature: *const AdNativeHandle → *mut AdNativeHandle.
   The free fn now owns the right to mutate the caller's struct.

2. Zero handle.ptr BEFORE invoking the platform release() call. A
   follow-up ad_free_handle on the same struct observes ptr == null
   and returns Ok without re-entering CFRelease, making an accidental
   double-call deterministic instead of corrupting the CF retain count.

3. ad_execute_action and ad_get now reject handle.ptr == NULL at the
   use site (after guarding the struct pointer itself) with a
   diagnostic last-error "handle.ptr is null — the handle has already
   been freed or was never resolved". Prevents feeding a freshly-zeroed
   handle back into adapter code.

Ownership contract captured in the ad_free_handle rustdoc: the FFI
owns the handle from ad_resolve_element onward; copying the struct
and calling ad_free_handle on either copy is undefined because the
library cannot detect forged non-null pointers. Callers that need a
"second copy" must re-resolve.

Tests (crates/ffi/tests/c_abi_harness.rs):
- free_handle_null_is_noop: adjusted to *mut signature, asserts the
  ptr stays null after the call.
- free_handle_zeroes_ptr_so_double_free_is_noop: non-macOS only (macOS
  would SIGBUS on the fake pointer's CFRelease before we could observe
  the zeroing; the logic is platform-agnostic so validating it on
  Windows/Linux covers the contract).
- execute_action_rejects_null_handle_ptr: null .ptr with non-null
  struct returns InvalidArgs or ErrInternal (main-thread guard), no UB.

81 FFI tests pass (72 lib + error_lifetime + 14 c_abi_harness variants
depending on target). Clippy clean.

crates/ffi/include/agent_desktop.h

@@ -293,22 +293,32 @@ AdResult ad_execute_action(const struct AdAdapter *adapter,
                            struct AdActionResult *out);
 
 /**
- * Releases a handle previously returned by `ad_resolve_element`.
+ * Releases a handle previously returned by `ad_resolve_element` and
+ * zeroes the caller's struct so accidentally calling this twice is
+ * a deterministic no-op instead of a double-free on the underlying
+ * `CFRelease`.
  *
  * On macOS this calls `CFRelease` on the underlying `AXUIElementRef`,
  * balancing the `CFRetain` that happened during `ad_resolve_element`.
  * On Windows/Linux the call is a no-op that returns `AD_RESULT_OK`
- * (platform adapters inherit the default `not_supported` impl, which
- * the FFI surface rewrites to `Ok` here so callers can apply the same
- * release pattern everywhere).
+ * (platform adapters inherit the default `not_supported` impl; the
+ * FFI surface translates it so callers apply the same release
+ * pattern everywhere).
+ *
+ * Ownership contract: the FFI owns the handle from the moment
+ * `ad_resolve_element` writes `ptr`. Copying the struct after that
+ * point and calling `ad_free_handle` on either copy is undefined —
+ * there is no way for the library to detect forged non-null pointers.
+ * Callers that legitimately need a "copy" should re-resolve.
  *
  * # Safety
  *
  * `adapter` must be a non-null pointer returned by `ad_adapter_create`.
- * `handle` must be null or a pointer previously populated by
- * `ad_resolve_element`. Double-free is undefined behavior.
+ * `handle` must be null or a `*mut AdNativeHandle` previously
+ * populated by `ad_resolve_element`. On return `(*handle).ptr` is
+ * `NULL` so a double-call is a no-op instead of a double-free.
  */
-AdResult ad_free_handle(const struct AdAdapter *adapter, const struct AdNativeHandle *handle);
+AdResult ad_free_handle(const struct AdAdapter *adapter, struct AdNativeHandle *handle);
 
 /**
  * # Safety

crates/ffi/src/actions/execute.rs

@@ -30,6 +30,13 @@ pub unsafe extern "C" fn ad_execute_action(
         crate::pointer_guard::guard_non_null!(action, c"action is null");
         let adapter = &*adapter;
         let handle_ref = &*handle;
+        if handle_ref.ptr.is_null() {
+            error::set_last_error(&agent_desktop_core::error::AdapterError::new(
+                agent_desktop_core::error::ErrorCode::InvalidArgs,
+                "handle.ptr is null — the handle has already been freed or was never resolved",
+            ));
+            return AdResult::ErrInvalidArgs;
+        }
         let action_ref = &*action;
         let core_action = match action_from_c(action_ref) {
             Ok(a) => a,

crates/ffi/src/actions/native_handle.rs

@@ -4,24 +4,34 @@ use crate::types::AdNativeHandle;
 use crate::AdAdapter;
 use agent_desktop_core::adapter::NativeHandle;
 
-/// Releases a handle previously returned by `ad_resolve_element`.
+/// Releases a handle previously returned by `ad_resolve_element` and
+/// zeroes the caller's struct so accidentally calling this twice is
+/// a deterministic no-op instead of a double-free on the underlying
+/// `CFRelease`.
 ///
 /// On macOS this calls `CFRelease` on the underlying `AXUIElementRef`,
 /// balancing the `CFRetain` that happened during `ad_resolve_element`.
 /// On Windows/Linux the call is a no-op that returns `AD_RESULT_OK`
-/// (platform adapters inherit the default `not_supported` impl, which
-/// the FFI surface rewrites to `Ok` here so callers can apply the same
-/// release pattern everywhere).
+/// (platform adapters inherit the default `not_supported` impl; the
+/// FFI surface translates it so callers apply the same release
+/// pattern everywhere).
+///
+/// Ownership contract: the FFI owns the handle from the moment
+/// `ad_resolve_element` writes `ptr`. Copying the struct after that
+/// point and calling `ad_free_handle` on either copy is undefined —
+/// there is no way for the library to detect forged non-null pointers.
+/// Callers that legitimately need a "copy" should re-resolve.
 ///
 /// # Safety
 ///
 /// `adapter` must be a non-null pointer returned by `ad_adapter_create`.
-/// `handle` must be null or a pointer previously populated by
-/// `ad_resolve_element`. Double-free is undefined behavior.
+/// `handle` must be null or a `*mut AdNativeHandle` previously
+/// populated by `ad_resolve_element`. On return `(*handle).ptr` is
+/// `NULL` so a double-call is a no-op instead of a double-free.
 #[no_mangle]
 pub unsafe extern "C" fn ad_free_handle(
     adapter: *const AdAdapter,
-    handle: *const AdNativeHandle,
+    handle: *mut AdNativeHandle,
 ) -> AdResult {
     trap_panic(|| unsafe {
         if adapter.is_null() {
@@ -34,16 +44,19 @@ pub unsafe extern "C" fn ad_free_handle(
         if handle.is_null() {
             return AdResult::Ok;
         }
-        let adapter = &*adapter;
         let raw = (*handle).ptr;
         if raw.is_null() {
             return AdResult::Ok;
         }
+        // Zero the caller-visible pointer *before* the platform release
+        // so a concurrent or accidental double-call through the same
+        // struct cannot re-enter CFRelease on the same underlying ref.
+        (*handle).ptr = std::ptr::null();
+        let adapter = &*adapter;
         let native = NativeHandle::from_ptr(raw);
         match adapter.inner.release_handle(&native) {
             Ok(()) => AdResult::Ok,
             Err(e) => {
-                // Not-supported on Windows/Linux is a no-op by contract.
                 if matches!(
                     e.code,
                     agent_desktop_core::error::ErrorCode::ActionNotSupported

crates/ffi/src/observation/get.rs

@@ -36,7 +36,15 @@ pub unsafe extern "C" fn ad_get(
         crate::pointer_guard::guard_non_null!(adapter, c"adapter is null");
         crate::pointer_guard::guard_non_null!(handle, c"handle is null");
         let adapter = &*adapter;
-        let native = NativeHandle::from_ptr((*handle).ptr);
+        let raw = (*handle).ptr;
+        if raw.is_null() {
+            set_last_error(&agent_desktop_core::error::AdapterError::new(
+                agent_desktop_core::error::ErrorCode::InvalidArgs,
+                "handle.ptr is null — the handle has already been freed or was never resolved",
+            ));
+            return AdResult::ErrInvalidArgs;
+        }
+        let native = NativeHandle::from_ptr(raw);
         let prop = match c_to_string(property) {
             Some(s) => s,
             None => {

crates/ffi/tests/c_abi_harness.rs

@@ -66,7 +66,7 @@ extern "C" {
         out: *mut AdNativeHandle,
     ) -> AdResult;
 
-    fn ad_free_handle(adapter: *const AdAdapter, handle: *const AdNativeHandle) -> AdResult;
+    fn ad_free_handle(adapter: *const AdAdapter, handle: *mut AdNativeHandle) -> AdResult;
 }
 
 fn with_adapter<F: FnOnce(*mut AdAdapter)>(body: F) {
@@ -306,14 +306,71 @@ fn find_returns_not_found_on_empty_query_against_no_window() {
 #[test]
 fn free_handle_null_is_noop() {
     with_adapter(|adapter| unsafe {
+        let mut handle = AdNativeHandle {
+            ptr: std::ptr::null(),
+        };
+        let rc = ad_free_handle(adapter, &mut handle);
+        assert_eq!(rc, AdResult::Ok);
+        assert!(handle.ptr.is_null());
+
+        let rc2 = ad_free_handle(adapter, std::ptr::null_mut());
+        assert_eq!(rc2, AdResult::Ok);
+    });
+}
+
+#[cfg(not(target_os = "macos"))]
+#[test]
+fn free_handle_zeroes_ptr_so_double_free_is_noop() {
+    // macOS is excluded: ad_free_handle invokes CFRelease on the
+    // underlying pointer, and a fabricated "fake live" pointer will
+    // SIGBUS before we can observe the zeroing. On Windows/Linux
+    // release_handle is NotSupported (no platform call), so the zeroing
+    // contract is safely observable with a fake pointer.
+    with_adapter(|adapter| unsafe {
+        let fake_live_ptr = 0x1234 as *const std::ffi::c_void;
+        let mut handle = AdNativeHandle { ptr: fake_live_ptr };
+
+        let _ = ad_free_handle(adapter, &mut handle);
+        assert!(handle.ptr.is_null());
+
+        let rc = ad_free_handle(adapter, &mut handle);
+        assert_eq!(rc, AdResult::Ok);
+    });
+}
+
+#[test]
+fn execute_action_rejects_null_handle_ptr() {
+    with_adapter(|adapter| unsafe {
+        let action = AdAction {
+            kind: 0,
+            text: std::ptr::null(),
+            scroll: AdScrollParams {
+                direction: 0,
+                amount: 0,
+            },
+            key: AdKeyCombo {
+                key: std::ptr::null(),
+                modifiers: std::ptr::null(),
+                modifier_count: 0,
+            },
+            drag: AdDragParams {
+                from: AdPoint { x: 0.0, y: 0.0 },
+                to: AdPoint { x: 0.0, y: 0.0 },
+                duration_ms: 0,
+            },
+        };
         let handle = AdNativeHandle {
             ptr: std::ptr::null(),
         };
-        let rc = ad_free_handle(adapter, &handle);
-        assert_eq!(rc, AdResult::Ok);
-
-        let rc2 = ad_free_handle(adapter, std::ptr::null());
-        assert_eq!(rc2, AdResult::Ok);
+        let mut out: AdActionResult = std::mem::zeroed();
+        let rc = ad_execute_action(adapter, &handle, &action, &mut out);
+        // Main-thread guard or null-ptr guard wins; both avoid UB and
+        // keep the struct-level handle accepted while rejecting the
+        // inner null pointer.
+        assert!(matches!(
+            rc,
+            AdResult::ErrInvalidArgs | AdResult::ErrInternal
+        ));
     });
 }