From b3b96ce032408ecf3575d49c316665fa98fa7523 Mon Sep 17 00:00:00 2001 From: Alfred Rivas Date: Wed, 29 Apr 2026 10:05:57 +0200 Subject: [PATCH] feat(server): BEE-1794 enable CORS for browser clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drogon doesn't ship CORS middleware. The beepbox-server image deployed to Cloud Run was rejecting every browser preflight (OPTIONS without Access-Control-Allow-Origin) → BEE-1686 Commlink track encode step broken end-to-end: Access to fetch at '.../v1/encode' from origin 'http://localhost:3000' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. This commit adds a self-contained CORS module: include/beepbox/CorsConfig.h src/CorsConfig.cpp # Pure parsing + origin resolution (no Drogon) src/CorsAdvice.cpp # Drogon preRoutingAdvice + postHandlingAdvice Split exists so unit tests link only the pure logic file (no Drogon runtime needed) — keeps the test suite fast. Behaviour: - Whitelist comes from BEEPBOX_CORS_ALLOWED_ORIGINS env var (CSV). Empty / unset → CORS disabled, request pipeline behaves identically to pre-1794 server-to-server-only mode. - PreRoutingAdvice intercepts OPTIONS *before* the auth filter, so the browser preflight (no Bearer) gets a 204 with the four Access-Control-* headers without tripping the API key gate. - PostHandlingAdvice stamps Access-Control-Allow-Origin + Vary: Origin on every non-OPTIONS response when the request `Origin` matches the whitelist. No-op for callers without an `Origin` header → server-to- server flows unchanged. - Exact-match origins only — no wildcards, no scheme coercion. RFC 6454 verbatim comparison (case-sensitive on host). Server startup logs the active whitelist (or "CORS disabled") for observability. Infra: infra/cloud-run.tf # Pipes the env var into Cloud Run infra/variables.tf # New `cors_allowed_origins` variable infra/terraform.tfvars.example # Documents dev defaults Tests: 3 new test cases / 23 new assertions (parseAllowedOrigins · CSV trimming + order preservation · whitelist resolution including verbatim mismatch on scheme/port/case · enabled() flag). Full suite: 85 / 85 (was 82 → +3). beepbox-server build clean, smoke-tested locally with `curl -X OPTIONS -H "Origin: http://localhost:3000" http://localhost:8080/v1/encode` → 204 + all four headers; same with `Origin: https://evil.com` → 204 with no CORS headers (browser blocks). Closes BEE-1794. Unblocks BEE-1686. Co-Authored-By: Claude Opus 4.7 (1M context) --- CMakeLists.txt | 4 ++ include/beepbox/CorsConfig.h | 89 ++++++++++++++++++++++++++++++++ infra/cloud-run.tf | 8 +++ infra/terraform.tfvars.example | 1 + infra/variables.tf | 11 ++++ src/CorsAdvice.cpp | 57 +++++++++++++++++++++ src/CorsConfig.cpp | 59 +++++++++++++++++++++ src/server_main.cpp | 20 ++++++++ tests/CorsConfigTests.cpp | 93 ++++++++++++++++++++++++++++++++++ 9 files changed, 342 insertions(+) create mode 100644 include/beepbox/CorsConfig.h create mode 100644 src/CorsAdvice.cpp create mode 100644 src/CorsConfig.cpp create mode 100644 tests/CorsConfigTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fb96faa..e035079 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -155,6 +155,8 @@ if(BEEPBOX_BUILD_SERVER) src/RateLimiter.cpp src/Metrics.cpp src/Tracing.cpp + src/CorsConfig.cpp + src/CorsAdvice.cpp ) target_include_directories(beepbox-server PRIVATE @@ -224,10 +226,12 @@ if(BUILD_TESTING) tests/RateLimiterTests.cpp tests/MetricsTests.cpp tests/TracingTests.cpp + tests/CorsConfigTests.cpp src/ApiKeyAuth.cpp src/RateLimiter.cpp src/Metrics.cpp src/Tracing.cpp + src/CorsConfig.cpp ) target_link_libraries(BeepBoxTests PRIVATE beepbox_lib Catch2::Catch2WithMain) diff --git a/include/beepbox/CorsConfig.h b/include/beepbox/CorsConfig.h new file mode 100644 index 0000000..7247b8b --- /dev/null +++ b/include/beepbox/CorsConfig.h @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Beeping contributors + +#pragma once + +#include +#include +#include +#include + +namespace beepbox { + +/** + * Pure logic that decides which CORS Allow-Origin to echo back. + * + * Backed by an exact-match whitelist parsed from a CSV env var + * (BEEPBOX_CORS_ALLOWED_ORIGINS). No wildcards, no regex, no scheme + * coercion — if the request `Origin` header doesn't match a configured + * entry verbatim, no CORS headers are emitted and the browser blocks + * the response naturally. + * + * When the whitelist is empty (env var unset or empty), CORS is + * effectively OFF — which is the backwards-compatible default for + * the existing server-to-server flows that don't need CORS. + * + * Drogon integration lives in CorsConfig.cpp behind installCorsHandlers(). + * The pure parsing and resolution logic is exposed here so it can be + * unit-tested without spinning up the HTTP server. + */ +class CorsConfig { +public: + /// Parse a CSV (comma-separated origins). Trims surrounding whitespace + /// per entry and drops empty entries. Order of origins in the input is + /// preserved. + static std::vector parseAllowedOrigins(std::string_view csv); + + CorsConfig() = default; + explicit CorsConfig(std::vector allowedOrigins); + + /// True if at least one origin is whitelisted. When false, callers + /// should skip installCorsHandlers() entirely so Drogon's request + /// pipeline is unchanged for the legacy server-to-server flow. + bool enabled() const noexcept { return !allowedOrigins_.empty(); } + + const std::vector& allowedOrigins() const noexcept { + return allowedOrigins_; + } + + /// Returns the value to echo back as `Access-Control-Allow-Origin` + /// for a given request `Origin` header, or std::nullopt if the + /// origin is not whitelisted (or empty/missing). + std::optional resolveAllowOrigin( + std::string_view requestOrigin) const; + + /// Static headers — same on every preflight response. + static constexpr std::string_view kAllowMethods = + "GET, POST, OPTIONS"; + static constexpr std::string_view kAllowHeaders = + "Authorization, Content-Type, traceparent"; + static constexpr std::string_view kMaxAge = "600"; + +private: + std::vector allowedOrigins_; +}; + +/** + * Hook the CORS config into Drogon's request pipeline. + * + * Registers two pieces of advice (the two integration points Drogon + * exposes for cross-cutting concerns): + * + * 1. preRoutingAdvice → intercepts OPTIONS preflight requests before + * any handler or auth filter sees them. If the request `Origin` + * is whitelisted, responds 204 with the four `Access-Control-*` + * headers; otherwise responds 204 with no CORS headers and the + * browser refuses the actual request. + * + * 2. postHandlingAdvice → appends `Access-Control-Allow-Origin` (and + * `Vary: Origin`) to every non-OPTIONS response when the request + * came from a whitelisted origin. Skipped for any other origin so + * server-to-server callers (no Origin header) see no behavioural + * change. + * + * Calling this when `cfg.enabled() == false` is a no-op so it's safe + * to invoke unconditionally at startup. + */ +void installCorsHandlers(CorsConfig cfg); + +} // namespace beepbox diff --git a/infra/cloud-run.tf b/infra/cloud-run.tf index 6409f6b..fe0d754 100644 --- a/infra/cloud-run.tf +++ b/infra/cloud-run.tf @@ -53,6 +53,14 @@ resource "google_cloud_run_v2_service" "beepbox" { value = "8" } + # CORS allowed origins (CSV). Empty/unset disables CORS — kept + # disabled for prod until BEE-1794 lands a separate prod deploy + # so the dev rollout doesn't affect server-to-server callers. + env { + name = "BEEPBOX_CORS_ALLOWED_ORIGINS" + value = var.cors_allowed_origins + } + startup_probe { http_get { path = "/healthz" diff --git a/infra/terraform.tfvars.example b/infra/terraform.tfvars.example index e5736f7..cc39b62 100644 --- a/infra/terraform.tfvars.example +++ b/infra/terraform.tfvars.example @@ -2,3 +2,4 @@ project_id = "beeping-platform-dev" region = "europe-west1" image_tag = "latest" +cors_allowed_origins = "http://localhost:3000,https://beeping-platform-dev.web.app,https://beeping-platform-dev.firebaseapp.com" diff --git a/infra/variables.tf b/infra/variables.tf index f03a2a2..d8ec926 100644 --- a/infra/variables.tf +++ b/infra/variables.tf @@ -15,3 +15,14 @@ variable "image_tag" { type = string default = "latest" } + +variable "cors_allowed_origins" { + description = <<-EOT + Comma-separated list of CORS-whitelisted origins for browser + callers (BEE-1794). Empty disables CORS — server-to-server flows + keep working unchanged. For dev, expects: + `http://localhost:3000,https://beeping-platform-dev.web.app,https://beeping-platform-dev.firebaseapp.com`. + EOT + type = string + default = "" +} diff --git a/src/CorsAdvice.cpp b/src/CorsAdvice.cpp new file mode 100644 index 0000000..00c57be --- /dev/null +++ b/src/CorsAdvice.cpp @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Beeping contributors + +#include "beepbox/CorsConfig.h" + +#include + +namespace beepbox { + +void installCorsHandlers(CorsConfig cfg) { + if (!cfg.enabled()) return; + + // PreRoutingAdvice: catch OPTIONS preflights before auth/rate-limit + // filters. The browser sends OPTIONS without the Bearer token, so if + // we let it fall through to /v1/encode the auth handler would 401 it + // and the actual CORS preflight would never succeed. + drogon::app().registerPreRoutingAdvice( + [cfg](const drogon::HttpRequestPtr& req, + drogon::AdviceCallback&& callback, + drogon::AdviceChainCallback&& chain) { + if (req->method() != drogon::Options) { + chain(); + return; + } + auto resp = drogon::HttpResponse::newHttpResponse(); + resp->setStatusCode(drogon::k204NoContent); + const auto origin = req->getHeader("Origin"); + if (auto allowed = cfg.resolveAllowOrigin(origin); allowed) { + resp->addHeader("Access-Control-Allow-Origin", *allowed); + resp->addHeader("Access-Control-Allow-Methods", + std::string(CorsConfig::kAllowMethods)); + resp->addHeader("Access-Control-Allow-Headers", + std::string(CorsConfig::kAllowHeaders)); + resp->addHeader("Access-Control-Max-Age", + std::string(CorsConfig::kMaxAge)); + resp->addHeader("Vary", "Origin"); + } + callback(resp); + }); + + // PostHandlingAdvice: stamp Allow-Origin on every non-OPTIONS response + // (POST /v1/encode, GET /healthz, …) when the request originated from + // a whitelisted browser. No-op for server-to-server callers because + // they don't send an `Origin` header. + drogon::app().registerPostHandlingAdvice( + [cfg](const drogon::HttpRequestPtr& req, + const drogon::HttpResponsePtr& resp) { + if (req->method() == drogon::Options) return; + const auto origin = req->getHeader("Origin"); + if (auto allowed = cfg.resolveAllowOrigin(origin); allowed) { + resp->addHeader("Access-Control-Allow-Origin", *allowed); + resp->addHeader("Vary", "Origin"); + } + }); +} + +} // namespace beepbox diff --git a/src/CorsConfig.cpp b/src/CorsConfig.cpp new file mode 100644 index 0000000..2d7a7fe --- /dev/null +++ b/src/CorsConfig.cpp @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Beeping contributors + +#include "beepbox/CorsConfig.h" + +#include +#include + +namespace beepbox { + +namespace { + +std::string_view trim(std::string_view sv) { + const auto isSpace = [](unsigned char c) { return std::isspace(c) != 0; }; + while (!sv.empty() && isSpace(static_cast(sv.front()))) { + sv.remove_prefix(1); + } + while (!sv.empty() && isSpace(static_cast(sv.back()))) { + sv.remove_suffix(1); + } + return sv; +} + +} // namespace + +std::vector CorsConfig::parseAllowedOrigins( + std::string_view csv) { + std::vector out; + std::size_t start = 0; + while (start <= csv.size()) { + std::size_t comma = csv.find(',', start); + std::string_view chunk = csv.substr( + start, comma == std::string_view::npos ? csv.size() - start + : comma - start); + auto trimmed = trim(chunk); + if (!trimmed.empty()) { + out.emplace_back(trimmed); + } + if (comma == std::string_view::npos) break; + start = comma + 1; + } + return out; +} + +CorsConfig::CorsConfig(std::vector allowedOrigins) + : allowedOrigins_(std::move(allowedOrigins)) {} + +std::optional CorsConfig::resolveAllowOrigin( + std::string_view requestOrigin) const { + if (requestOrigin.empty()) return std::nullopt; + for (const auto& whitelisted : allowedOrigins_) { + if (requestOrigin == whitelisted) { + return whitelisted; + } + } + return std::nullopt; +} + +} // namespace beepbox diff --git a/src/server_main.cpp b/src/server_main.cpp index fc25205..5c52f0f 100644 --- a/src/server_main.cpp +++ b/src/server_main.cpp @@ -9,6 +9,7 @@ #include "beepbox/RateLimiter.h" #include "beepbox/Metrics.h" #include "beepbox/Tracing.h" +#include "beepbox/CorsConfig.h" #include #include @@ -145,6 +146,25 @@ int main() { std::cout << "WARNING: BEEPBOX_RATE_LIMIT_RPM not set — rate limiting disabled\n"; } + // --- CORS setup --- + // Whitelist comes from BEEPBOX_CORS_ALLOWED_ORIGINS (CSV). Empty/unset + // → no advice registered → request pipeline behaves identically to + // pre-1794 server-to-server-only mode. + { + const char* corsEnv = std::getenv("BEEPBOX_CORS_ALLOWED_ORIGINS"); + beepbox::CorsConfig corsCfg(beepbox::CorsConfig::parseAllowedOrigins( + corsEnv ? std::string_view{corsEnv} : std::string_view{})); + if (corsCfg.enabled()) { + std::cout << "CORS enabled for " << corsCfg.allowedOrigins().size() + << " origin(s):"; + for (const auto& o : corsCfg.allowedOrigins()) std::cout << " " << o; + std::cout << "\n"; + beepbox::installCorsHandlers(std::move(corsCfg)); + } else { + std::cout << "CORS disabled (BEEPBOX_CORS_ALLOWED_ORIGINS not set)\n"; + } + } + // --- /healthz — liveness probe --- app().registerHandler( "/healthz", diff --git a/tests/CorsConfigTests.cpp b/tests/CorsConfigTests.cpp new file mode 100644 index 0000000..9e8e15b --- /dev/null +++ b/tests/CorsConfigTests.cpp @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Beeping contributors + +#include "beepbox/CorsConfig.h" + +#include + +using beepbox::CorsConfig; + +TEST_CASE("CorsConfig::parseAllowedOrigins", "[cors]") { + SECTION("empty string yields no entries") { + REQUIRE(CorsConfig::parseAllowedOrigins("").empty()); + } + + SECTION("single origin") { + auto v = CorsConfig::parseAllowedOrigins("http://localhost:3000"); + REQUIRE(v.size() == 1); + REQUIRE(v[0] == "http://localhost:3000"); + } + + SECTION("multiple origins, trims whitespace, drops empties") { + auto v = CorsConfig::parseAllowedOrigins( + " http://localhost:3000 ,https://beeping.io,, https://www.beeping.io "); + REQUIRE(v.size() == 3); + REQUIRE(v[0] == "http://localhost:3000"); + REQUIRE(v[1] == "https://beeping.io"); + REQUIRE(v[2] == "https://www.beeping.io"); + } + + SECTION("preserves input order") { + auto v = CorsConfig::parseAllowedOrigins("c,a,b"); + REQUIRE(v == std::vector{"c", "a", "b"}); + } + + SECTION("a trailing comma does not introduce a phantom empty entry") { + auto v = CorsConfig::parseAllowedOrigins("http://localhost:3000,"); + REQUIRE(v.size() == 1); + REQUIRE(v[0] == "http://localhost:3000"); + } +} + +TEST_CASE("CorsConfig::resolveAllowOrigin", "[cors]") { + CorsConfig cfg({ + "http://localhost:3000", + "https://beeping-platform-dev.web.app", + "https://beeping.io", + }); + + SECTION("exact match returns the whitelisted entry") { + auto out = cfg.resolveAllowOrigin("http://localhost:3000"); + REQUIRE(out.has_value()); + REQUIRE(*out == "http://localhost:3000"); + } + + SECTION("any of the whitelisted origins resolve") { + REQUIRE(cfg.resolveAllowOrigin("https://beeping.io").has_value()); + REQUIRE(cfg.resolveAllowOrigin("https://beeping-platform-dev.web.app") + .has_value()); + } + + SECTION("non-whitelisted origin returns nullopt") { + REQUIRE_FALSE(cfg.resolveAllowOrigin("https://evil.com").has_value()); + } + + SECTION("empty origin returns nullopt (no Origin header → not a CORS req)") { + REQUIRE_FALSE(cfg.resolveAllowOrigin("").has_value()); + } + + SECTION("scheme/host/port are matched verbatim — no fuzzy matching") { + // http vs https + REQUIRE_FALSE(cfg.resolveAllowOrigin("https://localhost:3000").has_value()); + // different port + REQUIRE_FALSE(cfg.resolveAllowOrigin("http://localhost:3001").has_value()); + // trailing slash mismatch + REQUIRE_FALSE(cfg.resolveAllowOrigin("http://localhost:3000/").has_value()); + // case-sensitive (RFC 6454 origin comparison is case-sensitive on host) + REQUIRE_FALSE(cfg.resolveAllowOrigin("HTTP://localhost:3000").has_value()); + } +} + +TEST_CASE("CorsConfig::enabled", "[cors]") { + SECTION("default-constructed is disabled") { + REQUIRE_FALSE(CorsConfig{}.enabled()); + } + + SECTION("empty whitelist is disabled") { + REQUIRE_FALSE(CorsConfig{{}}.enabled()); + } + + SECTION("non-empty whitelist is enabled") { + REQUIRE(CorsConfig{{"http://localhost:3000"}}.enabled()); + } +}