diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b33c604..df6580ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ Latest ------ +## Fixes +* [#343](https://github.com/cleverage/process-bundle/issues/343) The temporary files of `FileSplitterTask` (`file_splitter`), `CsvSplitterTask` (`csv_splitter`) and of the JSON buffer of `ProcessLauncherTask` (`json_buffering`) are created with a random name, exclusively (an existing path, symbolic links included, is never reused) and readable by their owner only, instead of a name derived from the time (`uniqid()`), opened in `w` mode with the permissions of the umask. Update documentation, add tests. + v5.4.1 ----- diff --git a/docs/reference/tasks/csv_splitter_task.md b/docs/reference/tasks/csv_splitter_task.md index 328f0b47..a7a4e89c 100644 --- a/docs/reference/tasks/csv_splitter_task.md +++ b/docs/reference/tasks/csv_splitter_task.md @@ -20,8 +20,9 @@ Accepted inputs Possible outputs ---------------- -`string`: path of a temporary CSV file (created in the system temporary directory) containing the headers and the next -`max_lines` data lines of the source file (the last file may contain fewer lines). +`string`: path of a temporary CSV file (created in the system temporary directory, with a random name and readable by +its owner only) containing the headers and the next `max_lines` data lines of the source file (the last file may +contain fewer lines). Options ------- diff --git a/docs/reference/tasks/file_splitter_task.md b/docs/reference/tasks/file_splitter_task.md index 18282cf4..eeadd791 100644 --- a/docs/reference/tasks/file_splitter_task.md +++ b/docs/reference/tasks/file_splitter_task.md @@ -20,8 +20,9 @@ file path as input. Any other input is ignored. Possible outputs ---------------- -`string`: path of a temporary file (created in the system temporary directory, with a `.tmp` extension) containing the -next `max_lines` lines of the source file (the last file may contain fewer lines). +`string`: path of a temporary file (created in the system temporary directory, with a random name, a `.tmp` extension +and readable by its owner only) containing the next `max_lines` lines of the source file (the last file may contain +fewer lines). Options ------- diff --git a/src/Filesystem/TemporaryFile.php b/src/Filesystem/TemporaryFile.php new file mode 100644 index 00000000..323714fc --- /dev/null +++ b/src/Filesystem/TemporaryFile.php @@ -0,0 +1,41 @@ + @@ -75,8 +79,6 @@ public function __construct( $consolePath = $consolePath->getProjectDir().'/bin/console'; } $this->consolePath = $consolePath; - // The project directory may be read-only (e.g. in a container) - $this->bufferPath = sys_get_temp_dir().'/cdm_buffer_'.uniqid('', true).'.json-stream'; } /** @@ -104,6 +106,8 @@ public function buildProcess(): static } if ($this->options[self::OPTION_JSON_BUFFERING]) { + // Created before the sub-process writes into it: random name, exclusive creation, owner only + $this->bufferPath = TemporaryFile::create('cdm_buffer_', '.json-stream'); $arguments = [...$arguments, '--output='.$this->bufferPath, '--output-format=json-stream']; } diff --git a/src/Task/File/Csv/CsvSplitterTask.php b/src/Task/File/Csv/CsvSplitterTask.php index 7d7a440e..d6c22cf5 100644 --- a/src/Task/File/Csv/CsvSplitterTask.php +++ b/src/Task/File/Csv/CsvSplitterTask.php @@ -16,6 +16,7 @@ use CleverAge\ProcessBundle\Exception\RuntimeException; use CleverAge\ProcessBundle\Filesystem\CsvFile; use CleverAge\ProcessBundle\Filesystem\CsvResource; +use CleverAge\ProcessBundle\Filesystem\TemporaryFile; use CleverAge\ProcessBundle\Model\ProcessState; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -93,7 +94,7 @@ public function finalize(ProcessState $state): void protected function splitCsv(CsvResource $csv, int $maxLines): string { - $tmpFilePath = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'php_'.uniqid('process', false).'.csv'; + $tmpFilePath = TemporaryFile::create('php_process', '.csv'); $tmpFile = fopen($tmpFilePath, 'wb+'); if (false === $tmpFile) { throw new RuntimeException("Unable to open temporary file {$tmpFilePath}"); diff --git a/src/Task/File/FileSplitterTask.php b/src/Task/File/FileSplitterTask.php index f2a4fcb0..f0cb6c1c 100644 --- a/src/Task/File/FileSplitterTask.php +++ b/src/Task/File/FileSplitterTask.php @@ -14,6 +14,7 @@ namespace CleverAge\ProcessBundle\Task\File; use CleverAge\ProcessBundle\Filesystem\SplFile; +use CleverAge\ProcessBundle\Filesystem\TemporaryFile; use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; use CleverAge\ProcessBundle\Model\IterableTaskInterface; use CleverAge\ProcessBundle\Model\ProcessState; @@ -78,7 +79,7 @@ public function next(ProcessState $state): bool protected function splitFile(SplFile $file, int $maxLines): string { - $tmpFilePath = sys_get_temp_dir().\DIRECTORY_SEPARATOR.'php_'.uniqid('process', false).'.tmp'; + $tmpFilePath = TemporaryFile::create('php_process', '.tmp'); $splitFile = new SplFile($tmpFilePath, 'wb', []); $writtenLines = 0; diff --git a/tests/Filesystem/TemporaryFileTest.php b/tests/Filesystem/TemporaryFileTest.php new file mode 100644 index 00000000..acebb15b --- /dev/null +++ b/tests/Filesystem/TemporaryFileTest.php @@ -0,0 +1,47 @@ +buildProcess(); $this->assertMatchesRegularExpression( - '#\'--output='.preg_quote(sys_get_temp_dir(), '#').'/cdm_buffer_[^/\']+\.json-stream\' \'--output-format=json-stream\'#', + '#\'--output=('.preg_quote(sys_get_temp_dir(), '#').'/cdm_buffer_[0-9a-f]{32}\.json-stream)\' \'--output-format=json-stream\'#', $subprocess->getProcess()->getCommandLine() ); + preg_match('#--output=([^\']+)#', $subprocess->getProcess()->getCommandLine(), $matches); + $bufferPath = $matches[1] ?? ''; + // Created before the sub-process writes into it, readable by its owner only + $this->assertFileExists($bufferPath); + $this->assertSame(0o600, fileperms($bufferPath) & 0o777); + unlink($bufferPath); } public function testMissingConsoleThrows(): void diff --git a/tests/Task/File/Csv/CsvSplitterTaskTest.php b/tests/Task/File/Csv/CsvSplitterTaskTest.php index 76aa2e94..14171178 100644 --- a/tests/Task/File/Csv/CsvSplitterTaskTest.php +++ b/tests/Task/File/Csv/CsvSplitterTaskTest.php @@ -18,6 +18,7 @@ use CleverAge\ProcessBundle\Context\ContextualOptionResolver; use CleverAge\ProcessBundle\Filesystem\CsvFile; use CleverAge\ProcessBundle\Filesystem\CsvResource; +use CleverAge\ProcessBundle\Filesystem\TemporaryFile; use CleverAge\ProcessBundle\Model\AbstractConfigurableTask; use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; @@ -33,6 +34,7 @@ use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; #[\PHPUnit\Framework\Attributes\CoversClass(CsvSplitterTask::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(TemporaryFile::class)] #[\PHPUnit\Framework\Attributes\UsesClass(CsvReaderTask::class)] #[\PHPUnit\Framework\Attributes\UsesClass(InputCsvReaderTask::class)] #[\PHPUnit\Framework\Attributes\UsesClass(AbstractCsvTask::class)] diff --git a/tests/Task/File/FileSplitterTaskTest.php b/tests/Task/File/FileSplitterTaskTest.php index 8bc84f7f..8dbbba88 100644 --- a/tests/Task/File/FileSplitterTaskTest.php +++ b/tests/Task/File/FileSplitterTaskTest.php @@ -17,6 +17,7 @@ use CleverAge\ProcessBundle\Configuration\TaskConfiguration; use CleverAge\ProcessBundle\Context\ContextualOptionResolver; use CleverAge\ProcessBundle\Filesystem\SplFile; +use CleverAge\ProcessBundle\Filesystem\TemporaryFile; use CleverAge\ProcessBundle\Model\ProcessHistory; use CleverAge\ProcessBundle\Model\ProcessState; use CleverAge\ProcessBundle\Task\File\FileSplitterTask; @@ -25,6 +26,7 @@ use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; #[\PHPUnit\Framework\Attributes\CoversClass(FileSplitterTask::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(TemporaryFile::class)] #[\PHPUnit\Framework\Attributes\UsesClass(SplFile::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfiguration::class)] #[\PHPUnit\Framework\Attributes\UsesClass(TaskConfiguration::class)]