From aae1d471483aacb02f28e2900463302c722f584c Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Tue, 23 Jun 2026 19:33:19 -0700 Subject: [PATCH 1/3] feat(storage): add storage-disk abstraction with local and S3 drivers (#3157) Introduce a pluggable storage layer under vendor/wheels/storage: a small uniform disk interface (put/get/exists/delete/url/signedUrl), LocalDisk and S3Disk drivers, and a StorageManager that resolves named disks from config (mirroring service()/model() resolution). S3 access uses a from-scratch SigV4 signer (no AWS SDK, no JARs): presigned expiring GET URLs plus Authorization-header signing for cfhttp put/get/delete. The HMAC key-derivation and SHA-256 canonical hashing reuse the same cross-engine-green primitives as wheels.auth.JwtService. Phase 1 of #2962. The hasOneAttached model macro (Phase 2), the built-in local-file-serving route, and the storage() controller/model mixin helper are deferred to follow-ups. Tests: storage spec green on Lucee 7 and Adobe CF 2023 (20/20 each); the SigV4 presign test pins the AWS-documented test vector. Fixed a reserved-scope shadowing bug in the spec (Anti-Pattern #11): `var url` reads the URL scope on both engines, so `expect(url)` saw the request struct, not the return value; renamed to `presigned`. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Peter Amiri --- .../3157-storage-disk-abstraction.added.md | 1 + .../interfaces/StorageDiskInterface.cfc | 67 +++++ vendor/wheels/storage/S3Signer.cfc | 272 ++++++++++++++++++ vendor/wheels/storage/StorageManager.cfc | 90 ++++++ vendor/wheels/storage/drivers/LocalDisk.cfc | 151 ++++++++++ vendor/wheels/storage/drivers/S3Disk.cfc | 133 +++++++++ .../tests/specs/storage/StorageSpec.cfc | 260 +++++++++++++++++ 7 files changed, 974 insertions(+) create mode 100644 changelog.d/3157-storage-disk-abstraction.added.md create mode 100644 vendor/wheels/interfaces/StorageDiskInterface.cfc create mode 100644 vendor/wheels/storage/S3Signer.cfc create mode 100644 vendor/wheels/storage/StorageManager.cfc create mode 100644 vendor/wheels/storage/drivers/LocalDisk.cfc create mode 100644 vendor/wheels/storage/drivers/S3Disk.cfc create mode 100644 vendor/wheels/tests/specs/storage/StorageSpec.cfc diff --git a/changelog.d/3157-storage-disk-abstraction.added.md b/changelog.d/3157-storage-disk-abstraction.added.md new file mode 100644 index 0000000000..bea23e98f9 --- /dev/null +++ b/changelog.d/3157-storage-disk-abstraction.added.md @@ -0,0 +1 @@ +- Added a pluggable storage-disk abstraction under `wheels.storage` with `LocalDisk` and `S3Disk` drivers behind a uniform `put/get/exists/delete/url/signedUrl` interface, resolved by name through `StorageManager`. S3 access — including presigned, expiring URLs — uses a from-scratch SigV4 signer over plain `cfhttp` (no AWS SDK, no JARs) (#3157). diff --git a/vendor/wheels/interfaces/StorageDiskInterface.cfc b/vendor/wheels/interfaces/StorageDiskInterface.cfc new file mode 100644 index 0000000000..22c0d16758 --- /dev/null +++ b/vendor/wheels/interfaces/StorageDiskInterface.cfc @@ -0,0 +1,67 @@ +/** + * Contract every storage disk driver must satisfy. + * + * A "disk" is a named, configured storage backend (local filesystem, S3, …). + * Drivers expose one small, uniform surface so application code can swap + * backends with a config change and no call-site edits — mirroring Laravel's + * Filesystem and Rails' ActiveStorage::Service abstractions. + * + * Implementations live under `wheels.storage.drivers.*`. Resolve a configured + * disk through `wheels.storage.StorageManager`. + * + * [section: Storage] + * [category: Interface] + */ +interface { + + /** + * Store content at the given key, creating intermediate paths as needed. + * + * @key The opaque storage key (path-like, forward-slash separated). + * @content Binary or string content to write. + * @contentType MIME type hint (used by cloud backends; ignored by local). + * @visibility "public" or "private"; backends that support ACLs honour it. + * @return The stored key. + */ + public any function put(required string key, required any content, string contentType, string visibility); + + /** + * Read the content stored at the given key as binary. + * + * @key The storage key. + * @return Binary content. Throws Wheels.Storage.NotFound when absent. + */ + public any function get(required string key); + + /** + * Whether an object exists at the given key. + * + * @key The storage key. + */ + public boolean function exists(required string key); + + /** + * Delete the object at the given key. + * + * @key The storage key. + * @return true when an object was deleted, false when nothing was there. + */ + public boolean function delete(required string key); + + /** + * A non-expiring URL for the object (public objects / served route). + * + * @key The storage key. + */ + public string function url(required string key); + + /** + * A signed, time-limited URL granting temporary access to a private object. + * + * @key The storage key. + * @expiresIn Seconds until the URL expires (default 300). + * @contentDisposition Optional Content-Disposition the download should carry. + */ + public string function signedUrl(required string key, numeric expiresIn, string contentDisposition); + +} diff --git a/vendor/wheels/storage/S3Signer.cfc b/vendor/wheels/storage/S3Signer.cfc new file mode 100644 index 0000000000..fb4d599f2a --- /dev/null +++ b/vendor/wheels/storage/S3Signer.cfc @@ -0,0 +1,272 @@ +/** + * AWS Signature Version 4 signer for S3, implemented from scratch — no AWS SDK, + * no JARs. Generates presigned GET URLs (query-string auth) and Authorization + * headers (header auth) for arbitrary S3 requests issued via `cfhttp`. + * + * The crypto primitives are the same proven, cross-engine-green building blocks + * `wheels.auth.JwtService` relies on (SHA-256 hex hashing + a chained HMAC-SHA256 + * key-derivation), driven through `javax.crypto.Mac` so binary signing keys work + * identically on Lucee 5/6/7, Adobe CF 2018-2025, and BoxLang. + * + * Reference: AWS "Authenticating Requests: Using Query Parameters (AWS Signature + * Version 4)" and "...Using the Authorization Header...". + * + * Usage: + * var signer = new wheels.storage.S3Signer( + * accessKeyId="AKIA…", secretAccessKey="…", region="us-east-1", bucket="my-bucket" + * ); + * var url = signer.presignGetUrl(key="reports/q3.pdf", expiresIn=300); + * + * [section: Storage] + * [category: Core] + */ +component output="false" { + + /** + * @accessKeyId AWS access key id. + * @secretAccessKey AWS secret access key. + * @region AWS region (e.g. "us-east-1"). + * @bucket S3 bucket name. + * @endpoint Override the host (e.g. for S3-compatible stores). Empty => derive from bucket+region. + * @usePathStyle When true, addresses as host/bucket/key rather than bucket.host/key. + */ + public S3Signer function init( + required string accessKeyId, + required string secretAccessKey, + required string region, + required string bucket, + string endpoint = "", + boolean usePathStyle = false + ) { + variables.accessKeyId = arguments.accessKeyId; + variables.secretAccessKey = arguments.secretAccessKey; + variables.region = arguments.region; + variables.bucket = arguments.bucket; + variables.usePathStyle = arguments.usePathStyle; + variables.service = "s3"; + + if (Len(arguments.endpoint)) { + variables.host = arguments.endpoint; + } else if (arguments.usePathStyle) { + variables.host = "s3." & arguments.region & ".amazonaws.com"; + } else { + variables.host = arguments.bucket & ".s3." & arguments.region & ".amazonaws.com"; + } + + variables.javaSystem = CreateObject("java", "java.lang.System"); + return this; + } + + /** + * Build a presigned GET URL for an object key. + * + * @key Object key (path-like; slashes preserved). + * @expiresIn Seconds until the link expires (default 300, max 604800 per SigV4). + * @contentDisposition Optional response-content-disposition override S3 will echo. + * @amzDate Optional ISO8601 basic timestamp ("yyyymmddTHHnnssZ"). Defaults to now (UTC). Overridable for deterministic tests. + */ + public string function presignGetUrl( + required string key, + numeric expiresIn = 300, + string contentDisposition = "", + string amzDate = "" + ) { + local.amzDate = Len(arguments.amzDate) ? arguments.amzDate : $amzNow(); + local.dateStamp = Left(local.amzDate, 8); + local.credentialScope = local.dateStamp & "/" & variables.region & "/" & variables.service & "/aws4_request"; + + // Canonical URI: path-style prefixes the bucket; virtual-hosted does not. + local.canonicalUri = variables.usePathStyle + ? "/" & $uriEncodePath(variables.bucket & "/" & arguments.key) + : "/" & $uriEncodePath(arguments.key); + + // Canonical query string — keys must be sorted by their encoded name. + local.params = { + "X-Amz-Algorithm" = "AWS4-HMAC-SHA256", + "X-Amz-Credential" = variables.accessKeyId & "/" & local.credentialScope, + "X-Amz-Date" = local.amzDate, + "X-Amz-Expires" = arguments.expiresIn, + "X-Amz-SignedHeaders" = "host" + }; + if (Len(arguments.contentDisposition)) { + local.params["response-content-disposition"] = arguments.contentDisposition; + } + local.canonicalQuery = $buildCanonicalQuery(local.params); + + local.canonicalHeaders = "host:" & variables.host & Chr(10); + local.signedHeaders = "host"; + local.payloadHash = "UNSIGNED-PAYLOAD"; + + local.canonicalRequest = "GET" & Chr(10) + & local.canonicalUri & Chr(10) + & local.canonicalQuery & Chr(10) + & local.canonicalHeaders & Chr(10) + & local.signedHeaders & Chr(10) + & local.payloadHash; + + local.signature = $signString(local.canonicalRequest, local.amzDate, local.dateStamp, local.credentialScope); + + local.scheme = "https://"; + return local.scheme & variables.host & local.canonicalUri & "?" & local.canonicalQuery + & "&X-Amz-Signature=" & local.signature; + } + + /** + * Sign an arbitrary S3 request, returning the headers (incl. Authorization) + * a caller adds to a `cfhttp` invocation. Used for put/get/delete/exists. + * + * @method HTTP verb. + * @key Object key. + * @payload Request body (binary or string); empty for GET/DELETE/HEAD. + * @amzDate Optional deterministic timestamp override. + * @return Struct of header name => value to attach to the request. + */ + public struct function signedHeaders( + required string method, + required string key, + any payload = "", + string amzDate = "" + ) { + local.amzDate = Len(arguments.amzDate) ? arguments.amzDate : $amzNow(); + local.dateStamp = Left(local.amzDate, 8); + local.credentialScope = local.dateStamp & "/" & variables.region & "/" & variables.service & "/aws4_request"; + + local.payloadHash = $sha256Hex(arguments.payload); + + local.canonicalUri = variables.usePathStyle + ? "/" & $uriEncodePath(variables.bucket & "/" & arguments.key) + : "/" & $uriEncodePath(arguments.key); + + // Headers signed for header-auth: host, x-amz-content-sha256, x-amz-date (sorted). + local.canonicalHeaders = "host:" & variables.host & Chr(10) + & "x-amz-content-sha256:" & local.payloadHash & Chr(10) + & "x-amz-date:" & local.amzDate & Chr(10); + local.signedHeaderList = "host;x-amz-content-sha256;x-amz-date"; + + local.canonicalRequest = UCase(arguments.method) & Chr(10) + & local.canonicalUri & Chr(10) + & "" & Chr(10) + & local.canonicalHeaders & Chr(10) + & local.signedHeaderList & Chr(10) + & local.payloadHash; + + local.signature = $signString(local.canonicalRequest, local.amzDate, local.dateStamp, local.credentialScope); + + local.authorization = "AWS4-HMAC-SHA256 " + & "Credential=" & variables.accessKeyId & "/" & local.credentialScope & ", " + & "SignedHeaders=" & local.signedHeaderList & ", " + & "Signature=" & local.signature; + + return { + "Authorization" = local.authorization, + "x-amz-content-sha256" = local.payloadHash, + "x-amz-date" = local.amzDate, + "Host" = variables.host + }; + } + + /** + * The resolved request host (virtual-hosted or path-style endpoint). + */ + public string function getHost() { + return variables.host; + } + + // ---- internals -------------------------------------------------------- + + /** + * Produce the lowercase-hex SigV4 signature for a canonical request. + */ + private string function $signString( + required string canonicalRequest, + required string amzDate, + required string dateStamp, + required string credentialScope + ) { + local.stringToSign = "AWS4-HMAC-SHA256" & Chr(10) + & arguments.amzDate & Chr(10) + & arguments.credentialScope & Chr(10) + & $sha256Hex(arguments.canonicalRequest); + + local.signingKey = $signingKey(arguments.dateStamp); + return LCase(BinaryEncode($hmac(local.signingKey, local.stringToSign), "hex")); + } + + /** + * Derive the SigV4 signing key: HMAC chain seeded with "AWS4"+secret. + */ + private binary function $signingKey(required string dateStamp) { + local.kSecret = CharsetDecode("AWS4" & variables.secretAccessKey, "UTF-8"); + local.kDate = $hmac(local.kSecret, arguments.dateStamp); + local.kRegion = $hmac(local.kDate, variables.region); + local.kService = $hmac(local.kRegion, variables.service); + return $hmac(local.kService, "aws4_request"); + } + + /** + * HMAC-SHA256 with a binary key, returning raw bytes. Uses javax.crypto.Mac + * directly so successive rounds can key off the previous round's binary + * output — the built-in HMac() takes only string keys. + */ + private binary function $hmac(required binary key, required string message) { + local.mac = CreateObject("java", "javax.crypto.Mac").getInstance("HmacSHA256"); + local.keySpec = CreateObject("java", "javax.crypto.spec.SecretKeySpec").init(arguments.key, "HmacSHA256"); + local.mac.init(local.keySpec); + return local.mac.doFinal(CharsetDecode(arguments.message, "UTF-8")); + } + + /** + * Lowercase hex SHA-256 of a string or binary payload. + */ + private string function $sha256Hex(required any content) { + if (IsBinary(arguments.content)) { + return LCase(Hash(arguments.content, "SHA-256")); + } + return LCase(Hash(arguments.content, "SHA-256", "UTF-8")); + } + + /** + * Build a sorted, RFC3986-encoded canonical query string from a struct. + */ + private string function $buildCanonicalQuery(required struct params) { + local.keys = StructKeyArray(arguments.params); + // SigV4 sorts by raw byte order of the encoded key name; for our fixed + // ASCII parameter names a case-sensitive text sort is byte-identical. + ArraySort(local.keys, "text"); + local.pairs = []; + for (local.k in local.keys) { + ArrayAppend(local.pairs, $uriEncodeSegment(local.k) & "=" & $uriEncodeSegment(arguments.params[local.k])); + } + return ArrayToList(local.pairs, "&"); + } + + /** + * RFC3986 encode a single value (slashes ARE encoded). Built on + * java.net.URLEncoder with the AWS-required fix-ups so it is byte-identical + * across engines. + */ + private string function $uriEncodeSegment(required any value) { + local.encoder = CreateObject("java", "java.net.URLEncoder"); + local.encoded = local.encoder.encode(ToString(arguments.value), "UTF-8"); + local.encoded = Replace(local.encoded, "+", "%20", "all"); + local.encoded = Replace(local.encoded, "*", "%2A", "all"); + local.encoded = Replace(local.encoded, "%7E", "~", "all"); + return local.encoded; + } + + /** + * RFC3986 encode an object key path, preserving forward slashes. + */ + private string function $uriEncodePath(required string key) { + return Replace($uriEncodeSegment(arguments.key), "%2F", "/", "all"); + } + + /** + * Current UTC time as an ISO8601 basic timestamp ("yyyymmddTHHnnssZ"). + */ + private string function $amzNow() { + local.utc = DateConvert("local2utc", Now()); + return DateFormat(local.utc, "yyyymmdd") & "T" & TimeFormat(local.utc, "HHmmss") & "Z"; + } + +} diff --git a/vendor/wheels/storage/StorageManager.cfc b/vendor/wheels/storage/StorageManager.cfc new file mode 100644 index 0000000000..51dce0059b --- /dev/null +++ b/vendor/wheels/storage/StorageManager.cfc @@ -0,0 +1,90 @@ +/** + * Resolves named storage disks from configuration and caches them. + * + * Mirrors Laravel's filesystem manager and AdonisJS Drive: configuration names + * each disk and assigns it a `driver` ("local" or "s3"); application code asks + * for a disk by name (or the default) and gets a uniform + * `wheels.interfaces.StorageDiskInterface` back. Register the manager as a + * singleton in `config/services.cfm` and expose it via a `storage()` helper. + * + * Config shape (set in config/settings.cfm): + * set(storage = { + * default = "local", + * disks = { + * local = { driver="local", root="/storage/uploads", urlPrefix="/uploads" }, + * s3 = { driver="s3", bucket="…", region="…", accessKeyId="…", secretAccessKey="…" } + * } + * }); + * + * [section: Storage] + * [category: Core] + */ +component output="false" { + + variables.drivers = { + "local" = "wheels.storage.drivers.LocalDisk", + "s3" = "wheels.storage.drivers.S3Disk" + }; + + /** + * @config The storage config struct: { default, disks: { : { driver, … } } }. + */ + public StorageManager function init(struct config = {}) { + variables.config = arguments.config; + variables.default = StructKeyExists(arguments.config, "default") ? arguments.config.default : "local"; + variables.disks = StructKeyExists(arguments.config, "disks") ? arguments.config.disks : {}; + variables.resolved = {}; + return this; + } + + /** + * Resolve a disk by name (default disk when omitted). Disks are lazily + * instantiated and cached for the manager's lifetime. + * + * @name The configured disk name. + */ + public any function disk(string name = "") { + local.diskName = Len(arguments.name) ? arguments.name : variables.default; + + if (StructKeyExists(variables.resolved, local.diskName)) { + return variables.resolved[local.diskName]; + } + + if (!StructKeyExists(variables.disks, local.diskName)) { + throw( + type = "Wheels.Storage.UnknownDisk", + message = "No storage disk named [#local.diskName#] is configured.", + extendedInfo = "Configured disks: #StructKeyList(variables.disks)#." + ); + } + + local.diskConfig = variables.disks[local.diskName]; + local.driverName = LCase(StructKeyExists(local.diskConfig, "driver") ? local.diskConfig.driver : ""); + if (!StructKeyExists(variables.drivers, local.driverName)) { + throw( + type = "Wheels.Storage.UnknownDriver", + message = "Disk [#local.diskName#] uses unknown driver [#local.driverName#].", + extendedInfo = "Known drivers: #StructKeyList(variables.drivers)#." + ); + } + + local.instance = CreateObject("component", variables.drivers[local.driverName]).init(config = local.diskConfig); + variables.resolved[local.diskName] = local.instance; + return local.instance; + } + + /** + * The name of the default disk. + */ + public string function getDefaultDiskName() { + return variables.default; + } + + /** + * Names of all configured disks. + */ + public array function diskNames() { + return StructKeyArray(variables.disks); + } + +} diff --git a/vendor/wheels/storage/drivers/LocalDisk.cfc b/vendor/wheels/storage/drivers/LocalDisk.cfc new file mode 100644 index 0000000000..9c8196ce4e --- /dev/null +++ b/vendor/wheels/storage/drivers/LocalDisk.cfc @@ -0,0 +1,151 @@ +/** + * Local filesystem storage disk. + * + * Stores objects under a configured `root` directory and exposes them through + * a `urlPrefix` (served by the application). Signed URLs carry an HMAC token + * over the key + expiry — there is no native filesystem presigning, so, like + * every framework that ships a local disk (Rails DiskController, Laravel + * `serve`, AdonisJS `serveFiles`), the application is expected to verify the + * token before streaming the file. + * + * [section: Storage] + * [category: Driver] + */ +component implements="wheels.interfaces.StorageDiskInterface" output="false" { + + /** + * @config Disk config: { root (required), urlPrefix="", signingKey="" }. + */ + public LocalDisk function init(required struct config) { + if (!StructKeyExists(arguments.config, "root") || !Len(arguments.config.root)) { + throw( + type = "Wheels.Storage.InvalidConfiguration", + message = "Local disk requires a non-empty 'root' directory." + ); + } + variables.root = $normalizeDir(arguments.config.root); + variables.urlPrefix = StructKeyExists(arguments.config, "urlPrefix") ? arguments.config.urlPrefix : ""; + variables.signingKey = StructKeyExists(arguments.config, "signingKey") ? arguments.config.signingKey : ""; + return this; + } + + public any function put(required string key, required any content, string contentType = "", string visibility = "") { + local.path = $resolve(arguments.key); + $ensureParentDir(local.path); + FileWrite(local.path, arguments.content); + return arguments.key; + } + + public any function get(required string key) { + local.path = $resolve(arguments.key); + if (!FileExists(local.path)) { + throw( + type = "Wheels.Storage.NotFound", + message = "No object stored at key [#arguments.key#]." + ); + } + return FileReadBinary(local.path); + } + + public boolean function exists(required string key) { + return FileExists($resolve(arguments.key)); + } + + public boolean function delete(required string key) { + local.path = $resolve(arguments.key); + if (FileExists(local.path)) { + FileDelete(local.path); + return true; + } + return false; + } + + public string function url(required string key) { + return $joinUrl(variables.urlPrefix, arguments.key); + } + + public string function signedUrl(required string key, numeric expiresIn = 300, string contentDisposition = "") { + if (!Len(variables.signingKey)) { + throw( + type = "Wheels.Storage.MissingSigningKey", + message = "Local disk signedUrl() requires a 'signingKey' in the disk config." + ); + } + local.expiresAt = $epochSeconds() + arguments.expiresIn; + local.token = $sign(arguments.key & "|" & local.expiresAt); + local.base = $joinUrl(variables.urlPrefix, arguments.key); + local.qs = "expires=" & local.expiresAt & "&signature=" & local.token; + if (Len(arguments.contentDisposition)) { + local.qs &= "&disposition=" & $uriEncode(arguments.contentDisposition); + } + return local.base & "?" & local.qs; + } + + /** + * Verify a signed-URL token for the application's serving route. + * + * @key The requested key. + * @expires The epoch-seconds expiry carried in the URL. + * @signature The token carried in the URL. + */ + public boolean function verifySignature(required string key, required numeric expires, required string signature) { + if (!Len(variables.signingKey)) { + return false; + } + if ($epochSeconds() > arguments.expires) { + return false; + } + local.expected = $sign(arguments.key & "|" & arguments.expires); + // Constant-time-ish compare on equal-length hex tokens. + return CompareNoCase(local.expected, arguments.signature) == 0; + } + + // ---- internals -------------------------------------------------------- + + private string function $sign(required string message) { + return LCase(HMac(arguments.message, variables.signingKey, "HMACSHA256", "UTF-8")); + } + + private string function $resolve(required string key) { + // Reject traversal — a key must stay inside root. + local.clean = Replace(arguments.key, "\", "/", "all"); + if (Find("..", local.clean)) { + throw( + type = "Wheels.Storage.InvalidKey", + message = "Storage key [#arguments.key#] must not contain '..'." + ); + } + return variables.root & "/" & local.clean; + } + + private string function $normalizeDir(required string dir) { + local.d = Replace(arguments.dir, "\", "/", "all"); + return REReplace(local.d, "/+$", ""); + } + + private void function $ensureParentDir(required string path) { + local.parent = GetDirectoryFromPath(arguments.path); + // Use java.io.File.mkdirs() rather than the Lucee-only DirectoryCreate + // recurse flag so directory creation behaves on every engine. + local.file = CreateObject("java", "java.io.File").init(local.parent); + if (!local.file.exists()) { + local.file.mkdirs(); + } + } + + private string function $joinUrl(required string prefix, required string key) { + local.p = REReplace(arguments.prefix, "/+$", ""); + local.k = REReplace(arguments.key, "^/+", ""); + return local.p & "/" & local.k; + } + + private string function $uriEncode(required string value) { + local.encoded = CreateObject("java", "java.net.URLEncoder").encode(arguments.value, "UTF-8"); + return Replace(local.encoded, "+", "%20", "all"); + } + + private numeric function $epochSeconds() { + return Int(CreateObject("java", "java.lang.System").currentTimeMillis() / 1000); + } + +} diff --git a/vendor/wheels/storage/drivers/S3Disk.cfc b/vendor/wheels/storage/drivers/S3Disk.cfc new file mode 100644 index 0000000000..443a961ecd --- /dev/null +++ b/vendor/wheels/storage/drivers/S3Disk.cfc @@ -0,0 +1,133 @@ +/** + * Amazon S3 (and S3-compatible) storage disk. + * + * Talks to S3 over plain `cfhttp` with from-scratch SigV4 request signing — + * no AWS SDK, no JARs (see `wheels.storage.S3Signer`). `url()` returns the + * object's public URL; `signedUrl()` returns a presigned, expiring GET URL. + * + * [section: Storage] + * [category: Driver] + */ +component implements="wheels.interfaces.StorageDiskInterface" output="false" { + + /** + * @config Disk config: { bucket, region, accessKeyId, secretAccessKey, + * visibility="private", endpoint="", usePathStyle=false }. + */ + public S3Disk function init(required struct config) { + for (local.required in ["bucket", "region", "accessKeyId", "secretAccessKey"]) { + if (!StructKeyExists(arguments.config, local.required) || !Len(arguments.config[local.required])) { + throw( + type = "Wheels.Storage.InvalidConfiguration", + message = "S3 disk requires a non-empty '#local.required#'." + ); + } + } + variables.bucket = arguments.config.bucket; + variables.region = arguments.config.region; + variables.visibility = StructKeyExists(arguments.config, "visibility") ? arguments.config.visibility : "private"; + variables.usePathStyle = StructKeyExists(arguments.config, "usePathStyle") ? arguments.config.usePathStyle : false; + variables.endpoint = StructKeyExists(arguments.config, "endpoint") ? arguments.config.endpoint : ""; + + variables.signer = new wheels.storage.S3Signer( + accessKeyId = arguments.config.accessKeyId, + secretAccessKey = arguments.config.secretAccessKey, + region = arguments.config.region, + bucket = arguments.config.bucket, + endpoint = variables.endpoint, + usePathStyle = variables.usePathStyle + ); + return this; + } + + public any function put(required string key, required any content, string contentType = "application/octet-stream", string visibility = "") { + local.headers = variables.signer.signedHeaders(method = "PUT", key = arguments.key, payload = arguments.content); + local.result = $request(method = "PUT", key = arguments.key, headers = local.headers, body = arguments.content, contentType = arguments.contentType); + if (Val(local.result.statusCode) >= 300) { + throw(type = "Wheels.Storage.RequestFailed", message = "S3 PUT failed for [#arguments.key#]: #local.result.statusCode#."); + } + return arguments.key; + } + + public any function get(required string key) { + local.headers = variables.signer.signedHeaders(method = "GET", key = arguments.key); + local.result = $request(method = "GET", key = arguments.key, headers = local.headers, getAsBinary = true); + if (Val(local.result.statusCode) == 404) { + throw(type = "Wheels.Storage.NotFound", message = "No object stored at key [#arguments.key#]."); + } + if (Val(local.result.statusCode) >= 300) { + throw(type = "Wheels.Storage.RequestFailed", message = "S3 GET failed for [#arguments.key#]: #local.result.statusCode#."); + } + return local.result.fileContent; + } + + public boolean function exists(required string key) { + local.headers = variables.signer.signedHeaders(method = "HEAD", key = arguments.key); + local.result = $request(method = "HEAD", key = arguments.key, headers = local.headers); + return Val(local.result.statusCode) < 300; + } + + public boolean function delete(required string key) { + local.headers = variables.signer.signedHeaders(method = "DELETE", key = arguments.key); + local.result = $request(method = "DELETE", key = arguments.key, headers = local.headers); + // S3 DELETE is idempotent — 204 whether or not the object existed. + return Val(local.result.statusCode) < 300; + } + + public string function url(required string key) { + return "https://" & variables.signer.getHost() & $objectPath(arguments.key); + } + + public string function signedUrl(required string key, numeric expiresIn = 300, string contentDisposition = "") { + return variables.signer.presignGetUrl( + key = arguments.key, + expiresIn = arguments.expiresIn, + contentDisposition = arguments.contentDisposition + ); + } + + // ---- internals -------------------------------------------------------- + + private string function $objectPath(required string key) { + local.k = REReplace(arguments.key, "^/+", ""); + return variables.usePathStyle ? "/" & variables.bucket & "/" & local.k : "/" & local.k; + } + + /** + * Issue a signed cfhttp request. Headers are copied into a plain struct + * before being attached one-by-one — never `attributeCollection=arguments`, + * which Adobe CF 2023/2025 reject on built-in tags. + */ + private struct function $request( + required string method, + required string key, + required struct headers, + any body = "", + string contentType = "", + boolean getAsBinary = false + ) { + local.targetUrl = "https://" & variables.signer.getHost() & $objectPath(arguments.key); + local.hdrs = {}; + for (local.name in arguments.headers) { + local.hdrs[local.name] = arguments.headers[local.name]; + } + + local.httpResult = ""; + cfhttp(method = arguments.method, url = local.targetUrl, result = "local.httpResult", getAsBinary = (arguments.getAsBinary ? "yes" : "auto"), timeout = 60) { + for (local.name in local.hdrs) { + cfhttpparam(type = "header", name = local.name, value = local.hdrs[local.name]); + } + if (Len(arguments.contentType)) { + cfhttpparam(type = "header", name = "Content-Type", value = arguments.contentType); + } + if (!IsSimpleValue(arguments.body) || Len(arguments.body)) { + cfhttpparam(type = "body", value = arguments.body); + } + } + return { + statusCode = ListFirst(local.httpResult.statusCode ?: "0", " "), + fileContent = local.httpResult.fileContent ?: "" + }; + } + +} diff --git a/vendor/wheels/tests/specs/storage/StorageSpec.cfc b/vendor/wheels/tests/specs/storage/StorageSpec.cfc new file mode 100644 index 0000000000..1b663da915 --- /dev/null +++ b/vendor/wheels/tests/specs/storage/StorageSpec.cfc @@ -0,0 +1,260 @@ +component extends="wheels.WheelsTest" { + + function run() { + + describe("Storage disk abstraction", function() { + + // ---- S3Signer (SigV4) ------------------------------------------- + + describe("S3Signer.presignGetUrl()", function() { + + it("reproduces the AWS-documented SigV4 presigned-GET test vector", function() { + // Official example from AWS "Authenticating Requests: Using Query + // Parameters (AWS Signature Version 4)". Pinning the published + // timestamp makes the signature deterministic. + var signer = new wheels.storage.S3Signer( + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + region = "us-east-1", + bucket = "examplebucket", + endpoint = "examplebucket.s3.amazonaws.com" + ); + + // NB: never name this local `url` — it is a CFML reserved scope and + // `expect(url)` would read the URL scope struct, not the return value + // (Anti-Pattern #11). Use `presigned` throughout this spec. + var presigned = signer.presignGetUrl( + key = "test.txt", + expiresIn = 86400, + amzDate = "20130524T000000Z" + ); + + expect(presigned).toInclude( + "X-Amz-Signature=aeeed9bbccd4d02ee5c0109b86d86835f995330da4c265957d157751f604d404" + ); + expect(presigned).toInclude("https://examplebucket.s3.amazonaws.com/test.txt?"); + expect(presigned).toInclude("X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20130524%2Fus-east-1%2Fs3%2Faws4_request"); + expect(presigned).toInclude("X-Amz-Expires=86400"); + }); + + it("preserves slashes in the object key path but encodes the credential", function() { + var signer = new wheels.storage.S3Signer( + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + region = "us-east-1", + bucket = "examplebucket", + endpoint = "examplebucket.s3.amazonaws.com" + ); + var presigned = signer.presignGetUrl(key = "reports/2026/q3.pdf", amzDate = "20130524T000000Z"); + expect(presigned).toInclude("/reports/2026/q3.pdf?"); + }); + + }); + + describe("S3Signer.signedHeaders()", function() { + + it("returns a SigV4 Authorization header plus the content/date headers", function() { + var signer = new wheels.storage.S3Signer( + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + region = "us-east-1", + bucket = "examplebucket" + ); + var headers = signer.signedHeaders(method = "GET", key = "test.txt", amzDate = "20130524T000000Z"); + + expect(headers).toHaveKey("Authorization"); + expect(headers).toHaveKey("x-amz-content-sha256"); + expect(headers).toHaveKey("x-amz-date"); + expect(headers.Authorization).toInclude("AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20130524/us-east-1/s3/aws4_request"); + expect(headers.Authorization).toInclude("SignedHeaders=host;x-amz-content-sha256;x-amz-date"); + expect(headers.Authorization).toInclude("Signature="); + }); + + }); + + // ---- LocalDisk -------------------------------------------------- + + describe("LocalDisk", function() { + + var ctx = {root = ""}; + + beforeEach(function() { + ctx.root = GetTempDirectory() & "wheels-storage-spec-" & CreateUUID(); + disk = new wheels.storage.drivers.LocalDisk(config = { + root = ctx.root, + urlPrefix = "/uploads", + signingKey = "a-test-signing-key-padded-to-32-bytes!!" + }); + }); + + afterEach(function() { + if (DirectoryExists(ctx.root)) { + DirectoryDelete(ctx.root, true); + } + }); + + it("stores, reports existence, reads back, and deletes an object", function() { + expect(disk.exists("a/b/hello.txt")).toBeFalse(); + + disk.put(key = "a/b/hello.txt", content = "hello world"); + expect(disk.exists("a/b/hello.txt")).toBeTrue(); + + var content = disk.get("a/b/hello.txt"); + expect(ToString(content)).toBe("hello world"); + + expect(disk.delete("a/b/hello.txt")).toBeTrue(); + expect(disk.exists("a/b/hello.txt")).toBeFalse(); + // Deleting an absent key reports false, never throws. + expect(disk.delete("a/b/hello.txt")).toBeFalse(); + }); + + it("throws Wheels.Storage.NotFound reading a missing key", function() { + expect(function() { + disk.get("nope.txt"); + }).toThrow("Wheels.Storage.NotFound"); + }); + + it("rejects path-traversal keys", function() { + expect(function() { + disk.put(key = "../escape.txt", content = "x"); + }).toThrow("Wheels.Storage.InvalidKey"); + }); + + it("builds a public url from the urlPrefix", function() { + expect(disk.url("a/b.png")).toBe("/uploads/a/b.png"); + }); + + it("builds a signed url whose token round-trips through verifySignature", function() { + var signed = disk.signedUrl(key = "a/b.png", expiresIn = 600); + expect(signed).toInclude("/uploads/a/b.png?expires="); + expect(signed).toInclude("signature="); + + var expires = ListFirst(ListLast(signed, "="), "&"); + var qs = ListLast(signed, "?"); + var sig = ReReplace(qs, ".*signature=([a-f0-9]+).*", "\1"); + var exp = Val(ReReplace(qs, ".*expires=([0-9]+).*", "\1")); + + expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = sig)).toBeTrue(); + // Tampering with the key invalidates the token. + expect(disk.verifySignature(key = "other.png", expires = exp, signature = sig)).toBeFalse(); + }); + + it("rejects an expired signed url", function() { + var signed = disk.signedUrl(key = "a/b.png", expiresIn = -10); + var qs = ListLast(signed, "?"); + var sig = ReReplace(qs, ".*signature=([a-f0-9]+).*", "\1"); + var exp = Val(ReReplace(qs, ".*expires=([0-9]+).*", "\1")); + expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = sig)).toBeFalse(); + }); + + it("requires a signingKey to produce a signed url", function() { + var unsigned = new wheels.storage.drivers.LocalDisk(config = {root = ctx.root, urlPrefix = "/u"}); + expect(function() { + unsigned.signedUrl(key = "a.png"); + }).toThrow("Wheels.Storage.MissingSigningKey"); + }); + + it("requires a non-empty root", function() { + expect(function() { + new wheels.storage.drivers.LocalDisk(config = {root = ""}); + }).toThrow("Wheels.Storage.InvalidConfiguration"); + }); + + }); + + // ---- S3Disk (non-network surface) ------------------------------- + + describe("S3Disk url helpers", function() { + + var s3 = ""; + beforeEach(function() { + s3 = new wheels.storage.drivers.S3Disk(config = { + bucket = "myapp-prod", + region = "us-east-1", + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + }); + }); + + it("builds a virtual-hosted public url", function() { + expect(s3.url("avatars/1.png")).toBe("https://myapp-prod.s3.us-east-1.amazonaws.com/avatars/1.png"); + }); + + it("delegates signedUrl to the SigV4 presigner", function() { + var presigned = s3.signedUrl(key = "avatars/1.png", expiresIn = 120); + expect(presigned).toInclude("https://myapp-prod.s3.us-east-1.amazonaws.com/avatars/1.png?"); + expect(presigned).toInclude("X-Amz-Algorithm=AWS4-HMAC-SHA256"); + expect(presigned).toInclude("X-Amz-Expires=120"); + expect(presigned).toInclude("X-Amz-Signature="); + }); + + it("requires bucket/region/credentials", function() { + expect(function() { + new wheels.storage.drivers.S3Disk(config = {bucket = "b", region = "us-east-1"}); + }).toThrow("Wheels.Storage.InvalidConfiguration"); + }); + + }); + + // ---- StorageManager --------------------------------------------- + + describe("StorageManager", function() { + + var manager = ""; + beforeEach(function() { + manager = new wheels.storage.StorageManager(config = { + default = "local", + disks = { + local = {driver = "local", root = GetTempDirectory() & "wheels-storage-mgr", urlPrefix = "/uploads"}, + s3 = { + driver = "s3", bucket = "myapp", region = "us-east-1", + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + } + } + }); + }); + + it("resolves the default disk when no name is given", function() { + var d = manager.disk(); + expect(d.url("x.png")).toBe("/uploads/x.png"); + }); + + it("resolves a named disk", function() { + var d = manager.disk("s3"); + expect(d.url("x.png")).toBe("https://myapp.s3.us-east-1.amazonaws.com/x.png"); + }); + + it("returns the same cached instance across calls", function() { + expect(manager.disk("local")).toBe(manager.disk("local")); + }); + + it("throws for an unknown disk name", function() { + expect(function() { + manager.disk("ftp"); + }).toThrow("Wheels.Storage.UnknownDisk"); + }); + + it("throws for a disk configured with an unknown driver", function() { + var bad = new wheels.storage.StorageManager(config = { + default = "weird", + disks = {weird = {driver = "gopher"}} + }); + expect(function() { + bad.disk(); + }).toThrow("Wheels.Storage.UnknownDriver"); + }); + + it("exposes default disk name and configured disk names", function() { + expect(manager.getDefaultDiskName()).toBe("local"); + expect(manager.diskNames()).toInclude("local"); + expect(manager.diskNames()).toInclude("s3"); + }); + + }); + + }); + + } + +} From 6eec28885c08213edb9859689710c9e2f763d62b Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Wed, 24 Jun 2026 05:51:27 -0700 Subject: [PATCH 2/3] fix(storage): harden S3 request failures, path encoding, and local signed-url binding Addresses the review on #3248. S3Disk treated a cfhttp connection failure as success. cfhttp does not set throwOnError, so a DNS/connection error returns a non-numeric status (e.g. "Connection Failure") whose Val() is 0, which the `< 300` guards read as OK: put() reported a stored object that was never written, get() returned the error body as the object, and exists()/delete() returned true. Centralize status interpretation in $assertSuccess and throw Wheels.Storage.RequestFailed on any non-2xx or unparseable status (put/get/delete). exists() now returns true on 2xx, false on 404, and throws on a connection failure / 5xx rather than reporting a definitive absence it cannot prove. $request preserves the raw status line for diagnostics instead of collapsing it via ListFirst. The wire path was built from the raw key while the signer signs the RFC3986-encoded canonical path, so keys containing spaces / reserved characters would fail with SignatureDoesNotMatch. Expose S3Signer.encodeKey() and route $objectPath through it so the request and public URLs are byte-identical to what was signed (mirrors the signer's canonicalUri for path-style vs virtual-hosted). LocalDisk.signedUrl() appended contentDisposition outside the HMAC, so a URL holder could alter the served disposition; it is now bound into the signed payload via $signaturePayload (an empty disposition reproduces the legacy "key|expires" payload, so already-issued URLs still verify). Replaced the mislabeled "constant-time-ish" CompareNoCase, which short-circuits on the first differing character, with a length-then-XOR-accumulate $secureEquals that does not leak the match length through timing. Adds 7 specs to StorageSpec.cfc: four asserting put/get/exists/delete throw on a refused connection (pointed at 127.0.0.1:1), one for public-url encoding, one for disposition binding, and one wrong-length-token guard. Storage bundle is 27/27 green on Lucee 7 and Adobe 2023. Signed-off-by: Peter Amiri Co-Authored-By: Claude Opus 4.8 --- vendor/wheels/storage/S3Signer.cfc | 13 ++++ vendor/wheels/storage/drivers/LocalDisk.cfc | 44 +++++++++-- vendor/wheels/storage/drivers/S3Disk.cfc | 66 ++++++++++++++--- .../tests/specs/storage/StorageSpec.cfc | 74 +++++++++++++++++++ 4 files changed, 181 insertions(+), 16 deletions(-) diff --git a/vendor/wheels/storage/S3Signer.cfc b/vendor/wheels/storage/S3Signer.cfc index fb4d599f2a..2a3a3a4a99 100644 --- a/vendor/wheels/storage/S3Signer.cfc +++ b/vendor/wheels/storage/S3Signer.cfc @@ -172,6 +172,19 @@ component output="false" { return variables.host; } + /** + * RFC3986-encode an object key for use as a request path (forward slashes + * preserved). The wire URL must use the same encoding the canonical request + * signs, or S3 returns SignatureDoesNotMatch for keys with spaces / reserved + * characters. Lets the disk build request/url paths that stay byte-identical + * to what was signed. + * + * @key Object key. + */ + public string function encodeKey(required string key) { + return $uriEncodePath(arguments.key); + } + // ---- internals -------------------------------------------------------- /** diff --git a/vendor/wheels/storage/drivers/LocalDisk.cfc b/vendor/wheels/storage/drivers/LocalDisk.cfc index 9c8196ce4e..dfe1a3f6e7 100644 --- a/vendor/wheels/storage/drivers/LocalDisk.cfc +++ b/vendor/wheels/storage/drivers/LocalDisk.cfc @@ -72,7 +72,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { ); } local.expiresAt = $epochSeconds() + arguments.expiresIn; - local.token = $sign(arguments.key & "|" & local.expiresAt); + local.token = $sign($signaturePayload(arguments.key, local.expiresAt, arguments.contentDisposition)); local.base = $joinUrl(variables.urlPrefix, arguments.key); local.qs = "expires=" & local.expiresAt & "&signature=" & local.token; if (Len(arguments.contentDisposition)) { @@ -87,17 +87,17 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { * @key The requested key. * @expires The epoch-seconds expiry carried in the URL. * @signature The token carried in the URL. + * @contentDisposition The disposition carried in the URL (bound into the token). */ - public boolean function verifySignature(required string key, required numeric expires, required string signature) { + public boolean function verifySignature(required string key, required numeric expires, required string signature, string contentDisposition = "") { if (!Len(variables.signingKey)) { return false; } if ($epochSeconds() > arguments.expires) { return false; } - local.expected = $sign(arguments.key & "|" & arguments.expires); - // Constant-time-ish compare on equal-length hex tokens. - return CompareNoCase(local.expected, arguments.signature) == 0; + local.expected = $sign($signaturePayload(arguments.key, arguments.expires, arguments.contentDisposition)); + return $secureEquals(local.expected, arguments.signature); } // ---- internals -------------------------------------------------------- @@ -106,6 +106,40 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { return LCase(HMac(arguments.message, variables.signingKey, "HMACSHA256", "UTF-8")); } + /** + * Canonical string the signed-URL HMAC covers. Binding the disposition in + * means a holder of a valid URL cannot alter the served Content-Disposition. + * Empty disposition reproduces the legacy "key|expires" payload, so URLs + * signed without one still verify. + */ + private string function $signaturePayload(required string key, required numeric expires, string contentDisposition = "") { + local.payload = arguments.key & "|" & arguments.expires; + if (Len(arguments.contentDisposition)) { + local.payload &= "|" & arguments.contentDisposition; + } + return local.payload; + } + + /** + * Length-independent equality for two hex tokens. Unlike CompareNoCase it + * does not short-circuit on the first differing character, so it does not + * leak how much of the token matched through timing. Both inputs are the + * fixed-width lowercase-hex output of $sign(), so a length mismatch can only + * be a forged/garbage token — comparing false there is correct. + */ + private boolean function $secureEquals(required string a, required string b) { + local.x = LCase(arguments.a); + local.y = LCase(arguments.b); + if (Len(local.x) != Len(local.y)) { + return false; + } + local.diff = 0; + for (local.i = 1; local.i <= Len(local.x); local.i++) { + local.diff = BitOr(local.diff, BitXor(Asc(Mid(local.x, local.i, 1)), Asc(Mid(local.y, local.i, 1)))); + } + return local.diff == 0; + } + private string function $resolve(required string key) { // Reject traversal — a key must stay inside root. local.clean = Replace(arguments.key, "\", "/", "all"); diff --git a/vendor/wheels/storage/drivers/S3Disk.cfc b/vendor/wheels/storage/drivers/S3Disk.cfc index 443a961ecd..8190ee49ff 100644 --- a/vendor/wheels/storage/drivers/S3Disk.cfc +++ b/vendor/wheels/storage/drivers/S3Disk.cfc @@ -43,9 +43,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { public any function put(required string key, required any content, string contentType = "application/octet-stream", string visibility = "") { local.headers = variables.signer.signedHeaders(method = "PUT", key = arguments.key, payload = arguments.content); local.result = $request(method = "PUT", key = arguments.key, headers = local.headers, body = arguments.content, contentType = arguments.contentType); - if (Val(local.result.statusCode) >= 300) { - throw(type = "Wheels.Storage.RequestFailed", message = "S3 PUT failed for [#arguments.key#]: #local.result.statusCode#."); - } + $assertSuccess(result = local.result, method = "PUT", key = arguments.key); return arguments.key; } @@ -55,23 +53,35 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { if (Val(local.result.statusCode) == 404) { throw(type = "Wheels.Storage.NotFound", message = "No object stored at key [#arguments.key#]."); } - if (Val(local.result.statusCode) >= 300) { - throw(type = "Wheels.Storage.RequestFailed", message = "S3 GET failed for [#arguments.key#]: #local.result.statusCode#."); - } + $assertSuccess(result = local.result, method = "GET", key = arguments.key); return local.result.fileContent; } public boolean function exists(required string key) { local.headers = variables.signer.signedHeaders(method = "HEAD", key = arguments.key); local.result = $request(method = "HEAD", key = arguments.key, headers = local.headers); - return Val(local.result.statusCode) < 300; + local.code = Val(local.result.statusCode); + if (local.code >= 200 && local.code < 300) { + return true; + } + if (local.code == 404) { + return false; + } + // A connection failure or 5xx is NOT "the object is absent" — reporting + // false there would be a silent failure, so surface it instead. + throw( + type = "Wheels.Storage.RequestFailed", + message = "S3 HEAD failed for [#arguments.key#]: #$statusDetail(local.result)#." + ); } public boolean function delete(required string key) { local.headers = variables.signer.signedHeaders(method = "DELETE", key = arguments.key); local.result = $request(method = "DELETE", key = arguments.key, headers = local.headers); - // S3 DELETE is idempotent — 204 whether or not the object existed. - return Val(local.result.statusCode) < 300; + // S3 DELETE is idempotent — 2xx whether or not the object existed — but a + // connection failure or 5xx must not masquerade as a successful delete. + $assertSuccess(result = local.result, method = "DELETE", key = arguments.key); + return true; } public string function url(required string key) { @@ -90,7 +100,38 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { private string function $objectPath(required string key) { local.k = REReplace(arguments.key, "^/+", ""); - return variables.usePathStyle ? "/" & variables.bucket & "/" & local.k : "/" & local.k; + // Encode through the signer so the wire path is byte-identical to the + // canonical path the SigV4 signature covers — otherwise S3 rejects keys + // containing spaces / reserved characters with SignatureDoesNotMatch. + // Mirrors the signer's canonicalUri (path-style prefixes the bucket). + return variables.usePathStyle + ? "/" & variables.signer.encodeKey(variables.bucket & "/" & local.k) + : "/" & variables.signer.encodeKey(local.k); + } + + /** + * Throw `Wheels.Storage.RequestFailed` unless the request returned a 2xx + * status. `cfhttp` does not set `throwOnError`, so a DNS/connection failure + * does not throw — it returns a non-numeric status (e.g. "Connection + * Failure") whose `Val()` is 0. The `< 200` guard catches that path too; + * without it a failed request would read as success and silently lose data. + */ + private void function $assertSuccess(required struct result, required string method, required string key) { + local.code = Val(arguments.result.statusCode); + if (local.code < 200 || local.code >= 300) { + throw( + type = "Wheels.Storage.RequestFailed", + message = "S3 #arguments.method# failed for [#arguments.key#]: #$statusDetail(arguments.result)#." + ); + } + } + + /** + * Human-readable status for error messages — the raw status line, or an + * explicit note when the request produced none (connection failure). + */ + private string function $statusDetail(required struct result) { + return Len(arguments.result.statusCode) ? arguments.result.statusCode : "no response (connection failure)"; } /** @@ -124,8 +165,11 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { cfhttpparam(type = "body", value = arguments.body); } } + // Preserve the raw status line — callers extract the numeric code with + // Val() (0 for a non-numeric connection-failure status) and use the full + // string for diagnostics. return { - statusCode = ListFirst(local.httpResult.statusCode ?: "0", " "), + statusCode = local.httpResult.statusCode ?: "", fileContent = local.httpResult.fileContent ?: "" }; } diff --git a/vendor/wheels/tests/specs/storage/StorageSpec.cfc b/vendor/wheels/tests/specs/storage/StorageSpec.cfc index 1b663da915..86908443c6 100644 --- a/vendor/wheels/tests/specs/storage/StorageSpec.cfc +++ b/vendor/wheels/tests/specs/storage/StorageSpec.cfc @@ -147,6 +147,26 @@ component extends="wheels.WheelsTest" { expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = sig)).toBeFalse(); }); + it("binds contentDisposition into the signed-url token", function() { + var signed = disk.signedUrl(key = "a/b.png", expiresIn = 600, contentDisposition = "attachment; filename=safe.png"); + var qs = ListLast(signed, "?"); + var sig = ReReplace(qs, ".*signature=([a-f0-9]+).*", "\1"); + var exp = Val(ReReplace(qs, ".*expires=([0-9]+).*", "\1")); + + // Verifying with the same disposition succeeds... + expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = sig, contentDisposition = "attachment; filename=safe.png")).toBeTrue(); + // ...but altering the disposition a holder was granted must invalidate the token. + expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = sig, contentDisposition = "attachment; filename=evil.exe")).toBeFalse(); + }); + + it("rejects a wrong-length signature without erroring", function() { + var signed = disk.signedUrl(key = "a/b.png", expiresIn = 600); + var qs = ListLast(signed, "?"); + var exp = Val(ReReplace(qs, ".*expires=([0-9]+).*", "\1")); + // A truncated token must compare false, never throw (length-mismatch path). + expect(disk.verifySignature(key = "a/b.png", expires = exp, signature = "deadbeef")).toBeFalse(); + }); + it("requires a signingKey to produce a signed url", function() { var unsigned = new wheels.storage.drivers.LocalDisk(config = {root = ctx.root, urlPrefix = "/u"}); expect(function() { @@ -180,6 +200,15 @@ component extends="wheels.WheelsTest" { expect(s3.url("avatars/1.png")).toBe("https://myapp-prod.s3.us-east-1.amazonaws.com/avatars/1.png"); }); + it("rfc3986-encodes spaces in the public url so it matches the signed wire path", function() { + // The signer signs the encoded canonical path, but the request URL + // and public url are built from the raw key. They must use the same + // encoding or S3 returns SignatureDoesNotMatch for keys containing + // spaces / reserved characters. + expect(s3.url("my docs/q3 report.pdf")) + .toBe("https://myapp-prod.s3.us-east-1.amazonaws.com/my%20docs/q3%20report.pdf"); + }); + it("delegates signedUrl to the SigV4 presigner", function() { var presigned = s3.signedUrl(key = "avatars/1.png", expiresIn = 120); expect(presigned).toInclude("https://myapp-prod.s3.us-east-1.amazonaws.com/avatars/1.png?"); @@ -196,6 +225,51 @@ component extends="wheels.WheelsTest" { }); + describe("S3Disk request failures", function() { + + // Point the disk at a closed local port so every request hits the + // cfhttp connection-failure path — hermetic and fast (no DNS, no + // external network; the TCP connect is refused immediately). cfhttp + // does not set throwOnError, so a failed request returns a + // non-numeric status whose Val() is 0. The driver MUST treat that as + // a failure, never as a stored/served object (silent data loss). + var s3down = ""; + beforeEach(function() { + s3down = new wheels.storage.drivers.S3Disk(config = { + bucket = "myapp", + region = "us-east-1", + accessKeyId = "AKIAIOSFODNN7EXAMPLE", + secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + endpoint = "127.0.0.1:1" + }); + }); + + it("throws instead of silently succeeding when put() cannot reach S3", function() { + expect(function() { + s3down.put(key = "x.txt", content = "data"); + }).toThrow("Wheels.Storage.RequestFailed"); + }); + + it("throws instead of returning a bogus body when get() cannot reach S3", function() { + expect(function() { + s3down.get(key = "x.txt"); + }).toThrow("Wheels.Storage.RequestFailed"); + }); + + it("throws instead of reporting existence when exists() cannot reach S3", function() { + expect(function() { + s3down.exists(key = "x.txt"); + }).toThrow("Wheels.Storage.RequestFailed"); + }); + + it("throws instead of reporting a successful delete when delete() cannot reach S3", function() { + expect(function() { + s3down.delete(key = "x.txt"); + }).toThrow("Wheels.Storage.RequestFailed"); + }); + + }); + // ---- StorageManager --------------------------------------------- describe("StorageManager", function() { From ed7660e07ddfab7d470a3a9f85391884a524d67b Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Wed, 24 Jun 2026 06:17:31 -0700 Subject: [PATCH 3/3] fix(storage): bump interface-count guard and make S3 request timeout configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The storage PR adds vendor/wheels/interfaces/StorageDiskInterface.cfc — the 24th interface file — which tripped the structural guard in InterfaceCompilationSpec ("finds exactly 23 interface files", now 24). Bump the expected count and note that it tracks the wheels.interfaces.* contract set. The full CI suite caught this; the storage-only bundle run and the diff-based bot review did not, since the guard lives in tests/specs/interfaces. Also make the S3 request timeout configurable (config.timeout, default 60s) and use a short timeout in the connection-failure specs. In CI the get() failure spec waited the full 60s cfhttp timeout (only GET hung; PUT/HEAD/DELETE were refused instantly), adding a minute to every core run; capping it keeps the suite fast without changing the production default. Storage + interfaces bundles: 30/30 green on Lucee 7 and Adobe 2023. Signed-off-by: Peter Amiri Co-Authored-By: Claude Opus 4.8 --- vendor/wheels/storage/drivers/S3Disk.cfc | 5 +++-- .../tests/specs/interfaces/InterfaceCompilationSpec.cfc | 5 +++-- vendor/wheels/tests/specs/storage/StorageSpec.cfc | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/vendor/wheels/storage/drivers/S3Disk.cfc b/vendor/wheels/storage/drivers/S3Disk.cfc index 8190ee49ff..7edc307bb3 100644 --- a/vendor/wheels/storage/drivers/S3Disk.cfc +++ b/vendor/wheels/storage/drivers/S3Disk.cfc @@ -12,7 +12,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { /** * @config Disk config: { bucket, region, accessKeyId, secretAccessKey, - * visibility="private", endpoint="", usePathStyle=false }. + * visibility="private", endpoint="", usePathStyle=false, timeout=60 }. */ public S3Disk function init(required struct config) { for (local.required in ["bucket", "region", "accessKeyId", "secretAccessKey"]) { @@ -28,6 +28,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { variables.visibility = StructKeyExists(arguments.config, "visibility") ? arguments.config.visibility : "private"; variables.usePathStyle = StructKeyExists(arguments.config, "usePathStyle") ? arguments.config.usePathStyle : false; variables.endpoint = StructKeyExists(arguments.config, "endpoint") ? arguments.config.endpoint : ""; + variables.timeout = StructKeyExists(arguments.config, "timeout") ? arguments.config.timeout : 60; variables.signer = new wheels.storage.S3Signer( accessKeyId = arguments.config.accessKeyId, @@ -154,7 +155,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" { } local.httpResult = ""; - cfhttp(method = arguments.method, url = local.targetUrl, result = "local.httpResult", getAsBinary = (arguments.getAsBinary ? "yes" : "auto"), timeout = 60) { + cfhttp(method = arguments.method, url = local.targetUrl, result = "local.httpResult", getAsBinary = (arguments.getAsBinary ? "yes" : "auto"), timeout = variables.timeout) { for (local.name in local.hdrs) { cfhttpparam(type = "header", name = local.name, value = local.hdrs[local.name]); } diff --git a/vendor/wheels/tests/specs/interfaces/InterfaceCompilationSpec.cfc b/vendor/wheels/tests/specs/interfaces/InterfaceCompilationSpec.cfc index 96f9bad87e..b1384d41b2 100644 --- a/vendor/wheels/tests/specs/interfaces/InterfaceCompilationSpec.cfc +++ b/vendor/wheels/tests/specs/interfaces/InterfaceCompilationSpec.cfc @@ -32,7 +32,7 @@ component extends="wheels.WheelsTest" { } }); - it("finds exactly 23 interface files", () => { + it("finds exactly 24 interface files", () => { var interfaceDir = expandPath("/wheels/interfaces"); var files = directoryList( path=interfaceDir, @@ -40,7 +40,8 @@ component extends="wheels.WheelsTest" { filter="*.cfc", type="file" ); - expect(arrayLen(files)).toBe(23); + // Bump when a wheels.interfaces.* contract is added or removed. + expect(arrayLen(files)).toBe(24); }); it("re-export wrappers extend their original interfaces", () => { diff --git a/vendor/wheels/tests/specs/storage/StorageSpec.cfc b/vendor/wheels/tests/specs/storage/StorageSpec.cfc index 86908443c6..44b130f89d 100644 --- a/vendor/wheels/tests/specs/storage/StorageSpec.cfc +++ b/vendor/wheels/tests/specs/storage/StorageSpec.cfc @@ -240,7 +240,8 @@ component extends="wheels.WheelsTest" { region = "us-east-1", accessKeyId = "AKIAIOSFODNN7EXAMPLE", secretAccessKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", - endpoint = "127.0.0.1:1" + endpoint = "127.0.0.1:1", + timeout = 5 }); });