fix(storage): flip remaining S1 S3 S4 S5 S6 storage contracts

changelog.d/storage-hardener-s1-s3-s6.changed.md

@@ -0,0 +1,5 @@
+- LocalDisk `$resolve` throws `Wheels.Storage.InvalidKey` for empty and slash-only keys instead of concatenating onto `root`
+- LocalDisk `url()` and `signedUrl()` RFC3986-encode object keys the same way S3 does (spaces and reserved characters)
+- `S3Disk.delete()` HEADs first and returns false when the object is missing. An existing object still deletes and returns true
+- `S3Disk.put()` sends a signed `x-amz-acl` (`public` maps to `public-read`; default and `private` send `private`)
+- `signedUrl()` / `presignGetUrl()` throw `Wheels.Storage.InvalidExpiresIn` when `expiresIn` is outside `1..604800`

changelog.d/storage-hardener-s1-s5.security.md

@@ -0,0 +1,2 @@
+- LocalDisk refuses empty and slash-only keys so `put()` cannot write the disk root
+- S3 `put()` honours visibility by sending a signed `x-amz-acl` header

vendor/wheels/storage/S3Signer.cfc

@@ -71,6 +71,7 @@ component output="false" {
 		string contentDisposition = "",
 		string amzDate = ""
 	) {
+		$assertExpiresIn(arguments.expiresIn);
 		local.amzDate = Len(arguments.amzDate) ? arguments.amzDate : $amzNow();
 		local.dateStamp = Left(local.amzDate, 8);
 		local.credentialScope = local.dateStamp & "/" & variables.region & "/" & variables.service & "/aws4_request";
@@ -120,14 +121,16 @@ component output="false" {
 	 * @payload Request body (binary or string); empty for GET/DELETE/HEAD.
 	 * @amzDate Optional deterministic timestamp override.
 	 * @range Optional Range header value (e.g. "bytes=0-9"). Empty keeps the current signed header set.
+	 * @acl Optional x-amz-acl value. Empty keeps the current signed header set (S8 vector).
 	 * @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 = "",
-		string range = ""
+		string range = "",
+		string acl = ""
 	) {
 		local.amzDate = Len(arguments.amzDate) ? arguments.amzDate : $amzNow();
 		local.dateStamp = Left(local.amzDate, 8);
@@ -139,19 +142,21 @@ component output="false" {
 			? "/" & $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";
-
+		// Optional range / acl insert in code-point order so the S8 no-acl vector
+		// stays byte-identical when both are empty.
+		local.canonicalHeaders = "host:" & variables.host & Chr(10);
+		local.signedHeaderList = "host";
 		if (Len(arguments.range)) {
-			local.canonicalHeaders = "host:" & variables.host & Chr(10)
-				& "range:" & arguments.range & Chr(10)
-				& "x-amz-content-sha256:" & local.payloadHash & Chr(10)
-				& "x-amz-date:" & local.amzDate & Chr(10);
-			local.signedHeaderList = "host;range;x-amz-content-sha256;x-amz-date";
+			local.canonicalHeaders &= "range:" & arguments.range & Chr(10);
+			local.signedHeaderList &= ";range";
+		}
+		if (Len(arguments.acl)) {
+			local.canonicalHeaders &= "x-amz-acl:" & arguments.acl & Chr(10);
+			local.signedHeaderList &= ";x-amz-acl";
 		}
+		local.canonicalHeaders &= "x-amz-content-sha256:" & local.payloadHash & Chr(10)
+			& "x-amz-date:" & local.amzDate & Chr(10);
+		local.signedHeaderList &= ";x-amz-content-sha256;x-amz-date";
 
 		local.canonicalRequest = UCase(arguments.method) & Chr(10)
 			& local.canonicalUri & Chr(10)
@@ -167,21 +172,19 @@ component output="false" {
 			& "SignedHeaders=" & local.signedHeaderList & ", "
 			& "Signature=" & local.signature;
 
-		if (Len(arguments.range)) {
-			return {
-				"Authorization" = local.authorization,
-				"x-amz-content-sha256" = local.payloadHash,
-				"x-amz-date" = local.amzDate,
-				"Host" = variables.host,
-				"Range" = arguments.range
-			};
-		}
-		return {
+		local.headers = {
 			"Authorization" = local.authorization,
 			"x-amz-content-sha256" = local.payloadHash,
 			"x-amz-date" = local.amzDate,
 			"Host" = variables.host
 		};
+		if (Len(arguments.range)) {
+			local.headers["Range"] = arguments.range;
+		}
+		if (Len(arguments.acl)) {
+			local.headers["x-amz-acl"] = arguments.acl;
+		}
+		return local.headers;
 	}
 
 	/**
@@ -293,6 +296,15 @@ component output="false" {
 		return Replace($uriEncodeSegment(arguments.key), "%2F", "/", "all");
 	}
 
+	private void function $assertExpiresIn(required numeric expiresIn) {
+		if (arguments.expiresIn < 1 || arguments.expiresIn > 604800) {
+			throw(
+				type = "Wheels.Storage.InvalidExpiresIn",
+				message = "signedUrl expiresIn must be between 1 and 604800 seconds (got #arguments.expiresIn#)."
+			);
+		}
+	}
+
 	/**
 	 * Current UTC time as an ISO8601 basic timestamp ("yyyymmddTHHnnssZ").
 	 */

vendor/wheels/storage/drivers/LocalDisk.cfc

@@ -73,6 +73,7 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" {
 	}
 
 	public string function signedUrl(required string key, numeric expiresIn = 300, string contentDisposition = "") {
+		$assertExpiresIn(arguments.expiresIn);
 		if (!Len(variables.signingKey)) {
 			throw(
 				type = "Wheels.Storage.MissingSigningKey",
@@ -157,6 +158,14 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" {
 				message = "Storage key [#arguments.key#] must not contain '..'."
 			);
 		}
+		// Empty or slash-only keys resolve to root itself. Throw rather than
+		// let put() write the disk root or get() treat the directory as NotFound.
+		if (!Len(REReplace(local.clean, "/", "", "all"))) {
+			throw(
+				type = "Wheels.Storage.InvalidKey",
+				message = "Storage key [#arguments.key#] must not be empty or slash-only."
+			);
+		}
 		return variables.root & "/" & local.clean;
 	}
 
@@ -178,12 +187,28 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" {
 	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;
+		return local.p & "/" & $uriEncodePath(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");
+		local.encoded = CreateObject("java", "java.net.URLEncoder").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;
+	}
+
+	private string function $uriEncodePath(required string key) {
+		return Replace($uriEncode(arguments.key), "%2F", "/", "all");
+	}
+
+	private void function $assertExpiresIn(required numeric expiresIn) {
+		if (arguments.expiresIn < 1 || arguments.expiresIn > 604800) {
+			throw(
+				type = "Wheels.Storage.InvalidExpiresIn",
+				message = "signedUrl expiresIn must be between 1 and 604800 seconds (got #arguments.expiresIn#)."
+			);
+		}
 	}
 
 	private numeric function $epochSeconds() {

vendor/wheels/storage/drivers/S3Disk.cfc

@@ -42,7 +42,14 @@ 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.visibility = Len(arguments.visibility) ? arguments.visibility : variables.visibility;
+		local.acl = $aclFromVisibility(local.visibility);
+		local.headers = variables.signer.signedHeaders(
+			method = "PUT",
+			key = arguments.key,
+			payload = arguments.content,
+			acl = local.acl
+		);
 		local.result = $request(method = "PUT", key = arguments.key, headers = local.headers, body = arguments.content, contentType = arguments.contentType);
 		$assertSuccess(result = local.result, method = "PUT", key = arguments.key);
 		return arguments.key;
@@ -77,10 +84,11 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" {
 	}
 
 	public boolean function delete(required string key) {
+		if (!exists(arguments.key)) {
+			return false;
+		}
 		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 — 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;
 	}
@@ -99,6 +107,17 @@ component implements="wheels.interfaces.StorageDiskInterface" output="false" {
 
 	// ---- internals --------------------------------------------------------
 
+	private string function $aclFromVisibility(required string visibility) {
+		local.v = LCase(Trim(arguments.visibility));
+		if (local.v == "public") {
+			return "public-read";
+		}
+		if (local.v == "private" || !Len(local.v)) {
+			return "private";
+		}
+		return arguments.visibility;
+	}
+
 	private string function $objectPath(required string key) {
 		local.k = REReplace(arguments.key, "^/+", "");
 		// Encode through the signer so the wire path is byte-identical to the

vendor/wheels/tests/_assets/storage/S3DiskDeleteStub.cfc

@@ -1,5 +1,16 @@
 component extends="wheels.storage.drivers.S3Disk" {
 
+	variables.objects = {};
+	variables.lastRequest = {};
+
+	public void function seed(required string key) {
+		variables.objects[arguments.key] = true;
+	}
+
+	public struct function lastRequest() {
+		return variables.lastRequest;
+	}
+
 	public struct function $request(
 		required string method,
 		required string key,
@@ -8,7 +19,32 @@ component extends="wheels.storage.drivers.S3Disk" {
 		string contentType = "",
 		boolean getAsBinary = false
 	) {
-		return {statusCode = "204 No Content", fileContent = ""};
+		local.copiedHeaders = {};
+		for (local.name in arguments.headers) {
+			local.copiedHeaders[local.name] = arguments.headers[local.name];
+		}
+		variables.lastRequest = {
+			method = arguments.method,
+			key = arguments.key,
+			headers = local.copiedHeaders,
+			contentType = arguments.contentType
+		};
+
+		if (arguments.method == "HEAD") {
+			if (StructKeyExists(variables.objects, arguments.key)) {
+				return {statusCode = "200 OK", fileContent = ""};
+			}
+			return {statusCode = "404 Not Found", fileContent = ""};
+		}
+		if (arguments.method == "DELETE") {
+			StructDelete(variables.objects, arguments.key);
+			return {statusCode = "204 No Content", fileContent = ""};
+		}
+		if (arguments.method == "PUT") {
+			variables.objects[arguments.key] = true;
+			return {statusCode = "200 OK", fileContent = ""};
+		}
+		return {statusCode = "200 OK", fileContent = ""};
 	}
 
 }