From 7ff063b87a51f64e53d5531ae1fc27793de8ce36 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Sun, 23 Aug 2026 17:12:07 -0500 Subject: [PATCH] feat: deliver module objects directly to browsers through opaque read URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0030 left exactly one way to get an image to a browser: read the object into the module's process and write it back out of a module HTTP route. Every byte of every asset travelled storage -> Ekklesia -> browser. For the slideshow this platform exists to replace that means Ekklesia in the path of every image on every display, permanently, for bytes that never change — and it would make Ekklesia responsible for re-implementing byte ranges, without which video seeking cannot work at all. A module can now ask a logical bucket for a temporary browser URL: const { url, expiresAt } = await assets.createReadUrl("slides/welcome.png"); The contract names intent, not mechanism. There is no signing, expiry format, endpoint, or bucket in it — a host with no S3 anywhere could satisfy the same method, which is what keeps a future single-module standalone host able to implement it. The URL is opaque to module code and a bearer capability to whoever holds it, so a module authorizes the request in its own route before asking: storage bounds reach, never identity. Lifetimes run one minute to six hours, fifteen by default, and out-of-range values are refused rather than clamped, so "expires forever" is unreachable. Logical keys stay canonical state; URLs are generated on demand and never persisted or logged. The provider endpoint and the browser delivery origin are separated because measurement showed they cannot be collapsed: an S3-compatible signature covers the request Host, so a URL signed for one origin and rewritten to another is rejected. The browser-facing origin is therefore a signing input (S3_DELIVERY_ENDPOINT), defaulting to S3_ENDPOINT — so the standard deployment configures nothing and delivery just works. The scheme is not signed, so a reverse proxy may terminate TLS provided it passes the original Host through; that is pinned by a test rather than described and hoped for. Read URLs are stable within a window of one third of their lifetime. Signing is a pure function of the clock, so naive re-signing yields a new URL per second — a guaranteed browser cache miss, and the reference client keys its by URL, so an unchanged image would be torn down and re-downloaded continuously. Quantizing the signing timestamp makes repeated calls byte-identical while leaving at least two-thirds of the lifetime; Cache-Control is set at signing time to the guaranteed remainder, because the pinned SeaweedFS image sends none of its own. Verified against live SeaweedFS: byte ranges answered directly with 206 and a correct Content-Range, so no range proxy is needed in Core; signing is local (200 URLs in 66ms), so no batch API is needed; unsigned requests for the same object are refused, so delivery never depends on a public bucket; tampered signatures, swapped keys, and swapped buckets all rejected. Signed write and delete URLs are deliberately not built. Co-Authored-By: Claude Opus 5 --- .env.example | 13 + README.md | 41 +- apps/api/src/server.ts | 15 +- .../tests/support/in-memory-module-storage.ts | 15 +- core/package.json | 1 + core/src/diagnostics/doctor.ts | 9 + core/src/storage/config.ts | 100 +++- core/src/storage/module-storage.ts | 53 +- core/src/storage/provider.ts | 22 +- core/src/storage/s3-provider.ts | 96 ++++ core/tests/doctor.test.ts | 3 +- core/tests/module-delivery.integration.ts | 458 ++++++++++++++++++ core/tests/module-storage.test.ts | 238 ++++++++- .../decisions/0031-direct-object-delivery.md | 214 ++++++++ docs/architecture/decisions/README.md | 1 + docs/architecture/platform-architecture.md | 60 ++- docs/development/api.md | 38 +- docs/development/security.md | 33 ++ docs/development/testing.md | 35 +- docs/module-development/README.md | 56 +++ packages/contracts/src/module-storage.ts | 117 +++++ pnpm-lock.yaml | 16 + vitest.migrations.integration.config.ts | 1 + 23 files changed, 1593 insertions(+), 42 deletions(-) create mode 100644 core/tests/module-delivery.integration.ts create mode 100644 docs/architecture/decisions/0031-direct-object-delivery.md diff --git a/.env.example b/.env.example index a0d4639..92080a3 100644 --- a/.env.example +++ b/.env.example @@ -25,6 +25,19 @@ S3_ENDPOINT=http://127.0.0.1:8333 S3_PORT=8333 S3_REGION=us-east-1 S3_FORCE_PATH_STYLE=true +# The origin a *browser* reaches object storage on, used only when signing temporary read URLs for +# direct delivery (ADR 0031). Leave it unset whenever S3_ENDPOINT is itself reachable from a browser, +# which is the case for this development stack: Compose publishes the service on loopback, so the +# endpoint above serves both roles and direct delivery works with no extra configuration. +# +# Set it when Core reaches storage by a name a browser cannot resolve — a Compose service name such +# as http://object-storage:8333, for instance — and browsers reach it through a public origin: +# S3_DELIVERY_ENDPOINT=https://storage.example.org +# It must be the origin the browser actually requests. An S3-compatible signature covers the request +# Host, so a URL signed for one origin cannot be rewritten to another afterwards; a reverse proxy in +# front of storage must therefore pass the original Host header through unchanged. The scheme is not +# signed, so terminating TLS at that proxy and forwarding plaintext to the service is fine. +# S3_DELIVERY_ENDPOINT= # Namespaces every physical bucket this installation owns, so several Ekklesia-compatible deployments # can share one S3-compatible service. Optional; defaults to "ekklesia". S3_BUCKET_PREFIX=ekklesia diff --git a/README.md b/README.md index c1bbc3f..ff0c838 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,8 @@ against an independent security audit) is complete: all five passes have landed independent whole-repository re-audit has closed, under the current trusted-contributor, not-yet-deployed threat model — and Phase 6 is unblocked and underway.** Phase 6 has since completed through 6D, and **Phase 7A passes 1 (the minimal operational module lifecycle), 2 (the module configuration and -secrets contract), and 3 (host-provided module object storage) have landed.** The operational +secrets contract), 3 (host-provided module object storage), and 4 (direct browser object delivery) +have landed.** The operational requirements that remain are prerequisites for two specific *future* activities — executing untrusted CI contributions, and a public production launch — and neither gates Phase 6; see the pass 5 closure below and @@ -561,12 +562,42 @@ modules, module UI, diagnostics, or any API response — errors are translated t carries no endpoint, bucket, request id, or XML. This also retires the transitional `bucket` *configuration* key ADR 0029 flagged: the manifest declaration is the requirement. -Browsers reach objects through a module's own routes (`browser → module API → scoped storage`), so no -S3 knowledge or credential goes near the browser. **Signed URLs, public object delivery, range -requests, and multipart upload are deliberately deferred**, and the provider is not yet -"production-ready" by ADR 0004's own bar. See +Multipart upload is deliberately deferred, and the provider is not yet "production-ready" by +ADR 0004's own bar. See [ADR 0030](docs/architecture/decisions/0030-host-provided-module-object-storage.md). +Phase 7A pass 4 takes Ekklesia **out of the data path** for assets a browser is going to fetch +anyway. A module asks its logical bucket for a temporary browser URL and returns it; the browser +fetches the object from the storage service directly: + +```ts +const { url, expiresAt } = await context.storage + .bucket("announcement-assets") + .createReadUrl("slides/welcome.png"); +``` + +The contract still names no mechanism — no signing, no endpoint, no expiry format — so a host with no +S3 in it could satisfy the same method. The URL is **opaque** to module code and a **bearer +capability** to whoever holds it, so a module authorizes the request in its own route before asking +for one: storage bounds *reach*, never *identity*. Lifetimes run from one minute to six hours, +fifteen minutes by default, and out-of-range requests are refused rather than clamped. Modules +persist logical keys and generate URLs on demand; a stored URL would be a stored credential. + +This is what makes media practical. A `Range` request against a delivery URL is answered by the +storage service directly — verified `206` with a correct `Content-Range` — so byte-range seeking needs +no proxy in Core. Signing is local, so a slideshow builds one URL per slide with no round trips, and +repeated calls inside a window return a byte-identical URL so browser caches and URL-keyed DOM +elements hold. + +Because an S3-compatible signature covers the request's `Host`, a URL cannot be signed for one origin +and rewritten to another. The origin a browser uses is therefore a signing input +(`S3_DELIVERY_ENDPOINT`), defaulting to `S3_ENDPOINT` — so the standard deployment configures nothing +and delivery simply works. A reverse proxy in front of storage must pass the original `Host` through +unchanged; it may terminate TLS, because the scheme is not signed. Buckets stay private: an unsigned +request for the same object is refused, and a test asserts it. **Signed write and delete URLs, and a +realtime refresh transport, are deliberately deferred.** See +[ADR 0031](docs/architecture/decisions/0031-direct-object-delivery.md). + **Still not the finished Dashboard.** There is no navigation editor or drag and drop, no nested folders, no icons or colors, no Dashboard Home or widgets, no design system, no client-side routing, and no module-management, settings, or storage UI — module enablement and configuration are an API and diff --git a/apps/api/src/server.ts b/apps/api/src/server.ts index 5229e9c..f368c55 100644 --- a/apps/api/src/server.ts +++ b/apps/api/src/server.ts @@ -12,11 +12,6 @@ import { } from "@ekklesia/core/authorization"; import { createDashboardStore } from "@ekklesia/core/dashboard"; import { createIdentityRuntime, type IdentityRateLimitOverride } from "@ekklesia/core/identity"; -import { - createModuleStorageLookup, - createS3StorageProvider, - type StorageProvider, -} from "@ekklesia/core/storage"; import { createEnvironmentModuleSecretSource, createModuleConfigurationStore, @@ -29,7 +24,12 @@ import { type StartedModuleRuntime, startDiscoveredModuleRuntime, } from "@ekklesia/core/modules"; -import { serve, type ServerType } from "@hono/node-server"; +import { + createModuleStorageLookup, + createS3StorageProvider, + type StorageProvider, +} from "@ekklesia/core/storage"; +import { type ServerType, serve } from "@hono/node-server"; import type { Hono } from "hono"; import { createApiApp } from "./app.js"; import { @@ -708,6 +708,9 @@ function readConfigFromEnvironment(): { storage: { accessKey: process.env.S3_ACCESS_KEY, bucketPrefix: process.env.S3_BUCKET_PREFIX, + // Optional, and unset is the ordinary case: it matters only where the name Core reaches + // storage by is not a name a browser could resolve. + deliveryEndpoint: process.env.S3_DELIVERY_ENDPOINT, endpoint: process.env.S3_ENDPOINT, forcePathStyle: process.env.S3_FORCE_PATH_STYLE, region: process.env.S3_REGION, diff --git a/apps/api/tests/support/in-memory-module-storage.ts b/apps/api/tests/support/in-memory-module-storage.ts index 7334043..e8d454d 100644 --- a/apps/api/tests/support/in-memory-module-storage.ts +++ b/apps/api/tests/support/in-memory-module-storage.ts @@ -2,11 +2,12 @@ import { type ModuleStorage, type ModuleStorageBody, ModuleStorageError, - type ModuleStorageListOptions, type ModuleStorageListing, + type ModuleStorageListOptions, type ModuleStorageObject, type ModuleStorageObjectMetadata, type ModuleStoragePutOptions, + type ModuleStorageReadUrl, } from "@ekklesia/contracts"; import type { DiscoveredModule, ModuleStorageLookup } from "@ekklesia/core/modules"; import { createModuleStorageLookup, type StorageProvider } from "@ekklesia/core/storage"; @@ -91,6 +92,18 @@ export function createInMemoryStorageProvider(): StorageProvider & { async close(): Promise { // Nothing to release. }, + async createReadUrl( + physicalBucket: string, + key: string, + urlOptions: { readonly expiresInSeconds: number; readonly downloadFilename?: string }, + ): Promise { + // Keyed by physical bucket like everything else here, so a route test can prove the URL a + // handler received belongs to that module's namespace and not another's. + return Object.freeze({ + expiresAt: new Date(Date.now() + urlOptions.expiresInSeconds * 1000), + url: `https://delivery.test/${physicalBucket}/${encodeURIComponent(key)}?token=fake`, + }); + }, async delete(physicalBucket: string, key: string): Promise { objectsIn(physicalBucket).delete(key); }, diff --git a/core/package.json b/core/package.json index 49e4675..805ed4c 100644 --- a/core/package.json +++ b/core/package.json @@ -54,6 +54,7 @@ }, "dependencies": { "@aws-sdk/client-s3": "3.1116.0", + "@aws-sdk/s3-request-presigner": "3.1116.0", "@ekklesia/contracts": "workspace:*", "better-auth": "1.6.23", "drizzle-orm": "0.45.2", diff --git a/core/src/diagnostics/doctor.ts b/core/src/diagnostics/doctor.ts index 0176279..94024af 100644 --- a/core/src/diagnostics/doctor.ts +++ b/core/src/diagnostics/doctor.ts @@ -1049,6 +1049,15 @@ export async function diagnoseEkklesia( await dependencies.probeStorage(options.storage); storageCheck = freezeCheck("object-storage.integration", "pass", "Object storage reachable", [ describeStorageProvider(options.storage), + // Said plainly rather than implied: this probe used the endpoint Core talks to, and a + // separate browser-facing origin is by definition something a browser reaches and this + // process may not. Reporting "reachable" without that caveat would let an operator conclude + // direct delivery works when only half of it was tested. + ...(options.storage.deliveryEndpoint === options.storage.endpoint + ? [] + : [ + "The browser delivery origin is configured separately and was not probed; verify it from a browser. Any reverse proxy in front of it must pass the original Host header through unchanged, or signed read URLs will be rejected.", + ]), ]); } catch { // A warning, never a failure: the provider is configured, and a module that is already diff --git a/core/src/storage/config.ts b/core/src/storage/config.ts index ae5b4d7..4affbbe 100644 --- a/core/src/storage/config.ts +++ b/core/src/storage/config.ts @@ -7,6 +7,9 @@ import { export type StorageConfigErrorCode = | "STORAGE_ACCESS_KEY_MISSING" | "STORAGE_BUCKET_PREFIX_INVALID" + | "STORAGE_DELIVERY_ENDPOINT_INSECURE_IN_PRODUCTION" + | "STORAGE_DELIVERY_ENDPOINT_INVALID" + | "STORAGE_DELIVERY_ENDPOINT_WITH_CREDENTIALS" | "STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION" | "STORAGE_ENDPOINT_INVALID" | "STORAGE_ENDPOINT_WITH_CREDENTIALS" @@ -16,6 +19,12 @@ export type StorageConfigErrorCode = const storageConfigMessages: Readonly> = Object.freeze({ STORAGE_ACCESS_KEY_MISSING: "A storage access key is required when a storage endpoint is set.", STORAGE_BUCKET_PREFIX_INVALID: "The storage bucket prefix must be lowercase kebab-case.", + STORAGE_DELIVERY_ENDPOINT_INSECURE_IN_PRODUCTION: + "A production browser delivery endpoint must use https, or browsers will refuse to load from it.", + STORAGE_DELIVERY_ENDPOINT_INVALID: + "The browser delivery endpoint must be an absolute http or https URL.", + STORAGE_DELIVERY_ENDPOINT_WITH_CREDENTIALS: + "The browser delivery endpoint must not embed credentials.", STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION: "A production storage endpoint must use https, exactly as the database transport must use TLS.", STORAGE_ENDPOINT_INVALID: "The storage endpoint must be an absolute http or https URL.", @@ -51,6 +60,8 @@ export interface StorageProviderConfigInput { readonly secretKey?: string | undefined; readonly forcePathStyle?: string | boolean | undefined; readonly bucketPrefix?: string | undefined; + /** The origin a **browser** reaches object storage on, when that differs from `endpoint`. */ + readonly deliveryEndpoint?: string | undefined; } /** @@ -70,6 +81,53 @@ export interface StorageProviderConfig { * deployment does not have. */ readonly forcePathStyle: boolean; readonly bucketPrefix: string; + /** + * The origin browser-facing read URLs are signed for. + * + * Equal to `endpoint` unless the deployment separates them, which is the common case once Core and + * the storage service share a private network: Core reaches the service at something like + * `http://object-storage:8333`, a name that means nothing in a browser, while the browser reaches + * it through a public name. + * + * The two roles cannot be collapsed by rewriting a URL after the fact. An S3-compatible signature + * covers the request's `Host` header, so a URL signed for one host and then re-pointed at another + * fails verification — which is why this is a *signing* input rather than a post-processing step. + * What the signature does **not** cover is the scheme, so terminating TLS at a reverse proxy and + * forwarding plaintext to the storage service verifies correctly, provided the proxy passes the + * original `Host` through unchanged. + */ + readonly deliveryEndpoint: string; +} + +interface EndpointErrorCodes { + readonly invalid: StorageConfigErrorCode; + readonly withCredentials: StorageConfigErrorCode; + readonly insecure: StorageConfigErrorCode; +} + +/** Applies one rule set to both endpoints, so the browser-facing origin can never be validated more + * loosely than the one Core itself uses. */ +function normalizeEndpoint( + value: string, + environment: StorageEnvironment, + codes: EndpointErrorCodes, +): string { + let parsed: URL; + try { + parsed = new URL(value); + } catch { + throw new StorageConfigError(codes.invalid); + } + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + throw new StorageConfigError(codes.invalid); + } + if (parsed.username !== "" || parsed.password !== "") { + throw new StorageConfigError(codes.withCredentials); + } + if (environment === "production" && parsed.protocol !== "https:") { + throw new StorageConfigError(codes.insecure); + } + return parsed.origin + (parsed.pathname === "/" ? "" : parsed.pathname.replace(/\/+$/u, "")); } const DEFAULT_REGION = "us-east-1"; @@ -114,22 +172,25 @@ export function createStorageProviderConfig( return undefined; } - let parsed: URL; - try { - parsed = new URL(endpoint); - } catch { - throw new StorageConfigError("STORAGE_ENDPOINT_INVALID"); - } - if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { - throw new StorageConfigError("STORAGE_ENDPOINT_INVALID"); - } - if (parsed.username !== "" || parsed.password !== "") { - throw new StorageConfigError("STORAGE_ENDPOINT_WITH_CREDENTIALS"); - } const environment = options.environment ?? "production"; - if (environment === "production" && parsed.protocol !== "https:") { - throw new StorageConfigError("STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION"); - } + const normalizedEndpoint = normalizeEndpoint(endpoint, environment, { + insecure: "STORAGE_ENDPOINT_INSECURE_IN_PRODUCTION", + invalid: "STORAGE_ENDPOINT_INVALID", + withCredentials: "STORAGE_ENDPOINT_WITH_CREDENTIALS", + }); + + // Unset means the one endpoint serves both roles, which is correct whenever Core and the browser + // reach storage by the same name — the repository's own development stack publishes the service on + // loopback, so the ordinary developer configures nothing here at all. + const deliveryInput = input?.deliveryEndpoint?.trim(); + const deliveryEndpoint = + deliveryInput === undefined || deliveryInput === "" + ? normalizedEndpoint + : normalizeEndpoint(deliveryInput, environment, { + insecure: "STORAGE_DELIVERY_ENDPOINT_INSECURE_IN_PRODUCTION", + invalid: "STORAGE_DELIVERY_ENDPOINT_INVALID", + withCredentials: "STORAGE_DELIVERY_ENDPOINT_WITH_CREDENTIALS", + }); const region = input?.region?.trim() === "" ? undefined : input?.region?.trim(); if (region !== undefined && !REGION_PATTERN.test(region)) { @@ -160,7 +221,8 @@ export function createStorageProviderConfig( bucketPrefix === undefined || bucketPrefix === "" ? DEFAULT_STORAGE_BUCKET_PREFIX : bucketPrefix, - endpoint: parsed.origin + (parsed.pathname === "/" ? "" : parsed.pathname.replace(/\/+$/u, "")), + deliveryEndpoint, + endpoint: normalizedEndpoint, forcePathStyle: parseBoolean(input?.forcePathStyle, true), region: region ?? DEFAULT_REGION, secretKey, @@ -177,9 +239,13 @@ export function createStorageProviderConfig( */ export function describeStorageProvider(config: StorageProviderConfig): string { const origin = new URL(config.endpoint).origin; + const deliveryOrigin = new URL(config.deliveryEndpoint).origin; + // Named only when it differs, so the ordinary single-origin deployment reads exactly as it did + // before this distinction existed. Both are host infrastructure names, never credentials. + const delivery = deliveryOrigin === origin ? "" : ` · browser delivery ${deliveryOrigin}`; return `S3-compatible endpoint ${origin} · region ${config.region} · ${ config.forcePathStyle ? "path-style" : "virtual-host-style" - } · bucket prefix "${config.bucketPrefix}"`; + } · bucket prefix "${config.bucketPrefix}"${delivery}`; } export { STORAGE_BUCKET_PREFIX_MAX_LENGTH }; diff --git a/core/src/storage/module-storage.ts b/core/src/storage/module-storage.ts index bd83ad4..6bad1c1 100644 --- a/core/src/storage/module-storage.ts +++ b/core/src/storage/module-storage.ts @@ -1,15 +1,21 @@ import { EMPTY_MODULE_STORAGE, isValidModuleStorageKey, + isValidModuleStorageReadUrlFilename, + MODULE_STORAGE_READ_URL_DEFAULT_SECONDS, + MODULE_STORAGE_READ_URL_MAX_SECONDS, + MODULE_STORAGE_READ_URL_MIN_SECONDS, type ModuleStorage, type ModuleStorageBody, type ModuleStorageBucket, ModuleStorageError, - type ModuleStorageListOptions, type ModuleStorageListing, + type ModuleStorageListOptions, type ModuleStorageObject, type ModuleStorageObjectMetadata, type ModuleStoragePutOptions, + type ModuleStorageReadUrl, + type ModuleStorageReadUrlOptions, } from "@ekklesia/contracts"; import { type ModuleStorageBinding, resolveModuleStorageBindings } from "./binding.js"; import type { StorageProvider } from "./provider.js"; @@ -58,6 +64,27 @@ function assertValidKey(key: unknown): asserts key is string { throw new ModuleStorageError("STORAGE_KEY_INVALID", echo); } +/** + * Resolves a requested read-URL lifetime against the contract's bounds. + * + * Enforced here rather than in a provider so every provider agrees on what a module may ask for, and + * so an out-of-range request is refused rather than quietly clamped — a module that asks for a + * week's lifetime has a misunderstanding worth surfacing, not a value worth rounding. + */ +function resolveExpirySeconds(requested: number | undefined): number { + if (requested === undefined) { + return MODULE_STORAGE_READ_URL_DEFAULT_SECONDS; + } + if ( + !Number.isInteger(requested) || + requested < MODULE_STORAGE_READ_URL_MIN_SECONDS || + requested > MODULE_STORAGE_READ_URL_MAX_SECONDS + ) { + throw new ModuleStorageError("STORAGE_READ_URL_EXPIRY_INVALID"); + } + return requested; +} + function createBucketHandle( provider: StorageProvider, binding: ModuleStorageBinding, @@ -69,6 +96,30 @@ function createBucketHandle( // where the host keeps its data. id: binding.logicalBucketId, + async createReadUrl( + key: string, + options: ModuleStorageReadUrlOptions = {}, + ): Promise { + assertValidKey(key); + const expiresInSeconds = resolveExpirySeconds(options.expiresIn); + if ( + options.downloadFilename !== undefined && + !isValidModuleStorageReadUrlFilename(options.downloadFilename) + ) { + throw new ModuleStorageError("STORAGE_READ_URL_FILENAME_INVALID", key); + } + // The bucket is ensured for the same reason every other operation ensures it: a URL for a + // bucket that was never created would resolve to a failure the browser reports rather than one + // the module can see. Memoized per binding, so a module signing forty slides pays for it once. + await ensure(); + return provider.createReadUrl(binding.physicalBucket, key, { + expiresInSeconds, + ...(options.downloadFilename === undefined + ? {} + : { downloadFilename: options.downloadFilename }), + }); + }, + async delete(key: string): Promise { assertValidKey(key); await ensure(); diff --git a/core/src/storage/provider.ts b/core/src/storage/provider.ts index 0d381e1..2088c86 100644 --- a/core/src/storage/provider.ts +++ b/core/src/storage/provider.ts @@ -1,10 +1,11 @@ import type { ModuleStorageBody, - ModuleStorageListOptions, ModuleStorageListing, + ModuleStorageListOptions, ModuleStorageObject, ModuleStorageObjectMetadata, ModuleStoragePutOptions, + ModuleStorageReadUrl, } from "@ekklesia/contracts"; /** @@ -42,6 +43,25 @@ export interface StorageProvider { options: ModuleStoragePutOptions, ): Promise; delete(physicalBucket: string, key: string): Promise; + /** + * A URL a browser can read one object from directly, valid for `expiresIn` seconds. + * + * The seconds and the download filename arrive already validated against the contract's bounds — + * a provider enforces neither, so two providers can never disagree about what a module is allowed + * to ask for. + * + * A provider that cannot issue such a URL has no business implementing this interface: the + * facility is part of the storage contract, not an optional extra a module has to feel around for. + * An implementation with no notion of signing could satisfy it by minting its own opaque token. + */ + createReadUrl( + physicalBucket: string, + key: string, + options: { + readonly expiresInSeconds: number; + readonly downloadFilename?: string | undefined; + }, + ): Promise; /** A cheap reachability probe for diagnostics only. Its result is **never** persisted and never * influences a module's lifecycle state — a provider being briefly unreachable is an operational * condition, not an administrative decision (ADR 0030). */ diff --git a/core/src/storage/s3-provider.ts b/core/src/storage/s3-provider.ts index 61f1dd7..29a271c 100644 --- a/core/src/storage/s3-provider.ts +++ b/core/src/storage/s3-provider.ts @@ -9,6 +9,7 @@ import { PutObjectCommand, S3Client, } from "@aws-sdk/client-s3"; +import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; import { type ModuleStorageBody, ModuleStorageError, @@ -17,6 +18,7 @@ import { type ModuleStorageObject, type ModuleStorageObjectMetadata, type ModuleStoragePutOptions, + type ModuleStorageReadUrl, } from "@ekklesia/contracts"; import type { StorageProviderConfig } from "./config.js"; import type { StorageProvider } from "./provider.js"; @@ -173,6 +175,43 @@ const DEFAULT_TRANSPORT_BOUNDS: S3TransportBounds = Object.freeze({ socketIdleTimeoutMs: 5_000, }); +/** + * How coarsely a read URL's signing timestamp is rounded down, as a fraction of its lifetime. + * + * Signing is a pure function of the request and the clock, so signing the same object twice a second + * apart yields two different URLs for the same bytes. That is worse than untidy. A browser caches by + * URL, so a changed query string is a guaranteed cache miss, and UI frameworks routinely key list + * items by URL — the reference consumer this was measured against renders ``, so a + * new URL for an unchanged image tears the element down and re-downloads it. A slideshow rebuilding + * its state every few seconds would therefore re-fetch every slide continuously, having gained a + * direct path to storage and lost the caching that made it worth having. + * + * Rounding the signing time down to a window makes repeated calls return a **byte-identical** URL + * within that window, so caches hit and keyed elements stay mounted. Dividing by three, rather than + * fixing a duration, keeps the guarantee proportional: whatever lifetime is asked for, at least + * two-thirds of it always remains when a URL is handed out. + */ +const READ_URL_STABILITY_DIVISOR = 3; + +interface ReadUrlWindow { + readonly signedAt: Date; + readonly expiresAt: Date; + readonly guaranteedSeconds: number; +} + +function readUrlWindow(nowMs: number, expiresInSeconds: number): ReadUrlWindow { + const windowSeconds = Math.max(1, Math.floor(expiresInSeconds / READ_URL_STABILITY_DIVISOR)); + const windowMs = windowSeconds * 1000; + const signedAtMs = Math.floor(nowMs / windowMs) * windowMs; + return { + expiresAt: new Date(signedAtMs + expiresInSeconds * 1000), + // Deliberately derived from the window rather than from the current clock: anything that varies + // second by second would end up in the signed query string and undo the stability above. + guaranteedSeconds: expiresInSeconds - windowSeconds, + signedAt: new Date(signedAtMs), + }; +} + export function createS3StorageProvider( config: StorageProviderConfig, /** Tightened only by the live provider suite, so it can prove the deadline actually fires without @@ -197,6 +236,25 @@ export function createS3StorageProvider( }, }); + /** + * The client read URLs are signed with. + * + * An S3-compatible signature covers the request's `Host`, so a URL cannot be signed against the + * endpoint Core talks to and then re-pointed at the origin a browser can reach — verification + * fails. The delivery origin therefore has to be a *signing input*, which is what this second + * client supplies. It never sends a request: presigning is a local computation, so this opens no + * socket and is built only when the two origins actually differ. + */ + const signingClient = + config.deliveryEndpoint === config.endpoint + ? client + : new S3Client({ + credentials: { accessKeyId: config.accessKey, secretAccessKey: config.secretKey }, + endpoint: config.deliveryEndpoint, + forcePathStyle: config.forcePathStyle, + region: config.region, + }); + return Object.freeze({ async ensureBucket(physicalBucket: string): Promise { try { @@ -326,6 +384,41 @@ export function createS3StorageProvider( } }, + async createReadUrl( + physicalBucket: string, + key: string, + options: { + readonly expiresInSeconds: number; + readonly downloadFilename?: string | undefined; + }, + ): Promise { + const window = readUrlWindow(Date.now(), options.expiresInSeconds); + try { + const url = await getSignedUrl( + signingClient, + new GetObjectCommand({ + Bucket: physicalBucket, + Key: key, + // The pinned SeaweedFS image returns no `Cache-Control` of its own, which leaves + // browsers guessing how long an asset may be reused. Stating it turns the stable URL + // above into an actual cache hit. It never outlives the URL itself. + ResponseCacheControl: `private, max-age=${window.guaranteedSeconds}`, + ...(options.downloadFilename === undefined + ? {} + : { + ResponseContentDisposition: `attachment; filename="${options.downloadFilename}"`, + }), + }), + { expiresIn: options.expiresInSeconds, signingDate: window.signedAt }, + ); + return Object.freeze({ expiresAt: window.expiresAt, url }); + } catch (error) { + // Signing is local, so a failure here is a malformed request rather than an outage — but it + // is translated exactly like any other so no SDK exception can reach a module. + throw translateStorageError(error, key); + } + }, + async probe(): Promise { try { // The cheapest call that proves the endpoint is reachable and the credentials sign @@ -338,6 +431,9 @@ export function createS3StorageProvider( async close(): Promise { client.destroy(); + if (signingClient !== client) { + signingClient.destroy(); + } }, }); } diff --git a/core/tests/doctor.test.ts b/core/tests/doctor.test.ts index 68375e3..a818795 100644 --- a/core/tests/doctor.test.ts +++ b/core/tests/doctor.test.ts @@ -1,4 +1,4 @@ -import { parsePermissionId, type ModuleManifest } from "@ekklesia/contracts"; +import { type ModuleManifest, parsePermissionId } from "@ekklesia/contracts"; import { describe, expect, it, vi } from "vitest"; import { createPermissionRegistry, PermissionRegistryError } from "../src/authorization/index.js"; import { @@ -1219,6 +1219,7 @@ describe("Ekklesia Doctor — object storage", () => { const storageConfig = { accessKey: "AKIAEXAMPLEACCESSID", bucketPrefix: "ekklesia", + deliveryEndpoint: "https://storage.example.test", endpoint: "https://storage.example.test", forcePathStyle: true, region: "us-east-1", diff --git a/core/tests/module-delivery.integration.ts b/core/tests/module-delivery.integration.ts new file mode 100644 index 0000000..24cfea2 --- /dev/null +++ b/core/tests/module-delivery.integration.ts @@ -0,0 +1,458 @@ +import { randomBytes } from "node:crypto"; +import { request } from "node:http"; +import type { ModuleStorage } from "@ekklesia/contracts"; +import { afterAll, describe, expect, it } from "vitest"; +import { createStorageProviderConfig } from "../src/storage/config.js"; +import { createModuleStorage } from "../src/storage/module-storage.js"; +import type { StorageProvider } from "../src/storage/provider.js"; +import { createS3StorageProvider } from "../src/storage/s3-provider.js"; + +/** + * Direct browser delivery against a **real S3-compatible server** (ADR 0031). + * + * Everything here is a claim that cannot be checked without a live provider. That a signature + * survives being served through a reverse proxy, that a byte range works without Core in the path, + * that a tampered URL is refused, that expiry is actually enforced — none of these are properties of + * the code in this repository, they are properties of the agreement between the signer and the + * server, and only the server can settle them. + * + * The bucket frugality of `module-storage.integration.ts` applies here for the same reason: a bucket + * is a collection on the pinned SeaweedFS image and collections come out of a finite, disk-derived + * budget. Tests share one bucket and separate by key. + */ +const endpointEnvironment = "EKKLESIA_TEST_STORAGE_ENDPOINT"; +const accessKeyEnvironment = "EKKLESIA_TEST_STORAGE_ACCESS_KEY"; +const secretKeyEnvironment = "EKKLESIA_TEST_STORAGE_SECRET_KEY"; + +class IntegrationHarnessError extends Error { + constructor(message: string) { + super(message); + this.name = "IntegrationHarnessError"; + this.stack = `${this.name}: ${this.message}`; + } +} + +function providerConfig(deliveryEndpoint?: string) { + const endpoint = process.env[endpointEnvironment]; + if (typeof endpoint !== "string" || endpoint.trim() === "") { + throw new IntegrationHarnessError("The storage integration endpoint is not configured."); + } + const config = createStorageProviderConfig( + { + accessKey: process.env[accessKeyEnvironment], + ...(deliveryEndpoint === undefined ? {} : { deliveryEndpoint }), + endpoint, + secretKey: process.env[secretKeyEnvironment], + }, + { environment: "development" }, + ); + if (!config) { + throw new IntegrationHarnessError("The storage integration configuration is invalid."); + } + return config; +} + +function endpointOrigin(): URL { + return new URL(providerConfig().endpoint); +} + +const openProviders: StorageProvider[] = []; + +function provider(deliveryEndpoint?: string): StorageProvider { + const created = createS3StorageProvider(providerConfig(deliveryEndpoint)); + openProviders.push(created); + return created; +} + +let cachedProvider: StorageProvider | undefined; +function sharedProvider(): StorageProvider { + cachedProvider ??= provider(); + return cachedProvider; +} + +const runPrefix = `ekkdel${randomBytes(3).toString("hex")}`; + +/** The scoped facade a module would receive — every test drives delivery through this, never through + * the provider, so nothing here can accidentally prove something a module could not do. */ +function moduleBucket(moduleId = "announcements", logicalBucketId = "announcement-assets") { + return createModuleStorage({ + bucketPrefix: runPrefix, + logicalBucketIds: [logicalBucketId], + moduleId, + provider: sharedProvider(), + }).bucket(logicalBucketId); +} + +let namespaceCounter = 0; +function keyNamespace(): string { + namespaceCounter += 1; + return `case-${namespaceCounter}`; +} + +/** Issues a request to the real backend while presenting an arbitrary `Host`, which is exactly what + * a reverse proxy in front of storage does. `fetch` cannot express this: `Host` is a forbidden + * header name there, so it would silently send the wrong one and prove nothing. */ +function getWithHost( + pathAndQuery: string, + hostHeader: string, +): Promise<{ status: number; headers: Record }> { + const target = endpointOrigin(); + return new Promise((resolve, reject) => { + const outbound = request( + { + headers: { Host: hostHeader }, + host: target.hostname, + method: "GET", + path: pathAndQuery, + port: target.port, + }, + (response) => { + response.resume(); + response.on("end", () => + resolve({ headers: response.headers, status: response.statusCode ?? 0 }), + ); + }, + ); + outbound.on("error", reject); + outbound.end(); + }); +} + +afterAll(async () => { + for (const open of openProviders.splice(0)) { + await open.close().catch(() => undefined); + } +}); + +describe.sequential("live direct object delivery", () => { + it("hands a browser a URL that returns the object, with its content type intact", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/welcome.png`; + const png = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + await bucket.put(key, png, { contentType: "image/png" }); + + const delivery = await bucket.createReadUrl(key); + // An ordinary fetch with no credentials, no SDK, and no Ekklesia in the path. + const response = await fetch(delivery.url); + + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("image/png"); + expect(new Uint8Array(await response.arrayBuffer())).toEqual(png); + expect(delivery.expiresAt.getTime()).toBeGreaterThan(Date.now()); + }); + + it("refuses the same object without the URL's authorization", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/private.png`; + await bucket.put(key, "x"); + const delivery = await bucket.createReadUrl(key); + const unsigned = new URL(delivery.url); + for (const parameter of [...unsigned.searchParams.keys()]) { + unsigned.searchParams.delete(parameter); + } + + // The bucket is not public: delivery works because the URL carries authorization, not because + // anyone may read the object. This is the assertion that would fail if a future change "fixed" + // delivery by making buckets anonymous. + expect((await fetch(unsigned.toString())).status).toBe(403); + }); + + it("refuses a tampered signature, a swapped key, and a swapped bucket", async () => { + const bucket = moduleBucket(); + const namespace = keyNamespace(); + await bucket.put(`${namespace}/a.txt`, "a"); + await bucket.put(`${namespace}/b.txt`, "b"); + const delivery = await bucket.createReadUrl(`${namespace}/a.txt`); + + const forged = new URL(delivery.url); + forged.searchParams.set("X-Amz-Signature", "0".repeat(64)); + expect((await fetch(forged.toString())).status).toBe(403); + + // The signature covers the path, so a URL is a capability for one object rather than for the + // bucket it happens to live in. + expect((await fetch(delivery.url.replace("/a.txt", "/b.txt"))).status).toBe(403); + + // And swapping the physical bucket is refused too, which is what stops a URL for one module's + // object from being edited into a URL for another module's. + const physical = new URL(delivery.url).pathname.split("/")[1] ?? ""; + expect(physical).not.toBe(""); + expect((await fetch(delivery.url.replace(physical, physical.replace(/.$/u, "z")))).status).toBe( + 403, + ); + }); + + it("stops working once it expires", async () => { + // Driven at the provider seam so the deadline can be one second: the module-facing contract + // deliberately refuses lifetimes that short, and waiting out its minute-long floor would buy no + // extra confidence about whether the server enforces expiry. + const bucket = `${runPrefix}-expiry`.toLowerCase(); + await sharedProvider().ensureBucket(bucket); + await sharedProvider().put(bucket, "fleeting.txt", "x", {}); + const delivery = await sharedProvider().createReadUrl(bucket, "fleeting.txt", { + expiresInSeconds: 1, + }); + + expect((await fetch(delivery.url)).status).toBe(200); + await new Promise((resolve) => setTimeout(resolve, 2_500)); + expect((await fetch(delivery.url)).status).toBe(403); + }); + + it("serves byte ranges directly, with no Core process in the data path", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/sermon.bin`; + const payload = "0123456789".repeat(500); + await bucket.put(key, payload, { contentType: "video/mp4" }); + + const delivery = await bucket.createReadUrl(key); + const ranged = await fetch(delivery.url, { headers: { Range: "bytes=10-19" } }); + + // This is what makes direct delivery worth having for media: seeking in a video is the provider's + // job, and Ekklesia never sees the bytes. A proxying route would have had to implement all of it. + expect(ranged.status).toBe(206); + expect(ranged.headers.get("content-range")).toBe(`bytes 10-19/${payload.length}`); + expect(await ranged.text()).toBe("0123456789"); + expect((await fetch(delivery.url)).headers.get("accept-ranges")).toBe("bytes"); + }); + + it("returns the same URL for the same object within a window, so caches and DOM keys hold", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/stable.png`; + await bucket.put(key, "x", { contentType: "image/png" }); + + // Signing timestamps have one-second granularity of their own, so the pair has to straddle a + // second for this to mean anything — otherwise it would pass with no windowing at all. That + // introduces the one race worth handling: the pair can land either side of a window boundary, + // which is not a defect but would look like one. A crossing cannot recur immediately, so one + // retry settles it deterministically rather than leaving a rare failure in the suite. + let first = await bucket.createReadUrl(key); + await new Promise((resolve) => setTimeout(resolve, 1_100)); + let second = await bucket.createReadUrl(key); + if (second.url !== first.url) { + first = await bucket.createReadUrl(key); + await new Promise((resolve) => setTimeout(resolve, 1_100)); + second = await bucket.createReadUrl(key); + } + + // A second apart and byte-identical. Without this a client that rebuilds its state on a timer + // re-downloads every asset each time, because the URL is the browser's cache key — and a UI that + // keys elements by URL remounts them. + expect(second.url).toBe(first.url); + expect(second.expiresAt.getTime()).toBe(first.expiresAt.getTime()); + expect((await fetch(second.url)).status).toBe(200); + }); + + it("asks the browser to cache the object for no longer than the URL lives", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/cacheable.png`; + await bucket.put(key, "x", { contentType: "image/png" }); + + const delivery = await bucket.createReadUrl(key); + const response = await fetch(delivery.url); + const cacheControl = response.headers.get("cache-control") ?? ""; + + expect(cacheControl).toMatch(/^private, max-age=\d+$/u); + const maxAge = Number(/max-age=(\d+)/u.exec(cacheControl)?.[1]); + expect(maxAge).toBeGreaterThan(0); + expect(delivery.expiresAt.getTime() - Date.now()).toBeGreaterThanOrEqual(maxAge * 1000); + // Revalidation still works, which is what keeps a long-running display cheap. + expect(response.headers.get("etag")).toMatch(/"[0-9a-f]{32}"/u); + }); + + it("offers a download name when asked, and displays inline otherwise", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/bulletin.pdf`; + await bucket.put(key, "%PDF-", { contentType: "application/pdf" }); + + const inline = await fetch((await bucket.createReadUrl(key)).url); + expect(inline.headers.get("content-disposition")).toBeNull(); + + const download = await bucket.createReadUrl(key, { downloadFilename: "Sunday Bulletin.pdf" }); + const saved = await fetch(download.url); + expect(saved.status).toBe(200); + expect(saved.headers.get("content-disposition")).toBe( + 'attachment; filename="Sunday Bulletin.pdf"', + ); + }); + + describe("the browser delivery origin is a signing input, not a rewrite", () => { + const deliveryHost = "storage.example.org"; + + it("signs for the delivery origin and verifies behind a Host-preserving proxy", async () => { + const bucket = createModuleStorage({ + bucketPrefix: runPrefix, + logicalBucketIds: ["announcement-assets"], + moduleId: "announcements", + // Core still reaches storage at the endpoint it always did; only signing differs. + provider: provider(`https://${deliveryHost}`), + }).bucket("announcement-assets"); + const key = `${keyNamespace()}/proxied.png`; + await bucket.put(key, "x", { contentType: "image/png" }); + + const delivery = await bucket.createReadUrl(key); + const signed = new URL(delivery.url); + expect(signed.host).toBe(deliveryHost); + + // Delivered through a proxy that passes the original Host through: the backend verifies it, + // even though the signed scheme was https and this hop is plaintext. SigV4 covers the Host + // header, the path, and the query — not the scheme — so terminating TLS at a proxy is fine. + const throughProxy = await getWithHost(signed.pathname + signed.search, deliveryHost); + expect(throughProxy.status).toBe(200); + + // A proxy that replaces Host with its backend's breaks the signature. This is the single + // configuration requirement direct delivery imposes on a deployment. + const hostRewritten = await getWithHost( + signed.pathname + signed.search, + `${endpointOrigin().hostname}:${endpointOrigin().port}`, + ); + expect(hostRewritten.status).toBe(403); + }); + + it("cannot be faked by rewriting a URL signed for the internal endpoint", async () => { + const bucket = moduleBucket(); + const key = `${keyNamespace()}/internal.png`; + await bucket.put(key, "x"); + const delivery = await bucket.createReadUrl(key); + + const rewritten = new URL(delivery.url); + rewritten.host = deliveryHost; + const attempted = await getWithHost(rewritten.pathname + rewritten.search, deliveryHost); + + // The reason `deliveryEndpoint` exists at all: post-hoc rewriting is not an option, so the + // browser-facing origin has to be known at signing time. + expect(attempted.status).toBe(403); + }); + }); + + it("keeps a module's delivery URLs inside its own bucket", async () => { + const announcements = moduleBucket("announcements", "assets"); + const video = moduleBucket("video", "assets"); + const key = `${keyNamespace()}/same-name.txt`; + await announcements.put(key, "from-announcements"); + await video.put(key, "from-video"); + + const fromAnnouncements = await fetch((await announcements.createReadUrl(key)).url); + const fromVideo = await fetch((await video.createReadUrl(key)).url); + + // Same logical bucket name, same key, two modules — and a URL from one never reaches the other's + // object, because the physical bucket it names came from the binding rather than from anything + // the module said. + expect(await fromAnnouncements.text()).toBe("from-announcements"); + expect(await fromVideo.text()).toBe("from-video"); + }); +}); + +/** + * An Announcements-shaped module, modelled on the real slideshow this platform exists to replace. + * + * The real one, today, hardcodes an object-storage origin, lists a public bucket over anonymous + * HTTP, and hands the browser absolute URLs it built by string concatenation. This is the same + * behaviour expressed against the module contract — and the point of writing it out is what it is + * **unable** to say. Its entire world is the parameter list: a `ModuleStorage` and two names it + * declared in its own manifest. There is no endpoint to hardcode, no credential to hold, no bucket + * to name, and no branch on which host is running it, so the identical function would run unchanged + * under a single-module standalone host (ADR 0031). + */ +interface Slide { + readonly key: string; + readonly url: string; + readonly expiresAt: Date; +} + +async function announcementsSlideshowState(storage: ModuleStorage): Promise { + const assets = storage.bucket("announcement-assets"); + const listing = await assets.list({ prefix: "slides/" }); + const images = listing.objects.filter((object) => /\.(png|jpe?g|webp)$/iu.test(object.key)); + return Promise.all( + images.map(async (object) => { + const delivery = await assets.createReadUrl(object.key); + return { expiresAt: delivery.expiresAt, key: object.key, url: delivery.url }; + }), + ); +} + +describe.sequential("an Announcements-shaped module delivers slides directly", () => { + it("lists its own slides and hands a browser URLs that work", async () => { + const storage = createModuleStorage({ + bucketPrefix: runPrefix, + logicalBucketIds: ["announcement-assets"], + moduleId: "announcements", + provider: sharedProvider(), + }); + const assets = storage.bucket("announcement-assets"); + const png = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + await assets.put("slides/welcome.png", png, { contentType: "image/png" }); + await assets.put("slides/offering.jpg", png, { contentType: "image/jpeg" }); + // Not an image, and the slideshow must not try to display it. + await assets.put("slides/notes.txt", "not a slide"); + + const state = await announcementsSlideshowState(storage); + + expect(state.map((slide) => slide.key)).toEqual(["slides/offering.jpg", "slides/welcome.png"]); + + // The display fetches every slide itself. Nothing in this repository carries the image bytes. + for (const slide of state) { + const response = await fetch(slide.url); + expect(response.status).toBe(200); + expect(new Uint8Array(await response.arrayBuffer())).toEqual(png); + } + expect(await fetch(state[0]?.url ?? "").then((r) => r.headers.get("content-type"))).toBe( + "image/jpeg", + ); + }); + + it("persists logical keys, never URLs", async () => { + const storage = createModuleStorage({ + bucketPrefix: runPrefix, + logicalBucketIds: ["announcement-assets"], + moduleId: "announcements", + provider: sharedProvider(), + }); + await storage.bucket("announcement-assets").put("slides/keep.png", "x", { + contentType: "image/png", + }); + + const state = await announcementsSlideshowState(storage); + const slide = state.find((candidate) => candidate.key === "slides/keep.png"); + if (!slide) { + throw new IntegrationHarnessError("The Announcements-shaped fixture lost its own slide."); + } + + // What a module writes to its own tables is the key: it is stable, it is meaningful, and it + // still resolves tomorrow. The URL is a temporary capability that happens to be how today's + // browser reaches it, and storing one would be storing a credential with an expiry date in it. + expect(slide.key).toBe("slides/keep.png"); + expect(slide.url).not.toBe(slide.key); + expect(slide.expiresAt.getTime()).toBeGreaterThan(Date.now()); + + // The module's own durable state names nothing about the host. The URL it hands the browser + // inevitably does — that asymmetry is the whole design, and it is asserted rather than assumed. + const persisted = JSON.stringify({ key: slide.key }); + expect(persisted).not.toMatch(/http|amz|signature|ekkdel|127\.0\.0\.1/iu); + expect(new URL(slide.url).searchParams.has("X-Amz-Signature")).toBe(true); + }); + + it("builds a whole slideshow's URLs without a round trip each", async () => { + const storage = createModuleStorage({ + bucketPrefix: runPrefix, + logicalBucketIds: ["announcement-assets"], + moduleId: "announcements", + provider: sharedProvider(), + }); + const assets = storage.bucket("announcement-assets"); + + // Warm the binding first, so what is measured is signing rather than the one lazy CreateBucket + // round trip every binding pays once. + await assets.createReadUrl("slides/warm.png"); + + const startedAt = Date.now(); + const urls = await Promise.all( + Array.from({ length: 40 }, (_, index) => assets.createReadUrl(`slides/deck-${index}.png`)), + ); + const elapsed = Date.now() - startedAt; + + expect(new Set(urls.map((delivery) => delivery.url)).size).toBe(40); + // Signing is local arithmetic, so a forty-slide deck costs no provider traffic and needs no + // batch API to stay cheap. A per-URL round trip could not come close to this. + expect(elapsed).toBeLessThan(1_000); + }); +}); diff --git a/core/tests/module-storage.test.ts b/core/tests/module-storage.test.ts index 3dee37d..b7b0f6d 100644 --- a/core/tests/module-storage.test.ts +++ b/core/tests/module-storage.test.ts @@ -2,10 +2,13 @@ import { EMPTY_MODULE_STORAGE, isValidModuleStorageKey, MODULE_STORAGE_KEY_MAX_BYTES, + MODULE_STORAGE_READ_URL_DEFAULT_SECONDS, + MODULE_STORAGE_READ_URL_MAX_SECONDS, + MODULE_STORAGE_READ_URL_MIN_SECONDS, type ModuleStorageBody, ModuleStorageError, - type ModuleStorageListOptions, type ModuleStorageListing, + type ModuleStorageListOptions, type ModuleStorageObject, type ModuleStorageObjectMetadata, type ModuleStoragePutOptions, @@ -81,6 +84,15 @@ function fakeProvider(options: { failEnsure?: () => Error } = {}): StorageProvid const provider: StorageProvider = { async close() {}, + async createReadUrl(physicalBucket, key, urlOptions) { + calls.push({ key, operation: "createReadUrl", physicalBucket }); + // Shaped like a real delivery URL — opaque, single-object, carrying an expiry — without any + // signing, so facade-level tests prove scoping and validation rather than cryptography. + return Object.freeze({ + expiresAt: new Date(Date.now() + urlOptions.expiresInSeconds * 1000), + url: `https://delivery.test/${physicalBucket}/${encodeURIComponent(key)}?token=fake`, + }); + }, async delete(physicalBucket, key) { calls.push({ key, operation: "delete", physicalBucket }); objectsIn(physicalBucket).delete(key); @@ -354,7 +366,17 @@ describe("module storage scoping", () => { // The handle names the *logical* bucket, and has no accessor for anything else. expect(bucket.id).toBe("announcement-assets"); - expect(Object.keys(bucket).sort()).toEqual(["delete", "get", "head", "id", "list", "put"]); + // Still the complete surface, now including delivery: a module can ask for a browser URL but has + // gained no way to ask where the object actually lives. + expect(Object.keys(bucket).sort()).toEqual([ + "createReadUrl", + "delete", + "get", + "head", + "id", + "list", + "put", + ]); expect(Object.keys(storage).sort()).toEqual(["bucket", "buckets"]); const serialized = JSON.stringify({ bucket, storage }); expect(serialized).not.toContain( @@ -569,6 +591,9 @@ describe("storage provider configuration", () => { expect(config).toEqual({ accessKey: "AKIAEXAMPLEACCESSID", bucketPrefix: "ekklesia", + // Unset, so the one endpoint serves both roles — the zero-configuration case, and the reason + // an ordinary deployment never has to think about browser delivery at all. + deliveryEndpoint: "https://storage.example.test", endpoint: "https://storage.example.test", // Path-style by default: every self-hosted S3-compatible service needs it, and virtual-host // addressing would require wildcard DNS a typical deployment does not have. @@ -578,6 +603,85 @@ describe("storage provider configuration", () => { }); }); + it("lets one endpoint serve both roles when no delivery origin is configured", () => { + // The zero-configuration case, and the one an ordinary church deployment stays in: the + // repository's own development stack publishes storage on loopback, so the endpoint Core uses is + // already an origin a browser can reach and direct delivery needs nothing configured. + expect(createStorageProviderConfig(base)?.deliveryEndpoint).toBe( + "https://storage.example.test", + ); + expect(createStorageProviderConfig({ ...base, deliveryEndpoint: " " })?.deliveryEndpoint).toBe( + "https://storage.example.test", + ); + }); + + it("keeps the browser delivery origin separate from the endpoint Core talks to", () => { + const config = createStorageProviderConfig( + { + ...base, + deliveryEndpoint: "https://storage.example.org/", + endpoint: "http://object-storage:8333", + }, + { environment: "development" }, + ); + + // The deployment this exists for: Core reaches storage by a private service name, browsers reach + // it by a public one, and the signature has to be made for the public one because it cannot be + // rewritten afterwards. + expect(config?.endpoint).toBe("http://object-storage:8333"); + expect(config?.deliveryEndpoint).toBe("https://storage.example.org"); + }); + + it("validates the delivery origin exactly as strictly as the endpoint", () => { + expect(() => createStorageProviderConfig({ ...base, deliveryEndpoint: "not-a-url" })).toThrow( + expect.objectContaining({ code: "STORAGE_DELIVERY_ENDPOINT_INVALID" }), + ); + expect(() => + createStorageProviderConfig({ ...base, deliveryEndpoint: "ftp://storage.example.org" }), + ).toThrow(expect.objectContaining({ code: "STORAGE_DELIVERY_ENDPOINT_INVALID" })); + expect(() => + createStorageProviderConfig({ ...base, deliveryEndpoint: "https://k:s@storage.example.org" }), + ).toThrow(expect.objectContaining({ code: "STORAGE_DELIVERY_ENDPOINT_WITH_CREDENTIALS" })); + }); + + it("refuses a plaintext delivery origin in production", () => { + // A browser on an https page will not load an http asset anyway, so allowing this would produce + // URLs that are silently unusable rather than a configuration error anyone can see. + expect(() => + createStorageProviderConfig( + { ...base, deliveryEndpoint: "http://storage.example.org" }, + { environment: "production" }, + ), + ).toThrow( + expect.objectContaining({ code: "STORAGE_DELIVERY_ENDPOINT_INSECURE_IN_PRODUCTION" }), + ); + }); + + it("describes a separate delivery origin without naming a credential", () => { + const split = createStorageProviderConfig( + { + ...base, + deliveryEndpoint: "https://storage.example.org", + endpoint: "http://object-storage:8333", + }, + { environment: "development" }, + ); + const single = createStorageProviderConfig(base); + if (!split || !single) { + throw new Error("The storage configuration fixture produced no configuration."); + } + const described = describeStorageProvider(split); + + expect(described).toContain("http://object-storage:8333"); + expect(described).toContain("https://storage.example.org"); + expect(described).not.toContain("AKIAEXAMPLEACCESSID"); + expect(described).not.toContain("sk-example-secret-material"); + + // And an ordinary single-origin deployment reads exactly as it did before the distinction + // existed, so nobody is asked to understand a split that does not apply to them. + expect(describeStorageProvider(single)).not.toContain("browser delivery"); + }); + it("requires both credentials once an endpoint is set", () => { expect(() => createStorageProviderConfig({ ...base, accessKey: undefined })).toThrow( expect.objectContaining({ code: "STORAGE_ACCESS_KEY_MISSING" }), @@ -773,3 +877,133 @@ describe("an Announcements-shaped module reads its slides without knowing any in expect(JSON.stringify(caught)).not.toContain("http"); }); }); + +describe("read URLs are bounded, scoped, and provider-neutral", () => { + function bucketFor(moduleId = "announcements", logicalBucketIds = ["assets"]) { + const provider = fakeProvider(); + const storage = createModuleStorage({ + bucketPrefix: "ekklesia", + logicalBucketIds, + moduleId, + provider, + }); + return { bucket: storage.bucket(logicalBucketIds[0] ?? "assets"), provider, storage }; + } + + it("takes the host's default lifetime when the module asks for none", async () => { + const { bucket, provider } = bucketFor(); + const delivery = await bucket.createReadUrl("welcome.png"); + + // A module that expresses no opinion gets the host's, which is the case that must stay easy: + // needing to know a duration to fetch an image would be provider knowledge leaking upward. + const expectedMs = MODULE_STORAGE_READ_URL_DEFAULT_SECONDS * 1000; + expect(delivery.expiresAt.getTime() - Date.now()).toBeGreaterThan(expectedMs - 5_000); + expect(delivery.expiresAt.getTime() - Date.now()).toBeLessThanOrEqual(expectedMs); + expect(provider.calls.at(-1)).toMatchObject({ + key: "welcome.png", + operation: "createReadUrl", + physicalBucket: expect.stringContaining("ekklesia-announcements--assets-"), + }); + }); + + it("accepts a lifetime inside the allowed range", async () => { + const { bucket } = bucketFor(); + await expect( + bucket.createReadUrl("welcome.png", { expiresIn: MODULE_STORAGE_READ_URL_MIN_SECONDS }), + ).resolves.toMatchObject({ url: expect.any(String) }); + await expect( + bucket.createReadUrl("welcome.png", { expiresIn: MODULE_STORAGE_READ_URL_MAX_SECONDS }), + ).resolves.toMatchObject({ url: expect.any(String) }); + }); + + it.each([ + ["below the floor", MODULE_STORAGE_READ_URL_MIN_SECONDS - 1], + ["above the ceiling", MODULE_STORAGE_READ_URL_MAX_SECONDS + 1], + ["a year", 365 * 24 * 60 * 60], + ["zero", 0], + ["negative", -60], + ["fractional", 90.5], + ["not a number", Number.NaN], + ["infinite", Number.POSITIVE_INFINITY], + ])("refuses a lifetime %s rather than clamping it", async (_label, expiresIn) => { + const { bucket, provider } = bucketFor(); + const before = provider.calls.length; + + await expect(bucket.createReadUrl("welcome.png", { expiresIn })).rejects.toMatchObject({ + code: "STORAGE_READ_URL_EXPIRY_INVALID", + }); + // Refused in the facade, so no provider ever sees a value it might interpret differently — and + // "expires forever" is unreachable rather than merely discouraged. + expect(provider.calls.filter((call) => call.operation === "createReadUrl")).toHaveLength(0); + expect(provider.calls.length).toBe(before); + }); + + it.each([ + ["a quote", 'in"jected.pdf'], + ["a carriage return", "a\r\nX-Injected: 1.pdf"], + ["a newline", "a\nb.pdf"], + ["a path separator", "slides/welcome.png"], + ["a backslash", "slides\\welcome.png"], + ["a semicolon", "a;filename=b.pdf"], + ["nothing at all", ""], + ])("refuses a download filename containing %s", async (_label, downloadFilename) => { + const { bucket } = bucketFor(); + + // The value lands in a Content-Disposition response header, so anything that could close the + // quoted string or start a new header is refused at the contract rather than escaped and hoped + // over. + await expect(bucket.createReadUrl("bulletin.pdf", { downloadFilename })).rejects.toMatchObject({ + code: "STORAGE_READ_URL_FILENAME_INVALID", + }); + }); + + it("accepts an ordinary download filename", async () => { + const { bucket } = bucketFor(); + await expect( + bucket.createReadUrl("bulletin.pdf", { downloadFilename: "Sunday Bulletin (2026).pdf" }), + ).resolves.toMatchObject({ url: expect.any(String) }); + }); + + it("refuses an invalid key before it reaches the provider", async () => { + const { bucket, provider } = bucketFor(); + + await expect(bucket.createReadUrl("/absolute.png")).rejects.toMatchObject({ + code: "STORAGE_KEY_INVALID", + }); + expect(provider.calls.filter((call) => call.operation === "createReadUrl")).toHaveLength(0); + }); + + it("cannot produce a URL for a bucket this module did not declare", () => { + const { storage } = bucketFor("announcements", ["assets"]); + + // There is no path to another module's object at all: the name does not resolve, so delivery + // inherits scoping from the facade rather than re-implementing it. + expect(() => storage.bucket("video-assets")).toThrow(ModuleStorageError); + }); + + it("sends two modules' identical logical names to two different physical buckets", async () => { + const provider = fakeProvider(); + const urlFor = async (moduleId: string) => { + const bucket = createModuleStorage({ + bucketPrefix: "ekklesia", + logicalBucketIds: ["assets"], + moduleId, + provider, + }).bucket("assets"); + await bucket.createReadUrl("same-key.txt"); + return provider.calls.at(-1)?.physicalBucket; + }; + + expect(await urlFor("announcements")).not.toBe(await urlFor("video")); + }); + + it("returns nothing but an opaque URL and an expiry", async () => { + const { bucket } = bucketFor(); + const delivery = await bucket.createReadUrl("welcome.png"); + + // No provider identity, no physical bucket as a separate field, no credentials, no endpoint — + // a module can act on this without being able to learn anything about the host from it. + expect(Object.keys(delivery).sort()).toEqual(["expiresAt", "url"]); + expect(delivery.expiresAt).toBeInstanceOf(Date); + }); +}); diff --git a/docs/architecture/decisions/0031-direct-object-delivery.md b/docs/architecture/decisions/0031-direct-object-delivery.md new file mode 100644 index 0000000..c3547dc --- /dev/null +++ b/docs/architecture/decisions/0031-direct-object-delivery.md @@ -0,0 +1,214 @@ +# 0031: Browsers fetch objects directly, through opaque temporary read URLs + +- Status: Accepted +- Date: 2026-08-23 +- Extends: [0004](0004-replace-unmaintained-minio.md), + [0021](0021-modules-as-hostable-products.md), + [0028](0028-module-operational-lifecycle.md), + [0030](0030-host-provided-module-object-storage.md) + +## Context + +[ADR 0030](0030-host-provided-module-object-storage.md) gave a module a scoped `context.storage` and +took every provider detail away from it. That is right for module code, but it left exactly one way +to get an image to a browser: read the object into the module's process and write it back out of a +module HTTP route. Every byte of every asset travels storage → Ekklesia → browser, and Ekklesia's +event loop carries it. + +For the product this platform exists to replace that is not a hypothetical cost. `The_River/Announcements` +(commit `9aab486`) is a slideshow: a display renders full-screen images for hours, rotating every +seven seconds, and the server rebuilds its slide list every thirty. Proxying it would put Ekklesia in +the path of every image on every display in the building, permanently, to deliver bytes that never +change. Worse, it would make Ekklesia responsible for re-implementing what an object store already +does — byte ranges above all, without which video seeking cannot work at all. + +The existing application solves this by making the bucket world-readable and hardcoding its address: + +```js +slides = files.map((name) => `http://10.40.0.2:9000/announcements/${encode(name)}`); +``` + +Direct delivery, at the cost of a permanently public bucket, an origin compiled into the client, and +no per-object control of any kind. We want the delivery path and none of the cost. + +## Decision + +A module can ask a logical bucket for a temporary, browser-usable URL for one object: + +```ts +const assets = context.storage.bucket("announcement-assets"); +const { url, expiresAt } = await assets.createReadUrl("slides/welcome.png"); +``` + +The browser then fetches the object from the host's storage service directly. Ekklesia is not in the +data path. + +### The contract names intent, not mechanism + +`createReadUrl` is the whole surface. There is no `presign`, no expiry format, no signature, no +region, no endpoint, and no bucket — a module names the key it already knows and receives something a +browser can use. The first implementation is S3-compatible presigned `GET`, but nothing about that +reaches the contract: a host with no S3 anywhere in it could satisfy the same method by minting its +own opaque token, and no module would change. This is the same reasoning that keeps `ModuleStorage` +free of provider vocabulary in ADR 0030, applied to delivery. + +The return value is `{ url, expiresAt }` and deliberately nothing else. `expiresAt` exists so that a +caller can decide *when to ask again* without reading the URL, which is what makes "the URL is +opaque" an honest rule rather than an aspiration. + +### A read URL is a bearer capability + +Anyone holding the URL can read that one object until it expires. It is not an authenticated +request, and this ADR does not pretend otherwise. + +The division of responsibility is therefore explicit: + +- **Whether this person may have the URL** is the module's own authorization decision, made in its + route before it asks for one. Storage does not know about people, permissions, or principals. +- **How far the URL reaches once issued** is the storage contract's guarantee: one object, in one + module's bucket, for a bounded time. + +Storage authorization does not replace domain authorization, and the contract deliberately offers no +hook that would let it try. + +### Expiry is bounded, and the bounds come from the reference consumer + +- **Default 15 minutes.** Announcements rebuilds its slide list every 30 seconds and re-broadcasts + state every 7. Fifteen minutes is roughly thirty times that natural refresh, so a display that + loses its connection for ten minutes still renders every slide the moment it reconnects — without + anyone building a refresh mechanism. +- **Floor of 1 minute.** Below that, ordinary clock skew between Ekklesia and the storage service + can expire a URL before a browser can spend it. +- **Ceiling of 6 hours.** This bounds the damage when a URL leaks. It outlasts any single gathering + while staying nowhere near a durable credential, and it binds well before the seven-day maximum + S3-compatible signing itself permits — so the contract's limit is the one that applies. + +Out-of-range requests are **refused, not clamped**. Silently shortening a lifetime would surface +later as URLs expiring for no visible reason; silently lengthening one would weaken a capability +without telling anyone. "Expires forever" is unreachable rather than discouraged. + +### Logical keys are canonical state; URLs are not + +A module persists the object key. It generates a URL when something is about to be delivered and then +forgets it. Storing a URL would be storing a credential with an expiry date embedded in it, and it +would break the moment the deployment's delivery origin changed. + +### Read URLs are stable within a window + +Signing is a pure function of the request and the clock, so signing the same object twice a second +apart produces two different URLs for identical bytes. That is not merely untidy. A browser caches by +URL, so a changed query string is a guaranteed cache miss — and UI frameworks routinely key list +items by URL. The reference client renders: + +```jsx + +``` + +A new URL for an unchanged image therefore tears the element down and re-downloads it. A slideshow +rebuilding its state every few seconds would re-fetch every slide continuously, having gained a +direct path to storage and lost the caching that made it worth having. + +So the signing timestamp is rounded down to a window of one third of the requested lifetime. Repeated +calls within that window return a **byte-identical** URL, and at least two-thirds of the lifetime +always remains when one is handed out. Ekklesia also sets `Cache-Control: private, max-age=N` at +signing time, where `N` is the guaranteed remainder — the pinned SeaweedFS image returns no +`Cache-Control` of its own, which would otherwise leave browsers guessing. + +### The provider endpoint and the browser delivery origin are different things + +They cannot be collapsed, and this was measured rather than assumed. An S3-compatible signature +covers the request's **`Host` header**, so a URL signed for one origin and then re-pointed at another +fails verification. Rewriting after signing is not available. + +The browser-facing origin is therefore a **signing input**. `S3_DELIVERY_ENDPOINT` sets it, and when +unset it is the provider endpoint — which is the ordinary case and the reason a normal deployment +configures nothing. + +What the signature does **not** cover is the scheme. Terminating TLS at a reverse proxy and +forwarding plaintext to the storage service verifies correctly, **provided the proxy passes the +original `Host` through unchanged**. That is the single configuration requirement direct delivery +imposes on a deployment, and it is pinned by an integration test rather than described here and +hoped for. + +### Zero configuration for an ordinary deployment + +The development stack publishes the storage service on loopback, so `S3_ENDPOINT` is already an +origin a browser can reach and serves both roles. An operator running the standard stack configures +no endpoint, no keys, no bucket, and no delivery origin; modules declare logical buckets, bindings +provision themselves, and delivery URLs work. `S3_DELIVERY_ENDPOINT` exists for the deployment where +Core reaches storage by a private service name — and only then. + +### What a browser sees + +A read URL reveals the storage origin, the physical bucket name, the object key, and signing query +parameters. This is accepted and stated plainly rather than worked around. + +Physical bucket names are **implementation metadata, not secrets**. Nothing is authorized by knowing +one: the bucket is not publicly readable, and a URL's signature covers its own path, so editing the +bucket or key in a URL produces a rejection rather than another object. Hiding them would mean +reintroducing a proxy layer and recreating the data hop this ADR exists to remove — a real cost for a +cosmetic gain. + +The distinction that matters is between **module code**, which must never depend on these details and +cannot reach them, and **the browser**, which inevitably sees the URL it is fetching. + +## Consequences + +### What this buys + +Byte ranges work directly against the storage service — verified, `206` with a correct +`Content-Range` — so video seeking becomes the provider's job and needs no range proxy in Core. This +was the single largest reason to prefer direct delivery over proxying, and it removes a whole +subsystem from the platform's future. + +Signing is a local computation: 200 URLs took 66 ms with no network traffic. A slideshow can build +forty delivery URLs as cheaply as it builds forty strings, so no batch API is needed and none is +added. + +### What it costs + +A leaked URL is usable until it expires. That is the nature of the mechanism, and it is why the +ceiling is 6 hours rather than a day and why URLs are never logged. + +`createReadUrl` does not check that the object exists. A URL for a missing key — or one deleted after +signing — returns `404` when fetched, which is what a browser already handles. Verifying up front +would add a round trip to every URL, and a module listing forty slides would pay forty of them for +something it just read. + +### Logging and diagnostics + +A read URL is treated as a bearer token. Complete URLs are never logged and never printed by Doctor. +Doctor reports the storage origin, and the delivery origin when it differs, and says explicitly that +a separate delivery origin was **not** probed — reporting "reachable" without that caveat would let +an operator conclude direct delivery works when only half of it was tested. + +### CORS + +The pinned SeaweedFS image reflects any `Origin` back in `Access-Control-Allow-Origin` and does not +set `Access-Control-Allow-Credentials`. Read URLs carry no cookies and no browser credentials, so a +reflected origin grants nothing beyond what possession of the URL already grants. Ekklesia therefore +configures no CORS policy of its own in this pass and relies on the provider's default, which is +sufficient for element loads (``, `