feat: copy files back only when a check reads them

src/Check/DatabaseCheck.php

@@ -24,7 +24,7 @@
  * student's solution has been executed, allowing you to first seed the database and then verify the contents of the
  * database.
  */
-class DatabaseCheck implements ListenableCheckInterface
+class DatabaseCheck implements ListenableCheckInterface, ReadsSolutionFiles
 {
     private Filesystem $filesystem;
     private ?string $dbContent = null;

src/Check/FileComparisonCheck.php

@@ -20,7 +20,7 @@
  * This check verifies that any additional files which should be created by a student, match the ones
  * created by the reference solution.
  */
-class FileComparisonCheck implements SimpleCheckInterface
+class FileComparisonCheck implements SimpleCheckInterface, ReadsSolutionFiles
 {
     /**
      * Return the check's name.

src/Check/ReadsSolutionFiles.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpSchool\PhpWorkshop\Check;
+
+/**
+ * A check that reads files the solutions wrote when they ran, eg. a database. The sandbox copies them back for it.
+ */
+interface ReadsSolutionFiles {}

src/ExerciseDispatcher.php

@@ -6,6 +6,7 @@
 
 use PhpSchool\PhpWorkshop\Check\CheckRepository;
 use PhpSchool\PhpWorkshop\Check\ListenableCheckInterface;
+use PhpSchool\PhpWorkshop\Check\ReadsSolutionFiles;
 use PhpSchool\PhpWorkshop\Check\PhpLintCheck;
 use PhpSchool\PhpWorkshop\Check\SimpleCheckInterface;
 use PhpSchool\PhpWorkshop\Event\EventDispatcher;
@@ -106,13 +107,15 @@ public function requireCheck(string $requiredCheck): void
      */
     public function verify(ExerciseInterface $exercise, Input $input): ResultAggregator
     {
-        $context = ExecutionContext::fromInputAndExercise($input, $exercise);
-
         $runner = $this->runnerManager->getRunner($exercise);
+        $requiredChecks = [...$runner->getRequiredChecks(), ...$exercise->getRequiredChecks()];
+
+        $readsSolutionFiles = array_filter($requiredChecks, fn(string $check) => is_a($check, ReadsSolutionFiles::class, true)) !== [];
+        $context = ExecutionContext::fromInputAndExercise($input, $exercise, $readsSolutionFiles);
 
         $exercise->defineListeners($this->eventDispatcher);
 
-        foreach ([...$runner->getRequiredChecks(), ...$exercise->getRequiredChecks()] as $requiredCheck) {
+        foreach ($requiredChecks as $requiredCheck) {
             $this->requireCheck($requiredCheck);
         }
 

src/ExerciseRunner/CgiRunner.php

@@ -122,8 +122,8 @@ function (RequestInterface $request) use ($context, $scenario, $reference, $stud
             ),
         );
 
-        $reference->finish();
-        $student->finish();
+        $reference->finish($context->readsSolutionFiles());
+        $student->finish($context->readsSolutionFiles());
 
         $this->eventDispatcher->dispatch(new CgiExerciseRunnerEvent('cgi.verify.finish', $context, $scenario));
         return $result;
@@ -340,7 +340,8 @@ public function run(ExecutionContext $context, OutputInterface $output): bool
             );
         }
 
-        $student->finish();
+        //no checks run, so nothing reads the files
+        $student->finish(false);
 
         $this->eventDispatcher->dispatch(new CgiExerciseRunnerEvent('cgi.run.finish', $context, $scenario));
         return $success;

src/ExerciseRunner/CliRunner.php

@@ -121,8 +121,8 @@ function (Collection $args) use ($context, $scenario, $reference, $student) {
             ),
         );
 
-        $reference->finish();
-        $student->finish();
+        $reference->finish($context->readsSolutionFiles());
+        $student->finish($context->readsSolutionFiles());
 
         $this->eventDispatcher->dispatch(new CliExerciseRunnerEvent('cli.verify.finish', $context, $scenario));
 
@@ -252,7 +252,8 @@ public function run(ExecutionContext $context, OutputInterface $output): bool
             );
         }
 
