From a585e5a812600e34a84db16c025f91d97f8536b6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 20:17:23 +0000 Subject: [PATCH 1/4] fix: require executor key for runtime command execution POST /v1/runtimes/:runtimeId/commands was registered without the api group, so the OPR_EXECUTOR_SECRET init hook never ran and the route accepted unauthenticated docker-exec into any runtime. Co-authored-by: Jake Barnby --- app/controllers.php | 1 + tests/e2e/ExecutorTest.php | 11 +++++++ tests/unit/Http/RouteAuthTest.php | 49 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 tests/unit/Http/RouteAuthTest.php diff --git a/app/controllers.php b/app/controllers.php index 12208bd..cb7e5e5 100644 --- a/app/controllers.php +++ b/app/controllers.php @@ -37,6 +37,7 @@ }); Http::post('/v1/runtimes/:runtimeId/commands') + ->groups(['api', 'runtimes']) ->desc('Execute a command inside an existing runtime') ->param('runtimeId', '', new Text(64), 'Unique runtime ID.') ->param('command', '', new Text(1024), 'Command to execute.') diff --git a/tests/e2e/ExecutorTest.php b/tests/e2e/ExecutorTest.php index 7db4b58..73b39f6 100644 --- a/tests/e2e/ExecutorTest.php +++ b/tests/e2e/ExecutorTest.php @@ -250,6 +250,17 @@ public function testGetRuntimesUnauthorized(): void $this->client->setKey($this->key); } + public function testCommandsUnauthorized(): void + { + $this->client->setKey(''); + $response = $this->client->call(Client::METHOD_POST, '/runtimes/test-commands-unauth/commands', [], [ + 'command' => 'true', + ]); + $this->assertEquals(401, $response['headers']['status-code']); + $this->assertEquals('Missing executor key', $response['body']['message']); + $this->client->setKey($this->key); + } + public function testBuild(): void { $output = ''; diff --git a/tests/unit/Http/RouteAuthTest.php b/tests/unit/Http/RouteAuthTest.php new file mode 100644 index 0000000..4777a08 --- /dev/null +++ b/tests/unit/Http/RouteAuthTest.php @@ -0,0 +1,49 @@ +assertNotEmpty($matches, 'Expected HTTP routes in app/controllers.php'); + + $paths = []; + foreach ($matches as $match) { + $path = $match[1]; + $body = $match[2]; + $paths[] = $path; + + if ($path === '/v1/health') { + $this->assertDoesNotMatchRegularExpression( + "/->groups\\(\\[[^\\]]*?'api'/", + $body, + 'Health must stay public for Docker healthchecks' + ); + continue; + } + + $this->assertMatchesRegularExpression( + "/->groups\\(\\[[^\\]]*?'api'/", + $body, + "Route {$path} must declare the api group so the executor secret check applies" + ); + } + + $this->assertContains('/v1/runtimes/:runtimeId/commands', $paths); + $this->assertContains('/v1/health', $paths); + } +} From 670b28a9c66ee7c756a9433feb985474a998f587 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 20:18:06 +0000 Subject: [PATCH 2/4] test: fail closed if controllers.php cannot be read Co-authored-by: Jake Barnby --- tests/unit/Http/RouteAuthTest.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/unit/Http/RouteAuthTest.php b/tests/unit/Http/RouteAuthTest.php index 4777a08..41c5987 100644 --- a/tests/unit/Http/RouteAuthTest.php +++ b/tests/unit/Http/RouteAuthTest.php @@ -10,8 +10,13 @@ final class RouteAuthTest extends TestCase { public function testNonPublicRoutesDeclareApiGroup(): void { - $source = (string) file_get_contents(\dirname(__DIR__, 3) . '/app/controllers.php'); + $controllers = \dirname(__DIR__, 3) . '/app/controllers.php'; + $source = \file_get_contents($controllers); + if ($source === false) { + $this->fail('Unable to read app/controllers.php'); + } + $matches = []; \preg_match_all( "/Http::(?:get|post|put|patch|delete)\\('([^']+)'\\)(.*?)(?=Http::(?:get|post|put|patch|delete|init|error)\\()/s", $source, @@ -23,8 +28,8 @@ public function testNonPublicRoutesDeclareApiGroup(): void $paths = []; foreach ($matches as $match) { - $path = $match[1]; - $body = $match[2]; + $path = $match[1] ?? ''; + $body = $match[2] ?? ''; $paths[] = $path; if ($path === '/v1/health') { From 1ed10f3d7c67dfa5751bd8896203892524522569 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 20:27:39 +0000 Subject: [PATCH 3/4] test: satisfy phpstan and rector on the route auth guard Co-authored-by: Jake Barnby --- tests/unit/Http/RouteAuthTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/Http/RouteAuthTest.php b/tests/unit/Http/RouteAuthTest.php index 41c5987..ab54cb2 100644 --- a/tests/unit/Http/RouteAuthTest.php +++ b/tests/unit/Http/RouteAuthTest.php @@ -28,8 +28,8 @@ public function testNonPublicRoutesDeclareApiGroup(): void $paths = []; foreach ($matches as $match) { - $path = $match[1] ?? ''; - $body = $match[2] ?? ''; + $path = $match[1]; + $body = $match[2]; $paths[] = $path; if ($path === '/v1/health') { @@ -44,7 +44,7 @@ public function testNonPublicRoutesDeclareApiGroup(): void $this->assertMatchesRegularExpression( "/->groups\\(\\[[^\\]]*?'api'/", $body, - "Route {$path} must declare the api group so the executor secret check applies" + \sprintf('Route %s must declare the api group so the executor secret check applies', $path) ); } From 0aa3cdd249d8d93b49a221e377fc5c1c5a2d5679 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 5 Oct 2026 17:30:37 +1300 Subject: [PATCH 4/4] Fix regex to handle end of input in RouteAuthTest Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/unit/Http/RouteAuthTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/Http/RouteAuthTest.php b/tests/unit/Http/RouteAuthTest.php index ab54cb2..4cc69d9 100644 --- a/tests/unit/Http/RouteAuthTest.php +++ b/tests/unit/Http/RouteAuthTest.php @@ -18,7 +18,7 @@ public function testNonPublicRoutesDeclareApiGroup(): void $matches = []; \preg_match_all( - "/Http::(?:get|post|put|patch|delete)\\('([^']+)'\\)(.*?)(?=Http::(?:get|post|put|patch|delete|init|error)\\()/s", + "/Http::(?:get|post|put|patch|delete)\\('([^']+)'\\)(.*?)(?=Http::(?:get|post|put|patch|delete|init|error)\\(|\\z)/s", $source, $matches, PREG_SET_ORDER