Bläddra i källkod

fix: validate INTERNAL_JOB_TOKEN entropy at api boot (SEC_REVIEW F35)

`InternalTokenMiddleware` compares with `hash_equals` (correct) and
fail-closes on an empty token (also correct), but accepted any
non-empty value at runtime. A deploy with `INTERNAL_JOB_TOKEN=foo`
booted cleanly. Combined with F25 (overly permissive network gate),
a docker-network neighbour could brute-force a weak shared secret
and reach `/internal/*`.

Add `App\App\Config::validateOrExit()` (mirrors the ui's
`App\App\Config::validateOrExit`) and call it from `api/public/index.php`
before `Container::build()`. The api now refuses to boot unless
`INTERNAL_JOB_TOKEN` matches `^[0-9a-fA-F]{32,}$` — 128 bits of
entropy floor; the `.env.example` documents 64 hex chars from
`openssl rand -hex 32` and that stays the recommendation.

Misconfigurations now crash on `docker compose up` with a clear
STDERR message instead of silently serving an unsafe surface to the
first internal request. Tests are unaffected: `AppTestCase`
constructs the container directly via `Container::build($settings)`
with whatever fixtures it likes, bypassing the index.php boot path.
The middleware's own empty-string fail-closed branch stays in place
as defence-in-depth for that test path and for any future call site
that builds the container outside `public/index.php`.

Regression tests in `api/tests/Unit/App/ConfigTest.php` cover empty,
missing-key, short-hex, non-hex, the literal 'foo' from the review,
exactly-32-hex, the documented 64-hex form, and uppercase-hex. A
subprocess test asserts `validateOrExit()` writes 'INTERNAL_JOB_TOKEN'
to STDERR and exits with code 1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 4 dagar sedan
förälder
incheckning
d39ab01a7c
4 ändrade filer med 194 tillägg och 2 borttagningar
  1. 1 1
      .env.example
  2. 6 1
      api/public/index.php
  3. 77 0
      api/src/App/Config.php
  4. 110 0
      api/tests/Unit/App/ConfigTest.php

+ 1 - 1
.env.example

@@ -39,7 +39,7 @@ SCORE_RECOMPUTE_INTERVAL_SECONDS=300
 SCORE_REPORT_HARD_CUTOFF_DAYS=365
 
 # Internal jobs
-INTERNAL_JOB_TOKEN=                       # 32-byte hex
+INTERNAL_JOB_TOKEN=                       # 32-byte hex (api refuses to boot if shorter than 32 hex chars)
 # Comma- or whitespace-separated CIDR list of sources allowed to reach
 # /internal/*. Empty (the default) means loopback-only (127.0.0.1/32 +
 # ::1/128). The bundled `compose.scheduler.yml` shares the api's network

+ 6 - 1
api/public/index.php

@@ -3,9 +3,14 @@
 declare(strict_types=1);
 
 use App\App\AppFactory;
+use App\App\Config;
 use App\App\Container;
 
 require __DIR__ . '/../vendor/autoload.php';
 
-$container = Container::build();
+/** @var array<string, mixed> $settings */
+$settings = require __DIR__ . '/../config/settings.php';
+// SEC_REVIEW F35: refuse to boot on a misconfigured INTERNAL_JOB_TOKEN.
+Config::validateOrExit($settings);
+$container = Container::build($settings);
 AppFactory::build($container)->run();

+ 77 - 0
api/src/App/Config.php

@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\App;
+
+/**
+ * Startup-time configuration validation for the api container.
+ *
+ * Mirrors the ui's `App\App\Config::validateOrExit`. Runs before
+ * `Container::build()` from `public/index.php` so a misconfigured deploy
+ * crashes on `docker compose up` rather than serving an unsafe surface
+ * to the first request.
+ *
+ * SEC_REVIEW F35: `INTERNAL_JOB_TOKEN` must have at least 32 hex chars
+ * of entropy. The `/internal/*` middleware compares with `hash_equals`
+ * which is correct, but a misconfigured value like `INTERNAL_JOB_TOKEN=foo`
+ * was previously accepted. Combined with F25 (network gate too permissive),
+ * a docker-network neighbour could brute-force a weak token. Boot-time
+ * validation closes the door at deploy time.
+ */
+final class Config
+{
+    /**
+     * Minimum number of hex characters required in `INTERNAL_JOB_TOKEN`.
+     * 32 hex chars = 128 bits of entropy. The documented format
+     * (`openssl rand -hex 32`) produces 64 hex chars; this is the floor.
+     */
+    public const INTERNAL_JOB_TOKEN_MIN_HEX_CHARS = 32;
+
+    /**
+     * @param array<string, mixed> $settings
+     */
+    public static function validateOrExit(array $settings): void
+    {
+        $errors = self::collectErrors($settings);
+        if ($errors === []) {
+            return;
+        }
+
+        fwrite(\STDERR, "[api] startup configuration error(s):\n");
+        foreach ($errors as $err) {
+            fwrite(\STDERR, "  - {$err}\n");
+        }
+        exit(1);
+    }
+
+    /**
+     * Pure validation — returns a list of human-readable error messages.
+     * Public so unit tests can exercise it without spawning a process.
+     *
+     * @param array<string, mixed> $settings
+     * @return list<string>
+     */
+    public static function collectErrors(array $settings): array
+    {
+        $errors = [];
+
+        $token = (string) ($settings['internal_job_token'] ?? '');
+        if (!self::isValidInternalJobToken($token)) {
+            $errors[] = sprintf(
+                'INTERNAL_JOB_TOKEN must be at least %d hex chars (generate with: openssl rand -hex 32)',
+                self::INTERNAL_JOB_TOKEN_MIN_HEX_CHARS,
+            );
+        }
+
+        return $errors;
+    }
+
+    private static function isValidInternalJobToken(string $token): bool
+    {
+        return preg_match(
+            sprintf('/^[0-9a-fA-F]{%d,}$/', self::INTERNAL_JOB_TOKEN_MIN_HEX_CHARS),
+            $token,
+        ) === 1;
+    }
+}

