From b678315cb5611ff57275d8d7ead5871f90473bd0 Mon Sep 17 00:00:00 2001 From: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:52:26 +0300 Subject: [PATCH 1/4] feat: cors Signed-off-by: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> --- README.md | 23 ++++- src/Config/Layer.ts | 72 +++++++++++++- src/Domain/Config.ts | 76 +++++++++++++++ src/Frontend/Cors.ts | 126 +++++++++++++++++++++++++ src/Http.ts | 8 +- tests/config.test.ts | 38 ++++++++ tests/cors.test.ts | 219 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 555 insertions(+), 7 deletions(-) create mode 100644 src/Frontend/Cors.ts create mode 100644 tests/cors.test.ts diff --git a/README.md b/README.md index a0b95d7..a8d3c31 100644 --- a/README.md +++ b/README.md @@ -68,9 +68,22 @@ backends: # Glob pattern support within the map "test-*": region: us-east-1 - minio2: - # simple config for matching glob buckets - buckets: "my-*" + + # Example Swift backend + swift-storage: + protocol: swift + auth_url: http://keystone.example.com/v3 + region: RegionOne + # Optional: override the Swift container name for all buckets in this backend + # container: my-fixed-container + credentials: + username: my-user + password: my-password + project_name: my-project + user_domain_name: Default + project_domain_name: Default + # Route all archive buckets to Swift + buckets: "archive-*" ``` ### Routing Logic @@ -81,5 +94,5 @@ resolves the backend using the following priority: 1. **Direct match**: Looks for `my-bucket` in all backends' `buckets` maps. 2. **Glob match (map)**: Looks for glob patterns (like `test-*`) in all backends' `buckets` maps. -3. **Glob match (string)**: If a backend has `buckets: "..."`, it checks if the - bucket name matches that pattern. +3. **Glob match (string)**: If a backend has `buckets: "string-*"`, it checks if + the bucket name matches that pattern. diff --git a/src/Config/Layer.ts b/src/Config/Layer.ts index 8efd2d0..016dde6 100644 --- a/src/Config/Layer.ts +++ b/src/Config/Layer.ts @@ -1,6 +1,7 @@ import { Config, Context, Effect, Layer, type Option, Schema } from "effect"; import { parse } from "@std/yaml"; import { + type BackendConfig, GlobalConfig, lookupBucket, type MaterializedBucket, @@ -55,6 +56,12 @@ export function parseConfig( "PROJECT_NAME", "USER_DOMAIN_NAME", "PROJECT_DOMAIN_NAME", + "CORS_ALLOWED_ORIGINS", + "CORS_ALLOWED_METHODS", + "CORS_ALLOWED_HEADERS", + "CORS_EXPOSED_HEADERS", + "CORS_MAX_AGE", + "CORS_CREDENTIALS", ]; for (const [key, value] of Object.entries(env)) { @@ -93,11 +100,64 @@ export function parseConfig( backend.credentials = {} as Record; } (backend.credentials as Record)[configKey] = value; + } else if (configKey.startsWith("cors_")) { + if (!backend.cors) { + backend.cors = {} as Record; + } + const corsKey = configKey.substring(5); + const camelCorsKey = corsKey.replace( + /_([a-z])/g, + (_, g) => g.toUpperCase(), + ); + + if ( + camelCorsKey === "allowedOrigins" || + camelCorsKey === "allowedMethods" || + camelCorsKey === "allowedHeaders" || camelCorsKey === "exposedHeaders" + ) { + (backend.cors as Record)[camelCorsKey] = value.split( + ",", + ).map((s) => s.trim()); + } else if (camelCorsKey === "maxAge") { + (backend.cors as Record)[camelCorsKey] = parseInt( + value, + 10, + ); + } else if (camelCorsKey === "credentials") { + (backend.cors as Record)[camelCorsKey] = + value.toLowerCase() === "true"; + } } else { backend[configKey] = value; } } + // Handle global CORS from env + const globalCors: Record = (yamlConfig && + typeof yamlConfig === "object" && "cors" in yamlConfig) + ? { ...(yamlConfig as { cors: Record }).cors } + : {}; + + for (const [key, value] of Object.entries(env)) { + if (!key.startsWith("HERALD_CORS_")) continue; + const corsKey = key.substring(12).toLowerCase(); + const camelCorsKey = corsKey.replace( + /_([a-z])/g, + (_, g) => g.toUpperCase(), + ); + + if ( + camelCorsKey === "allowedOrigins" || camelCorsKey === "allowedMethods" || + camelCorsKey === "allowedHeaders" || camelCorsKey === "exposedHeaders" + ) { + globalCors[camelCorsKey] = value.split(",").map((s) => s.trim()); + } else if (camelCorsKey === "maxAge") { + globalCors[camelCorsKey] = parseInt(value, 10); + } else if (camelCorsKey === "credentials") { + globalCors[camelCorsKey] = value.toLowerCase() === "true"; + } + } + // Default backend fallback if no backends defined at all if (Object.keys(backends).length === 0) { backends["default"] = { @@ -106,7 +166,17 @@ export function parseConfig( }; } - return Schema.decodeUnknownSync(GlobalConfig)({ backends }); + const validatedBackends: Record = {}; + for (const [id, b] of Object.entries(backends)) { + if (b.protocol === "s3" || b.protocol === "swift") { + validatedBackends[id] = b as BackendConfig; + } + } + + return Schema.decodeUnknownSync(GlobalConfig)({ + backends: validatedBackends, + cors: Object.keys(globalCors).length > 0 ? globalCors : undefined, + }); } export const HeraldConfigLive = Layer.effect( diff --git a/src/Domain/Config.ts b/src/Domain/Config.ts index 77f32a0..9543b6b 100644 --- a/src/Domain/Config.ts +++ b/src/Domain/Config.ts @@ -15,10 +15,22 @@ export const SwiftCredentials = Schema.Struct({ export const Credentials = Schema.Union(S3Credentials, SwiftCredentials); +export const CorsConfig = Schema.Struct({ + allowedOrigins: Schema.optional(Schema.Array(Schema.String)), + allowedMethods: Schema.optional(Schema.Array(Schema.String)), + allowedHeaders: Schema.optional(Schema.Array(Schema.String)), + exposedHeaders: Schema.optional(Schema.Array(Schema.String)), + maxAge: Schema.optional(Schema.Number), + credentials: Schema.optional(Schema.Boolean), +}); + +export type CorsConfig = Schema.Schema.Type; + export const BucketOverride = Schema.Struct({ endpoint: Schema.optional(Schema.String), bucket_name: Schema.optional(Schema.String), region: Schema.optional(Schema.String), + cors: Schema.optional(CorsConfig), }); export type BucketOverride = Schema.Schema.Type; @@ -37,6 +49,7 @@ export const S3Config = Schema.Struct({ region: Schema.optional(Schema.String), credentials: Schema.optional(S3Credentials), buckets: BucketsConfig, + cors: Schema.optional(CorsConfig), }); export const SwiftConfig = Schema.Struct({ @@ -46,6 +59,7 @@ export const SwiftConfig = Schema.Struct({ container: Schema.optional(Schema.String), credentials: Schema.optional(SwiftCredentials), buckets: BucketsConfig, + cors: Schema.optional(CorsConfig), }); export const BackendConfig = Schema.Union(S3Config, SwiftConfig); @@ -54,6 +68,7 @@ export type BackendConfig = Schema.Schema.Type; export const GlobalConfig = Schema.Struct({ backends: Schema.Record({ key: Schema.String, value: BackendConfig }), + cors: Schema.optional(CorsConfig), }); export type GlobalConfig = Schema.Schema.Type; @@ -165,3 +180,64 @@ export const lookupBucket = ( return Option.none(); }; + +export const resolveCorsConfig = ( + config: GlobalConfig, + bucketName: string, +): CorsConfig | undefined => { + // 1. Find the backend and bucket override + let bucketCors: CorsConfig | undefined; + let backendCors: CorsConfig | undefined; + + for (const backend of Object.values(config.backends)) { + const buckets = backend.buckets; + if (buckets && typeof buckets !== "string" && buckets[bucketName]) { + bucketCors = buckets[bucketName].cors; + backendCors = backend.cors; + break; + } + } + + // If not found by direct hit, try glob match (similar to lookupBucket) + if (!bucketCors) { + for (const backend of Object.values(config.backends)) { + const buckets = backend.buckets; + if (buckets && typeof buckets !== "string") { + for (const [key, override] of Object.entries(buckets)) { + if (globToRegex(key).test(bucketName)) { + bucketCors = (override as BucketOverride).cors; + backendCors = backend.cors; + break; + } + } + } + if (bucketCors) break; + } + } + + // If still not found, check if it's a general backend match + if (!bucketCors && !backendCors) { + for (const backend of Object.values(config.backends)) { + const buckets = backend.buckets; + if ( + typeof buckets === "string" && globToRegex(buckets).test(bucketName) + ) { + backendCors = backend.cors; + break; + } + } + } + + const globalCors = config.cors; + + if (!bucketCors && !backendCors && !globalCors) { + return undefined; + } + + // Merge with precedence: bucket > backend > global + return { + ...globalCors, + ...backendCors, + ...bucketCors, + }; +}; diff --git a/src/Frontend/Cors.ts b/src/Frontend/Cors.ts new file mode 100644 index 0000000..c4f94c9 --- /dev/null +++ b/src/Frontend/Cors.ts @@ -0,0 +1,126 @@ +import { + HttpMiddleware, + HttpServerRequest, + HttpServerResponse, +} from "@effect/platform"; +import { Effect } from "effect"; +import { HeraldConfig } from "../Config/Layer.ts"; +import { resolveCorsConfig } from "../Domain/Config.ts"; + +/** + * Extracts the bucket name from the request URL path. + * Assumes path format like /:bucket or /:bucket/* + */ +function extractBucketFromPath(url: string): string | undefined { + try { + const path = new URL(url, "http://localhost").pathname; + const parts = path.split("/").filter((p) => p.length > 0); + return parts.length > 0 ? parts[0] : undefined; + } catch { + return undefined; + } +} + +/** + * Adds CORS headers to a response based on the provided config. + */ +function addCorsHeaders( + response: HttpServerResponse.HttpServerResponse, + cors: NonNullable>, + request: HttpServerRequest.HttpServerRequest, +): HttpServerResponse.HttpServerResponse { + const origin = request.headers["origin"]; + const headers = { ...response.headers }; + + if (cors.allowedOrigins) { + if (cors.allowedOrigins.includes("*")) { + headers["access-control-allow-origin"] = "*"; + } else if (origin && cors.allowedOrigins.includes(origin)) { + headers["access-control-allow-origin"] = origin; + headers["vary"] = headers["vary"] + ? `${headers["vary"]}, Origin` + : "Origin"; + } + } + + if (cors.credentials) { + headers["access-control-allow-credentials"] = "true"; + } + + if (cors.exposedHeaders) { + headers["access-control-expose-headers"] = cors.exposedHeaders.join(", "); + } + + return HttpServerResponse.setHeaders(response, headers); +} + +/** + * Creates a 204 No Content response for OPTIONS preflight requests. + */ +function makePreflightResponse( + cors: NonNullable>, + request: HttpServerRequest.HttpServerRequest, +): HttpServerResponse.HttpServerResponse { + const origin = request.headers["origin"]; + const headers: Record = { + "access-control-max-age": String(cors.maxAge ?? 3600), + }; + + if (cors.allowedOrigins) { + if (cors.allowedOrigins.includes("*")) { + headers["access-control-allow-origin"] = "*"; + } else if (origin && cors.allowedOrigins.includes(origin)) { + headers["access-control-allow-origin"] = origin; + headers["vary"] = "Origin"; + } + } + + if (cors.credentials) { + headers["access-control-allow-credentials"] = "true"; + } + + if (cors.allowedMethods) { + headers["access-control-allow-methods"] = cors.allowedMethods.join(", "); + } else { + // Default to common S3 methods if not specified + headers["access-control-allow-methods"] = + "GET, PUT, POST, DELETE, HEAD, OPTIONS"; + } + + if (cors.allowedHeaders) { + headers["access-control-allow-headers"] = cors.allowedHeaders.join(", "); + } else { + const requestedHeaders = request.headers["access-control-request-headers"]; + if (requestedHeaders) { + headers["access-control-allow-headers"] = requestedHeaders; + } + } + + return HttpServerResponse.empty({ status: 204, headers }); +} + +/** + * Custom CORS middleware that resolves configuration per-request based on the bucket. + */ +export const corsMiddleware = HttpMiddleware.make((app) => + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; + const config = yield* HeraldConfig; + + const bucket = extractBucketFromPath(request.url); + const corsConfig = bucket + ? resolveCorsConfig(config.raw, bucket) + : config.raw.cors; + + if (!corsConfig) { + return yield* app; + } + + if (request.method === "OPTIONS") { + return makePreflightResponse(corsConfig, request); + } + + const response = yield* app; + return addCorsHeaders(response, corsConfig, request); + }) +); diff --git a/src/Http.ts b/src/Http.ts index 6fa2dbc..851f6c2 100644 --- a/src/Http.ts +++ b/src/Http.ts @@ -31,9 +31,15 @@ export const HttpServerHeraldLive = Layer.unwrapEffect( return HttpApiBuilder.serve(HttpMiddleware.logger).pipe( Layer.provide(HttpApiSwagger.layer()), Layer.provide(HttpApiBuilder.middlewareOpenApi()), - Layer.provide(HttpApiBuilder.middlewareCors()), Layer.provide(HttpHeraldLive), HttpServer.withLogAddress, + Layer.provide(HttpApiBuilder.middlewareCors({ + allowedOrigins: ["*"], + allowedMethods: ["GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS"], + allowedHeaders: ["*"], + exposedHeaders: ["*"], + credentials: true, + })), Layer.provide(NodeHttpServer.layer(createServer, { port })), Layer.provide(HeraldConfigLive), ); diff --git a/tests/config.test.ts b/tests/config.test.ts index 9b834d2..442f615 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -250,6 +250,44 @@ const cases: TestCase[] = [ }, }, }, + { + id: "priority_full_hierarchy", + name: "full priority hierarchy (direct > map-glob > string-glob)", + input: { + backends: { + string_glob: { + protocol: "s3", + endpoint: "http://string-glob.com", + buckets: "logs-*", + }, + map_glob: { + protocol: "s3", + endpoint: "http://map-glob.com", + buckets: { + "logs-2025-*": {}, + }, + }, + direct: { + protocol: "s3", + endpoint: "http://direct.com", + buckets: { + "logs-2025-01": {}, + }, + }, + }, + }, + expectedBuckets: { + "logs-2025-01": { backend_id: "direct", endpoint: "http://direct.com" }, + "logs-2025-02": { + backend_id: "map_glob", + endpoint: "http://map-glob.com", + }, + "logs-2024-12": { + backend_id: "string_glob", + endpoint: "http://string-glob.com", + }, + }, + }, ]; for (const tc of cases) { diff --git a/tests/cors.test.ts b/tests/cors.test.ts new file mode 100644 index 0000000..db11f8a --- /dev/null +++ b/tests/cors.test.ts @@ -0,0 +1,219 @@ +import { Effect, Option, Schema } from "effect"; +import { assertEquals, testEffect } from "./utils.ts"; +import { GlobalConfig, resolveCorsConfig } from "../src/Domain/Config.ts"; +import { parseConfig } from "../src/Config/Layer.ts"; +import { corsMiddleware } from "../src/Frontend/Cors.ts"; +import { HttpServerRequest, HttpServerResponse } from "@effect/platform"; +import { HeraldConfig } from "../src/Config/Layer.ts"; + +function makeMockRequest( + url: string, + init: RequestInit, +): HttpServerRequest.HttpServerRequest { + const req = new Request(url, init); + return { + method: req.method, + url: req.url, + headers: Object.fromEntries(req.headers.entries()), + remoteAddress: Option.none(), + } as unknown as HttpServerRequest.HttpServerRequest; +} + +testEffect("cors/resolveCorsConfig/inheritance", () => + Effect.gen(function* () { + yield* Effect.void; + const configInput = { + cors: { + allowedOrigins: ["https://global.com"], + credentials: false, + }, + backends: { + s3_main: { + protocol: "s3", + cors: { + allowedOrigins: ["https://backend.com"], + maxAge: 3600, + }, + buckets: { + bucket_with_cors: { + cors: { + allowedOrigins: ["https://bucket.com"], + credentials: true, + }, + }, + bucket_no_cors: {}, + }, + }, + other: { + protocol: "s3", + buckets: "*", + }, + }, + }; + + const config = Schema.decodeUnknownSync(GlobalConfig)(configInput); + + // 1. Bucket level override + const cors1 = resolveCorsConfig(config, "bucket_with_cors"); + assertEquals(cors1?.allowedOrigins, ["https://bucket.com"]); + assertEquals(cors1?.credentials, true); + assertEquals(cors1?.maxAge, 3600); // Inherited from backend + + // 2. Backend level override + const cors2 = resolveCorsConfig(config, "bucket_no_cors"); + assertEquals(cors2?.allowedOrigins, ["https://backend.com"]); + assertEquals(cors2?.credentials, false); // Inherited from global + assertEquals(cors2?.maxAge, 3600); + + // 3. Global level + const cors3 = resolveCorsConfig(config, "any-other-bucket"); + assertEquals(cors3?.allowedOrigins, ["https://global.com"]); + assertEquals(cors3?.credentials, false); + assertEquals(cors3?.maxAge, undefined); + })); + +testEffect("cors/parseConfig/env_vars", () => + Effect.gen(function* () { + yield* Effect.void; + const env = { + HERALD_CORS_ALLOWED_ORIGINS: "https://global.com, https://other.com", + HERALD_CORS_CREDENTIALS: "true", + HERALD_PROD_PROTOCOL: "s3", + HERALD_PROD_BUCKETS: "*", + HERALD_PROD_CORS_ALLOWED_ORIGINS: "https://s3.com", + HERALD_PROD_CORS_MAX_AGE: "7200", + }; + const config = parseConfig({ backends: {} }, env); + + assertEquals(config.cors?.allowedOrigins, [ + "https://global.com", + "https://other.com", + ]); + assertEquals(config.cors?.credentials, true); + + const prodBackend = config.backends.prod; + assertEquals(prodBackend.protocol, "s3"); + assertEquals(prodBackend.cors?.allowedOrigins, ["https://s3.com"]); + assertEquals(prodBackend.cors?.maxAge, 7200); + })); + +testEffect("cors/parseConfig/yaml_merge", () => + Effect.gen(function* () { + yield* Effect.void; + const yaml = { + cors: { + allowedOrigins: ["https://yaml.com"], + maxAge: 100, + }, + backends: { + s3: { + protocol: "s3", + buckets: "*", + cors: { + allowedMethods: ["GET"], + }, + }, + }, + }; + const env = { + HERALD_CORS_MAX_AGE: "200", + HERALD_S3_CORS_ALLOWED_METHODS: "POST, PUT", + }; + const config = parseConfig(yaml, env); + + assertEquals(config.cors?.allowedOrigins, ["https://yaml.com"]); + assertEquals(config.cors?.maxAge, 200); // Env overrides YAML + + const s3Backend = config.backends.s3; + assertEquals(s3Backend.cors?.allowedMethods, ["POST", "PUT"]); // Env overrides YAML + })); + +testEffect("cors/middleware/preflight", () => + Effect.gen(function* () { + const config: GlobalConfig = { + backends: { + s3: { + protocol: "s3", + buckets: "*", + cors: { + allowedOrigins: ["https://example.com"], + allowedMethods: ["GET", "PUT"], + credentials: true, + }, + }, + }, + }; + + const heraldConfig = { + raw: config, + lookupBucket: () => Option.none(), + }; + + const request = makeMockRequest("http://localhost/s3/obj", { + method: "OPTIONS", + headers: { + "origin": "https://example.com", + "access-control-request-method": "PUT", + }, + }); + + const middleware = corsMiddleware( + Effect.fail(new Error("Should not reach handler")), + ); + + const response = yield* middleware.pipe( + // deno-lint-ignore no-explicit-any + Effect.provideService(HeraldConfig, heraldConfig as any), + Effect.provideService(HttpServerRequest.HttpServerRequest, request), + ); + + assertEquals(response.status, 204); + assertEquals( + response.headers["access-control-allow-origin"], + "https://example.com", + ); + assertEquals(response.headers["access-control-allow-methods"], "GET, PUT"); + assertEquals(response.headers["access-control-allow-credentials"], "true"); + })); + +testEffect("cors/middleware/headers", () => + Effect.gen(function* () { + const config: GlobalConfig = { + backends: { + s3: { + protocol: "s3", + buckets: "*", + cors: { + allowedOrigins: ["*"], + exposedHeaders: ["x-amz-meta-custom"], + }, + }, + }, + }; + + const heraldConfig = { + raw: config, + lookupBucket: () => Option.none(), + }; + + const request = makeMockRequest("http://localhost/s3/obj", { + method: "GET", + headers: { "origin": "https://any.com" }, + }); + + const handler = Effect.succeed(HttpServerResponse.empty({ status: 200 })); + const middleware = corsMiddleware(handler); + + const response = yield* middleware.pipe( + // deno-lint-ignore no-explicit-any + Effect.provideService(HeraldConfig, heraldConfig as any), + Effect.provideService(HttpServerRequest.HttpServerRequest, request), + ); + + assertEquals(response.status, 200); + assertEquals(response.headers["access-control-allow-origin"], "*"); + assertEquals( + response.headers["access-control-expose-headers"], + "x-amz-meta-custom", + ); + })); From d6c2b21f48364528a4487bf4c3a079384d62c29c Mon Sep 17 00:00:00 2001 From: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:55:31 +0300 Subject: [PATCH 2/4] doc: update README on cors details Signed-off-by: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> --- README.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/README.md b/README.md index a8d3c31..a0c19e2 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,42 @@ backends: project_domain_name: Default # Route all archive buckets to Swift buckets: "archive-*" + +cors: + # Global CORS defaults + allowedOrigins: ["*"] + allowedMethods: ["GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS"] + allowedHeaders: ["*"] + exposedHeaders: ["*"] + maxAge: 3600 + credentials: true +``` + +### CORS Configuration + +Herald supports fine-grained CORS control at three levels with the following precedence: **Bucket > Backend > Global**. + +- **Global**: Defined at the root of the config file under `cors`. +- **Backend**: Defined within a backend block under `cors`. Overrides global settings. +- **Bucket**: Defined within a bucket definition under `cors`. Overrides both backend and global settings. + +Example with overrides: + +```yaml +cors: # Global defaults + allowedOrigins: ["*"] + credentials: false + +backends: + prod: + protocol: s3 + cors: # Backend-level override + allowedOrigins: ["https://app.example.com"] + credentials: true + buckets: + assets: + cors: # Bucket-level override + allowedOrigins: ["https://cdn.example.com"] ``` ### Routing Logic From bd2ab6fba944c67b3983b24f4ec2d956bd83b73d Mon Sep 17 00:00:00 2001 From: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> Date: Thu, 22 Jan 2026 07:59:11 +0300 Subject: [PATCH 3/4] fix: fmt --- README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a0c19e2..2f80edd 100644 --- a/README.md +++ b/README.md @@ -97,11 +97,14 @@ cors: ### CORS Configuration -Herald supports fine-grained CORS control at three levels with the following precedence: **Bucket > Backend > Global**. +Herald supports fine-grained CORS control at three levels with the following +precedence: **Bucket > Backend > Global**. - **Global**: Defined at the root of the config file under `cors`. -- **Backend**: Defined within a backend block under `cors`. Overrides global settings. -- **Bucket**: Defined within a bucket definition under `cors`. Overrides both backend and global settings. +- **Backend**: Defined within a backend block under `cors`. Overrides global + settings. +- **Bucket**: Defined within a bucket definition under `cors`. Overrides both + backend and global settings. Example with overrides: From f9b1c714d46993e1e5be0bb89f0bfc4ae6fa099f Mon Sep 17 00:00:00 2001 From: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> Date: Thu, 22 Jan 2026 08:20:16 +0300 Subject: [PATCH 4/4] fix: address feedback Signed-off-by: Yohe-Am <56622350+Yohe-Am@users.noreply.github.com> --- .github/workflows/checks.yml | 4 ---- README.md | 19 ++++++++++++++++++- benchmarks/utils.ts | 6 ++---- src/Config/Layer.ts | 13 ++++++++----- src/Domain/Config.ts | 15 ++++++++++++--- src/Http.ts | 13 ++++--------- tests/utils.ts | 6 ++---- x/s3-tests.ts | 6 ++---- 8 files changed, 48 insertions(+), 34 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index b2bdc8b..639adca 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -19,10 +19,6 @@ env: jobs: checks: runs-on: ubuntu-latest - env: - HERALD_SWIFTTEST_OS_USERNAME: ${{ secrets.OPENSTACK_USERNAME }} - HERALD_SWIFTTEST_OS_PASSWORD: ${{ secrets.OPENSTACK_PASSWORD }} - HERALD_SWIFTTEST_OS_PROJECT_NAME: ${{ secrets.OPENSTACK_PROJECT }} steps: - uses: actions/checkout@v4 with: diff --git a/README.md b/README.md index 2f80edd..00927ee 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ cors: allowedHeaders: ["*"] exposedHeaders: ["*"] maxAge: 3600 - credentials: true + credentials: false ``` ### CORS Configuration @@ -106,6 +106,23 @@ precedence: **Bucket > Backend > Global**. - **Bucket**: Defined within a bucket definition under `cors`. Overrides both backend and global settings. +#### Default Behavior + +If no CORS configuration is provided at any level, **CORS is disabled** and +Herald will not add any CORS-related headers to responses. Preflight `OPTIONS` +requests will be passed through to the backend. + +If you enable CORS by providing configuration at any level, the following +defaults are applied for any omitted fields: + +| Field | Default Value | Description | +| ---------------- | --------------------------------------- | ------------------------------------------------------ | +| `maxAge` | `3600` | Max age in seconds for preflight results | +| `allowedMethods` | `GET, PUT, POST, DELETE, HEAD, OPTIONS` | Allowed HTTP methods | +| `allowedHeaders` | (Mirrors request) | Defaults to mirroring `Access-Control-Request-Headers` | +| `credentials` | `false` | Whether to allow credentials | +| `allowedOrigins` | (None) | Headers only added if `Origin` matches an entry | + Example with overrides: ```yaml diff --git a/benchmarks/utils.ts b/benchmarks/utils.ts index deeda7b..27e4537 100644 --- a/benchmarks/utils.ts +++ b/benchmarks/utils.ts @@ -31,8 +31,7 @@ export type BenchmarkCase = { export const getSwiftConfig = () => Effect.gen(function* () { - const authUrl = yield* Config.string("HEARLD_SWIFTTEST_AUTH_URL").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_AUTH_URL")), + const authUrl = yield* Config.string("HERALD_SWIFTTEST_AUTH_URL").pipe( Config.orElse(() => Config.string("OS_AUTH_URL")), Config.withDefault("http://localhost:8080/auth/v1.0"), Config.option, @@ -56,8 +55,7 @@ export const getSwiftConfig = () => Config.orElse(() => Config.string("OS_PROJECT_NAME")), Config.option, ); - const region = yield* Config.string("HEARLD_SWIFTTEST_OS_REGION_NAME").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_OS_REGION_NAME")), + const region = yield* Config.string("HERALD_SWIFTTEST_OS_REGION_NAME").pipe( Config.orElse(() => Config.string("TF_VAR_OS_REGION_NAME")), Config.orElse(() => Config.string("OS_REGION_NAME")), Config.withDefault("dc3-a"), diff --git a/src/Config/Layer.ts b/src/Config/Layer.ts index 016dde6..343fa12 100644 --- a/src/Config/Layer.ts +++ b/src/Config/Layer.ts @@ -119,10 +119,10 @@ export function parseConfig( ",", ).map((s) => s.trim()); } else if (camelCorsKey === "maxAge") { - (backend.cors as Record)[camelCorsKey] = parseInt( - value, - 10, - ); + const parsed = parseInt(value, 10); + if (Number.isInteger(parsed) && Number.isFinite(parsed)) { + (backend.cors as Record)[camelCorsKey] = parsed; + } } else if (camelCorsKey === "credentials") { (backend.cors as Record)[camelCorsKey] = value.toLowerCase() === "true"; @@ -152,7 +152,10 @@ export function parseConfig( ) { globalCors[camelCorsKey] = value.split(",").map((s) => s.trim()); } else if (camelCorsKey === "maxAge") { - globalCors[camelCorsKey] = parseInt(value, 10); + const parsed = parseInt(value, 10); + if (Number.isInteger(parsed) && Number.isFinite(parsed)) { + globalCors[camelCorsKey] = parsed; + } } else if (camelCorsKey === "credentials") { globalCors[camelCorsKey] = value.toLowerCase() === "true"; } diff --git a/src/Domain/Config.ts b/src/Domain/Config.ts index 9543b6b..c1edc62 100644 --- a/src/Domain/Config.ts +++ b/src/Domain/Config.ts @@ -22,7 +22,14 @@ export const CorsConfig = Schema.Struct({ exposedHeaders: Schema.optional(Schema.Array(Schema.String)), maxAge: Schema.optional(Schema.Number), credentials: Schema.optional(Schema.Boolean), -}); +}).pipe( + Schema.filter((c) => { + if (c.allowedOrigins?.includes("*") && c.credentials) { + return "CORS configuration cannot have allowedOrigins: ['*'] when credentials: true"; + } + return true; + }), +); export type CorsConfig = Schema.Schema.Type; @@ -199,19 +206,21 @@ export const resolveCorsConfig = ( } // If not found by direct hit, try glob match (similar to lookupBucket) - if (!bucketCors) { + if (!bucketCors && !backendCors) { for (const backend of Object.values(config.backends)) { const buckets = backend.buckets; if (buckets && typeof buckets !== "string") { + let foundMatch = false; for (const [key, override] of Object.entries(buckets)) { if (globToRegex(key).test(bucketName)) { bucketCors = (override as BucketOverride).cors; backendCors = backend.cors; + foundMatch = true; break; } } + if (foundMatch) break; } - if (bucketCors) break; } } diff --git a/src/Http.ts b/src/Http.ts index 851f6c2..2765ce0 100644 --- a/src/Http.ts +++ b/src/Http.ts @@ -5,7 +5,7 @@ import { HttpServer, } from "@effect/platform"; import { NodeHttpServer } from "@effect/platform-node"; -import { Config, Effect, Layer } from "effect"; +import { Config, Effect, flow, Layer } from "effect"; // deno-lint-ignore no-external-import import { createServer } from "node:http"; @@ -16,6 +16,7 @@ import { HeraldConfigLive } from "./Config/Layer.ts"; import { HttpHealthLive } from "./Frontend/Health/Http.ts"; import { HttpS3Live } from "./Frontend/Http.ts"; import { HttpHeraldApi } from "./Api.ts"; +import { corsMiddleware } from "./Frontend/Cors.ts"; export const HttpHeraldLive = HttpApiBuilder.api(HttpHeraldApi).pipe( Layer.provide(HttpHealthLive), @@ -28,18 +29,12 @@ export const HttpServerHeraldLive = Layer.unwrapEffect( Config.integer("PORT"), 3000, ); - return HttpApiBuilder.serve(HttpMiddleware.logger).pipe( + const middleware = flow(corsMiddleware, HttpMiddleware.logger); + return HttpApiBuilder.serve(middleware).pipe( Layer.provide(HttpApiSwagger.layer()), Layer.provide(HttpApiBuilder.middlewareOpenApi()), Layer.provide(HttpHeraldLive), HttpServer.withLogAddress, - Layer.provide(HttpApiBuilder.middlewareCors({ - allowedOrigins: ["*"], - allowedMethods: ["GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS"], - allowedHeaders: ["*"], - exposedHeaders: ["*"], - credentials: true, - })), Layer.provide(NodeHttpServer.layer(createServer, { port })), Layer.provide(HeraldConfigLive), ); diff --git a/tests/utils.ts b/tests/utils.ts index 129a8cf..1847e02 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -405,8 +405,7 @@ function proxyRunner(tc: ProxyTestCase, t: Deno.TestContext) { const getSwiftConfig = () => Effect.gen(function* () { - const authUrl = yield* Config.string("HEARLD_SWIFTTEST_AUTH_URL").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_AUTH_URL")), + const authUrl = yield* Config.string("HERALD_SWIFTTEST_AUTH_URL").pipe( Config.orElse(() => Config.string("OS_AUTH_URL")), Config.withDefault("http://localhost:8080/auth/v1.0"), Config.option, @@ -430,8 +429,7 @@ const getSwiftConfig = () => Config.orElse(() => Config.string("OS_PROJECT_NAME")), Config.option, ); - const region = yield* Config.string("HEARLD_SWIFTTEST_OS_REGION_NAME").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_OS_REGION_NAME")), + const region = yield* Config.string("HERALD_SWIFTTEST_OS_REGION_NAME").pipe( Config.orElse(() => Config.string("TF_VAR_OS_REGION_NAME")), Config.orElse(() => Config.string("OS_REGION_NAME")), Config.withDefault("dc3-a"), diff --git a/x/s3-tests.ts b/x/s3-tests.ts index 75c0772..dca1405 100755 --- a/x/s3-tests.ts +++ b/x/s3-tests.ts @@ -54,8 +54,7 @@ function getMinioConfig(): GlobalConfig { const getSwiftConfig = () => Effect.gen(function* () { - const authUrl = yield* Config.string("HEARLD_SWIFTTEST_AUTH_URL").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_AUTH_URL")), + const authUrl = yield* Config.string("HERALD_SWIFTTEST_AUTH_URL").pipe( Config.orElse(() => Config.string("OS_AUTH_URL")), Config.withDefault("http://localhost:8080/auth/v1.0"), Config.option, @@ -79,8 +78,7 @@ const getSwiftConfig = () => Config.orElse(() => Config.string("OS_PROJECT_NAME")), Config.option, ); - const region = yield* Config.string("HEARLD_SWIFTTEST_OS_REGION_NAME").pipe( - Config.orElse(() => Config.string("HERALD_SWIFTTEST_OS_REGION_NAME")), + const region = yield* Config.string("HERALD_SWIFTTEST_OS_REGION_NAME").pipe( Config.orElse(() => Config.string("TF_VAR_OS_REGION_NAME")), Config.orElse(() => Config.string("OS_REGION_NAME")), Config.withDefault("dc3-a"),