-        $student->finish();
+        //no checks run, so nothing reads the files
+        $student->finish(false);
 
         $this->eventDispatcher->dispatch(new CliExerciseRunnerEvent('cli.run.finish', $context, $scenario));
 

src/ExerciseRunner/Context/ExecutionContext.php

@@ -14,9 +14,13 @@ public function __construct(
         private string $referenceExecutionDirectory,
         private ExerciseInterface $exercise,
         private Input $input,
+        private bool $readsSolutionFiles = false,
     ) {}
 
-    public static function fromInputAndExercise(Input $input, ExerciseInterface $exercise): ExecutionContext
+    /**
+     * @param bool $readsSolutionFiles a check reads the files the solutions wrote, see ReadsSolutionFiles
+     */
+    public static function fromInputAndExercise(Input $input, ExerciseInterface $exercise, bool $readsSolutionFiles = false): ExecutionContext
     {
         $program = $input->hasArgument('program') ? dirname($input->getRequiredArgument('program')) : (string) getcwd();
 
@@ -25,6 +29,7 @@ public static function fromInputAndExercise(Input $input, ExerciseInterface $exe
             System::randomTempDir(),
             $exercise,
             $input,
+            $readsSolutionFiles,
         );
     }
 
@@ -64,4 +69,12 @@ public function getReferenceExecutionDirectory(): string
     {
         return $this->referenceExecutionDirectory;
     }
+
+    /**
+     * A check reads the files the solutions wrote, so the sandbox must copy them back.
+     */
+    public function readsSolutionFiles(): bool
+    {
+        return $this->readsSolutionFiles;
+    }
 }

src/Process/Port/LockPortPicker.php

