Ver código fonte

fix: gate /api/docs and openapi.yaml behind API_DOCS_PUBLIC (SEC_REVIEW F68)

`/api/docs` (RapiDoc viewer) and `/api/v1/openapi.yaml` (the 48 KB
OpenAPI spec) were registered unconditionally. The spec exposes the
full route surface — admin endpoints, internal-job endpoints,
expected query/body shapes, error contracts — to any unauthenticated
caller. On a default deploy that's internet-reachable, this hands a
recon attacker the entire menu before they even start probing.

Add an `api_docs_public` setting (default false), driven by
`API_DOCS_PUBLIC=true|false` in the environment. `AppFactory::build`
now only registers the two routes when the flag is true; with the
default, both paths are never wired and Slim returns 404 like any
other unmapped path.

Operators who want the docs viewer (open APIs, dev environments) opt
in via `API_DOCS_PUBLIC=true`. `.env.example` documents the gate.

The Caddyfile's @docs-path CSP block is left in place — it only
takes effect for the 404 responses on the unregistered paths, which
is harmless. (Reshaping it to also gate on the env var would
duplicate the application-layer check; keeping things layered is
fine.)

Regression tests in
`api/tests/Integration/Public/DocsControllerTest.php`:

  - `testDocsPageIs404ByDefault` and `testOpenapiSpecIs404ByDefault`
    exercise the off state directly (the test fixture leaves
    `api_docs_public` at its default false).
  - The pre-existing F58 SRI tests
    (`testDocsPageEmbedsRapiDocWithSriIntegrity`,
    `testOpenapiSpecIsServed`) now route through a small
    `enableDocs()` helper that sets the container's
    `settings.api_docs_public` to true and rebuilds the app —
    mirroring the binding-override pattern
    `JobsAdminControllerTest::testRefreshGeoip412UnderMaxmindWithoutKey`
    already uses for the GeoIP downloader.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 dias atrás
pai
commit
df1a298c82

+ 8 - 0
.env.example

@@ -98,6 +98,14 @@ UI_ORIGIN=http://localhost:8080
 # Rate limiting (public API)
 API_RATE_LIMIT_PER_SECOND=60
 
+# Public API docs viewer.
+# `false` (default): /api/docs and /api/v1/openapi.yaml are not
+# registered — Slim returns 404 — so the OpenAPI spec doesn't leak
+# the full admin/internal route surface to unauthenticated callers.
+# Set `true` for open APIs / dev environments where the menu is
+# meant to be public. SEC_REVIEW F68.
+API_DOCS_PUBLIC=false
+
 # -----------------------------------------------------------------------------
 # ui container
 # -----------------------------------------------------------------------------

+ 8 - 0
api/config/settings.php