+ 110 - 0
api/tests/Unit/App/ConfigTest.php

@@ -0,0 +1,110 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Unit\App;
+
+use App\App\Config;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * SEC_REVIEW F35 — `INTERNAL_JOB_TOKEN` startup validation.
+ */
+final class ConfigTest extends TestCase
+{
+    public function testEmptyTokenIsRejected(): void
+    {
+        $errors = Config::collectErrors(['internal_job_token' => '']);
+        self::assertNotEmpty($errors);
+        self::assertStringContainsString('INTERNAL_JOB_TOKEN', $errors[0]);
+    }
+
+    public function testMissingKeyIsRejected(): void
+    {
+        $errors = Config::collectErrors([]);
+        self::assertNotEmpty($errors);
+        self::assertStringContainsString('INTERNAL_JOB_TOKEN', $errors[0]);
+    }
+
+    public function testShortHexIsRejected(): void
+    {
+        $errors = Config::collectErrors(['internal_job_token' => str_repeat('a', 31)]);
+        self::assertNotEmpty($errors);
+    }
+
+    public function testNonHexIsRejected(): void
+    {
+        // 32+ chars but contains non-hex — caught by the charset rule.
+        $errors = Config::collectErrors([
+            'internal_job_token' => 'this-token-is-32-plus-chars-but-not-hex-at-all',
+        ]);
+        self::assertNotEmpty($errors);
+    }
+
+    public function testWeakLiteralFooIsRejected(): void
+    {
+        // The exact case the SEC_REVIEW called out.
+        $errors = Config::collectErrors(['internal_job_token' => 'foo']);
+        self::assertNotEmpty($errors);
+    }
+
+    public function testThirtyTwoHexCharsAccepted(): void
+    {
+        $errors = Config::collectErrors([
+            'internal_job_token' => str_repeat('a', 32),
+        ]);
+        self::assertSame([], $errors);
+    }
+
+    public function testSixtyFourHexCharsAcceptedDocumentedForm(): void
+    {
+        // openssl rand -hex 32 → 64 hex chars; the documented form.
+        $errors = Config::collectErrors([
+            'internal_job_token' => bin2hex(random_bytes(32)),
+        ]);
+        self::assertSame([], $errors);
+    }
+
+    public function testUppercaseHexAccepted(): void
+    {
+        $errors = Config::collectErrors([
+            'internal_job_token' => str_repeat('A', 32),
+        ]);
+        self::assertSame([], $errors);
+    }
+
+    public function testValidateOrExitWritesToStderrAndExits(): void
+    {
+        // Run the failing branch in a subprocess so we can observe both
+        // the non-zero exit and the STDERR text without taking down PHPUnit.
+        $script = <<<'PHP'
+<?php
+declare(strict_types=1);
+require %s;
+App\App\Config::validateOrExit(['internal_job_token' => 'too-short']);
+PHP;
+        $autoload = var_export(__DIR__ . '/../../../vendor/autoload.php', true);
+        $script = sprintf($script, $autoload);
+
+        $tmp = tempnam(sys_get_temp_dir(), 'cfg');
+        self::assertIsString($tmp);
+        file_put_contents($tmp, $script);
+
+        $stderr = '';
+        $process = proc_open(
+            [PHP_BINARY, $tmp],
+            [1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
+            $pipes,
+        );
+        self::assertIsResource($process);
+        fclose($pipes[1]);
+        $stderr = (string) stream_get_contents($pipes[2]);
+        fclose($pipes[2]);
+        $exitCode = proc_close($process);
+
+        @unlink($tmp);
+
+        self::assertSame(1, $exitCode);
+        self::assertStringContainsString('INTERNAL_JOB_TOKEN', $stderr);
+    }
+}