fix(storage): address the DSN bucket through the endpoint and refuse a DSN that names no device

[abnegate]

Fixes two defects in OpenRuntimes\Executor\StorageFactory found while porting the WAL shipper to Rust: DAT-2206 and DAT-2205.

DAT-2206: a DSN that names no device silently became a Local device

getDevice() caught every parse failure, logged a warning and fell through to default: return new Local($root). So an unparseable DSN, or any scheme outside s3/dospaces/backblaze/linode/wasabi (including awss3, which DeviceType defines), produced a Local device rooted at /. Every caller then wrote inside its own container and reported success: the WAL shipper "shipped" segments to its own ephemeral filesystem and published a heartbeat next to them, backup Jobs wrote dumps into their own container, and the health check that reads the archive built the same Local device and answered from the controller's filesystem. Full green, nothing durable.

getDevice() now throws InvalidArgumentException for both cases, and the scheme is resolved through an exhaustive match over DeviceType, so a device added upstream cannot silently fall back to Local again.

An empty or null connection keeps its meaning: "no object storage is configured for this deployment". It short-circuits to Local before any parsing. Callers that depend on it (_EDGE_CONNECTIONS_BACKUPS_STORAGE unset, OPR_EXECUTOR_CONNECTION_STORAGE unset) are unaffected.

DAT-2205: the port was dropped and the bucket never reached a request

getDevice() read $dsn->getHost() and never $dsn->getPort(), so the shape the edge README documents, s3://user:password@host.docker.internal:30000/storage?insecure=true, resolved to http://host.docker.internal on port 80.

