more changes with upload. Security enhancements. Also got rid of php-watcher

composer.json

@@ -51,7 +51,6 @@
         "phpstan/phpstan": "^2.1",
         "phpunit/phpunit": "^9.6",
         "rregeer/phpunit-coverage-check": "^0.3.1",
-        "spatie/phpunit-watcher": "^1.23 || ^1.24",
         "squizlabs/php_codesniffer": "^3.11"
     },
     "config": {
@@ -62,7 +61,7 @@
         "sort-packages": true
     },
     "scripts": {
-        "test": "vendor/bin/phpunit-watcher watch",
+        "test": "phpunit",
         "test-ci": "phpunit",
         "test-coverage": "rm -f clover.xml && XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage --coverage-clover=clover.xml && vendor/bin/coverage-check clover.xml 100",
         "test-server": "echo \"Running Test Server\" && php -S localhost:8000 -t tests/server/",

flight/net/UploadedFile.php

@@ -33,6 +33,11 @@ class UploadedFile
      */
     private int $error;
 
+    /**
+     * @var bool $isPostUploadedFile Indicates if the file was uploaded via POST method.
+     */
+    private bool $isPostUploadedFile = false;
+
     /**
      * Constructs a new UploadedFile object.
      *
@@ -41,14 +46,20 @@ class UploadedFile
      * @param int $size The size of the uploaded file in bytes.
      * @param string $tmpName The temporary name of the uploaded file.
      * @param int $error The error code associated with the uploaded file.
+     * @param bool|null $isPostUploadedFile Indicates if the file was uploaded via POST method.
      */
-    public function __construct(string $name, string $mimeType, int $size, string $tmpName, int $error)
+    public function __construct(string $name, string $mimeType, int $size, string $tmpName, int $error, ?bool $isPostUploadedFile = null)
     {
         $this->name = $name;
         $this->mimeType = $mimeType;
         $this->size = $size;
         $this->tmpName = $tmpName;
         $this->error = $error;
+        if (is_uploaded_file($tmpName) === true) {
+            $this->isPostUploadedFile = true; // @codeCoverageIgnore
+        } else {
+            $this->isPostUploadedFile = $isPostUploadedFile ?? false;
+        }
     }
 
     /**
@@ -114,33 +125,43 @@ class UploadedFile
             throw new Exception($this->getUploadErrorMessage($this->error));
         }
 
+        if (is_writeable(dirname($targetPath)) === false) {
+            throw new Exception('Target directory is not writable');
+        }
+
+        // Prevent path traversal attacks
+        if (strpos($targetPath, '..') !== false) {
+            throw new Exception('Invalid target path: contains directory traversal');
+        }
+        // Prevent absolute paths (basic check for Unix/Windows)
+        if ($targetPath[0] === '/' || (strlen($targetPath) > 1 && $targetPath[1] === ':')) {
+            throw new Exception('Invalid target path: absolute paths not allowed');
+        }
+
+        // Prevent overwriting existing files
+        if (file_exists($targetPath)) {
+            throw new Exception('Target file already exists');
+        }
+
         // Check if this is a legitimate uploaded file (POST method uploads)
-        $isUploadedFile = is_uploaded_file($this->tmpName) === true;
+        $isUploadedFile = $this->isPostUploadedFile;
 
-        if ($isUploadedFile === true) {
+        // Prevent symlink attacks for non-POST uploads
-            // Standard POST upload - use move_uploaded_file for security
+        if (!$isUploadedFile && is_link($this->tmpName)) {
-            if (move_uploaded_file($this->tmpName, $targetPath) === false) {
+            throw new Exception('Invalid temp file: symlink detected');
-                throw new Exception('Cannot move uploaded file'); // @codeCoverageIgnore
         }
-        } elseif (getenv('PHPUNIT_TEST')) {
+
-            rename($this->tmpName, $targetPath);
+        $uploadFunctionToCall = $isUploadedFile === true ?
-        } elseif (file_exists($this->tmpName) === true && is_readable($this->tmpName) === true) {
+            // Standard POST upload - use move_uploaded_file for security
+            'move_uploaded_file' :
              // Handle non-POST uploads (PATCH, PUT, DELETE) or other valid temp files
-            // Verify the file is in a valid temp directory for security
+            'rename';
-            $tempDir = sys_get_temp_dir();
-            $uploadTmpDir = ini_get('upload_tmp_dir') ?: $tempDir;
 
-            if (strpos(realpath($this->tmpName), realpath($uploadTmpDir)) === 0 || 
+        $result = $uploadFunctionToCall($this->tmpName, $targetPath);
-                strpos(realpath($this->tmpName), realpath($tempDir)) === 0) {
+
-                if (rename($this->tmpName, $targetPath) === false) {
+        if ($result === false) {
             throw new Exception('Cannot move uploaded file');
         }
-            } else {
-                throw new Exception('Invalid temporary file location');
-            }
-        } else {
-            throw new Exception('Temporary file does not exist or is not readable');
-        }
     }
 
     /**

tests/UploadedFileTest.php

@@ -18,14 +18,24 @@ class UploadedFileTest extends TestCase
         if (file_exists('tmp_name')) {
             unlink('tmp_name');
         }
+        if (file_exists('existing.txt')) {
+            unlink('existing.txt');
+        }
+        if (file_exists('real_file')) {
+            unlink('real_file');
+        }
+
+		// not found with file_exists...just delete it brute force
+		@unlink('tmp_symlink');
     }
 
-    public function testMoveToSuccess(): void
+    public function testMoveToFalseSuccess(): void
     {
+        // This test would have passed in the real world but we can't actually force a post request in unit tests
         file_put_contents('tmp_name', 'test');
-        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK);
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, true);
+        $this->expectExceptionMessage('Cannot move uploaded file');
         $uploadedFile->moveTo('file.txt');
-        $this->assertFileExists('file.txt');
     }
 
     public function getFileErrorMessageTests(): array
@@ -53,4 +63,62 @@ class UploadedFileTest extends TestCase
         $this->expectExceptionMessage($message);
         $uploadedFile->moveTo('file.txt');
     }
+
+    public function testMoveToBadLocation(): void
+    {
+        file_put_contents('tmp_name', 'test');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, true);
+        $this->expectExceptionMessage('Target directory is not writable');
+        $uploadedFile->moveTo('/root/file.txt');
+    }
+
+    public function testMoveToSuccessNonPost(): void
+    {
+        file_put_contents('tmp_name', 'test');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false);
+        $uploadedFile->moveTo('file.txt');
+        $this->assertFileExists('file.txt');
+        $this->assertEquals('test', file_get_contents('file.txt'));
+    }
+
+    public function testMoveToPathTraversal(): void
+    {
+        file_put_contents('tmp_name', 'test');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false);
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Invalid target path: contains directory traversal');
+        $uploadedFile->moveTo('../file.txt');
+    }
+
+    public function testMoveToAbsolutePath(): void
+    {
+        file_put_contents('tmp_name', 'test');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false);
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Invalid target path: absolute paths not allowed');
+        $uploadedFile->moveTo('/tmp/file.txt');
+    }
+
+    public function testMoveToOverwrite(): void
+    {
+        file_put_contents('tmp_name', 'test');
+        file_put_contents('existing.txt', 'existing');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_name', UPLOAD_ERR_OK, false);
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Target file already exists');
+        $uploadedFile->moveTo('existing.txt');
+    }
+
+    public function testMoveToSymlinkNonPost(): void
+    {
+        file_put_contents('real_file', 'test');
+        if (file_exists('tmp_symlink')) {
+            unlink('tmp_symlink');
+        }
+        symlink('real_file', 'tmp_symlink');
+        $uploadedFile = new UploadedFile('file.txt', 'text/plain', 4, 'tmp_symlink', UPLOAD_ERR_OK, false);
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Invalid temp file: symlink detected');
+        $uploadedFile->moveTo('file.txt');
+    }
 }