@@ -14,8 +14,8 @@
  */
 final class LockPortPicker implements PortPicker
 {
-    //longer than a run can take, so the port is free again when release() is never called, eg. a crash
-    private const LOCK_SECONDS = 60;
+    //as long as a sandbox container lives, so the port is free again when release() is never called, eg. a crash
+    private const LOCK_SECONDS = 300;
 
     /**
      * @var array<int, LockInterface> port => its lock

src/Process/Sandbox/Docker/DockerSandbox.php

@@ -11,83 +11,40 @@
 use Symfony\Component\Process\Process;
 
 /**
- * Runs the code in a throwaway container with the solution's directory mounted.
- *
- * The workshop only supplies the image. All isolation flags are set here so a workshop cannot loosen them.
+ * Runs the code in the solution's container, see DockerSandboxFactory. Each execution is a `docker exec`.
  */
 final class DockerSandbox implements Sandbox
 {
     private const TIMEOUT = 30;
 
     /**
-     * @param string $portsNetwork the network when a port is published, see DockerSandboxFactory
+     * @param string $container the container's name, eg. php-school-1a2b3c4d5e6f7a8b
      * @param string $portsHost the address the port is published on, eg. 127.0.0.1
      */
     public function __construct(
         private string $directory,
         private ?int $port,
         private string $docker,
-        private string $basePath,
-        private string $image,
-        private string $portsNetwork,
+        private string $container,
         private string $portsHost,
         private PortPicker $portPicker,
     ) {}
 
     public function execute(ProcessInput $processInput): SandboxExecution
     {
-        $network = ['--network', 'none'];
-        if ($this->port !== null) {
-            //the same port on the docker host, the factory picked one free there
-            $network = ['--network', $this->portsNetwork, '-p', $this->portsHost . ':' . $this->port . ':' . $this->port];
-        }
-
         $envArgs = [];
         foreach ($processInput->getEnv() as $key => $value) {
-            $envArgs[] = '-e';
-            $envArgs[] = $key . '=' . $value;
+            array_push($envArgs, '-e', $key . '=' . $value);
         }
 
         return new SandboxExecution(new Process(
             [
-                $this->docker,
-                'run',
-                '--rm',
-                '-i',
-                '--init',
-                '--user',
-                getmyuid() . ':' . getmygid(),
-                ...$network,
-                '--read-only',
-                '--tmpfs',
-                '/tmp:rw,nosuid,nodev,size=64m',
-                '--cap-drop',
-                'ALL',
-                '--security-opt',
-                'no-new-privileges',
-                '--pids-limit',
-                '64',
-                '--memory',
-                '256m',
-                '--memory-swap',
-                '256m',
-                '--cpus',
-                '1',
-                ...$envArgs,
-                '-v',
-                $this->directory . ':/solution',
-                '-w',
-                '/solution',
-                $this->image,
-                //the docker client being killed does not stop the container, so enforce the timeout inside it too
-                'timeout',
-                '-s',
-                'KILL',
-                (string) self::TIMEOUT,
-                $processInput->getExecutable(),
-                ...$processInput->getArgs(),
+                $this->docker, 'exec', '-i', ...$envArgs, $this->container,
+                //the docker client being killed does not stop the process, so enforce the timeout inside it too
+                'timeout', '-s', 'KILL', (string) self::TIMEOUT,
+                $processInput->getExecutable(), ...$processInput->getArgs(),
             ],
-            $this->basePath,
+            $this->directory,
             null,
             $processInput->getInput(),
             self::TIMEOUT + 5,
@@ -99,10 +56,26 @@ public function getPort(): int
         return $this->port ?? throw new \RuntimeException('The scenario does not expose a port');
     }
 
-    public function finish(): void
+    public function finish(bool $copyFilesBack): void
     {
-        if ($this->port !== null) {
-            $this->portPicker->release($this->port);
+        try {
+            if ($copyFilesBack) {
+                //files only, so tar leaves the folder's mode alone. Not vendor, composer runs on this machine
+                Process::fromShellCommandline(
+                    sprintf(
+                        "%s exec %s sh -c 'find . -path ./vendor -prune -o ! -type d -print | tar -c -T -' | tar -x",
+                        escapeshellarg($this->docker),
+                        $this->container,
+                    ),
+                    $this->directory,
+                )->mustRun();
+            }
+        } finally {
+            (new Process([$this->docker, 'rm', '-f', '-v', $this->container]))->run();
+
+            if ($this->port !== null) {
+                $this->portPicker->release($this->port);
+            }
         }
     }
 }

src/Process/Sandbox/Docker/DockerSandboxFactory.php

@@ -17,6 +17,9 @@
  */
 final class DockerSandboxFactory implements SandboxFactory
 {
+    //seconds, longer than every execution of a solution takes. LockPortPicker holds the port as long
+    private const LIFETIME = 300;
+
     private ExecutableFinder $executableFinder;
     private string $basePath;
     private string $projectName;
@@ -63,11 +66,43 @@ public function createBuildProcess(): Process
         );
     }
 
+    /**
+     * Starts the solution's container and copies its directory in. Nothing is mounted from this machine, so the docker
+     * host can be another one.
+     */
     public function create(string $directory, ExerciseScenario $scenario): DockerSandbox
     {
+        $docker = $this->findDocker();
         $port = $scenario->exposesPort() ? $this->portPicker->pick() : null;
+        $container = 'php-school-' . bin2hex(random_bytes(8));
+
+        $network = ['--network', 'none'];
+        if ($port !== null) {
+            //the same port on the docker host, the picker picked one free there
+            $network = ['--network', $this->portsNetwork, '-p', $this->portsHost . ':' . $port . ':' . $port];
+        }
+
+        //all isolation flags are set here so a workshop cannot loosen them. It removes itself after LIFETIME seconds
+        //when the sandbox is never finished, eg. a crash
+        (new Process([
+            $docker, 'run', '-d', '--rm', '--name', $container, '--init', '--user', getmyuid() . ':' . getmygid(),
+            ...$network,
+            '--read-only', '--tmpfs', '/tmp:rw,nosuid,nodev,size=64m', '--cap-drop', 'ALL',
+            '--security-opt', 'no-new-privileges', '--pids-limit', '64', '--memory', '256m', '--memory-swap', '256m',
+            '--cpus', '1',
+            //a volume of its own, so the directory can be copied in
+            '-v', '/solution', '-w', '/solution',
+            $this->getImageName(), 'sleep', (string) self::LIFETIME,
+        ]))->mustRun();
+
+        //tar, as docker cp can not write to a read-only container. Files only, as tar can not change the folders the
+        //image made, eg. /solution
+        Process::fromShellCommandline(
+            sprintf('find . ! -type d | tar -c -T - | %s exec -i %s tar -x -C /solution', escapeshellarg($docker), $container),
+            $directory,
+        )->mustRun();
 
-        return new DockerSandbox($directory, $port, $this->findDocker(), $this->basePath, $this->getImageName(), $this->portsNetwork, $this->portsHost, $this->portPicker);
+        return new DockerSandbox($directory, $port, $docker, $container, $this->portsHost, $this->portPicker);
     }
 
     private function findDocker(): string

src/Process/Sandbox/Host/HostSandbox.php

@@ -45,7 +45,7 @@ public function getPort(): int
         return $this->port ?? throw new \RuntimeException('The scenario does not expose a port');
     }
 
-    public function finish(): void {}
+    public function finish(bool $copyFilesBack): void {}
 
     /**
      * Symfony Process inherits the parent env, so we unset every inherited var by setting it to false.

src/Process/Sandbox/Sandbox.php

@@ -18,6 +18,8 @@ public function getPort(): int;
 
     /**
      * Once every execution is done, frees the port.
+     *
+     * @param bool $copyFilesBack the files the code wrote are needed in the directory, eg. a check reads them
      */
-    public function finish(): void;
+    public function finish(bool $copyFilesBack): void;
 }

test/ExerciseDispatcherTest.php

@@ -6,6 +6,7 @@
 use PhpSchool\PhpWorkshop\Check\CheckRepository;
 use PhpSchool\PhpWorkshop\Check\ListenableCheckInterface;
 use PhpSchool\PhpWorkshop\Check\PhpLintCheck;
+use PhpSchool\PhpWorkshop\Check\ReadsSolutionFiles;
 use PhpSchool\PhpWorkshop\Check\SimpleCheckInterface;
 use PhpSchool\PhpWorkshop\Event\EventDispatcher;
 use PhpSchool\PhpWorkshop\Event\EventInterface;
@@ -240,6 +241,36 @@ public function testVerify(): void
         $this->assertTrue($result->isSuccessful());
     }
 
