Harden DockerProcessFactory into a real sandbox

src/Process/DockerProcessFactory.php

@@ -7,8 +7,15 @@
 use Symfony\Component\Process\ExecutableFinder;
 use Symfony\Component\Process\Process;
 
+/**
+ * Runs processes in a throwaway container built from the workshop's `.docker/runtime/Dockerfile`.
+ *
+ * The workshop only supplies the image. All isolation flags are set here so a workshop cannot loosen them.
+ */
 final class DockerProcessFactory implements ProcessFactory
 {
+    private const TIMEOUT = 30;
+
     private ExecutableFinder $executableFinder;
     private string $basePath;
     private string $projectName;
@@ -26,78 +33,109 @@ public function __construct(
         $this->composerCacheDir = $composerCacheDir;
     }
 
+    public function getImageName(): string
+    {
+        return strtolower($this->projectName) . '-runtime';
+    }
+
+    /**
+     * The process that builds the runtime image. Run it once on install/deploy, not per request.
+     */
+    public function createBuildProcess(): Process
+    {
+        return new Process(
+            [$this->findDocker(), 'build', '-t', $this->getImageName(), '.docker/runtime'],
+            $this->basePath,
+            null,
+            null,
+            300,
+        );
+    }
+
     public function create(ProcessInput $processInput): Process
     {
-        $mounts = [];
-        if ($processInput->getExecutable() === 'composer') {
-            //we need to mount a volume for composer cache
+        $isComposer = $processInput->getExecutable() === 'composer';
+
+        $args = $processInput->getArgs();
+        $mounts = [$processInput->getWorkingDirectory() . ':/solution'];
+        $env = ['COMPOSER_HOME' => '/tmp/composer'];
+
+        if ($isComposer) {
+            //we need to mount a volume for composer cache, and never run package plugins or scripts
             $mounts[] = $this->composerCacheDir . ':/tmp/composer';
+            $args = [...$args, '--no-plugins', '--no-scripts'];
         }
 
-        $env = [];
-        foreach ($processInput->getEnv() as $key => $value) {
-            $env[] = '-e';
-            $env[] = $key . '=' . $value;
+        $network = ['--network', 'none'];
+        if ($isComposer) {
+            //composer needs to download packages
+            $network = ['--network', 'bridge'];
+        } elseif (count($processInput->getExposedPorts()) > 0) {
+            $network = ['--network', 'bridge'];
+            foreach ($processInput->getExposedPorts() as $port) {
+                $network[] = '-p';
+                $network[] = '127.0.0.1:' . $port . ':' . $port;
+            }
         }
 
-        $ports = [];
-        foreach ($processInput->getExposedPorts() as $port) {
-            $ports[] = '-p';
-            $ports[] = $port . ':' . $port;
+        $envArgs = [];
+        foreach ([...$processInput->getEnv(), ...$env] as $key => $value) {
+            $envArgs[] = '-e';
+            $envArgs[] = $key . '=' . $value;
         }
 
-        $env[]  = '-e';
-        $env[]  = 'COMPOSER_HOME=/tmp/composer';
-
-        $p = new Process(
+        return new Process(
             [
-                ...$this->baseComposeCommand($mounts, $env, $ports),
-                'runtime',
+                $this->findDocker(),
+                '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,
+                ...array_merge(...array_map(fn($mount) => ['-v', $mount], $mounts)),
+                '-w',
+                '/solution',
+                $this->getImageName(),
+                //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(),
+                ...$args,
             ],
             $this->basePath,
-            [
-                'SOLUTION' => $processInput->getWorkingDirectory(),
-                'UID' => (string) getmyuid(),
-                'GID' => (string) getmygid(),
-            ],
+            null,
             $processInput->getInput(),
-            30,
+            self::TIMEOUT + 5,
         );
-
-        return $p;
     }
 
-    /**
-     * @param array<string> $mounts
-     * @param array<string> $env
-     * @param list<string> $ports
-     * @return array<string>
-     */
-    private function baseComposeCommand(array $mounts, array $env, array $ports): array
+    private function findDocker(): string
     {
         $dockerPath = $this->executableFinder->find('docker');
         if ($dockerPath === null) {
             throw ProcessNotFoundException::fromExecutable('docker');
         }
 
-        return [
-            $dockerPath,
-            'compose',
-            '-p',
-            $this->projectName,
-            '-f',
-            '.docker/runtime/docker-compose.yml',
-            'run',
-            '--user',
-            getmyuid() . ':' . getmygid(),
-            '--rm',
-            ...$env,
-            ...$ports,
-            '-w',
-            '/solution',
-            ...array_merge(...array_map(fn($mount) => ['-v', $mount], $mounts)),
-        ];
+        return $dockerPath;
     }
 }

test/Process/DockerProcessFactoryTest.php

@@ -10,128 +10,119 @@
 
 class DockerProcessFactoryTest extends TestCase
 {
-    public function testCreateThrowsExceptionIfDockerNotFound(): void
+    private function factory(): DockerProcessFactory
     {
-        static::expectException(ProcessNotFoundException::class);
-
         $finder = $this->createMock(ExecutableFinder::class);
         $finder->expects($this->once())
             ->method('find')
             ->with('docker')
-            ->willReturn(null);
-
-        $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('composer', [], __DIR__, [], []);
+            ->willReturn('/usr/local/bin/docker');
 
-        $factory->create($input);
+        return new DockerProcessFactory('/docker-dir', 'PHP8Appreciate', '/composer/cache/dir', $finder);
     }
 
-    public function testCreate(): void
+    /**
+     * @param list<string> $network
+     * @param list<string> $extra env and mount args
+     * @param list<string> $command
+     */
+    private function expectedCommand(array $network, array $extra, array $command): string
     {
-        $finder = $this->createMock(ExecutableFinder::class);
-        $finder->expects($this->once())
-            ->method('find')
-            ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
-
-        $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('php', [], __DIR__, [], []);
-
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' 'runtime' 'php'";
-        static::assertSame($cmd, $process->getCommandLine());
-        static::assertSame('/docker-dir', $process->getWorkingDirectory());
+        $parts = [
+            '/usr/local/bin/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',
+            ...$extra,
+            '-w', '/solution', 'php8appreciate-runtime', 'timeout', '-s', 'KILL', '30',
+            ...$command,
+        ];
+
+        return implode(' ', array_map(fn($p) => "'$p'", $parts));
     }
 
-    public function testCreateMountsComposerCacheDirIfExecutableIsComposer(): void
+    public function testCreateThrowsExceptionIfDockerNotFound(): void
     {
+        static::expectException(ProcessNotFoundException::class);
+
         $finder = $this->createMock(ExecutableFinder::class);
         $finder->expects($this->once())
             ->method('find')
             ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
+            ->willReturn(null);
 
         $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
         $input = new ProcessInput('composer', [], __DIR__, [], []);
 
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' '-v' '/composer/cache/dir:/tmp/composer' 'runtime' 'composer'";
-        static::assertSame($cmd, $process->getCommandLine());
-        static::assertSame('/docker-dir', $process->getWorkingDirectory());
+        $factory->create($input);
     }
 
-    public function testCreateWithArgs(): void
+    public function testCreateIsIsolatedWithNoNetwork(): void
     {
-        $finder = $this->createMock(ExecutableFinder::class);
-        $finder->expects($this->once())
-            ->method('find')
-            ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
-
-        $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('php', ['one', 'two'], __DIR__, [], []);
-
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' 'runtime' 'php' 'one' 'two'";
-        static::assertSame($cmd, $process->getCommandLine());
+        $process = $this->factory()->create(new ProcessInput('php', ['one', 'two'], __DIR__, [], []));
+
+        $expected = $this->expectedCommand(
+            ['--network', 'none'],
+            ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'],
+            ['php', 'one', 'two'],
+        );
+        static::assertSame($expected, $process->getCommandLine());
         static::assertSame('/docker-dir', $process->getWorkingDirectory());
+        static::assertSame(35.0, $process->getTimeout());
     }
 
-    public function testCreateWithEnv(): void
+    public function testComposerGetsNetworkCacheAndNoPluginsOrScripts(): void
     {
-        $finder = $this->createMock(ExecutableFinder::class);
-        $finder->expects($this->once())
-            ->method('find')
-            ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
+        $process = $this->factory()->create(new ProcessInput('composer', ['install'], __DIR__, [], []));
+
+        $expected = $this->expectedCommand(
+            ['--network', 'bridge'],
+            ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution', '-v', '/composer/cache/dir:/tmp/composer'],
+            ['composer', 'install', '--no-plugins', '--no-scripts'],
+        );
+        static::assertSame($expected, $process->getCommandLine());
+    }
 
-        $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('php', ['one', 'two'], __DIR__, ['SOME_VAR' => 'value'], []);
+    public function testExposedPortsArePublishedOnLoopbackOnly(): void
+    {
+        $process = $this->factory()->create(new ProcessInput('php', [], __DIR__, [], [8000]));
+
+        $expected = $this->expectedCommand(
+            ['--network', 'bridge', '-p', '127.0.0.1:8000:8000'],
+            ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'],
+            ['php'],
+        );
+        static::assertSame($expected, $process->getCommandLine());
+    }
 
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'SOME_VAR=value' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' 'runtime' 'php' 'one' 'two'";
-        static::assertSame($cmd, $process->getCommandLine());
-        static::assertSame('/docker-dir', $process->getWorkingDirectory());
+    public function testCreateWithEnv(): void
+    {
+        $process = $this->factory()->create(new ProcessInput('php', [], __DIR__, ['SOME_VAR' => 'value'], []));
+
+        $expected = $this->expectedCommand(
+            ['--network', 'none'],
+            ['-e', 'SOME_VAR=value', '-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'],
+            ['php'],
+        );
+        static::assertSame($expected, $process->getCommandLine());
     }
 
     public function testWithInput(): void
     {
-        $finder = $this->createMock(ExecutableFinder::class);
-        $finder->expects($this->once())
-            ->method('find')
-            ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
-
-        $factory = new DockerProcessFactory('/composer-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('php', [], __DIR__, [], [], 'someinput');
+        $process = $this->factory()->create(new ProcessInput('php', [], __DIR__, [], [], 'someinput'));
 
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' 'runtime' 'php'";
-        static::assertSame($cmd, $process->getCommandLine());
         static::assertSame('someinput', $process->getInput());
     }
 
-    public function testSolutionDirectoryIsPassedAsEnvVar(): void
+    public function testCreateBuildProcess(): void
     {
-        $finder = $this->createMock(ExecutableFinder::class);
-        $finder->expects($this->once())
-            ->method('find')
-            ->with('docker')
-            ->willReturn('/usr/local/bin/docker');
-
-        $factory = new DockerProcessFactory('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder);
-        $input = new ProcessInput('php', ['one', 'two'], __DIR__, ['SOME_VAR' => 'value'], []);
+        $process = $this->factory()->createBuildProcess();
 
-        $process = $factory->create($input);
-        $cmd  = "'/usr/local/bin/docker' 'compose' '-p' 'php8appreciate' '-f' '.docker/runtime/docker-compose.yml'";
-        $cmd .= " 'run' '--user' '" . getmyuid() . ":" . getmygid() . "' '--rm' '-e' 'SOME_VAR=value' '-e' 'COMPOSER_HOME=/tmp/composer' '-w' '/solution' 'runtime' 'php' 'one' 'two'";
-        static::assertSame($cmd, $process->getCommandLine());
+        static::assertSame(
+            "'/usr/local/bin/docker' 'build' '-t' 'php8appreciate-runtime' '.docker/runtime'",
+            $process->getCommandLine(),
+        );
         static::assertSame('/docker-dir', $process->getWorkingDirectory());
-        static::assertSame(['SOLUTION' => __DIR__, 'UID' => (string) getmyuid(), 'GID' => (string) getmygid()], $process->getEnv());
     }
 }