The bucket was folded into the device root (#242). That reaches a request only through getPath(). S3::write(), read(), exists()/getInfo() and delete() build their URI from the object path alone, so a caller that passes a whole object key, which is what every backup and WAL caller does, addressed the store with no bucket anywhere and landed in whatever bucket the first segment of the key happened to name.

utopia-php/storage 4.0.2 (utopia-php/storage#107) parses the endpoint URL, keeps its port and path, and signs that path in the SigV4 canonical request. The bucket therefore belongs in the endpoint, where it reaches every request whichever method builds the URI:

• endpoint = (insecure ? http : https)://host[:port]
• path-style: object URL = {endpoint}/{bucket}/{objectKey}
• virtual-hosted (bucket already the leading label of the host, as with dospaces://): object URL = {endpoint}/{objectKey}, bucket not repeated
• an explicit ?url= keeps its current meaning and wins

The root is left alone, so a caller that goes through getPath() resolves to exactly the object it did before: the bucket moved from the front of the root to the end of the endpoint, and the concatenation is unchanged.

This raises the utopia-php/storage floor to ^4.0.2. On 4.0.0 an endpoint path is neither signed nor stripped from the Host header, so the fix silently produces SignatureDoesNotMatch.

Resolved URLs, before and after

Object key wal-archive/db-abc/000000010000000000000001 in every row. "Before" values were read off the reverted implementation, not predicted.

DSN	Before	After
s3://key:secret@host.internal:30000/storage?insecure=true	http://host.internal/wal-archive/db-abc/0000…01	http://host.internal:30000/storage/wal-archive/db-abc/0000…01
s3://key:secret@s3.amazonaws.com/backups?region=us-east-1	https://s3.amazonaws.com/wal-archive/db-abc/0000…01	https://s3.amazonaws.com/backups/wal-archive/db-abc/0000…01
dospaces://key:secret@fra1.digitaloceanspaces.com/appwrite-backups?region=fra1	https://appwrite-backups.fra1.digitaloceanspaces.com/wal-archive/db-abc/0000…01	unchanged
s3://key:secret@mybucket.s3.us-east-1.amazonaws.com/mybucket?region=us-east-1	https://mybucket.s3.us-east-1.amazonaws.com/wal-archive/db-abc/0000…01	unchanged
s3://…?url=http://127.0.0.1:3900/mybucket	http://127.0.0.1:3900/mybucket/wal-archive/db-abc/0000…01	unchanged
getDevice('/build-cache', 's3://…@minio…/storage?insecure=true')->getPath(…)	http://minio…/storage/build-cache/cache-key/lz4-b1M/stores.sqfs	unchanged
s3://user:password@minio.edge.svc.cluster.local/storage?insecure=true	http://minio…/wal-archive/db-abc/0000…01	http://minio…/storage/wal-archive/db-abc/0000…01

Production (dospaces://) is byte-identical, as is every path that goes through getPath(), which covers the executor's build cache and edge's static-site reader.

The last row does change. The local and CI MinIO connection names bucket storage, and today that bucket is dropped: the object key's leading segment (wal-archive/, backups/) is what MinIO takes as the bucket, which is why the local chart creates all three. After this change those objects live under bucket storage, at the same keys. Both buckets are created by the chart and the clusters are rebuilt per run, so nothing is stranded, but it means the controller, the backup/restore Jobs and the WAL shipper have to be upgraded together, exactly as DAT-2205 requires. Worth noting that today's split is already incoherent in that deployment: Storage::upload() writes at {endpoint}/{key} while the static-site reader goes through getPath() and reads at {endpoint}/storage/{key}. This change converges them.

Why not make S3 honour its root instead

DAT-2205 offers that alternative. It is the wrong place. Device::getPath() is defined as root . '/' . filename and callers pass its result straight into write(); if write() also applied the root, every existing consumer (server-ce, cloud, edge, this repo's build cache) would double it. Fixing it in utopia-php/storage therefore means a major version and a coordinated upgrade of four repositories, to reach a place where root and bucket are still conflated. The endpoint already models "the thing every object key hangs off", 4.0.2 signs it correctly, and no change outside this repository is needed.

Tests

tests/unit/Executor/StorageFactoryTest.php:

• testConnectionResolvesToObjectUrl pins the exact object URL for all seven DSNs in the table above, by driving the real device through a recording PSR-18 client (the seam S3 already takes) and asserting the URI of the request it hands over. The Rust shipper carries the mirror of this table.
• testPathStyleConnectionPutsBucketAndPortOnTheWire binds a local listener, has a child process write through a real cURL client, and asserts the request line and Host header off the socket. It proves the recording client is not flattering the code, and that the SigV4 signature covers the bucket path.
• testRootedDeviceAddressesTheSameObjectThroughGetPath pins the build-cache round trip that must not move.
• testUnusableConnectionIsRefused covers no host, no scheme, unparseable, unknown scheme, misspelled scheme.
• testUnconfiguredConnectionResolvesToLocalDevice pins the deliberate empty-DSN meaning.
• testDeviceTypeFollowsTheScheme walks every DeviceType, and catches awss3 resolving to Local.

Seen red: reverted StorageFactory to its main implementation, keeping only the client parameter so the doubled client still reaches the device, and confirmed 11 of the 17 failed with the "Before" URLs above. The 6 that stayed green are the byte-identical rows, which is the intended result.

composer test:unit (29 tests, 94 assertions), composer format:check, composer analyze, composer refactor:check all pass.

What a deployment has to do about it

Any deployed s3:// DSN whose path names a bucket starts addressing a different object, because that is the defect. Two consumers of this factory are in that position, and both are the same in-cluster MinIO connection:

• _EDGE_CONNECTIONS_BACKUPS_STORAGE = s3://…@minio…/storage?insecure=true, keys wal-archive/{id}/… and backups/{id}/…, so the bucket is wal-archive / backups today and becomes storage with those as key prefixes.
• _EDGE_CONNECTIONS_STORAGE / _APP_CONNECTIONS_STORAGE = fra=s3://…@minio…/storage?insecure=true, keys /storage/functions/… and /storage/builds/…, so the bucket is storage today and stays storage with storage/ added to the key.

A deployment that wants today's addressing back does not need the objects moved. Drop the bucket from the DSN path. s3://user:password@minio.edge.svc.cluster.local?insecure=true resolves to a bare endpoint, the object key's leading segment keeps naming the bucket exactly as it does now, and every existing object stays where it is. That is covered by the path-style without a port and virtual-hosted rows above: a DSN with no bucket in its path is untouched by this change.

So the migration is per deployment and opt in: keep the bucket in the DSN and the bucket reaches the request (and the objects move once), or take it out and nothing changes. dospaces:// needs neither.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

The PR corrects storage DSN resolution so invalid schemes are rejected and S3-compatible endpoints retain their port and bucket path.

• Moves path-style buckets from the device root into the endpoint.
• Resolves every supported storage scheme exhaustively.
• Raises utopia-php/storage to 4.0.2 and adds request-level regression coverage.
• Documents the revised DSN behavior and migration options.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/Executor/StorageFactory.php	Rejects unusable DSNs, maps all supported device types explicitly, and constructs S3 endpoints with the configured port and bucket.
tests/unit/Executor/StorageFactoryTest.php	Adds broad coverage for resolved object URLs, real HTTP request addressing, rooted paths, invalid connections, and every supported device type.
composer.json	Raises the storage dependency floor to the endpoint-path-aware 4.0.2 release.
composer.lock	Resolves storage 4.0.2 and its updated transitive dependency set.
README.md	Documents path-style and explicit endpoint DSNs, port handling, bucket placement, and invalid-DSN rejection.

_{Reviews (3): Last reviewed commit: "fix(storage): keep file:// local, apply ..." | Re-trigger Greptile}