+    public function testVerifyTellsTheRunnerWhenACheckReadsSolutionFiles(): void
+    {
+        $exercise = new CliExerciseImpl('Some Exercise');
+
+        $check = $this->createMockForIntersectionOfInterfaces([SimpleCheckInterface::class, ReadsSolutionFiles::class]);
+        $check->method('canRun')->willReturn(true);
+        $check->method('getPosition')->willReturn(SimpleCheckInterface::CHECK_AFTER);
+        $check->method('getExerciseInterface')->willReturn(ExerciseInterface::class);
+        $check->method('check')->willReturn(new Success('Success!'));
+
+        $runner = $this->createMock(ExerciseRunnerInterface::class);
+        $runner->method('getRequiredChecks')->willReturn([get_class($check)]);
+        $runner->expects($this->once())
+            ->method('verify')
+            ->with($this->callback(fn(ExecutionContext $context) => $context->readsSolutionFiles()))
+            ->willReturn(new Success('Success!'));
+
+        $runnerManager = $this->createMock(RunnerManager::class);
+        $runnerManager->method('getRunner')->with($exercise)->willReturn($runner);
+
+        $exerciseDispatcher = new ExerciseDispatcher(
+            $runnerManager,
+            new ResultAggregator(),
+            new EventDispatcher(new ResultAggregator()),
+            new CheckRepository([$check]),
+        );
+
+        $exerciseDispatcher->verify($exercise, new Input('app', ['program' => $this->file]));
+    }
+
     public function testVerifyOnlyRunsRequiredChecks(): void
     {
         $input = new Input('app', ['program' => $this->file]);

test/ExerciseRunner/CliRunnerTest.php

@@ -236,7 +236,7 @@ public function getPort(): int
                 return 54321;
             }
 
-            public function finish(): void {}
+            public function finish(bool $copyFilesBack): void {}
         };
         $factory = $this->createMock(SandboxFactory::class);
         $factory->method('create')->willReturnCallback(function (string $directory, ExerciseScenario $scenario) use ($sandbox) {