From 123fd05a9a93ca41f1fbd62cb1eefef70e19d936 Mon Sep 17 00:00:00 2001
From: Tom Elliott <mastacontrola@users.noreply.github.com>
Date: Wed, 28 Oct 2026 08:45:07 -0500
Subject: [PATCH] FOGSSH: null the link on disconnect, and close only a live
 session

disconnect() unset() the declared $_link and $_sftp. PHP routes the next
read of an unset declared property to __get(), which looks it up in $data,
so every reuse of the shared instance warned 'Undefined array key "_link"'
at FOGSSH.php:133. Set them to null instead.

disconnect() also handed a false or null link to ssh2_disconnect(). On PHP 8
that is a TypeError, which @ does not suppress, so disconnect() after a
failed connect() or a second disconnect() was fatal. Close only a resource.

tests/fogssh-reuse-after-disconnect.test.php drives disconnect() twice on
the real class. Each check fails on its own defect: the TypeError on the
base, and the warning with only the unset() put back.

Fixes #1997

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 51229fe43e97822b36a5a5ff7583e1bfc7259513)
---
 packages/web/src/Net/FOGSSH.php              | 13 +++--
 tests/fogssh-reuse-after-disconnect.test.php | 56 ++++++++++++++++++++
 2 files changed, 66 insertions(+), 3 deletions(-)
 create mode 100644 tests/fogssh-reuse-after-disconnect.test.php

diff --git a/packages/web/src/Net/FOGSSH.php b/packages/web/src/Net/FOGSSH.php
index 3e4c738769..85453334b5 100644
--- a/packages/web/src/Net/FOGSSH.php
+++ b/packages/web/src/Net/FOGSSH.php
@@ -152,14 +152,21 @@ public function sftp()
      */
     public function disconnect()
     {
+        // Null, never unset(). An unset declared property routes the next
+        // read of it to __get(), which looks it up in $data and warns
+        // 'Undefined array key "_link"' on every reuse of this shared
+        // instance -- the next connect() or disconnect() reads both.
         if ($this->_sftp) {
             @ssh2_disconnect($this->_sftp);
             $this->_sftp = null;
-            unset($this->_sftp);
         }
-        $test = @ssh2_disconnect($this->_link);
+        // No session to close after a failed connect (false) or an earlier
+        // disconnect (null). ssh2_disconnect() takes only a resource, and on
+        // PHP 8 anything else is a TypeError that @ does not suppress.
+        $test = is_resource($this->_link)
+            ? @ssh2_disconnect($this->_link)
+            : false;
         $this->_link = null;
-        unset($this->_link);
         return $test;
     }
     /**
diff --git a/tests/fogssh-reuse-after-disconnect.test.php b/tests/fogssh-reuse-after-disconnect.test.php
new file mode 100644
index 0000000000..4af8f3ba09
--- /dev/null
+++ b/tests/fogssh-reuse-after-disconnect.test.php
@@ -0,0 +1,56 @@
+<?php
+/**
+ * FOGSSH can be used again after disconnect() without a warning.
+ *
+ * FOGSSH is one shared instance (LoadGlobals). disconnect() used to unset()
+ * its declared $_link and $_sftp. PHP then routes the next read of an unset
+ * property to __get(), which looks it up in $data and warns
+ * 'Undefined array key "_link"'. Every daemon pass that disconnected and
+ * reused the instance wrote that warning to servicemaster.log.
+ *
+ * disconnect() also passed a null or false link to ssh2_disconnect(). On
+ * PHP 8 that is a TypeError, which @ does not suppress, so a disconnect()
+ * after a failed connect() was fatal.
+ *
+ * Runs with or without the ssh2 extension: with it, the first disconnect()
+ * on a never-connected instance is the TypeError case.
+ *
+ * Usage: php tests/fogssh-reuse-after-disconnect.test.php
+ * Exit status 0 = pass, 1 = fail.
+ */
+
+require __DIR__ . '/lib/fog-test-harness.php';
+
+FogTestHarness::boot('fogssh-reuse-after-disconnect');
+
+$t = new FogChecks();
+
+$warnings = [];
+set_error_handler(function ($no, $str, $file, $line) use (&$warnings) {
+    $warnings[] = "$str at " . basename($file) . ":$line";
+    return true;
+});
+
+$ssh = new \FOG\Net\FOGSSH();
+$fatal = '';
+try {
+    $ssh->disconnect();
+    $ssh->disconnect();
+} catch (\Throwable $e) {
+    $fatal = get_class($e) . ': ' . $e->getMessage();
+}
+
+restore_error_handler();
+
+$t->check(
+    'disconnect() without a live session does not throw'
+    . ($fatal ? " ($fatal)" : ''),
+    '' === $fatal
+);
+$t->check(
+    'a second disconnect() reads $_link and $_sftp without a warning'
+    . ($warnings ? ' (' . implode('; ', $warnings) . ')' : ''),
+    [] === $warnings
+);
+
+$t->finish();