@@ -45,6 +45,14 @@ return [
     'internal_job_token' => getenv('INTERNAL_JOB_TOKEN') ?: '',
     'internal_cidr_allowlist' => (string) (getenv('INTERNAL_CIDR_ALLOWLIST') ?: ''),
     'ui_origin' => getenv('UI_ORIGIN') ?: 'http://localhost:8080',
+    // SEC_REVIEW F68: gate `/api/docs` and `/api/v1/openapi.yaml`
+    // behind an explicit env flag. The OpenAPI spec leaks the full
+    // surface (admin endpoints, internal-job endpoints, expected
+    // body shapes, error contracts); reachable-by-default would
+    // hand a recon attacker the entire menu. Operators who want
+    // public docs (open APIs, dev environments) set
+    // `API_DOCS_PUBLIC=true`. Default false → both routes 404.
+    'api_docs_public' => filter_var(getenv('API_DOCS_PUBLIC') ?: 'false', FILTER_VALIDATE_BOOL),
     'oidc_default_role' => $oidcDefaultRole,
     'score_hard_cutoff_days' => (int) (getenv('SCORE_REPORT_HARD_CUTOFF_DAYS') ?: 365),
     'rate_limit_per_second' => (int) (getenv('API_RATE_LIMIT_PER_SECOND') ?: 60),

+ 13 - 6
api/src/App/AppFactory.php

@@ -100,12 +100,19 @@ final class AppFactory
         /** @var InternalTokenMiddleware $internalToken */
         $internalToken = $container->get(InternalTokenMiddleware::class);
 
-        // Public docs (no auth, no rate limit). The viewer is HTML-only;
-        // the spec is the YAML file shipped with the api image.
-        /** @var DocsController $docs */
-        $docs = $container->get(DocsController::class);
-        $app->get('/api/v1/openapi.yaml', [$docs, 'spec']);
-        $app->get('/api/docs', [$docs, 'viewer']);
+        // Public docs viewer + OpenAPI spec. SEC_REVIEW F68: gated
+        // behind the `API_DOCS_PUBLIC` env flag (off by default) so
+        // a recon attacker can't pull the full admin/internal route
+        // surface from an unauthenticated endpoint. Operators who
+        // want public docs (open APIs, dev environments) set
+        // `API_DOCS_PUBLIC=true`; otherwise both routes are simply
+        // never registered, so they 404 like any other unmapped path.
+        if ((bool) $container->get('settings.api_docs_public')) {
+            /** @var DocsController $docs */
+            $docs = $container->get(DocsController::class);
+            $app->get('/api/v1/openapi.yaml', [$docs, 'spec']);
+            $app->get('/api/docs', [$docs, 'viewer']);
+        }
 
         $app->get('/healthz', function (ServerRequestInterface $request, ResponseInterface $response) use ($container): ResponseInterface {
             /** @var array{driver: string} $dbSettings */

+ 1 - 0
api/src/App/Container.php

@@ -131,6 +131,7 @@ final class Container
             'settings.rate_limit_per_second' => (int) ($settings['rate_limit_per_second'] ?? 60),
             'settings.internal_job_token' => (string) ($settings['internal_job_token'] ?? ''),
             'settings.internal_cidr_allowlist' => (string) ($settings['internal_cidr_allowlist'] ?? ''),
+            'settings.api_docs_public' => (bool) ($settings['api_docs_public'] ?? false),
             'settings.job_recompute_max_runtime_seconds' => (int) ($settings['job_recompute_max_runtime_seconds'] ?? 240),
             'settings.job_recompute_max_rows_per_tick' => (int) ($settings['job_recompute_max_rows_per_tick'] ?? 5000),
             'settings.job_audit_retention_days' => (int) ($settings['job_audit_retention_days'] ?? 180),

+ 40 - 0
api/tests/Integration/Public/DocsControllerTest.php

@@ -12,11 +12,18 @@ use App\Tests\Integration\Support\AppTestCase;
  * `crossorigin="anonymous"` so the browser refuses to execute the JS
  * if a CDN compromise or in-flight content modification serves
  * different bytes.
+ *
+ * Also covers SEC_REVIEW F68 — the docs routes are gated behind
+ * `API_DOCS_PUBLIC` (default off). Tests that reach the routes
+ * enable the flag and rebuild the app; the dedicated F68 cases
+ * leave it off and assert 404.
  */
 final class DocsControllerTest extends AppTestCase
 {
     public function testDocsPageEmbedsRapiDocWithSriIntegrity(): void
     {
+        $this->enableDocs();
+
         $resp = $this->request('GET', '/api/docs');
         self::assertSame(200, $resp->getStatusCode());
         self::assertStringContainsString('text/html', $resp->getHeaderLine('Content-Type'));
@@ -41,6 +48,7 @@ final class DocsControllerTest extends AppTestCase
     {
         // Sanity check that we didn't break the spec endpoint while
         // editing the controller.
+        $this->enableDocs();
         $resp = $this->request('GET', '/api/v1/openapi.yaml');
         // The spec file isn't shipped in tests; either 200 with content
         // or 500 with `spec_unavailable`. Both are acceptable here —
@@ -48,4 +56,36 @@ final class DocsControllerTest extends AppTestCase
         // edited controller.
         self::assertContains($resp->getStatusCode(), [200, 500]);
     }
+
+    public function testDocsPageIs404ByDefault(): void
+    {
+        // SEC_REVIEW F68: `/api/docs` is gated behind
+        // `API_DOCS_PUBLIC`. AppTestCase builds the container with
+        // the env var unset, so the default (false) applies and the
+        // route is never registered → Slim returns 404.
+        $resp = $this->request('GET', '/api/docs');
+        self::assertSame(404, $resp->getStatusCode());
+    }
+
+    public function testOpenapiSpecIs404ByDefault(): void
+    {
+        $resp = $this->request('GET', '/api/v1/openapi.yaml');
+        self::assertSame(404, $resp->getStatusCode());
+    }
+
+    /**
+     * Re-bind `settings.api_docs_public` to `true` and rebuild the app
+     * so the docs routes are registered. Same pattern
+     * `JobsAdminControllerTest::testRefreshGeoip412UnderMaxmindWithoutKey`
+     * uses to swap a binding mid-test.
+     */
+    private function enableDocs(): void
+    {
+        if (method_exists($this->container, 'set')) {
+            /** @var \DI\Container $c */
+            $c = $this->container;
+            $c->set('settings.api_docs_public', true);
+            $this->app = \App\App\AppFactory::build($this->container);
+        }
+    }
 }