Make document filepath and sha1sum readonly

Author: SebSept

src/Document.php

@@ -224,10 +224,7 @@ public function prepareInputForAdd($input)
     {
         global $CFG_GLPI;
 
-        // security (don't accept filename from $_REQUEST)
-        if (array_key_exists('filename', $_REQUEST)) {
-            unset($input['filename']);
-        }
+        $input = $this->filterFields($input);
 
         // current_filename is not necessary (item is new, current_filename should not exist
         // but used for display can lead to wrong file deletion in moveDocument() and moveUploadedDocument()
@@ -252,9 +249,6 @@ public function prepareInputForAdd($input)
         } elseif (!empty($input["upload_file"])) {
             // Move doc from upload dir
             $upload_ok = $this->moveUploadedDocument($input, $input["upload_file"]);
-        } elseif (isset($input['filepath']) && file_exists(GLPI_DOC_DIR . '/' . $input['filepath'])) {
-            // Document is created using an existing document file
-            $upload_ok = true;
         }
 
         // Tag
@@ -340,10 +334,7 @@ public function post_addItem()
 
     public function prepareInputForUpdate($input)
     {
-        // security (don't accept filename from $_REQUEST)
-        if (array_key_exists('filename', $_REQUEST)) {
-            unset($input['filename']);
-        }
+        $input = $this->filterFields($input);
 
         if (isset($input['current_filepath'])) {
             // Always use the values stored in DB to prevent arbitrary file deletion
@@ -1809,4 +1800,21 @@ private function canViewFileFromForm(string $itemtype, int $items_id): bool
         );
         return $control_manager->canAnswerForm($form, $parameters);
     }
+
+    /**
+     * Remove $input fields that should not be provided by user input
+     *
+     * @param  array<string, mixed> $input
+     * @return array<string, mixed>
+     */
+    private function filterFields(array $input): array
+    {
+        // security (don't accept filename from $_REQUEST)
+        if (array_key_exists('filename', $_REQUEST)) {
+            unset($input['filename']);
+        }
+
+        $blacklisted = ['filepath', 'sha1sum'];
+        return array_filter($input, static fn($v, $k) => !in_array($k, $blacklisted), ARRAY_FILTER_USE_BOTH);
+    }
 }

tests/functional/DocumentTest.php

@@ -189,6 +189,77 @@ public function testPrepareInputForAdd()
         $this->assertCount(7, $prepare);
     }
 
+    public function testPrepareInputForAddIgnoreBlacklistedFields(): void
+    {
+        $_ignored = 'should_be_ignored';
+        $input = ['name' => 'legit name', 'filepath' => $_ignored, 'sha1sum' =>  $_ignored];
+        $prepare = (new \Document())->prepareInputForAdd($input);
+
+        $this->assertArrayNotHasKey('filepath', $prepare);
+        $this->assertArrayNotHasKey('sha1sum', $prepare);
+    }
+
+    /**
+     * Ensure filepath and sha1sum are not updated by prepareInputForUpdate, even if they are present in the input.
+     */
+    public function testPrepareInputForUpdateIgnoreBlacklistedFields(): void
+    {
+        $_ignored = 'should_be_ignored';
+        $input = ['name' => 'legit name', 'filepath' => $_ignored, 'sha1sum' =>  $_ignored];
+
+        $doc_id = (new \Document())->add(['name' => 'le document']);
+        $prepare = (new \Document())->prepareInputForUpdate($input + ['id' => $doc_id]);
+
+        $this->assertArrayNotHasKey('filepath', $prepare);
+        $this->assertArrayNotHasKey('sha1sum', $prepare);
+    }
+
+    public function testPrepareInputForUpdateIgnoreBlacklistedFieldsWithUpload(): void
+    {
+        $this->login();
+
+        // Create a document with a mocked file upload
+        $mdoc = $this->getMockBuilder(\Document::class)
+            ->onlyMethods(['moveUploadedDocument'])
+            ->getMock();
+        $mdoc->method('moveUploadedDocument')->willReturn(true);
+
+        $doc_id = $mdoc->add([
+            'name'        => 'test document with upload',
+            'upload_file' => 'filename.txt',
+        ]);
+        $this->assertGreaterThan(0, $doc_id);
+
+        // Read back the stored values right after the upload-add
+        $doc = new \Document();
+        $this->assertTrue($doc->getFromDB($doc_id));
+        $original_filepath = $doc->fields['filepath'];
+        $original_sha1sum  = $doc->fields['sha1sum'];
+
+        // Attempt to update the document with blacklisted fields
+        $_fake = 'should_be_ignored';
+        $doc->update([
+            'id'       => $doc_id,
+            'name'     => 'updated name',
+            'filepath' => $_fake,
+            'sha1sum'  => $_fake,
+        ]);
+
+        // Re-read from DB and verify blacklisted fields were NOT overwritten
+        $doc_after = new \Document();
+        $this->assertTrue($doc_after->getFromDB($doc_id));
+
+        $this->assertNotSame($_fake, $doc_after->fields['filepath']);
+        $this->assertNotSame($_fake, $doc_after->fields['sha1sum']);
+
+        // Values must be identical to what they were right after the upload
+        $this->assertSame($original_filepath, $doc_after->fields['filepath']);
+        $this->assertSame($original_sha1sum, $doc_after->fields['sha1sum']);
+
+        // Non-blacklisted field must have been updated correctly
+        $this->assertSame('updated name', $doc_after->fields['name']);
+    }
+
     /** Cannot work without a real document uploaded.
      *  Mock would be a solution but GLPI will try to use
      *  a table based on mocked class name, this is wrong.

tests/imap/MailCollectorTest.php

@@ -1565,23 +1565,25 @@ public function testCleanContent(
 
     public function testBlacklistedDocumentNotImportedFromMail()
     {
+        global $DB;
+
         // Use existing test fixture files
         $png_file = GLPI_ROOT . '/tests/fixtures/uploads/foo.png';
 
         // Calculate SHA1 from existing files
         $png_sha1 = sha1_file($png_file);
 
         // Create blacklisted documents with matching SHA1
-        $this->createItem(
+        $doc = $this->createItem(
             \Document::class,
             [
                 'entities_id' => 0,
                 'name' => 'Blacklisted PNG Document',
                 'filename' => 'blacklisted.png',
-                'sha1sum' => $png_sha1,
                 'is_blacklisted' => 1,
             ]
         );
+        $this->assertTrue($DB->update(\Document::getTable(), ['sha1sum' => $png_sha1], ['id' => $doc->getID()]));
 
         // Test 1: Auto-import (mail collector) should be blocked
         \Safe\copy($png_file, GLPI_TMP_DIR . '/foo.png');