fix: PR #322 round 3 -- SendComplete guard, inline deny, 256KB size limit, test isolation

- ApprovePairRequestAsync and DenyPairRequestAsync both set pending.SendComplete in finally
  block after their send, so HandleIncomingPairHandshakeAsync can await it before returning
  (prevents caller's finally from closing the socket while a send is in-flight)
- Duplicate-request deny is now awaited inline instead of Task.Run fire-and-forget;
  both the lock check and the SendAsync happen outside the lock to avoid re-entrance
- ReadSingleMessageAsync: added 256KB limit to guard unauthenticated /pair path against OOM
- FiestaPairingTests constructor sets _bridgeServer.ServerPassword = 'test-token-isolation'
  so EnsureServerPassword() never falls through to ConnectionSettings.Load()/Save()

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: PureWeen

PolyPilot.Tests/FiestaPairingTests.cs

@@ -23,6 +23,9 @@ public class FiestaPairingTests : IDisposable
     public FiestaPairingTests()
     {
         _bridgeServer = new WsBridgeServer();
+        // Pre-set the server password so EnsureServerPassword() never falls through to
+        // ConnectionSettings.Load()/Save(), which would touch the real ~/.polypilot/settings.json.
+        _bridgeServer.ServerPassword = "test-token-isolation";
         _copilot = new CopilotService(
             new StubChatDatabase(),
             new StubServerManager(),

PolyPilot/Models/FiestaModels.cs

@@ -98,5 +98,8 @@ internal class PendingPairRequest
     public string RemoteIp { get; set; } = "";
     public WebSocket Socket { get; set; } = null!;
     public TaskCompletionSource<bool> CompletionSource { get; set; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+    /// <summary>Resolved by the winner after its SendAsync completes, so HandleIncomingPairHandshakeAsync
+    /// can wait for the send to finish before returning (which lets the caller close the socket safely).</summary>
+    public TaskCompletionSource SendComplete { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
     public DateTime ExpiresAt { get; set; }
 }

PolyPilot/Services/FiestaService.cs

@@ -420,25 +420,32 @@ public async Task HandleIncomingPairHandshakeAsync(WebSocket ws, string remoteIp
 
         // Capture the TCS before releasing the lock
         TaskCompletionSource<bool> tcs;
+        bool isDuplicate;
         lock (_stateLock)
         {
-            if (_pendingPairRequests.Count >= 1)
+            isDuplicate = _pendingPairRequests.Count >= 1;
+            if (!isDuplicate)
             {
-                // Already handling a pair request — deny immediately.
-                // Await the send so the message is delivered before the socket is dropped.
-                _ = Task.Run(async () =>
-                {
-                    try
-                    {
-                        await SendAsync(ws, BridgeMessage.Create(BridgeMessageTypes.FiestaPairResponse,
-                            new FiestaPairResponsePayload { RequestId = req.RequestId, Approved = false }), ct);
-                    }
-                    catch { }
-                });
-                return;
+                _pendingPairRequests[req.RequestId] = pending;
+                tcs = pending.CompletionSource;
             }
-            _pendingPairRequests[req.RequestId] = pending;
-            tcs = pending.CompletionSource;
+            else
+            {
+                tcs = null!; // won't be used
+            }
+        }
+
+        if (isDuplicate)
+        {
+            // Already handling a pair request — deny inline so the send completes
+            // before this method returns and the caller closes the socket.
+            try
+            {
+                await SendAsync(ws, BridgeMessage.Create(BridgeMessageTypes.FiestaPairResponse,
+                    new FiestaPairResponsePayload { RequestId = req.RequestId, Approved = false }), ct);
+            }
+            catch { }
+            return;
         }
 
         OnPairRequested?.Invoke(req.RequestId, req.HostName, remoteIp);
@@ -450,6 +457,9 @@ await SendAsync(ws, BridgeMessage.Create(BridgeMessageTypes.FiestaPairResponse,
         try
         {
             await tcs.Task.WaitAsync(expiryCts.Token);
+            // Winner's send is in-flight — wait for it to complete before returning so the
+            // caller's finally (socket close) doesn't race the outgoing message.
+            await pending.SendComplete.Task.WaitAsync(TimeSpan.FromSeconds(5));
         }
         catch (OperationCanceledException)
         {
@@ -463,8 +473,16 @@ await SendAsync(ws, BridgeMessage.Create(BridgeMessageTypes.FiestaPairResponse,
                         new FiestaPairResponsePayload { RequestId = req.RequestId, Approved = false }), CancellationToken.None);
                 }
                 catch { }
+                finally
+                {
+                    pending.SendComplete.TrySetResult();
+                }
+            }
+            else
+            {
+                // Approve already won — wait for its send to finish before closing socket
+                try { await pending.SendComplete.Task.WaitAsync(TimeSpan.FromSeconds(5)); } catch { }
             }
-            // else: Approve already won the race — skip sending a deny
         }
         finally
         {
@@ -511,6 +529,12 @@ await SendAsync(pending.Socket, BridgeMessage.Create(
             Console.WriteLine($"[Fiesta] Failed to send pair approval: {ex.Message}");
             return false;
         }
+        finally
+        {
+            // Signal that our send is complete so HandleIncomingPairHandshakeAsync
+            // can safely return (allowing the caller to close the socket).
+            pending.SendComplete.TrySetResult();
+        }
     }
 
     public async Task DenyPairRequestAsync(string requestId)
@@ -527,7 +551,6 @@ public async Task DenyPairRequestAsync(string requestId)
         if (!tcs.TrySetResult(false))
             return; // approve already won, don't race on the socket
 
-        // Send before anything unblocks HandleIncomingPairHandshakeAsync.
         try
         {
             await SendAsync(pending.Socket, BridgeMessage.Create(
@@ -536,7 +559,11 @@ await SendAsync(pending.Socket, BridgeMessage.Create(
                 CancellationToken.None);
         }
         catch { }
-        // TCS was already resolved above — HandleIncomingPairHandshakeAsync's await unblocks here.
+        finally
+        {
+            // Signal send complete so HandleIncomingPairHandshakeAsync can safely return.
+            pending.SendComplete.TrySetResult();
+        }
     }
 
     // Keep a synchronous shim for callers that can't await (e.g., Blazor @onclick non-async)
@@ -605,6 +632,7 @@ await SendAsync(ws, BridgeMessage.Create(
             var result = await ws.ReceiveAsync(buffer, ct);
             if (result.MessageType == WebSocketMessageType.Close) return null;
             sb.Append(Encoding.UTF8.GetString(buffer, 0, result.Count));
+            if (sb.Length > 256 * 1024) return null; // guard against unbounded frames on unauthenticated /pair path
             if (result.EndOfMessage) break;
         }
         return BridgeMessage.Deserialize(sb.ToString());